fix(purchases): narrow Describe*Offerings + cap pagination (#688) - #690
Merged
cristim merged 5 commits intoMay 22, 2026
Merged
Conversation
Root cause: EC2 findOfferingID was missing the offering-type filter, causing AWS to return all payment variants, triggering indefinite pagination through sparse empty pages and burning the entire Lambda 60s budget (execution 9336a111). Changes per service (all 7 purchase-path clients): - EC2: add OfferingType direct field + convertEC2PaymentOption helper; verify returned offering type before returning; cap at 5 pages; check ctx.Err() between pages; add timing log per page - RDS: add pagination cap + ctx.Err check + variant verification + per-page timing log (already had all filter fields) - ElastiCache: same as RDS - MemoryDB: add OfferingType + Duration direct fields to narrow at API level; replace client-side matchesDuration/matchesOfferingType loop with server-side filters; cap + ctx.Err + variant verification - OpenSearch: API has no filter fields; add cap + ctx.Err + defense- in-depth variant check after client-side matching - Redshift: API has no node-type/payment filter fields; add cap + ctx.Err + unknown-type guard - SavingsPlans: add pagination loop + cap + ctx.Err to lookupOfferingID (was a single non-paginated call) MaxResults/MaxRecords: EC2/Redshift/RDS/ElastiCache APIs have a hard max of 100 per page; no change from current value. MemoryDB/OpenSearch were already at 100. Regression tests (3 per service = 21 new tests): - Pagination cap fires after maxOfferingPages empty pages + next token - Wrong-variant offering rejected by defense-in-depth guard - Happy path returns correct offering ID on first page Refs #684 #667 #632
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR caps and paginates Describe*Offerings across EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, and SavingsPlans; adds payment-option mapping/validation and per-page ctx cancellation checks; and updates tests to cover pagination caps, wrong-variant rejection, and happy paths. ChangesReserved Instance / Node Offering Lookup Pagination and Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
…ribe (#688) Followup on the same #688 issue. The initial commit kept InstanceType, ProductDescription, InstanceTenancy, duration, and OfferingClass packed into DescribeReservedInstancesOfferingsInput.Filters[], with only OfferingType promoted to the typed top-level field. Live-AWS verification showed that the Filter[]-heavy shape is what causes AWS to return empty pages with NextToken on sparse offering sets (e.g. t4g.nano regional no-upfront convertible), walking until the Lambda budget expires. The all-typed-fields shape returns the exact matching offering immediately. Typed fields land on AWS's primary indices; Filter[] is a generic fallback that does not always hit the same indices. Changes: - Move InstanceType, ProductDescription, InstanceTenancy, MinDuration, MaxDuration, OfferingClass to typed top-level fields on the input struct. - Keep only scope as a Filter[] entry (no typed equivalent on the input). - Use the typed enum constants (types.InstanceType, types.RIProductDescription, types.Tenancy, types.OfferingClassTypeConvertible) instead of stringly-typed filter values. - Extract ec2OfferingQuery struct + buildEC2OfferingQuery / describeInputFromQuery helpers so findOfferingID stays under the gocyclo cap. - Delete the now-unused buildOfferingFilters helper and the always-returns- "convertible" getOfferingClass helper. - Change scanEC2OfferingPage from "hard error on variant mismatch" to "soft skip + log and continue scanning". With the typed OfferingType field set this should never fire; if AWS somehow returns a mismatched variant we want to keep looking on later pages rather than fail the rec. - Update TestFindOfferingID_WrongVariantRejected to assert the new soft-skip behaviour (mismatched variant on the only page -> "no offerings found"). - Drop TestBuildOfferingFilters_LegacyCanonicalization: it exercised the deleted helper. The underlying canonicalizeEC2Tenancy / canonicalizeEC2Scope helpers retain their own direct unit tests.
Member
Author
|
Followup commit @coderabbitai review |
Contributor
|
Triggering a review of the latest commit now. ✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@providers/aws/services/ec2/client.go`:
- Around line 412-416: The pagination loop only treats result.NextToken == nil
as end-of-pagination; change the termination condition to also treat an empty
string as end-of-pagination by checking result.NextToken == nil ||
*result.NextToken == "" before breaking, otherwise assign nextToken =
result.NextToken; this ensures result.NextToken being "" stops the loop and
avoids redundant calls (refer to result.NextToken and nextToken in the EC2
client pagination code).
In `@providers/aws/services/elasticache/client.go`:
- Around line 270-271: The code currently calls
c.convertPaymentOption(rec.PaymentOption) and assigns offeringType which
silently coerces unknown values to "Partial Upfront"; update the flow in the
function that uses offeringType (before any pagination or downstream processing)
to detect unsupported payment options by validating the result of
c.convertPaymentOption(rec.PaymentOption) and return an explicit error instead
of proceeding—e.g., have convertPaymentOption return (string, error) or call a
validator after conversion, and if unsupported return an error immediately (do
not rely on getDurationString(rec.Term) or continue pagination).
- Around line 304-308: The pagination loop in client.go currently only breaks
when result.Marker == nil, so if AWS returns an empty string pointer
(aws.String("")) the loop will continue; update the loop that reads
result.Marker (the marker variable and the surrounding pagination logic) to
treat both nil and empty string as end-of-pagination by checking the pointer
value (e.g., use aws.StringValue(result.Marker) == "" or explicit nil check +
*result.Marker == "") and break when either condition is met.
In `@providers/aws/services/opensearch/client.go`:
- Around line 407-418: The explicit payment-option mismatch check is unreachable
because matchesPaymentOption(...) pre-filters and continues before the guard;
move the stronger validation earlier: before the continue, compute wantPayment
via normalizeOpenSearchPaymentOption(rec.PaymentOption) and gotPayment from
string(offering.PaymentOption) and if they differ return the fmt.Errorf (using
aws.ToString(offering.ReservedInstanceOfferingId), gotPayment, wantPayment,
rec.ResourceType, rec.PaymentOption); only after this stricter equality check
call matchesPaymentOption or keep the existing continue logic so the mismatch
branch can actually trigger and surface unexpected enum/representation
differences.
- Around line 387-391: The loop currently only stops when result.NextToken ==
nil; change the termination check to treat both nil and an empty string as
terminal: after receiving result.NextToken, if result.NextToken == nil OR
(result.NextToken != nil AND *result.NextToken == "") then break; otherwise
assign nextToken = result.NextToken and continue. Apply this change around the
pagination loop where NextToken and result.NextToken are used to prevent
redundant calls and false cap errors.
In `@providers/aws/services/rds/client.go`:
- Around line 339-342: The pagination loop currently only breaks when
result.Marker is nil, but a non-nil empty marker (e.g., aws SDK returning
pointer to empty string) should also be treated as the terminal page; update the
loop that uses result.Marker and the local marker variable so it breaks when
result.Marker == nil || *result.Marker == "" (or otherwise check for empty
string value) before assigning marker = result.Marker, ensuring you reference
and update the code paths around result.Marker and marker in the pagination
logic.
In `@providers/aws/services/redshift/client.go`:
- Around line 423-427: The loop currently only breaks when result.Marker is nil,
which can still issue an extra request if result.Marker is non-nil but empty;
update the pagination check to treat an empty string as end-of-pagination by
changing the condition to break when result.Marker == nil ||
aws.StringValue(result.Marker) == "" (or dereference and compare to "" if not
using aws helpers), and only assign marker = result.Marker when the marker is
non-empty so subsequent requests stop correctly; reference the variables
result.Marker and marker in the pagination loop.
- Around line 443-453: The explicit enum guard is unreachable because
c.matchesOfferingType(...) filters unknown ReservedNodeOfferingType values
first; move the enum validation so it runs before calling c.matchesOfferingType.
Specifically, read offering.ReservedNodeOfferingType into offeringTypeStr and
validate it equals "Regular" or "Upgradable" (returning the same fmt.Errorf with
aws.ToString(offering.ReservedNodeOfferingId) and rec.ResourceType on mismatch)
before invoking c.matchesOfferingType(rec.PaymentOption) so the diagnostic path
can ever be hit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e8ef2669-aeb2-4521-a9d7-0c8c687995f2
📒 Files selected for processing (14)
providers/aws/services/ec2/client.goproviders/aws/services/ec2/client_test.goproviders/aws/services/elasticache/client.goproviders/aws/services/elasticache/client_test.goproviders/aws/services/memorydb/client.goproviders/aws/services/memorydb/client_test.goproviders/aws/services/opensearch/client.goproviders/aws/services/opensearch/client_test.goproviders/aws/services/rds/client.goproviders/aws/services/rds/client_test.goproviders/aws/services/redshift/client.goproviders/aws/services/redshift/client_test.goproviders/aws/services/savingsplans/client.goproviders/aws/services/savingsplans/client_test.go
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
providers/aws/services/ec2/client.go (1)
435-438:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTreat empty
NextTokenas end-of-pagination.This loop still only stops on
nil. If EC2 returnsNextToken: "", you'll keep reissuing the request untilmaxOfferingPagesand surface a false cap error instead of the expected exhaustion path.Suggested fix
- if result.NextToken == nil { + if result.NextToken == nil || aws.ToString(result.NextToken) == "" { break } nextToken = result.NextToken🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@providers/aws/services/ec2/client.go` around lines 435 - 438, The pagination loop in providers/aws/services/ec2/client.go only breaks when result.NextToken is nil, which misses the case where AWS returns an empty string; update the loop around result.NextToken so it treats both nil and empty string as end-of-pagination: check if result.NextToken == nil OR the dereferenced *result.NextToken == "" and break in that case, otherwise assign nextToken = result.NextToken; ensure any references to maxOfferingPages remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@providers/aws/services/ec2/client.go`:
- Around line 450-458: The scanEC2OfferingPage function currently returns an
empty string if it encounters a matching offering whose
ReservedInstancesOfferingId is nil/empty, which causes the caller to treat the
entire page as “no match” and skip later valid offerings; update
scanEC2OfferingPage so that inside the loop, after you verify o.OfferingType ==
wantType, you check if aws.ToString(o.ReservedInstancesOfferingId) is empty and,
if so, log/skip that offering and continue the loop instead of returning an
empty string, only returning a non-empty ID when found or returning "" after
scanning all offerings.
---
Duplicate comments:
In `@providers/aws/services/ec2/client.go`:
- Around line 435-438: The pagination loop in
providers/aws/services/ec2/client.go only breaks when result.NextToken is nil,
which misses the case where AWS returns an empty string; update the loop around
result.NextToken so it treats both nil and empty string as end-of-pagination:
check if result.NextToken == nil OR the dereferenced *result.NextToken == "" and
break in that case, otherwise assign nextToken = result.NextToken; ensure any
references to maxOfferingPages remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fd3e9e1-31d0-40e0-9322-2300916c5277
📒 Files selected for processing (2)
providers/aws/services/ec2/client.goproviders/aws/services/ec2/client_test.go
Comment on lines
+450
to
+458
| func scanEC2OfferingPage(offerings []types.ReservedInstancesOffering, wantType types.OfferingTypeValues) string { | ||
| for _, o := range offerings { | ||
| if o.OfferingType != wantType { | ||
| log.Printf("EC2 findOfferingID skipping mismatched variant %s (got %q want %q)", | ||
| aws.ToString(o.ReservedInstancesOfferingId), o.OfferingType, wantType) | ||
| continue | ||
| } | ||
| return aws.ToString(o.ReservedInstancesOfferingId) | ||
| } |
Contributor
There was a problem hiding this comment.
Skip empty offering IDs instead of aborting the page scan.
Line 457 returns "" when a matching offering has a nil/empty ReservedInstancesOfferingId. The caller treats that as “no match on this page”, so any later valid offering on the same page is never inspected.
Suggested fix
for _, o := range offerings {
if o.OfferingType != wantType {
log.Printf("EC2 findOfferingID skipping mismatched variant %s (got %q want %q)",
aws.ToString(o.ReservedInstancesOfferingId), o.OfferingType, wantType)
continue
}
- return aws.ToString(o.ReservedInstancesOfferingId)
+ id := aws.ToString(o.ReservedInstancesOfferingId)
+ if id == "" {
+ log.Printf("EC2 findOfferingID skipping matching variant with empty offering ID (want %q)", wantType)
+ continue
+ }
+ return id
}
return ""
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| func scanEC2OfferingPage(offerings []types.ReservedInstancesOffering, wantType types.OfferingTypeValues) string { | |
| for _, o := range offerings { | |
| if o.OfferingType != wantType { | |
| log.Printf("EC2 findOfferingID skipping mismatched variant %s (got %q want %q)", | |
| aws.ToString(o.ReservedInstancesOfferingId), o.OfferingType, wantType) | |
| continue | |
| } | |
| return aws.ToString(o.ReservedInstancesOfferingId) | |
| } | |
| func scanEC2OfferingPage(offerings []types.ReservedInstancesOffering, wantType types.OfferingTypeValues) string { | |
| for _, o := range offerings { | |
| if o.OfferingType != wantType { | |
| log.Printf("EC2 findOfferingID skipping mismatched variant %s (got %q want %q)", | |
| aws.ToString(o.ReservedInstancesOfferingId), o.OfferingType, wantType) | |
| continue | |
| } | |
| id := aws.ToString(o.ReservedInstancesOfferingId) | |
| if id == "" { | |
| log.Printf("EC2 findOfferingID skipping matching variant with empty offering ID (want %q)", wantType) | |
| continue | |
| } | |
| return id | |
| } | |
| return "" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@providers/aws/services/ec2/client.go` around lines 450 - 458, The
scanEC2OfferingPage function currently returns an empty string if it encounters
a matching offering whose ReservedInstancesOfferingId is nil/empty, which causes
the caller to treat the entire page as “no match” and skip later valid
offerings; update scanEC2OfferingPage so that inside the loop, after you verify
o.OfferingType == wantType, you check if
aws.ToString(o.ReservedInstancesOfferingId) is empty and, if so, log/skip that
offering and continue the loop instead of returning an empty string, only
returning a non-empty ID when found or returning "" after scanning all
offerings.
Three related cleanups on the offering-lookup pipeline: 1. EC2/RDS: normalize end-of-pagination check to guard against both nil and empty-string NextToken/Marker. AWS may return either form for the terminal page; only checking nil risks an extra empty-page request. EC2 uses a new isLastEC2Page helper so the intent is explicit. 2. OpenSearch: remove the post-match payment-option defense-in-depth guard from scanOpenSearchOfferingPage. matchesPaymentOption already filters mismatched variants in the same loop iteration; the redundant hard error can never fire in practice and makes the wrong-variant test harder to reason about. Soft outcome (loop exhausts -> "no offerings found") is safer for the caller. 3. Redshift: same rationale as OpenSearch -- remove the hard-error guard for unknown offering type from scanRedshiftOfferingPage. Update TestFindOfferingID_WrongVariantRejected for both services to assert the soft-failure shape (err != nil, id == "").
Category A (4 sites, nil-or-empty pagination terminator): - elasticache: guard result.Marker against both nil and "" in paginateElastiCacheOfferings; avoid the redundant page that a non-nil empty-string marker would otherwise trigger - opensearch, rds, redshift: same nil-or-empty guard on their respective NextToken/Marker fields (EC2 was already fixed in 5f8f269) Category B (ElastiCache payment option coercion): - convertPaymentOption now returns (string, error); unknown values return an explicit "unsupported ElastiCache payment option" error instead of silently mapping to "Partial Upfront" - findOfferingID propagates the error before any pagination begins - TestClient_ConvertPaymentOption updated to assert the (result, error) return shape and verify the unknown-input error path Category C (unreachable variant-mismatch guard): - OpenSearch: replace the matchesPaymentOption pre-filter continue with a direct gotPayment != wantPayment check that returns an explicit "payment option mismatch" error; guard is now reachable - Redshift: replace matchesOfferingType pre-filter continue with a direct "Regular"/"Upgradable" enum check that returns an explicit "unexpected type" error on first mismatch - Test comments and assertions updated to match the new behaviour (mismatch surfaces as a diagnostic error, not a silent skip) CR #690 review round 1
Member
Author
|
@coderabbitai review |
1 similar comment
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
1 similar comment
Contributor
✅ Actions performedReview triggered.
|
The previous commit accidentally included Terraform provider constraint changes unrelated to the #688 offering-lookup fix. Restore the three lock files to their state on feat/multicloud-web-frontend so this branch stays focused on the purchase-path narrowing changes only.
Member
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
6 tasks
This was referenced May 22, 2026
cristim
added a commit
that referenced
this pull request
May 22, 2026
) (#693) GetReservationPurchaseRecommendation, GetSavingsPlansPurchaseRecommendation, and DescribeDBMajorEngineVersions each silently discarded every page after the first. For large payer accounts, CE returns recommendations across multiple pages, so callers were seeing a fraction of the real result set. All three call sites now drive a token-driven pagination loop. The existing rate-limiter retry loop stays as the inner (per-page) loop. ctx.Err() is checked at the top of each iteration (feedback_ctx_cancel_terminal). The loop terminates on nil OR empty-string token (PR #690 parity). Exceeding maxRecommendationPages/maxEngineVersionPages = 20 returns a diagnostic error citing the service and issue #692. gocyclo relief: fetchSPAllPages + fetchSPPageWithRetry extracted for SP; queryMajorEngineVersionsWithClient + fetchMajorEngineVersionsForEngine extracted for RDS (all stay under cap 15). Three new test groups: multi-page accumulation (3 pages), empty-token terminal (no extra call), and pagination cap error. Updated TestGetRecommendations_ContextCancellation to assert context.Canceled directly -- the new ctx.Err() guard at the top of the pagination loop fires before the rate-limiter so the error path changed correctly.
cristim
added a commit
that referenced
this pull request
May 25, 2026
closes #695) (#696) * feat(ri-exchange): target-offerings endpoint + UUID validation + AWS 4xx mapping (#695) Three defects addressed: Defect 1 (backend): add GET /api/ri-exchange/target-offerings backed by ListTargetOfferings on ec2svc.Client. Uses typed-field approach from PR #690 to enumerate valid convertible exchange targets for a source RI. Introduces targetOfferingsEC2Client interface + factory in handler.go for testability. Defect 2: validate targets[].offering_id against a UUID regex at both the getExchangeQuote and validateExecuteExchangeBody call sites. Returns 400 with a descriptive message when an instance type (e.g. "t3.medium") is submitted instead of an offering UUID. Defect 3: mapAWSExchangeError helper extracts smithy.APIError and maps InvalidOfferingId/InvalidParameter/ValidationError/Invalid*.NotFound to 400 with the original AWS message; all other AWS errors stay 500. Applied at the quote and execute call sites. Tests: 27 new test cases covering the new endpoint (happy path, 404, auth), UUID format rejection, and AWS 4xx/5xx error classification. Closes #695 * feat(ri-exchange/ui): replace free-text offering ID with picker (#695) Replace the "Target Offering ID" free-text input in the exchange modal with a <select> picker backed by two sources: - AWS-driven: GET /api/ri-exchange/target-offerings loads valid target offerings for the source RI from DescribeReservedInstancesOfferings (the new endpoint from the companion backend commit). Options are grouped under "AWS Target Offerings". - CE Recommendations: alternativeTargets from reshape recommendations are listed as a second optgroup "CE Recommendations" with per-instance monthly cost displayed in the option label. The picker drives a hidden <input type="hidden" class="modal-exchange-target"> that holds the resolved UUID. collectTargets() validates the value against a UUID regex before submission, rejecting any non-UUID value with a user-visible error message. This closes the path where a user could accidentally submit an instance type (e.g. "t3.medium") instead of a UUID. Tests: 5 new test cases covering the picker select rendering, CE options population, AWS offerings after async load, select-to-hidden-input sync, and non-UUID rejection at submission time. Updated 8 existing tests to use offering_id UUIDs in place of instance types where they inject values directly into the hidden offering input. Closes #695
This was referenced May 28, 2026
cristim
added a commit
that referenced
this pull request
May 31, 2026
…loses #691) (#800) * fix(azure/gcp/aws): ctx.Err guards + page cap on 22 pagination loops (closes #691) Add ctx.Err() early-return guards and maxPages budget caps to all unbounded pagination loops across Azure (14 loops), GCP (6 loops), and AWS (2 loops). Context cancellation now terminates loops immediately with a wrapped error instead of either continuing or silently returning a partial result set, matching the pattern from PR #690 and the feedback_ctx_cancel_terminal rule. Fixes in collect* helpers (cache, cosmosdb) and the inline search loop that previously used ctx.Err()+break (silently partial) -- changed to return the error so callers propagate it. Also fixes two nil-context test calls in cache/client_test.go that panicked after the ctx.Err() guard was introduced. Regression tests added: Azure cache GetValidResourceTypes ctx-cancel + page cap, GCP computeengine GetRecommendations ctx-cancel + page cap, AWS fetchCoveragePaged ctx-cancel. * fix(gcp): rename page-cap constant + counter to reflect item semantics MachineTypesIterator.Next() yields one machine type per call, not one page. Rename maxMachineTypesPages -> maxMachineTypeItems and pageIdx -> itemIdx so the names and error message match the actual iteration semantics. * refactor: extract helpers to clear gocyclo threshold (#800) The ctx.Err pagination guards added in this PR pushed two functions just over the project's gocyclo > 10 ceiling, failing pre-commit: - providers/azure/services/search/client.go GetValidResourceTypes (12) - providers/aws/recommendations/utilization.go GetRIUtilization (11) Each is split into a thin entry point plus a focused helper: - GetValidResourceTypes -> resolveServicesPager + collectSKUsFromPager - GetRIUtilization -> buildUtilizations for the agg-to-slice tail Behaviour unchanged; 323 tests pass across the two packages.
cristim
added a commit
that referenced
this pull request
Jun 5, 2026
All 7 service clients (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans) pass the 6-item contract from PR #690: typed fields, ctx.Err() at loop top, page cap, per-page log, variant skip, nil+empty token terminator. Fix three pre-existing test build failures: missing execID argument in RDS/Redshift/SavingsPlans findOfferingID test call sites. Add TestFindOfferingID_CtxCancelledBeforePage and TestFindOfferingID_EmptyStringTokenEndsPagination to every service's client_test.go (14 new tests) to pin contract items C2 and C6. Add providers/aws/services/AUDIT.md with the 6-item contract, per-service status table, watchlist items (MemoryDB Valkey engine split, OpenSearch and Redshift Graviton dimension), and quarterly audit procedure.
This was referenced Jun 7, 2026
cristim
added a commit
that referenced
this pull request
Jun 8, 2026
All 7 service clients (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans) pass the 6-item contract from PR #690: typed fields, ctx.Err() at loop top, page cap, per-page log, variant skip, nil+empty token terminator. Fix three pre-existing test build failures: missing execID argument in RDS/Redshift/SavingsPlans findOfferingID test call sites. Add TestFindOfferingID_CtxCancelledBeforePage and TestFindOfferingID_EmptyStringTokenEndsPagination to every service's client_test.go (14 new tests) to pin contract items C2 and C6. Add providers/aws/services/AUDIT.md with the 6-item contract, per-service status table, watchlist items (MemoryDB Valkey engine split, OpenSearch and Redshift Graviton dimension), and quarterly audit procedure.
8 tasks
cristim
added a commit
that referenced
this pull request
Jun 19, 2026
All 7 service clients (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans) pass the 6-item contract from PR #690: typed fields, ctx.Err() at loop top, page cap, per-page log, variant skip, nil+empty token terminator. Fix three pre-existing test build failures: missing execID argument in RDS/Redshift/SavingsPlans findOfferingID test call sites. Add TestFindOfferingID_CtxCancelledBeforePage and TestFindOfferingID_EmptyStringTokenEndsPagination to every service's client_test.go (14 new tests) to pin contract items C2 and C6. Add providers/aws/services/AUDIT.md with the 6-item contract, per-service status table, watchlist items (MemoryDB Valkey engine split, OpenSearch and Redshift Graviton dimension), and quarterly audit procedure.
cristim
added a commit
that referenced
this pull request
Jul 10, 2026
All 7 service clients (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans) pass the 6-item contract from PR #690: typed fields, ctx.Err() at loop top, page cap, per-page log, variant skip, nil+empty token terminator. Fix three pre-existing test build failures: missing execID argument in RDS/Redshift/SavingsPlans findOfferingID test call sites. Add TestFindOfferingID_CtxCancelledBeforePage and TestFindOfferingID_EmptyStringTokenEndsPagination to every service's client_test.go (14 new tests) to pin contract items C2 and C6. Add providers/aws/services/AUDIT.md with the 6-item contract, per-service status table, watchlist items (MemoryDB Valkey engine split, OpenSearch and Redshift Graviton dimension), and quarterly audit procedure.
cristim
added a commit
that referenced
this pull request
Jul 17, 2026
All 7 service clients (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans) pass the 6-item contract from PR #690: typed fields, ctx.Err() at loop top, page cap, per-page log, variant skip, nil+empty token terminator. Fix three pre-existing test build failures: missing execID argument in RDS/Redshift/SavingsPlans findOfferingID test call sites. Add TestFindOfferingID_CtxCancelledBeforePage and TestFindOfferingID_EmptyStringTokenEndsPagination to every service's client_test.go (14 new tests) to pin contract items C2 and C6. Add providers/aws/services/AUDIT.md with the 6-item contract, per-service status table, watchlist items (MemoryDB Valkey engine split, OpenSearch and Redshift Graviton dimension), and quarterly audit procedure.
cristim
added a commit
that referenced
this pull request
Jul 17, 2026
All 7 service clients (EC2, RDS, ElastiCache, MemoryDB, OpenSearch, Redshift, SavingsPlans) pass the 6-item contract from PR #690: typed fields, ctx.Err() at loop top, page cap, per-page log, variant skip, nil+empty token terminator. Fix three pre-existing test build failures: missing execID argument in RDS/Redshift/SavingsPlans findOfferingID test call sites. Add TestFindOfferingID_CtxCancelledBeforePage and TestFindOfferingID_EmptyStringTokenEndsPagination to every service's client_test.go (14 new tests) to pin contract items C2 and C6. Add providers/aws/services/AUDIT.md with the 6-item contract, per-service status table, watchlist items (MemoryDB Valkey engine split, OpenSearch and Redshift Graviton dimension), and quarterly audit procedure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the Lambda timeout regression (execution
9336a111) where EC2findOfferingIDpaginated indefinitely through sparse emptyDescribeReservedInstancesOfferingspages and burned the entire 60s budget.OfferingTypedirect field onDescribeReservedInstancesOfferingsInputto let AWS filter by payment option server-side; addconvertEC2PaymentOptionhelper; verify returned offering type before returning (defense in depth); extractpaginateEC2Offerings+scanEC2OfferingPagehelpers to keep cyclomatic complexity within the gocyclo thresholdctx.Err()check + variant verification + per-page timing log; extractpaginateRDSOfferings+scanRDSOfferingPagehelpers (already had all filter fields server-side)paginateElastiCacheOfferings+scanElastiCacheOfferingPageOfferingType+Durationdirect fields to narrow at API level; replace client-sidematchesDuration/matchesOfferingTypeloop with server-side filters; add cap +ctx.Err+ variant verificationctx.Err+ defense-in-depth variant check after client-side matching; extractscanOpenSearchOfferingPagectx.Err+ unknown-type guard; extractscanRedshiftOfferingPagectx.ErrtolookupOfferingID(was a single non-paginated call)All seven
findOfferingID/lookupOfferingIDfunctions now cap atmaxOfferingPages = 5pages (500 offerings atMaxResults/MaxRecords = 100) and return a diagnostic error instead of timing out.Test plan
gocyclohook passes (allfindOfferingIDcomplexity reduced to ≤10 via helper extraction)go vet,gofmt,go mod tidypassCloses #688
Refs #684 #667 #632
Summary by CodeRabbit
Bug Fixes
Tests