Skip to content

Commit

Permalink
fixes: recursive loop when parent and child module has same local block
Browse files Browse the repository at this point in the history
  • Loading branch information
Rchanger committed Jun 29, 2021
1 parent b8fda7c commit 896408e
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 10 deletions.
17 changes: 15 additions & 2 deletions pkg/iac-providers/terraform/commons/local-references.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package commons

import (
"encoding/json"
"io/ioutil"
"reflect"
"regexp"
Expand Down Expand Up @@ -86,8 +87,20 @@ func (r *RefResolver) ResolveLocalRef(localRef, callerRef string) interface{} {
}

// replace the local value reference string with actual value
if reflect.TypeOf(val).Kind() == reflect.String {
valStr := val.(string)
if reflect.TypeOf(val).Kind() == reflect.String || reflect.TypeOf(val).Kind() == reflect.Map {
valStr := ""

if reflect.TypeOf(val).Kind() == reflect.Map {
data, err := json.Marshal(val)
if err != nil {
zap.S().Errorf("failed to convert expression '%v', ref: '%v'", localAttr.Expr, localRef)
return localRef
}
valStr = string(data)
} else {
valStr = val.(string)
}

resolvedVal := strings.Replace(localRef, localExpr, valStr, 1)
if callerRef != "" && strings.Contains(resolvedVal, callerRef) {
zap.S().Debugf("resolved str local value ref: '%v', value: '%v'", localRef, resolvedVal)
Expand Down
15 changes: 14 additions & 1 deletion pkg/iac-providers/terraform/commons/references.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
package commons

import (
"encoding/json"
"reflect"
"regexp"

hclConfigs "github.com/hashicorp/terraform/configs"
"go.uber.org/zap"
)

var (
Expand Down Expand Up @@ -67,8 +69,19 @@ func (r *RefResolver) ResolveRefs(config jsonObj) jsonObj {

// case 1: config value is a string; in resource config, refs
// are of the type string
config[k] = r.ResolveStrRef(v.(string), "")
value := r.ResolveStrRef(v.(string), "")

// check if value is string and can be converted to map
if valueStr, ok := value.(string); ok {
var temp jsonObj
err := json.Unmarshal([]byte(valueStr), &temp)
if err == nil {
value = temp
}
zap.S().Debugf("resolved value is not a json object", err)
}

config[k] = value
case vType == "tfv12.jsonObj" && vKind == reflect.Map:

// case 2: config value is of type jsonObj
Expand Down
27 changes: 25 additions & 2 deletions pkg/iac-providers/terraform/commons/variable-references.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package commons

import (
"encoding/json"
"io/ioutil"
"reflect"
"regexp"
Expand Down Expand Up @@ -87,8 +88,20 @@ func (r *RefResolver) ResolveVarRef(varRef, callerRef string) interface{} {
}
zap.S().Debugf("resolved variable ref '%v', value: '%v'", varRef, val)

if reflect.TypeOf(val).Kind() == reflect.String {
valStr := val.(string)
if reflect.TypeOf(val).Kind() == reflect.String || reflect.TypeOf(val).Kind() == reflect.Map {
valStr := ""

if reflect.TypeOf(val).Kind() == reflect.Map {
data, err := json.Marshal(val)
if err != nil {
zap.S().Errorf("failed to convert expression '%v', ref: '%v'", hclVar, varRef)
return varRef
}
valStr = string(data)
} else {
valStr = val.(string)
}

resolvedVal := strings.Replace(varRef, varExpr, valStr, 1)
if varRef == resolvedVal {
zap.S().Debugf("resolved str variable ref refers to self: '%v'", varRef)
Expand Down Expand Up @@ -150,6 +163,16 @@ func (r *RefResolver) ResolveVarRefFromParentModuleCall(varRef, callerRef string
// replace the variable reference string with actual value
if reflect.TypeOf(val).Kind() == reflect.String {
valStr := val.(string)

// if resolved variable value from parent module is local reference get the local value from parent module
if isLocalRef(valStr) {
t := NewRefResolver(r.Config.Parent, nil)
localVal := t.ResolveLocalRef(valStr, varRef)
if reflect.TypeOf(localVal).Kind() == reflect.String {
valStr = localVal.(string)
}
}

resolvedVal := strings.Replace(varRef, varExpr, valStr, 1)
if strings.Contains(valStr, varExpr) {
zap.S().Debugf("resolved str variable ref refers to self: '%v'", varRef)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,18 @@
"kms_data_key_reuse_period_seconds": 300,
"kms_master_key_id": "alias/aws/sqs",
"name": "terraform-example-queue",
"policy": "{\n \"Version\": \"2012-10-17\",\n \"Statement\": [{\n \"Sid\":\"Queue1_AnonymousAccess_AllActions_WhitelistIP\",\n \"Effect\": \"Allow\",\n \"Principal\": \"*\",\n \"Action\": \"sqs:*\",\n \"Resource\": \"arn:aws:sqs:*:111122223333:queue1\"\n }] \n}\n"
"policy": {
"Statement": [
{
"Action": "sqs:*",
"Effect": "Allow",
"Principal": "*",
"Resource": "arn:aws:sqs:*:111122223333:queue1",
"Sid": "Queue1_AnonymousAccess_AllActions_WhitelistIP"
}
],
"Version": "2012-10-17"
}
},
"skip_rules": null,
"max_severity": "",
Expand Down
8 changes: 8 additions & 0 deletions pkg/iac-providers/terraform/v14/load-dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,14 @@ func TestLoadIacDir(t *testing.T) {
nonRecursive: true,
wantErr: nilMultiErr,
},
{
name: "recursive loop while resolving locals with same name in parent and child module",
tfConfigDir: filepath.Join(testDataDir, "recursive-loop-duplicate-locals"),
tfJSONFile: filepath.Join(tfJSONDir, "recursive-loop-duplicate-locals.json"),
tfv14: TfV14{},
nonRecursive: true,
wantErr: nilMultiErr,
},
}

for _, tt := range table2 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
variable "tags" {
type = map
}

variable "name" { type = string }
variable "prefix" { type = string }

locals {
common_tags = merge({
CreatedBy = "Terraform"
}, var.tags)
name = var.prefix != "" ? "${var.prefix}-${var.name}" : var.name
}

resource "aws_iam_user" "lb" {
name = local.name
tags = merge(local.common_tags,
{
Name = local.name
})
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
locals {
common_tags = {
this = "that"
}
}

module "dummy" {
source = "./dummy"

tags = local.common_tags
name = "fred"
prefix = "joe"
}



16 changes: 13 additions & 3 deletions pkg/iac-providers/terraform/v14/testdata/tfjson/moduleconfigs.json
Original file line number Diff line number Diff line change
Expand Up @@ -403,12 +403,22 @@
"kms_data_key_reuse_period_seconds": 300,
"kms_master_key_id": "alias/aws/sqs",
"name": "terraform-example-queue",
"policy": "{\n \"Version\": \"2012-10-17\",\n \"Statement\": [{\n \"Sid\":\"Queue1_AnonymousAccess_AllActions_WhitelistIP\",\n \"Effect\": \"Allow\",\n \"Principal\": \"*\",\n \"Action\": \"sqs:*\",\n \"Resource\": \"arn:aws:sqs:*:111122223333:queue1\"\n }] \n}\n"
"policy": {
"Statement": [
{
"Action": "sqs:*",
"Effect": "Allow",
"Principal": "*",
"Resource": "arn:aws:sqs:*:111122223333:queue1",
"Sid": "Queue1_AnonymousAccess_AllActions_WhitelistIP"
}
],
"Version": "2012-10-17"
}
},
"skip_rules": null,
"max_severity": "",
"min_severity": ""

}
]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"aws_iam_user": [
{
"id": "aws_iam_user.lb",
"name": "lb",
"module_name": "dummy",
"source": "dummy/main.tf",
"plan_root": "./",
"line": 15,
"type": "aws_iam_user",
"config": {
"name": "joe != \"\" ? \"joe-fred\" : fred",
"tags": "${merge(${merge({\n CreatedBy = \"Terraform\"\n }, {\"this\":\"that\"})},\n {\n Name = joe != \"\" ? \"joe-fred\" : fred\n })}"
},
"skip_rules": null,
"max_severity": "",
"min_severity": ""
}
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"line": 13,
"type": "aws_iam_user",
"config": {
"name": "${lower(${local.foo} != null ? var.bar : var.foo)}"
"name": "${lower(bar != null ? bar : bar)}"
},
"skip_rules": null
}
Expand Down

0 comments on commit 896408e

Please sign in to comment.