terraform-provider-aws fix(iam):Sort policy_document json lists alphabetically

fix(iam):Sort policy_document json lists alphabetically

Open cailen opened this issue 4 months ago • 1 comments

Description

The aws_iam_policy_document data source's output json was being generated in reverse alphabetical order (Z-A). When planning out, sometimes the AWS API will order lists alphabetically. Another issue is when combining multiple principal blocks into one, the sort is being done before the combination.

It has been noted that the data source is actually still preserving the order, but when using the policy document to pass JSON to a policy via the json output, it specifically reversed. the sort order

This PR fixes both of these.

Replacing sort.Sort(sort.Reverse(sort.StringSlice(ret))) with sort.Strings(ret) in the policyDecodeConfigStringList function.
Removing sort.Sort(sort.Reverse(sort.StringSlice(i))) within the principal blocks loop and replacing it with a sort loop once the principals are done being gathered:
```
// Do a sort on the combined principals set
 for k, v := range raw {
 	if av, ok := v.([]string); ok {
 		sort.Strings(av)
 		raw[k] = av
 	}
 }
```

Relations

Relates #11801 Closes #6107 Closes #26088

References

On 5.72.1

% terraform plan
data.aws_iam_policy_document.test: Reading...
data.aws_iam_policy_document.test: Read complete after 0s [id=272213006]

Changes to Outputs:
  + test_json = jsonencode(
        {
          + Statement = [
              + {
                  + Action   = [
                      + "sqs:*",
                      + "s3:*",
                      + "logs:*",
                      + "iam:*",
                      + "cloudwatch:*",
                      + "application-autoscaling:*",
                    ]
                  + Effect   = "Allow"
                  + Resource = [
                      + "arn:aws:sqs:*:*:*",
                      + "arn:aws:s3:::*",
                      + "arn:aws:logs:*:*:log-group::log-stream:*",
                      + "arn:aws:logs:*:*:log-group:/aws/lambda/*",
                      + "arn:aws:lambda:*:*:function:*",
                      + "arn:aws:lambda:*:*:event-source-mapping:*",
                      + "arn:aws:iam::*:role/aws-service-role/*",
                      + "arn:aws:iam::*:role/*",
                      + "arn:aws:iam::*:policy/*",
                      + "arn:aws:cloudwatch:*:*:alarm:*",
                      + "arn:aws:cloudwatch:*:*:*",
                      + "arn:aws:application-autoscaling:*:*:*",
                    ]
                  + Sid      = "Test_One"
                },
              + {
                  + Action   = [
                      + "lambda:*",
                      + "kms:*",
                    ]
                  + Effect   = "Allow"
                  + Resource = "*"
                  + Sid      = "Test_Two"
                },
            ]
          + Version   = "2012-10-17"
        }
    )

You can apply this plan to save these new output values to the Terraform state, without changing any real infrastructure.

Using the PR build:

% terraform plan
╷
│ Warning: Provider development overrides are in effect
│ 
│ The following provider development overrides are set in the CLI configuration:
│  - hashicorp/aws in /Users/example/go/bin
│ 
│ The behavior may therefore not match any released version of the provider and applying changes may cause the state to become incompatible with
│ published releases.
╵
data.aws_iam_policy_document.test: Reading...
data.aws_iam_policy_document.test: Read complete after 0s [id=3814263106]

Changes to Outputs:
  + test_json = jsonencode(
        {
          + Statement = [
              + {
                  + Action   = [
                      + "application-autoscaling:*",
                      + "cloudwatch:*",
                      + "iam:*",
                      + "logs:*",
                      + "s3:*",
                      + "sqs:*",
                    ]
                  + Effect   = "Allow"
                  + Resource = [
                      + "arn:aws:application-autoscaling:*:*:*",
                      + "arn:aws:cloudwatch:*:*:*",
                      + "arn:aws:cloudwatch:*:*:alarm:*",
                      + "arn:aws:iam::*:policy/*",
                      + "arn:aws:iam::*:role/*",
                      + "arn:aws:iam::*:role/aws-service-role/*",
                      + "arn:aws:lambda:*:*:event-source-mapping:*",
                      + "arn:aws:lambda:*:*:function:*",
                      + "arn:aws:logs:*:*:log-group:/aws/lambda/*",
                      + "arn:aws:logs:*:*:log-group::log-stream:*",
                      + "arn:aws:s3:::*",
                      + "arn:aws:sqs:*:*:*",
                    ]
                  + Sid      = "Test_One"
                },
              + {
                  + Action   = [
                      + "kms:*",
                      + "lambda:*",
                    ]
                  + Effect   = "Allow"
                  + Resource = "*"
                  + Sid      = "Test_Two"
                },
            ]
          + Version   = "2012-10-17"
        }
    )

You can apply this plan to save these new output values to the Terraform state, without changing any real infrastructure.

Output from Acceptance Testing

