From f46c91d48a49527bee4559eec58e2d03e90be8b5 Mon Sep 17 00:00:00 2001 From: wata_mac Date: Sat, 28 Sep 2019 15:52:42 +0900 Subject: [PATCH] Remove deprecated `ignore-rule` option --- README.md | 1 - cmd/cli.go | 3 ++ cmd/cli_test.go | 6 +++ cmd/option.go | 10 ----- cmd/option_test.go | 24 ------------ docs/guides/config.md | 8 ---- rules/provider.go | 14 +++---- rules/provider_test.go | 19 +++------- tflint/config.go | 42 +++++++++------------ tflint/config_test.go | 37 +++--------------- tflint/test-fixtures/config/config.hcl | 5 --- tflint/test-fixtures/config/ignore_rule.hcl | 6 +++ 12 files changed, 49 insertions(+), 126 deletions(-) create mode 100644 tflint/test-fixtures/config/ignore_rule.hcl diff --git a/README.md b/README.md index 941ed9382..b5c1012b1 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,6 @@ Application Options: -f, --format=[default|json|checkstyle] Output format (default: default) -c, --config=FILE Config file name (default: .tflint.hcl) --ignore-module=SOURCE1,SOURCE2... Ignore module sources - --ignore-rule=RULE1,RULE2... Ignore rule names --var-file=FILE1,FILE2... Terraform variable file names --var='foo=bar' Set a Terraform variable --module Inspect modules diff --git a/cmd/cli.go b/cmd/cli.go index 10a10be23..17f3c5039 100644 --- a/cmd/cli.go +++ b/cmd/cli.go @@ -64,6 +64,9 @@ func (cli *CLI) Run(args []string) int { if option == "quiet" || option == "q" { return []string{}, errors.New("`quiet` option was removed in v0.11.0. The behavior is now default") } + if option == "ignore-rule" { + return []string{}, errors.New("`ignore-rule` option was removed in v0.12.0") + } return []string{}, fmt.Errorf("`%s` is unknown option. Please run `tflint --help`", option) } // Parse commandline flag diff --git a/cmd/cli_test.go b/cmd/cli_test.go index 7813325bb..4994b6f42 100644 --- a/cmd/cli_test.go +++ b/cmd/cli_test.go @@ -89,6 +89,12 @@ func TestCLIRun__noIssuesFound(t *testing.T) { Status: ExitCodeError, Stderr: "`quiet` option was removed in v0.11.0. The behavior is now default", }, + { + Name: "removed `--ignore-rule` option", + Command: "./tflint --ignore-rule aws_instance_invalid_type", + Status: ExitCodeError, + Stderr: "`ignore-rule` option was removed in v0.12.0", + }, { Name: "invalid options", Command: "./tflint --unknown", diff --git a/cmd/option.go b/cmd/option.go index 83165c4a7..cdc5f5fba 100644 --- a/cmd/option.go +++ b/cmd/option.go @@ -15,7 +15,6 @@ type Options struct { Format string `short:"f" long:"format" description:"Output format" choice:"default" choice:"json" choice:"checkstyle" default:"default"` Config string `short:"c" long:"config" description:"Config file name" value-name:"FILE" default:".tflint.hcl"` IgnoreModule string `long:"ignore-module" description:"Ignore module sources" value-name:"SOURCE1,SOURCE2..."` - IgnoreRule string `long:"ignore-rule" description:"Ignore rule names" value-name:"RULE1,RULE2..."` Varfile string `long:"var-file" description:"Terraform variable file names" value-name:"FILE1,FILE2..."` Variables []string `long:"var" description:"Set a Terraform variable" value-name:"'foo=bar'"` Module bool `long:"module" description:"Inspect modules"` @@ -37,13 +36,6 @@ func (opts *Options) toConfig() *tflint.Config { } } - ignoreRule := map[string]bool{} - if opts.IgnoreRule != "" { - for _, r := range strings.Split(opts.IgnoreRule, ",") { - ignoreRule[r] = true - } - } - varfile := []string{} if opts.Varfile != "" { varfile = strings.Split(opts.Varfile, ",") @@ -57,7 +49,6 @@ func (opts *Options) toConfig() *tflint.Config { log.Printf("[DEBUG] DeepCheck: %t", opts.Deep) log.Printf("[DEBUG] Force: %t", opts.Force) log.Printf("[DEBUG] IgnoreModule: %#v", ignoreModule) - log.Printf("[DEBUG] IgnoreRule: %#v", ignoreRule) log.Printf("[DEBUG] Varfile: %#v", varfile) log.Printf("[DEBUG] Variables: %#v", opts.Variables) @@ -73,7 +64,6 @@ func (opts *Options) toConfig() *tflint.Config { Region: opts.AwsRegion, }, IgnoreModule: ignoreModule, - IgnoreRule: ignoreRule, Varfile: varfile, Variables: opts.Variables, Rules: map[string]*tflint.RuleConfig{}, diff --git a/cmd/option_test.go b/cmd/option_test.go index 1ecb1c7b1..bf36eaa44 100644 --- a/cmd/option_test.go +++ b/cmd/option_test.go @@ -30,7 +30,6 @@ func Test_toConfig(t *testing.T) { Force: false, AwsCredentials: client.AwsCredentials{}, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -45,7 +44,6 @@ func Test_toConfig(t *testing.T) { Force: false, AwsCredentials: client.AwsCredentials{}, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -60,7 +58,6 @@ func Test_toConfig(t *testing.T) { Force: true, AwsCredentials: client.AwsCredentials{}, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -79,7 +76,6 @@ func Test_toConfig(t *testing.T) { Region: "us-east-1", }, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -97,7 +93,6 @@ func Test_toConfig(t *testing.T) { Region: "us-east-1", }, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -116,7 +111,6 @@ func Test_toConfig(t *testing.T) { Region: "us-east-1", }, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -131,22 +125,6 @@ func Test_toConfig(t *testing.T) { Force: false, AwsCredentials: client.AwsCredentials{}, IgnoreModule: map[string]bool{"module1": true, "module2": true}, - IgnoreRule: map[string]bool{}, - Varfile: []string{}, - Variables: []string{}, - Rules: map[string]*tflint.RuleConfig{}, - }, - }, - { - Name: "--ignore-rule", - Command: "./tflint --ignore-rule rule1,rule2", - Expected: &tflint.Config{ - Module: false, - DeepCheck: false, - Force: false, - AwsCredentials: client.AwsCredentials{}, - IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{"rule1": true, "rule2": true}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -161,7 +139,6 @@ func Test_toConfig(t *testing.T) { Force: false, AwsCredentials: client.AwsCredentials{}, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{"example1.tfvars", "example2.tfvars"}, Variables: []string{}, Rules: map[string]*tflint.RuleConfig{}, @@ -176,7 +153,6 @@ func Test_toConfig(t *testing.T) { Force: false, AwsCredentials: client.AwsCredentials{}, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{"foo=bar", "bar=baz"}, Rules: map[string]*tflint.RuleConfig{}, diff --git a/docs/guides/config.md b/docs/guides/config.md index 8573b29d1..f8adbe592 100644 --- a/docs/guides/config.md +++ b/docs/guides/config.md @@ -87,8 +87,6 @@ Set a Terraform variable from a passed value. This flag can be set multiple time ## `rule` blocks -CLI flag: `--ignore-rule` - You can make settings for each rule in the `rule` block. Currently, it can set only `enabled` option. If you set `enabled = false`, TFLint doesn't inspect configuration files by this rule. ```hcl @@ -96,9 +94,3 @@ rule "aws_instance_previous_type" { enabled = false } ``` - -You can also disable rules with the `--ignore-rule` option. - -``` -$ tflint --ignore-rule=aws_instance_invalid_type,aws_instance_previous_type -``` diff --git a/rules/provider.go b/rules/provider.go index 79ab43b89..d4898e4cc 100644 --- a/rules/provider.go +++ b/rules/provider.go @@ -58,20 +58,18 @@ func NewRules(c *tflint.Config) []Rule { } for _, rule := range allRules { + enabled := rule.Enabled() if r := c.Rules[rule.Name()]; r != nil { if r.Enabled { log.Printf("[DEBUG] `%s` is enabled", rule.Name()) - ret = append(ret, rule) - } else { - log.Printf("[DEBUG] `%s` is disabled", rule.Name()) - } - } else { - if !c.IgnoreRule[rule.Name()] && rule.Enabled() { - log.Printf("[DEBUG] `%s` is enabled", rule.Name()) - ret = append(ret, rule) } else { log.Printf("[DEBUG] `%s` is disabled", rule.Name()) } + enabled = r.Enabled + } + + if enabled { + ret = append(ret, rule) } } diff --git a/rules/provider_test.go b/rules/provider_test.go index 330ac954c..30f83bf75 100644 --- a/rules/provider_test.go +++ b/rules/provider_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/wata727/tflint/rules/awsrules" + "github.com/wata727/tflint/rules/terraformrules" "github.com/wata727/tflint/tflint" ) @@ -12,6 +13,7 @@ func Test_NewRules(t *testing.T) { // Mock rules in test DefaultRules = []Rule{ awsrules.NewAwsRouteNotSpecifiedTargetRule(), + terraformrules.NewTerraformDashInResourceNameRule(), } deepCheckRules = []Rule{ awsrules.NewAwsInstanceInvalidAMIRule(), @@ -39,15 +41,6 @@ func Test_NewRules(t *testing.T) { awsrules.NewAwsInstanceInvalidAMIRule(), }, }, - { - Name: "ignore_rule", - Config: &tflint.Config{ - IgnoreRule: map[string]bool{ - "aws_route_not_specified_target": true, - }, - }, - Expected: []Rule{}, - }, { Name: "enabled = false", Config: &tflint.Config{ @@ -60,19 +53,17 @@ func Test_NewRules(t *testing.T) { Expected: []Rule{}, }, { - Name: "`enabled = true` and `ignore_rule`", + Name: "enabled = true", Config: &tflint.Config{ - IgnoreRule: map[string]bool{ - "aws_route_not_specified_target": true, - }, Rules: map[string]*tflint.RuleConfig{ - "aws_route_not_specified_target": { + "terraform_dash_in_resource_name": { Enabled: true, }, }, }, Expected: []Rule{ awsrules.NewAwsRouteNotSpecifiedTargetRule(), + terraformrules.NewTerraformDashInResourceNameRule(), }, }, } diff --git a/tflint/config.go b/tflint/config.go index 31ec318dd..ca18c2f5e 100644 --- a/tflint/config.go +++ b/tflint/config.go @@ -17,15 +17,16 @@ var fallbackConfigFile = "~/.tflint.hcl" type rawConfig struct { Config *struct { - Module *bool `hcl:"module"` - DeepCheck *bool `hcl:"deep_check"` - Force *bool `hcl:"force"` - AwsCredentials *map[string]string `hcl:"aws_credentials"` - IgnoreModule *map[string]bool `hcl:"ignore_module"` - IgnoreRule *map[string]bool `hcl:"ignore_rule"` - Varfile *[]string `hcl:"varfile"` - Variables *[]string `hcl:"variables"` - TerraformVersion *string `hcl:"terraform_version"` + Module *bool `hcl:"module"` + DeepCheck *bool `hcl:"deep_check"` + Force *bool `hcl:"force"` + AwsCredentials *map[string]string `hcl:"aws_credentials"` + IgnoreModule *map[string]bool `hcl:"ignore_module"` + Varfile *[]string `hcl:"varfile"` + Variables *[]string `hcl:"variables"` + // Removed options + TerraformVersion *string `hcl:"terraform_version"` + IgnoreRule *map[string]bool `hcl:"ignore_rule"` } `hcl:"config,block"` Rules []RuleConfig `hcl:"rule,block"` } @@ -37,7 +38,6 @@ type Config struct { Force bool AwsCredentials client.AwsCredentials IgnoreModule map[string]bool - IgnoreRule map[string]bool Varfile []string Variables []string Rules map[string]*RuleConfig @@ -58,7 +58,6 @@ func EmptyConfig() *Config { Force: false, AwsCredentials: client.AwsCredentials{}, IgnoreModule: map[string]bool{}, - IgnoreRule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, Rules: map[string]*RuleConfig{}, @@ -121,7 +120,6 @@ func (c *Config) Merge(other *Config) *Config { ret.AwsCredentials = ret.AwsCredentials.Merge(other.AwsCredentials) ret.IgnoreModule = mergeBoolMap(ret.IgnoreModule, other.IgnoreModule) - ret.IgnoreRule = mergeBoolMap(ret.IgnoreRule, other.IgnoreRule) ret.Varfile = append(ret.Varfile, other.Varfile...) ret.Variables = append(ret.Variables, other.Variables...) @@ -136,11 +134,6 @@ func (c *Config) copy() *Config { ignoreModule[k] = v } - ignoreRule := make(map[string]bool) - for k, v := range c.IgnoreRule { - ignoreRule[k] = v - } - varfile := make([]string, len(c.Varfile)) copy(varfile, c.Varfile) @@ -159,7 +152,6 @@ func (c *Config) copy() *Config { Force: c.Force, AwsCredentials: c.AwsCredentials, IgnoreModule: ignoreModule, - IgnoreRule: ignoreRule, Varfile: varfile, Variables: variables, Rules: rules, @@ -180,8 +172,14 @@ func loadConfigFromFile(file string) (*Config, error) { return nil, diags } - if raw.Config != nil && raw.Config.TerraformVersion != nil { - return nil, errors.New("`terraform_version` was removed in v0.9.0 because the option is no longer used") + if raw.Config != nil { + if raw.Config.TerraformVersion != nil { + return nil, errors.New("`terraform_version` was removed in v0.9.0 because the option is no longer used") + } + + if raw.Config.IgnoreRule != nil { + return nil, errors.New("`ignore_rule` was removed in v0.12.0. Please define `rule` block with `enabled = false` instead") + } } cfg := raw.toConfig() @@ -190,7 +188,6 @@ func loadConfigFromFile(file string) (*Config, error) { log.Printf("[DEBUG] DeepCheck: %t", cfg.DeepCheck) log.Printf("[DEBUG] Force: %t", cfg.Force) log.Printf("[DEBUG] IgnoreModule: %#v", cfg.IgnoreModule) - log.Printf("[DEBUG] IgnoreRule: %#v", cfg.IgnoreRule) log.Printf("[DEBUG] Varfile: %#v", cfg.Varfile) log.Printf("[DEBUG] Variables: %#v", cfg.Variables) log.Printf("[DEBUG] Rules: %#v", cfg.Rules) @@ -245,9 +242,6 @@ func (raw *rawConfig) toConfig() *Config { if rc.IgnoreModule != nil { ret.IgnoreModule = *rc.IgnoreModule } - if rc.IgnoreRule != nil { - ret.IgnoreRule = *rc.IgnoreRule - } if rc.Varfile != nil { ret.Varfile = *rc.Varfile } diff --git a/tflint/config_test.go b/tflint/config_test.go index 36e4db569..746295596 100644 --- a/tflint/config_test.go +++ b/tflint/config_test.go @@ -36,10 +36,6 @@ func Test_LoadConfig(t *testing.T) { Profile: "production", CredsFile: "~/.aws/myapp", }, - IgnoreRule: map[string]bool{ - "aws_instance_invalid_type": true, - "aws_instance_previous_type": true, - }, IgnoreModule: map[string]bool{ "github.com/wata727/example-module": true, }, @@ -75,7 +71,6 @@ func Test_LoadConfig(t *testing.T) { SecretKey: "AWS_SECRET_KEY", Region: "us-east-1", }, - IgnoreRule: map[string]bool{}, IgnoreModule: map[string]bool{}, Varfile: []string{}, Variables: []string{}, @@ -150,6 +145,11 @@ func Test_LoadConfig_error(t *testing.T) { File: filepath.Join(currentDir, "test-fixtures", "config", "terraform_version.hcl"), Expected: "`terraform_version` was removed in v0.9.0 because the option is no longer used", }, + { + Name: "ignore_rule", + File: filepath.Join(currentDir, "test-fixtures", "config", "ignore_rule.hcl"), + Expected: "`ignore_rule` was removed in v0.12.0. Please define `rule` block with `enabled = false` instead", + }, } for _, tc := range cases { @@ -178,10 +178,6 @@ func Test_Merge(t *testing.T) { "github.com/wata727/example-1": true, "github.com/wata727/example-2": false, }, - IgnoreRule: map[string]bool{ - "aws_instance_invalid_type": false, - "aws_instance_invalid_ami": true, - }, Varfile: []string{"example1.tfvars", "example2.tfvars"}, Variables: []string{"foo=bar"}, Rules: map[string]*RuleConfig{ @@ -236,10 +232,6 @@ func Test_Merge(t *testing.T) { "github.com/wata727/example-1": true, "github.com/wata727/example-2": false, }, - IgnoreRule: map[string]bool{ - "aws_instance_invalid_type": false, - "aws_instance_invalid_ami": true, - }, Varfile: []string{"example1.tfvars", "example2.tfvars"}, Variables: []string{"foo=bar"}, Rules: map[string]*RuleConfig{ @@ -267,10 +259,6 @@ func Test_Merge(t *testing.T) { "github.com/wata727/example-2": true, "github.com/wata727/example-3": false, }, - IgnoreRule: map[string]bool{ - "aws_instance_invalid_ami": false, - "aws_instance_previous_type": true, - }, Varfile: []string{"example3.tfvars"}, Variables: []string{"bar=baz"}, Rules: map[string]*RuleConfig{ @@ -300,11 +288,6 @@ func Test_Merge(t *testing.T) { "github.com/wata727/example-2": true, "github.com/wata727/example-3": false, }, - IgnoreRule: map[string]bool{ - "aws_instance_invalid_type": false, - "aws_instance_invalid_ami": false, - "aws_instance_previous_type": true, - }, Varfile: []string{"example1.tfvars", "example2.tfvars", "example3.tfvars"}, Variables: []string{"foo=bar", "bar=baz"}, Rules: map[string]*RuleConfig{ @@ -347,10 +330,6 @@ func Test_copy(t *testing.T) { "github.com/wata727/example-1": true, "github.com/wata727/example-2": false, }, - IgnoreRule: map[string]bool{ - "aws_instance_invalid_type": false, - "aws_instance_invalid_ami": true, - }, Varfile: []string{"example1.tfvars", "example2.tfvars"}, Variables: []string{}, Rules: map[string]*RuleConfig{ @@ -402,12 +381,6 @@ func Test_copy(t *testing.T) { c.IgnoreModule["github.com/wata727/example-1"] = false }, }, - { - Name: "IgnoreRule", - SideEffect: func(c *Config) { - c.IgnoreRule["aws_instance_invalid_type"] = true - }, - }, { Name: "Varfile", SideEffect: func(c *Config) { diff --git a/tflint/test-fixtures/config/config.hcl b/tflint/test-fixtures/config/config.hcl index 11a7b3e32..5045b0d3d 100644 --- a/tflint/test-fixtures/config/config.hcl +++ b/tflint/test-fixtures/config/config.hcl @@ -11,11 +11,6 @@ config { shared_credentials_file = "~/.aws/myapp" } - ignore_rule = { - aws_instance_invalid_type = true - aws_instance_previous_type = true - } - ignore_module = { "github.com/wata727/example-module" = true } diff --git a/tflint/test-fixtures/config/ignore_rule.hcl b/tflint/test-fixtures/config/ignore_rule.hcl new file mode 100644 index 000000000..89f2b836f --- /dev/null +++ b/tflint/test-fixtures/config/ignore_rule.hcl @@ -0,0 +1,6 @@ +config { + ignore_rule = { + aws_instance_invalid_type = true + aws_instance_previous_type = true + } +}