Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new command line option for Terraform parser #62

Merged
merged 6 commits into from
Jan 17, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion cli/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type (
SearchExpression string
ExcludePatterns []string
Variables map[string]string
TerraformParser string
}

// ProfileOptions for default options from a project file
Expand Down Expand Up @@ -60,6 +61,7 @@ type (
ExcludePatterns arrayFlags
ExcludeFromFilenames arrayFlags
Variables arrayFlags
TerraformParser *string
ProfileFilename *string
TerraformBuiltInRules *bool
Terraform12BuiltInRules *bool
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cli/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
3 changes: 1 addition & 2 deletions cli/builtin_terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 17 additions & 0 deletions cli/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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),
Expand All @@ -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
}
Expand Down Expand Up @@ -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)
}
20 changes: 20 additions & 0 deletions cli/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func emptyCommandLineOptions() CommandLineOptions {
QueryExpression: &emptyString,
SearchExpression: &emptyString,
VerboseReport: &verbose,
TerraformParser: &emptyString,
}
}

Expand Down Expand Up @@ -121,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)
}
}
2 changes: 2 additions & 0 deletions docs/terraform.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ config-lint -terraform12 <FILE_OR_DIRECTORY_OF_TF_FILES>

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

```
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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-20200117170720-ade7f2547e48 // indirect
)
6 changes: 6 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,12 @@ 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/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=
Expand Down
14 changes: 11 additions & 3 deletions linter/linter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, 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":
return FileLinter{Filenames: filenames, ValueSource: vs, Loader: TerraformResourceLoader{}}, nil
if tfParser == "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 tfParser == "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":
Expand Down
4 changes: 2 additions & 2 deletions linter/linter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand All @@ -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{}, "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I missed them when I was skimming through quickly, but I don't see any tests around passing in a value here.

I'd like to see some.

if err == nil {
t.Errorf("Expecting NewLinter to return an error for unsupported type")
}
Expand Down