Skip to content
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

r/aws_neptune_cluster_instance: remove engine_version as ForceNew parameter and fixed default cluster parameter behavior. #33487

Merged
merged 15 commits into from
Oct 13, 2023

Conversation

triggan
Copy link
Contributor

@triggan triggan commented Sep 15, 2023

Description

Changes behavior of engine_version parameter on aws_neptune_cluster_instance resource to ignore this parameter. This complies with the AWS documentation for both create-db-instance and modify-db-instance. engine_version is controlled at a cluster level in Neptune.

More info in Issue #33467 .

This PR also addresses an issue where cluster parameters defined in a template with an engine-default value are seen as changes an terraform apply. Changes are included here to evaluate defined parameters in a template against defaults in a cluster parameter group to ensure only changes are submitted if the change includes a non-default value or if changing the value back to a default.

More info in Issue #20056.

Submitted new tests to check for default parameter values and ensure this is seen as a non-change. Also updated existing tests that were failing due to deprecated engine versions being used.

Relations

Closes #33467.
Closes #29785.
Closes #20056.
Closes #28260.

References

https://docs.aws.amazon.com/cli/latest/reference/neptune/create-db-instance.html
https://docs.aws.amazon.com/cli/latest/reference/neptune/modify-db-instance.html

Output from Acceptance Testing

