r/aws_resourcegroups_resource: fix crash when attempting to parse arn #40579
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the `group_arn` and `resource_arn` values were assumed to be ARNs in a format which could always be split by a `/` into at least two parts. The provider did no validation of these inputs prior to attempting to split the value, causing `index out of range` crashes when values were provided that did not match these assumptions. Instead of attempting to split ARNs of varying formatting to construct the `id` attribute, `id` will now be composed of a comma-delimited string combining `group_arn` and `resource_arn` in their entirety. This removes the logic and assumptions around splitting the ARN and should prevent future crashes regardless of the format provided. The more complete content of the `id` attribute will also enable supporting import for this resource, if desired. ```console % make testacc PKG=resourcegroups TESTS=TestAccResourceGroupsResource_ make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.23.3 test ./internal/service/resourcegroups/... -v -count 1 -parallel 20 -run='TestAccResourceGroupsResource_' -timeout 360m 2024/12/13 16:29:28 Initializing Terraform AWS Provider... --- PASS: TestAccResourceGroupsResource_basic (198.52s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/resourcegroups 203.664s ```
An `aws_resourcegroups_resource` resource can now be imported using a comma-delimited string combining `group_arn` and `resource_arn`. ```console % make testacc PKG=resourcegroups TESTS=TestAccResourceGroupsResource_ make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.23.3 test ./internal/service/resourcegroups/... -v -count 1 -parallel 20 -run='TestAccResourceGroupsResource_' -timeout 360m 2024/12/16 11:04:35 Initializing Terraform AWS Provider... --- PASS: TestAccResourceGroupsResource_basic (21.88s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/resourcegroups 28.172s ```
Community NoteVoting for Prioritization
For Submitters
|
github-actions
bot
added
documentation
jar-b
added
bug
ewbankkit
reviewed
Dec 16, 2024
The state upgrader will allow for migration of the pre-existing `id` format to the new comma-delimited format. An acceptance test was also added to cover the provider version upgrade workflow. ```console % make testacc PKG=resourcegroups TESTS=TestAccResourceGroupsResource_ make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... TF_ACC=1 go1.23.3 test ./internal/service/resourcegroups/... -v -count 1 -parallel 20 -run='TestAccResourceGroupsResource_' -timeout 360m 2024/12/17 16:33:36 Initializing Terraform AWS Provider... --- PASS: TestAccResourceGroupsResource_basic (22.66s) --- PASS: TestAccResourceGroupsResource_v5_82_0_upgrade (48.45s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/resourcegroups 54.879s ```
|
Acceptance tests after adding a state upgrader and including a new case for upgrading beyond % make testacc PKG=resourcegroups TESTS=TestAccResourceGroupsResource_
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/resourcegroups/... -v -count 1 -parallel 20 -run='TestAccResourceGroupsResource_' -timeout 360m
2024/12/17 16:33:36 Initializing Terraform AWS Provider...
--- PASS: TestAccResourceGroupsResource_basic (22.66s)
--- PASS: TestAccResourceGroupsResource_v5_82_0_upgrade (48.45s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/resourcegroups 54.879s |
ewbankkit
approved these changes
Dec 18, 2024
There was a problem hiding this comment.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccResourceGroupsResource_' PKG=resourcegroups
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.3 test ./internal/service/resourcegroups/... -v -count 1 -parallel 20 -run=TestAccResourceGroupsResource_ -timeout 360m
2024/12/18 10:10:41 Initializing Terraform AWS Provider...
=== RUN TestAccResourceGroupsResource_basic
=== PAUSE TestAccResourceGroupsResource_basic
=== RUN TestAccResourceGroupsResource_v5_82_0_upgrade
=== PAUSE TestAccResourceGroupsResource_v5_82_0_upgrade
=== CONT TestAccResourceGroupsResource_basic
=== CONT TestAccResourceGroupsResource_v5_82_0_upgrade
--- PASS: TestAccResourceGroupsResource_basic (26.51s)
--- PASS: TestAccResourceGroupsResource_v5_82_0_upgrade (54.69s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/resourcegroups 60.585s
|
This functionality has been released in v5.82.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
bot
removed
the
prioritized
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Previously the
group_arnandresource_arnvalues were assumed to be ARNs in a format which could always be split by a/into at least two parts. The provider did no validation of these inputs prior to attempting to split the value, causingindex out of rangecrashes when values were provided that did not match these assumptions.Instead of attempting to split ARNs of varying formatting to construct the
idattribute,idwill now be composed of a comma-delimited string combininggroup_arnandresource_arnin their entirety. This removes the logic and assumptions around splitting the ARN and should prevent future crashes regardless of the format provided. The more complete content of theidattribute will also enable supporting import for this resource (also added as part of this PR).Relations
Closes #40504
Closes #40458
Output from Acceptance Testing