% make testacc TESTS=TestAccIAMPolicyDocumentDataSource PKG=iam
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.2 test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMPolicyDocumentDataSource'  -timeout 360m
2024/10/18 13:20:01 Initializing Terraform AWS Provider...
=== RUN   TestAccIAMPolicyDocumentDataSource_basic
=== PAUSE TestAccIAMPolicyDocumentDataSource_basic
=== RUN   TestAccIAMPolicyDocumentDataSource_singleConditionValue
=== PAUSE TestAccIAMPolicyDocumentDataSource_singleConditionValue
=== RUN   TestAccIAMPolicyDocumentDataSource_multipleConditionKeys
=== PAUSE TestAccIAMPolicyDocumentDataSource_multipleConditionKeys
=== RUN   TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys
=== PAUSE TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys
=== RUN   TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue
=== PAUSE TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue
=== RUN   TestAccIAMPolicyDocumentDataSource_source
=== PAUSE TestAccIAMPolicyDocumentDataSource_source
=== RUN   TestAccIAMPolicyDocumentDataSource_sourceList
=== PAUSE TestAccIAMPolicyDocumentDataSource_sourceList
=== RUN   TestAccIAMPolicyDocumentDataSource_sourceConflicting
=== PAUSE TestAccIAMPolicyDocumentDataSource_sourceConflicting
=== RUN   TestAccIAMPolicyDocumentDataSource_sourceListConflicting
=== PAUSE TestAccIAMPolicyDocumentDataSource_sourceListConflicting
=== RUN   TestAccIAMPolicyDocumentDataSource_override
=== PAUSE TestAccIAMPolicyDocumentDataSource_override
=== RUN   TestAccIAMPolicyDocumentDataSource_overrideList
=== PAUSE TestAccIAMPolicyDocumentDataSource_overrideList
=== RUN   TestAccIAMPolicyDocumentDataSource_noStatementMerge
=== PAUSE TestAccIAMPolicyDocumentDataSource_noStatementMerge
=== RUN   TestAccIAMPolicyDocumentDataSource_noStatementOverride
=== PAUSE TestAccIAMPolicyDocumentDataSource_noStatementOverride
=== RUN   TestAccIAMPolicyDocumentDataSource_duplicateSid
=== PAUSE TestAccIAMPolicyDocumentDataSource_duplicateSid
=== RUN   TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON
=== PAUSE TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON
=== RUN   TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON
=== PAUSE TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON
=== RUN   TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice
=== PAUSE TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice
=== RUN   TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals
=== PAUSE TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals
=== RUN   TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
=== PAUSE TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
=== RUN   TestAccIAMPolicyDocumentDataSource_version20081017
=== PAUSE TestAccIAMPolicyDocumentDataSource_version20081017
=== CONT  TestAccIAMPolicyDocumentDataSource_basic
=== CONT  TestAccIAMPolicyDocumentDataSource_overrideList
=== CONT  TestAccIAMPolicyDocumentDataSource_noStatementOverride
=== CONT  TestAccIAMPolicyDocumentDataSource_source
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceList
=== CONT  TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys
=== CONT  TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON
=== CONT  TestAccIAMPolicyDocumentDataSource_version20081017
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice
=== CONT  TestAccIAMPolicyDocumentDataSource_duplicateSid
=== CONT  TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON
=== CONT  TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceListConflicting
=== CONT  TestAccIAMPolicyDocumentDataSource_override
=== CONT  TestAccIAMPolicyDocumentDataSource_noStatementMerge
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceConflicting
=== CONT  TestAccIAMPolicyDocumentDataSource_multipleConditionKeys
=== CONT  TestAccIAMPolicyDocumentDataSource_singleConditionValue
=== NAME  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
    policy_document_data_source_test.go:393: skipping tests; current partition (aws) does not equal aws-us-gov
--- SKIP: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov (0.56s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceListConflicting (7.30s)
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementMerge (35.36s)
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice (35.40s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceConflicting (35.46s)
--- PASS: TestAccIAMPolicyDocumentDataSource_overrideList (35.53s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceList (35.53s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON (35.63s)
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys (35.64s)
--- PASS: TestAccIAMPolicyDocumentDataSource_override (35.65s)
--- PASS: TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue (35.74s)
--- PASS: TestAccIAMPolicyDocumentDataSource_basic (35.77s)
--- PASS: TestAccIAMPolicyDocumentDataSource_singleConditionValue (35.77s)
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals (35.78s)
--- PASS: TestAccIAMPolicyDocumentDataSource_multipleConditionKeys (35.78s)
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementOverride (35.79s)
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateSid (37.94s)
--- PASS: TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON (43.66s)
--- PASS: TestAccIAMPolicyDocumentDataSource_source (44.44s)
--- PASS: TestAccIAMPolicyDocumentDataSource_version20081017 (46.05s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/iam        52.202s

...

Oct 18 '24 17:10 cailen

terraform-provider-aws terraform-provider-aws copied to clipboard

fix(iam):Sort policy_document json lists alphabetically

Description

Relations

References

Output from Acceptance Testing

terraform-provider-aws
terraform-provider-aws copied to clipboard