Merge pull request #27055 from mtt88/b-aws_iam_policy_document-more-j…

…son-validation data/aws_iam_policy_document - additional json validation
hashicorp · Oct 3, 2022 · 1a133f0 · 1a133f0
2 parents c9da374 + 4d20e1d
commit 1a133f0
Show file tree

Hide file tree

Showing 3 changed files with 120 additions and 7 deletions.
diff --git a/.changelog/27055.txt b/.changelog/27055.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+data-source/aws_iam_policy_document: Better handling when invalid JSON passed to `override_policy_documents`
+```
diff --git a/internal/service/iam/policy_document_data_source.go b/internal/service/iam/policy_document_data_source.go
@@ -33,23 +33,28 @@ func DataSourcePolicyDocument() *schema.Resource {
 				Computed: true,
 			},
 			"override_json": {
-				Type:       schema.TypeString,
-				Optional:   true,
-				Deprecated: "Use the attribute \"override_policy_documents\" instead.",
+				Type:         schema.TypeString,
+				Optional:     true,
+				ValidateFunc: validation.StringIsJSON,
+				Deprecated:   "Use the attribute \"override_policy_documents\" instead.",
 			},
 			"override_policy_documents": {
 				Type:     schema.TypeList,
 				Optional: true,
-				Elem:     &schema.Schema{Type: schema.TypeString},
+				Elem: &schema.Schema{
+					Type:         schema.TypeString,
+					ValidateFunc: validation.StringIsJSON,
+				},
 			},
 			"policy_id": {
 				Type:     schema.TypeString,
 				Optional: true,
 			},
 			"source_json": {
-				Type:       schema.TypeString,
-				Optional:   true,
-				Deprecated: "Use the attribute \"source_policy_documents\" instead.",
+				Type:         schema.TypeString,
+				Optional:     true,
+				ValidateFunc: validation.StringIsJSON,
+				Deprecated:   "Use the attribute \"source_policy_documents\" instead.",
 			},
 			"source_policy_documents": {
 				Type:     schema.TypeList,
@@ -256,6 +261,9 @@ func dataSourcePolicyDocumentRead(d *schema.ResourceData, meta interface{}) erro
 	// merge override_policy_documents policies into mergedDoc in order specified
 	if v, ok := d.GetOk("override_policy_documents"); ok && len(v.([]interface{})) > 0 {
 		for _, overrideJSON := range v.([]interface{}) {
+			if overrideJSON == nil {
+				continue
+			}
 			overrideDoc := &IAMPolicyDoc{}
 			if err := json.Unmarshal([]byte(overrideJSON.(string)), overrideDoc); err != nil {
 				return err

diff --git a/internal/service/iam/policy_document_data_source_test.go b/internal/service/iam/policy_document_data_source_test.go
@@ -263,6 +263,72 @@ func TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON(t *testing.T) {
 	})
 }
 
+func TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON(t *testing.T) {
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(t) },
+		ErrorCheck:               acctest.ErrorCheck(t, iam.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccPolicyDocumentDataSourceConfig_overridePolicyDocument_invalidJSON,
+				ExpectError: regexp.MustCompile(`"override_policy_documents.0" contains an invalid JSON: unexpected end of JSON input`),
+			},
+			{
+				Config: testAccPolicyDocumentDataSourceConfig_overridePolicyDocument_emptyString,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("data.aws_iam_policy_document.test", "json",
+						testAccPolicyDocumentExpectedJSONNoStatement,
+					),
+				),
+			},
+		},
+	})
+}
+
+func TestAccIAMPolicyDocumentDataSource_overrideJSONValidJSON(t *testing.T) {
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(t) },
+		ErrorCheck:               acctest.ErrorCheck(t, iam.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccPolicyDocumentDataSourceConfig_overrideJSON_invalidJSON,
+				ExpectError: regexp.MustCompile(`"override_json" contains an invalid JSON: unexpected end of JSON input`),
+			},
+			{
+				Config: testAccPolicyDocumentDataSourceConfig_overrideJSON_emptyString,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("data.aws_iam_policy_document.test", "json",
+						testAccPolicyDocumentExpectedJSONNoStatement,
+					),
+				),
+			},
+		},
+	})
+}
+
+func TestAccIAMPolicyDocumentDataSource_sourceJSONValidJSON(t *testing.T) {
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(t) },
+		ErrorCheck:               acctest.ErrorCheck(t, iam.EndpointsID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccPolicyDocumentDataSourceConfig_sourceJSON_invalidJSON,
+				ExpectError: regexp.MustCompile(`"source_json" contains an invalid JSON: unexpected end of JSON input`),
+			},
+			{
+				Config: testAccPolicyDocumentDataSourceConfig_sourceJSON_emptyString,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("data.aws_iam_policy_document.test", "json",
+						testAccPolicyDocumentExpectedJSONNoStatement,
+					),
+				),
+			},
+		},
+	})
+}
+
 // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/10777
 func TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice(t *testing.T) {
 	dataSourceName := "data.aws_iam_policy_document.test"
@@ -1441,3 +1507,39 @@ func testAccPolicyDocumentExpectedJSONStatementPrincipalIdentifiersMultiplePrinc
   ]
 }`, acctest.Partition())
 }
+
+var testAccPolicyDocumentDataSourceConfig_overridePolicyDocument_emptyString = `
+data "aws_iam_policy_document" "test" {
+  override_policy_documents = [""]
+}
+`
+
+var testAccPolicyDocumentDataSourceConfig_overridePolicyDocument_invalidJSON = `
+data "aws_iam_policy_document" "test" {
+  override_policy_documents = ["{"]
+}
+`
+
+var testAccPolicyDocumentDataSourceConfig_overrideJSON_emptyString = `
+data "aws_iam_policy_document" "test" {
+  override_json = ""
+}
+`
+
+var testAccPolicyDocumentDataSourceConfig_overrideJSON_invalidJSON = `
+data "aws_iam_policy_document" "test" {
+  override_json = "{"
+}
+`
+
+var testAccPolicyDocumentDataSourceConfig_sourceJSON_emptyString = `
+data "aws_iam_policy_document" "test" {
+  source_json = ""
+}
+`
+
+var testAccPolicyDocumentDataSourceConfig_sourceJSON_invalidJSON = `
+data "aws_iam_policy_document" "test" {
+  source_json = "{"
+}
+`