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.

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

Fix aws_iam_policy_document order #23060

Merged
merged 9 commits into from
Jul 8, 2022

Conversation

Synohara
Copy link

@Synohara Synohara commented Feb 9, 2022

Community Note

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

Closes #22274
Relates #22944 (maybe closes)

Output from acceptance testing(us-west-2):

$ make testacc TESTS=TestAccIAMRole_basic PKG=iam
=== RUN   TestAccIAMRole_basic
=== PAUSE TestAccIAMRole_basic
=== RUN   TestAccIAMRole_basicWithDescription
=== PAUSE TestAccIAMRole_basicWithDescription
=== CONT  TestAccIAMRole_basicWithDescription
=== CONT  TestAccIAMRole_basic
--- PASS: TestAccIAMRole_basic (38.51s)
--- PASS: TestAccIAMRole_basicWithDescription (89.66s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/iam        104.877s

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. size/XS Managed by automation to categorize the size of a PR. service/iam Issues and PRs that pertain to the iam service. labels Feb 9, 2022
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Welcome @Synohara 👋

It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.

Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.

Thanks again, and welcome to the community! 😃

@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels Feb 15, 2022
@ewbankkit ewbankkit force-pushed the iam-assume-role-fix branch from f969456 to 4776a59 Compare March 25, 2022 16:04
@github-actions github-actions bot added size/M Managed by automation to categorize the size of a PR. size/L Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. size/M Managed by automation to categorize the size of a PR. labels Mar 25, 2022
@imaginarynik
Copy link

@justinretzolk Could you please provide an update as to when will this be merged?!

@imaginarynik
Copy link

imaginarynik commented Apr 6, 2022

@Synohara Do you know if this PR is going to be merged and when? I'm waiting for a long time!

@justinretzolk
Copy link
Member

Hey @imaginarynik 👋 Thank you for checking in on this. Unfortunately, I'm not able to provide an estimate on when this will be merged due to the potential of shifting priorities. We prioritize work by count of ":+1:" reactions, as well as a few other things. A larger prioritization document is in the works, but in the meantime additional information may be found in the FAQ.

@jw-maynard
Copy link

@justinretzolk This being de-prioritized seem a little odd to me since this prevents Terraform from converging on a stable state. Reporting that an apply is needed when the IaC matches the infrastructure that's deployed seems like the antithesis of what Terraform aims to accomplish. It seems like this should be a pretty high priority for the team no matter how many or how few thumbs up requests are there.

If you need a direct example, I have a workspace in Terraform cloud impacted by this and I cannot easily look at the Run history and determine when the infrastructure was actually changed because every plan creates a "update" that needs is applied. Every run looks like a change. In addition, I can't run a refresh only plan to easily spot differences between the infra and the IaC because every refresh plan indicates things have been changed due to this bug.

@justinretzolk
Copy link
Member

Hey @jw-maynard (and everyone else) -- apologies for the bit of delay in getting back to you on this one. I wanted to wait until I had a bit of a firmer idea of next steps here before replying so that I could get you the best information possible. We are currently working on Q2 prioritization, and based on that planning, I'm fairly confident in saying that this one will be included in that roadmap.

To whomever picks this up for review, I believe #6107 would be closed by this as well. Not linking in the "Closed by" yet, in case my read is off.

@imaginarynik
Copy link

@justinretzolk thank you so much for the confirmation. I was wondering when would the Q2 be? Estimate?

@justinretzolk
Copy link
Member

@imaginarynik 👋 While I can't comment on exactly when it'll be reviewed and merged during the quarter, we've entered Q2 as of this week 🙂.

@imaginarynik
Copy link

@justinretzolk that is such a relief! THANK YOU SO MUCH! :)

@justinretzolk
Copy link
Member

As a quick update: now that it's published, I can confirm that this is on the roadmap for this quarter 🎉

@YakDriver YakDriver self-assigned this Jul 7, 2022
@YakDriver YakDriver force-pushed the iam-assume-role-fix branch from 1ff814f to b2acddd Compare July 8, 2022 17:00
@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 8, 2022
Copy link
Member

