Add PostProcess to FileLinter

For Kubernetes the variables should be resolved before JSON policy documents are parsed.
stelligent · May 18, 2018 · b6757ad · b6757ad
1 parent 29ea1aa
commit b6757ad
Show file tree

Hide file tree

Showing 8 changed files with 90 additions and 25 deletions.
diff --git a/TODO.md b/TODO.md
@@ -1,6 +1,5 @@
 # TODO
 
-* Terraform variable expressions can contain double quotes, which confuse the parser
 * Region is hard-coded to us-east-1 for GetValueFromS3
 * Use type switch as more idiomatic way to handle multiple types in match.go
 * Use log package for error reporting

diff --git a/linter/file_linter.go b/linter/file_linter.go
@@ -19,6 +19,7 @@ type (
 	FileResourceLoader interface {
 		Load(filename string) (FileResources, error)
 		ReplaceVariables(resources []assertion.Resource, variables []Variable) ([]assertion.Resource, error)
+		PostProcess(resources []assertion.Resource) ([]assertion.Resource, error)
 	}
 )
 
@@ -57,7 +58,11 @@ func (fl FileLinter) Validate(ruleSet assertion.RuleSet, options Options) (asser
 	if err != nil {
 		return assertion.ValidationReport{}, err
 	}
-	report, err := rl.ValidateResources(resolvedResources, rules)
+	resourcesToValidate, err := fl.Loader.PostProcess(resolvedResources)
+	if err != nil {
+		return assertion.ValidationReport{}, err
+	}
+	report, err := rl.ValidateResources(resourcesToValidate, rules)
 	if err != nil {
 		return report, err
 	}

diff --git a/linter/kubernetes.go b/linter/kubernetes.go
@@ -53,3 +53,7 @@ func (l KubernetesResourceLoader) Load(filename string) (FileResources, error) {
 func (l KubernetesResourceLoader) ReplaceVariables(resources []assertion.Resource, variables []Variable) ([]assertion.Resource, error) {
 	return resources, nil
 }
+
+func (l KubernetesResourceLoader) PostProcess(resources []assertion.Resource) ([]assertion.Resource, error) {
+	return resources, nil
+}
diff --git a/linter/rules_resource_loader.go b/linter/rules_resource_loader.go
@@ -55,3 +55,7 @@ func (l RulesResourceLoader) Load(filename string) (FileResources, error) {
 func (l RulesResourceLoader) ReplaceVariables(resources []assertion.Resource, variables []Variable) ([]assertion.Resource, error) {
 	return resources, nil
 }
+
+func (l RulesResourceLoader) PostProcess(resources []assertion.Resource) ([]assertion.Resource, error) {
+	return resources, nil
+}
diff --git a/linter/terraform.go b/linter/terraform.go
@@ -23,24 +23,6 @@ type (
 	}
 )
 
-func parsePolicy(templateResource interface{}) (map[string]interface{}, error) {
-	firstResource := templateResource.([]interface{})[0] // FIXME does this array always have 1 element?
-	properties := firstResource.(map[string]interface{})
-	for _, attribute := range []string{"assume_role_policy", "policy"} {
-		if policyAttribute, hasPolicyString := properties[attribute]; hasPolicyString {
-			if policyString, isString := policyAttribute.(string); isString {
-				var policy interface{}
-				err := json.Unmarshal([]byte(policyString), &policy)
-				if err != nil {
-					return properties, err
-				}
-				properties[attribute] = policy
-			}
-		}
-	}
-	return properties, nil
-}
-
 func loadHCL(filename string) (TerraformLoadResult, error) {
 	result := TerraformLoadResult{
 		Resources: []interface{}{},
@@ -128,10 +110,7 @@ func (l TerraformResourceLoader) Load(filename string) (FileResources, error) {
 			if templateResources != nil {
 				for _, templateResource := range templateResources.([]interface{}) {
 					for resourceID, templateResource := range templateResource.(map[string]interface{}) {
-						properties, err := parsePolicy(templateResource)
-						if err != nil {
-							return loaded, err
-						}
+						properties := getProperties(templateResource)
 						lineNumber := getResourceLineNumber(resourceType, resourceID, filename, result.AST)
 						tr := assertion.Resource{
 							ID:         resourceID,
@@ -156,6 +135,17 @@ func (l TerraformResourceLoader) ReplaceVariables(resources []assertion.Resource
 	return resources, nil
 }
 
+func (l TerraformResourceLoader) PostProcess(resources []assertion.Resource) ([]assertion.Resource, error) {
+	for _, r := range resources {
+		properties, err := parsePolicy(r.Properties)
+		if err != nil {
+			return resources, err
+		}
+		r.Properties = properties
+	}
+	return resources, nil
+}
+
 func replaceVariables(templateResource interface{}, variables []Variable) interface{} {
 	switch v := templateResource.(type) {
 	case map[string]interface{}:
@@ -205,5 +195,28 @@ func resolveValue(s string, variables []Variable) string {
 			}
 		}
 	}
-	return s
+	defaultReplacementValue := ""
+	return re.ReplaceAllString(s, defaultReplacementValue)
+}
+
+func parsePolicy(resource interface{}) (interface{}, error) {
+	properties := resource.(map[string]interface{})
+	for _, attribute := range []string{"assume_role_policy", "policy"} {
+		if policyAttribute, hasPolicyString := properties[attribute]; hasPolicyString {
+			if policyString, isString := policyAttribute.(string); isString {
+				var policy interface{}
+				err := json.Unmarshal([]byte(policyString), &policy)
+				if err != nil {
+					return properties, err
+				}
+				properties[attribute] = policy
+			}
+		}
+	}
+	return properties, nil
+}
+
+func getProperties(templateResource interface{}) map[string]interface{} {
+	first := templateResource.([]interface{})[0] // FIXME does this array always have 1 element?
+	return first.(map[string]interface{})
 }
diff --git a/linter/terraform_test.go b/linter/terraform_test.go
@@ -126,5 +126,24 @@ func TestTerraformPoliciesWithVariables(t *testing.T) {
 	}
 	if len(report.Violations) != 0 {
 		t.Errorf("TestTerraformPoliciesWithVariables returned %d violations, expecting 0", len(report.Violations))
+		t.Errorf("Violations: %v", report.Violations)
+	}
+}
+
+func TestTerraformHereDocWithExpression(t *testing.T) {
+	options := Options{
+		Tags:    []string{},
+		RuleIDs: []string{},
+	}
+	filenames := []string{"./testdata/resources/policy_with_expression.tf"}
+	linter := FileLinter{Filenames: filenames, ValueSource: TestingValueSource{}, Loader: TerraformResourceLoader{}}
+	ruleSet := loadRulesForTest("./testdata/rules/policy_variable.yml", t)
+	report, err := linter.Validate(ruleSet, options)
+	if err != nil {
+		t.Error("Expecting TestTerraformHereDocWithExpression to not return an error:" + err.Error())
+	}
+	if len(report.Violations) != 0 {
+		t.Errorf("TestTerraformPoliciesWithVariables returned %d violations, expecting 0", len(report.Violations))
+		t.Errorf("Violations: %v", report.Violations)
 	}
 }
diff --git a/linter/testdata/resources/policy_with_expression.tf b/linter/testdata/resources/policy_with_expression.tf
@@ -0,0 +1,17 @@
+resource "aws_iam_role" "role_with_variable" {
+    name = "non_compliant"
+    assume_role_policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [
+     {
+        "Action": "*",
+        "Principal": { "Service": "ec2.amazonaws.com" },
+        "Effect": "Allow", 
+        "Resources": "${var.resources == "foo" ? "foo": "*"}"
+     }
+  ]
+}
+EOF
+}
+
diff --git a/linter/yaml_resource_loader.go b/linter/yaml_resource_loader.go
@@ -54,3 +54,7 @@ func (l YAMLResourceLoader) Load(filename string) (FileResources, error) {
 func (l YAMLResourceLoader) ReplaceVariables(resources []assertion.Resource, variables []Variable) ([]assertion.Resource, error) {
 	return resources, nil
 }
+
+func (l YAMLResourceLoader) PostProcess(resources []assertion.Resource) ([]assertion.Resource, error) {
+	return resources, nil
+}