-
Notifications
You must be signed in to change notification settings - Fork 9.9k
IAM: New variable to apply delay in ms between create and update policy version #42054
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
Conversation
AWS: Policy Create and Update Changes
Changing delay to 5secs
Sanjay/terraform apply variables
Community GuidelinesThis comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀 Voting for Prioritization
Pull Request Authors
|
|
|
Hi @Sanjay3101, thanks for the PR. Can you please update the PR to allow maintainers to make edits? That will allow us to make any small changes if necessary |
gdavison
left a comment
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.
I have a few comments below. The PR also needs
- A changelog entry
- Documentation for the new attribute
| "delay_after_policy_creation_in_ms": { | ||
| Type: schema.TypeInt, | ||
| Optional: true, | ||
| Default: -1, | ||
| }, |
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.
The default value can be removed, and below the check should be d.GetOk. The ok value will be false if there is no value set.
| if delayAfterPolicyCreationInMs == -1 { | ||
| input := &iam.CreatePolicyVersionInput{ | ||
| PolicyArn: aws.String(d.Id()), | ||
| PolicyDocument: aws.String(policy), | ||
| SetAsDefault: true, | ||
| } | ||
| _, err = conn.CreatePolicyVersion(ctx, input) | ||
|
|
||
| _, err = conn.CreatePolicyVersion(ctx, input) | ||
| if err != nil { | ||
| return sdkdiag.AppendErrorf(diags, "updating IAM Policy (%s): %s", d.Id(), err) | ||
| } | ||
| } else { | ||
|
|
||
| // Creating a policy and setting its version as default in a single operation can expose a brief interval where | ||
| // valid STS tokens with attached Session Policies are rejected by AWS authorization servers that have | ||
| // not received the new default policy version. Separating this into two distinct actions of creating a policy version, | ||
| // pausing briefly, and then setting that to the default version can avoid this issue, and may be required | ||
| // in environments with very high S3 IO loads. | ||
|
|
||
| input := &iam.CreatePolicyVersionInput{ | ||
| PolicyArn: aws.String(d.Id()), | ||
| PolicyDocument: aws.String(policy), | ||
| SetAsDefault: false, | ||
| } | ||
|
|
||
| var policyVersionOutput *iam.CreatePolicyVersionOutput | ||
| policyVersionOutput, err = conn.CreatePolicyVersion(ctx, input) |
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.
The setup and call to CreatePolicyVersion can be combined and the conditional can be for the call to SetDefaultPolicyVersion
| func TestAccIAMPolicy_updateWithoutDelay(t *testing.T) { | ||
| ctx := acctest.Context(t) | ||
| var out awstypes.Policy | ||
| resourceName := "aws_iam_policy.test" | ||
| name := "test" | ||
| description := "policy_create_update_with_delay" | ||
| delayAfterPolicyCreationVariable := "delay_after_policy_creation_in_ms" | ||
|
|
||
| resource.ParallelTest(t, resource.TestCase{ | ||
| PreCheck: func() { acctest.PreCheck(ctx, t) }, | ||
| ErrorCheck: acctest.ErrorCheck(t, names.IAMServiceID), | ||
| ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, | ||
| CheckDestroy: testAccCheckPolicyDestroy(ctx), | ||
| Steps: []resource.TestStep{ | ||
| { | ||
| Config: testAccPolicyConfig_description(name, description), | ||
| Check: resource.ComposeTestCheckFunc( | ||
| testAccCheckPolicyExists(ctx, resourceName, &out), | ||
| resource.TestCheckResourceAttr(resourceName, names.AttrDescription, description), | ||
| resource.TestCheckResourceAttr(resourceName, delayAfterPolicyCreationVariable, "-1"), | ||
| ), | ||
| }, | ||
| }, | ||
| }) | ||
| } |
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.
This case will be covered by the basic test
| { | ||
| Config: testAccPolicyConfig_updateWithDelay(name, description, -1), | ||
| Check: resource.ComposeTestCheckFunc( | ||
| testAccCheckPolicyExists(ctx, resourceName, &out), | ||
| resource.TestCheckResourceAttr(resourceName, names.AttrDescription, description), | ||
| resource.TestCheckResourceAttr(resourceName, delayAfterPolicyCreationVariable, "-1"), | ||
| ), | ||
| }, | ||
| { | ||
| Config: testAccPolicyConfig_updateWithDelay(name, description, 3000), | ||
| Check: resource.ComposeTestCheckFunc( | ||
| testAccCheckPolicyExists(ctx, resourceName, &updatedPolicyOut), | ||
| testAccVerifyLatestPolicyId(&out, &updatedPolicyOut), | ||
| resource.TestCheckResourceAttr(resourceName, names.AttrDescription, description), | ||
| resource.TestCheckResourceAttr(resourceName, delayAfterPolicyCreationVariable, "3000"), | ||
| ), | ||
| }, |
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.
These two steps do not modify the Policy, other than the value of delay_after_policy_creation_in_ms, so it doesn't test the operation of the delay on creation
|
Warning This Issue has been closed, meaning that any additional comments are much easier for the maintainers to miss. Please assume that the maintainers will not see them. Ongoing conversations amongst community members are welcome, however, the issue will be locked after 30 days. Moving conversations to another venue, such as the AWS Provider forum, is recommended. If you have additional concerns, please open a new issue, referencing this one where needed. |
|
@Sanjay3101 Thanks for the contribution 🎉 👏. |
Description
Currently, when policies are applied, the AWS Terraform provider executes two actions simultaneously: Create a policy and setting it as default version.
These two operations when run in a single operation can expose a brief interval where valid STS tokens with attached Session Policies are rejected by AWS authorization servers that have not received the new default policy version. Separating this into two distinct actions of creating a policy version, pausing briefly, and then setting that to the default version can avoid this issue, and may be required in environments with very high S3 IO loads.
This pull request introduces a new variable delay_after_policy_creation_in_ms which can be used by users to apply a delay between these API calls.
Relations
Closes #0000
References
Output from Acceptance Testing