diff --git a/go.mod b/go.mod index 2823e875c..71a04895a 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/hashicorp/go-version v1.6.0 github.com/hashicorp/hcl/v2 v2.13.0 github.com/hashicorp/logutils v1.0.0 + github.com/hashicorp/terraform-registry-address v0.0.0-20220623143253-7d51757b572c github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 github.com/jessevdk/go-flags v1.5.0 github.com/jstemmer/go-junit-report v1.0.0 diff --git a/go.sum b/go.sum index 5bfe72edb..cfcff3434 100644 --- a/go.sum +++ b/go.sum @@ -199,6 +199,8 @@ github.com/hashicorp/hcl/v2 v2.13.0 h1:0Apadu1w6M11dyGFxWnmhhcMjkbAiKCv7G1r/2QgC github.com/hashicorp/hcl/v2 v2.13.0/go.mod h1:e4z5nxYlWNPdDSNYX+ph14EvWYMFm3eP0zIUqPc2jr0= github.com/hashicorp/logutils v1.0.0 h1:dLEQVugN8vlakKOUE3ihGLTZJRB4j+M2cdTm/ORI65Y= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= +github.com/hashicorp/terraform-registry-address v0.0.0-20220623143253-7d51757b572c h1:D8aRO6+mTqHfLsK/BC3j5OAoogv1WLRWzY1AaTo3rBg= +github.com/hashicorp/terraform-registry-address v0.0.0-20220623143253-7d51757b572c/go.mod h1:Wn3Na71knbXc1G8Lh+yu/dQWWJeFQEpDeJMtWMtlmNI= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734 h1:HKLsbzeOsfXmKNpr3GiT18XAblV0BjCbzL8KQAMZGa0= github.com/hashicorp/terraform-svchost v0.0.0-20200729002733-f050f53b9734/go.mod h1:kNDNcF7sN4DocDLBkQYz73HGKwN1ANB1blq4lIYLYvg= github.com/hashicorp/yamux v0.0.0-20181012175058-2f1d1f20f75d h1:kJCB4vdITiW1eC1vq2e6IsrXKrZit1bv/TDYFGMp4BQ= @@ -391,6 +393,7 @@ golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwY golang.org/x/net v0.0.0-20201031054903-ff519b6c9102/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.0.0-20201209123823-ac852fbbde11/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20201224014010-6772e930b67b/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= +golang.org/x/net v0.0.0-20210119194325-5f4716e94777/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20211216030914-fe4d6282115f h1:hEYJvxw1lSnWIl8X9ofsYMklzaDs90JI2az5YMd4fPM= diff --git a/rules/terraformrules/terraform_documented_outputs.go b/rules/terraformrules/terraform_documented_outputs.go index 8e23f41c5..db9a3bda9 100644 --- a/rules/terraformrules/terraform_documented_outputs.go +++ b/rules/terraformrules/terraform_documented_outputs.go @@ -4,6 +4,9 @@ import ( "fmt" "log" + "github.com/hashicorp/hcl/v2/gohcl" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -44,12 +47,43 @@ func (r *TerraformDocumentedOutputsRule) Check(runner *tflint.Runner) error { log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - for _, output := range runner.TFConfig.Module.Outputs { - if output.Description == "" { + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "output", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{{Name: "description"}}, + }, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + + for _, output := range body.Blocks { + attr, exists := output.Body.Attributes["description"] + if !exists { + runner.EmitIssue( + r, + fmt.Sprintf("`%s` output has no description", output.Labels[0]), + output.DefRange, + ) + continue + } + + var description string + diags = gohcl.DecodeExpression(attr.Expr, nil, &description) + if diags.HasErrors() { + return diags + } + + if description == "" { runner.EmitIssue( r, - fmt.Sprintf("`%s` output has no description", output.Name), - output.DeclRange, + fmt.Sprintf("`%s` output has no description", output.Labels[0]), + output.DefRange, ) } } diff --git a/rules/terraformrules/terraform_documented_variables.go b/rules/terraformrules/terraform_documented_variables.go index 470ac215a..dbe9f821f 100644 --- a/rules/terraformrules/terraform_documented_variables.go +++ b/rules/terraformrules/terraform_documented_variables.go @@ -4,6 +4,9 @@ import ( "fmt" "log" + "github.com/hashicorp/hcl/v2/gohcl" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -44,12 +47,43 @@ func (r *TerraformDocumentedVariablesRule) Check(runner *tflint.Runner) error { log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - for _, variable := range runner.TFConfig.Module.Variables { - if variable.Description == "" { + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "variable", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{{Name: "description"}}, + }, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + + for _, variable := range body.Blocks { + attr, exists := variable.Body.Attributes["description"] + if !exists { + runner.EmitIssue( + r, + fmt.Sprintf("`%s` variable has no description", variable.Labels[0]), + variable.DefRange, + ) + continue + } + + var description string + diags = gohcl.DecodeExpression(attr.Expr, nil, &description) + if diags.HasErrors() { + return diags + } + + if description == "" { runner.EmitIssue( r, - fmt.Sprintf("`%s` variable has no description", variable.Name), - variable.DeclRange, + fmt.Sprintf("`%s` variable has no description", variable.Labels[0]), + variable.DefRange, ) } } diff --git a/rules/terraformrules/terraform_module_pinned_source.go b/rules/terraformrules/terraform_module_pinned_source.go index e4fb5c6d2..b7c0de156 100644 --- a/rules/terraformrules/terraform_module_pinned_source.go +++ b/rules/terraformrules/terraform_module_pinned_source.go @@ -9,7 +9,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/hashicorp/go-getter" - "github.com/terraform-linters/tflint/terraform/configs" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -66,7 +66,17 @@ func (r *TerraformModulePinnedSourceRule) Check(runner *tflint.Runner) error { return err } - for _, module := range runner.TFConfig.Module.ModuleCalls { + body, diags := runner.GetModuleContent(moduleCallSchema, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + + for _, block := range body.Blocks { + module, diags := decodeModuleCall(block) + if diags.HasErrors() { + return diags + } + if err := r.checkModule(runner, module, config); err != nil { return err } @@ -75,10 +85,10 @@ func (r *TerraformModulePinnedSourceRule) Check(runner *tflint.Runner) error { return nil } -func (r *TerraformModulePinnedSourceRule) checkModule(runner *tflint.Runner, module *configs.ModuleCall, config terraformModulePinnedSourceRuleConfig) error { - log.Printf("[DEBUG] Walk `%s` attribute", module.Name+".source") +func (r *TerraformModulePinnedSourceRule) checkModule(runner *tflint.Runner, module *moduleCall, config terraformModulePinnedSourceRuleConfig) error { + log.Printf("[DEBUG] Walk `%s` attribute", module.name+".source") - source, err := getter.Detect(module.SourceAddrRaw, filepath.Dir(module.DeclRange.Filename), []getter.Detector{ + source, err := getter.Detect(module.source, filepath.Dir(module.defRange.Filename), []getter.Detector{ // https://github.com/hashicorp/terraform/blob/51b0aee36cc2145f45f5b04051a01eb6eb7be8bf/internal/getmodules/getter.go#L30-L52 new(getter.GitHubDetector), new(getter.GitDetector), @@ -116,8 +126,8 @@ func (r *TerraformModulePinnedSourceRule) checkModule(runner *tflint.Runner, mod if u.Hostname() == "" { runner.EmitIssue( r, - fmt.Sprintf("Module source %q is not a valid URL", module.SourceAddr), - module.SourceAddrRange, + fmt.Sprintf("Module source %q is not a valid URL", module.source), + module.sourceAttr.Expr.Range(), ) return nil @@ -135,14 +145,14 @@ func (r *TerraformModulePinnedSourceRule) checkModule(runner *tflint.Runner, mod runner.EmitIssue( r, - fmt.Sprintf(`Module source "%s" is not pinned`, module.SourceAddr), - module.SourceAddrRange, + fmt.Sprintf(`Module source "%s" is not pinned`, module.source), + module.sourceAttr.Expr.Range(), ) return nil } -func (r *TerraformModulePinnedSourceRule) checkRevision(runner *tflint.Runner, module *configs.ModuleCall, config terraformModulePinnedSourceRuleConfig, key string, value string) error { +func (r *TerraformModulePinnedSourceRule) checkRevision(runner *tflint.Runner, module *moduleCall, config terraformModulePinnedSourceRuleConfig, key string, value string) error { switch config.Style { // The "flexible" style requires a revision that is not a default branch case "flexible": @@ -150,8 +160,8 @@ func (r *TerraformModulePinnedSourceRule) checkRevision(runner *tflint.Runner, m if value == branch { runner.EmitIssue( r, - fmt.Sprintf("Module source \"%s\" uses a default branch as %s (%s)", module.SourceAddr, key, branch), - module.SourceAddrRange, + fmt.Sprintf("Module source \"%s\" uses a default branch as %s (%s)", module.source, key, branch), + module.sourceAttr.Expr.Range(), ) return nil @@ -163,8 +173,8 @@ func (r *TerraformModulePinnedSourceRule) checkRevision(runner *tflint.Runner, m if err != nil { runner.EmitIssue( r, - fmt.Sprintf("Module source \"%s\" uses a %s which is not a semantic version string", module.SourceAddr, key), - module.SourceAddrRange, + fmt.Sprintf("Module source \"%s\" uses a %s which is not a semantic version string", module.source, key), + module.sourceAttr.Expr.Range(), ) } default: diff --git a/rules/terraformrules/terraform_module_pinned_source_test.go b/rules/terraformrules/terraform_module_pinned_source_test.go index a277d326d..cb9d9afd3 100644 --- a/rules/terraformrules/terraform_module_pinned_source_test.go +++ b/rules/terraformrules/terraform_module_pinned_source_test.go @@ -144,7 +144,7 @@ module "unpinned" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::https://github.com/hashicorp/consul.git\" is not pinned", + Message: "Module source \"github.com/hashicorp/consul\" is not pinned", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, @@ -162,7 +162,7 @@ module "unpinned" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::ssh://git@github.com/hashicorp/consul.git\" is not pinned", + Message: "Module source \"git@github.com:hashicorp/consul.git\" is not pinned", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, @@ -180,7 +180,7 @@ module "default_git" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::https://github.com/hashicorp/consul.git?ref=master\" uses a default branch as ref (master)", + Message: "Module source \"github.com/hashicorp/consul.git?ref=master\" uses a default branch as ref (master)", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, @@ -219,7 +219,7 @@ rule "terraform_module_pinned_source" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::https://github.com/hashicorp/consul.git?ref=pinned\" uses a ref which is not a semantic version string", + Message: "Module source \"github.com/hashicorp/consul.git?ref=pinned\" uses a ref which is not a semantic version string", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, @@ -250,7 +250,7 @@ module "unpinned" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::https://bitbucket.org/hashicorp/tf-test-git.git\" is not pinned", + Message: "Module source \"bitbucket.org/hashicorp/tf-test-git\" is not pinned", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, @@ -268,7 +268,7 @@ module "default_git" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::https://bitbucket.org/hashicorp/tf-test-git.git?ref=master\" uses a default branch as ref (master)", + Message: "Module source \"bitbucket.org/hashicorp/tf-test-git.git?ref=master\" uses a default branch as ref (master)", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, @@ -299,7 +299,7 @@ rule "terraform_module_pinned_source" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::https://bitbucket.org/hashicorp/tf-test-git.git?ref=pinned\" uses a ref which is not a semantic version string", + Message: "Module source \"bitbucket.org/hashicorp/tf-test-git.git?ref=pinned\" uses a ref which is not a semantic version string", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, @@ -429,7 +429,7 @@ rule "terraform_module_pinned_source" { Expected: tflint.Issues{ { Rule: NewTerraformModulePinnedSourceRule(), - Message: "Module source \"git::https://github.com/hashicorp/consul.git?ref=foo\" uses a default branch as ref (foo)", + Message: "Module source \"github.com/hashicorp/consul.git?ref=foo\" uses a default branch as ref (foo)", Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{Line: 3, Column: 12}, diff --git a/rules/terraformrules/terraform_module_version.go b/rules/terraformrules/terraform_module_version.go index eeaabd50b..450236103 100644 --- a/rules/terraformrules/terraform_module_version.go +++ b/rules/terraformrules/terraform_module_version.go @@ -5,8 +5,8 @@ import ( "log" "regexp" - "github.com/terraform-linters/tflint/terraform/addrs" - "github.com/terraform-linters/tflint/terraform/configs" + tfaddr "github.com/hashicorp/terraform-registry-address" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -62,7 +62,17 @@ func (r *TerraformModuleVersionRule) Check(runner *tflint.Runner) error { return err } - for _, module := range runner.TFConfig.Module.ModuleCalls { + body, diags := runner.GetModuleContent(moduleCallSchema, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + + for _, block := range body.Blocks { + module, diags := decodeModuleCall(block) + if diags.HasErrors() { + return diags + } + if err := r.checkModule(runner, module, config); err != nil { return err } @@ -71,28 +81,26 @@ func (r *TerraformModuleVersionRule) Check(runner *tflint.Runner) error { return nil } -func (r *TerraformModuleVersionRule) checkModule(runner *tflint.Runner, module *configs.ModuleCall, config TerraformModuleVersionRuleConfig) error { - log.Printf("[DEBUG] Walk `%s` attribute", module.Name+".source") +func (r *TerraformModuleVersionRule) checkModule(runner *tflint.Runner, module *moduleCall, config TerraformModuleVersionRuleConfig) error { + log.Printf("[DEBUG] Walk `%s` attribute", module.name+".source") - source, err := addrs.ParseModuleSource(module.SourceAddrRaw) + _, err := tfaddr.ParseModuleSource(module.source) if err != nil { - return err - } - - switch source.(type) { - case addrs.ModuleSourceRegistry: - return r.checkVersion(runner, module, config) + // If parsing fails, the source does not expect to specify a version, + // such as local or remote. So instead of returning an error, + // it returns nil to stop the check. + return nil } - return nil + return r.checkVersion(runner, module, config) } -func (r *TerraformModuleVersionRule) checkVersion(runner *tflint.Runner, module *configs.ModuleCall, config TerraformModuleVersionRuleConfig) error { - if module.Version.Required == nil { +func (r *TerraformModuleVersionRule) checkVersion(runner *tflint.Runner, module *moduleCall, config TerraformModuleVersionRuleConfig) error { + if module.version == nil { runner.EmitIssue( r, - fmt.Sprintf("module %q should specify a version", module.Name), - module.DeclRange, + fmt.Sprintf("module %q should specify a version", module.name), + module.defRange, ) return nil @@ -102,21 +110,21 @@ func (r *TerraformModuleVersionRule) checkVersion(runner *tflint.Runner, module return nil } - if len(module.Version.Required) > 1 { + if len(module.version) > 1 { runner.EmitIssue( r, - fmt.Sprintf("module %q should specify an exact version, but multiple constraints were found", module.Name), - module.Version.DeclRange, + fmt.Sprintf("module %q should specify an exact version, but multiple constraints were found", module.name), + module.versionAttr.Range, ) return nil } - if !exactVersionRegexp.MatchString(module.Version.Required[0].String()) { + if !exactVersionRegexp.MatchString(module.version[0].String()) { runner.EmitIssue( r, - fmt.Sprintf("module %q should specify an exact version, but a range was found", module.Name), - module.Version.DeclRange, + fmt.Sprintf("module %q should specify an exact version, but a range was found", module.name), + module.versionAttr.Range, ) } diff --git a/rules/terraformrules/terraform_naming_convention.go b/rules/terraformrules/terraform_naming_convention.go index b940a5852..19fd13c54 100644 --- a/rules/terraformrules/terraform_naming_convention.go +++ b/rules/terraformrules/terraform_naming_convention.go @@ -7,6 +7,8 @@ import ( "strings" "github.com/hashicorp/hcl/v2" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -93,24 +95,48 @@ func (r *TerraformNamingConventionRule) Check(runner *tflint.Runner) error { var nameValidator *NameValidator + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "data", + LabelNames: []string{"type", "name"}, + Body: &hclext.BodySchema{}, + }, + { + Type: "module", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{}, + }, + { + Type: "output", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{}, + }, + { + Type: "resource", + LabelNames: []string{"type", "name"}, + Body: &hclext.BodySchema{}, + }, + { + Type: "variable", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{}, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + blocks := body.Blocks.ByType() + // data dataBlockName := "data" nameValidator, err = config.Data.getNameValidator(defaultNameValidator, &config, dataBlockName) if err != nil { return err } - for _, target := range runner.TFConfig.Module.DataResources { - nameValidator.checkBlock(runner, r, dataBlockName, target.Name, &target.DeclRange) - } - - // locals - localBlockName := "local value" - nameValidator, err = config.Locals.getNameValidator(defaultNameValidator, &config, localBlockName) - if err != nil { - return err - } - for _, target := range runner.TFConfig.Module.Locals { - nameValidator.checkBlock(runner, r, localBlockName, target.Name, &target.DeclRange) + for _, block := range blocks[dataBlockName] { + nameValidator.checkBlock(runner, r, dataBlockName, block.Labels[1], &block.DefRange) } // modules @@ -119,8 +145,8 @@ func (r *TerraformNamingConventionRule) Check(runner *tflint.Runner) error { if err != nil { return err } - for _, target := range runner.TFConfig.Module.ModuleCalls { - nameValidator.checkBlock(runner, r, moduleBlockName, target.Name, &target.DeclRange) + for _, block := range blocks[moduleBlockName] { + nameValidator.checkBlock(runner, r, moduleBlockName, block.Labels[0], &block.DefRange) } // outputs @@ -129,8 +155,8 @@ func (r *TerraformNamingConventionRule) Check(runner *tflint.Runner) error { if err != nil { return err } - for _, target := range runner.TFConfig.Module.Outputs { - nameValidator.checkBlock(runner, r, outputBlockName, target.Name, &target.DeclRange) + for _, block := range blocks[outputBlockName] { + nameValidator.checkBlock(runner, r, outputBlockName, block.Labels[0], &block.DefRange) } // resources @@ -139,8 +165,8 @@ func (r *TerraformNamingConventionRule) Check(runner *tflint.Runner) error { if err != nil { return err } - for _, target := range runner.TFConfig.Module.ManagedResources { - nameValidator.checkBlock(runner, r, resourceBlockName, target.Name, &target.DeclRange) + for _, block := range blocks[resourceBlockName] { + nameValidator.checkBlock(runner, r, resourceBlockName, block.Labels[1], &block.DefRange) } // variables @@ -149,8 +175,22 @@ func (r *TerraformNamingConventionRule) Check(runner *tflint.Runner) error { if err != nil { return err } - for _, target := range runner.TFConfig.Module.Variables { - nameValidator.checkBlock(runner, r, variableBlockName, target.Name, &target.DeclRange) + for _, block := range blocks[variableBlockName] { + nameValidator.checkBlock(runner, r, variableBlockName, block.Labels[0], &block.DefRange) + } + + // locals + localBlockName := "local value" + nameValidator, err = config.Locals.getNameValidator(defaultNameValidator, &config, localBlockName) + if err != nil { + return err + } + locals, diags := getLocals(runner) + if diags.HasErrors() { + return diags + } + for name, local := range locals { + nameValidator.checkBlock(runner, r, localBlockName, name, &local.defRange) } return nil diff --git a/rules/terraformrules/terraform_required_providers.go b/rules/terraformrules/terraform_required_providers.go index 754cfbb0b..b7370db8e 100644 --- a/rules/terraformrules/terraform_required_providers.go +++ b/rules/terraformrules/terraform_required_providers.go @@ -4,9 +4,11 @@ import ( "fmt" "log" - "github.com/hashicorp/hcl/v2" - "github.com/terraform-linters/tflint/terraform/configs" + tfaddr "github.com/hashicorp/terraform-registry-address" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" + "github.com/zclconf/go-cty/cty" ) // TerraformRequiredProvidersRule checks whether Terraform sets version constraints for all configured providers @@ -46,49 +48,109 @@ func (r *TerraformRequiredProvidersRule) Check(runner *tflint.Runner) error { log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - providers := make(map[string]hcl.Range) - module := runner.TFConfig.Module - - for _, provider := range module.ProviderConfigs { - if _, ok := providers[provider.Name]; !ok { - providers[provider.Name] = provider.DeclRange - } + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "provider", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + {Name: "version"}, + }, + }, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } - if provider.Version.Required != nil { + for _, provider := range body.Blocks { + if _, exists := provider.Body.Attributes["version"]; exists { runner.EmitIssue( r, - fmt.Sprintf(`%s: version constraint should be specified via "required_providers"`, provider.Addr().String()), - provider.DeclRange, + `provider version constraint should be specified via "required_providers"`, + provider.DefRange, ) } } - providers = providerResourceRanges(providers, module.ManagedResources) - providers = providerResourceRanges(providers, module.DataResources) + providerRefs, diags := getProviderRefs(runner) + if diags.HasErrors() { + return diags + } - for name, decl := range providers { - if provider, ok := module.ProviderRequirements.RequiredProviders[name]; !ok || provider.Requirement.Required == nil { - runner.EmitIssue(r, fmt.Sprintf(`Missing version constraint for provider "%s" in "required_providers"`, name), decl) - } + requiredProvidersSchema := []hclext.AttributeSchema{} + for name := range providerRefs { + requiredProvidersSchema = append(requiredProvidersSchema, hclext.AttributeSchema{Name: name}) } - return nil -} + body, diags = runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "terraform", + Body: &hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "required_providers", + Body: &hclext.BodySchema{ + Attributes: requiredProvidersSchema, + }, + }, + }, + }, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } -func providerResourceRanges(providers map[string]hcl.Range, resources map[string]*configs.Resource) map[string]hcl.Range { - for _, resource := range resources { - provider := resource.Provider + requiredProviders := hclext.Attributes{} + for _, terraform := range body.Blocks { + for _, requiredProvidersBlock := range terraform.Body.Blocks { + requiredProviders = requiredProvidersBlock.Body.Attributes + } + } + + for name, ref := range providerRefs { + if name == "terraform" { + // "terraform" provider is a builtin provider + // @see https://github.com/hashicorp/terraform/blob/v1.2.5/internal/addrs/provider.go#L106-L112 + continue + } - if provider.IsBuiltIn() { + provider, exists := requiredProviders[name] + if !exists { + runner.EmitIssue(r, fmt.Sprintf(`Missing version constraint for provider "%s" in "required_providers"`, name), ref.defRange) continue } - if _, ok := providers[provider.Type]; ok { + val, diags := provider.Expr.Value(nil) + if diags.HasErrors() { + return diags + } + // Look for a single static string, in case we have the legacy version-only + // format in the configuration. + if val.Type() == cty.String { continue } - providers[resource.Provider.Type] = resource.DeclRange + vm := val.AsValueMap() + if _, exists := vm["version"]; !exists { + if source, exists := vm["source"]; exists { + p, err := tfaddr.ParseProviderSource(source.AsString()) + if err != nil { + return err + } + + if p.IsBuiltIn() { + continue + } + } + runner.EmitIssue(r, fmt.Sprintf(`Missing version constraint for provider "%s" in "required_providers"`, name), ref.defRange) + } } - return providers + return nil } diff --git a/rules/terraformrules/terraform_required_providers_test.go b/rules/terraformrules/terraform_required_providers_test.go index 45906260d..24c2da8d7 100644 --- a/rules/terraformrules/terraform_required_providers_test.go +++ b/rules/terraformrules/terraform_required_providers_test.go @@ -190,7 +190,7 @@ provider "template" { Expected: tflint.Issues{ { Rule: NewTerraformRequiredProvidersRule(), - Message: `provider.template: version constraint should be specified via "required_providers"`, + Message: `provider version constraint should be specified via "required_providers"`, Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{ @@ -225,7 +225,7 @@ provider "template" { Expected: tflint.Issues{ { Rule: NewTerraformRequiredProvidersRule(), - Message: `provider.template.foo: version constraint should be specified via "required_providers"`, + Message: `provider version constraint should be specified via "required_providers"`, Range: hcl.Range{ Filename: "module.tf", Start: hcl.Pos{ @@ -262,6 +262,70 @@ resource "test_assertions" "foo" {} `, Expected: tflint.Issues{}, }, + { + Name: "resource provider ref", + Content: ` +terraform { + required_providers { + google = { + version = "~> 4.27.0" + } + } +} + +resource "google_compute_instance" "foo" { + provider = google-beta +}`, + Expected: tflint.Issues{ + { + Rule: NewTerraformRequiredProvidersRule(), + Message: `Missing version constraint for provider "google-beta" in "required_providers"`, + Range: hcl.Range{ + Filename: "module.tf", + Start: hcl.Pos{ + Line: 10, + Column: 1, + }, + End: hcl.Pos{ + Line: 10, + Column: 41, + }, + }, + }, + }, + }, + { + Name: "resource provider ref as string", + Content: ` +terraform { + required_providers { + google = { + version = "~> 4.27.0" + } + } +} + +resource "google_compute_instance" "foo" { + provider = "google-beta" +}`, + Expected: tflint.Issues{ + { + Rule: NewTerraformRequiredProvidersRule(), + Message: `Missing version constraint for provider "google-beta" in "required_providers"`, + Range: hcl.Range{ + Filename: "module.tf", + Start: hcl.Pos{ + Line: 10, + Column: 1, + }, + End: hcl.Pos{ + Line: 10, + Column: 41, + }, + }, + }, + }, + }, } rule := NewTerraformRequiredProvidersRule() diff --git a/rules/terraformrules/terraform_required_version.go b/rules/terraformrules/terraform_required_version.go index 0e7bd95fd..e33271176 100644 --- a/rules/terraformrules/terraform_required_version.go +++ b/rules/terraformrules/terraform_required_version.go @@ -4,6 +4,8 @@ import ( "log" "github.com/hashicorp/hcl/v2" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -44,15 +46,28 @@ func (r *TerraformRequiredVersionRule) Check(runner *tflint.Runner) error { log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - module := runner.TFConfig.Module - versionConstraints := module.CoreVersionConstraints - if len(versionConstraints) == 0 { - runner.EmitIssue( - r, - `terraform "required_version" attribute is required`, - hcl.Range{}, - ) - return nil + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "terraform", + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{{Name: "required_version"}}, + }, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + + for _, block := range body.Blocks { + if _, exists := block.Body.Attributes["required_version"]; !exists { + runner.EmitIssue( + r, + `terraform "required_version" attribute is required`, + hcl.Range{}, + ) + } } return nil diff --git a/rules/terraformrules/terraform_standard_module_structure.go b/rules/terraformrules/terraform_standard_module_structure.go index c51fb4d32..0d666c874 100644 --- a/rules/terraformrules/terraform_standard_module_structure.go +++ b/rules/terraformrules/terraform_standard_module_structure.go @@ -6,6 +6,8 @@ import ( "path/filepath" "github.com/hashicorp/hcl/v2" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -52,21 +54,43 @@ func (r *TerraformStandardModuleStructureRule) Check(runner *tflint.Runner) erro log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - r.checkFiles(runner) - r.checkVariables(runner) - r.checkOutputs(runner) + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "variable", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{}, + }, + { + Type: "output", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{}, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + + blocks := body.Blocks.ByType() + + r.checkFiles(runner, body.Blocks) + r.checkVariables(runner, blocks["variable"]) + r.checkOutputs(runner, blocks["output"]) return nil } -func (r *TerraformStandardModuleStructureRule) checkFiles(runner *tflint.Runner) { +func (r *TerraformStandardModuleStructureRule) checkFiles(runner *tflint.Runner, blocks hclext.Blocks) { if r.onlyJSON(runner) { return } f := runner.Files() + var dir string files := make(map[string]*hcl.File, len(f)) for name, file := range f { + dir = filepath.Dir(name) files[filepath.Base(name)] = file } @@ -77,54 +101,54 @@ func (r *TerraformStandardModuleStructureRule) checkFiles(runner *tflint.Runner) r, fmt.Sprintf("Module should include a %s file as the primary entrypoint", filenameMain), hcl.Range{ - Filename: filepath.Join(runner.TFConfig.Module.SourceDir, filenameMain), + Filename: filepath.Join(dir, filenameMain), Start: hcl.InitialPos, }, ) } - if files[filenameVariables] == nil && len(runner.TFConfig.Module.Variables) == 0 { + if files[filenameVariables] == nil && len(blocks.ByType()["variable"]) == 0 { runner.EmitIssue( r, fmt.Sprintf("Module should include an empty %s file", filenameVariables), hcl.Range{ - Filename: filepath.Join(runner.TFConfig.Module.SourceDir, filenameVariables), + Filename: filepath.Join(dir, filenameVariables), Start: hcl.InitialPos, }, ) } - if files[filenameOutputs] == nil && len(runner.TFConfig.Module.Outputs) == 0 { + if files[filenameOutputs] == nil && len(blocks.ByType()["output"]) == 0 { runner.EmitIssue( r, fmt.Sprintf("Module should include an empty %s file", filenameOutputs), hcl.Range{ - Filename: filepath.Join(runner.TFConfig.Module.SourceDir, filenameOutputs), + Filename: filepath.Join(dir, filenameOutputs), Start: hcl.InitialPos, }, ) } } -func (r *TerraformStandardModuleStructureRule) checkVariables(runner *tflint.Runner) { - for _, variable := range runner.TFConfig.Module.Variables { - if filename := variable.DeclRange.Filename; r.shouldMove(filename, filenameVariables) { +func (r *TerraformStandardModuleStructureRule) checkVariables(runner *tflint.Runner, variables hclext.Blocks) { + for _, variable := range variables { + if filename := variable.DefRange.Filename; r.shouldMove(filename, filenameVariables) { runner.EmitIssue( r, - fmt.Sprintf("variable %q should be moved from %s to %s", variable.Name, filename, filenameVariables), - variable.DeclRange, + fmt.Sprintf("variable %q should be moved from %s to %s", variable.Labels[0], filename, filenameVariables), + variable.DefRange, ) } } } -func (r *TerraformStandardModuleStructureRule) checkOutputs(runner *tflint.Runner) { - for _, variable := range runner.TFConfig.Module.Outputs { - if filename := variable.DeclRange.Filename; r.shouldMove(filename, filenameOutputs) { +func (r *TerraformStandardModuleStructureRule) checkOutputs(runner *tflint.Runner, outputs hclext.Blocks) { + for _, output := range outputs { + if filename := output.DefRange.Filename; r.shouldMove(filename, filenameOutputs) { runner.EmitIssue( r, - fmt.Sprintf("output %q should be moved from %s to %s", variable.Name, filename, filenameOutputs), - variable.DeclRange, + fmt.Sprintf("output %q should be moved from %s to %s", output.Labels[0], filename, filenameOutputs), + output.DefRange, ) } } diff --git a/rules/terraformrules/terraform_typed_variables.go b/rules/terraformrules/terraform_typed_variables.go index 2995b1ddf..590125845 100644 --- a/rules/terraformrules/terraform_typed_variables.go +++ b/rules/terraformrules/terraform_typed_variables.go @@ -4,7 +4,8 @@ import ( "fmt" "log" - "github.com/hashicorp/hcl/v2" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -45,50 +46,27 @@ func (r *TerraformTypedVariablesRule) Check(runner *tflint.Runner) error { log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - files := make(map[string]*struct{}) - for _, variable := range runner.TFConfig.Module.Variables { - files[variable.DeclRange.Filename] = nil - } - - for filename := range files { - if err := r.checkFileSchema(runner, filename); err != nil { - return err - } - } - - return nil -} - -func (r *TerraformTypedVariablesRule) checkFileSchema(runner *tflint.Runner, filename string) error { - file := runner.File(filename) - - content, _, diags := file.Body.PartialContent(&hcl.BodySchema{ - Blocks: []hcl.BlockHeaderSchema{ + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ { Type: "variable", LabelNames: []string{"name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{{Name: "type"}}, + }, }, }, - }) + }, sdk.GetModuleContentOption{}) if diags.HasErrors() { return diags } - for _, block := range content.Blocks.OfType("variable") { - _, _, diags := block.Body.PartialContent(&hcl.BodySchema{ - Attributes: []hcl.AttributeSchema{ - { - Name: "type", - Required: true, - }, - }, - }) - - if diags.HasErrors() { + for _, variable := range body.Blocks { + if _, exists := variable.Body.Attributes["type"]; !exists { runner.EmitIssue( r, - fmt.Sprintf("`%v` variable has no type", block.Labels[0]), - block.DefRange, + fmt.Sprintf("`%v` variable has no type", variable.Labels[0]), + variable.DefRange, ) } } diff --git a/rules/terraformrules/terraform_unused_declaration.go b/rules/terraformrules/terraform_unused_declaration.go index 4b935ba98..b8baa60ec 100644 --- a/rules/terraformrules/terraform_unused_declaration.go +++ b/rules/terraformrules/terraform_unused_declaration.go @@ -5,9 +5,8 @@ import ( "log" "github.com/hashicorp/hcl/v2" - "github.com/terraform-linters/tflint/terraform/addrs" - "github.com/terraform-linters/tflint/terraform/configs" - "github.com/terraform-linters/tflint/terraform/lang" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -15,9 +14,9 @@ import ( type TerraformUnusedDeclarationsRule struct{} type declarations struct { - Variables map[string]*configs.Variable - DataResources map[string]*configs.Resource - Locals map[string]*configs.Local + Variables map[string]*hclext.Block + DataResources map[string]*hclext.Block + Locals map[string]*local } // NewTerraformUnusedDeclarationsRule returns a new rule @@ -54,8 +53,11 @@ func (r *TerraformUnusedDeclarationsRule) Check(runner *tflint.Runner) error { log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - decl := r.declarations(runner.TFConfig.Module) - err := runner.WalkExpressions(func(expr hcl.Expression) error { + decl, err := r.declarations(runner) + if err != nil { + return err + } + err = runner.WalkExpressions(func(expr hcl.Expression) error { return r.checkForRefsInExpr(expr, decl) }) if err != nil { @@ -65,65 +67,85 @@ func (r *TerraformUnusedDeclarationsRule) Check(runner *tflint.Runner) error { for _, variable := range decl.Variables { runner.EmitIssue( r, - fmt.Sprintf(`variable "%s" is declared but not used`, variable.Name), - variable.DeclRange, + fmt.Sprintf(`variable "%s" is declared but not used`, variable.Labels[0]), + variable.DefRange, ) } for _, data := range decl.DataResources { runner.EmitIssue( r, - fmt.Sprintf(`data "%s" "%s" is declared but not used`, data.Type, data.Name), - data.DeclRange, + fmt.Sprintf(`data "%s" "%s" is declared but not used`, data.Labels[0], data.Labels[1]), + data.DefRange, ) } for _, local := range decl.Locals { runner.EmitIssue( r, - fmt.Sprintf(`local.%s is declared but not used`, local.Name), - local.DeclRange, + fmt.Sprintf(`local.%s is declared but not used`, local.name), + local.defRange, ) } return nil } -func (r *TerraformUnusedDeclarationsRule) declarations(module *configs.Module) *declarations { +func (r *TerraformUnusedDeclarationsRule) declarations(runner *tflint.Runner) (*declarations, error) { decl := &declarations{ - Variables: make(map[string]*configs.Variable, len(module.Variables)), - DataResources: make(map[string]*configs.Resource, len(module.DataResources)), - Locals: make(map[string]*configs.Local, len(module.Locals)), + Variables: map[string]*hclext.Block{}, + DataResources: map[string]*hclext.Block{}, + Locals: map[string]*local{}, } - for k, v := range module.Variables { - decl.Variables[k] = v + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "variable", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{}, + }, + { + Type: "data", + LabelNames: []string{"type", "name"}, + Body: &hclext.BodySchema{}, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return decl, diags } - for k, v := range module.DataResources { - decl.DataResources[k] = v + + for _, block := range body.Blocks { + if block.Type == "variable" { + decl.Variables[block.Labels[0]] = block + } else { + decl.DataResources[fmt.Sprintf("data.%s.%s", block.Labels[0], block.Labels[1])] = block + } } - for k, v := range module.Locals { - decl.Locals[k] = v + + locals, diags := getLocals(runner) + if diags.HasErrors() { + return decl, diags } + decl.Locals = locals - return decl + return decl, nil } func (r *TerraformUnusedDeclarationsRule) checkForRefsInExpr(expr hcl.Expression, decl *declarations) error { - refs, diags := lang.ReferencesInExpr(expr) + refs, diags := referencesInExpr(expr) if diags.HasErrors() { - log.Printf("[DEBUG] Cannot find references in expression, ignoring: %v", diags.Err()) + log.Printf("[DEBUG] Cannot find references in expression, ignoring: %v", diags) return nil } for _, ref := range refs { - switch sub := ref.Subject.(type) { - case addrs.InputVariable: - delete(decl.Variables, sub.Name) - case addrs.LocalValue: - delete(decl.Locals, sub.Name) - case addrs.Resource: + switch sub := ref.subject.(type) { + case inputVariableReference: + delete(decl.Variables, sub.name) + case localValueReference: + delete(decl.Locals, sub.name) + case dataResourceReference: delete(decl.DataResources, sub.String()) - case addrs.ResourceInstance: - delete(decl.DataResources, sub.Resource.String()) } } diff --git a/rules/terraformrules/terraform_unused_required_providers.go b/rules/terraformrules/terraform_unused_required_providers.go index 61881f2e2..2d523e6fa 100644 --- a/rules/terraformrules/terraform_unused_required_providers.go +++ b/rules/terraformrules/terraform_unused_required_providers.go @@ -4,7 +4,7 @@ import ( "fmt" "log" - "github.com/terraform-linters/tflint/terraform/configs" + "github.com/hashicorp/hcl/v2" "github.com/terraform-linters/tflint/tflint" ) @@ -45,51 +45,53 @@ func (r *TerraformUnusedRequiredProvidersRule) Check(runner *tflint.Runner) erro log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - for _, required := range runner.TFConfig.Module.ProviderRequirements.RequiredProviders { - r.checkProvider(runner, required) + providerRefs, diags := getProviderRefs(runner) + if diags.HasErrors() { + return diags } - return nil -} - -func (r *TerraformUnusedRequiredProvidersRule) checkProvider(runner *tflint.Runner, required *configs.RequiredProvider) { - for _, resource := range runner.TFConfig.Module.ManagedResources { - if r.usesProvider(resource, required) { - return - } - } - - for _, resource := range runner.TFConfig.Module.DataResources { - if r.usesProvider(resource, required) { - return + requiredProviders := hcl.Attributes{} + for _, file := range runner.Files() { + content, _, schemaDiags := file.Body.PartialContent(&hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{{Type: "terraform"}}, + }) + diags = diags.Extend(schemaDiags) + if diags.HasErrors() { + continue } - } - for _, provider := range runner.TFConfig.Module.ProviderConfigs { - if required.Name == provider.Name { - return - } - } + for _, block := range content.Blocks { + content, _, schemaDiags = block.Body.PartialContent(&hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{{Type: "required_providers"}}, + }) + diags = diags.Extend(schemaDiags) + if diags.HasErrors() { + continue + } - for _, module := range runner.TFConfig.Module.ModuleCalls { - for _, provider := range module.Providers { - if provider.InParent.Name == required.Name { - return + for _, block := range content.Blocks { + var attrDiags hcl.Diagnostics + requiredProviders, attrDiags = block.Body.JustAttributes() + diags = diags.Extend(attrDiags) + if diags.HasErrors() { + continue + } } } } + if diags.HasErrors() { + return diags + } - runner.EmitIssue( - r, - fmt.Sprintf("provider '%s' is declared in required_providers but not used by the module", required.Name), - required.DeclRange, - ) -} - -func (r *TerraformUnusedRequiredProvidersRule) usesProvider(resource *configs.Resource, required *configs.RequiredProvider) bool { - if resource.ProviderConfigRef != nil { - return resource.ProviderConfigRef.Name == required.Name + for _, required := range requiredProviders { + if _, exists := providerRefs[required.Name]; !exists { + runner.EmitIssue( + r, + fmt.Sprintf("provider '%s' is declared in required_providers but not used by the module", required.Name), + required.Range, + ) + } } - return resource.Provider.Type == required.Name + return nil } diff --git a/rules/terraformrules/terraform_unused_required_providers_test.go b/rules/terraformrules/terraform_unused_required_providers_test.go index 478b10e94..8b21989d9 100644 --- a/rules/terraformrules/terraform_unused_required_providers_test.go +++ b/rules/terraformrules/terraform_unused_required_providers_test.go @@ -159,7 +159,7 @@ func Test_TerraformUnusedRequiredProvidersRule(t *testing.T) { Filename: "module.tf", Start: hcl.Pos{ Line: 4, - Column: 14, + Column: 7, }, End: hcl.Pos{ Line: 6, @@ -196,7 +196,7 @@ func Test_TerraformUnusedRequiredProvidersRule(t *testing.T) { Filename: "module.tf", Start: hcl.Pos{ Line: 4, - Column: 14, + Column: 7, }, End: hcl.Pos{ Line: 6, @@ -229,7 +229,7 @@ func Test_TerraformUnusedRequiredProvidersRule(t *testing.T) { Filename: "module.tf", Start: hcl.Pos{ Line: 4, - Column: 14, + Column: 7, }, End: hcl.Pos{ Line: 6, diff --git a/rules/terraformrules/terraform_workspace_remote.go b/rules/terraformrules/terraform_workspace_remote.go index d3ee3b84b..e010858b1 100644 --- a/rules/terraformrules/terraform_workspace_remote.go +++ b/rules/terraformrules/terraform_workspace_remote.go @@ -6,8 +6,8 @@ import ( "github.com/hashicorp/hcl/v2" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/json" - "github.com/terraform-linters/tflint/terraform/addrs" - "github.com/terraform-linters/tflint/terraform/lang" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" "github.com/terraform-linters/tflint/tflint" ) @@ -49,8 +49,35 @@ func (r *TerraformWorkspaceRemoteRule) Check(runner *tflint.Runner) error { log.Printf("[TRACE] Check `%s` rule for `%s` runner", r.Name(), runner.TFConfigPath()) - backend := runner.TFConfig.Root.Module.Backend - if backend == nil || backend.Type != "remote" { + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "terraform", + Body: &hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "backend", + LabelNames: []string{"type"}, + Body: &hclext.BodySchema{}, + }, + }, + }, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return diags + } + + var remoteBackend bool + for _, terraform := range body.Blocks { + for _, backend := range terraform.Body.Blocks { + if backend.Labels[0] == "remote" { + remoteBackend = true + } + } + } + if !remoteBackend { return nil } @@ -65,16 +92,15 @@ func (r *TerraformWorkspaceRemoteRule) checkForTerraformWorkspaceInExpr(runner * return nil } - refs, diags := lang.ReferencesInExpr(expr) + refs, diags := referencesInExpr(expr) if diags.HasErrors() { - log.Printf("[DEBUG] Cannot find references in expression, ignoring: %v", diags.Err()) - return nil + return diags } for _, ref := range refs { - switch sub := ref.Subject.(type) { - case addrs.TerraformAttr: - if sub.Name == "workspace" { + switch sub := ref.subject.(type) { + case terraformReference: + if sub.name == "workspace" { runner.EmitIssue( r, "terraform.workspace should not be used with a 'remote' backend", diff --git a/rules/terraformrules/utils.go b/rules/terraformrules/utils.go new file mode 100644 index 000000000..b08a3439e --- /dev/null +++ b/rules/terraformrules/utils.go @@ -0,0 +1,407 @@ +package terraformrules + +import ( + "fmt" + "strings" + + "github.com/hashicorp/go-version" + "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/gohcl" + "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/terraform-linters/tflint-plugin-sdk/hclext" + sdk "github.com/terraform-linters/tflint-plugin-sdk/tflint" + "github.com/terraform-linters/tflint/tflint" +) + +type moduleCall struct { + name string + defRange hcl.Range + source string + sourceAttr *hclext.Attribute + version version.Constraints + versionAttr *hclext.Attribute +} + +func decodeModuleCall(block *hclext.Block) (*moduleCall, hcl.Diagnostics) { + module := &moduleCall{ + name: block.Labels[0], + defRange: block.DefRange, + } + diags := hcl.Diagnostics{} + + if source, exists := block.Body.Attributes["source"]; exists { + module.sourceAttr = source + sourceDiags := gohcl.DecodeExpression(source.Expr, nil, &module.source) + diags = diags.Extend(sourceDiags) + } + + if versionAttr, exists := block.Body.Attributes["version"]; exists { + module.versionAttr = versionAttr + + var versionVal string + versionDiags := gohcl.DecodeExpression(versionAttr.Expr, nil, &versionVal) + diags = diags.Extend(versionDiags) + if diags.HasErrors() { + return module, diags + } + + constraints, err := version.NewConstraint(versionVal) + if err != nil { + diags = append(diags, &hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid version constraint", + Detail: "This string does not use correct version constraint syntax.", + Subject: versionAttr.Expr.Range().Ptr(), + }) + } + module.version = constraints + } + + return module, diags +} + +var moduleCallSchema = &hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "module", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + {Name: "source"}, + {Name: "version"}, + }, + }, + }, + }, +} + +type local struct { + name string + defRange hcl.Range +} + +func getLocals(runner *tflint.Runner) (map[string]*local, hcl.Diagnostics) { + locals := map[string]*local{} + diags := hcl.Diagnostics{} + + for _, file := range runner.Files() { + content, _, schemaDiags := file.Body.PartialContent(&hcl.BodySchema{ + Blocks: []hcl.BlockHeaderSchema{{Type: "locals"}}, + }) + diags = diags.Extend(schemaDiags) + if diags.HasErrors() { + continue + } + + for _, block := range content.Blocks { + attrs, localsDiags := block.Body.JustAttributes() + diags = diags.Extend(localsDiags) + if diags.HasErrors() { + continue + } + + for name, attr := range attrs { + locals[name] = &local{ + name: attr.Name, + defRange: attr.Range, + } + } + } + } + + return locals, diags +} + +type providerRef struct { + name string + defRange hcl.Range +} + +func getProviderRefs(runner *tflint.Runner) (map[string]*providerRef, hcl.Diagnostics) { + providerRefs := map[string]*providerRef{} + + body, diags := runner.GetModuleContent(&hclext.BodySchema{ + Blocks: []hclext.BlockSchema{ + { + Type: "resource", + LabelNames: []string{"type", "name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + {Name: "provider"}, + }, + }, + }, + { + Type: "data", + LabelNames: []string{"type", "name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + {Name: "provider"}, + }, + }, + }, + { + Type: "provider", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{}, + }, + { + Type: "module", + LabelNames: []string{"name"}, + Body: &hclext.BodySchema{ + Attributes: []hclext.AttributeSchema{ + {Name: "providers"}, + }, + }, + }, + }, + }, sdk.GetModuleContentOption{}) + if diags.HasErrors() { + return providerRefs, diags + } + + for _, block := range body.Blocks { + switch block.Type { + case "resource": + fallthrough + case "data": + if attr, exists := block.Body.Attributes["provider"]; exists { + ref, decodeDiags := decodeProviderRef(attr.Expr, block.DefRange) + diags = diags.Extend(decodeDiags) + if diags.HasErrors() { + continue + } + providerRefs[ref.name] = ref + } else { + providerName := block.Labels[0] + if under := strings.Index(providerName, "_"); under != -1 { + providerName = providerName[:under] + } + providerRefs[providerName] = &providerRef{ + name: providerName, + defRange: block.DefRange, + } + } + case "provider": + providerRefs[block.Labels[0]] = &providerRef{ + name: block.Labels[0], + defRange: block.DefRange, + } + case "module": + if attr, exists := block.Body.Attributes["providers"]; exists { + pairs, mapDiags := hcl.ExprMap(attr.Expr) + diags = diags.Extend(mapDiags) + if diags.HasErrors() { + continue + } + + for _, pair := range pairs { + ref, decodeDiags := decodeProviderRef(pair.Value, block.DefRange) + diags = diags.Extend(decodeDiags) + if diags.HasErrors() { + continue + } + providerRefs[ref.name] = ref + } + } + } + } + + return providerRefs, nil +} + +func decodeProviderRef(expr hcl.Expression, defRange hcl.Range) (*providerRef, hcl.Diagnostics) { + expr, diags := shimTraversalInString(expr) + if diags.HasErrors() { + return nil, diags + } + + traversal, diags := hcl.AbsTraversalForExpr(expr) + if diags.HasErrors() { + return nil, diags + } + + return &providerRef{ + name: traversal.RootName(), + defRange: defRange, + }, nil +} + +type reference struct { + subject referencable +} + +type referencable interface { + referenceableSigil() +} + +type referenceable struct { +} + +func (r referenceable) referenceableSigil() { +} + +type inputVariableReference struct { + referencable + name string +} + +type localValueReference struct { + referencable + name string +} + +type terraformReference struct { + referencable + name string +} + +type dataResourceReference struct { + referencable + typeName string + name string +} + +func (d *dataResourceReference) String() string { + return fmt.Sprintf("data.%s.%s", d.typeName, d.name) +} + +// @see https://github.com/hashicorp/terraform/blob/v1.2.5/internal/lang/references.go#L75 +func referencesInExpr(expr hcl.Expression) ([]*reference, hcl.Diagnostics) { + references := []*reference{} + diags := hcl.Diagnostics{} + + for _, traversal := range expr.Variables() { + name := traversal.RootName() + + switch name { + case "var": + name, parseDiags := parseSingleAttrRef(traversal) + diags = diags.Extend(parseDiags) + references = append(references, &reference{subject: inputVariableReference{name: name}}) + case "local": + name, parseDiags := parseSingleAttrRef(traversal) + diags = diags.Extend(parseDiags) + references = append(references, &reference{subject: localValueReference{name: name}}) + case "terraform": + name, parseDiags := parseSingleAttrRef(traversal) + diags = diags.Extend(parseDiags) + references = append(references, &reference{subject: terraformReference{name: name}}) + case "data": + if len(traversal) < 3 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: `The "data" object must be followed by two attribute names: the data source type and the resource name.`, + Subject: traversal.SourceRange().Ptr(), + }) + continue + } + + var typeName string + switch tt := traversal[1].(type) { + case hcl.TraverseRoot: + typeName = tt.Name + case hcl.TraverseAttr: + typeName = tt.Name + default: + // If it isn't a TraverseRoot then it must be a "data" reference. + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: `The "data" object does not support this operation.`, + Subject: traversal[1].SourceRange().Ptr(), + }) + continue + } + + attrTrav, ok := traversal[2].(hcl.TraverseAttr) + if !ok { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: `A reference to a data source must be followed by at least one attribute access, specifying the resource name.`, + Subject: traversal[1].SourceRange().Ptr(), + }) + continue + } + + references = append(references, &reference{subject: dataResourceReference{typeName: typeName, name: attrTrav.Name}}) + } + } + + return references, diags +} + +// @see https://github.com/hashicorp/terraform/blob/v1.2.5/internal/addrs/parse_ref.go#L392 +func parseSingleAttrRef(traversal hcl.Traversal) (string, hcl.Diagnostics) { + var diags hcl.Diagnostics + + root := traversal.RootName() + rootRange := traversal[0].SourceRange() + + if len(traversal) < 2 { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: fmt.Sprintf("The %q object cannot be accessed directly. Instead, access one of its attributes.", root), + Subject: &rootRange, + }) + return "", diags + } + if attrTrav, ok := traversal[1].(hcl.TraverseAttr); ok { + return attrTrav.Name, diags + } + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Invalid reference", + Detail: fmt.Sprintf("The %q object does not support this operation.", root), + Subject: traversal[1].SourceRange().Ptr(), + }) + return "", diags +} + +// @see https://github.com/hashicorp/terraform/blob/v1.2.5/internal/configs/compat_shim.go#L34 +func shimTraversalInString(expr hcl.Expression) (hcl.Expression, hcl.Diagnostics) { + // ObjectConsKeyExpr is a special wrapper type used for keys on object + // constructors to deal with the fact that naked identifiers are normally + // handled as "bareword" strings rather than as variable references. Since + // we know we're interpreting as a traversal anyway (and thus it won't + // matter whether it's a string or an identifier) we can safely just unwrap + // here and then process whatever we find inside as normal. + if ocke, ok := expr.(*hclsyntax.ObjectConsKeyExpr); ok { + expr = ocke.Wrapped + } + + if _, ok := expr.(*hclsyntax.TemplateExpr); !ok { + return expr, nil + } + + strVal, diags := expr.Value(nil) + if diags.HasErrors() || strVal.IsNull() || !strVal.IsKnown() { + // Since we're not even able to attempt a shim here, we'll discard + // the diagnostics we saw so far and let the caller's own error + // handling take care of reporting the invalid expression. + return expr, nil + } + + // The position handling here isn't _quite_ right because it won't + // take into account any escape sequences in the literal string, but + // it should be close enough for any error reporting to make sense. + srcRange := expr.Range() + startPos := srcRange.Start // copy + startPos.Column++ // skip initial quote + startPos.Byte++ // skip initial quote + + traversal, tDiags := hclsyntax.ParseTraversalAbs( + []byte(strVal.AsString()), + srcRange.Filename, + startPos, + ) + diags = append(diags, tDiags...) + + return &hclsyntax.ScopeTraversalExpr{ + Traversal: traversal, + SrcRange: srcRange, + }, diags +}