From e0967300033666cb77d5a2fdb3fa42f350a04573 Mon Sep 17 00:00:00 2001 From: Matt Sladen Date: Mon, 5 Sep 2022 09:21:03 +0100 Subject: [PATCH 1/3] Add validation to source_policy_documents strings --- .../iam/policy_document_data_source.go | 5 +++- .../iam/policy_document_data_source_test.go | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/internal/service/iam/policy_document_data_source.go b/internal/service/iam/policy_document_data_source.go index 198f4b4bff1..b54afefecbd 100644 --- a/internal/service/iam/policy_document_data_source.go +++ b/internal/service/iam/policy_document_data_source.go @@ -54,7 +54,10 @@ func DataSourcePolicyDocument() *schema.Resource { "source_policy_documents": { Type: schema.TypeList, Optional: true, - Elem: &schema.Schema{Type: schema.TypeString}, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validation.All(validation.StringIsNotEmpty, validation.StringIsJSON), + }, }, "statement": { Type: schema.TypeList, diff --git a/internal/service/iam/policy_document_data_source_test.go b/internal/service/iam/policy_document_data_source_test.go index 8d0c27b99f3..9d13585278c 100644 --- a/internal/service/iam/policy_document_data_source_test.go +++ b/internal/service/iam/policy_document_data_source_test.go @@ -223,6 +223,24 @@ func TestAccIAMPolicyDocumentDataSource_duplicateSid(t *testing.T) { }) } +func TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJson(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_invalidJson, + ExpectError: regexp.MustCompile(`"source_policy_documents.0" contains an invalid JSON: unexpected end of JSON input`), + }, + { + Config: testAccPolicyDocumentDataSourceConfig_emptyString, + ExpectError: regexp.MustCompile(`expected "source_policy_documents.0" to not be an empty string, got`), + }, + }, + }) +} + // Reference: https://github.com/hashicorp/terraform-provider-aws/issues/10777 func TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice(t *testing.T) { dataSourceName := "data.aws_iam_policy_document.test" @@ -1277,6 +1295,18 @@ data "aws_iam_policy_document" "test" { } ` +var testAccPolicyDocumentDataSourceConfig_emptyString = ` +data "aws_iam_policy_document" "test" { + source_policy_documents = [""] +} +` + +var testAccPolicyDocumentDataSourceConfig_invalidJson = ` +data "aws_iam_policy_document" "test" { + source_policy_documents = ["{"] +} +` + func testAccPolicyDocumentExpectedJSONStatementPrincipalIdentifiersMultiplePrincipals() string { return fmt.Sprintf(`{ "Version": "2012-10-17", From 3083a3f4eec4bf21e566ce32a0c6f02348b1d2dc Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 7 Sep 2022 15:41:19 -0400 Subject: [PATCH 2/3] Add CHANGELOG entry. --- .changelog/26640.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/26640.txt diff --git a/.changelog/26640.txt b/.changelog/26640.txt new file mode 100644 index 00000000000..4eace9bf06d --- /dev/null +++ b/.changelog/26640.txt @@ -0,0 +1,3 @@ +```release-note:bug + data-source/aws_iam_policy_document: Prevent crash when `source_policy_documents` contains empty or invalid JSON documents +``` \ No newline at end of file From df075eeb6ff61347dc489caeaeb880676d92b3c3 Mon Sep 17 00:00:00 2001 From: Kit Ewbank Date: Wed, 7 Sep 2022 15:55:24 -0400 Subject: [PATCH 3/3] d/aws_iam_policy_document: Don't crash if 'source_policy_documents' contains an empty string. --- .../iam/policy_document_data_source.go | 6 ++++- .../iam/policy_document_data_source_test.go | 22 +++++++++++++------ internal/service/iam/policy_model.go | 2 +- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/internal/service/iam/policy_document_data_source.go b/internal/service/iam/policy_document_data_source.go index b54afefecbd..0331409dcd4 100644 --- a/internal/service/iam/policy_document_data_source.go +++ b/internal/service/iam/policy_document_data_source.go @@ -56,7 +56,7 @@ func DataSourcePolicyDocument() *schema.Resource { Optional: true, Elem: &schema.Schema{ Type: schema.TypeString, - ValidateFunc: validation.All(validation.StringIsNotEmpty, validation.StringIsJSON), + ValidateFunc: validation.StringIsJSON, }, }, "statement": { @@ -139,6 +139,10 @@ func dataSourcePolicyDocumentRead(d *schema.ResourceData, meta interface{}) erro // merge sourceDocs in order specified for sourceJSONIndex, sourceJSON := range v.([]interface{}) { + if sourceJSON == nil { + continue + } + sourceDoc := &IAMPolicyDoc{} if err := json.Unmarshal([]byte(sourceJSON.(string)), sourceDoc); 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 index 9d13585278c..4fae2d3cd88 100644 --- a/internal/service/iam/policy_document_data_source_test.go +++ b/internal/service/iam/policy_document_data_source_test.go @@ -223,19 +223,23 @@ func TestAccIAMPolicyDocumentDataSource_duplicateSid(t *testing.T) { }) } -func TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJson(t *testing.T) { +func TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON(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_invalidJson, + Config: testAccPolicyDocumentDataSourceConfig_invalidJSON, ExpectError: regexp.MustCompile(`"source_policy_documents.0" contains an invalid JSON: unexpected end of JSON input`), }, { - Config: testAccPolicyDocumentDataSourceConfig_emptyString, - ExpectError: regexp.MustCompile(`expected "source_policy_documents.0" to not be an empty string, got`), + Config: testAccPolicyDocumentDataSourceConfig_emptyString, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.aws_iam_policy_document.test", "json", + testAccPolicyDocumentExpectedJSONNoStatement, + ), + ), }, }, }) @@ -1297,16 +1301,20 @@ data "aws_iam_policy_document" "test" { var testAccPolicyDocumentDataSourceConfig_emptyString = ` data "aws_iam_policy_document" "test" { - source_policy_documents = [""] + source_policy_documents = [""] } ` -var testAccPolicyDocumentDataSourceConfig_invalidJson = ` +var testAccPolicyDocumentDataSourceConfig_invalidJSON = ` data "aws_iam_policy_document" "test" { - source_policy_documents = ["{"] + source_policy_documents = ["{"] } ` +var testAccPolicyDocumentExpectedJSONNoStatement = `{ + "Version": "2012-10-17" +}` + func testAccPolicyDocumentExpectedJSONStatementPrincipalIdentifiersMultiplePrincipals() string { return fmt.Sprintf(`{ "Version": "2012-10-17", diff --git a/internal/service/iam/policy_model.go b/internal/service/iam/policy_model.go index ab404f9fa36..7739c25f6da 100644 --- a/internal/service/iam/policy_model.go +++ b/internal/service/iam/policy_model.go @@ -13,7 +13,7 @@ const ( type IAMPolicyDoc struct { Version string `json:",omitempty"` Id string `json:",omitempty"` - Statements []*IAMPolicyStatement `json:"Statement"` + Statements []*IAMPolicyStatement `json:"Statement,omitempty"` } type IAMPolicyStatement struct {