Skip to content

Commit

Permalink
r/db_instance: deprecate name and add db_name (#22668)
Browse files Browse the repository at this point in the history 
* r/db_instance: deprecate 'name' and add 'db_name'

* Update CHANGELOG for #22668

* add addition conflicting arg; add deprecation message; add to docs

* r/aws_rds_instance: Add acceptance tests for new 'db_name' argument.

* Add changelog

* Revert inadvertent change

Co-authored-by: Kit Ewbank <[email protected]>
Co-authored-by: Dirk Avery <[email protected]>
  • Loading branch information
@anGie44 @ewbankkit @YakDriver
3 people authored Feb 7, 2022
1 parent c2903b2 commit 7875c87
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 21 deletions.
7 changes: 7 additions & 0 deletions .changelog/22668.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:note
resource/aws_db_instance The `name` argument has been deprecated. All configurations using `name` should be updated to use the `db_name` argument instead
```

```release-note:enhancement
resource/aws_db_instance: Add `db_name` argument
```
74 changes: 56 additions & 18 deletions internal/service/rds/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,16 @@ func ResourceInstance() *schema.Resource {
Type: schema.TypeBool,
Optional: true,
},
"db_name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{
"name",
"replicate_source_db",
},
},
"db_subnet_group_name": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -290,11 +300,15 @@ func ResourceInstance() *schema.Resource {
Computed: true,
},
"name": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"replicate_source_db"},
Type: schema.TypeString,
Optional: true,
Computed: true,
Deprecated: "Use db_name instead",
ForceNew: true,
ConflictsWith: []string{
"db_name",
"replicate_source_db",
},
},
"nchar_character_set_name": {
Type: schema.TypeString,
Expand Down Expand Up @@ -693,26 +707,31 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error {
}
}
} else if v, ok := d.GetOk("s3_import"); ok {
dbName := d.Get("db_name").(string)
if dbName == "" {
dbName = d.Get("name").(string)
}

if _, ok := d.GetOk("allocated_storage"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "allocated_storage": required field is not set`, d.Get("name").(string))

return fmt.Errorf(`provider.aws: aws_db_instance: %s: "allocated_storage": required field is not set`, dbName)
}
if _, ok := d.GetOk("engine"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "engine": required field is not set`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "engine": required field is not set`, dbName)
}
if _, ok := d.GetOk("password"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "password": required field is not set`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "password": required field is not set`, dbName)
}
if _, ok := d.GetOk("username"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "username": required field is not set`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "username": required field is not set`, dbName)
}

s3_bucket := v.([]interface{})[0].(map[string]interface{})
opts := rds.RestoreDBInstanceFromS3Input{
AllocatedStorage: aws.Int64(int64(d.Get("allocated_storage").(int))),
AutoMinorVersionUpgrade: aws.Bool(d.Get("auto_minor_version_upgrade").(bool)),
CopyTagsToSnapshot: aws.Bool(d.Get("copy_tags_to_snapshot").(bool)),
DBName: aws.String(d.Get("name").(string)),
DBName: aws.String(dbName),
DBInstanceClass: aws.String(d.Get("instance_class").(string)),
DBInstanceIdentifier: aws.String(identifier),
DeletionProtection: aws.Bool(d.Get("deletion_protection").(bool)),
Expand All @@ -735,10 +754,10 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error {
}

if _, ok := d.GetOk("character_set_name"); ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "character_set_name" doesn't work with with restores"`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "character_set_name" doesn't work with with restores"`, dbName)
}
if _, ok := d.GetOk("timezone"); ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "timezone" doesn't work with with restores"`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "timezone" doesn't work with with restores"`, dbName)
}

attr := d.Get("backup_retention_period")
Expand Down Expand Up @@ -882,7 +901,16 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error {
Tags: Tags(tags.IgnoreAWS()),
}

