[cristim]

What and why

Bumps cloud-commitments-go pkg, providers/aws, providers/azure and providers/gcp together from 551be568dc21 to 3def3f8986174f7b24cefe6fc0fd068cfd45cba8 (go main, includes go#435, #436 and #437). #799 needs insurance.LineItems and the selected amounts from pkg/insurance. 53 go commits come along.

Refs #799

Platform changes beyond go.mod

• internal/api/handler_ri_exchange.go: stop passing the source RI's AWS offering-type enum (No Upfront) as ListTargetOfferings.OfferingType. go now returns an error for a value that is not a no-upfront-style slug, where it used to ignore it. Without this change every GET /api/ri-exchange/target-offerings call for a real RI fails. Effective behaviour is unchanged: all payment options were being listed before.
• Test fakes (internal/api/target_offerings_zone_test.go, internal/server/ri_exchange_zone_test.go) now return offerings with class, tenancy and duration, because go verifies what AWS returns.

Changelog flags (behaviour, no IaC or IAM changes found)

• go#455/fix(auth): enforce permission scope on config writes #456/fix: prevent duplicate history actions and stale retries #469, fix(auth): enforce marketplace permission constraints #460/sec(iac): key CI/CD deploy trust on immutable repository IDs #451: EC2 offering lookups for exchange and purchase reject offerings that do not match the request (class, tenancy, duration, type, zone) and reject empty term, platform or instance type. Covered above.
• go#495 (breaking): Azure Savings Plans refuse a malformed idempotency token before the PUT. go#487 adds common.OrderIDFromToken, which fails loud. go#496: GCP Compute Engine fails loud on a token it cannot turn into a request ID.
• go#429: AWS purchases with an empty token are sent once, and a lost response is reported as unknown.
• go#497/fix(api): scope plans list, inventory filter and RI quote to allowed accounts #499: exchanges with an unknown accept outcome are flagged and reported as unknown, not failed.
• go#498: Azure fails closed on a live plan without an effective date. go#476: unknown Savings Plans terms are rejected instead of defaulting to one year.
• go#446: GroupCommitments removed from GCP (not used by the platform, build is clean).
• go#458/fix(frontend): preserve ladder selections when opening editor #465/fix(auth): enforce marketplace permission constraints #460: Commitment gains EC2 platform, tenancy and scope, used by dedupe.
• Remaining commits are pagination caps, test, CI and docs changes.

Verification (macOS, GOWORK=off)

• go build ./..., go vet ./..., go vet -tags integration ./..., go mod tidy -diff: clean.
• go test ./internal/... ./cmd/...: all pass. Before the fixes, TestListTargetOfferings_* (internal/api) and TestExecuteRIExchangeReshape_* (internal/server) failed on the bumped pin, which is the fail-before evidence.
• Not verified: real AWS, Azure or GCP accounts.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 4 billable files and costs up to $1.00.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 73 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: a10fb717-e972-4c08-8376-6c0d726fc3cf

📥 Commits

Reviewing files that changed from the base of the PR and between c234587 and b2a4828.

⛔ Files ignored due to path filters: 1

• go.sum is excluded by !**/*.sum

📒 Files selected for processing: 4

• go.mod
• internal/api/handler_ri_exchange.go
• internal/api/target_offerings_zone_test.go
• internal/server/ri_exchange_zone_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 review verdict: CLEAN at b2a4828

Reviewer: fresh Claude agent (Sonnet 5.5, high effort), not the author, cold brief. The PR body's changelog was treated as a claim and checked against go at 3def3f8. Coordinator: claude-cc-reviewer.

Blocking findings: none.

What was verified

• Idempotency tokens are safe on all three providers.
  • The platform builds tokens in one place only, internal/purchase/execution.go:680. It calls common.DeriveIdempotencyToken, which returns 64 lowercase hex characters (go pkg/common/tokens.go:64-67).
  • Azure: go#495's OrderIDFromToken requires at least 32 hex characters (tokens.go:122-131), so these tokens pass.
  • GCP: the token passes go#496's request-ID and name checks.
  • AWS: the ClientToken limit is 64 characters, so these tokens fit.
  • No call site sets an empty token.
• Unknown outcomes cannot lead to a duplicate buy.
  • A lost purchase response surfaces as "purchase outcome unknown".
  • RedriveRefusalReason (internal/purchase/manager.go:459-557) refuses both the reaper re-drive and the user retry, and keeps the suppressions. The sentinel text in go pkg/common/types.go:29 is unchanged.
  • Manual exchange: claimExchangeSubmit (internal/api/ri_exchange_idempotency.go:167-184) is never released.
  • Auto exchange: the record stays processing and the run halts.
• Stricter validation breaks no caller. ListTargetOfferings now rejects an empty product description and a non-positive duration. Both platform callers take those values from real DescribeReservedInstances data (internal/api/handler_ri_exchange.go:171, internal/server/handler_ri_exchange.go:82). The new Savings Plans term check in go#476 is unreachable behind convertTermToSeconds.
• Dedupe is unaffected. The platform does not compare common.Commitment; inventory uses its own InventoryCommitment type.
• The handler change keeps existing behaviour. The old code already dropped the "No Upfront" conversion error, so the offering type was ignored and every payment option was listed. Restoring the pass-through makes the TestListTargetOfferings_* zone tests fail.
• GroupCommitments has no references, including under the integration build tag.

Local verification (GOWORK=off; fakes and Docker integration, no real accounts):

• go build ./..., go vet ./..., go vet -tags integration ./... and go mod tidy -diff are clean.
• go test -race ./internal/... ./cmd/... -count=1 passes.
• go test -tags integration ./internal/purchase/... ./internal/server/... passes 1326 tests.
• Fail-before: TestListTargetOfferings_* and TestExecuteRIExchangeReshape_* fail on the bumped pin when the handler and fake fixes are removed.
• Gap: Azure and GCP token acceptance, AWS lost-response handling and a live exchange accept were checked by reading code and through fakes only, not against real accounts.

Non-blocking notes

• The exchange notification email lists unknown-outcome exchanges as failed. I filed fix(exchange): show unknown-outcome auto-exchanges as needing reconciliation in the notification email #1055 to show them as needing reconciliation.
• The purchase path still matches "outcome unknown" by its text rather than the typed sentinel. That still works, because the sentinel text is unchanged.

Gate

• CI is green at this SHA, with 25 passing checks.
• No drift from main.
• CodeRabbit hit its rate limit and posted no review or inline comments. It is optional under the review gate, because this independent review and exact-revision local verification cover the SHA.
• The labels match feat(insurance): per-recommendation Archera quotes (blocked on an Archera API mapping) #799.
• There are no Terraform changes, so merging is an ordinary code deploy to dev.

[cristim]

Independent review at b2a4828

Verdict: no blocking findings. Safe to merge at this SHA once the gate's other conditions hold (CI is green on the SHA: 25/25 checks pass).

Findings

• Low / nit (no action needed): no test asserts directly that the exchange target list is not filtered by payment option. The two TestListTargetOfferings_* zone tests catch the regression only indirectly, through go's unsupported EC2 payment option error. That is enough to stop a revert, and the behaviour itself is unchanged (see below).

What I checked

1. RI exchange change. Old go (551be568dc21, providers/aws/services/ec2/client.go ~L965): normalizeTargetOfferingsParams called convertEC2PaymentOption(p.OfferingType) and on error fell through with an unset offering type, so the AWS query had no OfferingType filter. New go (3def3f89, client.go L983-994): the same conversion returns unsupported EC2 payment option: No Upfront, which ListTargetOfferings propagates. The source RI's enum is the AWS-style No Upfront, so every call would have failed. Dropping the field (internal/api/handler_ri_exchange.go:171-177) gives the same query as before: no payment filter, all payment options listed, each offering still carries its own OfferingType. ListTargetOfferings is called only from targetOfferingsForSource (handler_ri_exchange.go:171). The execution path (ExecuteExchange, L1280) does not use it, so there is no money-path change.
2. Stricter go matching (offering_match.go L47-75): offerings AWS returns that mismatch zone, class, tenancy, duration, offering type, instance type, scope or product are dropped and logged. The fakes needed class, tenancy and duration to survive this; the fake changes are correct and minimal. Real AWS sets these fields.
3. Idempotency tokens. The only platform source of a token is common.DeriveIdempotencyToken(idempotencyLineageKey(exec), i) (internal/purchase/execution.go:680), a 64-char hex SHA-256. It satisfies go's OrderIDFromToken / IdempotencyGUID (>=32 hex chars, pkg/common/tokens.go:122), so Azure Savings Plans (fix(api): map run-now failures by cause and stamp its audit fields #495) and GCP (fix(auth): compare-and-swap writes for reset confirm and recovery-code login #496) never hit the new fail-loud path. No other caller sets IdempotencyToken.
4. Empty-token purchases (fix(ci): run the frontend jest suite on every frontend PR #429), ErrOutcomeUnknown. The platform never references the sentinel. It does not need to: re-drive and retry refusal is keyed on provider and service (internal/purchase/manager.go:496-535, RedriveRefusalReason), not on error text, and saveTerminalExecution (L317) keeps suppressions on any failure not proven pre-submission. So a lost response or an unknown outcome is not retried and is not released. The platform always supplies a token, so the single-send empty-token path is unused here. Fail-closed holds.
5. Exchange unknown outcome (fix(frontend): never offer Approve without the purchase amount #497/fix(api): scope plans list, inventory filter and RI quote to allowed accounts #499). The OutcomeUnknown flag lives in the go ladder and auto-exchange. The platform's ExecuteExchange seam (handler_ri_exchange.go:1280) keeps its prior error handling. Pre-existing gap, not introduced here: no typed unknown-outcome persistence (the code comment cites platform fix(purchase): refuse retry and re-drive of AWS EC2 and Redshift purchases #778).
6. Savings Plans term (fix(gcp): stop admin-password secret IAM bindings churning on every apply #476). calculateHoursInTerm and normalizeTermString now reject terms other than 1yr, 1, 3yr, 3 instead of defaulting to one year. The platform passes fmt.Sprintf("%dyr", rec.Term) (execution.go:1136), so a bad integer term now fails loudly rather than silently buying one year. This is an improvement.
7. Removed API. GroupCommitments (GCP, build(deps): re-pin cloud-commitments-go past #151 and migrate PurchaseResult.Cost (*float64) #446) has no platform callers: git grep is clean and the build passes. Commitment gained EC2 platform, tenancy and scope fields (additive).
8. IaC and IAM. The PR touches no terraform, cloudformation, arm or iac paths. The 53 go commits add no new AWS API calls the platform's roles lack (reviewed commit list; changes are pagination caps, verification, tests, CI, docs).

Local verification (macOS, GOWORK=off, scratch worktree detached at the head SHA)

• go build ./... and go vet ./...: clean. go vet -tags integration ./...: clean.
• go test ./internal/api/ ./internal/purchase/ ./internal/execution/ ./internal/scheduler/ ./internal/server/: 5274 passed.
• Mutation: re-added OfferingType: sourceRI.OfferingType to targetOfferingsForSource. TestListTargetOfferings_ZonalSourceForwardsAvailabilityZone and TestListTargetOfferings_RegionalSourcePassesNoAvailabilityZone fail with failed to list target offerings: unsupported EC2 payment option: No Upfront. The pin holds. Mutation reverted.
• Not verified: real AWS, Azure or GCP accounts; no purchase or exchange call was made.