Merge pull request #14669 from hashicorp/b-add-validation-iam-role-policy

provider/aws: validation: Add validation function for IAM Policies

builtin/providers/aws/resource_aws_iam_policy.go

@@ -24,24 +24,24 @@ func resourceAwsIamPolicy() *schema.Resource {
 		},
 
 		Schema: map[string]*schema.Schema{
-			"description": &schema.Schema{
+			"description": {
 				Type:     schema.TypeString,
 				ForceNew: true,
 				Optional: true,
 			},
-			"path": &schema.Schema{
+			"path": {
 				Type:     schema.TypeString,
 				Optional: true,
 				Default:  "/",
 				ForceNew: true,
 			},
-			"policy": &schema.Schema{
+			"policy": {
 				Type:             schema.TypeString,
 				Required:         true,
-				ValidateFunc:     validateJsonString,
+				ValidateFunc:     validateIAMPolicyJson,
 				DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs,
 			},
-			"name": &schema.Schema{
+			"name": {
 				Type:          schema.TypeString,
 				Optional:      true,
 				Computed:      true,
@@ -79,7 +79,7 @@ func resourceAwsIamPolicy() *schema.Resource {
 					return
 				},
 			},
-			"arn": &schema.Schema{
+			"arn": {
 				Type:     schema.TypeString,
 				Computed: true,
 			},

builtin/providers/aws/resource_aws_iam_policy_test.go

@@ -2,6 +2,7 @@ package aws
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -19,7 +20,7 @@ func TestAWSPolicy_namePrefix(t *testing.T) {
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckAWSPolicyDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccAWSPolicyPrefixNameConfig,
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSPolicyExists("aws_iam_policy.policy", &out),
@@ -31,6 +32,20 @@ func TestAWSPolicy_namePrefix(t *testing.T) {
 	})
 }
 
+func TestAWSPolicy_invalidJson(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccAWSPolicyInvalidJsonConfig,
+				ExpectError: regexp.MustCompile("invalid JSON"),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSPolicyExists(resource string, res *iam.GetPolicyOutput) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[resource]
@@ -94,3 +109,23 @@ resource "aws_iam_policy" "policy" {
 EOF
 }
 `
+const testAccAWSPolicyInvalidJsonConfig = `
+resource "aws_iam_policy" "policy" {
+	name_prefix = "test-policy-"
+	path = "/"
+  policy = <<EOF
+  {
+    "Version": "2012-10-17",
+    "Statement": [
+      {
+        "Action": [
+          "ec2:Describe*"
+        ],
+        "Effect": "Allow",
+        "Resource": "*"
+      }
+    ]
+  }
+  EOF
+}
+`

builtin/providers/aws/resource_aws_iam_role_policy.go

@@ -26,11 +26,13 @@ func resourceAwsIamRolePolicy() *schema.Resource {
 		},
 
 		Schema: map[string]*schema.Schema{
-			"policy": &schema.Schema{
+			"policy": {
-				Type:     schema.TypeString,
+				Type:             schema.TypeString,
-				Required: true,
+				Required:         true,
+				ValidateFunc:     validateIAMPolicyJson,
+				DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs,
 			},
-			"name": &schema.Schema{
+			"name": {
 				Type:          schema.TypeString,
 				Optional:      true,
 				Computed:      true,
@@ -38,13 +40,13 @@ func resourceAwsIamRolePolicy() *schema.Resource {
 				ConflictsWith: []string{"name_prefix"},
 				ValidateFunc:  validateIamRolePolicyName,
 			},
-			"name_prefix": &schema.Schema{
+			"name_prefix": {
 				Type:         schema.TypeString,
 				Optional:     true,
 				ForceNew:     true,
 				ValidateFunc: validateIamRolePolicyNamePrefix,
 			},
-			"role": &schema.Schema{
+			"role": {
 				Type:     schema.TypeString,
 				Required: true,
 				ForceNew: true,

builtin/providers/aws/resource_aws_iam_role_policy_test.go

@@ -2,6 +2,7 @@ package aws
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
@@ -22,7 +23,7 @@ func TestAccAWSIAMRolePolicy_basic(t *testing.T) {
 		Providers:    testAccProviders,
 		CheckDestroy: testAccCheckIAMRolePolicyDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccIAMRolePolicyConfig(role, policy1),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckIAMRolePolicy(
@@ -31,7 +32,7 @@ func TestAccAWSIAMRolePolicy_basic(t *testing.T) {
 					),
 				),
 			},
-			resource.TestStep{
+			{
 				Config: testAccIAMRolePolicyConfigUpdate(role, policy1, policy2),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckIAMRolePolicy(
@@ -53,7 +54,7 @@ func TestAccAWSIAMRolePolicy_namePrefix(t *testing.T) {
 		Providers:     testAccProviders,
 		CheckDestroy:  testAccCheckIAMRolePolicyDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccIAMRolePolicyConfig_namePrefix(role),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckIAMRolePolicy(
@@ -75,7 +76,7 @@ func TestAccAWSIAMRolePolicy_generatedName(t *testing.T) {
 		Providers:     testAccProviders,
 		CheckDestroy:  testAccCheckIAMRolePolicyDestroy,
 		Steps: []resource.TestStep{
-			resource.TestStep{
+			{
 				Config: testAccIAMRolePolicyConfig_generatedName(role),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckIAMRolePolicy(
@@ -88,6 +89,22 @@ func TestAccAWSIAMRolePolicy_generatedName(t *testing.T) {
 	})
 }
 
+func TestAccAWSIAMRolePolicy_invalidJSON(t *testing.T) {
+	role := acctest.RandString(10)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckIAMRolePolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccIAMRolePolicyConfig_invalidJSON(role),
+				ExpectError: regexp.MustCompile("invalid JSON"),
+			},
+		},
+	})
+}
+
 func testAccCheckIAMRolePolicyDestroy(s *terraform.State) error {
 	iamconn := testAccProvider.Meta().(*AWSClient).iamconn
 
@@ -328,3 +345,42 @@ EOF
 }
 `, role, policy1, policy2)
 }
+
+func testAccIAMRolePolicyConfig_invalidJSON(role string) string {
+	return fmt.Sprintf(`
+resource "aws_iam_role" "role" {
+	name = "tf_test_role_%s"
+	path = "/"
+	assume_role_policy = <<EOF
+{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Action": "sts:AssumeRole",
+      "Principal": {
+        "Service": "ec2.amazonaws.com"
+      },
+      "Effect": "Allow",
+      "Sid": ""
+    }
+  ]
+}
+EOF
+}
+
+resource "aws_iam_role_policy" "foo" {
+	name = "tf_test_policy_%s"
+	role = "${aws_iam_role.role.name}"
+	policy = <<EOF
+  {
+    "Version": "2012-10-17",
+    "Statement": {
+      "Effect": "Allow",
+      "Action": "*",
+      "Resource": "*"
+    }
+  }
+  EOF
+}
+`, role, role)
+}

builtin/providers/aws/structure.go

@@ -1886,7 +1886,10 @@ func normalizeJsonString(jsonString interface{}) (string, error) {
 		return s, err
 	}
 
+	// The error is intentionally ignored here to allow empty policies to passthrough validation.
+	// This covers any interpolated values
 	bytes, _ := json.Marshal(j)
+
 	return string(bytes[:]), nil
 }
 

builtin/providers/aws/validators.go

@@ -605,6 +605,23 @@ func validateJsonString(v interface{}, k string) (ws []string, errors []error) {
 	return
 }
 
+func validateIAMPolicyJson(v interface{}, k string) (ws []string, errors []error) {
+	// IAM Policy documents need to be valid JSON, and pass legacy parsing
+	value := v.(string)
+	if len(value) < 1 {
+		errors = append(errors, fmt.Errorf("%q contains an invalid JSON policy", k))
+		return
+	}
+	if value[:1] != "{" {
+		errors = append(errors, fmt.Errorf("%q conatains an invalid JSON policy", k))
+		return
+	}
+	if _, err := normalizeJsonString(v); err != nil {
+		errors = append(errors, fmt.Errorf("%q contains an invalid JSON: %s", k, err))
+	}
+	return
+}
+
 func validateCloudFormationTemplate(v interface{}, k string) (ws []string, errors []error) {
 	if looksLikeJsonString(v) {
 		if _, err := normalizeJsonString(v); err != nil {

builtin/providers/aws/validators_test.go

@@ -799,6 +799,65 @@ func TestValidateJsonString(t *testing.T) {
 	}
 }
 
+func TestValidateIAMPolicyJsonString(t *testing.T) {
+	type testCases struct {
+		Value    string
+		ErrCount int
+	}
+
+	invalidCases := []testCases{
+		{
+			Value:    `{0:"1"}`,
+			ErrCount: 1,
+		},
+		{
+			Value:    `{'abc':1}`,
+			ErrCount: 1,
+		},
+		{
+			Value:    `{"def":}`,
+			ErrCount: 1,
+		},
+		{
+			Value:    `{"xyz":[}}`,
+			ErrCount: 1,
+		},
+		{
+			Value:    ``,
+			ErrCount: 1,
+		},
+		{
+			Value:    `    {"xyz": "foo"}`,
+			ErrCount: 1,
+		},
+	}
+
+	for _, tc := range invalidCases {
+		_, errors := validateIAMPolicyJson(tc.Value, "json")
+		if len(errors) != tc.ErrCount {
+			t.Fatalf("Expected %q to trigger a validation error.", tc.Value)
+		}
+	}
+
+	validCases := []testCases{
+		{
+			Value:    `{}`,
+			ErrCount: 0,
+		},
+		{
+			Value:    `{"abc":["1","2"]}`,
+			ErrCount: 0,
+		},
+	}
+
+	for _, tc := range validCases {
+		_, errors := validateIAMPolicyJson(tc.Value, "json")
+		if len(errors) != tc.ErrCount {
+			t.Fatalf("Expected %q not to trigger a validation error.", tc.Value)
+		}
+	}
+}
+
 func TestValidateCloudFormationTemplate(t *testing.T) {
 	type testCases struct {
 		Value    string