fix: retain automated backups after RDS restore (#727)

spinifex/handlers/rds/backup_test.go

@@ -58,10 +58,15 @@ func instanceRevision(t *testing.T, svc *Service, id string) (*DBInstanceRecord,
 // Runs one backup pass the way the reconciler does: re-read at its revision,
 // then fire.
 func (h *snapshotHarness) runBackupPass(t *testing.T) bool {
+	t.Helper()
+	return h.runBackupPassFor(t, testDBID)
+}
+
+func (h *snapshotHarness) runBackupPassFor(t *testing.T, id string) bool {
 	t.Helper()
 	kv, err := h.svc.bucket(t.Context(), testAccountID)
 	require.NoError(t, err)
-	rec, rev := instanceRevision(t, h.svc, testDBID)
+	rec, rev := instanceRevision(t, h.svc, id)
 	fired, err := h.svc.runBackupWindow(t.Context(), kv, rev, testAccountID, rec)
 	require.NoError(t, err)
 	return fired

spinifex/handlers/rds/restore.go

@@ -51,7 +51,7 @@ func (s *Service) RestoreDBInstanceFromDBSnapshot(ctx context.Context, input *rd
 			snapshot.DBSnapshotIdentifier, snapshot.Status, SnapshotStatusAvailable)
 	}
 
-	req, err := resolveRestoreRequest(input, snapshot)
+	req, err := s.resolveRestoreRequest(input, snapshot)
 	if err != nil {
 		return nil, err
 	}
@@ -161,10 +161,9 @@ func (s *Service) RestoreDBInstanceFromDBSnapshot(ctx context.Context, input *rd
 	return &rds.RestoreDBInstanceFromDBSnapshotOutput{DBInstance: s.projectDBInstance(stored)}, nil
 }
 
-// Every field the request left out comes from the snapshot's own recorded
-// configuration, so a bare restore reproduces the instance the snapshot was
-// taken from rather than defaulting away from it.
-func resolveRestoreRequest(input *rds.RestoreDBInstanceFromDBSnapshotInput, snapshot *DBSnapshotRecord) (*validatedCreate, error) {
+// Request-overridable configuration comes from the snapshot when omitted.
+// Platform-owned settings use the defaults in force when the restore runs.
+func (s *Service) resolveRestoreRequest(input *rds.RestoreDBInstanceFromDBSnapshotInput, snapshot *DBSnapshotRecord) (*validatedCreate, error) {
 	// The snapshot's engine, never the request's: the datadir is written in one
 	// engine's on-disk format and no other can read it.
 	engine, err := LookupEngine(snapshot.Engine)
@@ -244,6 +243,7 @@ func resolveRestoreRequest(input *rds.RestoreDBInstanceFromDBSnapshotInput, snap
 		DeletionProtection:   aws.BoolValue(input.DeletionProtection),
 
 		AutoMinorVersionUpgrade: input.AutoMinorVersionUpgrade == nil || aws.BoolValue(input.AutoMinorVersionUpgrade),
+		BackupRetentionPeriod:   s.defaultRetentionDays(),
 
 		Tags: tagMap,
 	}, nil

spinifex/handlers/rds/restore_test.go

@@ -3,6 +3,7 @@ package handlers_rds
 import (
 	"errors"
 	"testing"
+	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/rds"
@@ -87,8 +88,8 @@ func TestRestoreDBInstanceFromDBSnapshot_IAMFailurePrecedesReservationAndVolume(
 	assert.Nil(t, h.launch.launcher.input)
 }
 
-// Everything the request left out comes from the snapshot, so a bare restore
-// reproduces the instance it was taken from rather than defaulting away from it.
+// Source-instance configuration comes from the snapshot when the restore
+// request does not override it; platform-owned settings use current defaults.
 func TestRestoreDBInstanceFromDBSnapshot_FallsBackToTheSnapshotsConfiguration(t *testing.T) {
 	h := newSnapshotHarness(t, false)
 	h.seedSnapshot(t)
@@ -107,6 +108,49 @@ func TestRestoreDBInstanceFromDBSnapshot_FallsBackToTheSnapshotsConfiguration(t
 	assert.Equal(t, []string{testDefaultSG}, stored.VpcSecurityGroupIDs)
 }
 
+func TestRestoreDBInstanceFromDBSnapshot_DefaultsRetentionAndTakesAnAutomatedBackup(t *testing.T) {
+	h := newSnapshotHarness(t, false)
+	now := time.Now().UTC()
+	h.svc.deps.Backup = BackupPolicy{
+		RetentionDays:     3,
+		BackupWindowBlock: openBackupWindow(now),
+	}
+	h.seedSnapshot(t)
+
+	_, err := h.svc.RestoreDBInstanceFromDBSnapshot(t.Context(), restoreInput(), testAccountID)
+	require.NoError(t, err)
+
+	stored := h.instance(t, testRestoredID)
+	assert.Equal(t, int64(3), h.svc.defaultRetentionDays())
+	assert.Equal(t, int64(3), stored.BackupRetentionPeriod)
+	assert.Empty(t, stored.PreferredBackupWindow)
+	assert.Empty(t, stored.PreferredMaintenanceWindow)
+	window, err := h.svc.resolvedBackupWindow(&stored)
+	require.NoError(t, err)
+	require.True(t, window.contains(now), "the lazily assigned window should be open")
+
+	newStubAgent(t, h.nc, testAccountID, testRestoredID, false)
+	stored.Status = StatusAvailable
+	seedInstance(t, h.svc, stored)
+	require.True(t, h.runBackupPassFor(t, testRestoredID))
+
+	stamps := h.automatedStamps(t, testRestoredID)
+	require.Len(t, stamps, 1)
+	kv, err := h.svc.bucket(t.Context(), testAccountID)
+	require.NoError(t, err)
+	var entry AutomatedBackupRecord
+	found, err := getJSON(t.Context(), kv, AutomatedBackupKey(testRestoredID, stamps[0]), &entry)
+	require.NoError(t, err)
+	require.True(t, found)
+	assert.Equal(t, testRestoredID, entry.DBInstanceIdentifier)
+	backup, found := h.snapshot(t, entry.DBSnapshotIdentifier)
+	require.True(t, found)
+	assert.Equal(t, testRestoredID, backup.DBInstanceIdentifier)
+	assert.Equal(t, SnapshotTypeAutomated, backup.SnapshotType)
+	assert.Equal(t, SnapshotStatusAvailable, backup.Status)
+	assert.NotNil(t, h.instance(t, testRestoredID).LastAutomatedBackupAt)
+}
+
 func TestRestoreDBInstanceFromDBSnapshot_HonoursTheRequestedOverrides(t *testing.T) {
 	h := newSnapshotHarness(t, false)
 	h.seedSnapshot(t)

spinifex/handlers/rds/validate.go

@@ -53,9 +53,8 @@ type validatedCreate struct {
 	// to true and so does the Terraform provider, so an unset request must not
 	// read back as false.
 	AutoMinorVersionUpgrade bool
-	// Automated backups as they will be in force: the retention defaulted when the
-	// request names none, and both windows in canonical text — assigned from the
-	// instance identifier when unnamed, never left empty.
+	// Automated backup settings in force. Create assigns unnamed windows, while
+	// restore leaves them empty for lazy deterministic assignment.
 	BackupRetentionPeriod      int64
 	PreferredBackupWindow      string
 	PreferredMaintenanceWindow string