From 665ad7ae35107008b4f3e67aac378b1a54197862 Mon Sep 17 00:00:00 2001 From: Dirk Avery <dirk.avery@gmail.com> Date: Wed, 10 May 2023 10:43:22 -0400 Subject: [PATCH] Fix additional tests --- internal/service/rds/instance.go | 92 +++++++++++++++++++++------ internal/service/rds/instance_test.go | 44 ++++++------- 2 files changed, 94 insertions(+), 42 deletions(-) diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index ce656efc171..a94cf4f314b 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -624,6 +624,18 @@ func ResourceInstance() *schema.Resource { } return nil }, + customdiff.ComputedIf("address", func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + return diff.HasChange("identifier") + }), + customdiff.ComputedIf("arn", func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + return diff.HasChange("identifier") + }), + customdiff.ComputedIf("endpoint", func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + return diff.HasChange("identifier") + }), + customdiff.ComputedIf("tags_all", func(ctx context.Context, diff *schema.ResourceDiff, meta interface{}) bool { + return diff.HasChange("identifier") + }), ), } } @@ -999,9 +1011,10 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in return sdkdiag.AppendErrorf(diags, "creating RDS DB Instance (restore from S3) (%s): %s", identifier, err) } - output := outputRaw.(*rds.RestoreDBInstanceFromS3Output) - - resourceID = aws.StringValue(output.DBInstance.DbiResourceId) + if outputRaw != nil { + output := outputRaw.(*rds.RestoreDBInstanceFromS3Output) + resourceID = aws.StringValue(output.DBInstance.DbiResourceId) + } } else if v, ok := d.GetOk("snapshot_identifier"); ok { input := &rds.RestoreDBInstanceFromDBSnapshotInput{ AutoMinorVersionUpgrade: aws.Bool(d.Get("auto_minor_version_upgrade").(bool)), @@ -1197,10 +1210,6 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in }, ) - output := outputRaw.(*rds.RestoreDBInstanceFromDBSnapshotOutput) - - resourceID = aws.StringValue(output.DBInstance.DbiResourceId) - // When using SQL Server engine with MultiAZ enabled, its not // possible to immediately enable mirroring since // BackupRetentionPeriod is not available as a parameter to @@ -1219,6 +1228,11 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in if err != nil { return sdkdiag.AppendErrorf(diags, "creating RDS DB Instance (restore from snapshot) (%s): %s", identifier, err) } + + if outputRaw != nil { + output := outputRaw.(*rds.RestoreDBInstanceFromDBSnapshotOutput) + resourceID = aws.StringValue(output.DBInstance.DbiResourceId) + } } else if v, ok := d.GetOk("restore_to_point_in_time"); ok { tfMap := v.([]interface{})[0].(map[string]interface{}) @@ -1364,9 +1378,10 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in return sdkdiag.AppendErrorf(diags, "creating RDS DB Instance (restore to point-in-time) (%s): %s", identifier, err) } - output := outputRaw.(*rds.RestoreDBInstanceToPointInTimeOutput) - - resourceID = aws.StringValue(output.DBInstance.DbiResourceId) + if outputRaw != nil { + output := outputRaw.(*rds.RestoreDBInstanceToPointInTimeOutput) + resourceID = aws.StringValue(output.DBInstance.DbiResourceId) + } } else { if _, ok := d.GetOk("allocated_storage"); !ok { diags = sdkdiag.AppendErrorf(diags, `"allocated_storage": required field is not set`) @@ -1550,7 +1565,6 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in } output := outputRaw.(*rds.CreateDBInstanceOutput) - resourceID = aws.StringValue(output.DBInstance.DbiResourceId) // This is added here to avoid unnecessary modification when ca_cert_identifier is the default one @@ -1560,12 +1574,18 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in } } - d.SetId(resourceID) - - if _, err := waitDBInstanceAvailableSDKv1(ctx, conn, d.Id(), d.Timeout(schema.TimeoutCreate)); err != nil { + var instance *rds.DBInstance + var err error + if instance, err = waitDBInstanceAvailableSDKv1(ctx, conn, d.Get("identifier").(string), d.Timeout(schema.TimeoutCreate)); err != nil { return sdkdiag.AppendErrorf(diags, "waiting for RDS DB Instance (%s) create: %s", identifier, err) } + if resourceID == "" { + resourceID = aws.StringValue(instance.DbiResourceId) + } + + d.SetId(resourceID) + if requiresModifyDbInstance { modifyDbInstanceInput.DBInstanceIdentifier = aws.String(identifier) @@ -2320,17 +2340,33 @@ func parseDBInstanceARN(s string) (dbInstanceARN, error) { return result, nil } +// findDBInstanceByIDSDKv1 in general should be called with a DbiResourceId of the form +// "db-BE6UI2KLPQP3OVDYD74ZEV6NUM" rather than a DB identifier. However, in some cases only +// the identifier is available, and can be used. func findDBInstanceByIDSDKv1(ctx context.Context, conn *rds.RDS, id string) (*rds.DBInstance, error) { - input := &rds.DescribeDBInstancesInput{ - Filters: []*rds.Filter{ + input := &rds.DescribeDBInstancesInput{} + + if regexp.MustCompile(`^db-[a-zA-Z0-9]{2,255}$`).MatchString(id) { + input.Filters = []*rds.Filter{ { Name: aws.String("dbi-resource-id"), Values: aws.StringSlice([]string{id}), }, - }, + } + } else { + input.DBInstanceIdentifier = aws.String(id) } output, err := conn.DescribeDBInstancesWithContext(ctx, input) + + // in case a DB has an *identifier* starting with "db-"" + if regexp.MustCompile(`^db-[a-zA-Z0-9]{2,255}$`).MatchString(id) && (output == nil || len(output.DBInstances) == 0) { + input = &rds.DescribeDBInstancesInput{ + DBInstanceIdentifier: aws.String(id), + } + output, err = conn.DescribeDBInstancesWithContext(ctx, input) + } + if tfawserr.ErrCodeEquals(err, rds.ErrCodeDBInstanceNotFoundFault) { return nil, &retry.NotFoundError{ LastError: err, @@ -2350,17 +2386,33 @@ func findDBInstanceByIDSDKv1(ctx context.Context, conn *rds.RDS, id string) (*rd return dbInstance, nil } +// findDBInstanceByIDSDKv2 in general should be called with a DbiResourceId of the form +// "db-BE6UI2KLPQP3OVDYD74ZEV6NUM" rather than a DB identifier. However, in some cases only +// the identifier is available, and can be used. func findDBInstanceByIDSDKv2(ctx context.Context, conn *rds_sdkv2.Client, id string) (*types.DBInstance, error) { - input := &rds_sdkv2.DescribeDBInstancesInput{ - Filters: []types.Filter{ + input := &rds_sdkv2.DescribeDBInstancesInput{} + + if regexp.MustCompile(`^db-[a-zA-Z0-9]{2,255}$`).MatchString(id) { + input.Filters = []types.Filter{ { Name: aws.String("dbi-resource-id"), Values: []string{id}, }, - }, + } + } else { + input.DBInstanceIdentifier = aws.String(id) } output, err := conn.DescribeDBInstances(ctx, input) + + // in case a DB has an *identifier* starting with "db-"" + if regexp.MustCompile(`^db-[a-zA-Z0-9]{2,255}$`).MatchString(id) && (output == nil || len(output.DBInstances) == 0) { + input = &rds_sdkv2.DescribeDBInstancesInput{ + DBInstanceIdentifier: aws.String(id), + } + output, err = conn.DescribeDBInstances(ctx, input) + } + if errs.IsA[*types.DBInstanceNotFoundFault](err) { return nil, &retry.NotFoundError{ LastError: err, diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index ac60ea1be7d..1be352d3ac2 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -636,7 +636,7 @@ func TestAccRDSInstance_dbSubnetGroupName(t *testing.T) { Config: testAccInstanceConfig_dbSubnetGroupName(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(ctx, resourceName, &dbInstance), - testAccCheckSubnetGroupExists(ctx, resourceName, &dbSubnetGroup), + testAccCheckSubnetGroupExists(ctx, dbSubnetGroupResourceName, &dbSubnetGroup), resource.TestCheckResourceAttrPair(resourceName, "db_subnet_group_name", dbSubnetGroupResourceName, "name"), ), }, @@ -698,7 +698,7 @@ func TestAccRDSInstance_DBSubnetGroupName_vpcSecurityGroupIDs(t *testing.T) { Config: testAccInstanceConfig_DBSubnetGroupName_vpcSecurityGroupIDs(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(ctx, resourceName, &dbInstance), - testAccCheckSubnetGroupExists(ctx, resourceName, &dbSubnetGroup), + testAccCheckSubnetGroupExists(ctx, dbSubnetGroupResourceName, &dbSubnetGroup), resource.TestCheckResourceAttrPair(resourceName, "db_subnet_group_name", dbSubnetGroupResourceName, "name"), ), }, @@ -1530,7 +1530,7 @@ func TestAccRDSInstance_ReplicateSourceDB_dbSubnetGroupName(t *testing.T) { Config: testAccInstanceConfig_ReplicateSourceDB_dbSubnetGroupName(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(ctx, resourceName, &dbInstance), - testAccCheckSubnetGroupExists(ctx, resourceName, &dbSubnetGroup), + testAccCheckSubnetGroupExists(ctx, dbSubnetGroupResourceName, &dbSubnetGroup), resource.TestCheckResourceAttrPair(resourceName, "db_subnet_group_name", dbSubnetGroupResourceName, "name"), ), }, @@ -1600,7 +1600,7 @@ func TestAccRDSInstance_ReplicateSourceDBDBSubnetGroupName_vpcSecurityGroupIDs(t Config: testAccInstanceConfig_ReplicateSourceDB_DBSubnetGroupName_vpcSecurityGroupIDs(rName), Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(ctx, resourceName, &dbInstance), - testAccCheckSubnetGroupExists(ctx, resourceName, &dbSubnetGroup), + testAccCheckSubnetGroupExists(ctx, dbSubnetGroupResourceName, &dbSubnetGroup), resource.TestCheckResourceAttrPair(resourceName, "db_subnet_group_name", dbSubnetGroupResourceName, "name"), ), }, @@ -2700,7 +2700,7 @@ func TestAccRDSInstance_SnapshotIdentifier_dbSubnetGroupName(t *testing.T) { testAccCheckInstanceExists(ctx, sourceDbResourceName, &sourceDbInstance), testAccCheckDBSnapshotExists(ctx, snapshotResourceName, &dbSnapshot), testAccCheckInstanceExists(ctx, resourceName, &dbInstance), - testAccCheckSubnetGroupExists(ctx, resourceName, &dbSubnetGroup), + testAccCheckSubnetGroupExists(ctx, dbSubnetGroupResourceName, &dbSubnetGroup), resource.TestCheckResourceAttrPair(resourceName, "db_subnet_group_name", dbSubnetGroupResourceName, "name"), ), }, @@ -2776,7 +2776,7 @@ func TestAccRDSInstance_SnapshotIdentifier_dbSubnetGroupNameVPCSecurityGroupIDs( testAccCheckInstanceExists(ctx, sourceDbResourceName, &sourceDbInstance), testAccCheckDBSnapshotExists(ctx, snapshotResourceName, &dbSnapshot), testAccCheckInstanceExists(ctx, resourceName, &dbInstance), - testAccCheckSubnetGroupExists(ctx, resourceName, &dbSubnetGroup), + testAccCheckSubnetGroupExists(ctx, dbSubnetGroupResourceName, &dbSubnetGroup), resource.TestCheckResourceAttrPair(resourceName, "db_subnet_group_name", dbSubnetGroupResourceName, "name"), ), }, @@ -3653,8 +3653,8 @@ func TestAccRDSInstance_MySQL_snapshotRestoreWithEngineVersion(t *testing.T) { Check: resource.ComposeAggregateTestCheckFunc( testAccCheckInstanceExists(ctx, restoreResourceName, &vRestoredInstance), testAccCheckInstanceExists(ctx, resourceName, &v), - // Hardcoded older version. Will to update when no longer compatible to upgrade from this to the default version. - resource.TestCheckResourceAttr(resourceName, "engine_version", "8.0.25"), + // Hardcoded older version. Will need to update when no longer compatible to upgrade from this to the default version. + resource.TestCheckResourceAttr(resourceName, "engine_version", "8.0.31"), resource.TestCheckResourceAttrPair(restoreResourceName, "engine_version", "data.aws_rds_engine_version.default", "version"), ), }, @@ -5404,17 +5404,17 @@ func testAccCheckInstanceAutomatedBackupsDelete(ctx context.Context) resource.Te log.Printf("[INFO] Trying to locate the DBInstance Automated Backup") describeOutput, err := conn.DescribeDBInstanceAutomatedBackupsWithContext(ctx, &rds.DescribeDBInstanceAutomatedBackupsInput{ - DBInstanceIdentifier: aws.String(rs.Primary.ID), + DBInstanceIdentifier: aws.String(rs.Primary.Attributes["identifier"]), }) if err != nil { return err } if describeOutput == nil || len(describeOutput.DBInstanceAutomatedBackups) == 0 { - return fmt.Errorf("Automated backup for %s not found", rs.Primary.ID) + return fmt.Errorf("Automated backup for %s not found", rs.Primary.Attributes["identifier"]) } - log.Printf("[INFO] Deleting automated backup for %s", rs.Primary.ID) + log.Printf("[INFO] Deleting automated backup for %s", rs.Primary.Attributes["identifier"]) _, err = conn.DeleteDBInstanceAutomatedBackupWithContext(ctx, &rds.DeleteDBInstanceAutomatedBackupInput{ DbiResourceId: describeOutput.DBInstanceAutomatedBackups[0].DbiResourceId, }) @@ -5436,7 +5436,7 @@ func testAccCheckInstanceDestroy(ctx context.Context) resource.TestCheckFunc { continue } - _, err := tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.ID) + _, err := tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.Attributes["identifier"]) if tfresource.NotFound(err) { continue @@ -5446,7 +5446,7 @@ func testAccCheckInstanceDestroy(ctx context.Context) resource.TestCheckFunc { return err } - return fmt.Errorf("RDS DB Instance %s still exists", rs.Primary.ID) + return fmt.Errorf("RDS DB Instance %s still exists", rs.Primary.Attributes["identifier"]) } return nil @@ -5566,7 +5566,7 @@ func testAccCheckInstanceDestroyWithFinalSnapshot(ctx context.Context) resource. return err } - _, err = tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.ID) + _, err = tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.Attributes["identifier"]) if tfresource.NotFound(err) { continue @@ -5576,7 +5576,7 @@ func testAccCheckInstanceDestroyWithFinalSnapshot(ctx context.Context) resource. return err } - return fmt.Errorf("RDS DB Instance %s still exists", rs.Primary.ID) + return fmt.Errorf("RDS DB Instance %s still exists", rs.Primary.Attributes["identifier"]) } return nil @@ -5606,7 +5606,7 @@ func testAccCheckInstanceDestroyWithoutFinalSnapshot(ctx context.Context) resour return fmt.Errorf("RDS DB Snapshot %s exists", finalSnapshotID) } - _, err = tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.ID) + _, err = tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.Attributes["identifier"]) if tfresource.NotFound(err) { continue @@ -5616,7 +5616,7 @@ func testAccCheckInstanceDestroyWithoutFinalSnapshot(ctx context.Context) resour return err } - return fmt.Errorf("RDS DB Instance %s still exists", rs.Primary.ID) + return fmt.Errorf("RDS DB Instance %s still exists", rs.Primary.Attributes["identifier"]) } return nil @@ -5656,13 +5656,13 @@ func testAccCheckInstanceExists(ctx context.Context, n string, v *rds.DBInstance return fmt.Errorf("Not found: %s", n) } - if rs.Primary.ID == "" { + if rs.Primary.Attributes["identifier"] == "" { return fmt.Errorf("No RDS DB Instance ID is set") } conn := acctest.Provider.Meta().(*conns.AWSClient).RDSConn() - output, err := tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.ID) + output, err := tfrds.FindDBInstanceByID(ctx, conn, rs.Primary.Attributes["identifier"]) if err != nil { return err } @@ -6910,7 +6910,7 @@ func testAccInstanceConfig_mySQLSnapshotRestoreEngineVersion(rName string) strin resource "aws_db_instance" "test" { allocated_storage = 20 engine = data.aws_rds_engine_version.default.engine - engine_version = "8.0.25" # test is from older to newer version, update when restore from this to current default version is incompatible + engine_version = "8.0.31" # test is from older to newer version, update when restore from this to current default version is incompatible identifier = %[1]q instance_class = data.aws_rds_orderable_db_instance.test.instance_class skip_final_snapshot = true @@ -8894,7 +8894,7 @@ func testAccInstanceConfig_SnapshotID_io1Storage(rName string, iops int) string return fmt.Sprintf(` data "aws_rds_orderable_db_instance" "test" { engine = "mariadb" - engine_version = "10.5.12" + engine_version = "10.6.12" license_model = "general-public-license" storage_type = "io1" @@ -10220,7 +10220,7 @@ data "aws_rds_orderable_db_instance" "test" { data "aws_rds_engine_version" "initial" { engine = "mysql" - preferred_versions = ["8.0.27", "8.0.26", "8.0.25"] + preferred_versions = ["8.0.32", "8.0.31", "8.0.30"] } data "aws_rds_engine_version" "updated" {