From 65798c7e5cced645e18f292e9edee9398e4acfc5 Mon Sep 17 00:00:00 2001 From: Nikita Pivkin Date: Fri, 15 Sep 2023 11:31:03 +0700 Subject: [PATCH] fix(terraform): convert input variables to expected type (#1453) * fix(terraform): convert input variables to expected type * do not use errors.Join --- pkg/scanners/terraform/parser/evaluator.go | 54 ++++++-- pkg/scanners/terraform/parser/parser_test.go | 129 +++++++++++++++++++ pkg/terraform/attribute.go | 9 ++ test/module_test.go | 2 +- 4 files changed, 182 insertions(+), 12 deletions(-) diff --git a/pkg/scanners/terraform/parser/evaluator.go b/pkg/scanners/terraform/parser/evaluator.go index 3ec73d3d7..a0bc6b872 100644 --- a/pkg/scanners/terraform/parser/evaluator.go +++ b/pkg/scanners/terraform/parser/evaluator.go @@ -2,7 +2,7 @@ package parser import ( "context" - "fmt" + "errors" "io/fs" "reflect" "time" @@ -15,7 +15,9 @@ import ( "github.com/aquasecurity/defsec/pkg/terraform" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/typeexpr" "github.com/zclconf/go-cty/cty" + "github.com/zclconf/go-cty/cty/convert" "github.com/zclconf/go-cty/cty/gocty" ) @@ -337,29 +339,59 @@ func (e *evaluator) copyVariables(from, to *terraform.Block) { func (e *evaluator) evaluateVariable(b *terraform.Block) (cty.Value, error) { if b.Label() == "" { - return cty.NilVal, fmt.Errorf("empty label - cannot resolve") - } - if override, exists := e.inputVars[b.Label()]; exists { - return override, nil + return cty.NilVal, errors.New("empty label - cannot resolve") } + attributes := b.Attributes() if attributes == nil { - return cty.NilVal, fmt.Errorf("cannot resolve variable with no attributes") + return cty.NilVal, errors.New("cannot resolve variable with no attributes") + } + + var valType cty.Type + var defaults *typeexpr.Defaults + if typeAttr, exists := attributes["type"]; exists { + ty, def, err := typeAttr.DecodeVarType() + if err != nil { + return cty.NilVal, err + } + valType = ty + defaults = def + } + + var val cty.Value + + if override, exists := e.inputVars[b.Label()]; exists { + val = override + } else if def, exists := attributes["default"]; exists { + val = def.NullableValue() + } else { + return cty.NilVal, errors.New("no value found") } - if def, exists := attributes["default"]; exists { - return def.NullableValue(), nil + + if valType != cty.NilType { + if defaults != nil { + val = defaults.Apply(val) + } + + typedVal, err := convert.Convert(val, valType) + if err != nil { + return cty.NilVal, err + } + return typedVal, nil } - return cty.NilVal, fmt.Errorf("no value found") + + return val, nil + } func (e *evaluator) evaluateOutput(b *terraform.Block) (cty.Value, error) { if b.Label() == "" { - return cty.NilVal, fmt.Errorf("empty label - cannot resolve") + return cty.NilVal, errors.New("empty label - cannot resolve") } attribute := b.GetAttribute("value") if attribute.IsNil() { - return cty.NilVal, fmt.Errorf("cannot resolve variable with no attributes") + return cty.NilVal, errors.New("cannot resolve output with no attributes") } return attribute.Value(), nil } diff --git a/pkg/scanners/terraform/parser/parser_test.go b/pkg/scanners/terraform/parser/parser_test.go index 2f18f8b63..f717ad344 100644 --- a/pkg/scanners/terraform/parser/parser_test.go +++ b/pkg/scanners/terraform/parser/parser_test.go @@ -729,3 +729,132 @@ resource "aws_s3_bucket_server_side_encryption_configuration" "this2" { assert.NotEmpty(t, attr.Value().AsString()) } } + +func Test_ForEachRefToLocals(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": ` +locals { + buckets = toset([ + "foo", + "bar", + ]) +} + +resource "aws_s3_bucket" "this" { + for_each = local.buckets + bucket = each.key +} +`, + }) + + parser := New(fs, "", OptionStopOnHCLError(true)) + if err := parser.ParseFS(context.TODO(), "."); err != nil { + t.Fatal(err) + } + modules, _, err := parser.EvaluateAll(context.TODO()) + assert.NoError(t, err) + assert.Len(t, modules, 1) + + rootModule := modules[0] + + blocks := rootModule.GetResourcesByType("aws_s3_bucket") + assert.Len(t, blocks, 2) + + for _, block := range blocks { + attr := block.GetAttribute("bucket") + require.NotNil(t, attr) + assert.Contains(t, []string{"foo", "bar"}, attr.AsStringValueOrDefault("", block).Value()) + } +} + +func Test_ForEachRefToVariableWithDefault(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": ` +variable "buckets" { + type = set(string) + default = ["foo", "bar"] +} + +resource "aws_s3_bucket" "this" { + for_each = var.buckets + bucket = each.key +} +`, + }) + + parser := New(fs, "", OptionStopOnHCLError(true)) + if err := parser.ParseFS(context.TODO(), "."); err != nil { + t.Fatal(err) + } + modules, _, err := parser.EvaluateAll(context.TODO()) + assert.NoError(t, err) + assert.Len(t, modules, 1) + + rootModule := modules[0] + + blocks := rootModule.GetResourcesByType("aws_s3_bucket") + assert.Len(t, blocks, 2) + + for _, block := range blocks { + attr := block.GetAttribute("bucket") + require.NotNil(t, attr) + assert.Contains(t, []string{"foo", "bar"}, attr.AsStringValueOrDefault("", block).Value()) + } +} + +func Test_ForEachRefToVariableFromFile(t *testing.T) { + fs := testutil.CreateFS(t, map[string]string{ + "main.tf": ` +variable "policy_rules" { + type = object({ + secure_tags = optional(map(object({ + session_matcher = optional(string) + priority = number + enabled = optional(bool, true) + })), {}) + }) +} + +resource "google_network_security_gateway_security_policy_rule" "secure_tag_rules" { + for_each = var.policy_rules.secure_tags + provider = google-beta + project = "test" + name = each.key + enabled = each.value.enabled + priority = each.value.priority + session_matcher = each.value.session_matcher +} +`, + "main.tfvars": ` +policy_rules = { + secure_tags = { + secure-tag-1 = { + session_matcher = "host() != 'google.com'" + priority = 1001 + } + } +} +`, + }) + + parser := New(fs, "", OptionStopOnHCLError(true)) + parser.SetTFVarsPaths("main.tfvars") + if err := parser.ParseFS(context.TODO(), "."); err != nil { + t.Fatal(err) + } + modules, _, err := parser.EvaluateAll(context.TODO()) + assert.NoError(t, err) + assert.Len(t, modules, 1) + + rootModule := modules[0] + + blocks := rootModule.GetResourcesByType("google_network_security_gateway_security_policy_rule") + assert.Len(t, blocks, 1) + + block := blocks[0] + + assert.Equal(t, "secure-tag-1", block.GetAttribute("name").AsStringValueOrDefault("", block).Value()) + assert.Equal(t, true, block.GetAttribute("enabled").AsBoolValueOrDefault(false, block).Value()) + assert.Equal(t, "host() != 'google.com'", block.GetAttribute("session_matcher").AsStringValueOrDefault("", block).Value()) + assert.Equal(t, 1001, block.GetAttribute("priority").AsIntValueOrDefault(0, block).Value()) +} diff --git a/pkg/terraform/attribute.go b/pkg/terraform/attribute.go index df4d26f97..6631c65ea 100644 --- a/pkg/terraform/attribute.go +++ b/pkg/terraform/attribute.go @@ -12,6 +12,7 @@ import ( defsecTypes "github.com/aquasecurity/defsec/pkg/types" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/ext/typeexpr" "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/zclconf/go-cty/cty" "github.com/zclconf/go-cty/cty/gocty" @@ -25,6 +26,14 @@ type Attribute struct { reference Reference } +func (a *Attribute) DecodeVarType() (cty.Type, *typeexpr.Defaults, error) { + t, def, diag := typeexpr.TypeConstraintWithDefaults(a.hclAttribute.Expr) + if diag.HasErrors() { + return cty.NilType, nil, diag + } + return t, def, nil +} + func NewAttribute(attr *hcl.Attribute, ctx *context.Context, module string, parent defsecTypes.Metadata, parentRef Reference, moduleSource string, moduleFS fs.FS) *Attribute { rng := defsecTypes.NewRange( attr.Range.Filename, diff --git a/test/module_test.go b/test/module_test.go index 8fc5d387f..feadc5ee5 100644 --- a/test/module_test.go +++ b/test/module_test.go @@ -604,7 +604,7 @@ module "something" { `, "modules/a/main.tf": ` variable "group" { - type = "string" + type = string } resource aws_iam_group_policy mfa {