-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
d/aws_iam_policy_document: handle duplicated conditions consistently #33093
Conversation
Community NoteVoting for Prioritization
For Submitters
|
028243e
to
a8c3b26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccIAMPolicyDocumentDataSource_' PKG=iam ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 3 -run=TestAccIAMPolicyDocumentDataSource_ -timeout 180m
=== 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_source
--- PASS: TestAccIAMPolicyDocumentDataSource_basic (20.90s)
=== CONT TestAccIAMPolicyDocumentDataSource_override
--- PASS: TestAccIAMPolicyDocumentDataSource_overrideList (20.96s)
=== CONT TestAccIAMPolicyDocumentDataSource_sourceListConflicting
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceListConflicting (4.06s)
=== CONT TestAccIAMPolicyDocumentDataSource_sourceConflicting
--- PASS: TestAccIAMPolicyDocumentDataSource_source (39.14s)
=== CONT TestAccIAMPolicyDocumentDataSource_sourceList
--- PASS: TestAccIAMPolicyDocumentDataSource_override (21.40s)
=== CONT TestAccIAMPolicyDocumentDataSource_multipleConditionKeys
=== CONT TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceConflicting (20.94s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceList (21.21s)
=== CONT TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON
--- PASS: TestAccIAMPolicyDocumentDataSource_multipleConditionKeys (21.08s)
=== CONT TestAccIAMPolicyDocumentDataSource_version20081017
--- PASS: TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue (20.97s)
=== CONT TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
acctest.go:900: skipping tests; current partition (aws) does not equal aws-us-gov
--- SKIP: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov (0.00s)
=== CONT TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals
--- PASS: TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON (22.19s)
=== CONT TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice
=== CONT TestAccIAMPolicyDocumentDataSource_singleConditionValue
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals (20.15s)
--- PASS: TestAccIAMPolicyDocumentDataSource_version20081017 (33.21s)
=== CONT TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice (20.01s)
=== CONT TestAccIAMPolicyDocumentDataSource_noStatementOverride
--- PASS: TestAccIAMPolicyDocumentDataSource_singleConditionValue (19.97s)
=== CONT TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys (20.51s)
=== CONT TestAccIAMPolicyDocumentDataSource_duplicateSid
=== CONT TestAccIAMPolicyDocumentDataSource_noStatementMerge
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementOverride (20.77s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON (22.04s)
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateSid (21.38s)
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementMerge (18.00s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/iam 146.815s
This functionality has been released in v5.14.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
This change fixes inconsistent behavior when
condition
blocks with duplicatetest
andvariable
arguments are provided. As outlined in #33068, previous behavior varied between overwriting values, merging values, and panicing. Now, thevalues
from all duplicated Conditions will be merged in all cases.When the
statement
block is used the data source has full control over the conversion from HCL to JSON, and merging Conditionvalues
on the users behalf is possible. With raw JSON input (eg. thesource_policy_documents
argument), the unmarshaling process will overwrite duplicated keys before it is processed by the data source. Investigation into whether we can warn or error in these cases is a separate effort (tracked broadly in #33026), and not in-scope for this particular change.Overwrite behavior change
In the case where "overwrite" was the previous behavior this will technically represent a breaking change. Given this was likely not the end users intent (the overwritten value could more easily be omitted entirely), we are deeming this acceptable to release in a minor version given the potential security risks.
Before:
After:
Relations
Closes #33068
Closes #33060
Closes #27020
Relates #33026
Relates hashicorp/terraform#28727
References
Output from Acceptance Testing