if attr, ok := d.GetOk("name"); ok {
if attr, ok := d.GetOk("db_name"); ok {
// "Note: This parameter [DBName] doesn't apply to the MySQL, PostgreSQL, or MariaDB engines."
// https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_RestoreDBInstanceFromDBSnapshot.html
switch strings.ToLower(d.Get("engine").(string)) {
case "mysql", "postgres", "mariadb":
// skip
default:
opts.DBName = aws.String(attr.(string))
}
} else if attr, ok := d.GetOk("name"); ok {
// "Note: This parameter [DBName] doesn't apply to the MySQL, PostgreSQL, or MariaDB engines."
// https://docs.aws.amazon.com/AmazonRDS/latest/APIReference/API_RestoreDBInstanceFromDBSnapshot.html
switch strings.ToLower(d.Get("engine").(string)) {
Expand Down Expand Up @@ -1078,6 +1106,10 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error {
input.AvailabilityZone = aws.String(v.(string))
}

if v, ok := d.GetOk("db_name"); ok {
input.DBName = aws.String(v.(string))
}

if v, ok := d.GetOk("domain"); ok {
input.Domain = aws.String(v.(string))
}
Expand Down Expand Up @@ -1158,22 +1190,27 @@ func resourceInstanceCreate(d *schema.ResourceData, meta interface{}) error {
}
}
} else {
dbName := d.Get("db_name").(string)
if dbName == "" {
dbName = d.Get("name").(string)
}

if _, ok := d.GetOk("allocated_storage"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "allocated_storage": required field is not set`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "allocated_storage": required field is not set`, dbName)
}
if _, ok := d.GetOk("engine"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "engine": required field is not set`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "engine": required field is not set`, dbName)
}
if _, ok := d.GetOk("password"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "password": required field is not set`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "password": required field is not set`, dbName)
}
if _, ok := d.GetOk("username"); !ok {
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "username": required field is not set`, d.Get("name").(string))
return fmt.Errorf(`provider.aws: aws_db_instance: %s: "username": required field is not set`, dbName)
}

opts := rds.CreateDBInstanceInput{
AllocatedStorage: aws.Int64(int64(d.Get("allocated_storage").(int))),
DBName: aws.String(d.Get("name").(string)),
DBName: aws.String(dbName),
DBInstanceClass: aws.String(d.Get("instance_class").(string)),
DBInstanceIdentifier: aws.String(identifier),
DeletionProtection: aws.Bool(d.Get("deletion_protection").(bool)),
Expand Down Expand Up @@ -1404,6 +1441,7 @@ func resourceInstanceRead(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error reading DB Instance (%s): %w", d.Id(), err)
}

d.Set("db_name", v.DBName)
d.Set("name", v.DBName)
d.Set("identifier", v.DBInstanceIdentifier)
d.Set("identifier_prefix", create.NamePrefixFromName(aws.StringValue(v.DBInstanceIdentifier)))
Expand Down
168 changes: 166 additions & 2 deletions internal/service/rds/instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,76 @@ func TestAccRDSInstance_basic(t *testing.T) {
resource.TestCheckResourceAttrSet(resourceName, "backup_window"),
resource.TestCheckResourceAttrSet(resourceName, "ca_cert_identifier"),
resource.TestCheckResourceAttr(resourceName, "copy_tags_to_snapshot", "false"),
resource.TestCheckResourceAttr(resourceName, "db_name", "baz"),
resource.TestCheckResourceAttr(resourceName, "db_subnet_group_name", "default"),
resource.TestCheckResourceAttr(resourceName, "deletion_protection", "false"),
resource.TestCheckResourceAttr(resourceName, "enabled_cloudwatch_logs_exports.#", "0"),
resource.TestCheckResourceAttrSet(resourceName, "endpoint"),
resource.TestCheckResourceAttr(resourceName, "engine", "mysql"),
resource.TestCheckResourceAttrSet(resourceName, "engine_version"),
resource.TestCheckResourceAttrSet(resourceName, "hosted_zone_id"),
resource.TestCheckResourceAttr(resourceName, "iam_database_authentication_enabled", "false"),
resource.TestCheckResourceAttrPair(resourceName, "instance_class", "data.aws_rds_orderable_db_instance.test", "instance_class"),
resource.TestCheckResourceAttr(resourceName, "license_model", "general-public-license"),
resource.TestCheckResourceAttrSet(resourceName, "maintenance_window"),
resource.TestCheckResourceAttr(resourceName, "max_allocated_storage", "0"),
resource.TestCheckResourceAttr(resourceName, "option_group_name", "default:mysql-5-6"),
resource.TestCheckResourceAttr(resourceName, "parameter_group_name", "default.mysql5.6"),
resource.TestCheckResourceAttr(resourceName, "port", "3306"),
resource.TestCheckResourceAttr(resourceName, "publicly_accessible", "false"),
resource.TestCheckResourceAttrSet(resourceName, "resource_id"),
resource.TestCheckResourceAttr(resourceName, "status", "available"),
resource.TestCheckResourceAttr(resourceName, "storage_encrypted", "false"),
resource.TestCheckResourceAttr(resourceName, "storage_type", "gp2"),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
resource.TestCheckResourceAttr(resourceName, "username", "test"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"apply_immediately",
"final_snapshot_identifier",
"password",
"skip_final_snapshot",
"delete_automated_backups",
},
},
},
})
}

func TestAccRDSInstance_NameDeprecated_basic(t *testing.T) {
var dbInstance1 rds.DBInstance

rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_db_instance.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceNameDeprecatedBasicConfig(rName),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInstanceExists(resourceName, &dbInstance1),
testAccCheckInstanceAttributes(&dbInstance1),
resource.TestCheckResourceAttr(resourceName, "identifier", rName),
resource.TestCheckResourceAttr(resourceName, "identifier_prefix", ""),
resource.TestCheckResourceAttr(resourceName, "allocated_storage", "10"),
resource.TestCheckNoResourceAttr(resourceName, "allow_major_version_upgrade"),
resource.TestCheckResourceAttr(resourceName, "auto_minor_version_upgrade", "true"),
acctest.MatchResourceAttrRegionalARN(resourceName, "arn", "rds", regexp.MustCompile(`db:.+`)),
resource.TestCheckResourceAttrSet(resourceName, "availability_zone"),
resource.TestCheckResourceAttr(resourceName, "backup_retention_period", "0"),
resource.TestCheckResourceAttrSet(resourceName, "backup_window"),
resource.TestCheckResourceAttrSet(resourceName, "ca_cert_identifier"),
resource.TestCheckResourceAttr(resourceName, "copy_tags_to_snapshot", "false"),
resource.TestCheckResourceAttr(resourceName, "db_name", "baz"),
resource.TestCheckResourceAttr(resourceName, "db_subnet_group_name", "default"),
resource.TestCheckResourceAttr(resourceName, "deletion_protection", "false"),
resource.TestCheckResourceAttr(resourceName, "enabled_cloudwatch_logs_exports.#", "0"),
Expand Down Expand Up @@ -1533,6 +1603,40 @@ func TestAccRDSInstance_s3Import_basic(t *testing.T) {
})
}

func TestAccRDSInstance_s3Import_NameDeprecated_basic(t *testing.T) {
var snap rds.DBInstance
bucket := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
uniqueId := sdkacctest.RandomWithPrefix("tf-acc-s3-import-test")
bucketPrefix := sdkacctest.RandString(5)

const resourceName = "aws_db_instance.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(t) },
ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID),
Providers: acctest.Providers,
CheckDestroy: testAccCheckInstanceDestroy,
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_S3Import_NameDeprecated_basic(bucket, bucketPrefix, uniqueId),
Check: resource.ComposeTestCheckFunc(
testAccCheckInstanceExists(resourceName, &snap),
resource.TestCheckResourceAttr(resourceName, "identifier", uniqueId),
resource.TestCheckResourceAttr(resourceName, "identifier_prefix", ""),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{
"password",
},
},
},
})
}

func TestAccRDSInstance_s3Import_namePrefix(t *testing.T) {
var snap rds.DBInstance
const identifierPrefix = "tf-acc-test-prefix-"
Expand Down Expand Up @@ -4028,7 +4132,32 @@ resource "aws_db_instance" "test" {
engine = data.aws_rds_orderable_db_instance.test.engine
engine_version = data.aws_rds_orderable_db_instance.test.engine_version
instance_class = data.aws_rds_orderable_db_instance.test.instance_class
name = "baz"
db_name = "baz"
parameter_group_name = "default.mysql5.6"
password = "barbarbarbar"
skip_final_snapshot = true
username = "test"
# Maintenance Window is stored in lower case in the API, though not strictly
# documented. Terraform will downcase this to match (as opposed to throw a
# validation error).
maintenance_window = "Fri:09:00-Fri:09:30"
}
`, rName))
}