@YakDriver YakDriver 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 testacc TESTS=TestAccIAMRole_ PKG=iam
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/iam/... -v -count 1 -parallel 20 -run='TestAccIAMRole_'  -timeout 180m
=== CONT  TestAccIAMRole_basic
=== CONT  TestAccIAMRole_InlinePolicy_ignoreOrder
=== CONT  TestAccIAMRole_InlinePolicy_outOfBandAdditionRemoved
=== CONT  TestAccIAMRole_disappears
=== CONT  TestAccIAMRole_InlinePolicy_basic
=== CONT  TestAccIAMRole_ManagedPolicy_outOfBandAdditionRemoved
=== CONT  TestAccIAMRole_nameGenerated
=== CONT  TestAccIAMRole_InlinePolicy_empty
=== CONT  TestAccIAMRole_tags
=== CONT  TestAccIAMRole_description
=== CONT  TestAccIAMRole_namePrefix
=== CONT  TestAccIAMRole_badJSON
=== CONT  TestAccIAMRole_testNameChange
=== CONT  TestAccIAMRole_InlinePolicy_outOfBandRemovalAddedBack
=== CONT  TestAccIAMRole_permissionsBoundary
=== CONT  TestAccIAMRole_ManagedPolicy_outOfBandRemovalAddedBack
=== CONT  TestAccIAMRole_ManagedPolicy_basic
=== CONT  TestAccIAMRole_maxSessionDuration
=== CONT  TestAccIAMRole_policiesForceDetach
=== CONT  TestAccIAMRole_InlinePolicy_outOfBandAdditionRemovedEmpty
--- PASS: TestAccIAMRole_badJSON (15.10s)
=== CONT  TestAccIAMRole_ManagedPolicy_outOfBandAdditionRemovedEmpty
--- PASS: TestAccIAMRole_disappears (80.53s)
=== CONT  TestAccIAMRole_ManagedPolicy_outOfBandAdditionIgnored
--- PASS: TestAccIAMRole_InlinePolicy_empty (95.19s)
=== CONT  TestAccIAMRole_InlinePolicy_outOfBandAdditionIgnored
--- PASS: TestAccIAMRole_namePrefix (113.21s)
--- PASS: TestAccIAMRole_policiesForceDetach (113.56s)
--- PASS: TestAccIAMRole_nameGenerated (113.62s)
--- PASS: TestAccIAMRole_basic (125.61s)
--- PASS: TestAccIAMRole_InlinePolicy_outOfBandRemovalAddedBack (171.36s)
--- PASS: TestAccIAMRole_ManagedPolicy_outOfBandRemovalAddedBack (171.90s)
--- PASS: TestAccIAMRole_ManagedPolicy_outOfBandAdditionRemovedEmpty (166.31s)
--- PASS: TestAccIAMRole_InlinePolicy_outOfBandAdditionRemovedEmpty (181.83s)
--- PASS: TestAccIAMRole_tags (181.95s)
--- PASS: TestAccIAMRole_ManagedPolicy_outOfBandAdditionRemoved (182.24s)
--- PASS: TestAccIAMRole_InlinePolicy_outOfBandAdditionRemoved (182.26s)
--- PASS: TestAccIAMRole_testNameChange (182.55s)
--- PASS: TestAccIAMRole_maxSessionDuration (195.78s)
--- PASS: TestAccIAMRole_ManagedPolicy_outOfBandAdditionIgnored (126.67s)
--- PASS: TestAccIAMRole_InlinePolicy_ignoreOrder (212.51s)
--- PASS: TestAccIAMRole_InlinePolicy_basic (213.57s)
--- PASS: TestAccIAMRole_description (217.49s)
--- PASS: TestAccIAMRole_ManagedPolicy_basic (223.60s)
--- PASS: TestAccIAMRole_InlinePolicy_outOfBandAdditionIgnored (137.41s)
--- PASS: TestAccIAMRole_permissionsBoundary (270.60s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/iam	272.739s

@github-actions
Copy link

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

@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/iam Issues and PRs that pertain to the iam service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
7 participants