Skip to content

Commit

Permalink
Merge pull request #33990 from hashicorp/b-rds-bluegreen-password
Browse files Browse the repository at this point in the history 
resource/aws_db_instance: Changing `password` bypasses Blue/Green deployment
  • Loading branch information
@gdavison
gdavison authored Oct 19, 2023
2 parents 1b4a5ae + 68d34bc commit 8054e73
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 47 deletions.
40 changes: 11 additions & 29 deletions internal/service/rds/certificate_data_source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ package rds

import (
"context"
"sort"
"time"

"github.com/aws/aws-sdk-go/aws"
Expand All @@ -14,6 +13,7 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-aws/internal/conns"
"github.com/hashicorp/terraform-provider-aws/internal/errs/sdkdiag"
"golang.org/x/exp/slices"
)

// @SDKDataSource("aws_rds_certificate")
Expand Down Expand Up @@ -100,22 +100,20 @@ func dataSourceCertificateRead(ctx context.Context, d *schema.ResourceData, meta
var certificate *rds.Certificate

if d.Get("latest_valid_till").(bool) {
sort.Sort(rdsCertificateValidTillSort(certificates))
slices.SortFunc(certificates, func(a, b *rds.Certificate) int {
return a.ValidTill.Compare(*b.ValidTill)
})
certificate = certificates[len(certificates)-1]
}

if len(certificates) > 1 {
return sdkdiag.AppendErrorf(diags, "multiple RDS Certificates match the criteria; try changing search query")
}

if certificate == nil && len(certificates) == 1 {
} else {
if len(certificates) > 1 {
return sdkdiag.AppendErrorf(diags, "multiple RDS Certificates match the criteria; try changing search query")
}
if len(certificates) == 0 {
return sdkdiag.AppendErrorf(diags, "no RDS Certificates match the criteria")
}
certificate = certificates[0]
}

if certificate == nil {
return sdkdiag.AppendErrorf(diags, "no RDS Certificates match the criteria")
}

d.SetId(aws.StringValue(certificate.CertificateIdentifier))

d.Set("arn", certificate.CertificateArn)
Expand All @@ -138,19 +136,3 @@ func dataSourceCertificateRead(ctx context.Context, d *schema.ResourceData, meta

return diags
}

type rdsCertificateValidTillSort []*rds.Certificate

func (s rdsCertificateValidTillSort) Len() int { return len(s) }
func (s rdsCertificateValidTillSort) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
func (s rdsCertificateValidTillSort) Less(i, j int) bool {
if s[i] == nil || s[i].ValidTill == nil {
return true
}

if s[j] == nil || s[j].ValidTill == nil {
return false
}

return (*s[i].ValidTill).Before(*s[j].ValidTill)
}
10 changes: 5 additions & 5 deletions internal/service/rds/certificate_data_source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestAccRDSCertificateDataSource_id(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccCertificateDataSourceConfig_id(),
Check: resource.ComposeTestCheckFunc(
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttrPair(dataSourceName, "id", "data.aws_rds_certificate.latest", "id"),
),
},
Expand All @@ -46,13 +46,13 @@ func TestAccRDSCertificateDataSource_latestValidTill(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccCertificateDataSourceConfig_latestValidTill(),
Check: resource.ComposeTestCheckFunc(
acctest.MatchResourceAttrRegionalARNNoAccount(dataSourceName, "arn", "rds", regexache.MustCompile(`cert:rds-ca-[0-9]{4}`)),
Check: resource.ComposeAggregateTestCheckFunc(
acctest.MatchResourceAttrRegionalARNNoAccount(dataSourceName, "arn", "rds", regexache.MustCompile(`cert:rds-ca-[-0-9a-z]+$`)),
resource.TestCheckResourceAttr(dataSourceName, "certificate_type", "CA"),
resource.TestCheckResourceAttr(dataSourceName, "customer_override", "false"),
resource.TestCheckNoResourceAttr(dataSourceName, "customer_override_valid_till"),
resource.TestMatchResourceAttr(dataSourceName, "id", regexache.MustCompile(`rds-ca-[0-9]{4}`)),
resource.TestMatchResourceAttr(dataSourceName, "thumbprint", regexache.MustCompile(`[0-9a-f]+`)),
resource.TestMatchResourceAttr(dataSourceName, "id", regexache.MustCompile(`^rds-ca-[-0-9a-z]+$`)),
resource.TestMatchResourceAttr(dataSourceName, "thumbprint", regexache.MustCompile(`^[0-9a-f]+$`)),
resource.TestMatchResourceAttr(dataSourceName, "valid_from", regexache.MustCompile(acctest.RFC3339RegexPattern)),
resource.TestMatchResourceAttr(dataSourceName, "valid_till", regexache.MustCompile(acctest.RFC3339RegexPattern)),
),
Expand Down
12 changes: 11 additions & 1 deletion internal/service/rds/instance.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,17 @@ func resourceInstanceUpdate(ctx context.Context, d *schema.ResourceData, meta in
"skip_final_snapshot",
"tags", "tags_all",
) {
if d.Get("blue_green_update.0.enabled").(bool) {
if d.Get("blue_green_update.0.enabled").(bool) && d.HasChangesExcept(
"allow_major_version_upgrade",
"blue_green_update",
"delete_automated_backups",
"final_snapshot_identifier",
"replicate_source_db",
"skip_final_snapshot",
"tags", "tags_all",
"deletion_protection",
"password",
) {
orchestrator := newBlueGreenOrchestrator(conn)
handler := newInstanceHandler(conn)
var cleaupWaiters []func(optFns ...tfresource.OptionsFunc)
Expand Down
91 changes: 79 additions & 12 deletions internal/service/rds/instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ func TestAccRDSInstance_password(t *testing.T) {
t.Skip("skipping long-running test in short mode")
}

var v rds.DBInstance
var v1, v2 rds.DBInstance
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_db_instance.test"

Expand All @@ -937,7 +937,7 @@ func TestAccRDSInstance_password(t *testing.T) {
{
Config: testAccInstanceConfig_password(rName, "valid-password-1"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInstanceExists(ctx, resourceName, &v),
testAccCheckInstanceExists(ctx, resourceName, &v1),
resource.TestCheckResourceAttr(resourceName, "password", "valid-password-1"),
),
},
Expand All @@ -955,7 +955,8 @@ func TestAccRDSInstance_password(t *testing.T) {
{
Config: testAccInstanceConfig_password(rName, "valid-password-2"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInstanceExists(ctx, resourceName, &v),
testAccCheckInstanceExists(ctx, resourceName, &v2),
testAccCheckDBInstanceNotRecreated(&v1, &v2),
resource.TestCheckResourceAttr(resourceName, "password", "valid-password-2"),
),
},
Expand Down Expand Up @@ -4816,6 +4817,7 @@ func TestAccRDSInstance_BlueGreenDeployment_updateEngineVersion(t *testing.T) {
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
},
Expand Down Expand Up @@ -4921,6 +4923,7 @@ func TestAccRDSInstance_BlueGreenDeployment_tags(t *testing.T) {
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
},
Expand Down Expand Up @@ -4971,6 +4974,7 @@ func TestAccRDSInstance_BlueGreenDeployment_updateInstanceClass(t *testing.T) {
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
},
Expand Down Expand Up @@ -5075,14 +5079,14 @@ func TestAccRDSInstance_BlueGreenDeployment_updateAndEnableBackups(t *testing.T)
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
},
})
}

// TODO: When the only change is to disable deletion_protection, bypass Blue/Green
func TestAccRDSInstance_BlueGreenDeployment_deletionProtection(t *testing.T) {
func TestAccRDSInstance_BlueGreenDeployment_deletionProtectionBypassesBlueGreen(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
t.Skip("skipping long-running test in short mode")
Expand Down Expand Up @@ -5117,14 +5121,14 @@ func TestAccRDSInstance_BlueGreenDeployment_deletionProtection(t *testing.T) {
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
{
Config: testAccInstanceConfig_BlueGreenDeployment_deletionProtection(rName, false),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInstanceExists(ctx, resourceName, &v2),
// TODO: This should bypass Blue/Green Deployment
// testAccCheckDBInstanceNotRecreated(&v2, &v3),
testAccCheckDBInstanceNotRecreated(&v1, &v2),
resource.TestCheckResourceAttr(resourceName, "deletion_protection", "false"),
resource.TestCheckResourceAttr(resourceName, "blue_green_update.0.enabled", "true"),
),
Expand All @@ -5140,12 +5144,48 @@ func TestAccRDSInstance_BlueGreenDeployment_deletionProtection(t *testing.T) {
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
},
})
}

func TestAccRDSInstance_BlueGreenDeployment_passwordBypassesBlueGreen(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
t.Skip("skipping long-running test in short mode")
}

var v1, v2 rds.DBInstance
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_db_instance.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, rds.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckInstanceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccInstanceConfig_BlueGreenDeployment_password(rName, "valid-password-1"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInstanceExists(ctx, resourceName, &v1),
resource.TestCheckResourceAttr(resourceName, "password", "valid-password-1"),
),
},
{
Config: testAccInstanceConfig_BlueGreenDeployment_password(rName, "valid-password-2"),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInstanceExists(ctx, resourceName, &v2),
testAccCheckDBInstanceNotRecreated(&v1, &v2),
resource.TestCheckResourceAttr(resourceName, "password", "valid-password-2"),
),
},
},
})
}

