-
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
resource/wafv2_web_acl: remove force_new property from arguments to prevent resource recreation #14616
Conversation
3861ca0
to
981800c
Compare
@@ -253,7 +252,7 @@ func resourceAwsWafv2WebACLUpdate(d *schema.ResourceData, meta interface{}) erro | |||
Scope: aws.String(d.Get("scope").(string)), | |||
LockToken: aws.String(d.Get("lock_token").(string)), | |||
DefaultAction: expandWafv2DefaultAction(d.Get("default_action").([]interface{})), | |||
Rules: expandWafv2Rules(d.Get("rule").(*schema.Set).List()), | |||
Rules: expandWafv2WebACLRules(d.Get("rule").(*schema.Set).List()), |
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.
addresses TestAccAwsWafv2WebACL_ManagedRuleGroupStatement
and TestAccAwsWafv2WebACL_RateBasedStatement
test failures as now they actually attempt to do an update
vs. a destroy/create
, exposing the bug reported in #14035
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.
Looks good to me 🚀 Two additional recommendations:
- Adding documentation to the
rule
configuration blockname
argument to note that it cannot be updated - Adjusting the CHANGELOG message to be a little more operator-centric rather than developer-centric (e.g. Prevent unnecessary resource recreation with
rule
updates)
Output from acceptance testing (test failure unrelated):
--- FAIL: TestAccAwsWafv2WebACL_RuleGroupReferenceStatement (2297.93s)
--- PASS: TestAccAwsWafv2WebACL_basic (3247.90s)
--- PASS: TestAccAwsWafv2WebACL_ChangeNameForceNew (4845.73s)
--- PASS: TestAccAwsWafv2WebACL_Disappears (2624.62s)
--- PASS: TestAccAwsWafv2WebACL_ManagedRuleGroupStatement (5485.60s)
--- PASS: TestAccAwsWafv2WebACL_MaxNestedOperatorStatements (4079.01s)
--- PASS: TestAccAwsWafv2WebACL_MaxNestedRateBasedStatements (4075.84s)
--- PASS: TestAccAwsWafv2WebACL_Minimal (3069.09s)
--- PASS: TestAccAwsWafv2WebACL_RateBasedStatement (5505.18s)
--- PASS: TestAccAwsWafv2WebACL_Tags (5556.15s)
--- PASS: TestAccAwsWafv2WebACL_updateRule (5690.97s)
--- PASS: TestAccAwsWafv2WebACL_UpdateRuleProperties (6207.71s)
thanks for the review @bflad! so interestingly enough,
and then applying this plan succeeds:
with no subsequent plan-time changes:
so i think it's safe to keep the docs as is 👍 the same behavior is true for the |
This has been released in version 3.2.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 for triage. Thanks! |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks! |
Community Note
Closes #13936
Fixes #14035
Fixes #14029
Notes:
rule
name
andrule
visibility_config
metric_name
can be changed via the SDK just like in the AWS Console, no longer requiring theForceNew
property which was also the cause of the forced recreation behaviorRelease note for CHANGELOG:
Output from acceptance testing: