From cf779f592f046c498432a832d285a350bad295ff Mon Sep 17 00:00:00 2001 From: Aaron Oman Date: Mon, 14 Dec 2020 13:11:20 -0800 Subject: [PATCH 1/4] Validate zero modules Adds two private functions in 'internal/module_config': - func (cfg ModuleConfig) collectMissing() []string - func findMissing(reflect.Value, string, string, []string) These are unexported functions that aid in introspecting a datastructure to see if it has all its expected data. These functions are implicitly invoked via: - func LoadModuleConfig(string) (ModuleConfig, error) If there are errors then the program aborts with appropriate output. --- go.mod | 1 + go.sum | 3 + internal/config/moduleconfig/module_config.go | 100 ++++++++++++++++++ 3 files changed, 104 insertions(+) diff --git a/go.mod b/go.mod index 8ab21e58f..33a65a2ce 100644 --- a/go.mod +++ b/go.mod @@ -10,6 +10,7 @@ require ( github.com/google/uuid v1.1.1 github.com/hashicorp/go-getter v1.4.2-0.20200106182914-9813cbd4eb02 github.com/hashicorp/terraform v0.12.26 + github.com/iancoleman/strcase v0.1.2 github.com/juju/ansiterm v0.0.0-20180109212912-720a0952cc2a // indirect github.com/k0kubun/colorstring v0.0.0-20150214042306-9440f1994b88 // indirect github.com/k0kubun/pp v3.0.1+incompatible diff --git a/go.sum b/go.sum index 88cf6e659..28bd08b05 100644 --- a/go.sum +++ b/go.sum @@ -204,6 +204,8 @@ github.com/hashicorp/terraform-config-inspect v0.0.0-20191212124732-c6ae6269b9d7 github.com/hashicorp/terraform-svchost v0.0.0-20191011084731-65d371908596/go.mod h1:kNDNcF7sN4DocDLBkQYz73HGKwN1ANB1blq4lIYLYvg= github.com/hashicorp/vault v0.10.4/go.mod h1:KfSyffbKxoVyspOdlaGVjIuwLobi07qD1bAbosPMpP0= github.com/hashicorp/yamux v0.0.0-20180604194846-3520598351bb/go.mod h1:+NfK9FKeTrX5uv1uIXGdwYDTeHna2qgaIlx54MXqjAM= +github.com/iancoleman/strcase v0.1.2 h1:gnomlvw9tnV3ITTAxzKSgTF+8kFWcU/f+TgttpXGz1U= +github.com/iancoleman/strcase v0.1.2/go.mod h1:SK73tn/9oHe+/Y0h39VT4UCxmurVJkR5NA7kMEAOgSE= github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= github.com/jhump/protoreflect v1.6.0/go.mod h1:eaTn3RZAmMBcV0fifFvlm6VHNz3wSkYyXYWUh7ymB74= @@ -227,6 +229,7 @@ github.com/kardianos/osext v0.0.0-20190222173326-2bc1f35cddc0/go.mod h1:1NbS8ALr github.com/keybase/go-crypto v0.0.0-20161004153544-93f5b35093ba/go.mod h1:ghbZscTyKdM07+Fw3KSi0hcJm+AlEUWj8QLlPtijN/M= github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= +github.com/konsorten/go-windows-terminal-sequences v1.0.1 h1:mweAR1A6xJ3oS2pRaGiHgQ4OO8tzTaLawm8vnODuwDk= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= diff --git a/internal/config/moduleconfig/module_config.go b/internal/config/moduleconfig/module_config.go index 9f8774a0f..e8d300b41 100644 --- a/internal/config/moduleconfig/module_config.go +++ b/internal/config/moduleconfig/module_config.go @@ -1,9 +1,16 @@ package moduleconfig import ( + "fmt" "io/ioutil" + "log" + "reflect" + "strings" yaml "gopkg.in/yaml.v2" + + "github.com/commitdev/zero/pkg/util/flog" + "github.com/iancoleman/strcase" ) type ModuleConfig struct { @@ -48,16 +55,109 @@ type TemplateConfig struct { OutputDir string `yaml:"outputDir"` } +// A "nice" wrapper around findMissing() +func (cfg ModuleConfig) collectMissing() []string { + var missing []string + findMissing(reflect.ValueOf(cfg), "", "", &missing) + + return missing +} + func LoadModuleConfig(filePath string) (ModuleConfig, error) { config := ModuleConfig{} + + var required = []string{} + fmt.Printf("%+#v\n", required) + data, err := ioutil.ReadFile(filePath) if err != nil { return config, err } + err = yaml.Unmarshal(data, &config) if err != nil { return config, err } + missing := config.collectMissing() + if len(missing) > 0 { + flog.Errorf("%v is missing information", filePath) + + for _, m := range missing { + flog.Errorf("\t %v\n", m) + } + + log.Fatal("") + } + return config, nil } + +// Recurses through a datastructure to find any missing data. +// This assumes several things: +// 1. The structure matches that defined by ModuleConfig and its child datastructures. +// 2. YAML struct field metadata is sufficient to define whether an attribute is missing or not. +// That is, "yaml:foo,omitempty" tells us this is not a required field because we can omit it. +// 3. Slices and arrays are assumed to be optional. +// +// As this function recurses through the datastructure, it builds up a string +// path representing each node's path within the datastructure. +// If the value of the current node is equal to the zero value for its datatype +// and its struct field does *not* have a "omitempty" value, then we assume it +// is missing and add it to the resultset. +func findMissing(obj reflect.Value, path, metadata string, missing *[]string) { + t := obj.Type() + switch t.Kind() { + case reflect.String: + if obj.String() == "" && !strings.Contains(metadata, "omitempty") { + *missing = append(*missing, path) + } + + case reflect.Slice, reflect.Array: + for i := 0; i < obj.Len(); i++ { + prefix := fmt.Sprintf("%v[%v]", path, i) + findMissing(obj.Index(i), prefix, metadata, missing) + } + + case reflect.Struct: + for i := 0; i < t.NumField(); i++ { + fieldType := t.Field(i) + fieldTags, _ := fieldType.Tag.Lookup("yaml") + fieldVal := obj.Field(i) + + tags := strings.Split(fieldTags, ",") + + hasOmitEmpty := false + // We have all metadata yaml tags, now let's remove the "omitempty" tag if + // it is present. + // Then if we have only one tag remaining, this must be the expected yaml + // identifer. + // Otherwise the name of the yaml identifier should match the struct + // attribute name. + for i := len(tags) - 1; i >= 0; i-- { + tag := tags[i] + if tag == "omitempty" { + hasOmitEmpty = true + tags = append(tags[:i], tags[i+1:]...) + } + } + + yamlName := strcase.ToLowerCamel(fieldType.Name) + if len(tags) == 1 && tags[0] != "" { // For some reason, empty tag lists are giving a count of 1. + yamlName = tags[0] + } + + prefix := yamlName + if path != "" { + prefix = fmt.Sprintf("%v.%v", path, yamlName) + } + + zeroVal := reflect.Zero(fieldType.Type) + if fieldVal == zeroVal && !hasOmitEmpty { + *missing = append(*missing, prefix) + } + + findMissing(fieldVal, prefix, fieldTags, missing) + } + } +} From 0eb577e4c389558dadc49afc387df2fa077fcb88 Mon Sep 17 00:00:00 2001 From: Aaron Oman Date: Mon, 14 Dec 2020 14:45:07 -0800 Subject: [PATCH 2/4] Set description and author in example zero module --- tests/test_data/modules/ci/zero-module.yml | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/test_data/modules/ci/zero-module.yml b/tests/test_data/modules/ci/zero-module.yml index ec6a69e4d..872ebdff8 100644 --- a/tests/test_data/modules/ci/zero-module.yml +++ b/tests/test_data/modules/ci/zero-module.yml @@ -1,6 +1,6 @@ name: "CI templates" -description: "" -author: "" +description: "CI description" +author: "CI author" icon: "" thumbnail: "" @@ -10,8 +10,8 @@ requiredCredentials: - github # Template variables to populate, these could be overwritten by the file spefic frontmatter variables -template: - # strictMode: true # will only parse files that includes the .tmpl.* extension, otherwise it will copy file +template: + # strictMode: true # will only parse files that includes the .tmpl.* extension, otherwise it will copy file delimiters: - "<%" - "%>" @@ -19,16 +19,15 @@ template: outputDir: ".circleci" # required context parameters: will throw a warning message at the end if any of the context parameters are not present -# contextRequired: +# contextRequired: # - cognitoPoolID # - cognitoClientID # parameters required from user to populate the template params -parameters: +parameters: - field: platform label: CI Platform # default: github - options: + options: - github - circlci - From b41c9a5439d6e1dcc5bdc6f895590770bbbf5a3b Mon Sep 17 00:00:00 2001 From: Aaron Oman Date: Tue, 15 Dec 2020 09:46:38 -0800 Subject: [PATCH 3/4] Remove unused code --- internal/config/moduleconfig/module_config.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/config/moduleconfig/module_config.go b/internal/config/moduleconfig/module_config.go index e8d300b41..5912cc6ee 100644 --- a/internal/config/moduleconfig/module_config.go +++ b/internal/config/moduleconfig/module_config.go @@ -66,9 +66,6 @@ func (cfg ModuleConfig) collectMissing() []string { func LoadModuleConfig(filePath string) (ModuleConfig, error) { config := ModuleConfig{} - var required = []string{} - fmt.Printf("%+#v\n", required) - data, err := ioutil.ReadFile(filePath) if err != nil { return config, err From ae5b303f74645e9b80a0d4c1a4c9d36ad54ee081 Mon Sep 17 00:00:00 2001 From: Aaron Oman Date: Tue, 15 Dec 2020 09:54:26 -0800 Subject: [PATCH 4/4] Remove unnecessary newline in error log --- internal/config/moduleconfig/module_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/config/moduleconfig/module_config.go b/internal/config/moduleconfig/module_config.go index 5912cc6ee..296d7d1c3 100644 --- a/internal/config/moduleconfig/module_config.go +++ b/internal/config/moduleconfig/module_config.go @@ -81,7 +81,7 @@ func LoadModuleConfig(filePath string) (ModuleConfig, error) { flog.Errorf("%v is missing information", filePath) for _, m := range missing { - flog.Errorf("\t %v\n", m) + flog.Errorf("\t %v", m) } log.Fatal("")