From 0051fb59735669a8f84b1729b8e18b2a91d4f7d7 Mon Sep 17 00:00:00 2001 From: james00012 Date: Tue, 7 Oct 2025 23:40:02 -0400 Subject: [PATCH 1/2] fix(rds): Skip deletion protection modification for Aurora cluster members - Skip deletion protection changes for RDS cluster members to avoid AWS API errors - Add comprehensive tests for cluster member deletion protection handling - Fix linting issues --- aws/resources/rds.go | 30 +++++++++++------ aws/resources/rds_test.go | 68 +++++++++++++++++++++++++++++++++------ 2 files changed, 79 insertions(+), 19 deletions(-) diff --git a/aws/resources/rds.go b/aws/resources/rds.go index d8620f80..c22fac86 100644 --- a/aws/resources/rds.go +++ b/aws/resources/rds.go @@ -22,20 +22,18 @@ func (di *DBInstances) getAll(ctx context.Context, configObj config.Config) ([]* var names []*string for _, database := range result.DBInstances { - // instance is deletion-protected while config object doesn't include deletion-protected + // Skip deletion-protected instances when config doesn't explicitly include them if database.DeletionProtection != nil && *database.DeletionProtection && !configObj.DBInstances.IncludeDeletionProtected { continue } - if !configObj.DBInstances.ShouldInclude(config.ResourceValue{ + if configObj.DBInstances.ShouldInclude(config.ResourceValue{ Time: database.InstanceCreateTime, Name: database.DBInstanceIdentifier, Tags: util.ConvertRDSTypeTagsToMap(database.TagList), }) { - continue + names = append(names, database.DBInstanceIdentifier) } - - names = append(names, database.DBInstanceIdentifier) } return names, nil @@ -51,14 +49,26 @@ func (di *DBInstances) nukeAll(names []*string) error { deletedNames := []*string{} for _, name := range names { - // Disable deletion protection - _, err := di.Client.ModifyDBInstance(context.TODO(), &rds.ModifyDBInstanceInput{ + // Check if instance is part of a cluster before trying to disable deletion protection + describeResp, err := di.Client.DescribeDBInstances(di.Context, &rds.DescribeDBInstancesInput{ DBInstanceIdentifier: name, - DeletionProtection: aws.Bool(false), - ApplyImmediately: aws.Bool(true), }) if err != nil { - logging.Warnf("[Failed] to disable deletion protection for %s: %s", *name, err) + logging.Warnf("[Failed] to describe instance %s: %s", *name, err) + continue + } + // Only disable deletion protection if instance is not part of a cluster + if len(describeResp.DBInstances) > 0 && describeResp.DBInstances[0].DBClusterIdentifier == nil { + _, modifyErr := di.Client.ModifyDBInstance(context.TODO(), &rds.ModifyDBInstanceInput{ + DBInstanceIdentifier: name, + DeletionProtection: aws.Bool(false), + ApplyImmediately: aws.Bool(true), + }) + if modifyErr != nil { + logging.Warnf("[Failed] to disable deletion protection for %s: %s", *name, modifyErr) + } + } else { + logging.Debugf("Skipping deletion protection modification for cluster member instance %s", *name) } params := &rds.DeleteDBInstanceInput{ diff --git a/aws/resources/rds_test.go b/aws/resources/rds_test.go index a242e9f5..88196628 100644 --- a/aws/resources/rds_test.go +++ b/aws/resources/rds_test.go @@ -20,9 +20,14 @@ type mockedDBInstance struct { RDSAPI DescribeDBInstancesOutput rds.DescribeDBInstancesOutput DeleteDBInstanceOutput rds.DeleteDBInstanceOutput + ModifyCallExpected bool + InstancesDeleted map[string]bool } func (m mockedDBInstance) ModifyDBInstance(ctx context.Context, params *rds.ModifyDBInstanceInput, optFns ...func(*rds.Options)) (*rds.ModifyDBInstanceOutput, error) { + if !m.ModifyCallExpected { + assert.Fail(m.t, "ModifyDBInstance should not be called for cluster member instances") + } assert.NotNil(m.t, params) assert.NotEmpty(m.t, *params.DBInstanceIdentifier) assert.False(m.t, *params.DeletionProtection) @@ -31,10 +36,20 @@ func (m mockedDBInstance) ModifyDBInstance(ctx context.Context, params *rds.Modi } func (m mockedDBInstance) DescribeDBInstances(ctx context.Context, params *rds.DescribeDBInstancesInput, optFns ...func(*rds.Options)) (*rds.DescribeDBInstancesOutput, error) { + // If specific instance is requested and it's been deleted, return empty result + if params.DBInstanceIdentifier != nil { + if m.InstancesDeleted != nil && m.InstancesDeleted[*params.DBInstanceIdentifier] { + return &rds.DescribeDBInstancesOutput{DBInstances: []types.DBInstance{}}, nil + } + } return &m.DescribeDBInstancesOutput, nil } func (m mockedDBInstance) DeleteDBInstance(ctx context.Context, params *rds.DeleteDBInstanceInput, optFns ...func(*rds.Options)) (*rds.DeleteDBInstanceOutput, error) { + // Mark instance as deleted for waiter + if m.InstancesDeleted != nil && params.DBInstanceIdentifier != nil { + m.InstancesDeleted[*params.DBInstanceIdentifier] = true + } return &m.DeleteDBInstanceOutput, nil } @@ -128,14 +143,49 @@ func TestDBInstances_NukeAll(t *testing.T) { t.Parallel() - di := DBInstances{ - Client: mockedDBInstance{ - t: t, - DeleteDBInstanceOutput: rds.DeleteDBInstanceOutput{}, - }, - } - di.Context = context.Background() + t.Run("standalone instance", func(t *testing.T) { + di := DBInstances{ + Client: mockedDBInstance{ + t: t, + DescribeDBInstancesOutput: rds.DescribeDBInstancesOutput{ + DBInstances: []types.DBInstance{ + { + DBInstanceIdentifier: aws.String("test-standalone"), + DBClusterIdentifier: nil, // Not part of a cluster + }, + }, + }, + DeleteDBInstanceOutput: rds.DeleteDBInstanceOutput{}, + ModifyCallExpected: true, // Should call ModifyDBInstance + InstancesDeleted: make(map[string]bool), // Track deleted instances + }, + } + di.Context = context.Background() + + err := di.nukeAll([]*string{aws.String("test-standalone")}) + require.NoError(t, err) + }) + + t.Run("cluster member instance", func(t *testing.T) { + di := DBInstances{ + Client: mockedDBInstance{ + t: t, + DescribeDBInstancesOutput: rds.DescribeDBInstancesOutput{ + DBInstances: []types.DBInstance{ + { + DBInstanceIdentifier: aws.String("test-cluster-member"), + DBClusterIdentifier: aws.String("my-aurora-cluster"), // Part of a cluster + }, + }, + }, + DeleteDBInstanceOutput: rds.DeleteDBInstanceOutput{}, + ModifyCallExpected: false, // Should NOT call ModifyDBInstance + InstancesDeleted: make(map[string]bool), // Track deleted instances + }, + } + di.Context = context.Background() - err := di.nukeAll([]*string{aws.String("test")}) - require.NoError(t, err) + err := di.nukeAll([]*string{aws.String("test-cluster-member")}) + require.NoError(t, err) + }) } From 4f4826d760d0c2b2048a642d4d1236bc1d431a92 Mon Sep 17 00:00:00 2001 From: james00012 Date: Sat, 11 Oct 2025 16:03:02 -0400 Subject: [PATCH 2/2] style: Apply gofmt -s formatting to rds_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix comment alignment in test file to comply with gofmt -s rules. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- aws/resources/rds_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/aws/resources/rds_test.go b/aws/resources/rds_test.go index 88196628..e1a7147b 100644 --- a/aws/resources/rds_test.go +++ b/aws/resources/rds_test.go @@ -156,7 +156,7 @@ func TestDBInstances_NukeAll(t *testing.T) { }, }, DeleteDBInstanceOutput: rds.DeleteDBInstanceOutput{}, - ModifyCallExpected: true, // Should call ModifyDBInstance + ModifyCallExpected: true, // Should call ModifyDBInstance InstancesDeleted: make(map[string]bool), // Track deleted instances }, } @@ -179,7 +179,7 @@ func TestDBInstances_NukeAll(t *testing.T) { }, }, DeleteDBInstanceOutput: rds.DeleteDBInstanceOutput{}, - ModifyCallExpected: false, // Should NOT call ModifyDBInstance + ModifyCallExpected: false, // Should NOT call ModifyDBInstance InstancesDeleted: make(map[string]bool), // Track deleted instances }, }