From 423b09b9fef3a19269bbd14680d2e13ded6f365f Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 24 Apr 2026 16:22:51 +0200 Subject: [PATCH] fix(settings): clamp RDS 3yr-no-upfront combos; correct ElastiCache etc. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two related fixes in the Settings → Purchasing form. 1. The per-service dropdowns let users pick AWS RDS 3-year + No Upfront, which the provider refuses (see `cmd/validators.go:warnRDS3YearNoUpfront`). The plan-creation modal already guarded against this through `commitmentOptions.isValidCombination`; the Settings form was the last place an invalid default could be persisted. 2. `commitmentOptions.ts` was marking ElastiCache / OpenSearch / Redshift / MemoryDB as also rejecting 3yr no-upfront, but that was over-cautious copy-paste. The backend validator only warns about RDS, and the newer `lib/purchase-compatibility.ts` calls RDS out as "the one hard rule". AWS does offer 3yr no-upfront on those other services, so the rules have been narrowed to RDS only. Wiring: - `settings.ts` gains `syncPaymentConstraintsForService(field)` and `syncAllServiceCommitmentConstraints()`. For each AWS service with an `invalidCombinations` entry (just RDS after this fix), payment ` - - - - - - + + + + + + @@ -198,7 +198,7 @@ describe('Settings Module', () => { expect((document.getElementById('gcp-compute-term') as HTMLSelectElement).value).toBe('1'); }); - test('sets up default payment to propagate to AWS services (after confirm)', async () => { + test('sets up default payment to propagate to AWS services (after confirm), clamping where per-service constraints reject the value', async () => { setupSettingsHandlers(); mockConfirmDialog.mockResolvedValueOnce(true); @@ -208,8 +208,15 @@ describe('Settings Module', () => { await new Promise((r) => setTimeout(r, 0)); expect(mockConfirmDialog).toHaveBeenCalledTimes(1); + // EC2 accepts no-upfront at both terms — propagation lands as-is. expect((document.getElementById('aws-ec2-payment') as HTMLSelectElement).value).toBe('no-upfront'); - expect((document.getElementById('aws-rds-payment') as HTMLSelectElement).value).toBe('no-upfront'); + // RDS 3yr rejects no-upfront (parent beforeEach seeds all service + // terms at "3"), so the constraint sync clamps RDS back to the + // first valid payment option instead of persisting an invalid + // combination the provider will refuse. + const rdsPayment = (document.getElementById('aws-rds-payment') as HTMLSelectElement).value; + expect(rdsPayment).not.toBe('no-upfront'); + expect(['partial-upfront', 'all-upfront']).toContain(rdsPayment); }); test('cancelling the cascade restores the default term to its prior value', async () => { @@ -719,4 +726,142 @@ describe('Settings Module', () => { expect(html).not.toMatch(/Azure and GCP reservations are always paid upfront/); }); }); + + // Guard against the RDS 3yr + no-upfront regression from follow-up to + // issue #12. The backend rejects that combination (and EC/OpenSearch/ + // Redshift 3yr no-upfront), so the Settings form must not allow it. + // Rules live in commitmentOptions.ts; these tests exercise the wiring + // that applies them to the per-service dropdowns. + describe('per-service term/payment combination constraints', () => { + const optVisible = (sel: HTMLSelectElement, value: string): boolean => { + const opt = Array.from(sel.options).find(o => o.value === value); + if (!opt) return false; + return !opt.hidden && !opt.disabled; + }; + + test('RDS 3yr hides "no-upfront" and keeps partial/all upfront selectable', async () => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'all-upfront', default_coverage: 80 }, + services: [{ provider: 'aws', service: 'rds', term: 3, payment: 'all-upfront' }], + }); + setupSettingsHandlers(); + await loadGlobalSettings(); + + const rdsPayment = document.getElementById('aws-rds-payment') as HTMLSelectElement; + expect(optVisible(rdsPayment, 'no-upfront')).toBe(false); + expect(optVisible(rdsPayment, 'partial-upfront')).toBe(true); + expect(optVisible(rdsPayment, 'all-upfront')).toBe(true); + }); + + test('RDS 1yr keeps all three payment options visible', async () => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 1, default_payment: 'no-upfront', default_coverage: 80 }, + services: [{ provider: 'aws', service: 'rds', term: 1, payment: 'no-upfront' }], + }); + setupSettingsHandlers(); + await loadGlobalSettings(); + + const rdsPayment = document.getElementById('aws-rds-payment') as HTMLSelectElement; + expect(optVisible(rdsPayment, 'no-upfront')).toBe(true); + expect(optVisible(rdsPayment, 'partial-upfront')).toBe(true); + expect(optVisible(rdsPayment, 'all-upfront')).toBe(true); + }); + + test('switching RDS term 1yr → 3yr while "no-upfront" is selected auto-clamps payment', async () => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 1, default_payment: 'no-upfront', default_coverage: 80 }, + services: [{ provider: 'aws', service: 'rds', term: 1, payment: 'no-upfront' }], + }); + setupSettingsHandlers(); + await loadGlobalSettings(); + + const rdsTerm = document.getElementById('aws-rds-term') as HTMLSelectElement; + const rdsPayment = document.getElementById('aws-rds-payment') as HTMLSelectElement; + expect(rdsPayment.value).toBe('no-upfront'); + + rdsTerm.value = '3'; + rdsTerm.dispatchEvent(new Event('change')); + + // no-upfront is now invalid; payment should snap to first valid option + expect(rdsPayment.value).not.toBe('no-upfront'); + expect(['partial-upfront', 'all-upfront']).toContain(rdsPayment.value); + expect(optVisible(rdsPayment, 'no-upfront')).toBe(false); + }); + + test('legacy-persisted invalid combo (RDS 3yr + no-upfront) is clamped on load', async () => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'all-upfront', default_coverage: 80 }, + // Simulate a config stored before this guardrail existed. + services: [{ provider: 'aws', service: 'rds', term: 3, payment: 'no-upfront' }], + }); + setupSettingsHandlers(); + await loadGlobalSettings(); + + const rdsPayment = document.getElementById('aws-rds-payment') as HTMLSelectElement; + expect(rdsPayment.value).not.toBe('no-upfront'); + }); + + test('EC2 3yr keeps all three payment options visible (no service-level restriction)', async () => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'no-upfront', default_coverage: 80 }, + services: [{ provider: 'aws', service: 'ec2', term: 3, payment: 'no-upfront' }], + }); + setupSettingsHandlers(); + await loadGlobalSettings(); + + const ec2Payment = document.getElementById('aws-ec2-payment') as HTMLSelectElement; + expect(optVisible(ec2Payment, 'no-upfront')).toBe(true); + expect(optVisible(ec2Payment, 'partial-upfront')).toBe(true); + expect(optVisible(ec2Payment, 'all-upfront')).toBe(true); + expect(ec2Payment.value).toBe('no-upfront'); + }); + + test.each(['elasticache', 'opensearch', 'redshift'])( + '%s 3yr keeps "no-upfront" visible (AWS only restricts RDS)', + async (service) => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'no-upfront', default_coverage: 80 }, + services: [{ provider: 'aws', service, term: 3, payment: 'no-upfront' }], + }); + setupSettingsHandlers(); + await loadGlobalSettings(); + + const payment = document.getElementById(`aws-${service}-payment`) as HTMLSelectElement; + expect(optVisible(payment, 'no-upfront')).toBe(true); + // And the selected value round-trips cleanly — the backend persists + // this service with no-upfront, and the UI should not clamp it. + expect(payment.value).toBe('no-upfront'); + }, + ); + + test('propagating global "no-upfront" to all services while term=3 clamps restricted services', async () => { + (api.getConfig as jest.Mock).mockResolvedValue({ + global: { enabled_providers: ['aws'], default_term: 3, default_payment: 'all-upfront', default_coverage: 80 }, + services: [ + { provider: 'aws', service: 'ec2', term: 3, payment: 'all-upfront' }, + { provider: 'aws', service: 'rds', term: 3, payment: 'all-upfront' }, + ], + }); + setupSettingsHandlers(); + await loadGlobalSettings(); + + // User changes the global default to no-upfront and confirms the propagation. + mockConfirmDialog.mockResolvedValue(true); + const defaultPayment = document.getElementById('setting-default-payment') as HTMLSelectElement; + defaultPayment.dataset['previous'] = 'all-upfront'; + defaultPayment.value = 'no-upfront'; + defaultPayment.dispatchEvent(new Event('change')); + + // Allow the async confirmDialog promise to resolve. + await Promise.resolve(); + await Promise.resolve(); + + const ec2Payment = document.getElementById('aws-ec2-payment') as HTMLSelectElement; + const rdsPayment = document.getElementById('aws-rds-payment') as HTMLSelectElement; + // EC2 accepts the propagated no-upfront (no restriction). + expect(ec2Payment.value).toBe('no-upfront'); + // RDS 3yr rejects no-upfront, so it clamps back to the first valid option. + expect(rdsPayment.value).not.toBe('no-upfront'); + }); + }); }); diff --git a/frontend/src/commitmentOptions.ts b/frontend/src/commitmentOptions.ts index d78c500d..70a91715 100644 --- a/frontend/src/commitmentOptions.ts +++ b/frontend/src/commitmentOptions.ts @@ -59,7 +59,13 @@ const commitmentConfigs: Record> = { terms: STANDARD_TERMS, payments: AWS_PAYMENTS }, - // RDS - no 3-year no-upfront option + // RDS - no 3-year no-upfront option. This is the only AWS service + // with a hard restriction; the backend validator in + // cmd/validators.go:warnRDS3YearNoUpfront agrees, and the newer + // lib/purchase-compatibility.ts calls it out as "the one hard rule". + // ElastiCache / OpenSearch / Redshift / MemoryDB were previously + // listed here too, but that was over-cautious copy-paste — AWS does + // offer 3yr no-upfront on those services. rds: { terms: STANDARD_TERMS, payments: AWS_PAYMENTS, @@ -67,38 +73,6 @@ const commitmentConfigs: Record> = { { term: 3, payment: 'no-upfront' } ] }, - // ElastiCache - no 3-year no-upfront option - elasticache: { - terms: STANDARD_TERMS, - payments: AWS_PAYMENTS, - invalidCombinations: [ - { term: 3, payment: 'no-upfront' } - ] - }, - // OpenSearch - no 3-year no-upfront option - opensearch: { - terms: STANDARD_TERMS, - payments: AWS_PAYMENTS, - invalidCombinations: [ - { term: 3, payment: 'no-upfront' } - ] - }, - // Redshift - no 3-year no-upfront option - redshift: { - terms: STANDARD_TERMS, - payments: AWS_PAYMENTS, - invalidCombinations: [ - { term: 3, payment: 'no-upfront' } - ] - }, - // MemoryDB - no 3-year no-upfront option - memorydb: { - terms: STANDARD_TERMS, - payments: AWS_PAYMENTS, - invalidCombinations: [ - { term: 3, payment: 'no-upfront' } - ] - }, // Default for AWS services not specifically configured _default: { terms: STANDARD_TERMS, diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index ec44e932..fe9b3ca1 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -7,6 +7,7 @@ import { initFederationPanel } from './federation'; import { confirmDialog } from './confirmDialog'; import { reflectDirtyState } from './settings-subnav'; import { showToast } from './toast'; +import { isValidCombination } from './commitmentOptions'; type AccountProvider = 'aws' | 'azure' | 'gcp'; @@ -911,6 +912,16 @@ export function setupSettingsHandlers(signal?: AbortSignal): void { }, { signal }); } + // Wire per-service term changes to the commitment-constraint sync so the + // payment dropdown narrows to supported combinations as the user picks a + // term. The initial sync runs after loadGlobalSettings populates the + // form with persisted values. + SERVICE_FIELDS.forEach(field => { + if (field.provider !== 'aws' || !field.paymentId) return; + const termEl = document.getElementById(field.termId) as HTMLSelectElement | null; + termEl?.addEventListener('change', () => syncPaymentConstraintsForService(field), { signal }); + }); + // Set up dirty-field tracking setupDirtyTracking(signal); @@ -995,6 +1006,10 @@ function propagateTermToServices(term: string): void { select.value = term; } }); + // Propagation changes term, which may invalidate each service's current + // payment (e.g., RDS jumping from 1yr to 3yr while "No Upfront" was set). + // Re-apply per-service constraints to keep the UI honest. + syncAllServiceCommitmentConstraints(); } /** @@ -1010,6 +1025,57 @@ function propagatePaymentToServices(payment: string): void { select.value = payment; } }); + // If the propagated payment is invalid for any service's current term + // (e.g., "No Upfront" against RDS 3yr), clamp to the first valid option + // on that service instead of silently persisting a combo the provider + // will reject. + syncAllServiceCommitmentConstraints(); +} + +/** + * Apply per-service commitment restrictions to a single service's payment + * dropdown. AWS services like RDS/ElastiCache/OpenSearch/Redshift don't + * accept 3-year No-Upfront — commitmentOptions.ts encodes which (term, + * payment) pairs each service rejects. We hide + disable the invalid + * option rows so the Settings UI can't save a combination the backend + * would reject. If the currently-selected payment becomes invalid after + * a term change, it auto-clamps to the first valid option so the form + * never holds an unsavable value. + * + * Azure and GCP service-default cards are out of scope here — Azure has + * only two payment options (neither restricted by term) and GCP has no + * payment selector at all. + */ +function syncPaymentConstraintsForService(field: typeof SERVICE_FIELDS[number]): void { + if (field.provider !== 'aws' || !field.paymentId) return; + const termEl = document.getElementById(field.termId) as HTMLSelectElement | null; + const paymentEl = document.getElementById(field.paymentId) as HTMLSelectElement | null; + if (!termEl || !paymentEl) return; + + const term = parseInt(termEl.value || '3', 10); + let firstValidOption: HTMLOptionElement | null = null; + for (const opt of Array.from(paymentEl.options)) { + const valid = isValidCombination(field.provider, field.service, term, opt.value); + // Belt-and-suspenders: `hidden` removes the option from the rendered + // dropdown, `disabled` prevents keyboard / programmatic selection in + // the rare browsers where a hidden