func TestAccRDSInstance_BlueGreenDeployment_updateWithDeletionProtection(t *testing.T) {
ctx := acctest.Context(t)
if testing.Short() {
Expand Down Expand Up @@ -5182,6 +5222,7 @@ func TestAccRDSInstance_BlueGreenDeployment_updateWithDeletionProtection(t *test
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
{
Expand All @@ -5205,14 +5246,14 @@ func TestAccRDSInstance_BlueGreenDeployment_updateWithDeletionProtection(t *test
"skip_final_snapshot",
"delete_automated_backups",
"blue_green_update",
"latest_restorable_time",
},
},
{
Config: testAccInstanceConfig_BlueGreenDeployment_updateInstanceClassWithDeletionProtection(rName, false),
Check: resource.ComposeAggregateTestCheckFunc(
testAccCheckInstanceExists(ctx, resourceName, &v3),
// TODO: This should bypass Blue/Green Deployment
// testAccCheckDBInstanceNotRecreated(&v2, &v3),
testAccCheckDBInstanceNotRecreated(&v2, &v3),
resource.TestCheckResourceAttr(resourceName, "deletion_protection", "false"),
resource.TestCheckResourceAttr(resourceName, "blue_green_update.0.enabled", "true"),
),
Expand Down Expand Up @@ -7644,7 +7685,9 @@ resource "aws_db_instance" "test" {
}

func testAccInstanceConfig_password(rName, password string) string {
return acctest.ConfigCompose(testAccInstanceConfig_orderableClassMySQL(), fmt.Sprintf(`
return acctest.ConfigCompose(
testAccInstanceConfig_orderableClassMySQL(),
fmt.Sprintf(`
resource "aws_db_instance" "test" {
allocated_storage = 5
engine = data.aws_rds_orderable_db_instance.test.engine
Expand Down Expand Up @@ -9247,7 +9290,7 @@ func testAccInstanceConfig_SnapshotID_allowMajorVersionUpgrade(rName string, all
return fmt.Sprintf(`
data "aws_rds_orderable_db_instance" "postgres13" {
engine = "postgres"
engine_version = "13.5"
engine_version = "13.12"
license_model = "postgresql-license"
storage_type = "standard"
Expand All @@ -9272,7 +9315,7 @@ resource "aws_db_snapshot" "test" {
data "aws_rds_orderable_db_instance" "postgres14" {
engine = "postgres"
engine_version = "14.1"
engine_version = "14.9"
license_model = "postgresql-license"
storage_type = "standard"
Expand Down Expand Up @@ -10800,6 +10843,30 @@ data "aws_rds_orderable_db_instance" "updated" {
`, rName, deletionProtection))
}

func testAccInstanceConfig_BlueGreenDeployment_password(rName, password string) string {
return acctest.ConfigCompose(
testAccInstanceConfig_orderableClassMySQL(),
fmt.Sprintf(`
resource "aws_db_instance" "test" {
identifier = %[1]q
allocated_storage = 10
backup_retention_period = 1
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
db_name = "test"
parameter_group_name = "default.${data.aws_rds_engine_version.default.parameter_group_family}"
skip_final_snapshot = true
password = %[2]q
username = "tfacctest"
blue_green_update {
enabled = true
}
}
`, rName, password))
}

func testAccInstanceConfig_gp3(rName string, orderableClassConfig func() string, allocatedStorage int) string {
return acctest.ConfigCompose(
orderableClassConfig(),
Expand Down

0 comments on commit 8054e73

Please sign in to comment.