func testAccInstanceNameDeprecatedBasicConfig(rName string) string {
return acctest.ConfigCompose(
testAccInstanceConfig_orderableClassMySQL(),
fmt.Sprintf(`
resource "aws_db_instance" "test" {
identifier = %[1]q
allocated_storage = 10
backup_retention_period = 0
engine = data.aws_rds_orderable_db_instance.test.engine
engine_version = data.aws_rds_orderable_db_instance.test.engine_version
instance_class = data.aws_rds_orderable_db_instance.test.instance_class
name = "baz" # deprecated
parameter_group_name = "default.mysql5.6"
password = "barbarbarbar"
skip_final_snapshot = true
Expand Down Expand Up @@ -4363,7 +4492,42 @@ resource "aws_db_instance" "test" {
engine_version = "5.6"
auto_minor_version_upgrade = true
instance_class = data.aws_rds_orderable_db_instance.test.instance_class
name = "baz"
db_name = "baz"
password = "barbarbarbar"
publicly_accessible = false
username = "foo"
backup_retention_period = 0
parameter_group_name = "default.mysql5.6"
skip_final_snapshot = true
multi_az = false
db_subnet_group_name = aws_db_subnet_group.foo.id
s3_import {
source_engine = data.aws_rds_orderable_db_instance.test.engine
source_engine_version = "5.6"
bucket_name = aws_s3_bucket.xtrabackup.bucket
bucket_prefix = %[2]q
ingestion_role = aws_iam_role.rds_s3_access_role.arn
}
}
`, uniqueId, bucketPrefix))
}

