From d9c3f3ca6f9f284be5ba2fc67d27d3c19b2bef90 Mon Sep 17 00:00:00 2001 From: joshsiv-mulga Date: Thu, 8 Oct 2026 21:51:08 +1000 Subject: [PATCH] refactor: extract RDS DB parameter group owner Move the parameter group and override records, keys and listing into domains/rds/parametergroup behind an Owner. Delete asks an owner-declared Dependants interface; instance-side resolution reads through its own parameterGroupValues interface. SetOverrides refuses an unknown group so no override key is written without its group. Stored shape, keys, public errors and propagation are unchanged. --- docs/service-interfaces.yaml | 1 + spinifex/domains/rds/parametergroup/owner.go | 315 ++++++++++++++++++ .../domains/rds/parametergroup/owner_test.go | 165 +++++++++ spinifex/domains/rds/parametergroup/record.go | 60 ++++ spinifex/handlers/rds/create.go | 2 +- spinifex/handlers/rds/modify.go | 6 +- spinifex/handlers/rds/parametergroup.go | 195 ++++------- .../rds/parametergroup_lifecycle_test.go | 9 +- spinifex/handlers/rds/parametergroup_test.go | 11 +- spinifex/handlers/rds/pending.go | 2 +- spinifex/handlers/rds/records.go | 31 -- spinifex/handlers/rds/restore.go | 2 +- spinifex/handlers/rds/store.go | 68 ---- spinifex/handlers/rds/store_test.go | 9 - spinifex/handlers/rds/tags.go | 6 +- 15 files changed, 619 insertions(+), 263 deletions(-) create mode 100644 spinifex/domains/rds/parametergroup/owner.go create mode 100644 spinifex/domains/rds/parametergroup/owner_test.go create mode 100644 spinifex/domains/rds/parametergroup/record.go diff --git a/docs/service-interfaces.yaml b/docs/service-interfaces.yaml index d0b0d7828..bdb6812a9 100644 --- a/docs/service-interfaces.yaml +++ b/docs/service-interfaces.yaml @@ -77,6 +77,7 @@ services: - spinifex/handlers/ecs - spinifex/handlers/rds - spinifex/domains/rds/subnetgroup + - spinifex/domains/rds/parametergroup - spinifex/domains/rds/engine - spinifex/domains/ecs/taskdefinition - spinifex/handlers/eks diff --git a/spinifex/domains/rds/parametergroup/owner.go b/spinifex/domains/rds/parametergroup/owner.go new file mode 100644 index 000000000..abebf7558 --- /dev/null +++ b/spinifex/domains/rds/parametergroup/owner.go @@ -0,0 +1,315 @@ +package parametergroup + +import ( + "context" + "errors" + "fmt" + "log/slog" + "maps" + "slices" + "strings" + "time" + + rdsengine "github.com/mulgadc/spinifex/spinifex/domains/rds/engine" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore" + "github.com/nats-io/nats.go/jetstream" +) + +// Dependants reports the sorted identifiers of the DB instances in the account that name the group, +// current or pending. +type Dependants interface { + InstancesUsing(ctx context.Context, accountID, name string) ([]string, error) +} + +// Bucket opens the account's RDS bucket, which holds the group's records. +type Bucket func(ctx context.Context, accountID string) (*kvstore.Bucket, error) + +// Owner is the sole writer of DB parameter group records and their overrides. +type Owner struct { + dependants Dependants + bucket Bucket +} + +// New returns an Owner. +func New(dependants Dependants, bucket Bucket) *Owner { + return &Owner{dependants: dependants, bucket: bucket} +} + +// Spec is a validated create request. It starts with no overrides: every value is a catalog default +// until SetOverrides stores one, so a fresh group and the default group resolve to the same effective set. +type Spec struct { + Name string + Family string + Description string + Tags map[string]string +} + +// Create stores a new group, or fails with DBParameterGroupAlreadyExists when the name is taken. +func (o *Owner) Create(ctx context.Context, accountID string, spec Spec) (*Record, error) { + kv, err := o.bucket(ctx, accountID) + if err != nil { + return nil, err + } + now := time.Now().UTC() + rec := &Record{ + Name: spec.Name, + AccountID: accountID, + Family: spec.Family, + Description: spec.Description, + Tags: spec.Tags, + CreatedAt: now, + UpdatedAt: now, + } + // Create rather than Set, so two concurrent creates of one name have exactly + // one winner instead of both reporting success over each other's family. + if _, err := kvstore.On[Record](kv).Create(ctx, MetaKey(spec.Name), rec); err != nil { + if errors.Is(err, kvstore.ErrExists) { + return nil, awserrors.Errorf(awserrors.ErrorDBParameterGroupAlreadyExists, + "DB parameter group %s already exists", spec.Name) + } + return nil, err + } + + slog.InfoContext(ctx, "rds: DB parameter group created", + "dbParameterGroup", spec.Name, "accountId", accountID, "family", spec.Family) + return rec, nil +} + +// Get returns the group, including the implicit default group synthesised for an engine that has +// never been created, or DBParameterGroupNotFound. +func (o *Owner) Get(ctx context.Context, accountID, name string) (*Record, error) { + kv, err := o.bucket(ctx, accountID) + if err != nil { + return nil, err + } + rec, _, err := get(ctx, kv, accountID, name) + return rec, err +} + +// List returns every group in the account, including the implicit default group of every registered +// engine, sorted by name and deduplicated: a customer group that happens to carry a default's name +// (which Create refuses) would otherwise be reported twice. +func (o *Owner) List(ctx context.Context, accountID string) ([]Record, error) { + kv, err := o.bucket(ctx, accountID) + if err != nil { + return nil, err + } + names, err := storedNames(ctx, kv) + if err != nil { + return nil, err + } + for _, engineName := range rdsengine.SupportedEngines() { + engine, err := rdsengine.LookupEngine(engineName) + if err != nil { + return nil, err + } + names = append(names, engine.DefaultParameterGroupName()) + } + slices.Sort(names) + names = slices.Compact(names) + + recs := make([]Record, 0, len(names)) + for _, name := range names { + rec, _, err := get(ctx, kv, accountID, name) + if err != nil { + // Deleted between the listing and this read; the same answer a describe + // one tick later would give. + if awserrors.IsErrorCode(err, awserrors.ErrorDBParameterGroupNotFound) { + continue + } + return nil, err + } + recs = append(recs, *rec) + } + return recs, nil +} + +// Overrides returns the stored overrides of one parameter group, keyed by parameter name. +func (o *Owner) Overrides(ctx context.Context, accountID, name string) (map[string]Override, error) { + kv, err := o.bucket(ctx, accountID) + if err != nil { + return nil, err + } + return overrides(ctx, kv, name) +} + +// SetOverrides stores each override at its own key, one write per parameter, so a modify touching one +// setting cannot clobber a concurrent change to another. It refuses a default group, which has no +// stored values of its own, and an unknown group, so no override is written without its group. +func (o *Owner) SetOverrides(ctx context.Context, accountID, name string, values []Override) error { + if _, ok := rdsengine.EngineForDefaultParameterGroup(name); ok { + return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "DB parameter group %s is a default group and cannot be modified", name) + } + kv, err := o.bucket(ctx, accountID) + if err != nil { + return err + } + if _, _, err := get(ctx, kv, accountID, name); err != nil { + return err + } + now := time.Now().UTC() + for _, value := range values { + value.UpdatedAt = now + if err := kvstore.On[Override](kv).Set(ctx, ParamKey(name, value.Name), &value); err != nil { + return err + } + } + return nil +} + +// Delete is refused for a default group, and while any instance still names the group, including one +// that is only deleting or only pending it: releasing it early would let a teardown lose the record of +// its configuration, and would make destroy ordering ambiguous. The values are removed before the +// group's own record, so a crash between the two leaves orphaned parameter keys rather than a group a +// later create could inherit silently. +func (o *Owner) Delete(ctx context.Context, accountID, name string) error { + if _, ok := rdsengine.EngineForDefaultParameterGroup(name); ok { + return awserrors.Errorf(awserrors.ErrorDBParameterGroupInvalidState, + "DB parameter group %s is a default group and cannot be deleted", name) + } + kv, err := o.bucket(ctx, accountID) + if err != nil { + return err + } + if _, _, err := get(ctx, kv, accountID, name); err != nil { + return err + } + users, err := o.dependants.InstancesUsing(ctx, accountID, name) + if err != nil { + return err + } + if len(users) > 0 { + return awserrors.Errorf(awserrors.ErrorDBParameterGroupInvalidState, + "DB parameter group %s is still used by %s", name, strings.Join(users, ", ")) + } + + values, err := overrides(ctx, kv, name) + if err != nil { + return err + } + for _, param := range slices.Sorted(maps.Keys(values)) { + if err := kv.Delete(ctx, ParamKey(name, param)); err != nil { + return fmt.Errorf("rds: delete parameter %s of group %s: %w", param, name, err) + } + } + if err := kv.Delete(ctx, MetaKey(name)); err != nil { + return fmt.Errorf("rds: delete DB parameter group %s: %w", name, err) + } + + slog.InfoContext(ctx, "rds: DB parameter group deleted", "dbParameterGroup", name, "accountId", accountID) + return nil +} + +// The stored record, or the lazily materialised default group. A default group is synthesised rather +// than written on read: the record carries nothing a write would preserve, and materialising it on a +// read would make a read path a writer for no gain. The record plus its revision, so a caller doing a +// CAS update is not forced into a second read. +func get(ctx context.Context, kv *kvstore.Bucket, accountID, name string) (*Record, uint64, error) { + rec, rev, err := kvstore.On[Record](kv).Get(ctx, MetaKey(name)) + if err == nil { + return rec, rev, nil + } + if !errors.Is(err, kvstore.ErrNotFound) { + return nil, 0, err + } + if engine, ok := rdsengine.EngineForDefaultParameterGroup(name); ok { + return defaultRecord(engine, accountID), 0, nil + } + return nil, 0, awserrors.Errorf(awserrors.ErrorDBParameterGroupNotFound, "DB parameter group %s not found", name) +} + +// The implicit group, identical for every account. It carries no tags and no stored values, so it +// resolves to the catalog defaults alone. +func defaultRecord(engine rdsengine.Engine, accountID string) *Record { + return &Record{ + Name: engine.DefaultParameterGroupName(), + AccountID: accountID, + Family: engine.ParameterGroupFamily(), + Description: fmt.Sprintf("Default parameter group for %s", engine.ParameterGroupFamily()), + } +} + +// overrides returns the stored overrides of one group, keyed by parameter name, against an +// already-opened bucket. +func overrides(ctx context.Context, kv *kvstore.Bucket, name string) (map[string]Override, error) { + paramPrefix := ParamsPrefix(name) + names, err := paramNames(ctx, kv, paramPrefix) + if err != nil { + return nil, err + } + out := make(map[string]Override, len(names)) + for _, param := range names { + v, _, err := kvstore.On[Override](kv).Get(ctx, paramPrefix+param) + if errors.Is(err, kvstore.ErrNotFound) { + // Reset between the listing and this read; the same answer a resolve one + // tick later would give. + continue + } + if err != nil { + return nil, err + } + out[param] = *v + } + return out, nil +} + +// storedNames walks the .../meta keys, which is what makes a group's own record findable among the +// per-parameter keys hanging off the same prefix. +func storedNames(ctx context.Context, kv *kvstore.Bucket) ([]string, error) { + keys, err := bucketKeys(ctx, kv) + if err != nil { + return nil, err + } + names := make([]string, 0, len(keys)) + for _, key := range keys { + rest, prefixed := strings.CutPrefix(key, prefix) + name, suffixed := strings.CutSuffix(rest, "/meta") + if !prefixed || !suffixed { + continue + } + if name == "" || strings.Contains(name, "/") { + continue + } + names = append(names, name) + } + return names, nil +} + +// paramNames returns the leaf names directly under paramPrefix. +func paramNames(ctx context.Context, kv *kvstore.Bucket, paramPrefix string) ([]string, error) { + keys, err := bucketKeys(ctx, kv) + if err != nil { + return nil, err + } + names := make([]string, 0, len(keys)) + for _, key := range keys { + name, ok := strings.CutPrefix(key, paramPrefix) + if !ok { + continue + } + if name == "" || strings.Contains(name, "/") { + continue + } + names = append(names, name) + } + return names, nil +} + +// An empty bucket yields no keys rather than an error, so a first-ever list answers with an empty +// result instead of failing. +func bucketKeys(ctx context.Context, b *kvstore.Bucket) ([]string, error) { + kv, err := b.KV(ctx) + if err != nil { + return nil, err + } + keys, err := kv.Keys(ctx) + if err != nil { + if errors.Is(err, jetstream.ErrNoKeysFound) { + return nil, nil + } + return nil, fmt.Errorf("rds: list keys: %w", err) + } + return keys, nil +} diff --git a/spinifex/domains/rds/parametergroup/owner_test.go b/spinifex/domains/rds/parametergroup/owner_test.go new file mode 100644 index 000000000..58e3bccb4 --- /dev/null +++ b/spinifex/domains/rds/parametergroup/owner_test.go @@ -0,0 +1,165 @@ +package parametergroup + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/mulgadc/spinifex/internal/testkit" + rdsengine "github.com/mulgadc/spinifex/spinifex/domains/rds/engine" + "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" + "github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + testAccount = "123456789012" + testGroup = "app-params" +) + +type fakeDependants struct { + users []string + err error +} + +func (f fakeDependants) InstancesUsing(context.Context, string, string) ([]string, error) { + return f.users, f.err +} + +func newOwner(t *testing.T, dependants Dependants) *Owner { + t.Helper() + _, _, js := testutil.StartTestJetStream(t) + b := kvstore.NewBucket(js, kvstore.Config{Name: "rds-account-" + testAccount, History: 1}) + return New(dependants, func(context.Context, string) (*kvstore.Bucket, error) { return b, nil }) +} + +func testFamily(t *testing.T) string { + t.Helper() + engine, err := rdsengine.LookupEngine("postgres") + require.NoError(t, err) + return engine.ParameterGroupFamily() +} + +func TestKeys_SitUnderPrefix(t *testing.T) { + t.Parallel() + assert.Equal(t, "db-parameter-groups/pg16/meta", MetaKey("pg16")) + assert.Equal(t, "db-parameter-groups/pg16/params/shared_buffers", ParamKey("pg16", "shared_buffers")) + assert.True(t, strings.HasPrefix(MetaKey("x"), Prefix())) + assert.True(t, strings.HasPrefix(ParamKey("x", "p"), ParamsPrefix("x"))) + + // A group's values must not be reachable by listing groups' meta records, or + // a list would report one group per parameter. + assert.False(t, strings.HasPrefix(ParamKey("x", "p"), MetaKey("x"))) +} + +func TestCreate_DuplicateNameIsRefusedWithoutOverwrite(t *testing.T) { + t.Parallel() + owner := newOwner(t, fakeDependants{}) + family := testFamily(t) + _, err := owner.Create(t.Context(), testAccount, Spec{Name: testGroup, Family: family, Description: "first"}) + require.NoError(t, err) + + _, err = owner.Create(t.Context(), testAccount, Spec{Name: testGroup, Family: family, Description: "second"}) + assert.Equal(t, awserrors.ErrorDBParameterGroupAlreadyExists, awserrors.ValidErrorCodeFromError(err)) + + rec, err := owner.Get(t.Context(), testAccount, testGroup) + require.NoError(t, err) + assert.Equal(t, "first", rec.Description) +} + +func TestDelete_RefusesWhileDependantsNameTheGroup(t *testing.T) { + t.Parallel() + owner := newOwner(t, fakeDependants{users: []string{"orders-db", "users-db"}}) + family := testFamily(t) + _, err := owner.Create(t.Context(), testAccount, Spec{Name: testGroup, Family: family, Description: "d"}) + require.NoError(t, err) + require.NoError(t, owner.SetOverrides(t.Context(), testAccount, testGroup, + []Override{{Name: "shared_buffers", Value: "512MB"}})) + + err = owner.Delete(t.Context(), testAccount, testGroup) + assert.Equal(t, awserrors.ErrorDBParameterGroupInvalidState, awserrors.ValidErrorCodeFromError(err)) + assert.ErrorContains(t, err, "orders-db, users-db") + + rec, err := owner.Get(t.Context(), testAccount, testGroup) + require.NoError(t, err) + assert.Equal(t, "d", rec.Description) + overrides, err := owner.Overrides(t.Context(), testAccount, testGroup) + require.NoError(t, err) + assert.Len(t, overrides, 1) +} + +// The in-use guard fails closed: when the instance side cannot answer, the +// group and its overrides stay rather than being deleted on an unknown answer. +func TestDelete_KeepsTheGroupWhenTheInUseCheckFails(t *testing.T) { + t.Parallel() + owner := newOwner(t, fakeDependants{err: errors.New("instance scan failed")}) + family := testFamily(t) + _, err := owner.Create(t.Context(), testAccount, Spec{Name: testGroup, Family: family, Description: "d"}) + require.NoError(t, err) + + require.ErrorContains(t, owner.Delete(t.Context(), testAccount, testGroup), "instance scan failed") + _, err = owner.Get(t.Context(), testAccount, testGroup) + require.NoError(t, err) +} + +// A default name is synthesised on read rather than written, so Get never +// materialises a record a later Create would then collide with. +func TestGet_DefaultGroupSynthesisesWithoutWriting(t *testing.T) { + t.Parallel() + owner := newOwner(t, fakeDependants{}) + engine, err := rdsengine.LookupEngine("postgres") + require.NoError(t, err) + name := engine.DefaultParameterGroupName() + + rec, err := owner.Get(t.Context(), testAccount, name) + require.NoError(t, err) + assert.Equal(t, name, rec.Name) + assert.Equal(t, engine.ParameterGroupFamily(), rec.Family) + + kv, err := owner.bucket(t.Context(), testAccount) + require.NoError(t, err) + names, err := storedNames(t.Context(), kv) + require.NoError(t, err) + assert.NotContains(t, names, name, "a synthesised default group must not be written to the bucket") +} + +func TestDelete_RefusesADefaultGroup(t *testing.T) { + t.Parallel() + owner := newOwner(t, fakeDependants{}) + engine, err := rdsengine.LookupEngine("postgres") + require.NoError(t, err) + + err = owner.Delete(t.Context(), testAccount, engine.DefaultParameterGroupName()) + assert.Equal(t, awserrors.ErrorDBParameterGroupInvalidState, awserrors.ValidErrorCodeFromError(err)) +} + +func TestSetOverrides_RefusesADefaultGroup(t *testing.T) { + t.Parallel() + owner := newOwner(t, fakeDependants{}) + engine, err := rdsengine.LookupEngine("postgres") + require.NoError(t, err) + + err = owner.SetOverrides(t.Context(), testAccount, engine.DefaultParameterGroupName(), + []Override{{Name: "shared_buffers", Value: "512MB"}}) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, awserrors.ValidErrorCodeFromError(err)) +} + +func TestSetOverrides_UnknownGroupIsNotFoundAndWritesNothing(t *testing.T) { + t.Parallel() + owner := newOwner(t, fakeDependants{}) + const name = "default.bogus" + + err := owner.SetOverrides(t.Context(), testAccount, name, + []Override{{Name: "work_mem", Value: "16384"}}) + assert.Equal(t, awserrors.ErrorDBParameterGroupNotFound, awserrors.ValidErrorCodeFromError(err)) + + kv, err := owner.bucket(t.Context(), testAccount) + require.NoError(t, err) + keys, err := bucketKeys(t.Context(), kv) + require.NoError(t, err) + for _, key := range keys { + assert.False(t, strings.HasPrefix(key, ParamsPrefix(name)), "override key %s written for an unknown group", key) + } +} diff --git a/spinifex/domains/rds/parametergroup/record.go b/spinifex/domains/rds/parametergroup/record.go new file mode 100644 index 000000000..f03c076d6 --- /dev/null +++ b/spinifex/domains/rds/parametergroup/record.go @@ -0,0 +1,60 @@ +// Package parametergroup owns the RDS DB parameter group: its durable record, the stored overrides +// that sit alongside it, and the in-use guard on its delete. Default groups are never stored; the +// engine catalogue is their only source. +package parametergroup + +import "time" + +const prefix = "db-parameter-groups/" + +// Prefix returns the per-account key prefix under which every DB parameter group's keys live. +// A group's own record is at .../meta and its values hang off .../params/, so listing groups walks +// the meta keys. +func Prefix() string { + return prefix +} + +// MetaKey returns the per-account KV key of a DB parameter group's own record. +func MetaKey(name string) string { + return prefix + name + "/meta" +} + +// ParamsPrefix returns the prefix holding one key per parameter value rather than one blob, so a +// modify touching a single parameter cannot clobber a concurrent change to another. +func ParamsPrefix(name string) string { + return prefix + name + "/params/" +} + +// ParamKey returns the per-account KV key of one parameter override in a DB parameter group. +func ParamKey(name, param string) string { + return ParamsPrefix(name) + param +} + +// Record is the db-parameter-groups/{name}/meta record. The values themselves live one key each +// under .../params/, so a modify touching one parameter cannot clobber a concurrent change to +// another. A default group, synthesised rather than stored, carries no tags and no CreatedAt. +type Record struct { + Name string `json:"name"` + AccountID string `json:"accountId"` + Family string `json:"family"` + Description string `json:"description"` + + Tags map[string]string `json:"tags,omitempty"` + + CreatedAt time.Time `json:"createdAt"` + UpdatedAt time.Time `json:"updatedAt"` +} + +func (r *Record) GetTags() map[string]string { return r.Tags } + +func (r *Record) SetTags(tags map[string]string) { r.Tags = tags } + +// Override is one stored value, at db-parameter-groups/{name}/params/{key}. ApplyMethod is the +// customer's request rather than a fact: whether a change lands live is decided by the parameter's +// own ApplyType. +type Override struct { + Name string `json:"name"` + Value string `json:"value"` + ApplyMethod string `json:"applyMethod,omitempty"` + UpdatedAt time.Time `json:"updatedAt"` +} diff --git a/spinifex/handlers/rds/create.go b/spinifex/handlers/rds/create.go index d51305378..a9aa9b41a 100644 --- a/spinifex/handlers/rds/create.go +++ b/spinifex/handlers/rds/create.go @@ -42,7 +42,7 @@ func (s *Service) CreateDBInstance(ctx context.Context, input *rds.CreateDBInsta // does not exist leaves no record behind. The set is literals only: the // class's memory has already been folded into every size-derived default, so // the agent never sees a formula. - parameters, err := s.resolveGroupParameters(ctx, kv, accountID, req.Engine, req.DBParameterGroupName, req.InstanceClass) + parameters, err := s.resolveGroupParameters(ctx, accountID, req.Engine, req.DBParameterGroupName, req.InstanceClass) if err != nil { return nil, err } diff --git a/spinifex/handlers/rds/modify.go b/spinifex/handlers/rds/modify.go index 0a26612f1..ea33f5259 100644 --- a/spinifex/handlers/rds/modify.go +++ b/spinifex/handlers/rds/modify.go @@ -242,11 +242,7 @@ func (s *Service) planModify(ctx context.Context, input *rds.ModifyDBInstanceInp if err != nil { return nil, err } - kv, err := s.bucket(ctx, accountID) - if err != nil { - return nil, err - } - if _, err := s.resolveGroupParameters(ctx, kv, accountID, engine, targetGroup, targetClass); err != nil { + if _, err := s.resolveGroupParameters(ctx, accountID, engine, targetGroup, targetClass); err != nil { return nil, err } } diff --git a/spinifex/handlers/rds/parametergroup.go b/spinifex/handlers/rds/parametergroup.go index c1289866f..2bb3e9e44 100644 --- a/spinifex/handlers/rds/parametergroup.go +++ b/spinifex/handlers/rds/parametergroup.go @@ -8,11 +8,11 @@ import ( "maps" "slices" "strings" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/rds" rdsengine "github.com/mulgadc/spinifex/spinifex/domains/rds/engine" + "github.com/mulgadc/spinifex/spinifex/domains/rds/parametergroup" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore" ) @@ -69,31 +69,16 @@ func (s *Service) CreateDBParameterGroup(ctx context.Context, input *rds.CreateD return nil, err } - kv, err := s.bucket(ctx, accountID) - if err != nil { - return nil, err - } - now := time.Now().UTC() - rec := DBParameterGroupRecord{ + rec, err := s.parameterGroups().Create(ctx, accountID, parametergroup.Spec{ Name: name, - AccountID: accountID, Family: family, Description: description, Tags: tags, - CreatedAt: now, - UpdatedAt: now, - } - if err := createJSON(ctx, kv, DBParameterGroupMetaKey(name), &rec); err != nil { - if errors.Is(err, kvstore.ErrExists) { - return nil, awserrors.Errorf(awserrors.ErrorDBParameterGroupAlreadyExists, - "DB parameter group %s already exists", name) - } + }) + if err != nil { return nil, err } - - slog.InfoContext(ctx, "rds: DB parameter group created", - "dbParameterGroup", name, "accountId", accountID, "family", family) - return &rds.CreateDBParameterGroupOutput{DBParameterGroup: s.projectParameterGroupRecord(&rec)}, nil + return &rds.CreateDBParameterGroupOutput{DBParameterGroup: s.projectParameterGroupRecord(rec)}, nil } // DescribeDBParameterGroups reports the implicit default group alongside the customer's own, whether @@ -107,12 +92,8 @@ func (s *Service) DescribeDBParameterGroups(ctx context.Context, input *rds.Desc if _, err := ReadFilters(input.Filters, filterDBParameterGroupFamily, filterEngine); err != nil { return nil, err } - kv, err := s.bucket(ctx, accountID) - if err != nil { - return nil, err - } if name := aws.StringValue(input.DBParameterGroupName); name != "" { - rec, _, err := getDBParameterGroup(ctx, kv, accountID, name) + rec, err := s.parameterGroups().Get(ctx, accountID, name) if err != nil { return nil, err } @@ -121,34 +102,15 @@ func (s *Service) DescribeDBParameterGroups(ctx context.Context, input *rds.Desc }, nil } - names, err := ListDBParameterGroupNames(ctx, kv) + // The default group is synthesised rather than read, so it appears exactly + // once whether or not a prior create persisted it. + recs, err := s.parameterGroups().List(ctx, accountID) if err != nil { return nil, err } - // The default group is synthesised rather than read, so it appears exactly - // once whether or not a prior create persisted it. - for _, name := range rdsengine.SupportedEngines() { - engine, err := rdsengine.LookupEngine(name) - if err != nil { - return nil, err - } - names = append(names, engine.DefaultParameterGroupName()) - } - slices.Sort(names) - names = slices.Compact(names) - - groups := make([]*rds.DBParameterGroup, 0, len(names)) - for _, name := range names { - rec, _, err := getDBParameterGroup(ctx, kv, accountID, name) - if err != nil { - // Deleted between the listing and this read; the same answer a describe - // one tick later would give. - if awserrors.IsErrorCode(err, awserrors.ErrorDBParameterGroupNotFound) { - continue - } - return nil, err - } - groups = append(groups, s.projectParameterGroupRecord(rec)) + groups := make([]*rds.DBParameterGroup, 0, len(recs)) + for i := range recs { + groups = append(groups, s.projectParameterGroupRecord(&recs[i])) } groups, next, err := Page(groups, parameterGroupPageKey, input.MaxRecords, input.Marker) if err != nil { @@ -181,15 +143,11 @@ func (s *Service) ModifyDBParameterGroup(ctx context.Context, input *rds.ModifyD "at most %d parameters may be modified in one request, got %d", maxParametersPerModify, len(input.Parameters)) } - kv, err := s.bucket(ctx, accountID) - if err != nil { - return nil, err - } // The group is read before its values are checked because the engine that // owns them comes from the group's family. Validating against anything else // would store one engine's parameter into another's group and defer the // failure to whichever instance next attached it. - rec, _, err := getDBParameterGroup(ctx, kv, accountID, name) + rec, err := s.parameterGroups().Get(ctx, accountID, name) if err != nil { return nil, err } @@ -202,18 +160,18 @@ func (s *Service) ModifyDBParameterGroup(ctx context.Context, input *rds.ModifyD return nil, err } - now := time.Now().UTC() - for _, update := range updates { - if err := putJSON(ctx, kv, DBParameterGroupParamKey(name, update.Name), &DBParameterRecord{ - Name: update.Name, - Value: update.Value, - ApplyMethod: update.ApplyMethod, - UpdatedAt: now, - }); err != nil { - return nil, err - } + values := make([]parametergroup.Override, len(updates)) + for i, update := range updates { + values[i] = parametergroup.Override{Name: update.Name, Value: update.Value, ApplyMethod: update.ApplyMethod} + } + if err := s.parameterGroups().SetOverrides(ctx, accountID, name, values); err != nil { + return nil, err } + kv, err := s.bucket(ctx, accountID) + if err != nil { + return nil, err + } if err := s.propagateParameterGroup(ctx, kv, accountID, name); err != nil { return nil, err } @@ -276,11 +234,7 @@ func (s *Service) DescribeDBParameters(ctx context.Context, input *rds.DescribeD return nil, err } - kv, err := s.bucket(ctx, accountID) - if err != nil { - return nil, err - } - rec, _, err := getDBParameterGroup(ctx, kv, accountID, name) + rec, err := s.parameterGroups().Get(ctx, accountID, name) if err != nil { return nil, err } @@ -290,7 +244,7 @@ func (s *Service) DescribeDBParameters(ctx context.Context, input *rds.DescribeD if err != nil { return nil, err } - overrides, err := ListDBParameterOverrides(ctx, kv, name) + overrides, err := s.parameterGroups().Overrides(ctx, accountID, name) if err != nil { return nil, err } @@ -350,42 +304,33 @@ func (s *Service) DeleteDBParameterGroup(ctx context.Context, input *rds.DeleteD "DB parameter group %s is a default group and cannot be deleted", name) } - kv, err := s.bucket(ctx, accountID) + if err := s.parameterGroups().Delete(ctx, accountID, name); err != nil { + return nil, err + } + return &rds.DeleteDBParameterGroupOutput{}, nil +} + +// The group's lifecycle owner, bound to this Service's bucket. +func (s *Service) parameterGroups() *parametergroup.Owner { + return parametergroup.New(parameterGroupReferences{s}, s.bucket) +} + +// The instance side of the group's in-use guard: current or pending attachment, +// matching the delete guard's own predicate. It reads instance records directly +// until instances have their own lifecycle owner. +type parameterGroupReferences struct{ s *Service } + +var _ parametergroup.Dependants = parameterGroupReferences{} + +func (r parameterGroupReferences) InstancesUsing(ctx context.Context, accountID, name string) ([]string, error) { + kv, err := r.s.bucket(ctx, accountID) if err != nil { return nil, err } - if _, _, err := getDBParameterGroup(ctx, kv, accountID, name); err != nil { - return nil, err - } - users, err := instancesUsingGroup(ctx, kv, func(rec *DBInstanceRecord) bool { + return instancesUsingGroup(ctx, kv, func(rec *DBInstanceRecord) bool { return rec.DBParameterGroupName == name || (rec.PendingModifiedValues != nil && rec.PendingModifiedValues.DBParameterGroupName == name) }) - if err != nil { - return nil, err - } - if len(users) > 0 { - return nil, awserrors.Errorf(awserrors.ErrorDBParameterGroupInvalidState, - "DB parameter group %s is still used by %s", name, strings.Join(users, ", ")) - } - - // The values go first: a crash between the two leaves orphaned parameter keys - // under a name a later create would then inherit silently. - overrides, err := ListDBParameterOverrides(ctx, kv, name) - if err != nil { - return nil, err - } - for _, param := range slices.Sorted(maps.Keys(overrides)) { - if err := kv.Delete(ctx, DBParameterGroupParamKey(name, param)); err != nil { - return nil, fmt.Errorf("rds: delete parameter %s of group %s: %w", param, name, err) - } - } - if err := kv.Delete(ctx, DBParameterGroupMetaKey(name)); err != nil { - return nil, fmt.Errorf("rds: delete DB parameter group %s: %w", name, err) - } - - slog.InfoContext(ctx, "rds: DB parameter group deleted", "dbParameterGroup", name, "accountId", accountID) - return &rds.DeleteDBParameterGroupOutput{}, nil } // One validated override from a ModifyDBParameterGroup request. @@ -450,36 +395,6 @@ func resolveApplyMethod(spec rdsengine.ParameterSpec, requested string) (string, } } -// The stored record, or the lazily materialised default group. A default group -// is synthesised rather than written on read: the record carries nothing a write -// would preserve, and materialising it on a describe would make a read path a -// writer for no gain. -func getDBParameterGroup(ctx context.Context, kv *kvstore.Bucket, accountID, name string) (*DBParameterGroupRecord, uint64, error) { - var rec DBParameterGroupRecord - rev, found, err := getJSONRevision(ctx, kv, DBParameterGroupMetaKey(name), &rec) - if err != nil { - return nil, 0, err - } - if found { - return &rec, rev, nil - } - if engine, ok := rdsengine.EngineForDefaultParameterGroup(name); ok { - return defaultParameterGroupRecord(engine, accountID), 0, nil - } - return nil, 0, awserrors.Errorf(awserrors.ErrorDBParameterGroupNotFound, "DB parameter group %s not found", name) -} - -// The implicit group, identical for every account. It carries no tags and no -// stored values, so it resolves to the catalog defaults alone. -func defaultParameterGroupRecord(engine rdsengine.Engine, accountID string) *DBParameterGroupRecord { - return &DBParameterGroupRecord{ - Name: engine.DefaultParameterGroupName(), - AccountID: accountID, - Family: engine.ParameterGroupFamily(), - Description: fmt.Sprintf("Default parameter group for %s", engine.ParameterGroupFamily()), - } -} - func isDefaultParameterGroupName(name string) bool { return strings.HasPrefix(strings.ToLower(name), defaultParameterGroupPrefix) } @@ -532,7 +447,7 @@ func projectParameter(spec rdsengine.ParameterSpec, value, storedApplyMethod str return out } -func (s *Service) projectParameterGroupRecord(rec *DBParameterGroupRecord) *rds.DBParameterGroup { +func (s *Service) projectParameterGroupRecord(rec *parametergroup.Record) *rds.DBParameterGroup { if rec == nil { return nil } @@ -544,6 +459,15 @@ func (s *Service) projectParameterGroupRecord(rec *DBParameterGroupRecord) *rds. } } +// parameterGroupValues is the instance side's narrow read of a DB parameter group: its family and its +// stored overrides, satisfied by the group's lifecycle owner. +type parameterGroupValues interface { + Get(ctx context.Context, accountID, name string) (*parametergroup.Record, error) + Overrides(ctx context.Context, accountID, name string) (map[string]parametergroup.Override, error) +} + +var _ parameterGroupValues = (*parametergroup.Owner)(nil) + // The effective set an instance of this class runs with under this group: catalog // defaults evaluated at the class, overlaid with the group's stored overrides. // A group that does not exist fails here rather than silently resolving to the @@ -552,8 +476,9 @@ func (s *Service) projectParameterGroupRecord(rec *DBParameterGroupRecord) *rds. // Every path that binds a group to an instance comes through here — create, // modify, restore, the deferred apply and group propagation — so the // cross-engine refusal is one check rather than five. -func (s *Service) resolveGroupParameters(ctx context.Context, kv *kvstore.Bucket, accountID string, engine rdsengine.Engine, group, instanceClass string) ([]Parameter, error) { - rec, _, err := getDBParameterGroup(ctx, kv, accountID, group) +func (s *Service) resolveGroupParameters(ctx context.Context, accountID string, engine rdsengine.Engine, group, instanceClass string) ([]Parameter, error) { + var groups parameterGroupValues = s.parameterGroups() + rec, err := groups.Get(ctx, accountID, group) if err != nil { return nil, err } @@ -562,7 +487,7 @@ func (s *Service) resolveGroupParameters(ctx context.Context, kv *kvstore.Bucket "DB parameter group %s is of family %s, which cannot be used by a %s DB instance; it requires a group of family %s", group, rec.Family, engine.Name, engine.ParameterGroupFamily()) } - overrides, err := ListDBParameterOverrides(ctx, kv, group) + overrides, err := groups.Overrides(ctx, accountID, group) if err != nil { return nil, err } diff --git a/spinifex/handlers/rds/parametergroup_lifecycle_test.go b/spinifex/handlers/rds/parametergroup_lifecycle_test.go index ee95bfae6..908fff69b 100644 --- a/spinifex/handlers/rds/parametergroup_lifecycle_test.go +++ b/spinifex/handlers/rds/parametergroup_lifecycle_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/rds" + "github.com/mulgadc/spinifex/spinifex/domains/rds/parametergroup" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -87,7 +88,7 @@ func TestDBParameterGroupRecord_DecodesAHandWrittenBlob(t *testing.T) { _, err = js.Put(t.Context(), "db-parameter-groups/"+testParameterGroup+"/params/work_mem", []byte(paramJSON)) require.NoError(t, err) - rec, _, err := getDBParameterGroup(t.Context(), kv, testAccountID, testParameterGroup) + rec, err := h.svc.parameterGroups().Get(t.Context(), testAccountID, testParameterGroup) require.NoError(t, err) assert.Equal(t, "tuned-pg", rec.Name) assert.Equal(t, testAccountID, rec.AccountID) @@ -95,7 +96,7 @@ func TestDBParameterGroupRecord_DecodesAHandWrittenBlob(t *testing.T) { assert.Equal(t, "Tuned for the orders workload", rec.Description) assert.Equal(t, map[string]string{"env": "prod"}, rec.Tags) - overrides, err := ListDBParameterOverrides(t.Context(), kv, testParameterGroup) + overrides, err := h.svc.parameterGroups().Overrides(t.Context(), testAccountID, testParameterGroup) require.NoError(t, err) override, ok := overrides["work_mem"] require.True(t, ok) @@ -120,8 +121,8 @@ func TestDescribeDBParameterGroups_TheDefaultGroupIsSynthesisedNotStored(t *test kv, err := h.svc.bucket(t.Context(), testAccountID) require.NoError(t, err) - var rec DBParameterGroupRecord - found, err := getJSON(t.Context(), kv, DBParameterGroupMetaKey(testDefaultPG), &rec) + var rec parametergroup.Record + found, err := getJSON(t.Context(), kv, parametergroup.MetaKey(testDefaultPG), &rec) require.NoError(t, err) assert.False(t, found, "a lazily-synthesised default group must not be written by a describe") } diff --git a/spinifex/handlers/rds/parametergroup_test.go b/spinifex/handlers/rds/parametergroup_test.go index fff53aa35..5f0e87613 100644 --- a/spinifex/handlers/rds/parametergroup_test.go +++ b/spinifex/handlers/rds/parametergroup_test.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/rds" rdsengine "github.com/mulgadc/spinifex/spinifex/domains/rds/engine" + "github.com/mulgadc/spinifex/spinifex/domains/rds/parametergroup" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -422,7 +423,7 @@ func TestModifyDBParameterGroup_ReturnsAPropagationFailure(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), testDBID) - overrides, listErr := ListDBParameterOverrides(t.Context(), h.kv(t), testParameterGroup) + overrides, listErr := h.svc.parameterGroups().Overrides(t.Context(), testAccountID, testParameterGroup) require.NoError(t, listErr) assert.Equal(t, "16384", overrides["work_mem"].Value, "the durable group edit remains available for a retry") @@ -588,11 +589,11 @@ func TestDescribeDBParameters_DerivesApplyMethodForLegacyOverrides(t *testing.T) require.NoError(t, err) kv, err := h.svc.bucket(t.Context(), testAccountID) require.NoError(t, err) - for _, rec := range []DBParameterRecord{ + for _, rec := range []parametergroup.Override{ {Name: "work_mem", Value: "16384"}, {Name: "shared_buffers", Value: "65536"}, } { - require.NoError(t, putJSON(t.Context(), kv, DBParameterGroupParamKey(testParameterGroup, rec.Name), &rec)) + require.NoError(t, putJSON(t.Context(), kv, parametergroup.ParamKey(testParameterGroup, rec.Name), &rec)) } params := describedParameters(t, h, testParameterGroup) @@ -688,11 +689,9 @@ func TestDeleteDBParameterGroup_RemovesTheGroupAndItsValues(t *testing.T) { &rds.DeleteDBParameterGroupInput{DBParameterGroupName: aws.String(testParameterGroup)}, testAccountID) require.NoError(t, err) - kv, err := h.svc.bucket(t.Context(), testAccountID) - require.NoError(t, err) // Orphaned values would be silently inherited by a later group of the same // name, which is a configuration nobody wrote. - overrides, err := ListDBParameterOverrides(t.Context(), kv, testParameterGroup) + overrides, err := h.svc.parameterGroups().Overrides(t.Context(), testAccountID, testParameterGroup) require.NoError(t, err) assert.Empty(t, overrides) diff --git a/spinifex/handlers/rds/pending.go b/spinifex/handlers/rds/pending.go index 51f138c8b..19bae3a17 100644 --- a/spinifex/handlers/rds/pending.go +++ b/spinifex/handlers/rds/pending.go @@ -155,7 +155,7 @@ func (s *Service) applyParameterGroup(ctx context.Context, kv *kvstore.Bucket, a // Reached from a deferred modify, whose group was checked at request time, // and from group propagation, where the binding was checked at attach. A // family mismatch here is corrupt state rather than a bad request. - resolved, err := s.resolveGroupParameters(ctx, kv, accountID, engine, group, instanceClass) + resolved, err := s.resolveGroupParameters(ctx, accountID, engine, group, instanceClass) if err != nil { return s.recordParameterApplyFailure(ctx, kv, accountID, rec, group, err) } diff --git a/spinifex/handlers/rds/records.go b/spinifex/handlers/rds/records.go index 8bff28051..3019f22e9 100644 --- a/spinifex/handlers/rds/records.go +++ b/spinifex/handlers/rds/records.go @@ -378,37 +378,6 @@ type Parameter struct { Value string `json:"value" locationName:"Value"` } -// DBParameterGroupRecord is the db-parameter-groups/{name}/meta record. The values themselves live one -// key each under .../params/, so a modify touching one parameter cannot clobber a concurrent change to -// another. -type DBParameterGroupRecord struct { - Name string `json:"name"` - AccountID string `json:"accountId"` - Family string `json:"family"` - Description string `json:"description"` - - Tags map[string]string `json:"tags,omitempty"` - - CreatedAt time.Time `json:"createdAt"` - UpdatedAt time.Time `json:"updatedAt"` -} - -var _ TaggedRecord = (*DBParameterGroupRecord)(nil) - -func (r *DBParameterGroupRecord) GetTags() map[string]string { return r.Tags } - -func (r *DBParameterGroupRecord) SetTags(tags map[string]string) { r.Tags = tags } - -// DBParameterRecord is one stored override, at db-parameter-groups/{name}/params/{key}. ApplyMethod is -// the customer's request rather than a fact: whether a change lands live is decided by the parameter's -// own ApplyType. -type DBParameterRecord struct { - Name string `json:"name"` - Value string `json:"value"` - ApplyMethod string `json:"applyMethod,omitempty"` - UpdatedAt time.Time `json:"updatedAt"` -} - // EngineHealth is the agent-reported state of the database engine, separate from Status: the reconciler // needs both to tell "stopped because we stopped it" from "stopped because it died". type EngineHealth string diff --git a/spinifex/handlers/rds/restore.go b/spinifex/handlers/rds/restore.go index e7eee62c2..b4b52510c 100644 --- a/spinifex/handlers/rds/restore.go +++ b/spinifex/handlers/rds/restore.go @@ -59,7 +59,7 @@ func (s *Service) RestoreDBInstanceFromDBSnapshot(ctx context.Context, input *rd } // resolveRestoreRequest forces the engine to the snapshot's, so a group of // another engine is refused here however the request named it. - parameters, err := s.resolveGroupParameters(ctx, kv, accountID, req.Engine, req.DBParameterGroupName, req.InstanceClass) + parameters, err := s.resolveGroupParameters(ctx, accountID, req.Engine, req.DBParameterGroupName, req.InstanceClass) if err != nil { return nil, err } diff --git a/spinifex/handlers/rds/store.go b/spinifex/handlers/rds/store.go index b22532124..0284fa637 100644 --- a/spinifex/handlers/rds/store.go +++ b/spinifex/handlers/rds/store.go @@ -101,28 +101,6 @@ func kvKeySegmentIdentifier(segment string) string { return strings.ReplaceAll(segment, kvKeyColon, ":") } -// DBParameterGroupsPrefix returns the parameter group prefix. A group's own record is at .../meta and -// its values hang off .../params/, so listing groups walks the meta keys. -func DBParameterGroupsPrefix() string { - return "db-parameter-groups/" -} - -// DBParameterGroupMetaKey returns the KV key of a DB parameter group's own record. -func DBParameterGroupMetaKey(name string) string { - return fmt.Sprintf("%s%s/meta", DBParameterGroupsPrefix(), name) -} - -// DBParameterGroupParamsPrefix returns the prefix holding one key per parameter value rather than one -// blob, so a ModifyDBParameterGroup touching a single parameter cannot clobber a concurrent change to another. -func DBParameterGroupParamsPrefix(name string) string { - return fmt.Sprintf("%s%s/params/", DBParameterGroupsPrefix(), name) -} - -// DBParameterGroupParamKey returns the KV key of one parameter override in a DB parameter group. -func DBParameterGroupParamKey(name, param string) string { - return DBParameterGroupParamsPrefix(name) + param -} - // AutomatedBackupsRootPrefix returns the automated-backup index prefix, kept separate from db-snapshots/ // so the retention sweep enumerates only automated backups, ordered lexically by their timestamp suffix. func AutomatedBackupsRootPrefix() string { @@ -331,52 +309,6 @@ func ListDBSnapshotIDs(ctx context.Context, kv *kvstore.Bucket) ([]string, error return names, nil } -// ListDBParameterGroupNames walks the .../meta keys, which is what makes a group's own record findable -// among the per-parameter keys hanging off the same prefix. -func ListDBParameterGroupNames(ctx context.Context, kv *kvstore.Bucket) ([]string, error) { - keys, err := bucketKeys(ctx, kv) - if err != nil { - return nil, err - } - prefix := DBParameterGroupsPrefix() - names := make([]string, 0, len(keys)) - for _, key := range keys { - rest, prefixed := strings.CutPrefix(key, prefix) - name, suffixed := strings.CutSuffix(rest, "/meta") - if !prefixed || !suffixed { - continue - } - if name == "" || strings.Contains(name, "/") { - continue - } - names = append(names, name) - } - return names, nil -} - -// ListDBParameterOverrides returns the stored overrides of one parameter group, keyed by parameter name. -func ListDBParameterOverrides(ctx context.Context, kv *kvstore.Bucket, 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]DBParameterRecord, len(names)) - for _, name := range names { - var rec DBParameterRecord - found, err := getJSON(ctx, kv, prefix+name, &rec) - if err != nil { - return nil, err - } - // 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 - } - } - return out, nil -} - // ListAutomatedBackups returns the automated-backup timestamps grouped by DB instance from one bucket // listing: the retention sweep needs every instance's set, and a Keys call per instance would cost one // listing each. diff --git a/spinifex/handlers/rds/store_test.go b/spinifex/handlers/rds/store_test.go index 784124086..f1b698f8f 100644 --- a/spinifex/handlers/rds/store_test.go +++ b/spinifex/handlers/rds/store_test.go @@ -18,9 +18,6 @@ func TestKeyPaths(t *testing.T) { t.Parallel() assert.Equal(t, "db-instances/orders-db", DBInstanceKey("orders-db")) assert.Equal(t, "db-snapshots/orders-db-final", DBSnapshotKey("orders-db-final")) - assert.Equal(t, "db-parameter-groups/pg16/meta", DBParameterGroupMetaKey("pg16")) - assert.Equal(t, "db-parameter-groups/pg16/params/shared_buffers", - DBParameterGroupParamKey("pg16", "shared_buffers")) assert.Equal(t, "backups/orders-db/automated/20260724T170000Z", AutomatedBackupKey("orders-db", "20260724T170000Z")) assert.Equal(t, "retained-volumes/vol-abc123", RetainedVolumeKey("vol-abc123")) @@ -33,15 +30,9 @@ func TestKeyPathsSitUnderTheirPrefix(t *testing.T) { t.Parallel() assert.True(t, strings.HasPrefix(DBInstanceKey("x"), DBInstancesPrefix())) assert.True(t, strings.HasPrefix(DBSnapshotKey("x"), DBSnapshotsPrefix())) - assert.True(t, strings.HasPrefix(DBParameterGroupMetaKey("x"), DBParameterGroupsPrefix())) - assert.True(t, strings.HasPrefix(DBParameterGroupParamKey("x", "p"), DBParameterGroupParamsPrefix("x"))) assert.True(t, strings.HasPrefix(AutomatedBackupKey("x", "t"), AutomatedBackupsPrefix("x"))) assert.True(t, strings.HasPrefix(RetainedVolumeKey("v"), RetainedVolumesPrefix())) assert.True(t, strings.HasPrefix(InstanceIndexKey("i"), InstanceIndexPrefix())) - - // A parameter group's values must not be reachable by listing groups' meta - // records, or DescribeDBParameterGroups would report one group per parameter. - assert.False(t, strings.HasPrefix(DBParameterGroupParamKey("x", "p"), DBParameterGroupMetaKey("x"))) } func TestNewStore_NilConn(t *testing.T) { diff --git a/spinifex/handlers/rds/tags.go b/spinifex/handlers/rds/tags.go index 0074eb5d6..8fe33a83b 100644 --- a/spinifex/handlers/rds/tags.go +++ b/spinifex/handlers/rds/tags.go @@ -10,6 +10,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/rds" + "github.com/mulgadc/spinifex/spinifex/domains/rds/parametergroup" "github.com/mulgadc/spinifex/spinifex/domains/rds/subnetgroup" "github.com/mulgadc/spinifex/spinifex/foundation/aws/errors" "github.com/mulgadc/spinifex/spinifex/foundation/state/kvstore" @@ -50,6 +51,7 @@ type taggableResource struct { // record, so a tag written to it would have nowhere to live and would read back // as absent on the next apply. var _ TaggedRecord = (*subnetgroup.Record)(nil) +var _ TaggedRecord = (*parametergroup.Record)(nil) var taggableResources = map[ResourceKind]taggableResource{ ResourceKindDBInstance: { @@ -68,8 +70,8 @@ var taggableResources = map[ResourceKind]taggableResource{ notFound: awserrors.ErrorDBSubnetGroupNotFound, }, ResourceKindDBParameterGroup: { - key: DBParameterGroupMetaKey, - newRecord: func() TaggedRecord { return &DBParameterGroupRecord{} }, + key: parametergroup.MetaKey, + newRecord: func() TaggedRecord { return ¶metergroup.Record{} }, notFound: awserrors.ErrorDBParameterGroupNotFound, }, }