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_rds_cluster: convert to framework #29135

Closed
wants to merge 43 commits into from

Conversation

johnsonaj
Copy link
Contributor

@johnsonaj johnsonaj commented Jan 27, 2023

Description

convert aws_rds_cluster to Plugin Framework. This allows more control over delete operation.

Relations

Relates #2588

References

Output from Acceptance Testing

$ make testacc TESTARGS='-run=TestAccRDSCluster_' PKG=rds

==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/rds/... -v -count 1 -parallel 20 -run='TestAccRDSCluster_'  -timeout 180m
    cluster_test.go:1760: serverless does not support snapshot restore on an empty volume
--- SKIP: TestAccRDSCluster_SnapshotIdentifierEngineMode_serverless (0.00s)
--- PASS: TestAccRDSCluster_missingUserNameCausesError (13.92s)
--- PASS: TestAccRDSCluster_basic (144.70s)
--- PASS: TestAccRDSCluster_updateIAMRoles (162.56s)
--- PASS: TestAccRDSCluster_encrypted (182.90s)
--- PASS: TestAccRDSCluster_kmsKey (198.25s)
--- PASS: TestAccRDSCluster_port (201.96s)
--- PASS: TestAccRDSCluster_EngineMode_parallelQuery (202.83s)
--- PASS: TestAccRDSCluster_serverlessV2ScalingConfiguration (223.38s)
--- PASS: TestAccRDSCluster_networkType (234.56s)
--- PASS: TestAccRDSCluster_takeFinalSnapshot (255.20s)
--- PASS: TestAccRDSCluster_Scaling_defaultMinCapacity (295.13s)
--- PASS: TestAccRDSCluster_availabilityZones (155.20s)
--- PASS: TestAccRDSCluster_EnabledCloudWatchLogsExports_mySQL (311.99s)
--- PASS: TestAccRDSCluster_scaling (364.85s)
--- PASS: TestAccRDSCluster_EngineMode_global (143.57s)
--- PASS: TestAccRDSCluster_dbSubnetGroupName (206.05s)
--- PASS: TestAccRDSCluster_EngineMode_multiMaster (193.12s)
--- PASS: TestAccRDSCluster_backtrackWindow (204.79s)
--- PASS: TestAccRDSCluster_snapshotIdentifier (433.79s)
--- PASS: TestAccRDSCluster_PointInTimeRestore_enabledCloudWatchLogsExports (455.19s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_tags (464.45s)
--- PASS: TestAccRDSCluster_copyTagsToSnapshot (283.23s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_deletionProtection (508.47s)
--- PASS: TestAccRDSCluster_iamAuth (164.40s)
--- PASS: TestAccRDSCluster_deletionProtection (233.59s)
--- PASS: TestAccRDSCluster_SnapshotIdentifierEngineMode_parallelQuery (563.86s)
--- PASS: TestAccRDSCluster_backupsUpdate (203.97s)
--- PASS: TestAccRDSCluster_pointInTimeRestore (448.65s)
--- PASS: TestAccRDSCluster_MigrateFromPluginSDK (221.08s)
--- PASS: TestAccRDSCluster_enableHTTPEndpoint (328.65s)
--- PASS: TestAccRDSCluster_MigrateFromPluginSDK_scalingConfiguration (334.32s)
--- PASS: TestAccRDSCluster_engineMode (479.37s)
--- PASS: TestAccRDSCluster_SnapshotIdentifierVPCSecurityGroupIDs_tags (420.03s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_vpcSecurityGroupIDs (493.31s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_preferredMaintenanceWindow (427.96s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_masterPassword (535.99s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_masterUsername (455.52s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_preferredBackupWindow (506.08s)
--- PASS: TestAccRDSCluster_SnapshotIdentifierEngineVersion_equal (455.32s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_kmsKeyID (375.80s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifierEngineModeGlobal_remove (169.69s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifierEngineMode_provisioned (147.81s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifierEngineModeGlobal_update (153.60s)
--- PASS: TestAccRDSCluster_SnapshotIdentifierEngineVersion_different (388.62s)
--- PASS: TestAccRDSCluster_identifierGenerated (183.89s)
--- PASS: TestAccRDSCluster_MigrateFromPluginSDK_allowMajorVersionUpgrade (992.79s)
--- PASS: TestAccRDSCluster_onlyMajorVersion (932.32s)
--- PASS: TestAccRDSCluster_identifierPrefix (173.82s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifierEngineMode_global (147.59s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifierEngineModeGlobal_add (144.16s)
--- PASS: TestAccRDSCluster_SnapshotIdentifierEngineMode_provisioned (397.36s)
--- PASS: TestAccRDSCluster_tags (192.14s)
--- PASS: TestAccRDSCluster_disappears (161.74s)
--- PASS: TestAccRDSCluster_SnapshotIdentifier_encryptedRestore (405.00s)
--- PASS: TestAccRDSCluster_engineVersion (463.49s)
--- PASS: TestAccRDSCluster_dbClusterInstanceClass (1925.31s)
--- PASS: TestAccRDSCluster_allocatedStorage (1905.64s)
--- PASS: TestAccRDSCluster_iops (1978.67s)
--- PASS: TestAccRDSCluster_storageType (1923.85s)
--- PASS: TestAccRDSCluster_EnabledCloudWatchLogsExports_postgresql (2313.76s)
--- PASS: TestAccRDSCluster_ReplicationSourceIdentifier_kmsKeyID (2022.37s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifier_replicationSourceIdentifier (2478.88s)
--- PASS: TestAccRDSCluster_allowMajorVersionUpgrade (2151.89s)
--- PASS: TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParameters (2415.95s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifier_secondaryClustersWriteForwarding (3008.53s)
--- PASS: TestAccRDSCluster_engineVersionWithPrimaryInstance (1580.59s)
--- PASS: TestAccRDSCluster_allowMajorVersionUpgradeWithCustomParametersApplyImm (2479.89s)
--- PASS: TestAccRDSCluster_GlobalClusterIdentifier_primarySecondaryClusters (2762.85s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/rds	3963.207s

@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/XL Managed by automation to categorize the size of a PR. flex Pertains to FLatteners and EXpanders. service/rds Issues and PRs that pertain to the rds service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Jan 27, 2023
@johnsonaj johnsonaj force-pushed the td-aws_rds_cluster_framework branch from 6bd1e98 to 2e3e665 Compare January 28, 2023 18:18
@github-actions github-actions bot added provider Pertains to the provider itself, rather than any interaction with AWS. sweeper Pertains to changes to or issues with the sweeper. and removed provider Pertains to the provider itself, rather than any interaction with AWS. labels Jan 28, 2023
@johnsonaj johnsonaj force-pushed the td-aws_rds_cluster_framework branch from 2dad1db to 99aafe4 Compare February 8, 2023 17:12
@johnsonaj johnsonaj marked this pull request as ready for review February 9, 2023 14:16
Copy link
Contributor

@gdavison gdavison left a comment

Choose a reason for hiding this comment

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

Looks good. I have a few suggestions for acceptance tests and validators

internal/framework/validators/arn.go Outdated Show resolved Hide resolved
internal/framework/validators/arn.go Outdated Show resolved Hide resolved
internal/framework/validators/timestamp.go Outdated Show resolved Hide resolved
internal/framework/validators/timestamp.go Outdated Show resolved Hide resolved
internal/service/rds/cluster_test.go Show resolved Hide resolved
internal/service/rds/cluster_test.go Show resolved Hide resolved
internal/service/rds/cluster_test.go Outdated Show resolved Hide resolved
internal/service/rds/cluster_fw.go Show resolved Hide resolved
Copy link
Member

@jar-b jar-b left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just one thought on testing.

internal/sweep/framework.go Outdated Show resolved Hide resolved
@@ -2193,6 +2202,40 @@ func TestAccRDSCluster_enableHTTPEndpoint(t *testing.T) {
})
}

func TestAccRDSCluster_MigrateFromPluginSDK(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Since this resource is so large, would it be worth writing a few different migration tests to cover the most common configurations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea! We will definitely have to accept some diffs but we can definitely write some test to cover more basic configurations. Might be worth pointing out expected diffs in the changelog as well

@johnsonaj johnsonaj force-pushed the td-aws_rds_cluster_framework branch from db391f9 to 2bd7d00 Compare February 15, 2023 18:17
@github-actions github-actions bot added the verify Pertains to the verify package (i.e., provider-level validating, diff suppression, etc.) label Feb 15, 2023
@johnsonaj johnsonaj force-pushed the td-aws_rds_cluster_framework branch from b6b1a46 to 429a300 Compare March 16, 2023 20:41
@github-actions github-actions bot added generators Relates to code generators. provider Pertains to the provider itself, rather than any interaction with AWS. and removed provider Pertains to the provider itself, rather than any interaction with AWS. labels Mar 16, 2023
@johnsonaj johnsonaj force-pushed the td-aws_rds_cluster_framework branch 2 times, most recently from 87984b6 to dccfa2d Compare March 24, 2023 14:24
@johnsonaj johnsonaj force-pushed the td-aws_rds_cluster_framework branch from dccfa2d to 9f24bdf Compare April 12, 2023 16:29
@github-actions github-actions bot removed flex Pertains to FLatteners and EXpanders. provider Pertains to the provider itself, rather than any interaction with AWS. verify Pertains to the verify package (i.e., provider-level validating, diff suppression, etc.) labels Apr 12, 2023
@johnsonaj johnsonaj marked this pull request as draft April 12, 2023 16:31
@johnsonaj johnsonaj closed this Nov 28, 2023
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 Dec 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
generators Relates to code generators. service/rds Issues and PRs that pertain to the rds service. size/XL Managed by automation to categorize the size of a PR. sweeper Pertains to changes to or issues with the sweeper. 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.

3 participants