r/aws_athena_workgroup - Add support for managed_query_results_configuration

[subham-ibmhc]

Rollback Plan

If a change needs to be reverted, we will publish an updated version of the library.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

Description

Adds support to aws_athena_workgroup resource for managed storage for Athena query results.

Relations

Closes #43246

References

https://docs.aws.amazon.com/athena/latest/APIReference/API_ManagedQueryResultsConfiguration.html
https://docs.aws.amazon.com/athena/latest/APIReference/API_WorkGroupConfiguration.html

Output from Acceptance Testing

% make testacc TESTS=TestAccAthenaWorkGroup_ManagedQueryResultsConfiguration PKG=athena
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
make: Running acceptance tests on branch: 🌿 r/aws_athena_workgroup-add_managed_storage 🌿...
TF_ACC=1 go1.24.10 test ./internal/service/athena/... -v -count 1 -parallel 20 -run='TestAccAthenaWorkGroup_ManagedQueryResultsConfiguration'  -timeout 360m -vet=off
2025/11/20 13:35:33 Creating Terraform AWS Provider (SDKv2-style)...
2025/11/20 13:35:33 Initializing Terraform AWS Provider (SDKv2-style)...
=== RUN   TestAccAthenaWorkGroup_ManagedQueryResultsConfiguration
=== PAUSE TestAccAthenaWorkGroup_ManagedQueryResultsConfiguration
=== CONT  TestAccAthenaWorkGroup_ManagedQueryResultsConfiguration
--- PASS: TestAccAthenaWorkGroup_ManagedQueryResultsConfiguration (34.18s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/athena     38.854s
...

[github-actions]

Community Guidelines

This 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

• Please vote on this Pull Request by adding a 👍 reaction to the original post to help the community and maintainers prioritize it.
• Please see our prioritization guide for additional information on how the maintainers handle prioritization.
• Please do not leave +1 or other comments that do not add relevant new information or questions; they generate extra noise for others following the Pull Request and do not help prioritize the request.

Pull Request Authors

• Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
• Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

[subham-ibmhc]

For the attribute managed_query_results_configuration.enabled, when set to true, the attribute result_configuration.output_location cannot be set.
I am not sure how to implement a validation for this, as I am not sure ConflictsWith works here?
Should a validation be added for this or is the documentation enough?

[jar-b]

Unless necessary for the API to function, we typically avoid provider-side defaults.
https://hashicorp.github.io/terraform-provider-aws/data-handling-and-conversion/#default-values

[jar-b]

Can we add additional steps to this test case to exercise disabling, then re-enabling the feature to ensure update works as expected?

[jar-b]

nit - making values and attribute names format as code can make the rendered result a bit more readable.

Suggested change

	* `enabled` - (Optional) If set to true, allows you to store query results in Athena owned storage. If set to false, workgroup member stores query results in location specified under result_configuration.output_location. The default is false. A workgroup cannot have the result_configuration.output_location parameter when you set this field to true.
	* `enabled` - (Optional) If set to `true`, allows you to store query results in Athena owned storage. If set to `false`, workgroup member stores query results in the location specified by `result_configuration.output_location`. The default is `false`. A workgroup cannot have the `result_configuration.output_location` set when this is `true`.

[jar-b]

For the attribute managed_query_results_configuration.enabled, when set to true, the attribute result_configuration.output_location cannot be set.
I am not sure how to implement a validation for this, as I am not sure ConflictsWith works here?

Because the attributes are split across arguments, you'll likely need a CustomizeDiff function which is able to retrieve configured values from the entire schema and then perform actions such as forcing replacements, marking attributes as computed, or throwing errors. The Plugin SDK V2 documentation on the topic is here, and grepping for CustomizeDiff will yield some prior art for handwritten customize diff logic in other resources.

https://developer.hashicorp.com/terraform/plugin/sdkv2/resources/customizing-differences

[manaty226]

@subham-ibmhc

Thanks for your work on this!
I noticed that #45223 addresses the same problem as my existing PR, #44273.
To avoid duplicated review effort and accelerate the merge, could we perhaps merge your changes into #44273?

Thanks!

[subham-ibmhc]

@subham-ibmhc

Thanks for your work on this! I noticed that #45223 addresses the same problem as my existing PR, #44273. To avoid duplicated review effort and accelerate the merge, could we perhaps merge your changes into #44273?

Thanks!

@manaty226

Thank you for your contribution. I would be glad to merge my changes into #44273.

However, while trying to test the update functionality, I have noticed a few issues with our previous approach. I suspect the same issues may be causing #37917, as the logic surrounding result_configuration is similar. I will try to summarise the issues below -

1. When the resource is initially created with enabled and encryption_configuration set, and later as part of update, encryption_configuration block is removed, the expand function (expandWorkGroupManagedQueryResultsConfigurationUpdates in my case) still returns an empty struct for encryptionConfiguration. This is because the config passed is a list with one element (empty kms key) , i.e. - [map[kms_key:]] instead of an empty list. Hence the removeEncryptionConfiguration flag is never set.
2. When the resource is initially created with enabled and encryption_configuration set, and later as part of update, enabled is set to false, the expand function maps enabled as false, and either ends up sending the encryption_configuration struct, which leads to the upstream API complaining to remove all other attributes in the managed_query_results_configuration block, or ends up sending encryption_configuration struct with an empty kms key (same as point 1), which leads to InvalidInputError from the upstream API as the kms key is required field with encryption_configuration.
3. As highlighted above, to implement the constraint that exists between enabled and output_location, a customDiff function will be needed since conflictsWith will not work for attributes split across arguments.

To fix these issues, I have changed the approach of the expandWorkGroupManagedQueryResultsConfigurationUpdates function, now it sets the remove encryption attribute if enabled is false and returns early, but also checks if the returned encryption_config from the expandManagedQueryResultsEncryptionConfiguration is meaningful and only then expands it, otherwise sets the remove_encryption attribute.
To test the above scenarios, I have also added some more test cases.

[manaty226]

Hi @subham-ibmhc

Thank you for the detailed investigation and for your willingness to merge your changes into #44273.

I fully understand the issues you identified regarding the update logic for encryption_configuration.
Your proposed fix for expandWorkGroupManagedQueryResultsConfigurationUpdates looks good, and I appreciate you adding the extra test cases to cover these scenarios.

Please proceed with the merge. Thanks again for your hard work on this!