func testAccInstanceConfig_S3Import_NameDeprecated_basic(bucketName, bucketPrefix, uniqueId string) string {
return acctest.ConfigCompose(
testAccInstanceConfig_S3Import_Base(bucketName, bucketPrefix),
fmt.Sprintf(`
resource "aws_db_instance" "test" {
identifier = "%[1]s-db"
allocated_storage = 5
engine = data.aws_rds_orderable_db_instance.test.engine
engine_version = "5.6"
auto_minor_version_upgrade = true
instance_class = data.aws_rds_orderable_db_instance.test.instance_class
name = "baz" # deprecated
password = "barbarbarbar"
publicly_accessible = false
username = "foo"
Expand Down
4 changes: 3 additions & 1 deletion website/docs/r/db_instance.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ encoding in Oracle and Microsoft SQL instances (collation). This can't be change
Supported in Amazon RDS](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.OracleCharacterSets.html)
or [Server-Level Collation for Microsoft SQL Server](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.SQLServer.CommonDBATasks.Collation.html) for more information.
* `copy_tags_to_snapshot` – (Optional, boolean) Copy all Instance `tags` to snapshots. Default is `false`.
* `db_name` - (Optional) The name of the database to create when the DB instance is created. If this parameter is not specified, no database is created in the DB instance. Note that this does not apply for Oracle or SQL Server engines. See the [AWS documentation](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/create-db-instance.html) for more details on what applies for those engines. If you are providing an Oracle db name, it needs to be in all upper case. Cannot be specified for a replica.
* `db_subnet_group_name` - (Optional) Name of [DB subnet group](/docs/providers/aws/r/db_subnet_group.html). DB instance will
be created in the VPC associated with the DB subnet group. If unspecified, will
be created in the `default` VPC, or in EC2 Classic, if available. When working
Expand Down Expand Up @@ -153,7 +154,7 @@ information on the [AWS
Documentation](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_Monitoring.html)
what IAM permissions are needed to allow Enhanced Monitoring for RDS Instances.
* `multi_az` - (Optional) Specifies if the RDS instance is multi-AZ
* `name` - (Optional) The name of the database to create when the DB instance is created. If this parameter is not specified, no database is created in the DB instance. Note that this does not apply for Oracle or SQL Server engines. See the [AWS documentation](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/create-db-instance.html) for more details on what applies for those engines. If you are providing an Oracle db name, it needs to be in all upper case. Cannot be specified for a replica.
* `name` - (Optional, **Deprecated** use `db_name` instead) The name of the database to create when the DB instance is created. If this parameter is not specified, no database is created in the DB instance. Note that this does not apply for Oracle or SQL Server engines. See the [AWS documentation](https://awscli.amazonaws.com/v2/documentation/api/latest/reference/rds/create-db-instance.html) for more details on what applies for those engines. If you are providing an Oracle db name, it needs to be in all upper case. Cannot be specified for a replica.
* `nchar_character_set_name` - (Optional, Forces new resource) The national character set is used in the NCHAR, NVARCHAR2, and NCLOB data types for Oracle instances. This can't be changed. See [Oracle Character Sets
Supported in Amazon RDS](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.OracleCharacterSets.html).
* `option_group_name` - (Optional) Name of the DB option group to associate.
Expand Down Expand Up @@ -280,6 +281,7 @@ In addition to all arguments above, the following attributes are exported:
* `backup_window` - The backup window.
* `ca_cert_identifier` - Specifies the identifier of the CA certificate for the
DB instance.
* `db_name` - The database name.
* `domain` - The ID of the Directory Service Active Directory domain the instance is joined to
* `domain_iam_role_name` - The name of the IAM role to be used when making API calls to the Directory Service.
* `endpoint` - The connection endpoint in `address:port` format.
Expand Down

0 comments on commit 7875c87

Please sign in to comment.