fix(aws/rds): refuse explicitly when the offering page cap hides uniqueness

[IAmJSD]

Root cause

Since #300 (for #288), paginateRDSOfferings in providers/aws/services/rds/client.go walks every DescribeReservedDBInstancesOfferings page so it can catch a second distinct exact match. The walk is still bounded by maxOfferingPages (5 pages × MaxRecords=100). Before #300 a match on an early page returned right away. Now the match is held until the last page, so if a lookup hits the cap after finding a match, it fails with the same generic pagination cap reached ... (issue #688) error used when nothing matched. The error doesn't mention the match it found, and the code didn't say that this outcome was intended.

Fix (decision: document as refuse-only and keep the cap)

• Keep the cap at 5 pages. The request filters on class, engine, multi-AZ, duration and payment option, so a result set near 500 rows isn't expected (the issue says this hasn't been seen). Raising the cap would only move the boundary and spend more of the Lambda budget that #688 protects. The maxOfferingPages doc comment now gives this reasoning.
• Refuse when the cap hides uniqueness. offeringPageAllowed now gets the match found so far. If the cap is reached after a match, it returns an error that names the offering, says it cannot be proven unique, and says it is refusing to purchase (issue #303). It never returns the unverified match. When nothing matched, the old (issue #688) error is unchanged. Both errors still contain pagination cap reached.
• Fixed the stale paginateRDSOfferings doc comment, which said it returns "the first matching offering ID".

No purchase path changes: before and after, this case ends in an error and no purchase. The change makes that outcome explicit and pins it with tests.

Closes #303

Verification

All evidence comes from unit tests with a mocked MockRDSClient. No real AWS APIs were called.

New tests in providers/aws/services/rds/client_test.go:

• TestFindOfferingID_MatchBeforeCapRefusesWhenMorePagesRemain: 5 pages, each with a next marker, and an exact match on page 1 or on page 5. It checks for an empty ID, an error containing pagination cap reached, the matched ID, cannot be proven unique and refusing to purchase, and exactly maxOfferingPages API calls (nothing fetched past the cap).
• TestFindOfferingID_MatchOnLastAllowedPageSucceeds: tests the boundary. A match on page 5 with no further marker completes the walk and returns the ID.

Fail-before / pass-after:

# before (client.go fix stashed, new tests kept)
cd providers/aws && go test ./services/rds/ -run 'MatchBeforeCap|LastAllowed' -count=1
--- FAIL: TestFindOfferingID_MatchBeforeCapRefusesWhenMorePagesRemain/match_on_page_1
    "pagination cap reached after 5 pages for RDS db.r6g.large mysql multi-az=false all-upfront (issue #688)" does not contain "\"offering-early\""
    ... does not contain "cannot be proven unique" / "refusing to purchase"
--- FAIL: .../match_on_page_5 (same)
FAIL

# after
--- PASS: TestFindOfferingID_MatchBeforeCapRefusesWhenMorePagesRemain (0.00s)
--- PASS: TestFindOfferingID_MatchOnLastAllowedPageSucceeds (0.00s)
--- PASS: TestFindOfferingID_PaginationCapFires (0.00s)   # existing no-match cap test unchanged

The boundary test passes both before and after. It's there to guard against an off-by-one in the cap, not to show the fix.

Also run, all clean:

cd providers/aws
AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null go test -race -short -count=1 ./...   # rc=0
go build ./... && go vet ./...
golangci-lint v2.10.1 run --allow-parallel-runners ./services/rds/...   # 0 issues
gocyclo -over 10 services/rds/client.go                                 # silent
gofmt -l services/rds/ ; git diff --check ; make tidy-check             # clean

AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null was set only because the local machine's default AWS profile is invalid. The tests don't need credentials.

Not covered: a real account result set large enough to reach the cap. I haven't seen one, and this PR doesn't exercise one.

Out of scope

• fix(aws/rds): refuse the 'SQL Server' engine spelling in the licensed-engine check #302 (refuse the SQL Server engine spelling in lookupEngineName) is in the same package and is being fixed separately. This PR leaves it alone.
• client.go was already over the 500-line guideline in CLAUDE.md before this change (about 740 lines). This PR adds about 15 lines and doesn't split the file.

Labels

Maintainer, please copy the issue's triage labels onto this PR: triaged, priority/p3, severity/low, urgency/eventually, impact/few, effort/xs, type/bug.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 2 billable files and costs up to $0.50.}

Or wait 48 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: fcc7368b-0be8-4783-a094-69b0539bef41

📥 Commits

Reviewing files that changed from the base of the PR and between 160f6d6 and 223d789.

📒 Files selected for processing: 2

• providers/aws/services/rds/client.go
• providers/aws/services/rds/client_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent adversarial review: CLEAN at dbac61dabd4df8ddc554ea9ad600f2e8615fe3ef (claude-go-pr-merge-2, non-author reviewer). No blocking findings.

• findOfferingID returns a match only via the loop break (nil marker) after pickOfferingID, so no path returns an unverified match. Page counting unchanged: exactly 5 pages with a nil marker still succeeds; refusal only when page 5 returned a marker. offeringPageAllowed has no other callers.
• Fail-before: with origin/main's client.go, TestFindOfferingID_MatchBeforeCapRefusesWhenMorePagesRemain fails; passes at this head.
• Mutations: cap raised to maxOfferingPages+1 fails TestFindOfferingID_PaginationCapFires; found != "" forced false fails the new test.
• Local verification (macOS, unit/mock, no AWS): (cd providers/aws && go test -race -short ./services/rds/... ./recommendations/...) pass; make build, make vet pass.

Still needed before merge: owner approval of the fork CI runs and green CI on this SHA, and the author marking the PR ready. Labels mirrored from the linked issue. CodeRabbit skipped this draft; CR skipped: quota/draft, so the merge comment will carry CR waived: quota, adversarial review + local verification + green CI once CI is green.

[cristim]

Merge gate record at 223d7893cdc8ab135146581aa96e731e85d0f265 (merge driver)

• Independent review CLEAN at dbac61da (verdict above). Each first-parent commit after it is a merge of main, and each merge's tree equals git merge-tree --write-tree of its parents. The PR's own change is therefore unchanged and the verdict stands. The latest merge is GitHub's Update branch, which the author consented to on fix(azure): attribute shared-scope vm reservations to their billing subscription #423.
• Local verification on macOS at this head (fixture/mock evidence, no live cloud calls): cd providers/aws && go test -race -short ./... && go vet ./... passes.
• CI green on this head: CI - Build & Test 39051326868, pre-commit 39051326857. mergeState CLEAN, no unresolved threads, labels mirrored from fix(aws/rds): offering lookup scans every page, so a match past the page cap now errors #303. CR waived (quota): adversarial review + local verification + green CI.

Squash-merging with --match-head-commit 223d7893.