You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The two override UIs landed in #72 (inline payment selector on existing rows) and #106 (create-override modal for new rows) accept any (service, term, payment) combination — including ones the global Settings → Purchasing cards already block as invalid. Concrete example: RDS 3yr no-upfront is hidden/disabled in the global RDS card via commitmentOptions.ts's invalidCombinations, but selecting RDS + 3yr + no-upfront in the override modal saves cleanly, and the inline payment selector on an existing RDS override with term: 3 lets the user pick no-upfront despite it being unsupported by AWS.
The validation logic the override UIs should reuse already exists.
Existing logic to reuse
frontend/src/commitmentOptions.ts is the single source of truth for "which (provider, service, term, payment) tuples are valid":
The same commitmentConfigs.aws[<service>] keys already exist for ec2, rds, elasticache, opensearch, redshift, savingsplans, sagemaker. PR #54's fetchAndPopulateCommitmentOptions overlays this with server-probed data when available, so reusing these helpers also picks up live AWS offerings automatically.
settings.ts already calls syncAllServiceCommitmentConstraints() to clamp the global-card selects to valid combos — the equivalent should run for the override surfaces.
Fix direction
Override-create modal (buildOverrideModal flow in settings.ts, added by #106)
When the service select changes, repopulate the payment options via getValidPaymentOptions(provider, service, term) and the term options via getValidTermOptions(provider, service, payment) — same dependency loop the global cards use. If the currently-chosen term/payment becomes invalid for the new service, snap it to the first valid option (or to "Inherit"). Block submit if isValidCombination returns false on the final tuple.
Inline payment selector (buildPaymentOverrideSelect in settings.ts, added by #72)
Today it lists all 3 AWS payment options unconditionally. It should call getValidPaymentOptions('aws', override.service, override.term) and only render the options that pass — for an RDS override with term: 3, that's all-upfront and partial-upfront only, no no-upfront. If the row's current payment happens to be invalid (e.g., the row was created via direct API curl before this fix, or pre-existed at row-creation but the term changed), still render the current value so the row isn't silently mutated, but mark it as a problem and surface a "Reset" hint.
Backend save-side guard (defense in depth)
internal/api/handler_accounts.go:saveAccountServiceOverride currently does no per-service combo validation; it accepts whatever the client sends. The commitmentopts.Service.Validate machinery (added by #54 for the global-config PUT /api/config/service/{service} path via checkCommitmentOptionCombo) should be invoked here too. If the live probe data is unavailable (ErrNoData), the validator falls back to permissive — fine, the frontend's hardcoded rules remain the gate.
Acceptance criteria
Selecting rds in the override modal with term=3 hides no-upfront from the payment dropdown (or surfaces a clear "not available" state).
The inline payment selector on an existing RDS override with term: 3 does not list no-upfront.
A direct PUT /api/accounts/:id/service-overrides/aws/rds with {term: 3, payment: "no-upfront"} returns 400 from the backend, citing the invalid combination, when probe data is present.
Save-success path remains unchanged for valid combinations on every (service, term, payment) the global cards allow today.
Tests in settings-accounts.test.ts cover at least: modal hides invalid options, inline selector hides invalid options, modal submit is blocked when the combination would be invalid.
Out of scope
New per-service rules. The fix is wiring the existing rules through; if AWS adds a new restriction tomorrow, that's an update to commitmentOptions.ts (or to the probe data) — independent of the wiring this issue tracks.
P1 — the override UIs accept invalid (term, payment) combos that the global Settings cards block (e.g., RDS 3yr no-upfront). The validity logic already exists in commitmentOptions.ts; this is purely a wiring gap. Saving invalid combos can lead to bad recommendations or backend errors when the data is actually consumed. Fix is effort/s given the existing helpers. (triage agent wave1-B)
Created the override above with payment=all-upfront, then re-opened the Service Overrides dialog → the row's inline .override-payment-select dropdown shows only ["", "partial-upfront", "all-upfront"]. no-upfront correctly excluded.
This part is NOT implemented by PR #176. Inspected internal/api/handler_accounts.go::saveAccountServiceOverride (current feat/multicloud-web-frontend head):
No call to commitmentopts.Service.Validate / checkCommitmentOptionCombo between the unmarshal and the save. A direct curl with {term: 3, payment: "no-upfront"} against an AWS RDS account would still 200 today.
Recommendation
Two options:
Close this issue and file a smaller follow-up — the two user-facing criteria (the ones a normal operator hits) are working. The backend guard is defence-in-depth and a separate, smaller change.
Keep this open until criterion 3 lands.
I'd lean (1) since #176's frontend coverage already prevents the issue's concrete RDS+3yr no-upfront example from happening through the UI (the only practical attack surface for a normal user). The curl-direct case is admin-only and covered by the existing requirePermission("update", "accounts") gate.
Happy to ship the backend guard as a follow-up PR if you'd prefer (2).
Closing as verified-done — PR #176 merged 2026-04-30 with the fix and your verification comment confirmed both acceptance criteria against the deployed instance. The auto-close trailer didn't fire (likely squash-merge subject mismatch); doing it manually here.
Activity
cristim commentedon Apr 28, 2026
P1 — the override UIs accept invalid (term, payment) combos that the global Settings cards block (e.g., RDS 3yr no-upfront). The validity logic already exists in
commitmentOptions.ts; this is purely a wiring gap. Saving invalid combos can lead to bad recommendations or backend errors when the data is actually consumed. Fix is effort/s given the existing helpers. (triage agent wave1-B)fix(ux/settings): override surfaces reuse commitmentOptions validity …
fix(ux/settings): override surfaces reuse commitmentOptions validity …
cristim commentedon Apr 30, 2026
Verification — closing the loop
PR #176 landed today (commit
a21c1405, deployed at 14:49Z). Browser-verified the two frontend acceptance criteria against the deployed instance:✅ Criterion 1 — override-create modal hides invalid options on
service+termchangeOpened "Add override" → set service=
rds, term=3→ payment dropdown showed["", "partial-upfront", "all-upfront"].no-upfrontcorrectly excluded.✅ Criterion 2 — inline payment selector on existing RDS+3yr override row
Created the override above with
payment=all-upfront, then re-opened the Service Overrides dialog → the row's inline.override-payment-selectdropdown shows only["", "partial-upfront", "all-upfront"].no-upfrontcorrectly excluded.❌ Criterion 3 — backend save-side guard returns 400 on invalid combo
This part is NOT implemented by PR #176. Inspected
internal/api/handler_accounts.go::saveAccountServiceOverride(currentfeat/multicloud-web-frontendhead):No call to
commitmentopts.Service.Validate/checkCommitmentOptionCombobetween the unmarshal and the save. A direct curl with{term: 3, payment: "no-upfront"}against an AWS RDS account would still 200 today.Recommendation
Two options:
I'd lean (1) since #176's frontend coverage already prevents the issue's concrete RDS+3yr no-upfront example from happening through the UI (the only practical attack surface for a normal user). The curl-direct case is admin-only and covered by the existing
requirePermission("update", "accounts")gate.Happy to ship the backend guard as a follow-up PR if you'd prefer (2).
fix(api): backend save-side guard for account service override saves (#…
cristim commentedon May 3, 2026
Closing as verified-done — PR #176 merged 2026-04-30 with the fix and your verification comment confirmed both acceptance criteria against the deployed instance. The auto-close trailer didn't fire (likely squash-merge subject mismatch); doing it manually here.