diff --git a/.changelog/36518.txt b/.changelog/36518.txt new file mode 100644 index 00000000000..c394822277c --- /dev/null +++ b/.changelog/36518.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_db_instance: Adds warning when setting `character_set_name` when `replicate_source_db`, `restore_to_point_in_time`, or `snapshot_identifier` is set +``` diff --git a/internal/errs/diag.go b/internal/errs/diag.go index 220564a0cf7..46b08e3f4fb 100644 --- a/internal/errs/diag.go +++ b/internal/errs/diag.go @@ -74,6 +74,21 @@ func withPath(d diag.Diagnostic, path cty.Path) diag.Diagnostic { return d } +// newAttributeConflictsError is included for use with NewAttributeConflictsWillBeError. +// The typical behavior is covered using the schema ConflictsWith parameter. +func newAttributeConflictsError(path, otherPath cty.Path) diag.Diagnostic { + return NewAttributeErrorDiagnostic( + path, + "Invalid Attribute Combination", + fmt.Sprintf("Attribute %q cannot be specified when %q is specified.", + PathString(path), + PathString(otherPath), + ), + ) +} + +// NewAttributeConflictsWhenError returns an error diagnostic indicating that the attribute at the given path cannot be +// specified when the attribute at otherPath has the given value. func NewAttributeConflictsWhenError(path, otherPath cty.Path, otherValue string) diag.Diagnostic { return NewAttributeErrorDiagnostic( path, @@ -86,18 +101,32 @@ func NewAttributeConflictsWhenError(path, otherPath cty.Path, otherValue string) ) } -func NewAttributeRequiredWhenError(neededPath, path cty.Path, value string) diag.Diagnostic { +// NewAttributeRequiredWhenError returns an error diagnostic indicating that the attribute at neededPath is required when the +// attribute at otherPath has the given value. +func NewAttributeRequiredWhenError(neededPath, otherPath cty.Path, value string) diag.Diagnostic { return NewAttributeErrorDiagnostic( - path, + otherPath, "Invalid Attribute Combination", fmt.Sprintf("Attribute %q must be specified when %q is %q.", PathString(neededPath), - PathString(path), + PathString(otherPath), value, ), ) } +// NewAttributeConflictsWillBeError returns a warning diagnostic indicating that the attribute at the given path cannot be +// specified when the attribute at otherPath is set. +// This is intended to be used for situations where the conflict will become an error in a future release. +func NewAttributeConflictsWillBeError(path, otherPath cty.Path) diag.Diagnostic { + return willBeError( + newAttributeConflictsError(path, otherPath), + ) +} + +// NewAttributeConflictsWhenWillBeError returns a warning diagnostic indicating that the attribute at the given path cannot be +// specified when the attribute at otherPath has the given value. +// This is intended to be used for situations where the conflict will become an error in a future release. func NewAttributeConflictsWhenWillBeError(path, otherPath cty.Path, otherValue string) diag.Diagnostic { return willBeError( NewAttributeConflictsWhenError(path, otherPath, otherValue), diff --git a/internal/service/rds/instance.go b/internal/service/rds/instance.go index 05829884813..1036c045255 100644 --- a/internal/service/rds/instance.go +++ b/internal/service/rds/instance.go @@ -21,6 +21,7 @@ import ( "github.com/aws/aws-sdk-go/service/rds" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" tfawserr_sdkv2 "github.com/hashicorp/aws-sdk-go-base/v2/tfawserr" + "github.com/hashicorp/go-cty/cty" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry" @@ -152,6 +153,9 @@ func ResourceInstance() *schema.Resource { Computed: true, ForceNew: true, ValidateFunc: validation.StringInSlice(backupTarget_Values(), false), + ConflictsWith: []string{ + "s3_import", + }, }, "backup_window": { Type: schema.TypeString, @@ -182,6 +186,22 @@ func ResourceInstance() *schema.Resource { Optional: true, Computed: true, ForceNew: true, + ConflictsWith: []string{ + "s3_import", + }, + DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool { + for _, conflictAttr := range []string{ + "replicate_source_db", + // s3_import is handled by the schema ConflictsWith + "snapshot_identifier", + "restore_to_point_in_time", + } { + if _, ok := d.GetOk(conflictAttr); ok { + return true + } + } + return false + }, }, "copy_tags_to_snapshot": { Type: schema.TypeBool, @@ -628,6 +648,9 @@ func ResourceInstance() *schema.Resource { Optional: true, Computed: true, ForceNew: true, + ConflictsWith: []string{ + "s3_import", + }, }, "username": { Type: schema.TypeString, @@ -697,6 +720,23 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in var resourceID string // will be assigned depending on how it is created + if _, ok := d.GetOk("character_set_name"); ok { + charSetPath := cty.GetAttrPath("character_set_name") + for _, conflictAttr := range []string{ + "replicate_source_db", + // s3_import is handled by the schema ConflictsWith + "snapshot_identifier", + "restore_to_point_in_time", + } { + if _, ok := d.GetOk(conflictAttr); ok { + diags = append(diags, errs.NewAttributeConflictsWillBeError( + charSetPath, + cty.GetAttrPath(conflictAttr), + )) + } + } + } + if v, ok := d.GetOk("replicate_source_db"); ok { sourceDBInstanceID := v.(string) input := &rds.CreateDBInstanceReadReplicaInput{ @@ -902,15 +942,6 @@ func resourceInstanceCreate(ctx context.Context, d *schema.ResourceData, meta in if _, ok := d.GetOk("username"); !ok { diags = sdkdiag.AppendErrorf(diags, `"username": required field is not set`) } - if _, ok := d.GetOk("character_set_name"); ok { - diags = sdkdiag.AppendErrorf(diags, `"character_set_name" doesn't work with restores"`) - } - if _, ok := d.GetOk("timezone"); ok { - diags = sdkdiag.AppendErrorf(diags, `"timezone" doesn't work with restores"`) - } - if _, ok := d.GetOk("backup_target"); ok { - diags = sdkdiag.AppendErrorf(diags, `"backup_target" doesn't work with restores"`) - } if diags.HasError() { return diags } diff --git a/internal/service/rds/instance_test.go b/internal/service/rds/instance_test.go index 588e6ae0581..7945647db6d 100644 --- a/internal/service/rds/instance_test.go +++ b/internal/service/rds/instance_test.go @@ -2044,6 +2044,83 @@ func TestAccRDSInstance_ReplicateSourceDB_caCertificateIdentifier(t *testing.T) }) } +func TestAccRDSInstance_ReplicateSourceDB_characterSet_Source(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var dbInstance, sourceDbInstance rds.DBInstance + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceResourceName := "aws_db_instance.source" + resourceName := "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.RDSServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckInstanceDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_characterSet_Source(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(ctx, sourceResourceName, &sourceDbInstance), + resource.TestCheckResourceAttr(sourceResourceName, "character_set_name", "WE8ISO8859P15"), + testAccCheckInstanceExists(ctx, resourceName, &dbInstance), + resource.TestCheckResourceAttrPair(resourceName, "replicate_source_db", sourceResourceName, "identifier"), + resource.TestCheckResourceAttr(resourceName, "character_set_name", "WE8ISO8859P15"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "apply_immediately", + "final_snapshot_identifier", + "password", + "manage_master_user_password", + "skip_final_snapshot", + "delete_automated_backups", + }, + }, + }, + }) +} + +func TestAccRDSInstance_ReplicateSourceDB_characterSet_Replica(t *testing.T) { + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var dbInstance, sourceDbInstance rds.DBInstance + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceResourceName := "aws_db_instance.source" + resourceName := "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.RDSServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories, + CheckDestroy: testAccCheckInstanceDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_characterSet_Replica(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExists(ctx, sourceResourceName, &sourceDbInstance), + resource.TestCheckResourceAttr(sourceResourceName, "character_set_name", "WE8ISO8859P15"), + testAccCheckInstanceExists(ctx, resourceName, &dbInstance), + resource.TestCheckResourceAttrPair(resourceName, "replicate_source_db", sourceResourceName, "identifier"), + resource.TestCheckResourceAttr(resourceName, "character_set_name", "WE8ISO8859P15"), + ), + }, + }, + }) +} + func TestAccRDSInstance_ReplicateSourceDB_replicaMode(t *testing.T) { ctx := acctest.Context(t) if testing.Short() { @@ -2191,6 +2268,49 @@ func TestAccRDSInstance_ReplicateSourceDB_CrossRegion_parameterGroupNameEquivale }) } +func TestAccRDSInstance_ReplicateSourceDB_CrossRegion_characterSet(t *testing.T) { + t.Skip("Skipping due to upstream error") + ctx := acctest.Context(t) + if testing.Short() { + t.Skip("skipping long-running test in short mode") + } + + var dbInstance, sourceDbInstance rds.DBInstance + var providers []*schema.Provider + + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + sourceResourceName := "aws_db_instance.source" + resourceName := "aws_db_instance.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(ctx, t) }, + ErrorCheck: acctest.ErrorCheck(t, names.RDSServiceID), + ProtoV5ProviderFactories: acctest.ProtoV5FactoriesPlusProvidersAlternate(ctx, t, &providers), + CheckDestroy: testAccCheckInstanceDestroy(ctx), + Steps: []resource.TestStep{ + { + Config: testAccInstanceConfig_ReplicateSourceDB_CrossRegion_CharacterSet(rName), + Check: resource.ComposeAggregateTestCheckFunc( + testAccCheckInstanceExistsWithProvider(ctx, sourceResourceName, &sourceDbInstance, acctest.RegionProviderFunc(acctest.AlternateRegion(), &providers)), + resource.TestCheckResourceAttr(sourceResourceName, "character_set_name", "WE8ISO8859P15"), + testAccCheckInstanceExistsWithProvider(ctx, resourceName, &dbInstance, acctest.RegionProviderFunc(acctest.Region(), &providers)), + resource.TestCheckResourceAttrPair(resourceName, "replicate_source_db", sourceResourceName, "arn"), + resource.TestCheckResourceAttr(resourceName, "character_set_name", "WE8ISO8859P15"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "apply_immediately", + "password", + }, + }, + }, + }) +} + func TestAccRDSInstance_s3Import(t *testing.T) { acctest.Skip(t, "RestoreDBInstanceFromS3 cannot restore from MySQL version 5.6") @@ -6402,6 +6522,23 @@ func testAccInstanceConfig_orderableClassCustomSQLServerWeb() string { return testAccInstanceConfig_orderableClass("custom-sqlserver-web", "", "gp2") } +func testAccInstanceConfig_orderableClassOracleEnterprise() string { + return fmt.Sprintf(` +data "aws_rds_engine_version" "default" { + engine = %[1]q +} + +data "aws_rds_orderable_db_instance" "test" { + engine = data.aws_rds_engine_version.default.engine + engine_version = data.aws_rds_engine_version.default.version + license_model = %[2]q + storage_type = %[3]q + + preferred_instance_classes = [%[4]s] +} +`, tfrds.InstanceEngineOracleEnterprise, "bring-your-own-license", "gp2", strings.Replace(mainInstanceClasses, "db.t3.small", "frodo", 1)) +} + func testAccInstanceConfig_basic(rName string) string { return acctest.ConfigCompose( testAccInstanceConfig_orderableClassMySQL(), @@ -6971,6 +7108,8 @@ resource "aws_db_instance" "test" { username = "tfacctest" backup_retention_period = 0 + character_set_name = "WE8ISO8859P15" + parameter_group_name = "default.${data.aws_rds_engine_version.default.parameter_group_family}" skip_final_snapshot = true multi_az = false @@ -9599,6 +9738,70 @@ resource "aws_db_instance" "test" { `, rName)) } +func testAccInstanceConfig_ReplicateSourceDB_characterSet_Source(rName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassOracleEnterprise(), + fmt.Sprintf(` +resource "aws_db_instance" "test" { + identifier = %[1]q + replicate_source_db = aws_db_instance.source.identifier + instance_class = data.aws_rds_orderable_db_instance.test.instance_class + skip_final_snapshot = true + apply_immediately = true +} + +resource "aws_db_instance" "source" { + identifier = "%[1]s-source" + allocated_storage = 20 + 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 + storage_type = data.aws_rds_orderable_db_instance.test.storage_type + db_name = "MAINDB" + username = "oadmin" + password = "avoid-plaintext-passwords" + skip_final_snapshot = true + apply_immediately = true + backup_retention_period = 1 + + character_set_name = "WE8ISO8859P15" +} +`, rName)) +} + +func testAccInstanceConfig_ReplicateSourceDB_characterSet_Replica(rName string) string { + return acctest.ConfigCompose( + testAccInstanceConfig_orderableClassOracleEnterprise(), + fmt.Sprintf(` +resource "aws_db_instance" "test" { + identifier = %[1]q + replicate_source_db = aws_db_instance.source.identifier + instance_class = data.aws_rds_orderable_db_instance.test.instance_class + skip_final_snapshot = true + apply_immediately = true + + character_set_name = "NE8ISO8859P10" +} + +resource "aws_db_instance" "source" { + identifier = "%[1]s-source" + allocated_storage = 20 + 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 + storage_type = data.aws_rds_orderable_db_instance.test.storage_type + db_name = "MAINDB" + username = "oadmin" + password = "avoid-plaintext-passwords" + skip_final_snapshot = true + apply_immediately = true + backup_retention_period = 1 + + character_set_name = "WE8ISO8859P15" +} +`, rName)) +} + func testAccInstanceConfig_ReplicateSourceDB_replicaMode(rName, replicaMode string) string { return fmt.Sprintf(` data "aws_rds_engine_version" "default" { @@ -9776,6 +9979,41 @@ data "aws_rds_orderable_db_instance" "test" { `, rName, tfrds.InstanceEngineOracleEnterprise, strings.Replace(mainInstanceClasses, "db.t3.small", "frodo", 1), parameters)) } +func testAccInstanceConfig_ReplicateSourceDB_CrossRegion_CharacterSet(rName string) string { + return acctest.ConfigCompose( + acctest.ConfigMultipleRegionProvider(2), + testAccInstanceConfig_orderableClassOracleEnterprise(), fmt.Sprintf(` +resource "aws_db_instance" "test" { + provider = "aws" + + identifier = %[1]q + replicate_source_db = aws_db_instance.source.arn + instance_class = data.aws_rds_orderable_db_instance.test.instance_class + skip_final_snapshot = true + apply_immediately = true +} + +resource "aws_db_instance" "source" { + provider = "awsalternate" + + identifier = "%[1]s-source" + allocated_storage = 20 + 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 + storage_type = data.aws_rds_orderable_db_instance.test.storage_type + db_name = "MAINDB" + username = "oadmin" + password = "avoid-plaintext-passwords" + skip_final_snapshot = true + apply_immediately = true + backup_retention_period = 1 + + character_set_name = "WE8ISO8859P15" +} +`, rName)) +} + func testAccInstanceConfig_baseMonitoringRole(rName string) string { return fmt.Sprintf(` data "aws_partition" "current" {} diff --git a/website/docs/r/db_instance.html.markdown b/website/docs/r/db_instance.html.markdown index f37bf58ba86..2530f640c4a 100644 --- a/website/docs/r/db_instance.html.markdown +++ b/website/docs/r/db_instance.html.markdown @@ -303,10 +303,11 @@ Defaults to true. * `blue_green_update` - (Optional) Enables low-downtime updates using [RDS Blue/Green deployments][blue-green]. See [`blue_green_update`](#blue_green_update) below. * `ca_cert_identifier` - (Optional) The identifier of the CA certificate for the DB instance. -* `character_set_name` - (Optional) The character set name to use for DB -encoding in Oracle and Microsoft SQL instances (collation). This can't be changed. See [Oracle Character Sets -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. +* `character_set_name` - (Optional) The character set name to use for DB encoding in Oracle and Microsoft SQL instances (collation). + This can't be changed. + See [Oracle Character Sets 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. + Cannot be set with `replicate_source_db`, `restore_to_point_in_time`, `s3_import`, or `snapshot_identifier`. * `copy_tags_to_snapshot` – (Optional, boolean) Copy all Instance `tags` to snapshots. Default is `false`. * `custom_iam_instance_profile` - (Optional) The instance profile associated with the underlying Amazon EC2 instance of an RDS Custom DB instance. * `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.