ux(settings): account override UIs should reuse commitmentOptions validity rules (e.g., RDS 3yr no-upfront) · Issue #107 · LeanerCloud/cloud-commitments-cli · GitHub
Skip to content

ux(settings): account override UIs should reuse commitmentOptions validity rules (e.g., RDS 3yr no-upfront) #107

Closed

Description

@cristim

Summary

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":

  • isValidCombination(provider, service, term, payment): boolean — commitmentOptions.ts:154
  • getValidPaymentOptions(provider, service, term): PaymentOption[] — commitmentOptions.ts:169
  • getValidTermOptions(provider, service, payment): TermOption[] — commitmentOptions.ts:186
  • populateTermSelect(...) / populatePaymentSelect(...) — commitmentOptions.ts:203,227 already wire the per-service Settings cards.

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.
  • Azure/GCP override surfaces. They don't have a create modal yet (still in ux(settings): add UI to create per-account service overrides #104 follow-up), and inline editing is AWS-only today.

Activity

  1. cristim commented on Apr 28, 2026

    @cristim
    MemberAuthor

    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)

  2. added 2 commits that reference this issue on Apr 29, 2026
    61e7ed5
    e6c0830
  3. cristim commented on Apr 30, 2026

    @cristim
    MemberAuthor

    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+term change

    Opened "Add override" → set service=rds, term=3 → payment dropdown showed ["", "partial-upfront", "all-upfront"]. no-upfront correctly 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-select dropdown shows only ["", "partial-upfront", "all-upfront"]. no-upfront correctly 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 (current feat/multicloud-web-frontend head):

    var req AccountServiceOverrideRequest
    if err := json.Unmarshal([]byte(httpReq.Body), &req); err != nil {
        return nil, NewClientError(400, "invalid request body")
    }
    // … buildServiceOverride(...) …
    if err := h.config.SaveAccountServiceOverride(ctx, override); err != nil {
        return nil, fmt.Errorf("accounts: %w", err)
    }

    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:

    1. 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.
    2. 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).

  4. cristim commented on May 3, 2026

    @cristim
    MemberAuthor

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions