fix(purchases): narrow Describe*Offerings calls + cap pagination across 7 services (lookup root cause behind 60s budget overrun)

[cristim]

Symptom

Execution 9336a111-85c5-4b74-9991-367350de9bac (Lambda 60s budget, dev env, pre-#684) failed with:

EC2: DescribeReservedInstancesOfferings ... canceled, context deadline exceeded
AUDIT LOSS: failed to save execution status: failed to begin transaction: context deadline exceeded

CloudWatch report: Duration: 60000.77 ms -- the Lambda spent its entire budget paginating through DescribeReservedInstancesOfferings and was killed before it could even write the failed status (compounding into #632-style audit loss).

Reproduction via AWS CLI:

aws ec2 describe-reserved-instances-offerings ... --max-results 100
  -> returns 0 offerings + NextToken
aws ec2 describe-reserved-instances-offerings ...  (no max-results)
  -> never returns; killed manually

Distinction from #667

#667 diagnosed the same failure shape as an SDK-retry budget overrun and asserted "This is NOT a pagination issue -- buildOfferingFilters correctly narrows by ... so the result set is typically a single page." The CLI reproduction above falsifies that assertion: AWS returns empty pages with NextToken even for the narrow filter set, walking sparse offerings indefinitely. #684 (closing #667) caps the per-rec wall clock at 30s via context.WithTimeout and makes hangs observable, but the underlying lookup still spends the full budget; the rec fails every time it hits this offering layout. This issue is the lookup-correctness fix that #684's budget cap is masking.

Root cause: offering lookup is under-constrained

providers/aws/services/ec2/client.go:320-346 (buildOfferingFilters):

return []types.Filter{
    {Name: aws.String("instance-type"),       Values: []string{rec.ResourceType}},
    {Name: aws.String("product-description"), Values: []string{platform}},
    {Name: aws.String("instance-tenancy"),    Values: []string{tenancy}},
    {Name: aws.String("scope"),               Values: []string{scope}},
    {Name: aws.String("duration"),            Values: []string{strconv.FormatInt(c.getDurationValue(rec.Term), 10)}},
    {Name: aws.String("offering-class"),      Values: []string{c.getOfferingClass(rec.PaymentOption)}},  // always "convertible"
}

Critical gap: the offering-type filter (payment option) is never set. AWS therefore returns every payment variant of every offering that matches the other filters, and the loop at client.go:367-398 then returns result.ReservedInstancesOfferings[0] from the first non-empty page without verifying its OfferingType matches the requested payment option.

Two failure modes follow:

1. Deep pagination on sparse offerings: AWS walks pages of empty results with NextToken until it lands on a matching offering set or the budget expires.
2. Wrong-variant return: if the first non-empty page contains a no-upfront variant when the rec asked for all-upfront (or vice versa), the wrong offering is purchased silently.

The same shape recurs across the sibling services that copy buildOfferingFilters-style logic: RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans. Each has its own filter syntax (Filters[] for EC2, direct fields for the others) but the same "filter set under-constrains the API" pattern likely applies.

Fix scope: sweep all 7 purchase-path services

For each of providers/aws/services/{ec2,rds,elasticache,memorydb,opensearch,redshift,savingsplans}/client.go:

1. Narrow the describe call to the minimum needed for a deterministic match. Each service's Describe*Offerings API supports filters beyond what we currently set; identify the maximally narrow filter set (region is implicit via the client's region; instance/node/cluster type, term, payment option, offering class, product description, tenancy/scope, etc. where applicable) and apply all of them.

2. Add the missing payment-option filter explicitly (EC2 offering-type, equivalents per service):

   Rec value	AWS value
   no-upfront	No Upfront
   partial-upfront	Partial Upfront
   all-upfront	All Upfront

3. Verify the returned offering's payment option matches before returning it (defense in depth -- even with the filter, the loop should reject mismatched variants instead of grabbing offerings[0]).

4. Cap pagination at a fixed page count (e.g. maxOfferingPages = 5 -- with MaxResults=100 that's up to 500 offerings) and fail fast with an explicit pagination cap reached after N pages error that names the rec. This converts "Lambda timeout + audit loss" into "fast diagnostic error + clean status save".

5. Honour ctx.Done() between pages so a parent-context cancellation surfaces immediately instead of waiting for the next AWS response.

6. Increase MaxResults from 100 to 1000 (the AWS max for these APIs) so fewer round-trips are needed when a match is buried.

Acceptance criteria

• Every findOfferingID (or equivalent) across the 7 services applies the maximally narrow filter set, including the payment-option mapping
• Each returns an error if AWS hands back an offering whose payment option does not match the rec
• Each caps pagination at a fixed page count and emits a diagnostic error citing the rec details
• Each checks ctx.Err() between pages
• Regression test per service: mock returning N empty pages with NextToken -> assert fast failure with the cap error (not a context-deadline error)
• Regression test per service: mock returning a non-matching offering on page 1 -> assert it is rejected (not silently purchased)
• CloudWatch repro: t4g.nano rec on the dev env returns a deterministic error within seconds instead of timing out at 60s

Risk / blast radius

• Touches all 7 purchase-path service clients. Each has unit tests; new tests are required.
• The verification step (AWS + Azure: add read-only sanity checks + AWS RI exchange #3) tightens semantics: a previously-silent-wrong-variant purchase becomes a loud error. This is the correct behaviour but may surface real recs that have ambiguous PaymentOption strings.
• No frontend / API contract change.

Cross-references

• fix(purchases): EC2 RI purchase times out under transient AWS SDK retries; Lambda 60s budget blown (P0) #667 (closing-PR fix(purchases): tighten AWS SDK retry budget + bump Lambda timeout to 300s (closes #683) #684) -- this issue is the lookup-correctness sibling
• fix(purchases): tighten AWS SDK retry budget + bump Lambda timeout to 300s (closes #683) #684 -- per-rec 30s WithTimeout makes this fail faster; this issue makes it not fail at all
• bug(purchases): approve strands execution in 'approved' (purchase never runs, no error) on interrupted sync execution #632 -- the audit-loss compounding symptom (when the lookup eats the whole budget, the failed-status write can't run)

[cristim]

Surveying the codebase outside the PR #690 footprint (Azure, GCP, and AWS recommendations client) for the same failure patterns.

---

Smell #2 + #1: Azure pagination loops -- no ctx.Err() check, no page cap

All five Azure service clients iterate for pager.More() { pager.NextPage(ctx) } for three operations each (GetRecommendations, GetExistingCommitments, GetValidResourceTypes) with no page budget and no ctx.Err() guard between pages. The Azure SDK pager does propagate context cancellation through NextPage(ctx), but only on the next call -- a slow page that takes the remaining Lambda budget will not be interrupted mid-wait.

• providers/azure/services/compute/client.go:136,171,362 -- Smell Setup CI/CD Process #2, medium effort (5 loops, 3 files share identical shape)
• providers/azure/services/cache/client.go:136,170,359 -- Smell Setup CI/CD Process #2
• providers/azure/services/database/client.go:138,172 -- Smell Setup CI/CD Process #2
• providers/azure/services/cosmosdb/client.go:139,173,364 -- Smell Setup CI/CD Process #2
• providers/azure/services/search/client.go:136,170,358 -- Smell Setup CI/CD Process #2

Risk: could exhaust Lambda budget on a slow or high-volume subscription result set.
Effort: small (add if err := ctx.Err(); err != nil { return ..., err } at the top of each loop body, add a page cap constant).

---

Smell #5: Azure Retail Prices API -- NextPageLink parsed but never followed

AzureRetailPrice structs in cache/client.go:115, database/client.go:117, cosmosdb/client.go:118, and search/client.go:115 decode NextPageLink from the response but none of the get*Pricing functions follow it. If the pricing API paginates (it does for broad queries like Cosmos DB's serviceName eq 'Azure Cosmos DB' without an SKU filter), only page-1 pricing is used. The compute client (compute/client.go) decodes into a bare AzureRetailPrice struct with no NextPageLink field at all, so it always truncates silently.

• providers/azure/services/cosmosdb/client.go:417-491 -- Smell Remove Hard SES dependency #5, risk: could silently truncate pricing data, producing wrong cost estimates
• providers/azure/services/search/client.go:408-483 -- Smell Remove Hard SES dependency #5 (broad filter, no SKU predicate)
• providers/azure/services/cache/client.go:417-492 -- Smell Remove Hard SES dependency #5
• providers/azure/services/database/client.go:425-499 -- Smell Remove Hard SES dependency #5
• providers/azure/services/compute/client.go:410-484 -- Smell Remove Hard SES dependency #5 (no NextPageLink field at all)

Risk: silently truncates; wrong pricing leads to bad purchase decisions.
Effort: small-medium (add NextPageLink follow loop in each get*Pricing helper).

---

Smell #5: GCP Cloud Billing ListSKUs -- single-page call, no pagination

realBillingService.ListSKUs calls .Do() once and returns. The cloudbilling REST API returns a nextPageToken in ListSkusResponse; SKU catalogs regularly span multiple pages. The callers in computeengine/client.go:485-520, cloudsql/client.go:398-430, and memorystore/client.go:377-411 silently truncate at the first page.

• providers/gcp/services/computeengine/client.go:143-145 (realBillingService.ListSKUs) -- Smell Remove Hard SES dependency #5, risk: silently truncates SKU catalog; pricing lookup may return zero, triggering hardcoded fallback discount
• providers/gcp/services/cloudsql/client.go:101-103 -- same
• providers/gcp/services/memorystore/client.go:113-115 -- same

Effort: small (add PageToken loop in ListSKUs; the interface signature can stay the same by collecting all pages internally).

---

Smell #2: GCP iterator loops -- no ctx.Err() check

computeengine/client.go:205,247,440, cloudsql/client.go:163, memorystore/client.go:163,202 use unbounded for { it.Next() } loops. GCP iterators honour context cancellation on Next(), but only surface it as an error -- there is no early-exit check between iterations.

• providers/gcp/services/computeengine/client.go:205 -- Smell Setup CI/CD Process #2
• providers/gcp/services/computeengine/client.go:247 -- Smell Setup CI/CD Process #2

Risk: could exhaust Lambda budget on large result sets before the iterator surfaces cancellation.
Effort: trivial (check ctx.Err() at loop top).

---

Summary: 14 distinct loop locations across 8 files. Primary smells: #2 (no ctx.Err() between pages) and #5 (single-page calls that silently truncate). No smell #3 (typed-field vs Filters[]) or #4 (unverified [0] return) found outside the PR #690 footprint.

[cristim]

Context Cancellation Findings

Found 2 context-cancellation issues in AWS recommendations code (outside PR #690 scope):

Finding 1: GetAllRecommendations (providers/aws/recommendations/client.go:106-127)

• Loops services calling GetRecommendationsForService; silently continues on error
• Returns partial results + nil error when context deadline fires mid-loop
• Line 119-120: if err != nil { continue } then return allRecommendations, nil

Finding 2: getSavingsPlansRecommendations (providers/aws/recommendations/client.go:418-464)

• Loops plan types; rate-limiter + continue on error
• Same pattern: partial results + nil error when deadline fires mid-loop
• Line 452-454: if err != nil { fmt.Printf(...); continue } then return allRecommendations, nil

Both match feedback_ctx_cancel_terminal.md: never continue on context.Canceled/DeadlineExceeded; return immediately.

[cristim]

AWS follow-up survey (outside PR #690 footprint)

Surveyed all AWS Go files outside the 7 excluded service dirs:

• providers/aws/recommendations/client.go + ratelimiter.go
• providers/aws/provider.go
• providers/aws/internal/tagging/purchase_tags.go
• providers/aws/service_client.go
• cmd/helpers.go, cmd/main.go, cmd/multi_service.go

---

Findings

F1 — cmd/multi_service.go:1335 (smell 5 — single-page call that should paginate)
queryMajorEngineVersions calls rdsClient.DescribeDBMajorEngineVersions once per engine type and reads output.DBMajorEngineVersions without threading the Marker token. The SDK confirms both Input.Marker and Output.Marker exist (rds@v1.97.3). On accounts with many engine versions the last page(s) are silently dropped, producing incomplete extended-support data used to filter RI recommendations.
Effort: small.

F2 — providers/aws/recommendations/client.go:79 (smell 5 — single-page call that should paginate)
GetRecommendations calls GetReservationPurchaseRecommendation once and never reads result.NextPageToken. The CE SDK (costexplorer@v1.61.0) exposes both Input.NextPageToken and Output.NextPageToken. Large payer accounts (>100 instance-type/region/term combinations) will silently miss later pages of recommendations.
Effort: small.

F3 — providers/aws/recommendations/client.go:446 (smell 5 — single-page call that should paginate)
getSavingsPlansRecommendations calls GetSavingsPlansPurchaseRecommendation once per plan type without threading NextPageToken. Same root cause as F2; accounts with many SP details can have results truncated.
Effort: small.

F4 — cmd/multi_service.go:1265 (smell 2 — context cancellation swallowed in pagination loop)
queryRunningInstanceEngineVersions runs DescribeDBInstances in a goroutine-per-region fan-out. When the call fails (line 1265), the error is logged and the goroutine breaks — but context cancellation (ctx.Err()) is treated identically to a transient API error. The parent wg.Wait() at line 1302 then returns a partial result with no error, silently producing an incomplete instance-version map that feeds the recommendation filter.
Effort: small.

---

Summary

4 findings, all in the AWS-only paths outside PR #690. No High-risk/Small-effort finding that clearly belongs on #691 (which is bounded-pagination on describe/list calls; F2 and F3 are CE purchase-recommendation single-page calls — related shape but different service). F1, F2, F3 are all smell 5 (missing pagination); F4 is smell 2 (ctx cancellation swallowed). Deferring to you on whether any of these warrant adding to #691 or a new issue.

[cristim]

PR #690 (merged into feat/multicloud-web-frontend) addresses this issue end-to-end across the four offering-lookup-bearing commits on the branch:

• 237c30f — initial fix: narrow Describe*Offerings server-side filters (EC2 OfferingType, MemoryDB OfferingType + Duration); add pagination cap at 5 pages (maxOfferingPages); ctx.Err() between pages; defense-in-depth payment-variant verification; per-page timing log; cover the seven purchase clients (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans); 21 new regression tests.
• 5f8f269 — EC2 follow-up: live-AWS verification showed that packing InstanceType/ProductDescription/InstanceTenancy/Duration/OfferingClass into Filters[] was what caused the empty-page walk; promote them to typed top-level fields on DescribeReservedInstancesOfferingsInput so AWS hits its primary indices and returns the matching offering immediately.
• 95eca72 — soft-skip guards + nil/empty token parity: EC2/RDS pagination terminator guards against both nil and empty-string NextToken/Marker; EC2 uses an isLastEC2Page helper for clarity; OpenSearch removes a redundant post-match defense-in-depth that could never fire.
• f2becf7 — CR fix(purchases): narrow Describe*Offerings + cap pagination (#688) #690 follow-up: ElastiCache pagination terminator gets the same nil-or-empty guard; ElastiCache convertPaymentOption now returns (string, error) so unknown payment options surface explicitly instead of being silently coerced to "Partial Upfront"; OpenSearch and Redshift unreachable variant-mismatch guards repositioned to fire on first mismatch.

CI sanity, AWS/Azure read-only dry runs, and pre-commit (gocyclo, vet, gofmt) all pass on the merged commit. CodeRabbit's review on PR #690 was addressed across the four commits above.

Recommending close.