Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add validation for iam policy document sid #40562

Merged
merged 6 commits into from
Dec 13, 2024
Merged

add validation for iam policy document sid #40562

merged 6 commits into from
Dec 13, 2024

Conversation

YakDriver
Copy link
Member

@YakDriver YakDriver commented Dec 13, 2024 •
edited
Loading

Description

Plan-time validate for statement sid containing only alphanumeric characters.

Relations

Closes #12262
Closes #10771

References

Output from Acceptance Testing

% make t T=TestAccIAMPolicyDocumentDataSource_validateSid K=iam
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMPolicyDocumentDataSource_validateSid'  -timeout 360m
2024/12/13 13:09:52 Initializing Terraform AWS Provider...
=== RUN   TestAccIAMPolicyDocumentDataSource_validateSid
=== PAUSE TestAccIAMPolicyDocumentDataSource_validateSid
=== CONT  TestAccIAMPolicyDocumentDataSource_validateSid
--- PASS: TestAccIAMPolicyDocumentDataSource_validateSid (2.58s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/iam	6.626s
% make t T=TestAccIAMPolicyDocumentDataSource_ K=iam
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMPolicyDocumentDataSource_'  -timeout 360m
2024/12/13 13:12:20 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_validateSid
=== PAUSE TestAccIAMPolicyDocumentDataSource_validateSid
=== 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_validateSid
=== CONT  TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON
=== CONT  TestAccIAMPolicyDocumentDataSource_duplicateSid
=== CONT  TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
=== CONT  TestAccIAMPolicyDocumentDataSource_version20081017
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceList
=== CONT  TestAccIAMPolicyDocumentDataSource_overrideList
=== CONT  TestAccIAMPolicyDocumentDataSource_override
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceListConflicting
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceConflicting
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice
=== CONT  TestAccIAMPolicyDocumentDataSource_noStatementOverride
=== CONT  TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys
=== CONT  TestAccIAMPolicyDocumentDataSource_source
=== CONT  TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals
=== CONT  TestAccIAMPolicyDocumentDataSource_noStatementMerge
=== CONT  TestAccIAMPolicyDocumentDataSource_multipleConditionKeys
=== NAME  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
    policy_document_data_source_test.go:409: skipping tests; current partition (aws) does not equal aws-us-gov
--- SKIP: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov (0.41s)
=== CONT  TestAccIAMPolicyDocumentDataSource_singleConditionValue
--- PASS: TestAccIAMPolicyDocumentDataSource_validateSid (2.23s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceListConflicting (2.67s)
--- PASS: TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue (15.91s)
--- PASS: TestAccIAMPolicyDocumentDataSource_multipleConditionKeys (16.01s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON (16.03s)
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys (16.11s)
--- PASS: TestAccIAMPolicyDocumentDataSource_basic (16.15s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceConflicting (16.16s)
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals (16.16s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceList (16.16s)
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateSid (16.18s)
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice (16.18s)
--- PASS: TestAccIAMPolicyDocumentDataSource_singleConditionValue (15.78s)
--- PASS: TestAccIAMPolicyDocumentDataSource_overrideList (16.20s)
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementOverride (16.21s)
--- PASS: TestAccIAMPolicyDocumentDataSource_override (16.22s)
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementMerge (16.27s)
--- PASS: TestAccIAMPolicyDocumentDataSource_version20081017 (19.88s)
--- PASS: TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON (20.87s)
--- PASS: TestAccIAMPolicyDocumentDataSource_source (21.38s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/iam	25.523s

@YakDriver YakDriver requested a review from a team as a code owner December 13, 2024 18:02
@github-actions GitHub Actions
Copy link

Community Note

Voting for Prioritization

  • Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
  • Please see our prioritization guide for information on how we prioritize.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

  • Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
  • For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
  • Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/iam Issues and PRs that pertain to the iam service. prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. labels Dec 13, 2024
nam054
nam054 approved these changes Dec 13, 2024
View reviewed changes
Copy link
Contributor

@nam054 nam054 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

> make t T=TestAccIAMPolicyDocumentDataSource_validateSid K=iam
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMPolicyDocumentDataSource_validateSid'  -timeout 360m
go: downloading github.com/aws/aws-sdk-go-v2/service/timestreamquery v1.29.1
2024/12/13 10:43:49 Initializing Terraform AWS Provider...
=== RUN   TestAccIAMPolicyDocumentDataSource_validateSid
=== PAUSE TestAccIAMPolicyDocumentDataSource_validateSid
=== CONT  TestAccIAMPolicyDocumentDataSource_validateSid
--- PASS: TestAccIAMPolicyDocumentDataSource_validateSid (2.11s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/iam	7.292s
> % make t T=TestAccIAMPolicyDocumentDataSource_ K=iam
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMPolicyDocumentDataSource_'  -timeout 360m
2024/12/13 10:45: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_validateSid
=== PAUSE TestAccIAMPolicyDocumentDataSource_validateSid
=== 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_validateSid
=== CONT  TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceList
=== CONT  TestAccIAMPolicyDocumentDataSource_singleConditionValue
=== CONT  TestAccIAMPolicyDocumentDataSource_multipleConditionKeys
=== CONT  TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
=== CONT  TestAccIAMPolicyDocumentDataSource_version20081017
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals
=== CONT  TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys
=== CONT  TestAccIAMPolicyDocumentDataSource_source
=== CONT  TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue
=== CONT  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice
=== CONT  TestAccIAMPolicyDocumentDataSource_override
=== CONT  TestAccIAMPolicyDocumentDataSource_overrideList
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceListConflicting
=== CONT  TestAccIAMPolicyDocumentDataSource_noStatementOverride
=== CONT  TestAccIAMPolicyDocumentDataSource_sourceConflicting
=== CONT  TestAccIAMPolicyDocumentDataSource_duplicateSid
=== NAME  TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov
    policy_document_data_source_test.go:409: skipping tests; current partition (aws) does not equal aws-us-gov
--- SKIP: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipalsGov (0.22s)
=== CONT  TestAccIAMPolicyDocumentDataSource_noStatementMerge
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceListConflicting (4.23s)
--- PASS: TestAccIAMPolicyDocumentDataSource_validateSid (4.28s)
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateConditionKeys (26.10s)
--- PASS: TestAccIAMPolicyDocumentDataSource_conditionWithBoolValue (26.36s)
--- PASS: TestAccIAMPolicyDocumentDataSource_multipleConditionKeys (26.37s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceList (26.38s)
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_multiplePrincipals (26.49s)
--- PASS: TestAccIAMPolicyDocumentDataSource_singleConditionValue (26.85s)
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementMerge (26.65s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourceConflicting (26.92s)
--- PASS: TestAccIAMPolicyDocumentDataSource_overrideList (26.95s)
--- PASS: TestAccIAMPolicyDocumentDataSource_StatementPrincipalIdentifiers_stringAndSlice (26.97s)
--- PASS: TestAccIAMPolicyDocumentDataSource_noStatementOverride (26.97s)
--- PASS: TestAccIAMPolicyDocumentDataSource_basic (27.01s)
--- PASS: TestAccIAMPolicyDocumentDataSource_override (27.37s)
--- PASS: TestAccIAMPolicyDocumentDataSource_duplicateSid (27.72s)
--- PASS: TestAccIAMPolicyDocumentDataSource_sourcePolicyValidJSON (27.72s)
--- PASS: TestAccIAMPolicyDocumentDataSource_version20081017 (30.78s)
--- PASS: TestAccIAMPolicyDocumentDataSource_source (32.53s)
--- PASS: TestAccIAMPolicyDocumentDataSource_overridePolicyDocumentValidJSON (32.57s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/iam	37.668s

@YakDriver YakDriver merged commit 83bf679 into main Dec 13, 2024
35 checks passed
@YakDriver YakDriver deleted the add-validation-for-iam-policy-document-sid branch December 13, 2024 18:48
@github-actions github-actions bot added this to the v5.82.0 milestone Dec 13, 2024
terraform-aws-provider bot pushed a commit that referenced this pull request Dec 13, 2024
Update CHANGELOG.md for #40562
22dc8bc
@github-actions github-actions bot removed the prioritized Part of the maintainer teams immediate focus. To be addressed within the current quarter. label Dec 19, 2024
@github-actions GitHub Actions
Copy link

This functionality has been released in v5.82.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!

@DerekTBrown
Copy link
Contributor

@YakDriver @nam054 this is potentially a breaking change for users that have invalid characters in their SIDs. What is the recommended path for this sort of update? It is a poor experience for people to have their previously-working Terraform all the sudden break due to a minor update.

@DerekTBrown
Copy link
Contributor

Looks like this is being discussed here: #40639

@ewbankkit ewbankkit mentioned this pull request Dec 19, 2024
Closed
@DerekTBrown DerekTBrown mentioned this pull request Dec 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nam054 nam054 nam054 approved these changes

Assignees
No one assigned
Labels
service/iam Issues and PRs that pertain to the iam service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Milestone
v5.82.0
Development

Successfully merging this pull request may close these issues.

aws_iam_policy_document doesn't detect invalid SIDs
4 participants
@YakDriver @DerekTBrown @nam054 @kangaechu