From 8746b9e79d3ea3b7a040677b44920b13daf2e59e Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Tue, 29 Sep 2026 09:16:39 +1000 Subject: [PATCH] fix: page every RDS Describe call by MaxRecords and Marker (#1131) --- spinifex/gateway/rds/enginecatalog.go | 34 +++--- spinifex/gateway/rds/enginecatalog_test.go | 31 +++++- spinifex/handlers/rds/backup.go | 9 +- spinifex/handlers/rds/describe.go | 34 +++--- spinifex/handlers/rds/events.go | 26 ++--- spinifex/handlers/rds/events_test.go | 22 ++++ spinifex/handlers/rds/paging.go | 74 +++++++++++++ spinifex/handlers/rds/paging_test.go | 107 +++++++++++++++++++ spinifex/handlers/rds/parametergroup.go | 29 +++-- spinifex/handlers/rds/parametergroup_test.go | 51 +++++++++ spinifex/handlers/rds/snapshot.go | 6 +- spinifex/handlers/rds/subnetgroup.go | 21 ++-- 12 files changed, 367 insertions(+), 77 deletions(-) create mode 100644 spinifex/handlers/rds/paging.go create mode 100644 spinifex/handlers/rds/paging_test.go diff --git a/spinifex/gateway/rds/enginecatalog.go b/spinifex/gateway/rds/enginecatalog.go index 07d4c7c0f..d29ec9483 100644 --- a/spinifex/gateway/rds/enginecatalog.go +++ b/spinifex/gateway/rds/enginecatalog.go @@ -43,10 +43,6 @@ var ( // filter here rather than a required parameter, and an unknown one is an empty // list rather than the rejection create-db-instance gives it. func DescribeDBEngineVersions(ctx context.Context, input *rds.DescribeDBEngineVersionsInput, _ *nats.Conn, _ Caller) (any, error) { - if err := rejectMarker(input.Marker, "DescribeDBEngineVersions"); err != nil { - return nil, err - } - filter := handlers_rds.EngineVersionFilter{} filter.Engine.AddParam(aws.StringValue(input.Engine)) filter.EngineVersion.AddParam(aws.StringValue(input.EngineVersion)) @@ -74,15 +70,17 @@ func DescribeDBEngineVersions(ctx context.Context, input *rds.DescribeDBEngineVe // DefaultOnly, IncludeAll, ListSupportedCharacterSets and ListSupportedTimezones // are accepted and not read: each is an identity on a catalog of one available // version per engine with no character-set or timezone list to populate. - return &rds.DescribeDBEngineVersionsOutput{DBEngineVersions: handlers_rds.EngineVersions(filter)}, nil + versions, marker, err := handlers_rds.Page(handlers_rds.EngineVersions(filter), + handlers_rds.EngineVersionPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } + return &rds.DescribeDBEngineVersionsOutput{DBEngineVersions: versions, Marker: marker}, nil } // Engine is required, so an absent one is MissingParameter and an unknown one is // the InvalidParameterValue LookupEngine already words. Everything else narrows. func DescribeOrderableDBInstanceOptions(ctx context.Context, input *rds.DescribeOrderableDBInstanceOptionsInput, nc *nats.Conn, _ Caller, env Env) (any, error) { - if err := rejectMarker(input.Marker, "DescribeOrderableDBInstanceOptions"); err != nil { - return nil, err - } if aws.StringValue(input.AvailabilityZoneGroup) != "" { return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, "AvailabilityZoneGroup is not supported: this platform exposes a single zone and names none") @@ -133,9 +131,12 @@ func DescribeOrderableDBInstanceOptions(ctx context.Context, input *rds.Describe if err != nil { return nil, err } - return &rds.DescribeOrderableDBInstanceOptionsOutput{ - OrderableDBInstanceOptions: handlers_rds.OrderableOptions(filter, runnable), - }, nil + options, marker, err := handlers_rds.Page(handlers_rds.OrderableOptions(filter, runnable), + handlers_rds.OrderableOptionPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } + return &rds.DescribeOrderableDBInstanceOptionsOutput{OrderableDBInstanceOptions: options, Marker: marker}, nil } // Which EC2 instance types the cluster's nodes report they can run, as a @@ -171,17 +172,6 @@ func clusterRunnableTypes(ctx context.Context, nc *nats.Conn, env Env) (func(str return func(instanceType string) bool { return supported[instanceType] }, nil } -// Neither action ever issues a Marker, so one in a request can only have been -// fabricated. Answering it as page one would report the whole catalog as if it -// were a later page. -func rejectMarker(marker *string, action string) error { - if aws.StringValue(marker) == "" { - return nil - } - return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, - "Marker is not supported: %s returns every row in a single page", action) -} - // Both members are required by the shape, and treating either as absent would // silently widen the result rather than narrow it. func filterEntry(entry *rds.Filter) (string, []string, error) { diff --git a/spinifex/gateway/rds/enginecatalog_test.go b/spinifex/gateway/rds/enginecatalog_test.go index 3096c6889..9a3a99138 100644 --- a/spinifex/gateway/rds/enginecatalog_test.go +++ b/spinifex/gateway/rds/enginecatalog_test.go @@ -238,10 +238,32 @@ func TestDescribeCatalogs_RejectMalformedFilters(t *testing.T) { } } -// Neither action ever issues a Marker, so one in a request was fabricated by the -// caller. This diverges from the other RDS describes, which parse and ignore it, -// because those may one day paginate and these two never will. -func TestDescribeCatalogs_RejectAMarker(t *testing.T) { +// Following each Marker returns the whole catalog once, in the unpaged order, +// with the Marker surviving the XML round trip. +func TestDescribeOrderableDBInstanceOptions_PagesTheCatalog(t *testing.T) { + nc := newStubbedNATS(t) + var classes []string + marker := "" + for { + query := map[string]string{"Action": "DescribeOrderableDBInstanceOptions", "Engine": "postgres", "MaxRecords": "2"} + if marker != "" { + query["Marker"] = marker + } + body, err := Dispatch(t.Context(), "DescribeOrderableDBInstanceOptions", query, nc, testCaller, testEnv) + require.NoError(t, err) + var out rds.DescribeOrderableDBInstanceOptionsOutput + require.NoError(t, xmlutil.UnmarshalXML(&out, xml.NewDecoder(bytes.NewReader(body)), + "DescribeOrderableDBInstanceOptionsResult")) + require.LessOrEqual(t, len(out.OrderableDBInstanceOptions), 2) + classes = append(classes, classNames(out.OrderableDBInstanceOptions)...) + if marker = aws.StringValue(out.Marker); marker == "" { + break + } + } + assert.Equal(t, handlers_rds.SupportedInstanceClasses(), classes) +} + +func TestDescribeCatalogs_RejectAMarkerTheyDidNotIssue(t *testing.T) { nc := newStubbedNATS(t) for _, action := range []string{"DescribeDBEngineVersions", "DescribeOrderableDBInstanceOptions"} { t.Run(action, func(t *testing.T) { @@ -250,7 +272,6 @@ func TestDescribeCatalogs_RejectAMarker(t *testing.T) { }, nc, testCaller, testEnv) require.Error(t, err) assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue) - assert.Contains(t, err.Error(), "Marker") }) } } diff --git a/spinifex/handlers/rds/backup.go b/spinifex/handlers/rds/backup.go index bf4f3f8f8..3b7673e72 100644 --- a/spinifex/handlers/rds/backup.go +++ b/spinifex/handlers/rds/backup.go @@ -395,6 +395,9 @@ func AutomatedBackupStamp(at time.Time) string { // listable through DescribeDBSnapshots --snapshot-type automated. func (s *Service) DescribeDBInstanceAutomatedBackups(ctx context.Context, input *rds.DescribeDBInstanceAutomatedBackupsInput, accountID string) (*rds.DescribeDBInstanceAutomatedBackupsOutput, error) { + if input == nil { + input = &rds.DescribeDBInstanceAutomatedBackupsInput{} + } wanted, err := validateDescribeAutomatedBackupsRequest(input) if err != nil { return nil, err @@ -436,7 +439,11 @@ func (s *Service) DescribeDBInstanceAutomatedBackups(ctx context.Context, } backups = append(backups, s.projectAutomatedBackup(&rec, len(stamps))) } - return &rds.DescribeDBInstanceAutomatedBackupsOutput{DBInstanceAutomatedBackups: backups}, nil + backups, next, err := Page(backups, automatedBackupPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } + return &rds.DescribeDBInstanceAutomatedBackupsOutput{DBInstanceAutomatedBackups: backups, Marker: next}, nil } // A filter this phase cannot honour is rejected rather than dropped, since a diff --git a/spinifex/handlers/rds/describe.go b/spinifex/handlers/rds/describe.go index 6e9a376bb..54ab9a3c8 100644 --- a/spinifex/handlers/rds/describe.go +++ b/spinifex/handlers/rds/describe.go @@ -24,29 +24,29 @@ const ( // polling a create would otherwise read an empty list as "gone" rather than // "not ready". func (s *Service) DescribeDBInstances(ctx context.Context, input *rds.DescribeDBInstancesInput, accountID string) (*rds.DescribeDBInstancesOutput, error) { + if input == nil { + input = &rds.DescribeDBInstancesInput{} + } kv, err := s.bucket(ctx, accountID) if err != nil { return nil, err } - var matches func(*DBInstanceRecord) bool - if input != nil { - matches, err = dbInstanceFilterMatcher(input.Filters) + matches, err := dbInstanceFilterMatcher(input.Filters) + if err != nil { + return nil, err + } + if id := aws.StringValue(input.DBInstanceIdentifier); id != "" { + rec, _, err := s.getDBInstance(ctx, kv, id) if err != nil { return nil, err } - if id := aws.StringValue(input.DBInstanceIdentifier); id != "" { - rec, _, err := s.getDBInstance(ctx, kv, id) - if err != nil { - return nil, err - } - // A named instance the filters exclude is reported as absent rather than - // returned anyway, so the two halves of one request cannot disagree. - if matches != nil && !matches(rec) { - return nil, errors.New(awserrors.ErrorDBInstanceNotFound) - } - return &rds.DescribeDBInstancesOutput{DBInstances: []*rds.DBInstance{s.projectDBInstance(rec)}}, nil + // A named instance the filters exclude is reported as absent rather than + // returned anyway, so the two halves of one request cannot disagree. + if matches != nil && !matches(rec) { + return nil, errors.New(awserrors.ErrorDBInstanceNotFound) } + return &rds.DescribeDBInstancesOutput{DBInstances: []*rds.DBInstance{s.projectDBInstance(rec)}}, nil } ids, err := ListDBInstanceIDs(ctx, kv) @@ -72,7 +72,11 @@ func (s *Service) DescribeDBInstances(ctx context.Context, input *rds.DescribeDB } instances = append(instances, s.projectDBInstance(&rec)) } - return &rds.DescribeDBInstancesOutput{DBInstances: instances}, nil + instances, next, err := Page(instances, dbInstancePageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } + return &rds.DescribeDBInstancesOutput{DBInstances: instances, Marker: next}, nil } // The filter names AWS documents for this action and clients actually send. diff --git a/spinifex/handlers/rds/events.go b/spinifex/handlers/rds/events.go index 8a4da1d72..e43da2f6f 100644 --- a/spinifex/handlers/rds/events.go +++ b/spinifex/handlers/rds/events.go @@ -33,9 +33,8 @@ const ( // once, so the CAS is retried rather than dropped on first contention. eventWriteAttempts = 8 - // AWS's defaults for the read: a one-hour window and a 100-record page. + // AWS's default window for the read. defaultEventDuration = time.Hour - maxEventRecords = 100 ) // AWS's SourceType values. Only the ones some phase actually writes are @@ -209,16 +208,14 @@ func (s *Service) DescribeEvents(ctx context.Context, input *rds.DescribeEventsI // Oldest first across resources, as AWS returns them, so a client that has // already read up to a timestamp can resume from it. slices.SortFunc(events, func(a, b Event) int { - if cmp := a.Date.Compare(b.Date); cmp != 0 { - return cmp - } - return strings.Compare(a.SourceIdentifier, b.SourceIdentifier) + return strings.Compare(eventPageKey(a), eventPageKey(b)) }) - if limit := eventRecordLimit(input); len(events) > limit { - events = events[:limit] + events, marker, err := Page(events, eventPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err } - out := &rds.DescribeEventsOutput{Events: make([]*rds.Event, 0, len(events))} + out := &rds.DescribeEventsOutput{Events: make([]*rds.Event, 0, len(events)), Marker: marker} for _, event := range events { out.Events = append(out.Events, s.projectEvent(accountID, event)) } @@ -267,12 +264,11 @@ func eventWindow(input *rds.DescribeEventsInput) (timeWindow, error) { return window, nil } -func eventRecordLimit(input *rds.DescribeEventsInput) int { - requested := aws.Int64Value(input.MaxRecords) - if requested <= 0 || requested > maxEventRecords { - return maxEventRecords - } - return int(requested) +// Fixed-width nanoseconds lead, so the key sorts by date, and the remaining +// fields break a tie the same way on every read. +func eventPageKey(event Event) string { + return pageKey(fmt.Sprintf("%020d", event.Date.UnixNano()), + event.SourceIdentifier, event.SourceType, event.Message) } // A fully qualified read is one Get. Anything broader has to enumerate, because diff --git a/spinifex/handlers/rds/events_test.go b/spinifex/handlers/rds/events_test.go index 2094b4e23..c23266a5e 100644 --- a/spinifex/handlers/rds/events_test.go +++ b/spinifex/handlers/rds/events_test.go @@ -102,6 +102,28 @@ func TestDescribeEvents_ReadsAcrossResourcesAndScopesToOneWhenAsked(t *testing.T assert.Equal(t, []string{"snapshot event"}, messagesOf(scoped)) } +// A truncated read carries a Marker, so a client can tell the list was cut and +// resume from it. +func TestDescribeEvents_PagesOldestFirst(t *testing.T) { + t.Parallel() + svc := newTestService(t) + now := time.Now().UTC() + seedEvent(t, svc, EventSourceTypeDBInstance, testDBID, now.Add(-3*time.Minute), "first") + seedEvent(t, svc, EventSourceTypeDBInstance, testDBID, now.Add(-2*time.Minute), "second") + seedEvent(t, svc, EventSourceTypeDBInstance, testDBID, now.Add(-time.Minute), "third") + + first, err := svc.DescribeEvents(t.Context(), &rds.DescribeEventsInput{MaxRecords: aws.Int64(2)}, testAccountID) + require.NoError(t, err) + assert.Equal(t, []string{"first", "second"}, messagesOf(first.Events)) + require.NotNil(t, first.Marker) + + rest, err := svc.DescribeEvents(t.Context(), + &rds.DescribeEventsInput{MaxRecords: aws.Int64(2), Marker: first.Marker}, testAccountID) + require.NoError(t, err) + assert.Equal(t, []string{"third"}, messagesOf(rest.Events)) + assert.Nil(t, rest.Marker) +} + func TestDescribeEvents_RejectsMalformedRequests(t *testing.T) { cases := []struct { name string diff --git a/spinifex/handlers/rds/paging.go b/spinifex/handlers/rds/paging.go new file mode 100644 index 000000000..1ca97f341 --- /dev/null +++ b/spinifex/handlers/rds/paging.go @@ -0,0 +1,74 @@ +package handlers_rds + +import ( + "encoding/base64" + "slices" + "strings" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/rds" + "github.com/mulgadc/spinifex/spinifex/awserrors" +) + +// AWS's page size when a Describe call names none. +const defaultMaxRecords = 100 + +// Joins the fields of a compound page key. It sorts below every printable byte, +// so a key compares field by field exactly as the fields do. +const pageKeySeparator = "\x00" + +// One page of items, already in ascending key order. The Marker is base64 of the +// next item's key, as AWS issues it; a resume starts at the first key not below +// it, so that item's deletion skips nothing. MaxRecords outside 1-100 is 100. +func Page[T any](items []T, key func(T) string, maxRecords *int64, marker *string) ([]T, *string, error) { + start := 0 + if encoded := aws.StringValue(marker); encoded != "" { + decoded, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + return nil, nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "Marker %q was not issued by this service", encoded) + } + start, _ = slices.BinarySearchFunc(items, string(decoded), func(item T, target string) int { + return strings.Compare(key(item), target) + }) + } + + limit := defaultMaxRecords + if requested := aws.Int64Value(maxRecords); requested > 0 && requested < defaultMaxRecords { + limit = int(requested) + } + rest := items[start:] + if len(rest) <= limit { + return rest, nil, nil + } + return rest[:limit], aws.String(base64.StdEncoding.EncodeToString([]byte(key(rest[limit])))), nil +} + +func pageKey(fields ...string) string { + return strings.Join(fields, pageKeySeparator) +} + +// The key each paged listing is already sorted by. +func parameterPageKey(p *rds.Parameter) string { return aws.StringValue(p.ParameterName) } + +func dbInstancePageKey(i *rds.DBInstance) string { return aws.StringValue(i.DBInstanceIdentifier) } + +func dbSnapshotPageKey(s *rds.DBSnapshot) string { return aws.StringValue(s.DBSnapshotIdentifier) } + +func automatedBackupPageKey(b *rds.DBInstanceAutomatedBackup) string { + return aws.StringValue(b.DBInstanceIdentifier) +} + +func subnetGroupPageKey(g *rds.DBSubnetGroup) string { return aws.StringValue(g.DBSubnetGroupName) } + +func parameterGroupPageKey(g *rds.DBParameterGroup) string { + return aws.StringValue(g.DBParameterGroupName) +} + +func EngineVersionPageKey(v *rds.DBEngineVersion) string { + return pageKey(aws.StringValue(v.Engine), aws.StringValue(v.EngineVersion)) +} + +func OrderableOptionPageKey(o *rds.OrderableDBInstanceOption) string { + return pageKey(aws.StringValue(o.Engine), aws.StringValue(o.EngineVersion), aws.StringValue(o.DBInstanceClass)) +} diff --git a/spinifex/handlers/rds/paging_test.go b/spinifex/handlers/rds/paging_test.go new file mode 100644 index 000000000..92bf8530e --- /dev/null +++ b/spinifex/handlers/rds/paging_test.go @@ -0,0 +1,107 @@ +package handlers_rds + +import ( + "encoding/base64" + "fmt" + "slices" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/rds" + "github.com/mulgadc/spinifex/spinifex/awserrors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func identity(s string) string { return s } + +func pageNames(n int) []string { + names := make([]string, 0, n) + for i := range n { + names = append(names, fmt.Sprintf("item-%03d", i)) + } + return names +} + +// Follows every Marker to the end, returning the pages as served. +func walkPages(t *testing.T, items []string, maxRecords int64) [][]string { + t.Helper() + var pages [][]string + var marker *string + for { + page, next, err := Page(items, identity, aws.Int64(maxRecords), marker) + require.NoError(t, err) + pages = append(pages, page) + if next == nil { + return pages + } + marker = next + } +} + +func TestPage_WalksEveryItemOnceInOrder(t *testing.T) { + t.Parallel() + items := pageNames(53) + pages := walkPages(t, items, 20) + + require.Len(t, pages, 3) + assert.Len(t, pages[0], 20) + assert.Len(t, pages[1], 20) + assert.Len(t, pages[2], 13) + assert.Equal(t, items, slices.Concat(pages...)) +} + +// AWS's marker is base64 of the next row's name, and the last page carries none. +func TestPage_MarkerNamesTheNextItem(t *testing.T) { + t.Parallel() + items := pageNames(25) + _, next, err := Page(items, identity, aws.Int64(20), nil) + require.NoError(t, err) + assert.Equal(t, base64.StdEncoding.EncodeToString([]byte("item-020")), aws.StringValue(next)) + + last, next, err := Page(items, identity, aws.Int64(20), next) + require.NoError(t, err) + assert.Equal(t, items[20:], last) + assert.Nil(t, next) +} + +func TestPage_ResumesPastAnItemDeletedBetweenPages(t *testing.T) { + t.Parallel() + items := pageNames(30) + _, marker, err := Page(items, identity, aws.Int64(10), nil) + require.NoError(t, err) + + remaining := slices.Delete(slices.Clone(items), 10, 11) + page, _, err := Page(remaining, identity, aws.Int64(10), marker) + require.NoError(t, err) + assert.Equal(t, items[11:21], page) +} + +func TestPage_ClampsAnUnsetOrOutOfRangeMaxRecordsToTheDefault(t *testing.T) { + t.Parallel() + items := pageNames(defaultMaxRecords + 5) + for _, maxRecords := range []*int64{nil, aws.Int64(0), aws.Int64(-1), aws.Int64(500)} { + page, next, err := Page(items, identity, maxRecords, nil) + require.NoError(t, err) + assert.Len(t, page, defaultMaxRecords) + assert.NotNil(t, next) + } +} + +func TestPage_RejectsAMarkerItDidNotIssue(t *testing.T) { + t.Parallel() + _, _, err := Page(pageNames(5), identity, nil, aws.String("not base64!")) + require.Error(t, err) + assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue) +} + +// The compound keys the catalog listings page by must sort as the listings do, +// which a separator above '-' or '.' would break for engine names sharing a prefix. +func TestPageKey_SortsFieldByField(t *testing.T) { + t.Parallel() + keys := []string{ + OrderableOptionPageKey(&rds.OrderableDBInstanceOption{Engine: aws.String("aurora"), DBInstanceClass: aws.String("db.t3.micro")}), + OrderableOptionPageKey(&rds.OrderableDBInstanceOption{Engine: aws.String("aurora-mysql"), DBInstanceClass: aws.String("db.m5.large")}), + } + assert.True(t, slices.IsSorted(keys)) +} diff --git a/spinifex/handlers/rds/parametergroup.go b/spinifex/handlers/rds/parametergroup.go index 20f7a2fd2..561597551 100644 --- a/spinifex/handlers/rds/parametergroup.go +++ b/spinifex/handlers/rds/parametergroup.go @@ -84,20 +84,21 @@ func (s *Service) CreateDBParameterGroup(ctx context.Context, input *rds.CreateD // or not anything has materialised it yet — a client that lists groups before // creating its first instance must still see the one it can name. func (s *Service) DescribeDBParameterGroups(ctx context.Context, input *rds.DescribeDBParameterGroupsInput, accountID string) (*rds.DescribeDBParameterGroupsOutput, error) { + if input == nil { + input = &rds.DescribeDBParameterGroupsInput{} + } kv, err := s.bucket(ctx, accountID) if err != nil { return nil, err } - if input != nil { - if name := aws.StringValue(input.DBParameterGroupName); name != "" { - rec, _, err := getDBParameterGroup(ctx, kv, accountID, name) - if err != nil { - return nil, err - } - return &rds.DescribeDBParameterGroupsOutput{ - DBParameterGroups: []*rds.DBParameterGroup{s.projectParameterGroupRecord(rec)}, - }, nil + if name := aws.StringValue(input.DBParameterGroupName); name != "" { + rec, _, err := getDBParameterGroup(ctx, kv, accountID, name) + if err != nil { + return nil, err } + return &rds.DescribeDBParameterGroupsOutput{ + DBParameterGroups: []*rds.DBParameterGroup{s.projectParameterGroupRecord(rec)}, + }, nil } names, err := ListDBParameterGroupNames(ctx, kv) @@ -125,7 +126,11 @@ func (s *Service) DescribeDBParameterGroups(ctx context.Context, input *rds.Desc } groups = append(groups, s.projectParameterGroupRecord(rec)) } - return &rds.DescribeDBParameterGroupsOutput{DBParameterGroups: groups}, nil + groups, next, err := Page(groups, parameterGroupPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } + return &rds.DescribeDBParameterGroupsOutput{DBParameterGroups: groups, Marker: next}, nil } // Stores validated overrides, one KV key per parameter, so a modify touching one @@ -280,6 +285,10 @@ func (s *Service) DescribeDBParameters(ctx context.Context, input *rds.DescribeD } out.Parameters = append(out.Parameters, projectParameter(spec, value, override.ApplyMethod, isOverride)) } + out.Parameters, out.Marker, err = Page(out.Parameters, parameterPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } return out, nil } diff --git a/spinifex/handlers/rds/parametergroup_test.go b/spinifex/handlers/rds/parametergroup_test.go index 322bafede..5d0c3385e 100644 --- a/spinifex/handlers/rds/parametergroup_test.go +++ b/spinifex/handlers/rds/parametergroup_test.go @@ -1,6 +1,7 @@ package handlers_rds import ( + "encoding/base64" "strings" "testing" @@ -599,6 +600,56 @@ func TestDescribeDBParameters_FiltersOnSource(t *testing.T) { assert.Len(t, defaults.Parameters, len(enginePostgres.CatalogParameterNames())-1) } +// AWS's observed first page of a fresh postgres18 group at MaxRecords 20: twenty +// parameters by name, and a Marker that is base64 of the twenty-first's name. +func TestDescribeDBParameters_PagesByName(t *testing.T) { + t.Parallel() + h := newCreateHarness(t, testBaseDomain) + catalog := enginePostgres.CatalogParameterNames() + + var listed []string + var marker *string + for page := 0; ; page++ { + out, err := h.svc.DescribeDBParameters(t.Context(), &rds.DescribeDBParametersInput{ + DBParameterGroupName: aws.String(testDefaultPG), + MaxRecords: aws.Int64(20), + Marker: marker, + }, testAccountID) + require.NoError(t, err) + if page == 0 { + require.Len(t, out.Parameters, 20) + assert.Equal(t, base64.StdEncoding.EncodeToString([]byte(catalog[20])), aws.StringValue(out.Marker)) + } + for _, p := range out.Parameters { + listed = append(listed, aws.StringValue(p.ParameterName)) + } + if out.Marker == nil { + break + } + marker = out.Marker + } + assert.Equal(t, catalog, listed) +} + +func TestDescribeDBParameterGroups_PagesByName(t *testing.T) { + t.Parallel() + h := newCreateHarness(t, testBaseDomain) + _, err := h.svc.CreateDBParameterGroup(t.Context(), parameterGroupInput("alpha"), testAccountID) + require.NoError(t, err) + + first, err := h.svc.DescribeDBParameterGroups(t.Context(), + &rds.DescribeDBParameterGroupsInput{MaxRecords: aws.Int64(2)}, testAccountID) + require.NoError(t, err) + assert.Equal(t, []string{"alpha", testDefaultMariaDB}, parameterGroupNames(first)) + require.NotNil(t, first.Marker) + + rest, err := h.svc.DescribeDBParameterGroups(t.Context(), + &rds.DescribeDBParameterGroupsInput{MaxRecords: aws.Int64(2), Marker: first.Marker}, testAccountID) + require.NoError(t, err) + assert.Equal(t, []string{testDefaultPG}, parameterGroupNames(rest)) + assert.Nil(t, rest.Marker) +} + func TestDeleteDBParameterGroup_RemovesTheGroupAndItsValues(t *testing.T) { t.Parallel() h := newCreateHarness(t, testBaseDomain) diff --git a/spinifex/handlers/rds/snapshot.go b/spinifex/handlers/rds/snapshot.go index f3810e10c..0a831bbf2 100644 --- a/spinifex/handlers/rds/snapshot.go +++ b/spinifex/handlers/rds/snapshot.go @@ -262,7 +262,11 @@ func (s *Service) DescribeDBSnapshots(ctx context.Context, input *rds.DescribeDB } snapshots = append(snapshots, s.projectDBSnapshot(&rec)) } - return &rds.DescribeDBSnapshotsOutput{DBSnapshots: snapshots}, nil + snapshots, next, err := Page(snapshots, dbSnapshotPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } + return &rds.DescribeDBSnapshotsOutput{DBSnapshots: snapshots, Marker: next}, nil } // An empty filter matches everything, as AWS does. diff --git a/spinifex/handlers/rds/subnetgroup.go b/spinifex/handlers/rds/subnetgroup.go index 5390a460d..a4e059cbc 100644 --- a/spinifex/handlers/rds/subnetgroup.go +++ b/spinifex/handlers/rds/subnetgroup.go @@ -91,18 +91,19 @@ func (s *Service) CreateDBSubnetGroup(ctx context.Context, input *rds.CreateDBSu // A named group that does not exist is an error, matching AWS; an unnamed // request lists the account's groups. func (s *Service) DescribeDBSubnetGroups(ctx context.Context, input *rds.DescribeDBSubnetGroupsInput, accountID string) (*rds.DescribeDBSubnetGroupsOutput, error) { + if input == nil { + input = &rds.DescribeDBSubnetGroupsInput{} + } kv, err := s.bucket(ctx, accountID) if err != nil { return nil, err } - if input != nil { - if name := aws.StringValue(input.DBSubnetGroupName); name != "" { - rec, _, err := getDBSubnetGroup(ctx, kv, name) - if err != nil { - return nil, err - } - return &rds.DescribeDBSubnetGroupsOutput{DBSubnetGroups: []*rds.DBSubnetGroup{s.projectSubnetGroup(rec)}}, nil + if name := aws.StringValue(input.DBSubnetGroupName); name != "" { + rec, _, err := getDBSubnetGroup(ctx, kv, name) + if err != nil { + return nil, err } + return &rds.DescribeDBSubnetGroupsOutput{DBSubnetGroups: []*rds.DBSubnetGroup{s.projectSubnetGroup(rec)}}, nil } names, err := listNames(ctx, kv, DBSubnetGroupsPrefix()) @@ -123,7 +124,11 @@ func (s *Service) DescribeDBSubnetGroups(ctx context.Context, input *rds.Describ } groups = append(groups, s.projectSubnetGroup(&rec)) } - return &rds.DescribeDBSubnetGroupsOutput{DBSubnetGroups: groups}, nil + groups, next, err := Page(groups, subnetGroupPageKey, input.MaxRecords, input.Marker) + if err != nil { + return nil, err + } + return &rds.DescribeDBSubnetGroupsOutput{DBSubnetGroups: groups, Marker: next}, nil } // Refused while any instance still names the group, including one that is only