Skip to content

Conversation

joelmccoy
Copy link
Contributor

@joelmccoy joelmccoy commented Apr 19, 2024

Description

This changes the aws_guardduty_detector_feature additional_configuration type from a list to a set. This is so that the order of the additional configurations does not matter. Before pushing this fix, if you didn't provide the additional_configurations in the order the AWS API returns them, there would be a force replace on every terraform apply.

I added an additional acceptance test to confirm that the ordering of the additional_configurations doesn't matter and that an empty plan is returned no matter the order.

Relations

Closes #36400

References

Output from Acceptance Testing

%  AWS_PROFILE=sandbox make testacc TESTS=TestAccGuard
Duty_serial/DetectorFeature PKG=guardduty 
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.21.8 test ./internal/service/guardduty/... -v -count 1 -parallel 20 -run='TestAccGuardDuty_serial/Detect
orFeature'  -timeout 360m
=== RUN   TestAccGuardDuty_serial
=== PAUSE TestAccGuardDuty_serial
=== CONT  TestAccGuardDuty_serial
=== RUN   TestAccGuardDuty_serial/DetectorFeature
=== RUN   TestAccGuardDuty_serial/DetectorFeature/basic
=== RUN   TestAccGuardDuty_serial/DetectorFeature/additional_configuration
=== RUN   TestAccGuardDuty_serial/DetectorFeature/additional_configuration_order
=== RUN   TestAccGuardDuty_serial/DetectorFeature/multiple
--- PASS: TestAccGuardDuty_serial (120.23s)
    --- PASS: TestAccGuardDuty_serial/DetectorFeature (120.23s)
        --- PASS: TestAccGuardDuty_serial/DetectorFeature/basic (15.78s)
        --- PASS: TestAccGuardDuty_serial/DetectorFeature/additional_configuration (45.32s)
        --- PASS: TestAccGuardDuty_serial/DetectorFeature/additional_configuration_order (22.66s)
        --- PASS: TestAccGuardDuty_serial/DetectorFeature/multiple (36.47s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/guardduty  124.516s
...

Copy link
Contributor

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 size/M Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. service/guardduty Issues and PRs that pertain to the guardduty service. labels Apr 19, 2024
@terraform-aws-provider terraform-aws-provider bot added the needs-triage Waiting for first response or review from a maintainer. label Apr 19, 2024
@joelmccoy joelmccoy force-pushed the b-guardduty_feature_additional_configuration_order branch 2 times, most recently from a3b639b to b3d9d70 Compare April 19, 2024 01:53
@joelmccoy joelmccoy marked this pull request as ready for review April 19, 2024 01:55
@joelmccoy joelmccoy force-pushed the b-guardduty_feature_additional_configuration_order branch from b3d9d70 to 4c458a3 Compare April 19, 2024 17:54
@justinretzolk justinretzolk added bug Addresses a defect in current functionality. and removed needs-triage Waiting for first response or review from a maintainer. labels May 1, 2024
@sbkg0002
Copy link

sbkg0002 commented Jan 3, 2025

@sharmajikabeta13 Is there a planning when this will be merged?

@icco
Copy link

icco commented Oct 11, 2025

@jar-b could we get this reviewed and merged? Should fix #36400 and #36695. If needed I rebased this against main: #44627

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Addresses a defect in current functionality. service/guardduty Issues and PRs that pertain to the guardduty service. size/M 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

Development

Successfully merging this pull request may close these issues.

[Bug]: aws_guardduty_detector_feature additional_configuration blocks must be in a particular order, else they force replacement on every run

5 participants