mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 09:01:31 +00:00
fix: eliminate RDS Terraform drift (#729)
This commit is contained in:
@@ -70,8 +70,8 @@ const arnSegmentCount = 7
|
||||
// Region and account are checked here rather than at policy evaluation, so a
|
||||
// foreign-account reference never reaches the evaluator at all.
|
||||
func ParseARN(arn, region, accountID string) (ParsedARN, error) {
|
||||
// The identifier cannot contain a colon, so the trailing segment SplitN
|
||||
// leaves intact is rejected below if it holds one.
|
||||
// SplitN leaves an automated snapshot's rds: prefix in the identifier. Other
|
||||
// resource kinds still reject that extra separator below.
|
||||
parts := strings.SplitN(arn, ":", arnSegmentCount)
|
||||
if len(parts) != arnSegmentCount {
|
||||
return ParsedARN{}, arnError(arn, "expected the form arn:aws:rds:{region}:{account}:{type}:{name}")
|
||||
@@ -91,7 +91,13 @@ func ParseARN(arn, region, accountID string) (ParsedARN, error) {
|
||||
return ParsedARN{}, arnError(arn, fmt.Sprintf("%q is not an RDS resource type", parts[5]))
|
||||
}
|
||||
identifier := parts[6]
|
||||
if identifier == "" || strings.ContainsAny(identifier, ":/") {
|
||||
malformed := identifier == "" || strings.Contains(identifier, "/")
|
||||
if kind == ResourceKindDBSnapshot {
|
||||
malformed = malformed || validateDBSnapshotReference(identifier) != nil
|
||||
} else {
|
||||
malformed = malformed || strings.Contains(identifier, ":")
|
||||
}
|
||||
if malformed {
|
||||
return ParsedARN{}, arnError(arn, "the resource name is empty or malformed")
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ package handlers_rds
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/mulgadc/spinifex/spinifex/awserrors"
|
||||
"github.com/stretchr/testify/assert"
|
||||
@@ -11,19 +12,22 @@ import (
|
||||
// Every ARN the builders mint must parse back into the parts it was built
|
||||
// from, so a resource named by a describe is addressable by a tag action.
|
||||
func TestParseARN_RoundTripsEveryResourceType(t *testing.T) {
|
||||
automated := AutomatedSnapshotIdentifier("orders-db", time.Date(2026, 7, 24, 3, 4, 0, 0, time.UTC))
|
||||
cases := []struct {
|
||||
name string
|
||||
kind ResourceKind
|
||||
identifier string
|
||||
arn string
|
||||
}{
|
||||
{ResourceKindDBInstance, "orders-db", DBInstanceARN(testRegion, testAccountID, "orders-db")},
|
||||
{ResourceKindDBSnapshot, "orders-db-2026-07-24", DBSnapshotARN(testRegion, testAccountID, "orders-db-2026-07-24")},
|
||||
{ResourceKindDBSubnetGroup, "prod-db-subnets", DBSubnetGroupARN(testRegion, testAccountID, "prod-db-subnets")},
|
||||
{ResourceKindDBParameterGroup, "postgres16-tuned", DBParameterGroupARN(testRegion, testAccountID, "postgres16-tuned")},
|
||||
{"instance", ResourceKindDBInstance, "orders-db", DBInstanceARN(testRegion, testAccountID, "orders-db")},
|
||||
{"manual snapshot", ResourceKindDBSnapshot, "orders-db-2026-07-24", DBSnapshotARN(testRegion, testAccountID, "orders-db-2026-07-24")},
|
||||
{"automated snapshot", ResourceKindDBSnapshot, automated, DBSnapshotARN(testRegion, testAccountID, automated)},
|
||||
{"subnet group", ResourceKindDBSubnetGroup, "prod-db-subnets", DBSubnetGroupARN(testRegion, testAccountID, "prod-db-subnets")},
|
||||
{"parameter group", ResourceKindDBParameterGroup, "postgres16-tuned", DBParameterGroupARN(testRegion, testAccountID, "postgres16-tuned")},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(string(tc.kind), func(t *testing.T) {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
parsed, err := ParseARN(tc.arn, testRegion, testAccountID)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tc.kind, parsed.Kind)
|
||||
@@ -44,6 +48,9 @@ func TestParseARN_RejectsMalformedAndForeignARNs(t *testing.T) {
|
||||
{"slash delimited", "arn:aws:rds:" + testRegion + ":" + testAccountID + ":db/orders-db"},
|
||||
{"truncated", "arn:aws:rds:" + testRegion + ":" + testAccountID + ":db"},
|
||||
{"extra segment", DBInstanceARN(testRegion, testAccountID, "orders-db") + ":extra"},
|
||||
{"snapshot with a foreign namespace", DBSnapshotARN(testRegion, testAccountID, "other:orders-db")},
|
||||
{"snapshot with two embedded colons", DBSnapshotARN(testRegion, testAccountID, "rds:orders-db:extra")},
|
||||
{"snapshot with a slash", DBSnapshotARN(testRegion, testAccountID, "rds:orders/db")},
|
||||
{"empty identifier", "arn:aws:rds:" + testRegion + ":" + testAccountID + ":db:"},
|
||||
{"another service", "arn:aws:ec2:" + testRegion + ":" + testAccountID + ":db:orders-db"},
|
||||
{"another partition", "arn:aws-cn:rds:" + testRegion + ":" + testAccountID + ":db:orders-db"},
|
||||
|
||||
@@ -158,7 +158,10 @@ func newDBInstanceRecord(accountID string, req *validatedCreate, placement *endp
|
||||
DBParameterGroupName: req.DBParameterGroupName,
|
||||
DeletionProtection: req.DeletionProtection,
|
||||
|
||||
AutoMinorVersionUpgrade: req.AutoMinorVersionUpgrade,
|
||||
AutoMinorVersionUpgrade: req.AutoMinorVersionUpgrade,
|
||||
CopyTagsToSnapshot: req.CopyTagsToSnapshot,
|
||||
MonitoringInterval: req.MonitoringInterval,
|
||||
EnablePerformanceInsights: req.EnablePerformanceInsights,
|
||||
|
||||
BackupRetentionPeriod: req.BackupRetentionPeriod,
|
||||
PreferredBackupWindow: req.PreferredBackupWindow,
|
||||
|
||||
@@ -520,6 +520,8 @@ func TestValidateCreateRequest_RejectsUnimplementedParameters(t *testing.T) {
|
||||
{"StorageEncryptedFalse", func(in *rds.CreateDBInstanceInput) { in.StorageEncrypted = aws.Bool(false) }, "StorageEncrypted"},
|
||||
{"IAMDatabaseAuth", func(in *rds.CreateDBInstanceInput) { in.EnableIAMDatabaseAuthentication = aws.Bool(true) }, "EnableIAMDatabaseAuthentication"},
|
||||
{"Iops", func(in *rds.CreateDBInstanceInput) { in.Iops = aws.Int64(3000) }, "Iops"},
|
||||
{"MaxAllocatedStorage", func(in *rds.CreateDBInstanceInput) { in.MaxAllocatedStorage = aws.Int64(500) }, "MaxAllocatedStorage"},
|
||||
{"StorageThroughput", func(in *rds.CreateDBInstanceInput) { in.StorageThroughput = aws.Int64(250) }, "StorageThroughput"},
|
||||
{"KmsKeyId", func(in *rds.CreateDBInstanceInput) { in.KmsKeyId = aws.String("arn:aws:kms:::key/abc") }, "KmsKeyId"},
|
||||
{"AvailabilityZone", func(in *rds.CreateDBInstanceInput) { in.AvailabilityZone = aws.String("ap-southeast-2b") }, "AvailabilityZone"},
|
||||
{"DBSecurityGroups", func(in *rds.CreateDBInstanceInput) { in.DBSecurityGroups = aws.StringSlice([]string{"classic"}) }, "DBSecurityGroups"},
|
||||
@@ -567,6 +569,7 @@ func TestValidateCreateRequest_RejectsMalformedRequests(t *testing.T) {
|
||||
in.DBInstanceIdentifier = aws.String(strings.Repeat("a", maxDBInstanceIdentifierLen+1))
|
||||
}, awserrors.ErrorInvalidParameterValue},
|
||||
{"UnknownEngine", func(in *rds.CreateDBInstanceInput) { in.Engine = aws.String("mysql") }, awserrors.ErrorInvalidParameterValue},
|
||||
{"MinorEngineVersion", func(in *rds.CreateDBInstanceInput) { in.EngineVersion = aws.String("18.4") }, awserrors.ErrorInvalidParameterValue},
|
||||
{"UnknownClass", func(in *rds.CreateDBInstanceInput) { in.DBInstanceClass = aws.String("db.x99.mega") }, awserrors.ErrorInvalidParameterValue},
|
||||
{"StorageBelowMinimum", func(in *rds.CreateDBInstanceInput) {
|
||||
in.AllocatedStorage = aws.Int64(minAllocatedStorageGiB - 1)
|
||||
@@ -614,6 +617,10 @@ func TestValidateCreateRequest_AcceptsInertParameters(t *testing.T) {
|
||||
input.MonitoringInterval = aws.Int64(60)
|
||||
input.StorageEncrypted = aws.Bool(true)
|
||||
|
||||
_, err := newCreateValidator().validateCreateRequest(input)
|
||||
req, err := newCreateValidator().validateCreateRequest(input)
|
||||
require.NoError(t, err)
|
||||
assert.True(t, req.AutoMinorVersionUpgrade)
|
||||
assert.True(t, req.CopyTagsToSnapshot)
|
||||
assert.True(t, req.EnablePerformanceInsights)
|
||||
assert.Equal(t, int64(60), req.MonitoringInterval)
|
||||
}
|
||||
|
||||
@@ -145,13 +145,17 @@ func (s *Service) projectDBInstance(rec *DBInstanceRecord) *rds.DBInstance {
|
||||
StorageType: aws.String(rec.StorageType),
|
||||
StorageEncrypted: aws.Bool(rec.StorageEncrypted),
|
||||
MasterUsername: aws.String(rec.MasterUsername),
|
||||
DbInstancePort: aws.Int64(rec.Port),
|
||||
MultiAZ: aws.Bool(false),
|
||||
PubliclyAccessible: aws.Bool(false),
|
||||
DeletionProtection: aws.Bool(rec.DeletionProtection),
|
||||
// Echoed, not acted on: nothing upgrades a pinned version, but a client
|
||||
// that set it has to read back what it set.
|
||||
AutoMinorVersionUpgrade: aws.Bool(rec.AutoMinorVersionUpgrade),
|
||||
InstanceCreateTime: aws.Time(rec.CreatedAt),
|
||||
// Accepted inert settings are echoed so a client reads back what it set
|
||||
// instead of planning a change no modify can deliver.
|
||||
AutoMinorVersionUpgrade: aws.Bool(rec.AutoMinorVersionUpgrade),
|
||||
CopyTagsToSnapshot: aws.Bool(rec.CopyTagsToSnapshot),
|
||||
MonitoringInterval: aws.Int64(rec.MonitoringInterval),
|
||||
PerformanceInsightsEnabled: aws.Bool(rec.EnablePerformanceInsights),
|
||||
InstanceCreateTime: aws.Time(rec.CreatedAt),
|
||||
// The Terraform provider reads tags from the describe as well as from
|
||||
// ListTagsForResource, so the two have to agree.
|
||||
TagList: tagsToAWS(rec.Tags),
|
||||
|
||||
@@ -156,6 +156,45 @@ func TestDescribeDBInstances_EchoesAutoMinorVersionUpgrade(t *testing.T) {
|
||||
assert.False(t, aws.BoolValue(out.DBInstances[0].AutoMinorVersionUpgrade))
|
||||
}
|
||||
|
||||
func TestDescribeDBInstances_EchoesAcceptedInertSettings(t *testing.T) {
|
||||
h := newCreateHarness(t, "")
|
||||
input := validCreateInput()
|
||||
input.CopyTagsToSnapshot = aws.Bool(true)
|
||||
input.EnablePerformanceInsights = aws.Bool(true)
|
||||
input.MonitoringInterval = aws.Int64(60)
|
||||
|
||||
_, err := h.svc.CreateDBInstance(t.Context(), input, testAccountID)
|
||||
require.NoError(t, err)
|
||||
|
||||
rec := h.record(t, testDBInstanceID)
|
||||
assert.True(t, rec.CopyTagsToSnapshot)
|
||||
assert.True(t, rec.EnablePerformanceInsights)
|
||||
assert.Equal(t, int64(60), rec.MonitoringInterval)
|
||||
|
||||
out, err := h.svc.DescribeDBInstances(t.Context(), &rds.DescribeDBInstancesInput{
|
||||
DBInstanceIdentifier: aws.String(testDBInstanceID),
|
||||
}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, out.DBInstances, 1)
|
||||
assert.True(t, aws.BoolValue(out.DBInstances[0].CopyTagsToSnapshot))
|
||||
assert.True(t, aws.BoolValue(out.DBInstances[0].PerformanceInsightsEnabled))
|
||||
assert.Equal(t, int64(60), aws.Int64Value(out.DBInstances[0].MonitoringInterval))
|
||||
}
|
||||
|
||||
func TestDescribeDBInstances_EchoesOmittedInertSettingsAsDisabled(t *testing.T) {
|
||||
h := newCreateHarness(t, "")
|
||||
seedCreated(t, h, testDBInstanceID)
|
||||
|
||||
out, err := h.svc.DescribeDBInstances(t.Context(), &rds.DescribeDBInstancesInput{
|
||||
DBInstanceIdentifier: aws.String(testDBInstanceID),
|
||||
}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, out.DBInstances, 1)
|
||||
assert.False(t, aws.BoolValue(out.DBInstances[0].CopyTagsToSnapshot))
|
||||
assert.False(t, aws.BoolValue(out.DBInstances[0].PerformanceInsightsEnabled))
|
||||
assert.Zero(t, aws.Int64Value(out.DBInstances[0].MonitoringInterval))
|
||||
}
|
||||
|
||||
// A filter nobody implemented is refused rather than dropped: dropping it
|
||||
// returns exactly the rows the caller asked to exclude.
|
||||
func TestDescribeDBInstances_RejectsAnUnrecognizedFilter(t *testing.T) {
|
||||
@@ -208,6 +247,7 @@ func TestProjectDBInstance_OmitsAnUnsettledEndpoint(t *testing.T) {
|
||||
Port: 5432,
|
||||
})
|
||||
assert.Nil(t, instance.Endpoint)
|
||||
assert.Equal(t, int64(5432), aws.Int64Value(instance.DbInstancePort))
|
||||
assert.Nil(t, instance.DBName, "a request that named no database has none, not an empty one")
|
||||
assert.Nil(t, h.svc.projectDBInstance(nil))
|
||||
}
|
||||
|
||||
@@ -62,15 +62,10 @@ func SupportedEngines() []string {
|
||||
return slices.Sorted(maps.Keys(engines))
|
||||
}
|
||||
|
||||
// An empty version takes the pin. A supplied one must name the pinned major,
|
||||
// with or without a minor, since a mismatch would silently run a version the
|
||||
// customer did not ask for.
|
||||
// An empty version takes the pin. A supplied one must name the pinned major
|
||||
// exactly, since the image does not promise any particular minor version.
|
||||
func (e Engine) ValidateVersion(version string) error {
|
||||
v := strings.TrimSpace(version)
|
||||
if v == "" || v == e.MajorVersion {
|
||||
return nil
|
||||
}
|
||||
if major, _, ok := strings.Cut(v, "."); ok && major == e.MajorVersion {
|
||||
if version == "" || version == e.MajorVersion {
|
||||
return nil
|
||||
}
|
||||
return awserrors.Errorf(awserrors.ErrorInvalidParameterValue,
|
||||
|
||||
@@ -31,10 +31,10 @@ func TestEngineValidateVersion(t *testing.T) {
|
||||
engine, err := LookupEngine("postgres")
|
||||
require.NoError(t, err)
|
||||
|
||||
for _, version := range []string{"", "18", "18.1", "18.4"} {
|
||||
for _, version := range []string{"", "18"} {
|
||||
assert.NoError(t, engine.ValidateVersion(version), "version %q", version)
|
||||
}
|
||||
for _, version := range []string{"17", "16.2", "19", "latest"} {
|
||||
for _, version := range []string{"17", "16.2", "18.1", "18.4", " 18 ", "19", "latest"} {
|
||||
assert.Error(t, engine.ValidateVersion(version), "version %q", version)
|
||||
}
|
||||
|
||||
|
||||
@@ -29,6 +29,9 @@ type modifyPlan struct {
|
||||
SecurityGroupIDs []string
|
||||
DeletionProtection *bool
|
||||
AutoMinorVersionUpgrade *bool
|
||||
CopyTagsToSnapshot *bool
|
||||
MonitoringInterval *int64
|
||||
EnablePerformanceInsights *bool
|
||||
BackupRetentionPeriod *int64
|
||||
PreferredBackupWindow string
|
||||
PreferredMaintenanceWindow string
|
||||
@@ -54,7 +57,8 @@ func (p *modifyPlan) disruptive() bool {
|
||||
// deferred must not report a configuration change that has not happened.
|
||||
func (p *modifyPlan) immediate() bool {
|
||||
return p.MasterUserPassword != "" || p.SecurityGroupIDs != nil || p.DeletionProtection != nil ||
|
||||
p.AutoMinorVersionUpgrade != nil || p.BackupRetentionPeriod != nil ||
|
||||
p.AutoMinorVersionUpgrade != nil || p.CopyTagsToSnapshot != nil ||
|
||||
p.MonitoringInterval != nil || p.EnablePerformanceInsights != nil || p.BackupRetentionPeriod != nil ||
|
||||
p.PreferredBackupWindow != "" || p.PreferredMaintenanceWindow != ""
|
||||
}
|
||||
|
||||
@@ -139,7 +143,11 @@ func (s *Service) ModifyDBInstance(ctx context.Context, input *rds.ModifyDBInsta
|
||||
s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, id,
|
||||
"Modification recorded; it will be applied during the next maintenance window, and the database will be unavailable while it is.",
|
||||
EventCategoryConfigurationChange, EventCategoryNotification)
|
||||
return &rds.ModifyDBInstanceOutput{DBInstance: s.projectDBInstance(rec)}, nil
|
||||
stored, _, err := s.getDBInstance(ctx, kv, id)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return &rds.ModifyDBInstanceOutput{DBInstance: s.projectDBInstance(stored)}, nil
|
||||
}
|
||||
|
||||
moved, kv, err := s.beginTransition(ctx, accountID, id, StatusModifying, StatusAvailable, StatusFailed)
|
||||
@@ -224,6 +232,15 @@ func (s *Service) planModify(ctx context.Context, input *rds.ModifyDBInstanceInp
|
||||
if input.AutoMinorVersionUpgrade != nil && aws.BoolValue(input.AutoMinorVersionUpgrade) != rec.AutoMinorVersionUpgrade {
|
||||
plan.AutoMinorVersionUpgrade = input.AutoMinorVersionUpgrade
|
||||
}
|
||||
if input.CopyTagsToSnapshot != nil && aws.BoolValue(input.CopyTagsToSnapshot) != rec.CopyTagsToSnapshot {
|
||||
plan.CopyTagsToSnapshot = input.CopyTagsToSnapshot
|
||||
}
|
||||
if input.MonitoringInterval != nil && aws.Int64Value(input.MonitoringInterval) != rec.MonitoringInterval {
|
||||
plan.MonitoringInterval = input.MonitoringInterval
|
||||
}
|
||||
if input.EnablePerformanceInsights != nil && aws.BoolValue(input.EnablePerformanceInsights) != rec.EnablePerformanceInsights {
|
||||
plan.EnablePerformanceInsights = input.EnablePerformanceInsights
|
||||
}
|
||||
if err := s.planBackupSettings(input, rec, plan); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -338,6 +355,15 @@ func (s *Service) applyImmediateModify(ctx context.Context, kv jetstream.KeyValu
|
||||
if plan.AutoMinorVersionUpgrade != nil {
|
||||
stored.AutoMinorVersionUpgrade = *plan.AutoMinorVersionUpgrade
|
||||
}
|
||||
if plan.CopyTagsToSnapshot != nil {
|
||||
stored.CopyTagsToSnapshot = *plan.CopyTagsToSnapshot
|
||||
}
|
||||
if plan.MonitoringInterval != nil {
|
||||
stored.MonitoringInterval = *plan.MonitoringInterval
|
||||
}
|
||||
if plan.EnablePerformanceInsights != nil {
|
||||
stored.EnablePerformanceInsights = *plan.EnablePerformanceInsights
|
||||
}
|
||||
if plan.BackupRetentionPeriod != nil {
|
||||
stored.BackupRetentionPeriod = *plan.BackupRetentionPeriod
|
||||
}
|
||||
|
||||
@@ -120,6 +120,10 @@ func TestModifyDBInstance_AppliesTheNonDisruptiveSettingsAtOnce(t *testing.T) {
|
||||
in := modifyInput()
|
||||
in.MasterUserPassword = aws.String("N3w-Sup3rSecret!")
|
||||
in.DeletionProtection = aws.Bool(true)
|
||||
in.AutoMinorVersionUpgrade = aws.Bool(true)
|
||||
in.CopyTagsToSnapshot = aws.Bool(true)
|
||||
in.MonitoringInterval = aws.Int64(60)
|
||||
in.EnablePerformanceInsights = aws.Bool(true)
|
||||
in.BackupRetentionPeriod = aws.Int64(7)
|
||||
in.PreferredBackupWindow = aws.String("03:00-04:00")
|
||||
in.PreferredMaintenanceWindow = aws.String("sun:05:00-sun:06:00")
|
||||
@@ -138,11 +142,18 @@ func TestModifyDBInstance_AppliesTheNonDisruptiveSettingsAtOnce(t *testing.T) {
|
||||
|
||||
rec := h.record(t)
|
||||
assert.True(t, rec.DeletionProtection)
|
||||
assert.True(t, rec.AutoMinorVersionUpgrade)
|
||||
assert.True(t, rec.CopyTagsToSnapshot)
|
||||
assert.Equal(t, int64(60), rec.MonitoringInterval)
|
||||
assert.True(t, rec.EnablePerformanceInsights)
|
||||
assert.Equal(t, int64(7), rec.BackupRetentionPeriod)
|
||||
assert.Equal(t, "03:00-04:00", rec.PreferredBackupWindow)
|
||||
assert.Equal(t, "sun:05:00-sun:06:00", rec.PreferredMaintenanceWindow)
|
||||
assert.NotNil(t, rec.MasterPasswordUpdatedAt)
|
||||
assert.Nil(t, rec.PendingModifiedValues)
|
||||
assert.True(t, aws.BoolValue(out.DBInstance.CopyTagsToSnapshot))
|
||||
assert.Equal(t, int64(60), aws.Int64Value(out.DBInstance.MonitoringInterval))
|
||||
assert.True(t, aws.BoolValue(out.DBInstance.PerformanceInsightsEnabled))
|
||||
}
|
||||
|
||||
// Only the fact of the rotation is recorded. A password that reached KV
|
||||
@@ -406,15 +417,19 @@ func TestModifyDBInstance_DefersADisruptiveChangeToTheMaintenanceWindow(t *testi
|
||||
in := modifyInput()
|
||||
in.AllocatedStorage = aws.Int64(50)
|
||||
in.DBInstanceClass = aws.String("db.m5.large")
|
||||
in.CopyTagsToSnapshot = aws.Bool(true)
|
||||
out, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, string(StatusAvailable), aws.StringValue(out.DBInstance.DBInstanceStatus))
|
||||
assert.True(t, aws.BoolValue(out.DBInstance.CopyTagsToSnapshot),
|
||||
"the response must include immediate fields applied alongside a deferred change")
|
||||
assert.Empty(t, h.cmdr.calls)
|
||||
assert.Empty(t, h.storage.modified)
|
||||
assert.Empty(t, h.launch.launcher.terminated)
|
||||
|
||||
rec := h.record(t)
|
||||
assert.True(t, rec.CopyTagsToSnapshot)
|
||||
require.NotNil(t, rec.PendingModifiedValues)
|
||||
assert.Equal(t, int64(50), aws.Int64Value(rec.PendingModifiedValues.AllocatedStorage))
|
||||
assert.Equal(t, "db.m5.large", rec.PendingModifiedValues.DBInstanceClass)
|
||||
|
||||
@@ -222,14 +222,15 @@ func (s *Service) DescribeDBParameters(ctx context.Context, input *rds.DescribeD
|
||||
out := &rds.DescribeDBParametersOutput{}
|
||||
for _, param := range CatalogParameterNames() {
|
||||
spec, _ := LookupParameter(param)
|
||||
value, isOverride := overrides[param]
|
||||
override, isOverride := overrides[param]
|
||||
value := override.Value
|
||||
if !isOverride {
|
||||
value = spec.DefaultAt(memoryMiB)
|
||||
}
|
||||
if source != "" && source != parameterSource(isOverride) {
|
||||
continue
|
||||
}
|
||||
out.Parameters = append(out.Parameters, projectParameter(spec, value, isOverride))
|
||||
out.Parameters = append(out.Parameters, projectParameter(spec, value, override.ApplyMethod, isOverride))
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
@@ -421,7 +422,7 @@ func parameterSource(isOverride bool) string {
|
||||
|
||||
// A computed default is reported as engine-default with its literal value, never
|
||||
// as the formula that produced it.
|
||||
func projectParameter(spec ParameterSpec, value string, isOverride bool) *rds.Parameter {
|
||||
func projectParameter(spec ParameterSpec, value, storedApplyMethod string, isOverride bool) *rds.Parameter {
|
||||
out := &rds.Parameter{
|
||||
ParameterName: aws.String(spec.Name),
|
||||
ParameterValue: aws.String(value),
|
||||
@@ -434,9 +435,11 @@ func projectParameter(spec ParameterSpec, value string, isOverride bool) *rds.Pa
|
||||
if allowed := spec.AllowedValues(); allowed != "" {
|
||||
out.AllowedValues = aws.String(allowed)
|
||||
}
|
||||
// AWS reports the method a change to this parameter would take, which for a
|
||||
// static setting is the only one it can take.
|
||||
if spec.ApplyType == ApplyTypeStatic {
|
||||
// An override reports the method the customer stored. The fallback preserves
|
||||
// defaults and records written before ApplyMethod was persisted.
|
||||
if storedApplyMethod != "" {
|
||||
out.ApplyMethod = aws.String(storedApplyMethod)
|
||||
} else if spec.ApplyType == ApplyTypeStatic {
|
||||
out.ApplyMethod = aws.String(ApplyMethodPendingReboot)
|
||||
} else {
|
||||
out.ApplyMethod = aws.String(ApplyMethodImmediate)
|
||||
@@ -468,5 +471,9 @@ func (s *Service) resolveGroupParameters(ctx context.Context, kv jetstream.KeyVa
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return ResolveEffectiveParameters(instanceClass, overrides)
|
||||
values := make(map[string]string, len(overrides))
|
||||
for name, override := range overrides {
|
||||
values[name] = override.Value
|
||||
}
|
||||
return ResolveEffectiveParameters(instanceClass, values)
|
||||
}
|
||||
|
||||
@@ -177,7 +177,7 @@ func TestModifyDBParameterGroup_StoresValidatedOverrides(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
|
||||
out, err := h.svc.ModifyDBParameterGroup(t.Context(), modifyParameters(testParameterGroup,
|
||||
parameter("work_mem", "16384", ""),
|
||||
parameter("work_mem", "16384", ApplyMethodPendingReboot),
|
||||
parameter("shared_buffers", "65536", ApplyMethodPendingReboot),
|
||||
), testAccountID)
|
||||
require.NoError(t, err)
|
||||
@@ -186,6 +186,8 @@ func TestModifyDBParameterGroup_StoresValidatedOverrides(t *testing.T) {
|
||||
params := describedParameters(t, h, testParameterGroup)
|
||||
assert.Equal(t, "16384", aws.StringValue(params["work_mem"].ParameterValue))
|
||||
assert.Equal(t, ParameterSourceUser, aws.StringValue(params["work_mem"].Source))
|
||||
assert.Equal(t, ApplyMethodPendingReboot, aws.StringValue(params["work_mem"].ApplyMethod),
|
||||
"a dynamic parameter must report the customer's stored method")
|
||||
assert.Equal(t, "65536", aws.StringValue(params["shared_buffers"].ParameterValue))
|
||||
assert.Equal(t, ParameterSourceUser, aws.StringValue(params["shared_buffers"].Source))
|
||||
// Untouched parameters stay engine defaults rather than becoming user values.
|
||||
@@ -290,6 +292,24 @@ func TestDescribeDBParameters_ReportsComputedDefaultsAsLiterals(t *testing.T) {
|
||||
assert.NotEmpty(t, aws.StringValue(shared.AllowedValues))
|
||||
}
|
||||
|
||||
func TestDescribeDBParameters_DerivesApplyMethodForLegacyOverrides(t *testing.T) {
|
||||
h := newCreateHarness(t, testBaseDomain)
|
||||
_, err := h.svc.CreateDBParameterGroup(t.Context(), parameterGroupInput(testParameterGroup), testAccountID)
|
||||
require.NoError(t, err)
|
||||
kv, err := h.svc.bucket(t.Context(), testAccountID)
|
||||
require.NoError(t, err)
|
||||
for _, rec := range []DBParameterRecord{
|
||||
{Name: "work_mem", Value: "16384"},
|
||||
{Name: "shared_buffers", Value: "65536"},
|
||||
} {
|
||||
require.NoError(t, putJSON(t.Context(), kv, DBParameterGroupParamKey(testParameterGroup, rec.Name), &rec))
|
||||
}
|
||||
|
||||
params := describedParameters(t, h, testParameterGroup)
|
||||
assert.Equal(t, ApplyMethodImmediate, aws.StringValue(params["work_mem"].ApplyMethod))
|
||||
assert.Equal(t, ApplyMethodPendingReboot, aws.StringValue(params["shared_buffers"].ApplyMethod))
|
||||
}
|
||||
|
||||
func TestDescribeDBParameters_FiltersOnSource(t *testing.T) {
|
||||
h := newCreateHarness(t, testBaseDomain)
|
||||
_, err := h.svc.CreateDBParameterGroup(t.Context(), parameterGroupInput(testParameterGroup), testAccountID)
|
||||
|
||||
@@ -31,11 +31,12 @@ type DBInstanceRecord struct {
|
||||
// Blocks DeleteDBInstance outright. Settable at create and modify.
|
||||
DeletionProtection bool `json:"deletionProtection,omitempty"`
|
||||
|
||||
// Inert — the engine version is pinned, so nothing upgrades either way.
|
||||
// Recorded anyway because a describe that does not echo it back reads as
|
||||
// false against a Terraform schema that defaults it to true, and the plan
|
||||
// then never settles on a change no modify can deliver.
|
||||
AutoMinorVersionUpgrade bool `json:"autoMinorVersionUpgrade"`
|
||||
// Accepted inert settings are recorded so describe echoes what the client set.
|
||||
// Dropping them would leave Terraform planning changes no modify can deliver.
|
||||
AutoMinorVersionUpgrade bool `json:"autoMinorVersionUpgrade"`
|
||||
CopyTagsToSnapshot bool `json:"copyTagsToSnapshot"`
|
||||
MonitoringInterval int64 `json:"monitoringInterval"`
|
||||
EnablePerformanceInsights bool `json:"enablePerformanceInsights"`
|
||||
|
||||
// The backup policy in force. Both windows are stored in AWS's canonical text
|
||||
// so a describe reads back the string a later modify compares against; an
|
||||
|
||||
@@ -240,6 +240,7 @@ func (s *Service) resolveRestoreRequest(input *rds.RestoreDBInstanceFromDBSnapsh
|
||||
DeletionProtection: aws.BoolValue(input.DeletionProtection),
|
||||
|
||||
AutoMinorVersionUpgrade: input.AutoMinorVersionUpgrade == nil || aws.BoolValue(input.AutoMinorVersionUpgrade),
|
||||
CopyTagsToSnapshot: aws.BoolValue(input.CopyTagsToSnapshot),
|
||||
BackupRetentionPeriod: s.defaultRetentionDays(),
|
||||
|
||||
Tags: tagMap,
|
||||
|
||||
@@ -159,15 +159,18 @@ func TestRestoreDBInstanceFromDBSnapshot_HonoursTheRequestedOverrides(t *testing
|
||||
input.DBInstanceClass = aws.String("db.t3.large")
|
||||
input.AllocatedStorage = aws.Int64(50)
|
||||
input.Port = aws.Int64(6432)
|
||||
input.CopyTagsToSnapshot = aws.Bool(true)
|
||||
input.Tags = []*rds.Tag{{Key: aws.String("env"), Value: aws.String("staging")}}
|
||||
|
||||
_, err := h.svc.RestoreDBInstanceFromDBSnapshot(t.Context(), input, testAccountID)
|
||||
out, err := h.svc.RestoreDBInstanceFromDBSnapshot(t.Context(), input, testAccountID)
|
||||
require.NoError(t, err)
|
||||
|
||||
stored := h.instance(t, testRestoredID)
|
||||
assert.Equal(t, "db.t3.large", stored.DBInstanceClass)
|
||||
assert.Equal(t, int64(50), stored.AllocatedStorage)
|
||||
assert.Equal(t, int64(6432), stored.Port)
|
||||
assert.True(t, stored.CopyTagsToSnapshot)
|
||||
assert.True(t, aws.BoolValue(out.DBInstance.CopyTagsToSnapshot))
|
||||
assert.Equal(t, map[string]string{"env": "staging"}, stored.Tags)
|
||||
assert.Equal(t, int64(50), aws.Int64Value(h.launch.volumes.created[0].Size))
|
||||
}
|
||||
|
||||
@@ -291,13 +291,13 @@ func ListDBParameterGroupNames(ctx context.Context, kv jetstream.KeyValue) ([]st
|
||||
}
|
||||
|
||||
// The stored overrides of one parameter group, keyed by parameter name.
|
||||
func ListDBParameterOverrides(ctx context.Context, kv jetstream.KeyValue, group string) (map[string]string, error) {
|
||||
func ListDBParameterOverrides(ctx context.Context, kv jetstream.KeyValue, group string) (map[string]DBParameterRecord, error) {
|
||||
prefix := DBParameterGroupParamsPrefix(group)
|
||||
names, err := listNames(ctx, kv, prefix)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out := make(map[string]string, len(names))
|
||||
out := make(map[string]DBParameterRecord, len(names))
|
||||
for _, name := range names {
|
||||
var rec DBParameterRecord
|
||||
found, err := getJSON(ctx, kv, prefix+name, &rec)
|
||||
@@ -307,7 +307,7 @@ func ListDBParameterOverrides(ctx context.Context, kv jetstream.KeyValue, group
|
||||
// A parameter reset between the listing and this read is simply gone,
|
||||
// which is the answer a resolve one tick later would give too.
|
||||
if found {
|
||||
out[name] = rec.Value
|
||||
out[name] = rec
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws"
|
||||
"github.com/aws/aws-sdk-go/service/rds"
|
||||
@@ -231,6 +232,24 @@ func TestTagActions_MissingSnapshotIsRejected(t *testing.T) {
|
||||
assert.Contains(t, err.Error(), awserrors.ErrorDBSnapshotNotFound)
|
||||
}
|
||||
|
||||
func TestListTagsForResource_AcceptsAnAutomatedSnapshotARN(t *testing.T) {
|
||||
h := newCreateHarness(t, "")
|
||||
id := AutomatedSnapshotIdentifier(testDBInstanceID, time.Date(2026, 7, 24, 3, 4, 0, 0, time.UTC))
|
||||
kv, err := h.svc.bucket(t.Context(), testAccountID)
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, putJSON(t.Context(), kv, DBSnapshotKey(id), &DBSnapshotRecord{
|
||||
DBSnapshotIdentifier: id,
|
||||
AccountID: testAccountID,
|
||||
Tags: map[string]string{"retention": "automated"},
|
||||
}))
|
||||
|
||||
out, err := h.svc.ListTagsForResource(t.Context(), &rds.ListTagsForResourceInput{
|
||||
ResourceName: aws.String(DBSnapshotARN(testRegion, testAccountID, id)),
|
||||
}, testAccountID)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, awsTags("retention", "automated"), out.TagList)
|
||||
}
|
||||
|
||||
func TestTagActions_ForeignAccountARNIsRejected(t *testing.T) {
|
||||
h := newCreateHarness(t, "")
|
||||
seedCreated(t, h, testDBInstanceID)
|
||||
|
||||
@@ -52,7 +52,10 @@ type validatedCreate struct {
|
||||
// engine version is pinned, so nothing upgrades either way. AWS defaults it
|
||||
// to true and so does the Terraform provider, so an unset request must not
|
||||
// read back as false.
|
||||
AutoMinorVersionUpgrade bool
|
||||
AutoMinorVersionUpgrade bool
|
||||
CopyTagsToSnapshot bool
|
||||
MonitoringInterval int64
|
||||
EnablePerformanceInsights bool
|
||||
// Automated backup settings in force. Create assigns unnamed windows, while
|
||||
// restore leaves them empty for lazy deterministic assignment.
|
||||
BackupRetentionPeriod int64
|
||||
@@ -171,7 +174,10 @@ func (s *Service) validateCreateRequest(input *rds.CreateDBInstanceInput) (*vali
|
||||
DBParameterGroupName: paramGroup,
|
||||
DeletionProtection: aws.BoolValue(input.DeletionProtection),
|
||||
|
||||
AutoMinorVersionUpgrade: input.AutoMinorVersionUpgrade == nil || aws.BoolValue(input.AutoMinorVersionUpgrade),
|
||||
AutoMinorVersionUpgrade: input.AutoMinorVersionUpgrade == nil || aws.BoolValue(input.AutoMinorVersionUpgrade),
|
||||
CopyTagsToSnapshot: aws.BoolValue(input.CopyTagsToSnapshot),
|
||||
MonitoringInterval: aws.Int64Value(input.MonitoringInterval),
|
||||
EnablePerformanceInsights: aws.BoolValue(input.EnablePerformanceInsights),
|
||||
|
||||
BackupRetentionPeriod: retention,
|
||||
PreferredBackupWindow: backupWindow,
|
||||
@@ -211,7 +217,7 @@ func validateDBInstanceIdentifier(id string) error {
|
||||
// drop it. Each rejection below is a parameter whose omission would create a
|
||||
// false safety, security or availability guarantee. Parameters that are merely
|
||||
// inert — AutoMinorVersionUpgrade, Performance Insights, Enhanced Monitoring,
|
||||
// CopyTagsToSnapshot — are deliberately absent and accepted as no-ops.
|
||||
// CopyTagsToSnapshot — are accepted and echoed so clients can converge.
|
||||
func rejectUnimplemented(input *rds.CreateDBInstanceInput) error {
|
||||
if aws.BoolValue(input.MultiAZ) {
|
||||
return unimplemented("MultiAZ", "this platform is single-AZ; a standby would not exist")
|
||||
@@ -231,6 +237,12 @@ func rejectUnimplemented(input *rds.CreateDBInstanceInput) error {
|
||||
if aws.Int64Value(input.Iops) > 0 {
|
||||
return unimplemented("Iops", "provisioned IOPS are not implemented; storage is gp3")
|
||||
}
|
||||
if aws.Int64Value(input.MaxAllocatedStorage) > 0 {
|
||||
return unimplemented("MaxAllocatedStorage", "storage autoscaling is not implemented")
|
||||
}
|
||||
if aws.Int64Value(input.StorageThroughput) > 0 {
|
||||
return unimplemented("StorageThroughput", "provisioned throughput is not implemented; storage is gp3")
|
||||
}
|
||||
if aws.StringValue(input.KmsKeyId) != "" {
|
||||
return unimplemented("KmsKeyId", "storage is encrypted with the cluster key, not a customer-managed one")
|
||||
}
|
||||
|
||||
@@ -83,6 +83,9 @@ func TestAPIMatrix(t *testing.T) {
|
||||
{"MultiAZ", func(in *rds.CreateDBInstanceInput) { in.MultiAZ = aws.Bool(true) }},
|
||||
{"StorageEncryptedFalse", func(in *rds.CreateDBInstanceInput) { in.StorageEncrypted = aws.Bool(false) }},
|
||||
{"EngineVersionOtherThanThePin", func(in *rds.CreateDBInstanceInput) { in.EngineVersion = aws.String("16.4") }},
|
||||
{"MinorEngineVersion", func(in *rds.CreateDBInstanceInput) { in.EngineVersion = aws.String("18.4") }},
|
||||
{"MaxAllocatedStorage", func(in *rds.CreateDBInstanceInput) { in.MaxAllocatedStorage = aws.Int64(100) }},
|
||||
{"StorageThroughput", func(in *rds.CreateDBInstanceInput) { in.StorageThroughput = aws.Int64(250) }},
|
||||
{"IAMDatabaseAuthentication", func(in *rds.CreateDBInstanceInput) {
|
||||
in.EnableIAMDatabaseAuthentication = aws.Bool(true)
|
||||
}},
|
||||
@@ -242,19 +245,26 @@ func TestAPIMatrix(t *testing.T) {
|
||||
require.NotNil(t, created.DBInstance)
|
||||
t.Cleanup(func() { deleteInstance(t, f, probeID) })
|
||||
|
||||
// An inert parameter is accepted and then reported honestly,
|
||||
// so a customer reading the instance back is never told a feature is on when
|
||||
// nothing implements it.
|
||||
t.Run("InertParametersAreAcceptedAndReportedOff", func(t *testing.T) {
|
||||
// Accepted inert parameters are echoed so Terraform does not plan changes no
|
||||
// modify can deliver.
|
||||
t.Run("InertParametersAreAcceptedAndEchoed", func(t *testing.T) {
|
||||
instance := created.DBInstance
|
||||
// The exception, echoed rather than reported off: nothing upgrades a pinned
|
||||
// version, but the provider's schema defaults it to true, so a false
|
||||
// read-back would leave every default configuration with an uncleanable diff.
|
||||
assert.True(t, aws.BoolValue(instance.AutoMinorVersionUpgrade),
|
||||
"a client that set it has to read back what it set")
|
||||
assert.True(t, aws.BoolValue(instance.AutoMinorVersionUpgrade))
|
||||
assert.True(t, aws.BoolValue(instance.PerformanceInsightsEnabled))
|
||||
assert.Equal(t, int64(60), aws.Int64Value(instance.MonitoringInterval))
|
||||
assert.True(t, aws.BoolValue(instance.CopyTagsToSnapshot))
|
||||
|
||||
_, err := f.AWS.RDS.ModifyDBInstance(&rds.ModifyDBInstanceInput{
|
||||
DBInstanceIdentifier: aws.String(probeID),
|
||||
EnablePerformanceInsights: aws.Bool(false),
|
||||
MonitoringInterval: aws.Int64(0),
|
||||
CopyTagsToSnapshot: aws.Bool(false),
|
||||
})
|
||||
require.NoError(t, err)
|
||||
instance, err = harness.DescribeDBInstance(f.AWS, probeID)
|
||||
require.NoError(t, err)
|
||||
assert.False(t, aws.BoolValue(instance.PerformanceInsightsEnabled))
|
||||
assert.Zero(t, aws.Int64Value(instance.MonitoringInterval),
|
||||
"enhanced monitoring is not implemented, so no interval is in effect")
|
||||
assert.Zero(t, aws.Int64Value(instance.MonitoringInterval))
|
||||
assert.False(t, aws.BoolValue(instance.CopyTagsToSnapshot))
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user