feat: add override_action to aws_networkfirewall_firewall_policy

[teddylear]

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

Description

Add overide_action to aws_networkfirewall_firewall_policy

Output from acceptance testing:

terraform-provider-aws on  f-aws_networkfirewall_firewall_policy-add-stateless-rule-group-ref-override via 🐹 v1.17.3 took 6m 26s
[I] ➜  make testacc TESTS=TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference PKG=networkfirewall

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/networkfirewall/... -v -count 1 -parallel 20 -run='TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupRe
ference'  -timeout 180m
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged (156.55s)
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference (190.90s)
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction (314.68s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/networkfirewall    318.072s

terraform-provider-aws on  f-aws_networkfirewall_firewall_policy-add-stateless-rule-group-ref-override via 🐹 v1.17.3 took 6m 59s
[I] ➜ make testacc TESTS=TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference PKG=networkfirewall

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/networkfirewall/... -v -count 1 -parallel 20 -run='TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPr
iorityReference'  -timeout 180m
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference (170.63s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/networkfirewall    173.704s

terraform-provider-aws on  f-aws_networkfirewall_firewall_policy-add-stateless-rule-group-ref-override [!] via 🐹 v1.17.3 took 7s 
[I] ➜ make testacc TESTS=TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference PKG=networkfirewall

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/networkfirewall/... -v -count 1 -parallel 20 -run='TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOv
errideActionReference'  -timeout 180m
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference (148.27s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/networkfirewall    151.381s

Closes #25032

[teddylear]

@justinretzolk Hey I noticed the the Semgrep tests failed here, but the error message is unclear. Can you (or another engineer) provide more insights into the specific error with this check? As for the terrafmt check, I can fix that

[justinretzolk]

Hey @teddylear 👋 Thank you for your contribution, and for checking in on this! I took a quick look and it seems like the errors you were seeing in the tests have cleared up, so this should be good to go as soon as one of the maintainers is able to take a look at it 🙂

[GlennChia]

Hi @teddylear, thanks for the PR! This is a really useful addition that will benefit many practitioners. I left some review comments that are mainly to align the resource configuration to the API documentation where I noticed action was a nested argument in override. We usually follow the API documentation closely so that we can easily add new nested arguments should they be introduced in the future without making breaking changes. Let me know if you have any questions!

[teddylear]

@GlennChia Thanks for the review! I updated the PR from your comments above. One question I had was for the test I had to update the attribute I was checking on this line. Is there a way I can update the attribute from firewall_policy.0.stateful_rule_group_reference.0.override.0.action to firewall_policy.0.stateful_rule_group_reference.override.action? I'm assuming there should be only one override block and one action per stateful_rule_group_reference, so having an index on it does't make sense here.

[GlennChia]

Hi @teddylear, you're right to use firewall_policy.0.stateful_rule_group_reference.0.override.0.action because override is a struct (in the provider defined as a list with one element action), hence we still have to index it with .0 to extract action. Only if the argument is of TypeMap like Tags can we directly do something like override.action.

Changes look good to me! I realised for action that it's actually optional and added a small suggestion to change that. Finally, we will need a changelog entry to record your changes. In this case support for a new argument. Example: https://github.com/hashicorp/terraform-provider-aws/pull/22396/files#diff-49cb1cdcd29781976b8252c4783ae8382940622737037d741804bf1b68936599

[teddylear]

@GlennChia Added changes. Thanks again for another quick review, please let me know if there's anything else I should change!

[GlennChia]

Changes LGTM! Approving this PR, pending Hashi Maintainer final review.

[teddylear]

@GlennChia Only requested re-approval so that hashi maintainer can see that this was reviewed already. Thanks again for reviewing this and all the feedback!

[ewbankkit]

LGTM 🚀.

% make testacc TESTARGS='-run=TestAccNetworkFirewallFirewallPolicy_' PKG=networkfirewall ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/networkfirewall/... -v -count 1 -parallel 3  -run=TestAccNetworkFirewallFirewallPolicy_ -timeout 180m
=== RUN   TestAccNetworkFirewallFirewallPolicy_basic
=== PAUSE TestAccNetworkFirewallFirewallPolicy_basic
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulDefaultActions
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulDefaultActions
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulEngineOption
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulEngineOption
=== RUN   TestAccNetworkFirewallFirewallPolicy_updateStatefulEngineOption
=== PAUSE TestAccNetworkFirewallFirewallPolicy_updateStatefulEngineOption
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged
=== RUN   TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_multipleStatefulRuleGroupReferences
=== PAUSE TestAccNetworkFirewallFirewallPolicy_multipleStatefulRuleGroupReferences
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupPriorityReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupPriorityReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_statelessRuleGroupReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statelessRuleGroupReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_updateStatelessRuleGroupReference
=== PAUSE TestAccNetworkFirewallFirewallPolicy_updateStatelessRuleGroupReference
=== RUN   TestAccNetworkFirewallFirewallPolicy_multipleStatelessRuleGroupReferences
=== PAUSE TestAccNetworkFirewallFirewallPolicy_multipleStatelessRuleGroupReferences
=== RUN   TestAccNetworkFirewallFirewallPolicy_statelessCustomAction
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statelessCustomAction
=== RUN   TestAccNetworkFirewallFirewallPolicy_updateStatelessCustomAction
=== PAUSE TestAccNetworkFirewallFirewallPolicy_updateStatelessCustomAction
=== RUN   TestAccNetworkFirewallFirewallPolicy_multipleStatelessCustomActions
=== PAUSE TestAccNetworkFirewallFirewallPolicy_multipleStatelessCustomActions
=== RUN   TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction
=== PAUSE TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction
=== RUN   TestAccNetworkFirewallFirewallPolicy_tags
=== PAUSE TestAccNetworkFirewallFirewallPolicy_tags
=== RUN   TestAccNetworkFirewallFirewallPolicy_disappears
=== PAUSE TestAccNetworkFirewallFirewallPolicy_disappears
=== CONT  TestAccNetworkFirewallFirewallPolicy_basic
=== CONT  TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupPriorityReference
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged
--- PASS: TestAccNetworkFirewallFirewallPolicy_basic (155.43s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceManaged (157.08s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupPriorityReference (184.84s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_multipleStatefulRuleGroupReferences
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupOverrideActionReference (147.50s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupPriorityReference (170.73s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_updateStatelessCustomAction
--- PASS: TestAccNetworkFirewallFirewallPolicy_multipleStatefulRuleGroupReferences (172.47s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_disappears
--- PASS: TestAccNetworkFirewallFirewallPolicy_disappears (137.79s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_tags
--- PASS: TestAccNetworkFirewallFirewallPolicy_updateStatefulRuleGroupReference (199.76s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction
--- PASS: TestAccNetworkFirewallFirewallPolicy_tags (147.45s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_multipleStatelessCustomActions
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReferenceAndCustomAction (276.37s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_multipleStatelessRuleGroupReferences
--- PASS: TestAccNetworkFirewallFirewallPolicy_updateStatelessCustomAction (564.90s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statelessCustomAction
--- PASS: TestAccNetworkFirewallFirewallPolicy_multipleStatelessCustomActions (282.56s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_updateStatefulEngineOption
--- PASS: TestAccNetworkFirewallFirewallPolicy_multipleStatelessRuleGroupReferences (205.58s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_statelessCustomAction (153.40s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulEngineOption
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulRuleGroupReference (170.02s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_updateStatelessRuleGroupReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulEngineOption (167.31s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statefulDefaultActions
--- PASS: TestAccNetworkFirewallFirewallPolicy_updateStatefulEngineOption (302.76s)
=== CONT  TestAccNetworkFirewallFirewallPolicy_statelessRuleGroupReference
--- PASS: TestAccNetworkFirewallFirewallPolicy_updateStatelessRuleGroupReference (181.77s)
--- PASS: TestAccNetworkFirewallFirewallPolicy_statefulDefaultActions (163.15s)
--- PASS: TestAccNetworkFirewallFirewallPolicy_statelessRuleGroupReference (166.69s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/networkfirewall	1398.541s

[ewbankkit]

@teddylear Thanks for the contribution 🎉 👏.

[github-actions]

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

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.