$ make testacc PKG=neptune ACCTEST_PARALLELISM=2 TESTS=TestAccNeptuneCluster_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/neptune/... -v -count 1 -parallel 2 -run='TestAccNeptuneCluster_'  -timeout 180m
=== RUN   TestAccNeptuneCluster_basic
=== PAUSE TestAccNeptuneCluster_basic
=== RUN   TestAccNeptuneCluster_copyTagsToSnapshot
=== PAUSE TestAccNeptuneCluster_copyTagsToSnapshot
=== RUN   TestAccNeptuneCluster_namePrefix
=== PAUSE TestAccNeptuneCluster_namePrefix
=== RUN   TestAccNeptuneCluster_serverlessConfiguration
=== PAUSE TestAccNeptuneCluster_serverlessConfiguration
=== RUN   TestAccNeptuneCluster_takeFinalSnapshot
=== PAUSE TestAccNeptuneCluster_takeFinalSnapshot
=== RUN   TestAccNeptuneCluster_tags
=== PAUSE TestAccNeptuneCluster_tags
=== RUN   TestAccNeptuneCluster_updateIAMRoles
=== PAUSE TestAccNeptuneCluster_updateIAMRoles
=== RUN   TestAccNeptuneCluster_kmsKey
=== PAUSE TestAccNeptuneCluster_kmsKey
=== RUN   TestAccNeptuneCluster_encrypted
=== PAUSE TestAccNeptuneCluster_encrypted
=== RUN   TestAccNeptuneCluster_backupsUpdate
=== PAUSE TestAccNeptuneCluster_backupsUpdate
=== RUN   TestAccNeptuneCluster_iamAuth
=== PAUSE TestAccNeptuneCluster_iamAuth
=== RUN   TestAccNeptuneCluster_updateCloudWatchLogsExports
=== PAUSE TestAccNeptuneCluster_updateCloudWatchLogsExports
=== RUN   TestAccNeptuneCluster_updateEngineVersion
=== PAUSE TestAccNeptuneCluster_updateEngineVersion
=== RUN   TestAccNeptuneCluster_updateEngineMajorVersion
=== PAUSE TestAccNeptuneCluster_updateEngineMajorVersion
=== RUN   TestAccNeptuneCluster_GlobalClusterIdentifier_PrimarySecondaryClusters
=== PAUSE TestAccNeptuneCluster_GlobalClusterIdentifier_PrimarySecondaryClusters
=== RUN   TestAccNeptuneCluster_deleteProtection
=== PAUSE TestAccNeptuneCluster_deleteProtection
=== RUN   TestAccNeptuneCluster_disappears
=== PAUSE TestAccNeptuneCluster_disappears
=== RUN   TestAccNeptuneCluster_restoreFromSnapshot
=== PAUSE TestAccNeptuneCluster_restoreFromSnapshot
=== CONT  TestAccNeptuneCluster_basic
=== CONT  TestAccNeptuneCluster_backupsUpdate
--- PASS: TestAccNeptuneCluster_basic (157.46s)
=== CONT  TestAccNeptuneCluster_tags
--- PASS: TestAccNeptuneCluster_backupsUpdate (193.14s)
=== CONT  TestAccNeptuneCluster_encrypted
--- PASS: TestAccNeptuneCluster_encrypted (146.30s)
=== CONT  TestAccNeptuneCluster_kmsKey
--- PASS: TestAccNeptuneCluster_tags (207.66s)
=== CONT  TestAccNeptuneCluster_updateIAMRoles
--- PASS: TestAccNeptuneCluster_kmsKey (168.39s)
=== CONT  TestAccNeptuneCluster_serverlessConfiguration
--- PASS: TestAccNeptuneCluster_updateIAMRoles (189.32s)
=== CONT  TestAccNeptuneCluster_takeFinalSnapshot
--- PASS: TestAccNeptuneCluster_serverlessConfiguration (143.71s)
=== CONT  TestAccNeptuneCluster_disappears
--- PASS: TestAccNeptuneCluster_disappears (161.00s)
=== CONT  TestAccNeptuneCluster_restoreFromSnapshot
--- PASS: TestAccNeptuneCluster_takeFinalSnapshot (305.14s)
=== CONT  TestAccNeptuneCluster_updateEngineVersion
--- PASS: TestAccNeptuneCluster_restoreFromSnapshot (556.86s)
=== CONT  TestAccNeptuneCluster_updateEngineMajorVersion
=== NAME  TestAccNeptuneCluster_updateEngineVersion
--- PASS: TestAccNeptuneCluster_updateEngineVersion (1568.07s)
=== CONT  TestAccNeptuneCluster_namePrefix
--- PASS: TestAccNeptuneCluster_namePrefix (163.43s)
=== CONT  TestAccNeptuneCluster_deleteProtection
--- PASS: TestAccNeptuneCluster_deleteProtection (235.46s)
=== CONT  TestAccNeptuneCluster_updateCloudWatchLogsExports
--- PASS: TestAccNeptuneCluster_updateCloudWatchLogsExports (225.14s)
=== CONT  TestAccNeptuneCluster_copyTagsToSnapshot
--- PASS: TestAccNeptuneCluster_copyTagsToSnapshot (267.52s)
=== CONT  TestAccNeptuneCluster_iamAuth
--- PASS: TestAccNeptuneCluster_iamAuth (153.94s)
=== CONT  TestAccNeptuneCluster_GlobalClusterIdentifier_PrimarySecondaryClusters
--- PASS: TestAccNeptuneCluster_updateEngineMajorVersion (2477.73s)
--- PASS: TestAccNeptuneCluster_GlobalClusterIdentifier_PrimarySecondaryClusters (3287.73s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/neptune    6760.988s

$ make testacc PKG=neptune ACCTEST_PARALLELISM=2 TESTS=TestAccNeptuneClusterParameterGroup_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/neptune/... -v -count 1 -parallel 2 -run='TestAccNeptuneClusterParameterGroup_'  -timeout 180m
=== RUN   TestAccNeptuneClusterParameterGroup_basic
=== PAUSE TestAccNeptuneClusterParameterGroup_basic
=== RUN   TestAccNeptuneClusterParameterGroup_namePrefix
=== PAUSE TestAccNeptuneClusterParameterGroup_namePrefix
=== RUN   TestAccNeptuneClusterParameterGroup_generatedName
=== PAUSE TestAccNeptuneClusterParameterGroup_generatedName
=== RUN   TestAccNeptuneClusterParameterGroup_description
=== PAUSE TestAccNeptuneClusterParameterGroup_description
=== RUN   TestAccNeptuneClusterParameterGroup_NamePrefix_parameter
=== PAUSE TestAccNeptuneClusterParameterGroup_NamePrefix_parameter
=== RUN   TestAccNeptuneClusterParameterGroup_parameter
=== PAUSE TestAccNeptuneClusterParameterGroup_parameter
=== RUN   TestAccNeptuneClusterParameterGroup_parameterDefault
=== PAUSE TestAccNeptuneClusterParameterGroup_parameterDefault
=== RUN   TestAccNeptuneClusterParameterGroup_tags
=== PAUSE TestAccNeptuneClusterParameterGroup_tags
=== CONT  TestAccNeptuneClusterParameterGroup_basic
=== CONT  TestAccNeptuneClusterParameterGroup_NamePrefix_parameter
--- PASS: TestAccNeptuneClusterParameterGroup_basic (26.07s)
=== CONT  TestAccNeptuneClusterParameterGroup_parameterDefault
--- PASS: TestAccNeptuneClusterParameterGroup_NamePrefix_parameter (44.03s)
=== CONT  TestAccNeptuneClusterParameterGroup_tags
--- PASS: TestAccNeptuneClusterParameterGroup_parameterDefault (22.19s)
=== CONT  TestAccNeptuneClusterParameterGroup_generatedName
--- PASS: TestAccNeptuneClusterParameterGroup_generatedName (25.74s)
=== CONT  TestAccNeptuneClusterParameterGroup_description
--- PASS: TestAccNeptuneClusterParameterGroup_description (25.68s)
=== CONT  TestAccNeptuneClusterParameterGroup_parameter
--- PASS: TestAccNeptuneClusterParameterGroup_tags (61.46s)
=== CONT  TestAccNeptuneClusterParameterGroup_namePrefix
--- PASS: TestAccNeptuneClusterParameterGroup_namePrefix (25.84s)
--- PASS: TestAccNeptuneClusterParameterGroup_parameter (42.35s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/neptune    142.149s

make testacc PKG=neptune ACCTEST_PARALLELISM=2 TESTS=TestAccNeptuneOrderableDBInstanceDataSource
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/neptune/... -v -count 1 -parallel 2 -run='TestAccNeptuneOrderableDBInstanceDataSource'  -timeout 180m
=== RUN   TestAccNeptuneOrderableDBInstanceDataSource_basic
=== PAUSE TestAccNeptuneOrderableDBInstanceDataSource_basic
=== RUN   TestAccNeptuneOrderableDBInstanceDataSource_preferred
=== PAUSE TestAccNeptuneOrderableDBInstanceDataSource_preferred
=== CONT  TestAccNeptuneOrderableDBInstanceDataSource_basic
=== CONT  TestAccNeptuneOrderableDBInstanceDataSource_preferred
--- PASS: TestAccNeptuneOrderableDBInstanceDataSource_basic (20.92s)
--- PASS: TestAccNeptuneOrderableDBInstanceDataSource_preferred (21.07s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/neptune    21.176s

$ make testacc PKG=neptune ACCTEST_PARALLELISM=2 TESTS=TestAccNeptuneEngineVersionDataSource
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/neptune/... -v -count 1 -parallel 2 -run='TestAccNeptuneEngineVersionDataSource'  -timeout 180m
=== RUN   TestAccNeptuneEngineVersionDataSource_basic
=== PAUSE TestAccNeptuneEngineVersionDataSource_basic
=== RUN   TestAccNeptuneEngineVersionDataSource_preferred
=== PAUSE TestAccNeptuneEngineVersionDataSource_preferred
=== RUN   TestAccNeptuneEngineVersionDataSource_defaultOnly
=== PAUSE TestAccNeptuneEngineVersionDataSource_defaultOnly
=== CONT  TestAccNeptuneEngineVersionDataSource_basic
=== CONT  TestAccNeptuneEngineVersionDataSource_defaultOnly
--- PASS: TestAccNeptuneEngineVersionDataSource_defaultOnly (19.86s)
=== CONT  TestAccNeptuneEngineVersionDataSource_preferred
--- PASS: TestAccNeptuneEngineVersionDataSource_basic (19.88s)
--- PASS: TestAccNeptuneEngineVersionDataSource_preferred (16.98s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/neptune    36.951s

@github-actions
Copy link

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/XS Managed by automation to categorize the size of a PR. service/neptune Issues and PRs that pertain to the neptune service. labels Sep 15, 2023
@terraform-aws-provider terraform-aws-provider bot added needs-triage Waiting for first response or review from a maintainer. partner Contribution from a partner. labels Sep 15, 2023
@jar-b jar-b removed the needs-triage Waiting for first response or review from a maintainer. label Sep 18, 2023
Added new test to ensure parameters with default values are ignored.
Updated existing tests that were referencing deprecated engine versions.
@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. and removed size/XS Managed by automation to categorize the size of a PR. labels Sep 20, 2023
@triggan triggan changed the title r/aws_neptune_cluster_instance: remove engine_version as ForceNew parameter. r/aws_neptune_cluster_instance: remove engine_version as ForceNew parameter and fixed default cluster parameter behavior. Sep 20, 2023
@ewbankkit ewbankkit added the bug Addresses a defect in current functionality. label Oct 12, 2023
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

% ACCTEST_TIMEOUT=720m make testacc TESTARGS='-run=TestAccNeptuneClusterParameterGroup_\|TestAccNeptuneClusterInstance_' PKG=neptune ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/neptune/... -v -count 1 -parallel 3  -run=TestAccNeptuneClusterParameterGroup_\|TestAccNeptuneClusterInstance_ -timeout 720m
=== RUN   TestAccNeptuneClusterInstance_basic
=== PAUSE TestAccNeptuneClusterInstance_basic
=== RUN   TestAccNeptuneClusterInstance_disappears
=== PAUSE TestAccNeptuneClusterInstance_disappears
=== RUN   TestAccNeptuneClusterInstance_nameGenerated
=== PAUSE TestAccNeptuneClusterInstance_nameGenerated
=== RUN   TestAccNeptuneClusterInstance_namePrefix
=== PAUSE TestAccNeptuneClusterInstance_namePrefix
=== RUN   TestAccNeptuneClusterInstance_tags
=== PAUSE TestAccNeptuneClusterInstance_tags
=== RUN   TestAccNeptuneClusterInstance_withAZ
=== PAUSE TestAccNeptuneClusterInstance_withAZ
=== RUN   TestAccNeptuneClusterInstance_withSubnetGroup
=== PAUSE TestAccNeptuneClusterInstance_withSubnetGroup
=== RUN   TestAccNeptuneClusterInstance_kmsKey
=== PAUSE TestAccNeptuneClusterInstance_kmsKey
=== RUN   TestAccNeptuneClusterParameterGroup_basic
=== PAUSE TestAccNeptuneClusterParameterGroup_basic
=== RUN   TestAccNeptuneClusterParameterGroup_namePrefix
=== PAUSE TestAccNeptuneClusterParameterGroup_namePrefix
=== RUN   TestAccNeptuneClusterParameterGroup_generatedName
=== PAUSE TestAccNeptuneClusterParameterGroup_generatedName
=== RUN   TestAccNeptuneClusterParameterGroup_description
=== PAUSE TestAccNeptuneClusterParameterGroup_description
=== RUN   TestAccNeptuneClusterParameterGroup_NamePrefix_parameter
=== PAUSE TestAccNeptuneClusterParameterGroup_NamePrefix_parameter
=== RUN   TestAccNeptuneClusterParameterGroup_parameter
=== PAUSE TestAccNeptuneClusterParameterGroup_parameter
=== RUN   TestAccNeptuneClusterParameterGroup_parameterDefault
=== PAUSE TestAccNeptuneClusterParameterGroup_parameterDefault
=== RUN   TestAccNeptuneClusterParameterGroup_tags
=== PAUSE TestAccNeptuneClusterParameterGroup_tags
=== CONT  TestAccNeptuneClusterInstance_basic
=== CONT  TestAccNeptuneClusterParameterGroup_basic
=== CONT  TestAccNeptuneClusterParameterGroup_NamePrefix_parameter
--- PASS: TestAccNeptuneClusterParameterGroup_basic (26.84s)
=== CONT  TestAccNeptuneClusterInstance_tags
--- PASS: TestAccNeptuneClusterParameterGroup_NamePrefix_parameter (44.17s)
=== CONT  TestAccNeptuneClusterInstance_kmsKey
--- PASS: TestAccNeptuneClusterInstance_kmsKey (1611.66s)
=== CONT  TestAccNeptuneClusterInstance_withSubnetGroup
--- PASS: TestAccNeptuneClusterInstance_tags (1646.13s)
=== CONT  TestAccNeptuneClusterInstance_withAZ
--- PASS: TestAccNeptuneClusterInstance_basic (1816.14s)
=== CONT  TestAccNeptuneClusterParameterGroup_generatedName
--- PASS: TestAccNeptuneClusterParameterGroup_generatedName (27.37s)
=== CONT  TestAccNeptuneClusterParameterGroup_description
--- PASS: TestAccNeptuneClusterParameterGroup_description (24.11s)
=== CONT  TestAccNeptuneClusterInstance_nameGenerated
--- PASS: TestAccNeptuneClusterInstance_withSubnetGroup (1604.78s)
=== CONT  TestAccNeptuneClusterInstance_namePrefix
--- PASS: TestAccNeptuneClusterInstance_withAZ (1605.46s)
=== CONT  TestAccNeptuneClusterParameterGroup_parameterDefault
--- PASS: TestAccNeptuneClusterParameterGroup_parameterDefault (20.19s)
=== CONT  TestAccNeptuneClusterParameterGroup_tags
--- PASS: TestAccNeptuneClusterInstance_nameGenerated (1456.80s)
=== CONT  TestAccNeptuneClusterInstance_disappears
--- PASS: TestAccNeptuneClusterParameterGroup_tags (54.69s)
=== CONT  TestAccNeptuneClusterParameterGroup_parameter
--- PASS: TestAccNeptuneClusterParameterGroup_parameter (41.37s)
=== CONT  TestAccNeptuneClusterParameterGroup_namePrefix
--- PASS: TestAccNeptuneClusterParameterGroup_namePrefix (23.75s)
--- PASS: TestAccNeptuneClusterInstance_namePrefix (1730.71s)
--- PASS: TestAccNeptuneClusterInstance_disappears (2174.27s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/neptune	5504.026s

@ewbankkit
Copy link
Contributor

@triggan Thanks for the contribution 🎉 👏.

@ewbankkit ewbankkit merged commit 2ca7785 into hashicorp:main Oct 13, 2023
41 checks passed
@github-actions github-actions bot added this to the v5.22.0 milestone Oct 13, 2023
@github-actions
Copy link

This functionality has been released in v5.22.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!

Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. partner Contribution from a partner. service/neptune Issues and PRs that pertain to the neptune 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
3 participants