From 772df88ce90f01081d2fff77e8cf9f14a1b833c5 Mon Sep 17 00:00:00 2001 From: Trey McElhattan Date: Fri, 17 Jan 2020 11:53:57 -0500 Subject: [PATCH 1/6] refactoring to include cli option tfparser --- cli/app.go | 5 ++++- cli/app_test.go | 2 +- cli/builtin_terraform_test.go | 3 +-- cli/options.go | 17 +++++++++++++++++ cli/options_test.go | 1 + go.mod | 2 +- go.sum | 4 ++++ linter/linter.go | 14 +++++++++++--- linter/linter_test.go | 4 ++-- 9 files changed, 42 insertions(+), 10 deletions(-) diff --git a/cli/app.go b/cli/app.go index 3587d06..2389492 100644 --- a/cli/app.go +++ b/cli/app.go @@ -29,6 +29,7 @@ type ( SearchExpression string ExcludePatterns []string Variables map[string]string + TerraformParser string } // ProfileOptions for default options from a project file @@ -60,6 +61,7 @@ type ( ExcludePatterns arrayFlags ExcludeFromFilenames arrayFlags Variables arrayFlags + TerraformParser *string ProfileFilename *string TerraformBuiltInRules *bool Terraform12BuiltInRules *bool @@ -254,11 +256,12 @@ func applyRules(ruleSets []assertion.RuleSet, args arrayFlags, options LinterOpt ResourcesScanned: []assertion.ScannedResource{}, } + parser := options.TerraformParser filenames := excludeFilenames(getFilenames(args), options.ExcludePatterns) vs := assertion.StandardValueSource{Variables: options.Variables} for _, ruleSet := range ruleSets { - l, err := linter.NewLinter(ruleSet, vs, filenames) + l, err := linter.NewLinter(ruleSet, vs, filenames, parser) if err != nil { fmt.Println(err) return -1 diff --git a/cli/app_test.go b/cli/app_test.go index 932e120..f20de51 100644 --- a/cli/app_test.go +++ b/cli/app_test.go @@ -109,7 +109,7 @@ func TestBuiltRules(t *testing.T) { } vs := assertion.StandardValueSource{} filenames := []string{"assets/terraform.yml", "assets/lint-rules.yml"} - l, err := linter.NewLinter(ruleSet, vs, filenames) + l, err := linter.NewLinter(ruleSet, vs, filenames, "") if err != nil { t.Errorf("Expecting NewLinter to not return error: %s", err.Error()) } diff --git a/cli/builtin_terraform_test.go b/cli/builtin_terraform_test.go index dcabc71..b2ebdca 100644 --- a/cli/builtin_terraform_test.go +++ b/cli/builtin_terraform_test.go @@ -108,13 +108,12 @@ func TestTerraformBuiltInRules(t *testing.T) { {"ecs.tf", "ECS_ENVIRONMENT_SECRETS", 0, 1}, } for _, tc := range testCases { - filenames := []string{"testdata/builtin/terraform/" + tc.Filename} options := linter.Options{ RuleIDs: []string{tc.RuleID}, } vs := assertion.StandardValueSource{} - l, err := linter.NewLinter(ruleSet, vs, filenames) + l, err := linter.NewLinter(ruleSet, vs, filenames, "") report, err := l.Validate(ruleSet, options) assert.Nil(t, err, "Validate failed for file") warningMessage := fmt.Sprintf("Expecting %d warnings for RuleID %s in File %s", tc.WarningCount, tc.RuleID, tc.Filename) diff --git a/cli/options.go b/cli/options.go index cfaecde..6346086 100644 --- a/cli/options.go +++ b/cli/options.go @@ -17,6 +17,8 @@ func getCommandLineOptions() CommandLineOptions { commandLineOptions.TerraformBuiltInRules = flag.Bool("terraform", false, "Use built-in rules for Terraform") commandLineOptions.Terraform12BuiltInRules = flag.Bool("terraform12", false, "Use built-in rules for Terraform v0.12") flag.Var(&commandLineOptions.RulesFilenames, "rules", "Rules file, can be specified multiple times") + //flag.Var(&commandLineOptions.Parser, "parser", "Version of Terraform parser to use (either tf12 or tf11") + commandLineOptions.TerraformParser = flag.String("tfparser", "", "Version of Terraform parser to use (must be either 'tf12' or 'tf11')") commandLineOptions.Tags = flag.String("tags", "", "Run only tests with tags in this comma separated list") commandLineOptions.Ids = flag.String("ids", "", "Run only the rules in this comma separated list") commandLineOptions.IgnoreIds = flag.String("ignore-ids", "", "Ignore the rules in this comma separated list") @@ -42,6 +44,10 @@ func getLinterOptions(o CommandLineOptions, p ProfileOptions) (LinterOptions, er if err != nil { return LinterOptions{}, err } + tfParser, err := validateParser(*o.TerraformParser) + if err != nil { + return LinterOptions{}, err + } linterOptions := LinterOptions{ Tags: makeTagList(*o.Tags, p.Tags), RuleIDs: makeRulesList(*o.Ids, p.IDs), @@ -50,6 +56,7 @@ func getLinterOptions(o CommandLineOptions, p ProfileOptions) (LinterOptions, er SearchExpression: *o.SearchExpression, ExcludePatterns: allExcludePatterns, Variables: mergeVariables(p.Variables, parseVariables(o.Variables)), + TerraformParser: tfParser, } return linterOptions, nil } @@ -164,3 +171,13 @@ func loadExcludePatterns(patterns []string, excludeFromFilenames []string) ([]st } return patterns, nil } + +func validateParser(parser string) (string, error) { + validOptions := []string{"", "tf11", "tf12"} + for _, option := range validOptions { + if parser == option { + return parser, nil + } + } + return "", fmt.Errorf("tf-parser \"%s\" is not valid. Choose from [\"tf11\", \"tf12\"].\n", parser) +} diff --git a/cli/options_test.go b/cli/options_test.go index 6185022..bea2cc5 100644 --- a/cli/options_test.go +++ b/cli/options_test.go @@ -14,6 +14,7 @@ func emptyCommandLineOptions() CommandLineOptions { QueryExpression: &emptyString, SearchExpression: &emptyString, VerboseReport: &verbose, + TerraformParser: &emptyString, } } diff --git a/go.mod b/go.mod index 112ec5e..43fc8cc 100644 --- a/go.mod +++ b/go.mod @@ -18,5 +18,5 @@ require ( github.com/stretchr/testify v1.4.0 github.com/zclconf/go-cty v1.1.1 golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect - golang.org/x/tools v0.0.0-20200108195415-316d2f248479 // indirect + golang.org/x/tools v0.0.0-20200117161641-43d50277825c // indirect ) diff --git a/go.sum b/go.sum index e9a4538..4698bd0 100644 --- a/go.sum +++ b/go.sum @@ -460,6 +460,10 @@ golang.org/x/tools v0.0.0-20200107184032-11e9d9cc0042 h1:BKiPVwWbEdmAh+5CBwk13CY golang.org/x/tools v0.0.0-20200107184032-11e9d9cc0042/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200108195415-316d2f248479 h1:csuS+MHeEA2eWhyjQCMaPMq4z1+/PohkBSjJZHSIbOE= golang.org/x/tools v0.0.0-20200108195415-316d2f248479/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20200117065230-39095c1d176c h1:FodBYPZKH5tAN2O60HlglMwXGAeV/4k+NKbli79M/2c= +golang.org/x/tools v0.0.0-20200117065230-39095c1d176c/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20200117161641-43d50277825c h1:2EA2K0k9bcvvEDlqD8xdlOhCOqq+O/p9Voqi4x9W1YU= +golang.org/x/tools v0.0.0-20200117161641-43d50277825c/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= diff --git a/linter/linter.go b/linter/linter.go index 0528b0b..f8ecfaf 100644 --- a/linter/linter.go +++ b/linter/linter.go @@ -23,15 +23,23 @@ type ( ) // NewLinter create the right kind of Linter based on the type argument -func NewLinter(ruleSet assertion.RuleSet, vs assertion.ValueSource, filenames []string) (Linter, error) { +func NewLinter(ruleSet assertion.RuleSet, vs assertion.ValueSource, filenames []string, parser string) (Linter, error) { assertion.Debugf("Filenames to scan: %v\n", filenames) switch ruleSet.Type { case "Kubernetes": return FileLinter{Filenames: filenames, ValueSource: vs, Loader: KubernetesResourceLoader{}}, nil case "Terraform": - return FileLinter{Filenames: filenames, ValueSource: vs, Loader: TerraformResourceLoader{}}, nil + if parser == "tf12" { + return FileLinter{Filenames: filenames, ValueSource: vs, Loader: Terraform12ResourceLoader{}}, nil + } else { + return FileLinter{Filenames: filenames, ValueSource: vs, Loader: TerraformResourceLoader{}}, nil + } case "Terraform12": - return FileLinter{Filenames: filenames, ValueSource: vs, Loader: Terraform12ResourceLoader{}}, nil + if parser == "tf11" { + return FileLinter{Filenames: filenames, ValueSource: vs, Loader: TerraformResourceLoader{}}, nil + } else { + return FileLinter{Filenames: filenames, ValueSource: vs, Loader: Terraform12ResourceLoader{}}, nil + } case "LintRules": return FileLinter{Filenames: filenames, ValueSource: vs, Loader: RulesResourceLoader{}}, nil case "YAML": diff --git a/linter/linter_test.go b/linter/linter_test.go index ea61957..425a0a1 100644 --- a/linter/linter_test.go +++ b/linter/linter_test.go @@ -25,7 +25,7 @@ func TestNewLinter(t *testing.T) { vs := MockValueSource{} for _, tc := range testCases { ruleSet := loadRulesForTest(tc.Filename, t) - l, err := NewLinter(ruleSet, vs, []string{}) + l, err := NewLinter(ruleSet, vs, []string{}, "") if err != nil { t.Errorf("Expecting TestNewLinter to not return an error: %s", err.Error()) } @@ -39,7 +39,7 @@ func TestNewLinter(t *testing.T) { func TestUnknownLinterType(t *testing.T) { ruleSet := loadRulesForTest("./testdata/rules/unknown.yml", t) vs := MockValueSource{} - _, err := NewLinter(ruleSet, vs, []string{}) + _, err := NewLinter(ruleSet, vs, []string{}, "") if err == nil { t.Errorf("Expecting NewLinter to return an error for unsupported type") } From 9ee1aeb045c73c30c10d3fc08e93c029ee8f7c60 Mon Sep 17 00:00:00 2001 From: Jim Rohrer Date: Fri, 17 Jan 2020 11:03:05 -0600 Subject: [PATCH 2/6] Added test to ValidateParser --- cli/options_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/cli/options_test.go b/cli/options_test.go index bea2cc5..e9ae6db 100644 --- a/cli/options_test.go +++ b/cli/options_test.go @@ -122,3 +122,22 @@ func TestLoadProfile(t *testing.T) { t.Errorf("Expecting single tag in profile: %v\n", p.Tags) } } + +func TestValidateParser(t *testing.T) { + parser, err := validateParser("") + if err != nil { + t.Errorf("Expected %s, got %v", parser, err) + } + parser, err = validateParser("tf11") + if err != nil { + t.Errorf("Expected %s, got %v", parser, err) + } + parser, err = validateParser("tf12") + if err != nil { + t.Errorf("Expected %s, got %v", parser, err) + } + parser, err = validateParser("tf13") + if err == nil { + t.Errorf("Expected %v, got nil", err) + } +} From 130579abec4ec51a84416bfc0fa3d94fad17baca Mon Sep 17 00:00:00 2001 From: Jim Rohrer Date: Fri, 17 Jan 2020 11:06:50 -0600 Subject: [PATCH 3/6] Updated docs with -tfparser flag --- README.md | 2 ++ docs/terraform.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/README.md b/README.md index 2536dd4..e4c14d2 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,8 @@ view them via the -help option. * -verbose - Output a verbose report * -version - Get program version + + * -tfparser - Set the Terraform parser version. Options are `tf11` or `tf12`. # Rules diff --git a/docs/terraform.md b/docs/terraform.md index b5d1b55..328d3fb 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -19,6 +19,8 @@ config-lint -terraform12 The Terraform12 parser is fully backwards compatible with previous versions of Terraform. +If you wish to force a specific parser version, add the `-tfparser tf11|tf12` flag. This is useful if you have a lot of rules with `Type: Terraform` but your Terraform files include Terraform 12 syntax. + ## Custom Terraform rules for your project or organization ``` From 537950c600dc54d53c108b333684eed5f2fd3cc7 Mon Sep 17 00:00:00 2001 From: Trey McElhattan Date: Fri, 17 Jan 2020 12:20:31 -0500 Subject: [PATCH 4/6] changing parameter name to tfParser for more clarity --- go.mod | 2 +- go.sum | 2 ++ linter/linter.go | 6 +++--- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 43fc8cc..dd8659c 100644 --- a/go.mod +++ b/go.mod @@ -18,5 +18,5 @@ require ( github.com/stretchr/testify v1.4.0 github.com/zclconf/go-cty v1.1.1 golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect - golang.org/x/tools v0.0.0-20200117161641-43d50277825c // indirect + golang.org/x/tools v0.0.0-20200117170720-ade7f2547e48 // indirect ) diff --git a/go.sum b/go.sum index 4698bd0..6d6d8ce 100644 --- a/go.sum +++ b/go.sum @@ -464,6 +464,8 @@ golang.org/x/tools v0.0.0-20200117065230-39095c1d176c h1:FodBYPZKH5tAN2O60HlglMw golang.org/x/tools v0.0.0-20200117065230-39095c1d176c/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/tools v0.0.0-20200117161641-43d50277825c h1:2EA2K0k9bcvvEDlqD8xdlOhCOqq+O/p9Voqi4x9W1YU= golang.org/x/tools v0.0.0-20200117161641-43d50277825c/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= +golang.org/x/tools v0.0.0-20200117170720-ade7f2547e48 h1:XfvHzzsGwwI2bSS5CSATpEdarP7UY+5bU6A0/50E5bE= +golang.org/x/tools v0.0.0-20200117170720-ade7f2547e48/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/api v0.4.0/go.mod h1:8k5glujaEP+g9n7WNsDg8QP6cUVNI86fCNMcbazEtwE= diff --git a/linter/linter.go b/linter/linter.go index f8ecfaf..a5a2f66 100644 --- a/linter/linter.go +++ b/linter/linter.go @@ -23,19 +23,19 @@ type ( ) // NewLinter create the right kind of Linter based on the type argument -func NewLinter(ruleSet assertion.RuleSet, vs assertion.ValueSource, filenames []string, parser string) (Linter, error) { +func NewLinter(ruleSet assertion.RuleSet, vs assertion.ValueSource, filenames []string, tfParser string) (Linter, error) { assertion.Debugf("Filenames to scan: %v\n", filenames) switch ruleSet.Type { case "Kubernetes": return FileLinter{Filenames: filenames, ValueSource: vs, Loader: KubernetesResourceLoader{}}, nil case "Terraform": - if parser == "tf12" { + if tfParser == "tf12" { return FileLinter{Filenames: filenames, ValueSource: vs, Loader: Terraform12ResourceLoader{}}, nil } else { return FileLinter{Filenames: filenames, ValueSource: vs, Loader: TerraformResourceLoader{}}, nil } case "Terraform12": - if parser == "tf11" { + if tfParser == "tf11" { return FileLinter{Filenames: filenames, ValueSource: vs, Loader: TerraformResourceLoader{}}, nil } else { return FileLinter{Filenames: filenames, ValueSource: vs, Loader: Terraform12ResourceLoader{}}, nil From 99b6d1ca216962ed886d84ba79544e6f3e7d3b57 Mon Sep 17 00:00:00 2001 From: Trey McElhattan Date: Fri, 17 Jan 2020 12:23:56 -0500 Subject: [PATCH 5/6] same thing in app.go as previous commit --- cli/app.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/app.go b/cli/app.go index 2389492..6682b6a 100644 --- a/cli/app.go +++ b/cli/app.go @@ -256,12 +256,12 @@ func applyRules(ruleSets []assertion.RuleSet, args arrayFlags, options LinterOpt ResourcesScanned: []assertion.ScannedResource{}, } - parser := options.TerraformParser + tfParser := options.TerraformParser filenames := excludeFilenames(getFilenames(args), options.ExcludePatterns) vs := assertion.StandardValueSource{Variables: options.Variables} for _, ruleSet := range ruleSets { - l, err := linter.NewLinter(ruleSet, vs, filenames, parser) + l, err := linter.NewLinter(ruleSet, vs, filenames, tfParser) if err != nil { fmt.Println(err) return -1 From 61ef11c67dbae2f4ab0f16975c6790b99fe6060b Mon Sep 17 00:00:00 2001 From: Trey McElhattan Date: Fri, 17 Jan 2020 12:38:55 -0500 Subject: [PATCH 6/6] updating test functions to test passing in values for tfparser flag option --- linter/linter_test.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/linter/linter_test.go b/linter/linter_test.go index 425a0a1..d149c34 100644 --- a/linter/linter_test.go +++ b/linter/linter_test.go @@ -24,14 +24,17 @@ func TestNewLinter(t *testing.T) { vs := MockValueSource{} for _, tc := range testCases { + tfParserOptions := []string{"", "tf11", "tf12"} ruleSet := loadRulesForTest(tc.Filename, t) - l, err := NewLinter(ruleSet, vs, []string{}, "") - if err != nil { - t.Errorf("Expecting TestNewLinter to not return an error: %s", err.Error()) - } - n := reflect.TypeOf(l).Name() - if n != tc.TypeName { - t.Errorf("Expecting NewLinter expected %s, not %s ", tc.TypeName, n) + for _, tfPO := range tfParserOptions { + l, err := NewLinter(ruleSet, vs, []string{}, tfPO) + if err != nil { + t.Errorf("Expecting TestNewLinter to not return an error: %s", err.Error()) + } + n := reflect.TypeOf(l).Name() + if n != tc.TypeName { + t.Errorf("Expecting NewLinter expected %s, not %s ", tc.TypeName, n) + } } } } @@ -39,9 +42,12 @@ func TestNewLinter(t *testing.T) { func TestUnknownLinterType(t *testing.T) { ruleSet := loadRulesForTest("./testdata/rules/unknown.yml", t) vs := MockValueSource{} - _, err := NewLinter(ruleSet, vs, []string{}, "") - if err == nil { - t.Errorf("Expecting NewLinter to return an error for unsupported type") + tfParserOptions := []string{"", "tf11", "tf12"} + for _, tfPO := range tfParserOptions { + _, err := NewLinter(ruleSet, vs, []string{}, tfPO) + if err == nil { + t.Errorf("Expecting NewLinter to return an error for unsupported type") + } } }