From afa3691e29a590ce7b54e0e754c38df280051a55 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 1 Jun 2026 16:20:20 +0200 Subject: [PATCH] feat(inventory/ri-exchange): honor Main Header global filter (closes #871) RI Exchange was the last Inventory sub-tab that ignored the Main Header Provider/Account chips; Active Commitments and Coverage already honor them (PR #866/#881). QA 2.9. Frontend (riexchange.ts): - loadRIExchange is now provider-aware. AWS shows convertible RIs + reshape recs + history; Azure shows exchangeable VM reservations + history with a provider-aware "reshape not available" note; GCP shows a "RI Exchange isn't available for GCP" empty state across all sections and calls no list endpoint. - The convertible-RI list is scoped to the single-selected account chip and renders scoped, provider-aware empty states (named account/subscription). - An in-progress exchange modal is closed synchronously on any filter change so the user can't act on a now-hidden RI (mutating workflow). - New Azure table built via DOM/textContent (no innerHTML). Backend (handler_ri_exchange.go): - listConvertibleRIs accepts ?account_id= and returns an empty list when the chip selects an account other than the running ambient AWS account; a real STS failure fails closed. Azure already scopes by subscription_id; GCP has no endpoint (frontend short-circuits to empty). Tests: frontend chip-scoping, GCP not-available state, Azure path, and selection-clear on filter change; backend account-scope mismatch returns empty and resolve-error fails closed. --- .../__tests__/riexchange-permissions.test.ts | 4 + frontend/src/__tests__/riexchange.test.ts | 140 ++++++++++ frontend/src/api/index.ts | 2 + frontend/src/api/riexchange.ts | 25 +- frontend/src/api/types.ts | 16 ++ frontend/src/riexchange.ts | 256 +++++++++++++++++- internal/api/handler.go | 7 + internal/api/handler_ri_exchange.go | 26 +- internal/api/handler_ri_exchange_test.go | 43 +++ 9 files changed, 502 insertions(+), 17 deletions(-) diff --git a/frontend/src/__tests__/riexchange-permissions.test.ts b/frontend/src/__tests__/riexchange-permissions.test.ts index 1d0ca8998..4a82b8749 100644 --- a/frontend/src/__tests__/riexchange-permissions.test.ts +++ b/frontend/src/__tests__/riexchange-permissions.test.ts @@ -20,6 +20,10 @@ jest.mock('../navigation', () => ({ jest.mock('../state', () => ({ getCurrentUser: jest.fn(), + // loadRIExchange reads the provider/account chips to scope the request + // (issue #871); default to the AWS, all-accounts path used by these tests. + getCurrentProvider: jest.fn(() => 'aws'), + getCurrentAccountIDs: jest.fn(() => []), })); import * as api from '../api'; diff --git a/frontend/src/__tests__/riexchange.test.ts b/frontend/src/__tests__/riexchange.test.ts index 8929fa280..a64b4c00b 100644 --- a/frontend/src/__tests__/riexchange.test.ts +++ b/frontend/src/__tests__/riexchange.test.ts @@ -5,6 +5,7 @@ // Mock the api module defensively (riexchange.ts imports it) jest.mock('../api', () => ({ listConvertibleRIs: jest.fn(), + listExchangeableAzureRIs: jest.fn(), getRIUtilization: jest.fn(), getReshapeRecommendations: jest.fn(), getExchangeQuote: jest.fn(), @@ -543,6 +544,15 @@ describe('reshape recommendations empty state', () => { (api.getRIUtilization as jest.Mock).mockResolvedValue([]); (api.getRIExchangeHistory as jest.Mock).mockResolvedValue([]); (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({ recommendations: [], recs_staleness: '', recs_collected_at: null }); + // resetAllMocks() in afterEach wipes the state mock implementations; + // loadRIExchange reads the chips to scope the request (issue #871), so + // restore the AWS/all-accounts default here. + const stateMod = jest.requireMock('../state') as { + getCurrentProvider: jest.Mock; + getCurrentAccountIDs: jest.Mock; + }; + stateMod.getCurrentProvider.mockReturnValue('aws'); + stateMod.getCurrentAccountIDs.mockReturnValue([]); }); afterEach(() => { @@ -677,6 +687,8 @@ describe('RI Exchange filter subscriptions (issue #186)', () => { const stateMod = jest.requireMock('../state') as { subscribeProvider: jest.Mock; subscribeAccount: jest.Mock; + getCurrentProvider: jest.Mock; + getCurrentAccountIDs: jest.Mock; }; stateMod.subscribeProvider.mockImplementation((cb: () => void) => { _providerListeners.push(cb); @@ -686,6 +698,10 @@ describe('RI Exchange filter subscriptions (issue #186)', () => { _accountListeners.push(cb); return () => undefined; }); + // loadRIExchange now reads the provider/account chips to scope the + // request (issue #871); restore their implementations too. + stateMod.getCurrentProvider.mockImplementation(() => 'aws'); + stateMod.getCurrentAccountIDs.mockImplementation(() => []); }); afterEach(() => { @@ -738,3 +754,127 @@ describe('RI Exchange filter subscriptions (issue #186)', () => { expect(api.listConvertibleRIs).toHaveBeenCalledTimes(1); }); }); + +// issue #871: RI Exchange must honour the Main Header global Provider/Account +// filter, matching the Active Commitments + Coverage sub-tabs (#866/#881). +describe('RI Exchange global filter scoping (issue #871)', () => { + let instancesEl: HTMLDivElement; + let recsEl: HTMLDivElement; + let historyEl: HTMLDivElement; + let riExchangePanel: HTMLDivElement; + + const stateMod = (): { + getCurrentProvider: jest.Mock; + getCurrentAccountIDs: jest.Mock; + subscribeProvider: jest.Mock; + subscribeAccount: jest.Mock; + } => jest.requireMock('../state'); + + beforeEach(() => { + instancesEl = document.createElement('div'); + instancesEl.id = 'ri-exchange-instances-list'; + recsEl = document.createElement('div'); + recsEl.id = 'ri-exchange-recommendations-list'; + historyEl = document.createElement('div'); + historyEl.id = 'ri-exchange-history-list'; + riExchangePanel = document.createElement('div'); + riExchangePanel.id = 'inventory-ri-exchange'; + document.body.append(instancesEl, recsEl, historyEl, riExchangePanel); + + (api.listConvertibleRIs as jest.Mock).mockResolvedValue([]); + (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([]); + (api.getRIUtilization as jest.Mock).mockResolvedValue([]); + (api.getReshapeRecommendations as jest.Mock).mockResolvedValue({ recommendations: [], recs_staleness: '', recs_collected_at: null }); + (api.getRIExchangeHistory as jest.Mock).mockResolvedValue([]); + + _providerListeners.length = 0; + _accountListeners.length = 0; + const s = stateMod(); + s.subscribeProvider.mockImplementation((cb: () => void) => { _providerListeners.push(cb); return () => undefined; }); + s.subscribeAccount.mockImplementation((cb: () => void) => { _accountListeners.push(cb); return () => undefined; }); + s.getCurrentProvider.mockReturnValue('aws'); + s.getCurrentAccountIDs.mockReturnValue([]); + }); + + afterEach(() => { + document.body.innerHTML = ''; + jest.clearAllMocks(); + }); + + it('forwards the single selected account to the AWS list endpoint', async () => { + stateMod().getCurrentAccountIDs.mockReturnValue(['123456789012']); + await loadRIExchange(); + expect(api.listConvertibleRIs).toHaveBeenCalledWith('123456789012'); + }); + + it('does not forward an account id when more than one account is selected', async () => { + stateMod().getCurrentAccountIDs.mockReturnValue(['111111111111', '222222222222']); + await loadRIExchange(); + expect(api.listConvertibleRIs).toHaveBeenCalledWith(undefined); + }); + + it('renders a scoped empty-state naming the AWS account', async () => { + stateMod().getCurrentAccountIDs.mockReturnValue(['123456789012']); + (api.listConvertibleRIs as jest.Mock).mockResolvedValue([]); + await loadRIExchange(); + expect(instancesEl.textContent).toContain('123456789012'); + }); + + it('loads Azure reservations (not AWS RIs) when provider is azure', async () => { + stateMod().getCurrentProvider.mockReturnValue('azure'); + (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([ + { reservation_order_id: 'o1', reservation_id: 'r1', sku: 'Standard_D2s_v3', quantity: 2, region: 'eastus', term: 'P1Y', instance_flexibility: 'On', display_name: 'web-rsv' }, + ]); + await loadRIExchange(); + expect(api.listExchangeableAzureRIs).toHaveBeenCalled(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(instancesEl.textContent).toContain('Standard_D2s_v3'); + expect(instancesEl.textContent).toContain('web-rsv'); + // Reshape recommendations are AWS-only -> provider-aware not-applicable copy. + expect(recsEl.textContent).toContain('not available for Azure'); + }); + + it('shows a provider-aware empty state for Azure with no reservations', async () => { + stateMod().getCurrentProvider.mockReturnValue('azure'); + stateMod().getCurrentAccountIDs.mockReturnValue(['sub-abc']); + (api.listExchangeableAzureRIs as jest.Mock).mockResolvedValue([]); + await loadRIExchange(); + expect(api.listExchangeableAzureRIs).toHaveBeenCalledWith('sub-abc'); + expect(instancesEl.textContent).toContain('No exchangeable reservations for Azure subscription sub-abc'); + }); + + it('shows the not-available empty state for GCP and calls no list endpoint', async () => { + stateMod().getCurrentProvider.mockReturnValue('gcp'); + await loadRIExchange(); + expect(api.listConvertibleRIs).not.toHaveBeenCalled(); + expect(api.listExchangeableAzureRIs).not.toHaveBeenCalled(); + expect(instancesEl.textContent).toContain("isn't available for GCP"); + expect(recsEl.textContent).toContain("isn't available for GCP"); + expect(historyEl.textContent).toContain("isn't available for GCP"); + }); + + it('does not leave stale AWS rows when switching to GCP', async () => { + // First load AWS with a row present. + (api.listConvertibleRIs as jest.Mock).mockResolvedValue([{ + reserved_instance_id: 'ri-1', instance_type: 'm5.large', availability_zone: 'us-east-1a', + instance_count: 1, start: '2026-01-01T00:00:00Z', end: '2027-01-01T00:00:00Z', + offering_type: 'Convertible', fixed_price: 0, usage_price: 0, state: 'active', normalization_factor: 4, + }]); + await loadRIExchange(); + expect(instancesEl.textContent).toContain('m5.large'); + // Switch to GCP -> table must be replaced by the empty state. + stateMod().getCurrentProvider.mockReturnValue('gcp'); + await loadRIExchange(); + expect(instancesEl.textContent).not.toContain('m5.large'); + expect(instancesEl.textContent).toContain("isn't available for GCP"); + }); + + it('closes an in-progress exchange modal when the filter changes', async () => { + const modal = createModal(); + modal.classList.remove('hidden'); // simulate an open exchange modal + setupRIExchangeHandlers(); + _providerListeners.forEach(cb => cb()); + // Modal is closed synchronously on the chip event. + expect(modal.classList.contains('hidden')).toBe(true); + }); +}); diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index 8936978f2..7feb085bf 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -43,6 +43,7 @@ export type { SavingsBreakdownValue, SavingsAnalyticsFilters, ConvertibleRI, + ExchangeableAzureRI, RIUtilization, ReshapeRecommendation, OfferingOption, @@ -181,6 +182,7 @@ export { export type { ReshapeRecommendationsResponse } from './riexchange'; export { listConvertibleRIs, + listExchangeableAzureRIs, getRIUtilization, getReshapeRecommendations, getExchangeQuote, diff --git a/frontend/src/api/riexchange.ts b/frontend/src/api/riexchange.ts index 31c7041de..d23423d78 100644 --- a/frontend/src/api/riexchange.ts +++ b/frontend/src/api/riexchange.ts @@ -5,6 +5,7 @@ import { apiRequest } from './client'; import type { ConvertibleRI, + ExchangeableAzureRI, RIUtilization, ReshapeRecommendation, ExchangeQuoteRequest, @@ -17,13 +18,31 @@ import type { } from './types'; /** - * List active convertible Reserved Instances + * List active convertible Reserved Instances for the running AWS account. + * + * The optional accountID scopes the listing to a single AWS account so the + * page honours the Main Header global account filter (issue #871). The + * backend returns an empty list when the selected account is not the running + * AWS account. */ -export async function listConvertibleRIs(): Promise { - const resp = await apiRequest<{ instances: ConvertibleRI[] }>('/ri-exchange/instances'); +export async function listConvertibleRIs(accountID?: string): Promise { + const qs = accountID ? `?account_id=${encodeURIComponent(accountID)}` : ''; + const resp = await apiRequest<{ instances: ConvertibleRI[] }>(`/ri-exchange/instances${qs}`); return resp.instances ?? []; } +/** + * List active Azure VM reservations eligible for the cross-SKU/cross-region + * exchange flow (issue #871). The optional subscriptionID scopes the + * capacity-provider registration check to one subscription; the listing + * itself is tenant-wide on the backend. + */ +export async function listExchangeableAzureRIs(subscriptionID?: string): Promise { + const qs = subscriptionID ? `?subscription_id=${encodeURIComponent(subscriptionID)}` : ''; + const resp = await apiRequest<{ reservations: ExchangeableAzureRI[] }>(`/ri-exchange/azure-instances${qs}`); + return resp.reservations ?? []; +} + /** * Get per-RI utilization from Cost Explorer */ diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index e051f26ac..900627ba2 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -512,6 +512,22 @@ export interface ConvertibleRI { normalization_factor: number; } +// ExchangeableAzureRI is one Azure VM reservation eligible for the +// cross-SKU/cross-region exchange flow, returned by +// GET /api/ri-exchange/azure-instances. Mirrors the Go +// azurecompute.ExchangeableReservation struct (issue #871). +export interface ExchangeableAzureRI { + reservation_order_id: string; + reservation_id: string; + sku: string; + quantity: number; + region?: string; + term?: string; + expiry_date?: string; + instance_flexibility: string; + display_name?: string; +} + export interface RIUtilization { reserved_instance_id: string; utilization_percent: number; diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 8f51cdfcc..576a89bbb 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -10,6 +10,7 @@ import { switchTab, switchSettingsSubTab } from './navigation'; import { confirmDialog } from './confirmDialog'; import type { ConvertibleRI, + ExchangeableAzureRI, RIUtilization, ReshapeRecommendation, ExchangeQuoteSummary, @@ -18,6 +19,7 @@ import type { OfferingOption, TargetOffering, ReshapeRecommendationsResponse, + Provider, } from './api'; import { openModal, closeModal } from './modal'; import { showSkeletonRows, teardownSkeleton } from './lib/skeleton'; @@ -33,6 +35,32 @@ let currentRecommendations: ReshapeRecommendation[] = []; // Generation counter to prevent stale utilization data from overwriting fresh data let utilizationGeneration = 0; +// The AWS account the convertible-RI list is currently scoped to (the +// single-select account chip value, or undefined for all-accounts). Tracked +// at module scope so the asynchronous utilization re-render preserves the +// scoped empty-state copy instead of falling back to the unscoped message +// (issue #871). +let currentRIAccountID: string | undefined; + +// Human-readable provider labels for empty-state copy and the not-available +// message. Mirrors the map in inventory.ts. +const PROVIDER_LABELS: Record = { aws: 'AWS', azure: 'Azure', gcp: 'GCP' }; + +function providerLabel(provider: string): string { + return PROVIDER_LABELS[provider] ?? provider.toUpperCase(); +} + +// resolveScope reads the Main Header global Provider/Account chips so every +// RI Exchange load is scoped consistently with the other Inventory sub-tabs +// (issue #871). The account chip is single-select; an account_id is forwarded +// only when exactly one account is active. +function resolveScope(): { provider: Provider | ''; accountID?: string } { + const provider = state.getCurrentProvider(); + const accountIDs = state.getCurrentAccountIDs(); + const accountID = accountIDs.length === 1 ? accountIDs[0] : undefined; + return { provider, accountID }; +} + // Mode label mapping — single source of truth const MODE_LABELS: Record = { manual: "Manual Approval", auto: "Fully Automated" }; const MODE_VALUES: Record = Object.fromEntries( @@ -43,16 +71,92 @@ const MODE_VALUES: Record = Object.fromEntries( void MODE_VALUES; /** - * Load the RI Exchange tab — called when tab is activated + * Load the RI Exchange tab — called when tab is activated and whenever the + * Main Header global Provider/Account filter changes (issue #871). + * + * RI Exchange is multi-provider: + * - AWS: convertible RIs + reshape recommendations + exchange history. + * - Azure: exchangeable VM reservations + exchange history. Reshape + * recommendations are a Cost-Explorer/AWS concept, so that + * section shows a provider-aware "not applicable" empty state. + * - GCP: no RI-exchange concept; every section shows a "not available + * for GCP" empty state and no list endpoint is called. */ export async function loadRIExchange(): Promise { + const { provider, accountID } = resolveScope(); + + if (provider === 'gcp') { + renderGCPEmptyStates(); + return; + } + + if (provider === 'azure') { + await Promise.all([ + loadExchangeableAzureRIs(accountID), + loadExchangeHistory(), + ]); + renderReshapeNotApplicable('azure'); + return; + } + + // AWS (default / empty provider): full convertible-RI flow. await Promise.all([ - loadConvertibleRIs(), + loadConvertibleRIs(accountID), loadReshapeRecommendations(), loadExchangeHistory(), ]); } +/** + * Render the "not available for GCP" empty state across all three RI Exchange + * sections. GCP has no Reserved-Instance exchange concept, so we never call a + * list endpoint and never leave stale AWS/Azure rows behind. + */ +function renderGCPEmptyStates(): void { + currentRIs = []; + currentUtilization = new Map(); + currentRecommendations = []; + currentRIAccountID = undefined; + + const instances = document.getElementById('ri-exchange-instances-list'); + if (instances) renderEmptyState(instances, "RI Exchange isn't available for GCP."); + + const recs = document.getElementById('ri-exchange-recommendations-list'); + if (recs) renderEmptyState(recs, "RI Exchange isn't available for GCP."); + + const history = document.getElementById('ri-exchange-history-list'); + if (history) renderEmptyState(history, "RI Exchange isn't available for GCP."); + + renderReshapeStalenessBanner('', null); +} + +/** + * Render a provider-aware "reshape recommendations not applicable" message. + * Reshape recommendations are derived from AWS Cost Explorer, so they do not + * apply to Azure (or any non-AWS) provider. + */ +function renderReshapeNotApplicable(provider: string): void { + currentRecommendations = []; + const container = document.getElementById('ri-exchange-recommendations-list'); + if (container) { + renderEmptyState(container, `Reshape recommendations are not available for ${providerLabel(provider)}.`); + } + renderReshapeStalenessBanner('', null); +} + +/** + * Render a plain `.empty` paragraph via textContent (no innerHTML) so backend + * or provider-derived strings can never inject markup. Wipes any prior + * children (skeleton, stale table) for a clean handoff. + */ +function renderEmptyState(container: HTMLElement, message: string): void { + container.textContent = ''; + const p = document.createElement('p'); + p.className = 'empty'; + p.textContent = message; + container.appendChild(p); +} + /** * True when the RI Exchange sub-tab is the currently visible panel. * The sub-tab panel id is "inventory-ri-exchange" (see index.html). @@ -96,12 +200,22 @@ export function setupRIExchangeHandlers(): void { }); } - // issue #186: reload when the global provider/account filter changes - // so the RI Exchange tables stay consistent with the rest of the UI. - // Coalesce the two events into a single reload (provider change also - // fires an account change via the topbar-filters.ts clearing logic). + // issue #186 / #871: reload (scoped to the new provider/account) when the + // global Main Header filter changes so the RI Exchange tables stay + // consistent with the rest of the Inventory sub-tabs. Coalesce the two + // events into a single reload (provider change also fires an account change + // via the topbar-filters.ts clearing logic). + // + // RI Exchange is a mutating workflow, so we ALSO close any in-progress + // exchange modal the moment the filter changes — the source RI it targets + // may no longer be visible (e.g. provider switched AWS -> Azure), and acting + // on a now-hidden RI would be confusing and potentially wrong. We close + // synchronously on the chip event (not inside the coalescing microtask) so + // the stale selection is torn down even if the reload is skipped because the + // sub-tab is off-screen. let reloadQueued = false; const scheduleReload = (): void => { + closeExchangeModalIfOpen(); if (!isRIExchangeSubtabActive() || reloadQueued) return; reloadQueued = true; queueMicrotask(() => { @@ -113,11 +227,24 @@ export function setupRIExchangeHandlers(): void { state.subscribeAccount(scheduleReload); } +/** + * Close the RI Exchange quote/execute modal if it is currently open. Called + * on a global filter change so the user can't act on an exchange selection + * whose source RI may no longer be in scope (issue #871). No-op when the + * modal is absent or already hidden. + */ +function closeExchangeModalIfOpen(): void { + const modal = document.getElementById('ri-exchange-modal'); + if (modal && !modal.classList.contains('hidden')) { + closeModal(modal); + } +} + // ────────────────────────────────────────────── // Convertible RIs table // ────────────────────────────────────────────── -async function loadConvertibleRIs(): Promise { +async function loadConvertibleRIs(accountID?: string): Promise { const container = document.getElementById('ri-exchange-instances-list'); if (!container) return; @@ -128,9 +255,13 @@ async function loadConvertibleRIs(): Promise { // the children on success for a clean handoff. showSkeletonRows(container, 3, 8); + currentRIAccountID = accountID; + try { - currentRIs = await api.listConvertibleRIs(); - renderRIsTable(container); + // issue #871: scope to the selected account so the page honours the + // Main Header global filter. + currentRIs = await api.listConvertibleRIs(accountID); + renderRIsTable(container, accountID); // Load utilization asynchronously (Cost Explorer is slow) utilizationGeneration++; void loadUtilization(utilizationGeneration); @@ -141,23 +272,61 @@ async function loadConvertibleRIs(): Promise { } } +/** + * Load and render the Azure exchangeable VM reservations (issue #871). + * Azure reservations have no utilization/reshape pipeline, so this renders a + * standalone table into the same Active-list container the AWS path uses. + * The optional accountID is the selected subscription chip; it scopes the + * capacity-provider registration check on the backend. + */ +async function loadExchangeableAzureRIs(accountID?: string): Promise { + const container = document.getElementById('ri-exchange-instances-list'); + if (!container) return; + + // Azure table has 6 columns: Reservation / SKU / Quantity / Region / + // Term / Expiry (see renderAzureRIsTable). + showSkeletonRows(container, 3, 6); + + // AWS-only module state is irrelevant on the Azure path; reset it so a + // later provider switch back to AWS starts clean and reshape copy that + // reads currentRIs.length isn't skewed by stale AWS rows. + currentRIs = []; + currentUtilization = new Map(); + currentRIAccountID = undefined; + + try { + const reservations = await api.listExchangeableAzureRIs(accountID); + renderAzureRIsTable(container, reservations, accountID); + } catch (error) { + teardownSkeleton(container); + const err = error as Error; + container.innerHTML = `

Failed to load Azure reservations: ${escapeHtml(err.message)}

`; + } +} + async function loadUtilization(generation: number): Promise { try { const utilization = await api.getRIUtilization(); // Discard if a newer load was started while we were waiting if (generation !== utilizationGeneration) return; currentUtilization = new Map(utilization.map(u => [u.reserved_instance_id, u])); - // Re-render table with utilization data + // Re-render table with utilization data, preserving the active account + // scope so a scoped empty-state message isn't lost (issue #871). const container = document.getElementById('ri-exchange-instances-list'); - if (container) renderRIsTable(container); + if (container) renderRIsTable(container, currentRIAccountID); } catch (error) { console.error('Failed to load RI utilization:', error); } } -function renderRIsTable(container: HTMLElement): void { +function renderRIsTable(container: HTMLElement, accountID?: string): void { if (!currentRIs || currentRIs.length === 0) { - container.innerHTML = '

No active convertible Reserved Instances found.

'; + // issue #871: name the active scope so the user knows the result is + // filtered (by the account chip) rather than globally empty. + const msg = accountID + ? `No active convertible Reserved Instances for AWS account ${accountID}.` + : 'No active convertible Reserved Instances found for AWS.'; + renderEmptyState(container, msg); return; } @@ -212,6 +381,67 @@ function renderRIsTable(container: HTMLElement): void { }); } +/** + * Render the Azure exchangeable-reservations table (issue #871). Built + * entirely via DOM construction / textContent — never innerHTML — so any + * Azure-derived field (SKU, display name, region) cannot inject markup. + * + * Azure reservations are listed read-only here; the quote/execute modal is + * AWS-specific (offering-UUID flow), so no per-row Exchange button is shown. + */ +function renderAzureRIsTable( + container: HTMLElement, + reservations: ExchangeableAzureRI[], + accountID?: string, +): void { + if (!reservations || reservations.length === 0) { + const msg = accountID + ? `No exchangeable reservations for Azure subscription ${accountID}.` + : 'No exchangeable reservations found for Azure.'; + renderEmptyState(container, msg); + return; + } + + container.textContent = ''; + const table = document.createElement('table'); + + const thead = document.createElement('thead'); + const headerRow = document.createElement('tr'); + for (const label of ['Reservation', 'SKU', 'Quantity', 'Region', 'Term', 'Expiry']) { + const th = document.createElement('th'); + th.textContent = label; + headerRow.appendChild(th); + } + thead.appendChild(headerRow); + table.appendChild(thead); + + const tbody = document.createElement('tbody'); + for (const r of reservations) { + tbody.appendChild(buildAzureRIRow(r)); + } + table.appendChild(tbody); + container.appendChild(table); +} + +function buildAzureRIRow(r: ExchangeableAzureRI): HTMLTableRowElement { + const tr = document.createElement('tr'); + + appendAzureCell(tr, r.display_name || r.reservation_id); + appendAzureCell(tr, r.sku); + appendAzureCell(tr, String(r.quantity)); + appendAzureCell(tr, r.region || '—'); + appendAzureCell(tr, r.term || '—'); + appendAzureCell(tr, r.expiry_date ? formatDate(r.expiry_date) : '—'); + + return tr; +} + +function appendAzureCell(tr: HTMLTableRowElement, text: string): void { + const td = document.createElement('td'); + td.textContent = text; + tr.appendChild(td); +} + // ────────────────────────────────────────────── // Reshape recommendations // ────────────────────────────────────────────── diff --git a/internal/api/handler.go b/internal/api/handler.go index 65b378695..1f6e1884e 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -89,6 +89,13 @@ type Handler struct { // the integration suite runs hermetically. reshapeAccountResolver func(context.Context) (string, error) + // Optional resolver for the running AWS account number, injected by + // the listConvertibleRIs tests so the account-scoping branch can run + // without live STS credentials. When nil (production default), the + // handler calls h.resolveAWSAccountID. Returns the raw AWS account + // number (e.g. "123456789012"), matching the account_id chip value. + riInstancesAccountResolver func(context.Context) (string, error) + // Optional org-discovery factory used by tests to avoid live AWS // Organizations API calls. When nil (production default), the handler // falls back to accounts.DiscoverOrgAccounts which dials Organizations diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index 5e6f9c284..cacb25abe 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -234,12 +234,36 @@ func (h *Handler) loadAWSConfigWithRegion(ctx context.Context, region string) (a return cfg, nil } -// listConvertibleRIs returns all active convertible Reserved Instances. +// listConvertibleRIs returns all active convertible Reserved Instances for +// the running AWS account. +// +// The optional ?account_id= query parameter narrows the listing to a single +// AWS account so the page honours the Main Header global account filter +// (issue #871). Convertible RIs are read from the deployment's ambient AWS +// credentials, which resolve to exactly one account number; when the chip +// selects a different account, none of these RIs belong to it, so we return +// an empty list rather than the unscoped fleet. A real STS failure fails +// closed (returns an error) instead of silently leaking the ambient account's +// RIs under another account's filter. func (h *Handler) listConvertibleRIs(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) { if _, err := h.requirePermission(ctx, req, "view", "purchases"); err != nil { return nil, err } + if accountID := req.QueryStringParameters["account_id"]; accountID != "" { + resolve := h.resolveAWSAccountID + if h.riInstancesAccountResolver != nil { + resolve = h.riInstancesAccountResolver + } + runningAccountID, err := resolve(ctx) + if err != nil { + return nil, fmt.Errorf("failed to resolve running AWS account for RI scope: %w", err) + } + if runningAccountID != accountID { + return &ConvertibleRIsResponse{Instances: []ec2svc.ConvertibleRI{}}, nil + } + } + region := req.QueryStringParameters["region"] cfg, err := h.loadAWSConfigWithRegion(ctx, region) if err != nil { diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index 8ed43ee48..3aa0feae8 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/internal/api/handler_ri_exchange_test.go @@ -26,6 +26,49 @@ func TestListConvertibleRIs_RequiresAdmin(t *testing.T) { assert.Contains(t, err.Error(), "authentication") } +// issue #871: the AWS convertible-RI list must honour the Main Header global +// account filter. When the ?account_id= chip selects an account other than the +// running (ambient) AWS account, none of these RIs belong to it, so the +// handler returns an empty list without touching AWS config. +func TestListConvertibleRIs_AccountScopeMismatchReturnsEmpty(t *testing.T) { + mockAuth := &mockAuthForExchange{} + h := &Handler{ + auth: mockAuth, + // Running account differs from the requested account_id. + riInstancesAccountResolver: func(_ context.Context) (string, error) { + return "999999999999", nil + }, + } + + resp, err := h.listConvertibleRIs(context.Background(), &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer test-token"}, + QueryStringParameters: map[string]string{"account_id": "123456789012"}, + }) + require.NoError(t, err) + typed, ok := resp.(*ConvertibleRIsResponse) + require.True(t, ok, "expected *ConvertibleRIsResponse, got %T", resp) + assert.Empty(t, typed.Instances, "RIs from a different account must not leak under an account_id filter") +} + +// issue #871: a real STS resolution failure must fail closed (propagate the +// error) rather than silently falling through to the unscoped fleet. +func TestListConvertibleRIs_AccountResolveErrorFailsClosed(t *testing.T) { + mockAuth := &mockAuthForExchange{} + h := &Handler{ + auth: mockAuth, + riInstancesAccountResolver: func(_ context.Context) (string, error) { + return "", fmt.Errorf("sts get-caller-identity denied") + }, + } + + _, err := h.listConvertibleRIs(context.Background(), &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer test-token"}, + QueryStringParameters: map[string]string{"account_id": "123456789012"}, + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "resolve running AWS account") +} + func TestGetRIUtilization_RequiresAdmin(t *testing.T) { h := &Handler{} _, err := h.getRIUtilization(context.Background(), &events.LambdaFunctionURLRequest{})