Skip to content

Commit

Permalink
Fix additional tests
Browse files Browse the repository at this point in the history
  • Loading branch information
YakDriver committed May 10, 2023
1 parent 4c756c2 commit 665ad7a
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 42 deletions.
92 changes: 72 additions & 20 deletions internal/service/rds/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}),
),
}
}
Expand Down Expand Up @@ -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)),
Expand Down Expand Up @@ -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
Expand All @@ -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{})

Expand Down Expand Up @@ -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`)
Expand Down Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
44 changes: 22 additions & 22 deletions internal/service/rds/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
),
},
Expand Down Expand Up @@ -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"),
),
},
Expand Down Expand Up @@ -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"),
),
},
Expand Down Expand Up @@ -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"),
),
},
Expand Down Expand Up @@ -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"),
),
},
Expand Down Expand Up @@ -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"),
),
},
Expand Down Expand Up @@ -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"),
),
},
Expand Down Expand Up @@ -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,
})
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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" {
Expand Down

0 comments on commit 665ad7a

Please sign in to comment.