diff --git a/frontend/src/__tests__/history-marketplace-sell-button.test.ts b/frontend/src/__tests__/history-marketplace-sell-button.test.ts new file mode 100644 index 000000000..9da8ae1b1 --- /dev/null +++ b/frontend/src/__tests__/history-marketplace-sell-button.test.ts @@ -0,0 +1,234 @@ +/** + * History inline "Sell on Marketplace" button tests (issue #292, FINDING D). + * + * Regression guard for the "dead Sell button" gap: canSellOnMarketplace used to + * require offering_class === 'standard', but nothing ever wrote a 'standard' + * row end-to-end -- CUDly stamps 'convertible' on its own EC2 purchases, and + * externally-created Standard RIs arrive with an EMPTY offering_class until the + * backend lazily populates it from AWS on the list call. So the button never + * rendered for the very case the feature targets. + * + * The fix shows the Sell button for completed AWS EC2 rows whose offering_class + * is 'standard' OR still unknown (empty), and lets the backend + * populateOfferingClass + gate (it 400s a fetched 'convertible') decide. The + * backend authorizeSessionSell remains the real security boundary; these tests + * only verify the UX gate. + * + * Tested matrix: + * 1. completed aws/ec2 row, offering_class 'standard' -> shown. + * 2. completed aws/ec2 row, offering_class EMPTY (unknown) -> shown (fix). + * 3. completed aws/ec2 row, offering_class 'convertible' -> hidden. + * 4. completed aws/ec2 row with an active listing -> hidden. + * 5. completed aws/rds row, offering_class EMPTY -> hidden. + * 6. completed azure/compute row, offering_class EMPTY -> hidden. + * 7. anonymous (no current user cached) -> hidden. + */ + +import { loadHistory } from '../history'; + +jest.mock('../api', () => ({ + getHistory: jest.fn(), + createMarketplaceListing: jest.fn(), + cancelMarketplaceListing: jest.fn(), +})); + +jest.mock('../navigation', () => ({ + switchTab: jest.fn(), +})); + +jest.mock('../utils', () => ({ + formatCurrency: jest.fn((val) => `$${val || 0}`), + formatDate: jest.fn((val) => (val ? new Date(val).toLocaleDateString() : '')), + formatTerm: jest.fn((years) => (years == null ? '' : `${years} Year${years === 1 ? '' : 's'}`)), + escapeHtml: jest.fn((str) => str || ''), + escapeHtmlAttr: jest.fn((str) => str || ''), + amortizedMonthly: jest.fn((monthly) => monthly), + populateAccountFilter: jest.fn(() => Promise.resolve()), +})); + +jest.mock('../confirmDialog', () => ({ + confirmDialog: jest.fn(), +})); + +jest.mock('../toast', () => ({ + showToast: jest.fn(), +})); + +jest.mock('../state', () => ({ + getCurrentUser: jest.fn(), + getCurrentProvider: jest.fn().mockReturnValue(''), + setCurrentProvider: jest.fn(), + getCurrentAccountIDs: jest.fn().mockReturnValue([]), + setCurrentAccountIDs: jest.fn(), + subscribeProvider: jest.fn().mockReturnValue(() => {}), + subscribeAccount: jest.fn().mockReturnValue(() => {}), + getAmortizeUpfront: jest.fn().mockReturnValue(false), + setAmortizeUpfront: jest.fn(), + subscribeAmortizeUpfront: jest.fn().mockReturnValue(() => {}), +})); + +import * as api from '../api'; +import { getCurrentUser } from '../state'; + +// Administrators group GUID -- mirrors ADMINISTRATORS_GROUP_ID in +// frontend/src/permissions.ts. Without this, isAdmin() returns false and +// canAccess('admin', '*') in canSellOnMarketplace's fallback branch does not +// grant. Seeded-group GUID, not the label string. +const ADMIN_GROUP_ID = '00000000-0000-5000-8000-000000000001'; +const ADMIN_USER = { id: 'admin-uuid', email: 'admin@example.com', groups: [ADMIN_GROUP_ID] }; + +// A recent purchase with plenty of term left so the remaining-months guard in +// canSellOnMarketplace (needs >= 1 month remaining) always passes. +const RECENT = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000).toISOString(); // -30 days + +function setupDOM(): void { + while (document.body.firstChild) document.body.removeChild(document.body.firstChild); + + const mkInput = (id: string): HTMLInputElement => { + const el = document.createElement('input'); + el.type = 'date'; + el.id = id; + return el; + }; + const mkSelect = (id: string): HTMLSelectElement => { + const el = document.createElement('select'); + el.id = id; + const opt = document.createElement('option'); + opt.value = ''; + opt.textContent = 'All'; + el.appendChild(opt); + return el; + }; + const mkDiv = (id: string): HTMLDivElement => { + const el = document.createElement('div'); + el.id = id; + return el; + }; + + document.body.appendChild(mkInput('history-start')); + document.body.appendChild(mkInput('history-end')); + document.body.appendChild(mkSelect('history-provider-filter')); + document.body.appendChild(mkSelect('history-account-filter')); + document.body.appendChild(mkDiv('history-summary')); + document.body.appendChild(mkDiv('history-list')); + document.body.appendChild(mkDiv('purchases-approval-queue')); +} + +// A completed AWS EC2 RI row with a long remaining term. Overrides tune the +// offering_class / provider / service / listing_state for each case. +function makeRow(overrides: Record) { + return { + purchase_id: 'ri-1', + timestamp: RECENT, + provider: 'aws', + service: 'ec2', + resource_type: 't3.large', + region: 'us-east-1', + count: 1, + term: 36, + upfront_cost: 1200, + estimated_savings: 300, + plan_name: '', + status: 'completed', + offering_class: 'standard', + ...overrides, + }; +} + +function sellIds(): (string | undefined)[] { + const list = document.getElementById('history-list')!; + const buttons = list.querySelectorAll('.history-marketplace-sell-btn'); + return Array.from(buttons).map((b) => b.dataset['marketplaceSellId']); +} + +describe('History inline Sell on Marketplace button (issue #292)', () => { + beforeEach(() => { + setupDOM(); + jest.clearAllMocks(); + (getCurrentUser as jest.Mock).mockReturnValue(ADMIN_USER); + }); + + test('shows Sell for a completed AWS EC2 row with offering_class=standard', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'ri-standard', offering_class: 'standard' })], + }); + + await loadHistory(); + + expect(sellIds()).toEqual(['ri-standard']); + }); + + test('shows Sell for a completed AWS EC2 row with EMPTY offering_class (the reachability fix)', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'ri-unknown', offering_class: '' })], + }); + + await loadHistory(); + + // The backend lazily populates + gates; the UI must surface the button so + // an externally-created Standard RI is actually listable end-to-end. + expect(sellIds()).toEqual(['ri-unknown']); + }); + + test('hides Sell for a known convertible RI', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'ri-convertible', offering_class: 'convertible' })], + }); + + await loadHistory(); + + expect(sellIds()).toEqual([]); + }); + + test('hides Sell when a listing is already active (Cancel is shown instead)', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [ + makeRow({ purchase_id: 'ri-active', offering_class: 'standard', listing_state: 'active', listing_id: 'ril-1' }), + ], + }); + + await loadHistory(); + + expect(sellIds()).toEqual([]); + }); + + test('hides Sell for a non-EC2 AWS row even when offering_class is unknown', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'rds-unknown', service: 'rds', offering_class: '' })], + }); + + await loadHistory(); + + expect(sellIds()).toEqual([]); + }); + + test('hides Sell for a non-AWS row even when offering_class is unknown', async () => { + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [ + makeRow({ purchase_id: 'az-unknown', provider: 'azure', service: 'compute', offering_class: '' }), + ], + }); + + await loadHistory(); + + expect(sellIds()).toEqual([]); + }); + + test('hides Sell when no user is cached (anonymous)', async () => { + (getCurrentUser as jest.Mock).mockReturnValue(null); + (api.getHistory as jest.Mock).mockResolvedValue({ + summary: {}, + purchases: [makeRow({ purchase_id: 'ri-anon', offering_class: '' })], + }); + + await loadHistory(); + + expect(sellIds()).toEqual([]); + }); +}); diff --git a/frontend/src/__tests__/permissions.test.ts b/frontend/src/__tests__/permissions.test.ts index eb0c779c3..034de5b75 100644 --- a/frontend/src/__tests__/permissions.test.ts +++ b/frontend/src/__tests__/permissions.test.ts @@ -74,6 +74,9 @@ describe('permissions', () => { // Added by PR #804: revoke-own gates the History inline Revoke button // for completed Azure purchases within the free-cancel window. 'revoke-own:purchases', + // Added by issue #292: sell-own gates the History "Sell on Marketplace" + // button for completed Standard RIs the user purchased themselves. + 'sell-own:purchases', ]; expected.forEach((p) => expect(perms.has(p)).toBe(true)); // approve-own must NOT be in the standard user permission set (issue #1407). diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index e3fb66f73..e59ac15d9 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -141,9 +141,11 @@ export { runPlannedPurchase, deletePlannedPurchase, createPlannedPurchases, - revokePurchase + revokePurchase, + createMarketplaceListing, + cancelMarketplaceListing } from './purchases'; -export type { RetryPurchaseResult, RevokePurchaseResult } from './purchases'; +export type { RetryPurchaseResult, RevokePurchaseResult, MarketplacePriceTier, MarketplaceListResult } from './purchases'; // Re-export users functions export { diff --git a/frontend/src/api/purchases.ts b/frontend/src/api/purchases.ts index b0f19bb65..a965c74c2 100644 --- a/frontend/src/api/purchases.ts +++ b/frontend/src/api/purchases.ts @@ -168,3 +168,48 @@ export async function createPlannedPurchases(planId: string, count: number, star body: JSON.stringify({ count, start_date: startDate }) }); } + +// ── RI Marketplace (issue #292) ─────────────────────────────────────────────── + +export interface MarketplacePriceTier { + /** Number of months remaining this price tier covers. */ + term_months: number; + /** USD list price per unit for this tier. */ + price: number; +} + +export interface MarketplaceListResult { + listing_id: string; + listing_state: string; + price_schedule: MarketplacePriceTier[]; + aws_fee_percent: number; + note?: string; +} + +/** + * Create an AWS RI Marketplace listing for a Standard Reserved Instance. + * purchaseId is the purchase_history.purchase_id (AWS ReservedInstancesId). + * priceSchedule is optional: when omitted the backend computes a default. + */ +export async function createMarketplaceListing( + purchaseId: string, + priceSchedule?: MarketplacePriceTier[], +): Promise { + const body = priceSchedule && priceSchedule.length > 0 + ? JSON.stringify({ price_schedule: priceSchedule }) + : undefined; + return apiRequest(`/purchases/${purchaseId}/marketplace-list`, { + method: 'POST', + ...(body ? { body } : {}), + }); +} + +/** + * Cancel an active AWS RI Marketplace listing. + */ +export async function cancelMarketplaceListing(purchaseId: string): Promise<{ listing_id: string; listing_state: string }> { + return apiRequest<{ listing_id: string; listing_state: string }>( + `/purchases/${purchaseId}/marketplace-cancel`, + { method: 'POST' }, + ); +} diff --git a/frontend/src/history.ts b/frontend/src/history.ts index 92678e7e3..0bd0c160c 100644 --- a/frontend/src/history.ts +++ b/frontend/src/history.ts @@ -17,6 +17,14 @@ import { getAccountName } from './recommendations'; const VALID_PROVIDERS: api.Provider[] = ['aws', 'azure', 'gcp']; +// AWS Marketplace fee/pricing constants (must stay in sync with handler_marketplace.go). +// awsMarketplaceFeePercent: transaction fee deducted from listing proceeds (published by AWS). +const AWS_MARKETPLACE_FEE_PERCENT = 12; +// awsMarketplaceNetFactor: fraction of list price the seller receives after the fee. +const AWS_MARKETPLACE_NET_FACTOR = 1 - AWS_MARKETPLACE_FEE_PERCENT / 100; +// awsMarketplaceBuyerDiscountFactor: applied to residual RI value to compute the default list price. +const AWS_MARKETPLACE_BUYER_DISCOUNT = 0.95; + type StatusFilter = 'all' | 'pending' | 'completed' | 'failed' | 'expired' | 'cancelled'; // Cache of the last-rendered purchase list so the status-chip click handler @@ -593,6 +601,82 @@ function retryThresholdReached(p: HistoryPurchase): boolean { return (p.retry_attempt_n ?? 0) >= RETRY_THRESHOLD; } +// canSellOnMarketplace returns true when the current session is permitted +// to list the given completed history row on the AWS RI Marketplace +// (issue #292). UX gate only -- the backend authorizeSessionSell in +// internal/api/handler_marketplace.go remains the security boundary; a +// false-positive here surfaces as a 403 toast on click. +// +// Conditions: +// * row must be a completed purchase (status "completed" or absent); +// * row must be an AWS EC2 RI (marketplace is EC2-only); +// * offering_class must be "standard" OR still unknown (empty). CUDly stamps +// "convertible" on its own EC2 purchases, but externally-created Standard +// RIs (and pre-migration rows) have an empty offering_class until the +// backend lazily populates it from AWS on the list call. The backend +// definitively gates -- it 400s a fetched "convertible" -- so we show the +// button for unknown-class EC2 rows and let the backend decide, otherwise +// the feature is unreachable end-to-end for the very case it targets; +// * no active listing already (listing_state != "active"); and +// * admin, or non-admin user (sell-own covers their own accounts -- +// we can't efficiently check per-account ownership client-side, so +// we show the button for all non-admin users and let the backend 403 +// when the account is out of scope). +function canSellOnMarketplace(p: HistoryPurchase): boolean { + const status = normalizeStatus(p).toLowerCase(); + if (status !== 'completed') return false; + // Marketplace listing is AWS EC2 Standard-RI only. Gate on provider/service + // so unknown-class rows for non-EC2 providers never show the Sell button. + if ((p.provider || '').toLowerCase() !== 'aws') return false; + if ((p.service || '').toLowerCase() !== 'ec2') return false; + const offeringClass = (p.offering_class || '').toLowerCase(); + if (offeringClass !== 'standard' && offeringClass !== '') return false; + if ((p.listing_state || '').toLowerCase() === 'active') return false; + const user = getCurrentUser(); + if (!user) return false; + // Gate on the sell verbs, not bare sign-in, so we don't show Sell to a + // role that lacks sell-own/sell-any and avoid frontend/backend auth drift + // (the backend authorizeSessionSell would 403 anyway). admin:* satisfies + // sell-own here since it is not carved out of admin. + if ( + !canAccess('admin', '*') && + !canAccess('sell-any', 'purchases') && + !canAccess('sell-own', 'purchases') + ) { + return false; + } + // Guard against listing a matured RI: compute remaining months from the + // purchase timestamp and the total term. term is in months; timestamp is + // the purchase date. We require at least 1 full month remaining. + const termMonths = typeof p.term === 'number' ? p.term : Number(p.term) || 0; + if (termMonths <= 0) return false; + const purchaseMs = new Date(p.timestamp).getTime(); + if (!Number.isFinite(purchaseMs)) return false; + const elapsedMonths = (Date.now() - purchaseMs) / (1000 * 60 * 60 * 24 * 30.4375); + const remainingMonths = termMonths - elapsedMonths; + if (remainingMonths < 1) return false; + return true; +} + +// canCancelMarketplaceListing returns true when there is an active listing +// that the current session can cancel. +function canCancelMarketplaceListing(p: HistoryPurchase): boolean { + if ((p.listing_state || '').toLowerCase() !== 'active') return false; + const user = getCurrentUser(); + if (!user) return false; + // Same sell-verb gate as canSellOnMarketplace: cancelling a listing is a + // marketplace write, so require sell-own/sell-any (or admin) rather than + // bare sign-in to keep the UX gate aligned with authorizeSessionSell. + if ( + !canAccess('admin', '*') && + !canAccess('sell-any', 'purchases') && + !canAccess('sell-own', 'purchases') + ) { + return false; + } + return true; +} + // shortExecID renders the first 8 chars of a UUID so inline lineage // links ("Retried as #abc12345") stay readable in the table cell. The // full ID is preserved in the data-history-status attribute so the @@ -615,7 +699,9 @@ function sameRowActions(btn: HTMLButtonElement): HTMLButtonElement[] { const cell = btn.closest('td') || btn.parentElement; if (!cell) return [btn]; return Array.from( - cell.querySelectorAll('.history-approve-btn, .history-cancel-btn, .history-revoke-btn'), + cell.querySelectorAll( + '.history-approve-btn, .history-cancel-btn, .history-revoke-btn, .history-marketplace-sell-btn, .history-marketplace-cancel-btn', + ), ); } @@ -694,15 +780,29 @@ function renderActionCell(p: HistoryPurchase): string { // see "this is a retry" provenance. lineage.push(`↻ Retry #${p.retry_attempt_n}`); } - if (lineage.length > 0) { - return lineage.join(' '); + // Build the trailing action buttons so they compose with lineage links + // — a retry-descendant can also have an active listing that needs a + // Cancel button, or be an Azure row still inside its revoke window. + const trailingActions: string[] = []; + if (p.purchase_id) { + // Completed Azure row within revocation window: Revoke button (issue + // #290). Only Azure supports direct in-app revocation; AWS and GCP have + // no cancel API so the button is suppressed for those providers. + if (canRevokeCompletedRow(p)) { + trailingActions.push(``); + } + // Completed Standard RI rows (AWS): Cancel listing / Sell on Marketplace + // (issue #292). Mutually exclusive with revoke in practice (revoke is + // Azure-only, marketplace is AWS Standard-RI-only). + if (canCancelMarketplaceListing(p)) { + trailingActions.push(``); + } else if (canSellOnMarketplace(p)) { + trailingActions.push(``); + } } - // Completed Azure row within revocation window: show Revoke button - // (issue #290). Only Azure supports direct in-app revocation; AWS and - // GCP have no cancel API so the button is suppressed for those providers. - if (canRevokeCompletedRow(p) && p.purchase_id) { - return ``; + if (lineage.length > 0 || trailingActions.length > 0) { + return [...lineage, ...trailingActions].join(' '); } return escapeHtml(p.plan_name || '-'); @@ -1068,6 +1168,128 @@ function wireRowActionHandlers(container: HTMLElement): void { } }); }); + + // Wire Sell on Marketplace button (issue #292). + // Flow: pricing/schedule modal (RI summary + default price + 12% fee) → + // user confirms → createMarketplaceListing. We never skip the pricing + // modal (CR finding: going straight from confirmDialog to the API call + // denies the user informed consent about the price and fee). + container.querySelectorAll('.history-marketplace-sell-btn[data-marketplace-sell-id]').forEach(btn => { + btn.addEventListener('click', async () => { + const id = btn.dataset['marketplaceSellId']; + if (!id) return; + + // Look up the purchase record so we can show a meaningful price summary. + const purchase = lastPurchases.find(p => p.purchase_id === id); + + // Build a pricing modal body with RI summary and fee breakdown. + const bodyEl = document.createElement('div'); + bodyEl.className = 'marketplace-pricing-modal-body'; + + if (purchase) { + const termMonths = typeof purchase.term === 'number' ? purchase.term : Number(purchase.term) || 0; + const purchaseMs = new Date(purchase.timestamp).getTime(); + const elapsedMonths = Number.isFinite(purchaseMs) + ? (Date.now() - purchaseMs) / (1000 * 60 * 60 * 24 * 30.4375) + : 0; + const remainingMonths = Math.max(0, Math.round(termMonths - elapsedMonths)); + const upfront = purchase.upfront_cost ?? 0; + const monthly = purchase.monthly_cost ?? 0; + // Prorate the upfront cost to its residual value over the remaining + // term. Using the full upfront overstates the listing value for a + // partially elapsed RI (a 12-month RI at month 6 has only half its + // upfront value left). Mirrors resolveMarketplacePriceSchedule in + // internal/api/handler_marketplace.go, which drops the upfront term to + // 0 when the original term is unknown (<=0); we do the same here. + const upfrontRemaining = termMonths > 0 ? upfront * (remainingMonths / termMonths) : 0; + const totalValue = upfrontRemaining + monthly * remainingMonths; + const listPrice = totalValue * AWS_MARKETPLACE_BUYER_DISCOUNT; + const netProceeds = listPrice * AWS_MARKETPLACE_NET_FACTOR; + + const summaryEl = document.createElement('dl'); + summaryEl.className = 'marketplace-pricing-summary'; + const addRow = (label: string, value: string): void => { + const dt = document.createElement('dt'); + dt.textContent = label; + const dd = document.createElement('dd'); + dd.textContent = value; + summaryEl.appendChild(dt); + summaryEl.appendChild(dd); + }; + addRow('RI ID', id); + addRow('Region', purchase.region || '-'); + addRow('Resource type', purchase.resource_type || '-'); + addRow('Remaining term', remainingMonths === 1 ? '1 month' : `${remainingMonths} months`); + addRow('Default list price', formatCurrency(listPrice)); + addRow(`AWS fee (${AWS_MARKETPLACE_FEE_PERCENT}%)`, formatCurrency(listPrice * (AWS_MARKETPLACE_FEE_PERCENT / 100))); + addRow('Estimated net proceeds', formatCurrency(netProceeds)); + bodyEl.appendChild(summaryEl); + } + + const noteEl = document.createElement('p'); + noteEl.className = 'marketplace-pricing-note'; + noteEl.textContent = `AWS charges a ${AWS_MARKETPLACE_FEE_PERCENT}% transaction fee on proceeds. The default schedule prices the listing at ${(1 - AWS_MARKETPLACE_BUYER_DISCOUNT) * 100}% below remaining value. You can adjust pricing by contacting your administrator or modifying the schedule via the API. This action cannot be undone without cancelling the listing.`; + bodyEl.appendChild(noteEl); + + const ok = await confirmDialog({ + title: 'List this RI on the AWS Marketplace?', + body: bodyEl, + confirmLabel: 'Confirm listing', + destructive: false, + }); + if (!ok) return; + + const rowActions = sameRowActions(btn); + rowActions.forEach(b => { b.disabled = true; }); + try { + await api.createMarketplaceListing(id); + } catch (sellError) { + console.error('Failed to list RI on Marketplace:', sellError); + const err = sellError as Error; + showToast({ message: `Failed to list on Marketplace: ${err.message || 'unknown error'}`, kind: 'error' }); + rowActions.forEach(b => { b.disabled = false; }); + return; + } + showToast({ message: 'RI listed on Marketplace successfully', kind: 'success', timeout: 5_000 }); + try { + await loadHistory(); + } catch (reloadError) { + console.error('Failed to reload history after Marketplace listing:', reloadError); + } + }); + }); + + // Wire Cancel listing button (issue #292) + container.querySelectorAll('.history-marketplace-cancel-btn[data-marketplace-cancel-id]').forEach(btn => { + btn.addEventListener('click', async () => { + const id = btn.dataset['marketplaceCancelId']; + if (!id) return; + const ok = await confirmDialog({ + title: 'Cancel this Marketplace listing?', + body: 'This will remove the listing from the AWS Marketplace. Any existing buyer negotiations will be cancelled. You can relist the RI at any time.', + confirmLabel: 'Cancel listing', + destructive: true, + }); + if (!ok) return; + const rowActions = sameRowActions(btn); + rowActions.forEach(b => { b.disabled = true; }); + try { + await api.cancelMarketplaceListing(id); + } catch (cancelError) { + console.error('Failed to cancel Marketplace listing:', cancelError); + const err = cancelError as Error; + showToast({ message: `Failed to cancel listing: ${err.message || 'unknown error'}`, kind: 'error' }); + rowActions.forEach(b => { b.disabled = false; }); + return; + } + showToast({ message: 'Marketplace listing cancelled', kind: 'success', timeout: 5_000 }); + try { + await loadHistory(); + } catch (reloadError) { + console.error('Failed to reload history after Marketplace cancel:', reloadError); + } + }); + }); } // isPendingRow returns true when a history row represents a purchase diff --git a/frontend/src/permissions.generated.ts b/frontend/src/permissions.generated.ts index 71105363d..142989e97 100644 --- a/frontend/src/permissions.generated.ts +++ b/frontend/src/permissions.generated.ts @@ -25,6 +25,7 @@ export const USER_PERMS: ReadonlySet = new Set([ 'delete:plans', 'retry-own:purchases', 'revoke-own:purchases', + 'sell-own:purchases', 'update:plans', 'update:purchases', 'view:history', diff --git a/frontend/src/permissions.ts b/frontend/src/permissions.ts index d2c434b51..f30481e26 100644 --- a/frontend/src/permissions.ts +++ b/frontend/src/permissions.ts @@ -55,6 +55,13 @@ export type Action = | 'update-any' | 'revoke-own' | 'revoke-any' + // sell-own / sell-any gate the "Sell on Marketplace" button (issue #292). + // Mirrors the backend ActionSellOwn / ActionSellAny constants in + // internal/auth/types.go. sell-own:purchases is granted to every + // authenticated user via DefaultUserPermissions; sell-any has no default + // non-admin grant. + | 'sell-own' + | 'sell-any' | 'admin'; // Resource names. Closed enum for the same reason. diff --git a/frontend/src/types.ts b/frontend/src/types.ts index add694e1f..4e29f7e4a 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -282,6 +282,18 @@ export interface HistoryPurchase { revoked_at?: string; // revoked_via: "direct-api" or "support-case". Absent unless revoked. revoked_via?: string; + + // OfferingClass is "standard" or "convertible" for EC2 RIs. Absent + // on non-EC2 rows and on rows written before migration 000068. + // The "Sell on Marketplace" button only renders when this equals + // "standard" (issue #292). + offering_class?: string; + // ListingID is the AWS ReservedInstancesListingId. Non-empty when an + // active or recently-closed marketplace listing exists (issue #292). + listing_id?: string; + // ListingState is the AWS marketplace listing state: "active", + // "cancelled", or "closed". Empty when not listed. + listing_state?: string; } // Savings Analytics types diff --git a/internal/analytics/collector_test.go b/internal/analytics/collector_test.go index eeba52712..a988e7a87 100644 --- a/internal/analytics/collector_test.go +++ b/internal/analytics/collector_test.go @@ -430,6 +430,18 @@ func (m *mockConfigStore) UpsertRIUtilizationCache(_ context.Context, _ string, return nil } +func (m *mockConfigStore) UpdatePurchaseHistoryListing(_ context.Context, _, _, _ string) error { + return nil +} + +func (m *mockConfigStore) ClaimMarketplaceListingSlot(_ context.Context, _ string) (bool, error) { + return true, nil +} + +func (m *mockConfigStore) StampOfferingClass(_ context.Context, _, _ string) error { + return nil +} + // strPtr is a test helper for *string fields. func strPtr(s string) *string { return &s } diff --git a/internal/api/handler.go b/internal/api/handler.go index 41e673366..e415214e3 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -83,6 +83,11 @@ type Handler struct { // armreservations-backed client. azureExchangeFactory func(subscriptionID string) azureExchangeClient + // Optional marketplace EC2 client factory injected by tests. When nil + // (the production default), buildMarketplaceEC2Client uses + // awsprovider.NewEC2ClientDirect. + marketplaceEC2Factory func(aws.Config) marketplaceEC2Client + // Optional account-resolver injection point used by the reshape // handler integration test. When nil (the production default), the // handler calls h.resolveAWSCloudAccountID which in turn invokes diff --git a/internal/api/handler_marketplace.go b/internal/api/handler_marketplace.go new file mode 100644 index 000000000..fb0afd296 --- /dev/null +++ b/internal/api/handler_marketplace.go @@ -0,0 +1,628 @@ +package api + +// handler_marketplace.go implements the Sell-on-Marketplace flow for Standard +// Reserved Instances (issue #292). +// +// Endpoints: +// POST /api/purchases/{id}/marketplace-list create a listing +// POST /api/purchases/{id}/marketplace-cancel cancel an active listing +// +// Both endpoints require the caller to hold sell-any:purchases (admin or a +// custom operator group) or sell-own:purchases for rows they purchased +// themselves. The purchase_id in the URL path is the AWS ReservedInstancesId +// stamped into purchase_history.purchase_id at completion. + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "math" + "strings" + "time" + + "github.com/LeanerCloud/CUDly/internal/auth" + "github.com/LeanerCloud/CUDly/internal/config" + "github.com/LeanerCloud/CUDly/pkg/logging" + awsprovider "github.com/LeanerCloud/CUDly/providers/aws" + ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" + "github.com/aws/aws-lambda-go/events" + "github.com/aws/aws-sdk-go-v2/aws" + ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" + smithy "github.com/aws/smithy-go" + "github.com/google/uuid" +) + +// marketplaceEC2Client is the narrow EC2 interface the marketplace handlers +// need. Using a minimal interface keeps test stubs small. +type marketplaceEC2Client interface { + CreateMarketplaceListing(ctx context.Context, req ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) + DescribeMarketplaceListing(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error) + CancelMarketplaceListing(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error) + // FetchOfferingClass calls AWS DescribeReservedInstances to determine + // whether a given Reserved Instance is 'standard' or 'convertible'. Used + // when the purchase_history row has no offering_class stored (pre-migration + // 000087 rows and externally-created Standard RIs). + FetchOfferingClass(ctx context.Context, reservedInstancesID string) (string, error) +} + +// buildMarketplaceEC2Client honors the injected factory for tests, falling +// back to the direct AWS SDK constructor in production. +func (h *Handler) buildMarketplaceEC2Client(cfg aws.Config) marketplaceEC2Client { + if h.marketplaceEC2Factory != nil { + return h.marketplaceEC2Factory(cfg) + } + return awsprovider.NewEC2ClientDirect(cfg) +} + +// MarketplacePriceTier is the JSON-decodable shape accepted in the request body. +type MarketplacePriceTier struct { + // TermMonths is the remaining-months count this tier covers. + TermMonths int64 `json:"term_months"` + // Price is the USD list price per unit for this tier. + Price float64 `json:"price"` +} + +// MarketplaceListRequest is the request body for POST .../marketplace-list. +// PriceSchedule is optional: when absent the handler computes a default from +// the row's upfront_cost, monthly_cost, term, and a 5% discount to attract +// buyers (documented in the response body so the caller can see what was used). +type MarketplaceListRequest struct { + PriceSchedule []MarketplacePriceTier `json:"price_schedule,omitempty"` +} + +// MarketplaceListResponse is the JSON response body for a successful listing. +type MarketplaceListResponse struct { + ListingID string `json:"listing_id"` + ListingState string `json:"listing_state"` + PriceSchedule []MarketplacePriceTier `json:"price_schedule"` + AWSFeePercent float64 `json:"aws_fee_percent"` + Note string `json:"note,omitempty"` +} + +// validateMarketplaceListRequest performs the pre-flight checks shared by the +// listing flow: session auth, UUID validation, row lookup, offering_class / +// RBAC / duplicate-listing gates, and optional body decode. It returns the +// validated history row and the decoded request body. Extracted from +// marketplaceList to keep that handler's cyclomatic complexity in check. +func (h *Handler) validateMarketplaceListRequest(ctx context.Context, req *events.LambdaFunctionURLRequest, purchaseID string) (*config.PurchaseHistoryRecord, MarketplaceListRequest, error) { + var body MarketplaceListRequest + + session, err := h.requireSession(ctx, req) + if err != nil { + return nil, body, err + } + + err = validateUUID(purchaseID) + if err != nil { + return nil, body, err + } + + // Look up the purchase_history row to validate offering_class + get metadata. + row, err := h.config.GetPurchaseHistoryByPurchaseID(ctx, purchaseID) + if err != nil { + return nil, body, fmt.Errorf("failed to look up purchase: %w", err) + } + if row == nil { + return nil, body, NewClientError(404, "purchase not found") + } + + // Reject explicitly non-standard classes before the RBAC call (fast path). + // Empty offering_class is allowed here: the listing handler populates it + // lazily from AWS before performing the definitive check. + if isKnownNonStandardOfferingClass(row.OfferingClass) { + return nil, body, NewClientError(400, "only Standard Reserved Instances can be listed on the AWS Marketplace; this purchase has offering_class="+row.OfferingClass) + } + + // Enforce sell-any / sell-own RBAC. + if err := h.authorizeSessionSell(ctx, session, row.CloudAccountID); err != nil { + return nil, body, err + } + + // Reject if a listing is already active to avoid duplicate listings. + if strings.EqualFold(row.ListingState, config.ListingStateActive) { + return nil, body, NewClientError(409, fmt.Sprintf("an active marketplace listing %s already exists for this RI; cancel it first", row.ListingID)) + } + + // Decode optional body. + if req.Body != "" { + if err := json.Unmarshal([]byte(req.Body), &body); err != nil { + return nil, body, NewClientError(400, "invalid request body: "+err.Error()) + } + } + + return row, body, nil +} + +// marketplaceList handles POST /api/purchases/{id}/marketplace-list. +// The {id} must be the purchase_history.purchase_id (AWS ReservedInstancesId). +func (h *Handler) marketplaceList(ctx context.Context, req *events.LambdaFunctionURLRequest, purchaseID string) (any, error) { + row, body, err := h.validateMarketplaceListRequest(ctx, req, purchaseID) + if err != nil { + return nil, err + } + + // Compute actual remaining months from the purchase timestamp and total + // term so the default price schedule reflects real remaining value + // rather than the full contract term (which overprices older RIs). + remainingMonths := computeRemainingMonths(row.Timestamp, row.Term) + + // Validate and normalise the price schedule. Row-total UpfrontCost and the + // instance Count are passed so pricing is computed per instance; recurring + // (monthly) cost is deliberately excluded because the Marketplace buyer + // assumes recurring charges post-transfer (issue #292 money-path review). + schedule, err := resolveMarketplacePriceSchedule(body.PriceSchedule, remainingMonths, row.Term, row.Count, row.UpfrontCost) + if err != nil { + return nil, NewClientError(400, err.Error()) + } + + // Build per-provider AWS config for the region the RI lives in. + cfg, err := h.loadAWSConfigWithRegion(ctx, row.Region) + if err != nil { + return nil, fmt.Errorf("failed to load AWS config: %w", err) + } + + ec2Client := h.buildMarketplaceEC2Client(cfg) + + // Populate offering_class from AWS when the DB row has none. This covers + // two cases: (1) pre-migration 000087 rows purchased by CUDly before this + // column existed, and (2) externally-created Standard RIs whose + // offering_class was never stamped. After population the value is persisted + // so subsequent requests do not require an extra AWS API call. + offeringClass := row.OfferingClass + if offeringClass == "" { + offeringClass, err = h.populateOfferingClass(ctx, purchaseID, ec2Client) + if err != nil { + return nil, err + } + } + // Definitive offering_class gate: only Standard RIs can be listed. + if !strings.EqualFold(offeringClass, string(ec2types.OfferingClassTypeStandard)) { + return nil, NewClientError(400, "only Standard Reserved Instances can be listed on the AWS Marketplace; this purchase has offering_class="+offeringClass) + } + + awsSchedule := make([]ec2svc.MarketplacePriceTier, 0, len(schedule)) + for _, t := range schedule { + awsSchedule = append(awsSchedule, ec2svc.MarketplacePriceTier{ + Term: t.TermMonths, + Price: t.Price, + }) + } + + result, err := h.reserveAndCreateListing(ctx, purchaseID, row, ec2Client, awsSchedule) + if err != nil { + return nil, err + } + + return &MarketplaceListResponse{ + ListingID: result.ListingID, + ListingState: result.State, + PriceSchedule: schedule, + AWSFeePercent: awsMarketplaceFeePercent, + Note: fmt.Sprintf("AWS charges a %d%% transaction fee on the listing proceeds. Net proceeds = ListingPrice * %.2f.", awsMarketplaceFeePercent, awsMarketplaceNetFactor), + }, nil +} + +// reserveAndCreateListing atomically claims the marketplace-listing slot for the +// row, creates the AWS listing, and persists it, releasing the claim on every +// failure path so a failed attempt never leaves the row stuck in the transient +// pending state. Extracted from marketplaceList to keep that handler under the +// gocyclo budget. Returns the persisted listing result on success. +func (h *Handler) reserveAndCreateListing(ctx context.Context, purchaseID string, row *config.PurchaseHistoryRecord, ec2Client marketplaceEC2Client, awsSchedule []ec2svc.MarketplacePriceTier) (ec2svc.MarketplaceListingResult, error) { + // List every RI in the row: a row of N Standard RIs must list all N, not a + // single unit (issue #292 multi-count fix). Floor at 1 for legacy rows that + // somehow recorded a non-positive count so a valid Standard RI still lists, + // and reject an implausibly large count rather than silently truncating it + // into int32 (which could list the wrong number of RIs on the money path). + instanceCount := int32(1) + switch { + case row.Count > math.MaxInt32: + return ec2svc.MarketplaceListingResult{}, NewClientError(500, fmt.Sprintf("purchase count %d exceeds the marketplace listing limit", row.Count)) + case row.Count > 1: + instanceCount = int32(row.Count) + } + + // Atomically reserve the listing slot before calling AWS. The read-only + // listing_state check in validateMarketplaceListRequest is not enough on its + // own: two concurrent requests can both pass it, and AWS assigns a fresh + // ClientToken per call so the provider will not dedup them, leaving two live + // listings for one RI. The claim serializes concurrent creates for the same + // purchase_id (issue #292 CR concurrent-create guard). + claimed, err := h.config.ClaimMarketplaceListingSlot(ctx, purchaseID) + if err != nil { + return ec2svc.MarketplaceListingResult{}, fmt.Errorf("failed to reserve marketplace listing slot: %w", err) + } + if !claimed { + return ec2svc.MarketplaceListingResult{}, NewClientError(409, "a marketplace listing is already active or in progress for this RI; cancel it first") + } + + result, err := ec2Client.CreateMarketplaceListing(ctx, ec2svc.MarketplaceListingRequest{ + ReservedInstancesID: purchaseID, + ClientToken: uuid.New().String(), + PriceSchedule: awsSchedule, + InstanceCount: instanceCount, + }) + if err != nil { + // AWS created nothing: release the claim so a retry is not blocked. + h.releaseMarketplaceClaim(ctx, purchaseID, row) + logging.Warnf("marketplace: CreateReservedInstancesListing for purchase %s failed: %v", purchaseID, err) + return ec2svc.MarketplaceListingResult{}, mapAWSMarketplaceError("AWS marketplace listing failed", err) + } + + // Persist the listing ID and state. On DB failure, attempt a compensating + // rollback (cancel the just-created listing) to avoid a desync where the + // user sees success but the listing is invisible in subsequent renders, then + // release the claim so the row does not stay stuck in the pending state. + if dbErr := h.config.UpdatePurchaseHistoryListing(ctx, purchaseID, result.ListingID, result.State); dbErr != nil { + logging.Errorf("marketplace: listing created (%s / %s) but DB update failed: %v; attempting rollback", result.ListingID, result.State, dbErr) + if _, rollbackErr := ec2Client.CancelMarketplaceListing(ctx, result.ListingID); rollbackErr != nil { + logging.Errorf("marketplace: rollback cancel for listing %s also failed: %v", result.ListingID, rollbackErr) + } else { + logging.Warnf("marketplace: listing %s rolled back (canceled) after DB failure", result.ListingID) + } + h.releaseMarketplaceClaim(ctx, purchaseID, row) + return ec2svc.MarketplaceListingResult{}, fmt.Errorf("listing created but could not be persisted; listing has been rolled back: %w", dbErr) + } + + return result, nil +} + +// populateOfferingClass calls AWS DescribeReservedInstances to determine the +// offering class for an RI whose purchase_history row lacks one, then persists +// the result so subsequent requests are served from the DB. Returns the class +// string on success; returns an error if AWS reports no such RI. +// +// The DB stamp is best-effort: a write failure is logged but does not block the +// listing request because the in-memory class value is authoritative for this +// request. +func (h *Handler) populateOfferingClass(ctx context.Context, purchaseID string, ec2Client marketplaceEC2Client) (string, error) { + class, err := ec2Client.FetchOfferingClass(ctx, purchaseID) + if err != nil { + // Route through mapAWSMarketplaceError so an AWS client fault (e.g. an + // InvalidReservedInstancesID.NotFound for an RI that has expired or been + // exchanged) surfaces as a 400, not a generic 500. Server-side AWS + // failures still map to 502. + return "", mapAWSMarketplaceError("offering_class not set and could not fetch from AWS", err) + } + if stampErr := h.config.StampOfferingClass(ctx, purchaseID, class); stampErr != nil { + logging.Errorf("marketplace: failed to persist offering_class %q for purchase %s: %v", class, purchaseID, stampErr) + } + return class, nil +} + +// releaseMarketplaceClaim restores a purchase_history row's listing fields to +// the state captured before ClaimMarketplaceListingSlot reserved the slot. It +// runs on every failure path after a successful claim so a failed listing +// attempt does not leave the row stuck in the transient pending state (which +// would block future list attempts). Best-effort: on error it logs loudly +// because the row may stay pending until the #292 status poller reconciles it. +func (h *Handler) releaseMarketplaceClaim(ctx context.Context, purchaseID string, row *config.PurchaseHistoryRecord) { + if err := h.config.UpdatePurchaseHistoryListing(ctx, purchaseID, row.ListingID, row.ListingState); err != nil { + logging.Errorf("marketplace: failed to release listing claim for purchase %s (row may be stuck in %q): %v", purchaseID, config.ListingStatePending, err) + } +} + +// marketplaceCancel handles POST /api/purchases/{id}/marketplace-cancel. +func (h *Handler) marketplaceCancel(ctx context.Context, req *events.LambdaFunctionURLRequest, purchaseID string) (any, error) { + session, err := h.requireSession(ctx, req) + if err != nil { + return nil, err + } + + err = validateUUID(purchaseID) + if err != nil { + return nil, err + } + + row, err := h.config.GetPurchaseHistoryByPurchaseID(ctx, purchaseID) + if err != nil { + return nil, fmt.Errorf("failed to look up purchase: %w", err) + } + if row == nil { + return nil, NewClientError(404, "purchase not found") + } + + err = h.authorizeSessionSell(ctx, session, row.CloudAccountID) + if err != nil { + return nil, err + } + + if !strings.EqualFold(row.ListingState, config.ListingStateActive) { + return nil, NewClientError(409, "no active listing found for this RI; current state: "+row.ListingState) + } + + cfg, err := h.loadAWSConfigWithRegion(ctx, row.Region) + if err != nil { + return nil, fmt.Errorf("failed to load AWS config: %w", err) + } + + ec2Client := h.buildMarketplaceEC2Client(cfg) + + result, err := ec2Client.CancelMarketplaceListing(ctx, row.ListingID) + if err != nil { + logging.Warnf("marketplace: CancelReservedInstancesListing for listing %s failed: %v", row.ListingID, err) + return nil, mapAWSMarketplaceError("AWS cancel listing failed", err) + } + + // The listing is already canceled in AWS; there is no compensating rollback + // available if the DB write fails. Return an internal error so the caller + // knows the state is out of sync and can retry or contact an administrator. + if dbErr := h.config.UpdatePurchaseHistoryListing(ctx, purchaseID, result.ListingID, result.State); dbErr != nil { + logging.Errorf("marketplace: listing canceled in AWS (%s) but DB update failed: %v; state is out of sync", result.ListingID, dbErr) + return nil, fmt.Errorf("listing canceled in AWS but could not be persisted: %w", dbErr) + } + + return map[string]string{"listing_id": result.ListingID, "listing_state": result.State}, nil +} + +// authorizeSessionSell returns nil when the session is permitted to perform a +// sell/marketplace action under the sell-any / sell-own RBAC rules. The +// cloudAccountID is the cloud account that owns the RI (used for sell-own to +// confirm the session's allowed accounts cover that account). Returns a 403 +// ClientError otherwise. +// +// sell-own semantics: a non-admin user can list/cancel RIs for cloud accounts +// they are permitted to access (allowed_accounts covers the account). This is +// intentionally looser than cancel-own (which checks the session UserID against +// created_by_user_id) because purchase_history rows lack a created_by_user_id. +func (h *Handler) authorizeSessionSell(ctx context.Context, session *Session, cloudAccountID *string) error { + if h.auth == nil { + return NewClientError(500, "authentication service not configured") + } + + // Admins are recognized by holding the full-access admin capability + // (auth migrated from role-based to group-membership-only, issue #907). + isAdmin, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionAdmin, auth.ResourceAll) + if err != nil { + return fmt.Errorf("permission check failed: %w", err) + } + if isAdmin { + return nil + } + + hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionSellAny, auth.ResourcePurchases) + if err != nil { + return fmt.Errorf("permission check failed: %w", err) + } + if hasAny { + return nil + } + + hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionSellOwn, auth.ResourcePurchases) + if err != nil { + return fmt.Errorf("permission check failed: %w", err) + } + if !hasOwn { + return NewClientError(403, "permission denied: requires sell-any or sell-own on purchases") + } + + // sell-own: verify the session covers the cloud account that holds the RI. + // When cloudAccountID is nil (ambient/legacy row), deny for non-admins. + if cloudAccountID == nil { + return NewClientError(403, "permission denied: cannot sell an RI from an ambiguous (non-per-account) purchase row without sell-any") + } + return h.authorizeAllowedAccount(ctx, session, *cloudAccountID) +} + +// authorizeAllowedAccount returns nil when the session's allowed_accounts +// permit access to the given cloud account UUID. Returns 403 otherwise. +func (h *Handler) authorizeAllowedAccount(ctx context.Context, session *Session, cloudAccountID string) error { + if h.auth != nil { + // Admins are recognized by holding the full-access admin capability + // (auth migrated from role-based to group-membership-only, issue #907). + isAdmin, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionAdmin, auth.ResourceAll) + if err != nil { + return fmt.Errorf("admin permission check failed: %w", err) + } + if isAdmin { + return nil + } + } + allowed, err := h.getAllowedAccounts(ctx, session) + if err != nil { + return fmt.Errorf("failed to check allowed accounts: %w", err) + } + // Empty list means "no restriction" (the user has access to all accounts). + if len(allowed) == 0 { + return nil + } + for _, id := range allowed { + if id == "*" || id == cloudAccountID { + return nil + } + } + return NewClientError(403, "permission denied: purchase is in a cloud account not covered by your session's allowed accounts") +} + +// computeRemainingMonths returns the number of whole months remaining on an RI +// given its purchase timestamp and total term in months. The result is floored +// at 1 so defensive callers always get a positive value. +func computeRemainingMonths(purchaseTime time.Time, termMonths int) int { + if purchaseTime.IsZero() || termMonths <= 0 { + return 1 + } + elapsed := time.Since(purchaseTime) + elapsedMonths := elapsed.Hours() / (24 * 30.4375) + remaining := float64(termMonths) - elapsedMonths + r := int(math.Floor(remaining)) + if r < 1 { + return 1 + } + return r +} + +// awsMarketplaceFeePercent is the AWS Marketplace transaction fee percentage +// charged against listing proceeds. Published at +// https://aws.amazon.com/marketplace/ri-marketplace/faq/ +const awsMarketplaceFeePercent = 12 + +// awsMarketplaceNetFactor is the fraction of list price the seller keeps +// after the AWS Marketplace fee: 1 - awsMarketplaceFeePercent/100. +// Derived from awsMarketplaceFeePercent so the two stay in sync automatically. +const awsMarketplaceNetFactor = 1 - awsMarketplaceFeePercent/100.0 + +// awsMarketplaceBuyerDiscountFactor is the discount applied to the computed +// residual RI value when building the default listing price. A 5% discount +// makes the listing attractive to buyers while still recovering most of the +// seller's remaining cost basis. Applied before AWS deducts its fee. +const awsMarketplaceBuyerDiscountFactor = 0.95 + +// awsMarketplaceMinPriceFloorFraction is the minimum ratio of prorated +// residual value that a caller-supplied price schedule must total across its +// tiers. At 5%, a schedule totalling less than 1/20 of what the default +// schedule would offer is rejected with a 400 so a user cannot accidentally +// (or maliciously) list an RI at $0 or a nominal amount. The default schedule +// uses awsMarketplaceBuyerDiscountFactor (95%) of residual; this floor is +// intentionally far below that to give sellers flexibility while preventing +// zero-price listings. +const awsMarketplaceMinPriceFloorFraction = 0.05 + +// awsMarketplaceClientFaultCodes is the set of AWS error codes that represent +// client-side faults for Marketplace listing operations. These map to 4xx +// responses so the caller receives an actionable message. Server-side AWS +// errors remain 5xx. +var awsMarketplaceClientFaultCodes = map[string]bool{ + "InvalidReservedInstancesId": true, + "InvalidReservedInstancesId.NotFound": true, + "InvalidParameterValue": true, + "InvalidParameter": true, + "IncorrectState": true, + "InvalidReservedInstancesListingId": true, + "ReservedInstancesListingAlreadyExists": true, + "SellerNotRegistered": true, + "AuthFailure": true, + "UnauthorizedOperation": true, +} + +// mapAWSMarketplaceError maps an AWS SDK error to an appropriate ClientError. +// AWS client-fault errors (4xx-category codes) produce a 4xx response with the +// original AWS message so the caller gets actionable feedback. All other errors +// produce a 502 (AWS-side failure). +func mapAWSMarketplaceError(opMsg string, err error) error { + var apiErr smithy.APIError + if errors.As(err, &apiErr) { + if awsMarketplaceClientFaultCodes[apiErr.ErrorCode()] || apiErr.ErrorFault() == smithy.FaultClient { + return NewClientError(400, apiErr.ErrorMessage()) + } + } + return NewClientError(502, opMsg+": "+err.Error()) +} + +// isKnownNonStandardOfferingClass reports true when offering_class is +// explicitly set to a non-standard value (e.g. "convertible"). An empty string +// returns false because the class is not yet known and will be resolved lazily. +func isKnownNonStandardOfferingClass(class string) bool { + return class != "" && !strings.EqualFold(class, string(ec2types.OfferingClassTypeStandard)) +} + +// marketplaceResidualPerUnit returns the prorated per-instance upfront residual +// of an RI: the still-unconsumed portion of the upfront cost, divided by the +// instance count. This is the seller's recoverable basis on the RI Marketplace. +// +// Two money-path invariants (issue #292 review) are baked in here so every +// caller shares them: +// - Per instance: purchase_history.UpfrontCost is the row total for all +// `count` instances, but Marketplace prices are per instance, so divide by +// count. A legacy/corrupt count <= 0 is treated as 1 (matching +// reserveAndCreateListing's floor-at-1) so a single unit still prices. +// - Upfront only: recurring (monthly) cost is excluded because the buyer +// assumes recurring charges after transfer -- the seller recovers only the +// upfront remainder. Including it would overprice partial-upfront RIs and +// make no-upfront RIs unpriceable. +// +// Returns 0 when the term or upfront basis is non-positive (a $0 no-upfront RI +// or a fully-elapsed term), signaling callers to skip the per-unit price floor. +func marketplaceResidualPerUnit(remainingMonths, originalTerm, count int, upfrontCost float64) float64 { + if remainingMonths <= 0 || originalTerm <= 0 || upfrontCost <= 0 { + return 0 + } + units := count + if units <= 0 { + units = 1 + } + upfrontRemaining := upfrontCost * (float64(remainingMonths) / float64(originalTerm)) + return upfrontRemaining / float64(units) +} + +// checkSuppliedScheduleFloor validates a caller-supplied price schedule against +// two money-path guardrails (issue #292): +// 1. no tier's TermMonths may exceed the RI's remaining months -- an +// out-of-range term would inflate the tier's implied value arbitrarily; and +// 2. each tier's one-time sale Price must be at least +// awsMarketplaceMinPriceFloorFraction of the per-unit residual prorated to +// that tier's term. +// +// AWS's PriceScheduleSpecification.Price is the ONE-TIME sale price a buyer pays +// when TermMonths remain (NOT a per-month rate), so the floor is applied to +// Price directly -- never Price*TermMonths, which would over-credit a cheap +// schedule up to ~term-fold and let a near-zero listing slip past the floor. +// When the per-unit residual is zero (a $0 no-upfront RI) the price floor is +// skipped, but the term-range check still applies. +func checkSuppliedScheduleFloor(tiers []MarketplacePriceTier, remainingMonths, originalTerm, count int, upfrontCost float64) error { + if remainingMonths <= 0 { + return nil + } + perUnit := marketplaceResidualPerUnit(remainingMonths, originalTerm, count, upfrontCost) + for i, t := range tiers { + if t.TermMonths > int64(remainingMonths) { + return fmt.Errorf( + "price_schedule[%d]: term_months (%d) exceeds the RI's remaining term (%d months)", + i, t.TermMonths, remainingMonths) + } + if perUnit <= 0 { + continue + } + proratedResidual := perUnit * (float64(t.TermMonths) / float64(remainingMonths)) + floor := proratedResidual * awsMarketplaceMinPriceFloorFraction + if t.Price < floor { + return fmt.Errorf( + "price_schedule[%d]: price (%.2f) is below the minimum floor (%.2f, %.0f%% of the %.2f per-unit residual for %d remaining months); raise the price or omit the schedule to use the default", + i, t.Price, floor, awsMarketplaceMinPriceFloorFraction*100, proratedResidual, t.TermMonths) + } + } + return nil +} + +// resolveMarketplacePriceSchedule returns a normalised price schedule for the +// given RI. A caller-supplied schedule is validated (positive term/price, the +// term-range and per-unit price-floor guardrails in checkSuppliedScheduleFloor) +// and returned unchanged. When the caller omitted the schedule (nil / empty) a +// single-tier default is computed from the per-unit upfront residual. +// +// remainingMonths must be the actual remaining months (from computeRemainingMonths, +// NOT the raw term, which would overprice older RIs). originalTerm is the full +// contract term in months. count is the row's instance count (pricing is per +// instance). upfrontCost is the row-total upfront paid across all instances. +func resolveMarketplacePriceSchedule(supplied []MarketplacePriceTier, remainingMonths, originalTerm, count int, upfrontCost float64) ([]MarketplacePriceTier, error) { + if len(supplied) > 0 { + for i, t := range supplied { + if t.TermMonths <= 0 { + return nil, fmt.Errorf("price_schedule[%d]: term_months must be a positive integer", i) + } + if t.Price <= 0 { + return nil, fmt.Errorf("price_schedule[%d]: price must be positive (received %.4f); use a non-zero listing price", i, t.Price) + } + } + if err := checkSuppliedScheduleFloor(supplied, remainingMonths, originalTerm, count, upfrontCost); err != nil { + return nil, err + } + return supplied, nil + } + + // Default: a single tier priced at the per-unit upfront residual discounted + // by awsMarketplaceBuyerDiscountFactor. Per instance (Count-divided) and + // upfront-only (recurring is the buyer's post-transfer obligation). The + // awsMarketplaceFeePercent% AWS fee is deducted by the Marketplace on top. + // The upfront cost is prorated by (remaining/original) so an older RI is not + // overpriced (a 12-month RI at month 6 retains only half its upfront value). + if remainingMonths <= 0 { + remainingMonths = 1 // defensive: should not happen for an active RI + } + listPrice := marketplaceResidualPerUnit(remainingMonths, originalTerm, count, upfrontCost) * awsMarketplaceBuyerDiscountFactor + if listPrice < 0 { + listPrice = 0 + } + return []MarketplacePriceTier{ + {TermMonths: int64(remainingMonths), Price: listPrice}, + }, nil +} diff --git a/internal/api/handler_marketplace_test.go b/internal/api/handler_marketplace_test.go new file mode 100644 index 000000000..c504b5ef0 --- /dev/null +++ b/internal/api/handler_marketplace_test.go @@ -0,0 +1,606 @@ +package api + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/LeanerCloud/CUDly/internal/auth" + "github.com/LeanerCloud/CUDly/internal/config" + ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" + "github.com/aws/aws-lambda-go/events" + "github.com/aws/aws-sdk-go-v2/aws" + smithy "github.com/aws/smithy-go" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// validMarketplacePurchaseID is a syntactically valid UUID accepted by +// validateUUID; the marketplace path keys on purchase_history.purchase_id. +const validMarketplacePurchaseID = "11111111-1111-1111-1111-111111111111" + +// stubMarketplaceEC2 is a configurable stub implementing marketplaceEC2Client. +// Each field, when set, overrides the corresponding method's behavior and +// records the last request so tests can assert on what was sent to AWS. +type stubMarketplaceEC2 struct { + createFn func(ctx context.Context, req ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) + cancelFn func(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error) + fetchClassFn func(ctx context.Context, reservedInstancesID string) (string, error) + + lastCreateReq ec2svc.MarketplaceListingRequest + createCallCount int + cancelCallCount int + lastCancelID string +} + +func (s *stubMarketplaceEC2) CreateMarketplaceListing(ctx context.Context, req ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) { + s.createCallCount++ + s.lastCreateReq = req + if s.createFn != nil { + return s.createFn(ctx, req) + } + return ec2svc.MarketplaceListingResult{ListingID: "ril-default", State: config.ListingStateActive}, nil +} + +func (s *stubMarketplaceEC2) DescribeMarketplaceListing(_ context.Context, listingID string) (ec2svc.MarketplaceListingResult, error) { + return ec2svc.MarketplaceListingResult{ListingID: listingID, State: config.ListingStateActive}, nil +} + +func (s *stubMarketplaceEC2) CancelMarketplaceListing(ctx context.Context, listingID string) (ec2svc.MarketplaceListingResult, error) { + s.cancelCallCount++ + s.lastCancelID = listingID + if s.cancelFn != nil { + return s.cancelFn(ctx, listingID) + } + return ec2svc.MarketplaceListingResult{ListingID: listingID, State: config.ListingStateCancelled}, nil +} + +func (s *stubMarketplaceEC2) FetchOfferingClass(ctx context.Context, reservedInstancesID string) (string, error) { + if s.fetchClassFn != nil { + return s.fetchClassFn(ctx, reservedInstancesID) + } + // Default: return "standard" so tests that set offering_class="" still proceed. + return "standard", nil +} + +// marketplaceTestAPIError is an smithy.APIError with a controllable fault so +// the error-mapping tests can exercise both the client- and server-fault paths. +type marketplaceTestAPIError struct { + code string + message string + fault smithy.ErrorFault +} + +func (e *marketplaceTestAPIError) Error() string { return e.code + ": " + e.message } +func (e *marketplaceTestAPIError) ErrorCode() string { return e.code } +func (e *marketplaceTestAPIError) ErrorMessage() string { return e.message } +func (e *marketplaceTestAPIError) ErrorFault() smithy.ErrorFault { return e.fault } + +// newMarketplaceHandler wires a Handler with the supplied mocks and stub EC2 +// client, pre-seeding the cached AWS config so loadAWSConfigWithRegion does not +// hit the real SDK. +func newMarketplaceHandler(cfgStore *MockConfigStore, authSvc *MockAuthService, ec2 *stubMarketplaceEC2) *Handler { + h := &Handler{ + config: cfgStore, + auth: authSvc, + marketplaceEC2Factory: func(_ aws.Config) marketplaceEC2Client { + return ec2 + }, + } + h.awsCfgOnce.Do(func() { h.awsCfg = aws.Config{Region: "us-east-1"} }) + return h +} + +func standardRow() *config.PurchaseHistoryRecord { + acct := "acct-1" + return &config.PurchaseHistoryRecord{ + PurchaseID: validMarketplacePurchaseID, + OfferingClass: "standard", + CloudAccountID: &acct, + Region: "us-east-1", + Count: 3, + Term: 12, + UpfrontCost: 1200, + Timestamp: time.Now(), + } +} + +func marketplaceReq() *events.LambdaFunctionURLRequest { + return &events.LambdaFunctionURLRequest{ + Headers: map[string]string{"authorization": "Bearer test-token"}, + } +} + +func adminSession(authSvc *MockAuthService) { + authSvc.On("ValidateSession", mock.Anything, "test-token"). + Return(&Session{UserID: "admin", Email: "admin@test.com"}, nil) + authSvc.On("HasPermissionAPI", mock.Anything, mock.Anything, auth.ActionAdmin, auth.ResourceAll). + Return(true, nil).Maybe() +} + +// --- Gap 3 handler tests (issue #292) --- + +func TestMarketplaceList_ConvertibleRejected(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + authSvc.On("ValidateSession", mock.Anything, "test-token"). + Return(&Session{UserID: "admin"}, nil) + + row := standardRow() + row.OfferingClass = "convertible" + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(row, nil) + + h := newMarketplaceHandler(cfgStore, authSvc, &stubMarketplaceEC2{}) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 400, ce.code) + assert.Contains(t, err.Error(), "Standard") + cfgStore.AssertExpectations(t) +} + +func TestMarketplaceList_SellOwnAllowed(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + ec2 := &stubMarketplaceEC2{} + + authSvc.On("ValidateSession", mock.Anything, "test-token"). + Return(&Session{UserID: "user-1"}, nil) + authSvc.On("HasPermissionAPI", mock.Anything, "user-1", auth.ActionAdmin, auth.ResourceAll). + Return(false, nil) + authSvc.On("HasPermissionAPI", mock.Anything, "user-1", auth.ActionSellAny, auth.ResourcePurchases). + Return(false, nil) + authSvc.On("HasPermissionAPI", mock.Anything, "user-1", auth.ActionSellOwn, auth.ResourcePurchases). + Return(true, nil) + // allowed accounts cover the row's cloud account. + authSvc.On("GetAllowedAccountsAPI", mock.Anything, "user-1"). + Return([]string{"acct-1"}, nil) + + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(true, nil) + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "ril-default", config.ListingStateActive). + Return(nil) + + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + resp, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.NoError(t, err) + typed, ok := resp.(*MarketplaceListResponse) + require.True(t, ok) + assert.Equal(t, "ril-default", typed.ListingID) + // multi-count: the row has Count=3, so the outbound request must list all 3. + assert.Equal(t, int32(3), ec2.lastCreateReq.InstanceCount) + cfgStore.AssertExpectations(t) + authSvc.AssertExpectations(t) +} + +func TestMarketplaceList_SellOwnDeniedWrongAccount(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + + authSvc.On("ValidateSession", mock.Anything, "test-token"). + Return(&Session{UserID: "user-1"}, nil) + authSvc.On("HasPermissionAPI", mock.Anything, "user-1", auth.ActionAdmin, auth.ResourceAll). + Return(false, nil) + authSvc.On("HasPermissionAPI", mock.Anything, "user-1", auth.ActionSellAny, auth.ResourcePurchases). + Return(false, nil) + authSvc.On("HasPermissionAPI", mock.Anything, "user-1", auth.ActionSellOwn, auth.ResourcePurchases). + Return(true, nil) + // allowed accounts do NOT include acct-1. + authSvc.On("GetAllowedAccountsAPI", mock.Anything, "user-1"). + Return([]string{"acct-other"}, nil) + + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + + h := newMarketplaceHandler(cfgStore, authSvc, &stubMarketplaceEC2{}) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 403, ce.code) + authSvc.AssertExpectations(t) +} + +func TestMarketplaceList_DuplicateActiveListing409(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + row := standardRow() + row.ListingState = config.ListingStateActive + row.ListingID = "ril-existing" + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(row, nil) + + h := newMarketplaceHandler(cfgStore, authSvc, &stubMarketplaceEC2{}) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 409, ce.code) + assert.Contains(t, err.Error(), "ril-existing") +} + +func TestMarketplaceList_AWSClientFaultMapsTo400(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(true, nil) + // AWS create fails, so the claim must be released back to the unlisted state. + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "", ""). + Return(nil) + + ec2 := &stubMarketplaceEC2{ + createFn: func(_ context.Context, _ ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) { + return ec2svc.MarketplaceListingResult{}, &marketplaceTestAPIError{ + code: "InvalidReservedInstancesId", message: "bad RI", fault: smithy.FaultClient, + } + }, + } + + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 400, ce.code) + assert.Contains(t, err.Error(), "bad RI") +} + +func TestMarketplaceList_AWSServerFaultMapsTo502(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(true, nil) + // AWS create fails, so the claim must be released back to the unlisted state. + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "", ""). + Return(nil) + + ec2 := &stubMarketplaceEC2{ + createFn: func(_ context.Context, _ ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) { + return ec2svc.MarketplaceListingResult{}, &marketplaceTestAPIError{ + code: "InternalError", message: "aws broke", fault: smithy.FaultServer, + } + }, + } + + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 502, ce.code) +} + +func TestMarketplaceList_UnknownErrorMapsTo502(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(true, nil) + // AWS create fails, so the claim must be released back to the unlisted state. + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "", ""). + Return(nil) + + ec2 := &stubMarketplaceEC2{ + createFn: func(_ context.Context, _ ec2svc.MarketplaceListingRequest) (ec2svc.MarketplaceListingResult, error) { + return ec2svc.MarketplaceListingResult{}, errors.New("network reset") + }, + } + + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 502, ce.code) +} + +func TestMarketplaceList_DBFailureCompensatingRollback(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(true, nil) + // DB persist fails after the listing was created. + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "ril-default", config.ListingStateActive). + Return(errors.New("db down")) + // After the compensating cancel, the claim is released back to unlisted so + // the row is not left stuck in the pending state. + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "", ""). + Return(nil) + + ec2 := &stubMarketplaceEC2{} + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + assert.Contains(t, err.Error(), "rolled back") + // The compensating cancel must have fired against the just-created listing. + assert.Equal(t, 1, ec2.cancelCallCount) + assert.Equal(t, "ril-default", ec2.lastCancelID) + cfgStore.AssertExpectations(t) +} + +func TestMarketplaceList_DefaultScheduleProrationMath(t *testing.T) { + // remaining=12, term=12, count=1, upfront=1200 => per-unit residual + // 1200*12/12/1 = 1200; default price = 1200*0.95 = 1140. + schedule, err := resolveMarketplacePriceSchedule(nil, 12, 12, 1, 1200) + require.NoError(t, err) + require.Len(t, schedule, 1) + assert.Equal(t, int64(12), schedule[0].TermMonths) + assert.InDelta(t, 1140.0, schedule[0].Price, 0.001) + + // Half the term elapsed, count=1: remaining=6, term=12, upfront=1200. + // per-unit residual = 1200*6/12/1 = 600; recurring is excluded (the buyer + // assumes it post-transfer); default price = 600*0.95 = 570. + schedule, err = resolveMarketplacePriceSchedule(nil, 6, 12, 1, 1200) + require.NoError(t, err) + require.Len(t, schedule, 1) + assert.Equal(t, int64(6), schedule[0].TermMonths) + assert.InDelta(t, 570.0, schedule[0].Price, 0.001) + + // Per-instance pricing: count=3 on the same $1200 row-total upfront divides + // the residual by the instance count, so the default per-unit price is a + // third of the count=1 price (1200/3=400 per-unit residual; 400*0.95=380), + // NOT the row-total (which would 3x-overprice each listed unit). + schedule, err = resolveMarketplacePriceSchedule(nil, 12, 12, 3, 1200) + require.NoError(t, err) + require.Len(t, schedule, 1) + assert.Equal(t, int64(12), schedule[0].TermMonths) + assert.InDelta(t, 380.0, schedule[0].Price, 0.001) +} + +func TestMarketplaceCancel_HappyPath(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + row := standardRow() + row.ListingState = config.ListingStateActive + row.ListingID = "ril-cancel" + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(row, nil) + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "ril-cancel", config.ListingStateCancelled). + Return(nil) + + ec2 := &stubMarketplaceEC2{} + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + resp, err := h.marketplaceCancel(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.NoError(t, err) + m, ok := resp.(map[string]string) + require.True(t, ok) + assert.Equal(t, config.ListingStateCancelled, m["listing_state"]) + assert.Equal(t, "ril-cancel", ec2.lastCancelID) + cfgStore.AssertExpectations(t) +} + +func TestMarketplaceCancel_NoActiveListing409(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + row := standardRow() // ListingState empty -> not active + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(row, nil) + + h := newMarketplaceHandler(cfgStore, authSvc, &stubMarketplaceEC2{}) + _, err := h.marketplaceCancel(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 409, ce.code) +} + +// TestMarketplaceList_ConcurrentClaimLoses409 reproduces the concurrent-create +// race (issue #292 CR): the pre-flight listing_state read passes, but a parallel +// request has already reserved the listing slot, so the atomic claim loses. The +// handler must return 409 and must NOT call AWS (creating a second live listing +// for one RI would be a real double-listing on the money path). This test fails +// on the pre-guard code, which called AWS unconditionally after the read-check. +func TestMarketplaceList_ConcurrentClaimLoses409(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + // A concurrent request already holds the slot: the atomic claim reports lost. + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(false, nil) + + ec2 := &stubMarketplaceEC2{} + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 409, ce.code) + // The guard must fire before AWS: no listing may be created when the claim loses. + assert.Equal(t, 0, ec2.createCallCount) + cfgStore.AssertExpectations(t) +} + +// TestMarketplaceList_ClaimErrorMapsToInternal verifies a claim DB error is +// surfaced (not swallowed) and blocks the AWS call, so a broken slot-reservation +// never silently proceeds to create an unguarded listing. +func TestMarketplaceList_ClaimErrorMapsToInternal(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(standardRow(), nil) + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(false, errors.New("db down")) + + ec2 := &stubMarketplaceEC2{} + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + assert.Contains(t, err.Error(), "reserve marketplace listing slot") + // A failed claim must not fall through to AWS. + assert.Equal(t, 0, ec2.createCallCount) + cfgStore.AssertExpectations(t) +} + +// --- Regression tests for the three blocking defects fixed in this PR --- + +// TestResolveMarketplacePriceSchedule_ZeroPriceRejected verifies Defect 3: +// Price=0 must be rejected with a 400 so a user cannot list an RI for free. +// This is the fail-before/pass-after regression test (the old code accepted +// Price >= 0, so Price=0 was accepted; the fix requires Price > 0). +func TestResolveMarketplacePriceSchedule_ZeroPriceRejected(t *testing.T) { + _, err := resolveMarketplacePriceSchedule([]MarketplacePriceTier{ + {TermMonths: 6, Price: 0}, + }, 6, 12, 1, 1200) + require.Error(t, err, "Price=0 must be rejected") + assert.Contains(t, err.Error(), "positive") +} + +// TestResolveMarketplacePriceSchedule_BelowFloorRejected verifies Defect A +// (money): the floor is enforced PER TIER on the one-time sale Price, not +// Price*TermMonths. A $5 one-time price on a $1,200 per-unit residual with 12 +// of 12 months remaining is 0.4% of residual and must be rejected (floor is 5% +// = $60). This FAILS on the pre-fix code, which multiplied Price*TermMonths +// (5*12 = $60) and compared against a $60 floor, letting it pass. +func TestResolveMarketplacePriceSchedule_BelowFloorRejected(t *testing.T) { + _, err := resolveMarketplacePriceSchedule([]MarketplacePriceTier{ + {TermMonths: 12, Price: 5.0}, + }, 12, 12, 1, 1200) + require.Error(t, err, "a $5 one-time price on a $1,200 residual must be rejected by the per-tier floor") + assert.Contains(t, err.Error(), "floor") + assert.Contains(t, err.Error(), "residual") +} + +// TestResolveMarketplacePriceSchedule_PerUnitFloor verifies Defect B (money): +// the floor basis is PER INSTANCE. On a $1,200 row-total upfront with count=3, +// the per-unit residual is $400, so the 5% floor is $20/unit. A $19 one-time +// price is rejected; a $21 price is accepted -- proving the floor divides the +// row total by count instead of applying the $1,200 row total per unit. +func TestResolveMarketplacePriceSchedule_PerUnitFloor(t *testing.T) { + _, errBelow := resolveMarketplacePriceSchedule([]MarketplacePriceTier{ + {TermMonths: 12, Price: 19.0}, + }, 12, 12, 3, 1200) + require.Error(t, errBelow, "$19 is below the $20 per-unit floor (5%% of $400 per-unit residual)") + assert.Contains(t, errBelow.Error(), "per-unit residual") + + schedule, errAbove := resolveMarketplacePriceSchedule([]MarketplacePriceTier{ + {TermMonths: 12, Price: 21.0}, + }, 12, 12, 3, 1200) + require.NoError(t, errAbove, "$21 clears the $20 per-unit floor") + require.Len(t, schedule, 1) + assert.InDelta(t, 21.0, schedule[0].Price, 0.001) +} + +// TestResolveMarketplacePriceSchedule_TermExceedsRemainingRejected verifies the +// Defect A term-range guardrail: a tier whose TermMonths exceeds the RI's +// remaining months is rejected outright (an out-of-range term would let a +// caller arbitrarily inflate the tier's implied value). remaining=6, tier +// term=12 -> rejected. +func TestResolveMarketplacePriceSchedule_TermExceedsRemainingRejected(t *testing.T) { + _, err := resolveMarketplacePriceSchedule([]MarketplacePriceTier{ + {TermMonths: 12, Price: 1000}, + }, 6, 12, 1, 1200) + require.Error(t, err, "term_months exceeding remaining term must be rejected") + assert.Contains(t, err.Error(), "exceeds the RI's remaining term") +} + +// TestMarketplaceList_EmptyOfferingClassFetchedStandard verifies Defect 2: +// When purchase_history.offering_class is empty (pre-migration rows and +// externally-created RIs), the handler fetches the class from AWS, persists +// it, and proceeds when AWS reports "standard". This is the sell path becoming +// reachable for externally-created Standard RIs. +func TestMarketplaceList_EmptyOfferingClassFetchedStandard(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + // Row has no offering_class (simulates pre-migration or externally-created RI). + row := standardRow() + row.OfferingClass = "" + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(row, nil) + // The handler must stamp the fetched class back to the DB. + cfgStore.On("StampOfferingClass", mock.Anything, validMarketplacePurchaseID, "standard"). + Return(nil) + cfgStore.On("ClaimMarketplaceListingSlot", mock.Anything, validMarketplacePurchaseID). + Return(true, nil) + cfgStore.On("UpdatePurchaseHistoryListing", mock.Anything, validMarketplacePurchaseID, "ril-default", config.ListingStateActive). + Return(nil) + + // AWS reports this RI is "standard". + ec2 := &stubMarketplaceEC2{ + fetchClassFn: func(_ context.Context, id string) (string, error) { + assert.Equal(t, validMarketplacePurchaseID, id) + return "standard", nil + }, + } + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + resp, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.NoError(t, err, "standard RI with empty offering_class must be listable after lazy-populate") + typed, ok := resp.(*MarketplaceListResponse) + require.True(t, ok) + assert.Equal(t, "ril-default", typed.ListingID) + cfgStore.AssertExpectations(t) +} + +// TestMarketplaceList_EmptyOfferingClassFetchedConvertible verifies Defect 2 +// guard: when AWS reports the RI is "convertible", the handler must reject +// with 400 even if offering_class was empty in the DB (not silently proceed). +func TestMarketplaceList_EmptyOfferingClassFetchedConvertible(t *testing.T) { + cfgStore := &MockConfigStore{} + authSvc := &MockAuthService{} + adminSession(authSvc) + + row := standardRow() + row.OfferingClass = "" + cfgStore.On("GetPurchaseHistoryByPurchaseID", mock.Anything, validMarketplacePurchaseID). + Return(row, nil) + // The handler stamps the fetched class back even when it is convertible. + cfgStore.On("StampOfferingClass", mock.Anything, validMarketplacePurchaseID, "convertible"). + Return(nil) + + ec2 := &stubMarketplaceEC2{ + fetchClassFn: func(_ context.Context, _ string) (string, error) { + return "convertible", nil + }, + } + h := newMarketplaceHandler(cfgStore, authSvc, ec2) + _, err := h.marketplaceList(context.Background(), marketplaceReq(), validMarketplacePurchaseID) + + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 400, ce.code) + assert.Contains(t, err.Error(), "Standard") + cfgStore.AssertExpectations(t) +} diff --git a/internal/api/router.go b/internal/api/router.go index 6eab313bd..81e5c5ffc 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -188,6 +188,12 @@ func (r *Router) registerRoutes() { // expected_refund_amount in the POST /revoke body. {PathPrefix: "/api/purchases/", PathSuffix: "/revoke/calculate", Method: "GET", Handler: r.calculateRevokeHandler, Auth: AuthUser}, + // RI Marketplace listing (issue #292). Session-authed only; the + // handler enforces the sell-any/sell-own RBAC matrix and checks + // that the purchase_history row has offering_class='standard'. + {PathPrefix: "/api/purchases/", PathSuffix: "/marketplace-list", Method: "POST", Handler: r.marketplaceListHandler, Auth: AuthUser}, + {PathPrefix: "/api/purchases/", PathSuffix: "/marketplace-cancel", Method: "POST", Handler: r.marketplaceCancelHandler, Auth: AuthUser}, + // Planned purchases endpoints (must come before generic /api/purchases/{id}). // All now AuthUser (PR-A of #660): handler-level requirePermission // is the actual gate for pause/resume/run/delete. @@ -595,6 +601,14 @@ func (r *Router) calculateRevokeHandler(ctx context.Context, req *events.LambdaF return r.h.calculateAzureRevoke(ctx, req, params["id"]) } +func (r *Router) marketplaceListHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + return r.h.marketplaceList(ctx, req, params["id"]) +} + +func (r *Router) marketplaceCancelHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { + return r.h.marketplaceCancel(ctx, req, params["id"]) +} + func (r *Router) getPlannedPurchasesHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { return r.h.getPlannedPurchases(ctx, req) } diff --git a/internal/auth/service_group_test.go b/internal/auth/service_group_test.go index 38a0126bc..7eeae6715 100644 --- a/internal/auth/service_group_test.go +++ b/internal/auth/service_group_test.go @@ -279,13 +279,14 @@ func TestService_GetUserPermissions(t *testing.T) { permissions, err := service.GetUserPermissions(ctx, "user-123") require.NoError(t, err) - // 11 = 6 read/plan-author + delete:plans (PR-A #660) + // 12 = 6 read/plan-author + delete:plans (PR-A #660) // + update:purchases (PR-A #660) // + cancel-own:purchases (issue #46) // + retry-own:purchases (issue #47) - // + revoke-own:purchases (issue #290). + // + revoke-own:purchases (issue #290) + // + sell-own:purchases (issue #292). // NOTE: approve-own removed (issue #1407, four-eyes). - assert.Len(t, permissions, 11) + assert.Len(t, permissions, 12) mockStore.AssertExpectations(t) }) @@ -354,11 +355,11 @@ func TestService_GetUserPermissions(t *testing.T) { permissions, err := service.GetUserPermissions(ctx, "user-123") require.NoError(t, err) - // 11 standard-group (incl. delete:plans (PR-A #660) + update:purchases (PR-A #660) + // 12 standard-group (incl. delete:plans (PR-A #660) + update:purchases (PR-A #660) // + cancel-own (#46) + retry-own (#47) - // + revoke-own (#290):purchases; approve-own removed per #1407 four-eyes) - // + 1 group1 + 1 group2 = 13 - assert.Len(t, permissions, 13) + // + revoke-own (#290) + sell-own (#292):purchases; approve-own removed + // per #1407 four-eyes) + 1 group1 + 1 group2 = 14 + assert.Len(t, permissions, 14) mockStore.AssertExpectations(t) }) @@ -401,13 +402,14 @@ func TestService_GetUserPermissions(t *testing.T) { require.NoError(t, err) // Should have only the resolvable group's permissions; the missing // group is skipped. - // 11 = 6 read/plan-author + delete:plans (PR-A #660) + // 12 = 6 read/plan-author + delete:plans (PR-A #660) // + update:purchases (PR-A #660) // + cancel-own:purchases (issue #46) // + retry-own:purchases (issue #47) - // + revoke-own:purchases (issue #290). + // + revoke-own:purchases (issue #290) + // + sell-own:purchases (issue #292). // NOTE: approve-own removed (issue #1407, four-eyes). - assert.Len(t, permissions, 11) + assert.Len(t, permissions, 12) mockStore.AssertExpectations(t) }) diff --git a/internal/auth/types.go b/internal/auth/types.go index 7fc9451b9..dbe5d0026 100644 --- a/internal/auth/types.go +++ b/internal/auth/types.go @@ -461,6 +461,20 @@ const ( // escalating to admin. ActionRevokeOwn = "revoke-own" ActionRevokeAny = "revoke-any" + // ActionSellOwn / ActionSellAny gate the "Sell on Marketplace" button on + // Standard RI purchase history rows (issue #292). Mirror image of the + // cancel-{own,any} / retry-{own,any} / approve-{own,any} family: + // + // * RoleAdmin — implicit via {ActionAdmin, ResourceAll}; covers + // both verbs. + // * RoleUser — DefaultUserPermissions() adds sell-own:purchases so + // every user can list RIs they purchased themselves. + // * RoleReadOnly — neither verb. + // + // sell-any has no default non-admin grant; the constant exists so a + // custom operator group can list any Standard RI without admin escalation. + ActionSellOwn = "sell-own" + ActionSellAny = "sell-any" ) // Predefined resources. @@ -540,6 +554,12 @@ func DefaultUserPermissions() []Permission { // creator UUID matches. Legacy rows with NULL creator are out of reach // for non-admins (email-token paths have no revocation escape hatch). {Action: ActionRevokeOwn, Resource: ResourcePurchases}, + // sell-own:purchases — every authenticated user can list Standard + // RIs they purchased themselves on the AWS Marketplace (issue #292). + // The handler still requires the purchase_history row to carry + // offering_class = 'standard' and the cloud account to match the + // row's cloud_account_id before creating the listing. + {Action: ActionSellOwn, Resource: ResourcePurchases}, } } diff --git a/internal/auth/types_test.go b/internal/auth/types_test.go index c6ed9ebe0..7c222c6ca 100644 --- a/internal/auth/types_test.go +++ b/internal/auth/types_test.go @@ -21,8 +21,9 @@ func TestDefaultPermissions(t *testing.T) { // + cancel-own:purchases (issue #46) // + retry-own:purchases (issue #47) // + revoke-own:purchases (issue #290) - // NOTE: approve-own was removed (issue #1407, four-eyes) = 11. - assert.Len(t, perms, 11) + // + sell-own:purchases (issue #292) + // NOTE: approve-own was removed (issue #1407, four-eyes) = 12. + assert.Len(t, perms, 12) actions := make(map[string]bool) for _, p := range perms { @@ -44,6 +45,8 @@ func TestDefaultPermissions(t *testing.T) { // user permission. Self-approval requires an explicit custom-group grant. assert.False(t, actions[ActionApproveOwn+":"+ResourcePurchases], "approve-own must not be in DefaultUserPermissions (four-eyes, issue #1407)") + // sell-own:purchases must be a default user permission (issue #292). + assert.True(t, actions[ActionSellOwn+":"+ResourcePurchases]) }) t.Run("DefaultReadOnlyPermissions returns readonly access", func(t *testing.T) { diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go index 992c7f995..f2f726336 100644 --- a/internal/config/interfaces.go +++ b/internal/config/interfaces.go @@ -172,9 +172,11 @@ type StoreInterface interface { // across providers (aws/123 vs azure/123) from leaking the wrong rows. GetPurchaseHistoryFiltered(ctx context.Context, filter PurchaseHistoryFilter) ([]PurchaseHistoryRecord, error) // GetPurchaseHistoryByPurchaseID returns the single purchase_history row - // whose purchase_id matches. Returns (nil, nil) when no row is found. - // Used by the revoke endpoint to load the record before calling the - // provider cancel API (issue #290). + // whose purchase_id matches (AWS ReservedInstancesId / Azure reservation + // ID). Returns (nil, nil) when no row is found. Used by the revoke + // endpoint to load the record before calling the provider cancel API + // (issue #290) and by the marketplace-list handler to validate + // offering_class and look up the cloud account (issue #292). GetPurchaseHistoryByPurchaseID(ctx context.Context, purchaseID string) (*PurchaseHistoryRecord, error) // MarkPurchaseRevoked stamps revoked_at, revoked_via, and optionally // support_case_id on a purchase_history row identified by purchase_id. @@ -209,6 +211,32 @@ type StoreInterface interface { // finalize_revocations scheduled sweep calls this to retry the DB write. GetPurchaseHistoryInFlight(ctx context.Context) ([]*PurchaseHistoryRecord, error) + // UpdatePurchaseHistoryListing stamps the AWS marketplace listing_id and + // listing_state onto a purchase_history row. Called after + // CreateReservedInstancesListing succeeds (listing_state="active") and + // on subsequent poll/cancel transitions (issue #292). + UpdatePurchaseHistoryListing(ctx context.Context, purchaseID, listingID, listingState string) error + + // StampOfferingClass writes the offering_class value to a purchase_history + // row identified by purchase_id. Called by the marketplace-list handler + // when offering_class is absent in the DB (pre-migration 000087 rows and + // externally-created Standard RIs): after fetching the class from AWS + // DescribeReservedInstances it is persisted so subsequent requests do not + // incur an extra AWS API call. + StampOfferingClass(ctx context.Context, purchaseID, offeringClass string) error + + // ClaimMarketplaceListingSlot atomically reserves the marketplace-listing + // slot for a purchase_history row so two concurrent marketplace-list + // requests cannot both proceed to create a duplicate AWS listing (issue + // #292). It transitions listing_state to ListingStatePending only when the + // row is not already listed or mid-listing, and reports whether this call + // won the claim: (true, nil) means the caller reserved the slot and must + // then persist the real listing on success or release the slot back to its + // prior state on failure; (false, nil) means another request already holds + // an active or pending listing (the caller maps this to a 409). Modeled on + // FlipPurchaseRevocationInFlight. + ClaimMarketplaceListingSlot(ctx context.Context, purchaseID string) (bool, error) + // RI Exchange history SaveRIExchangeRecord(ctx context.Context, record *RIExchangeRecord) error GetRIExchangeRecord(ctx context.Context, id string) (*RIExchangeRecord, error) diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 2065a1d04..cff7b69c2 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -1638,8 +1638,8 @@ func (s *PostgresStore) SavePurchaseHistory(ctx context.Context, record *Purchas account_id, purchase_id, timestamp, provider, service, region, resource_type, count, term, payment, upfront_cost, monthly_cost, estimated_savings, plan_id, plan_name, ramp_step, cloud_account_id, - source, revocation_window_closes_at - ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19) + source, revocation_window_closes_at, offering_class + ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20) ` _, err := s.db.Exec(ctx, query, @@ -1662,6 +1662,7 @@ func (s *PostgresStore) SavePurchaseHistory(ctx context.Context, record *Purchas record.CloudAccountID, record.Source, record.RevocationWindowClosesAt, + nullStringFromString(record.OfferingClass), ) if err != nil { @@ -1671,13 +1672,71 @@ func (s *PostgresStore) SavePurchaseHistory(ctx context.Context, record *Purchas return nil } +// UpdatePurchaseHistoryListing stamps the marketplace listing fields onto a +// purchase_history row identified by its purchase_id (RI reservation ID). +// Called by the marketplace-list handler after CreateReservedInstancesListing +// succeeds (listing_id + listing_state="active") and by the status-poll path on +// the later closed/canceled transitions. +func (s *PostgresStore) UpdatePurchaseHistoryListing(ctx context.Context, purchaseID, listingID, listingState string) error { + query := ` + UPDATE purchase_history + SET listing_id = $1, listing_state = $2 + WHERE purchase_id = $3 + ` + tag, err := s.db.Exec(ctx, query, listingID, listingState, purchaseID) + if err != nil { + return fmt.Errorf("failed to update listing state for purchase %s: %w", purchaseID, err) + } + if tag.RowsAffected() == 0 { + return fmt.Errorf("purchase %s not found in history", purchaseID) + } + return nil +} + +// StampOfferingClass writes the offering_class value onto a purchase_history +// row identified by purchase_id. Used by the marketplace-list handler to +// lazily persist offering_class fetched from AWS DescribeReservedInstances for +// rows created before migration 000087 or for externally-created Standard RIs. +// A no-match (row not found) is treated as a non-fatal warning by callers. +func (s *PostgresStore) StampOfferingClass(ctx context.Context, purchaseID, offeringClass string) error { + query := `UPDATE purchase_history SET offering_class = $1 WHERE purchase_id = $2` + _, err := s.db.Exec(ctx, query, offeringClass, purchaseID) + if err != nil { + return fmt.Errorf("failed to stamp offering_class for purchase %s: %w", purchaseID, err) + } + return nil +} + +// ClaimMarketplaceListingSlot atomically reserves the marketplace-listing slot +// for a purchase_history row so two concurrent marketplace-list requests cannot +// both proceed to create a duplicate AWS listing (issue #292). The single +// conditional UPDATE transitions listing_state to ListingStatePending only when +// the row is not already active or pending, so exactly one racing request wins. +// It returns (true, nil) when this call reserved the slot and (false, nil) when +// the row is already active/pending (or absent); the caller maps false to a 409. +func (s *PostgresStore) ClaimMarketplaceListingSlot(ctx context.Context, purchaseID string) (bool, error) { + query := ` + UPDATE purchase_history + SET listing_state = $1 + WHERE purchase_id = $2 + AND lower(COALESCE(listing_state, '')) NOT IN ($3, $4) + ` + tag, err := s.db.Exec(ctx, query, + ListingStatePending, purchaseID, ListingStateActive, ListingStatePending) + if err != nil { + return false, fmt.Errorf("failed to claim marketplace listing slot for purchase %s: %w", purchaseID, err) + } + return tag.RowsAffected() == 1, nil +} + // GetPurchaseHistory retrieves purchase history for an account. func (s *PostgresStore) GetPurchaseHistory(ctx context.Context, accountID string, limit int) ([]PurchaseHistoryRecord, error) { query := ` SELECT account_id, purchase_id, timestamp, provider, service, region, resource_type, count, term, payment, upfront_cost, monthly_cost, estimated_savings, plan_id, plan_name, ramp_step, cloud_account_id, - revocation_window_closes_at, revoked_at, revoked_via, support_case_id + revocation_window_closes_at, revoked_at, revoked_via, support_case_id, + offering_class, listing_id, listing_state FROM purchase_history WHERE account_id = $1 ORDER BY timestamp DESC @@ -1693,7 +1752,8 @@ func (s *PostgresStore) GetAllPurchaseHistory(ctx context.Context, limit int) ([ SELECT account_id, purchase_id, timestamp, provider, service, region, resource_type, count, term, payment, upfront_cost, monthly_cost, estimated_savings, plan_id, plan_name, ramp_step, cloud_account_id, - revocation_window_closes_at, revoked_at, revoked_via, support_case_id + revocation_window_closes_at, revoked_at, revoked_via, support_case_id, + offering_class, listing_id, listing_state FROM purchase_history ORDER BY timestamp DESC LIMIT $1 @@ -1868,7 +1928,8 @@ func (s *PostgresStore) GetPurchaseHistoryFiltered( SELECT account_id, purchase_id, timestamp, provider, service, region, resource_type, count, term, payment, upfront_cost, monthly_cost, estimated_savings, plan_id, plan_name, ramp_step, cloud_account_id, - revocation_window_closes_at, revoked_at, revoked_via, support_case_id + revocation_window_closes_at, revoked_at, revoked_via, support_case_id, + offering_class, listing_id, listing_state FROM purchase_history%s ORDER BY timestamp DESC LIMIT $%d @@ -1883,10 +1944,13 @@ func (s *PostgresStore) GetPurchaseHistoryFiltered( // account_id, purchase_id, timestamp, provider, service, region, // resource_type, count, term, payment, upfront_cost, monthly_cost, // estimated_savings, plan_id, plan_name, ramp_step, cloud_account_id, -// revocation_window_closes_at, revoked_at, revoked_via, support_case_id +// revocation_window_closes_at, revoked_at, revoked_via, support_case_id, +// offering_class, listing_id, listing_state // -// The revocation columns were added in migration 000057. Queries must -// include them explicitly so the Scan targets stay in sync. +// The revocation columns were added in migration 000057; the marketplace +// listing columns (offering_class, listing_id, listing_state) in the #292 +// migration. Queries must include them explicitly so the Scan targets stay +// in sync. func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string, args ...any) ([]PurchaseHistoryRecord, error) { rows, err := s.db.Query(ctx, query, args...) if err != nil { @@ -1896,69 +1960,113 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string, records := make([]PurchaseHistoryRecord, 0) for rows.Next() { - var record PurchaseHistoryRecord - var planID, planName, cloudAccountID sql.NullString - var monthlyCost sql.NullFloat64 - var revocationWindowClosesAt, revokedAt *time.Time - var revokedVia, supportCaseID sql.NullString - - err := rows.Scan( - &record.AccountID, - &record.PurchaseID, - &record.Timestamp, - &record.Provider, - &record.Service, - &record.Region, - &record.ResourceType, - &record.Count, - &record.Term, - &record.Payment, - &record.UpfrontCost, - &monthlyCost, - &record.EstimatedSavings, - &planID, - &planName, - &record.RampStep, - &cloudAccountID, - &revocationWindowClosesAt, - &revokedAt, - &revokedVia, - &supportCaseID, - ) + record, err := scanPurchaseHistoryRow(rows) if err != nil { - return nil, fmt.Errorf("failed to scan purchase history: %w", err) + return nil, err } + records = append(records, record) + } - // Handle nullable monthly_cost: nil means "provider did not return a - // monthly breakdown"; 0.0 means "explicitly $0 recurring charge". - if monthlyCost.Valid { - v := monthlyCost.Float64 - record.MonthlyCost = &v - } + return records, rows.Err() +} - // Handle nullable strings - if planID.Valid { - record.PlanID = planID.String - } - if planName.Valid { - record.PlanName = planName.String - } - if cloudAccountID.Valid { - record.CloudAccountID = &cloudAccountID.String - } - record.RevocationWindowClosesAt = revocationWindowClosesAt - record.RevokedAt = revokedAt - if revokedVia.Valid { - record.RevokedVia = revokedVia.String - } - if supportCaseID.Valid { - record.SupportCaseID = supportCaseID.String - } +// scanPurchaseHistoryRow scans a single purchase_history row and reconciles the +// nullable columns into a PurchaseHistoryRecord. It is pulled out of +// queryPurchaseHistory to keep that function under the cyclomatic limit. The +// column order must match the SELECT lists in GetPurchaseHistory / +// GetAllPurchaseHistory / GetPurchaseHistoryFiltered (base columns, then the +// revocation columns from issue #290, then the marketplace columns from #292). +// purchaseHistoryNullables holds the nullable scan targets shared by every +// purchase_history reader (scanPurchaseHistoryRow, GetPurchaseHistoryByPurchaseID). +// Centralizing the NULL reconciliation in applyTo keeps each reader's +// cyclomatic complexity under the gocyclo budget as columns accrue across +// issues #290 (revocation) and #292 (marketplace). +type purchaseHistoryNullables struct { + monthlyCost sql.NullFloat64 + planID sql.NullString + planName sql.NullString + cloudAccountID sql.NullString + revocationWindowClosesAt *time.Time + revokedAt *time.Time + revokedVia sql.NullString + supportCaseID sql.NullString + offeringClass sql.NullString + listingID sql.NullString + listingState sql.NullString +} + +// applyTo reconciles the nullable columns into record. monthly_cost is left as +// a nil pointer when the provider returned no monthly breakdown (distinct from +// an explicit $0 recurring charge); the revocation-window timestamps map +// directly onto their pointer fields. +func (n *purchaseHistoryNullables) applyTo(record *PurchaseHistoryRecord) { + if n.monthlyCost.Valid { + v := n.monthlyCost.Float64 + record.MonthlyCost = &v + } + if n.planID.Valid { + record.PlanID = n.planID.String + } + if n.planName.Valid { + record.PlanName = n.planName.String + } + if n.cloudAccountID.Valid { + record.CloudAccountID = &n.cloudAccountID.String + } + record.RevocationWindowClosesAt = n.revocationWindowClosesAt + record.RevokedAt = n.revokedAt + if n.revokedVia.Valid { + record.RevokedVia = n.revokedVia.String + } + if n.supportCaseID.Valid { + record.SupportCaseID = n.supportCaseID.String + } + if n.offeringClass.Valid { + record.OfferingClass = n.offeringClass.String + } + if n.listingID.Valid { + record.ListingID = n.listingID.String + } + if n.listingState.Valid { + record.ListingState = n.listingState.String + } +} + +func scanPurchaseHistoryRow(rows pgx.Rows) (PurchaseHistoryRecord, error) { + var record PurchaseHistoryRecord + var n purchaseHistoryNullables - records = append(records, record) + if err := rows.Scan( + &record.AccountID, + &record.PurchaseID, + &record.Timestamp, + &record.Provider, + &record.Service, + &record.Region, + &record.ResourceType, + &record.Count, + &record.Term, + &record.Payment, + &record.UpfrontCost, + &n.monthlyCost, + &record.EstimatedSavings, + &n.planID, + &n.planName, + &record.RampStep, + &n.cloudAccountID, + &n.revocationWindowClosesAt, + &n.revokedAt, + &n.revokedVia, + &n.supportCaseID, + &n.offeringClass, + &n.listingID, + &n.listingState, + ); err != nil { + return PurchaseHistoryRecord{}, fmt.Errorf("failed to scan purchase history: %w", err) } - return records, rows.Err() + n.applyTo(&record) + return record, nil } // GetPurchaseHistoryByPurchaseID returns the single purchase_history row @@ -1966,14 +2074,16 @@ func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string, // does not exist. The revocation-window columns (revocation_window_closes_at, // revoked_at, revoked_via, support_case_id) are read alongside the base // columns so the revoke endpoint can check idempotency without a second round -// trip (issue #290). +// trip (issue #290). The marketplace columns (offering_class, listing_id, +// listing_state) are read so the marketplace-list handler can validate +// offering_class and look up the cloud account (issue #292). func (s *PostgresStore) GetPurchaseHistoryByPurchaseID(ctx context.Context, purchaseID string) (*PurchaseHistoryRecord, error) { query := ` SELECT account_id, purchase_id, timestamp, provider, service, region, resource_type, count, term, payment, upfront_cost, monthly_cost, estimated_savings, plan_id, plan_name, ramp_step, cloud_account_id, revocation_window_closes_at, revoked_at, revoked_via, support_case_id, - revocation_in_flight + revocation_in_flight, offering_class, listing_id, listing_state FROM purchase_history WHERE purchase_id = $1 LIMIT 1 @@ -1989,10 +2099,11 @@ func (s *PostgresStore) GetPurchaseHistoryByPurchaseID(ctx context.Context, purc } var r PurchaseHistoryRecord - var planID, planName, cloudAccountID sql.NullString - var revocationWindowClosesAt, revokedAt *time.Time - var revokedVia, supportCaseID sql.NullString + var n purchaseHistoryNullables + // Note the extra revocation_in_flight bool between support_case_id and the + // marketplace columns; it scans straight into r.RevocationInFlight rather + // than through the shared nullables struct. if err := rows.Scan( &r.AccountID, &r.PurchaseID, @@ -2007,36 +2118,25 @@ func (s *PostgresStore) GetPurchaseHistoryByPurchaseID(ctx context.Context, purc &r.UpfrontCost, &r.MonthlyCost, &r.EstimatedSavings, - &planID, - &planName, + &n.planID, + &n.planName, &r.RampStep, - &cloudAccountID, - &revocationWindowClosesAt, - &revokedAt, - &revokedVia, - &supportCaseID, + &n.cloudAccountID, + &n.revocationWindowClosesAt, + &n.revokedAt, + &n.revokedVia, + &n.supportCaseID, &r.RevocationInFlight, + &n.offeringClass, + &n.listingID, + &n.listingState, ); err != nil { return nil, fmt.Errorf("GetPurchaseHistoryByPurchaseID scan: %w", err) } - if planID.Valid { - r.PlanID = planID.String - } - if planName.Valid { - r.PlanName = planName.String - } - if cloudAccountID.Valid { - r.CloudAccountID = &cloudAccountID.String - } - r.RevocationWindowClosesAt = revocationWindowClosesAt - r.RevokedAt = revokedAt - if revokedVia.Valid { - r.RevokedVia = revokedVia.String - } - if supportCaseID.Valid { - r.SupportCaseID = supportCaseID.String - } + // MonthlyCost is scanned directly above (not through n.monthlyCost), so + // leave n.monthlyCost zero-valued; applyTo will not overwrite r.MonthlyCost. + n.applyTo(&r) return &r, rows.Err() } diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go index bd85724e5..e29285cd4 100644 --- a/internal/config/store_postgres_pgxmock_test.go +++ b/internal/config/store_postgres_pgxmock_test.go @@ -745,6 +745,8 @@ func TestPGXMock_GetPurchaseHistory_Success(t *testing.T) { "estimated_savings", "plan_id", "plan_name", "ramp_step", "cloud_account_id", // revocation columns (issue #290) "revocation_window_closes_at", "revoked_at", "revoked_via", "support_case_id", + // marketplace columns (issue #292) + "offering_class", "listing_id", "listing_state", } rows := pgxmock.NewRows(cols). AddRow("acc-1", "pur-1", now, "aws", "ec2", "us-east-1", @@ -752,11 +754,16 @@ func TestPGXMock_GetPurchaseHistory_Success(t *testing.T) { sql.NullString{Valid: true, String: "plan-1"}, sql.NullString{Valid: true, String: "My Plan"}, 1, sql.NullString{Valid: true, String: "cloud-acct-1"}, - nil, nil, sql.NullString{}, sql.NullString{}). + // revocation columns (issue #290) + nil, nil, sql.NullString{}, sql.NullString{}, + // marketplace columns (issue #292) + sql.NullString{Valid: true, String: "standard"}, + sql.NullString{}, sql.NullString{}). AddRow("acc-1", "pur-2", now, "aws", "rds", "us-west-2", "db.t3.medium", 1, 3, "all-upfront", 200.0, 0.0, 100.0, sql.NullString{}, sql.NullString{}, 0, sql.NullString{}, - nil, nil, sql.NullString{}, sql.NullString{}) + nil, nil, sql.NullString{}, sql.NullString{}, + sql.NullString{}, sql.NullString{}, sql.NullString{}) mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).WillReturnRows(rows) records, err := store.GetPurchaseHistory(ctx, "acc-1", 10) @@ -767,17 +774,96 @@ func TestPGXMock_GetPurchaseHistory_Success(t *testing.T) { assert.NoError(t, mock.ExpectationsWereMet()) } +// TestPGXMock_GetPurchaseHistoryByPurchaseID_Success asserts the DISTINCT +// 25-column scan order used by GetPurchaseHistoryByPurchaseID. Unlike the +// 24-column GetPurchaseHistory reader it adds revocation_in_flight (a plain +// bool, NOT a nullable) at position 22, between support_case_id and the +// marketplace columns (issue #290 Finding #6, migration 000072). A regression +// here -- e.g. dropping the column or scanning it through the nullables struct +// -- would shift every marketplace column by one and silently mis-read +// offering_class / listing_id / listing_state. +func TestPGXMock_GetPurchaseHistoryByPurchaseID_Success(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + ctx := context.Background() + + now := time.Now().Truncate(time.Second) + cols := []string{ + "account_id", "purchase_id", "timestamp", "provider", "service", "region", + "resource_type", "count", "term", "payment", "upfront_cost", "monthly_cost", + "estimated_savings", "plan_id", "plan_name", "ramp_step", "cloud_account_id", + // revocation columns (issue #290) + "revocation_window_closes_at", "revoked_at", "revoked_via", "support_case_id", + // partial-success reconciliation bool at position 22 (issue #290 Finding #6) + "revocation_in_flight", + // marketplace columns (issue #292) + "offering_class", "listing_id", "listing_state", + } + // monthly_cost scans straight into the *float64 r.MonthlyCost (not through + // the nullables struct), so the mock value must be a *float64. + monthly := 50.0 + rows := pgxmock.NewRows(cols). + AddRow("acc-1", "pur-1", now, "aws", "ec2", "us-east-1", + "m5.large", 2, 1, "no-upfront", 100.0, &monthly, 200.0, + sql.NullString{Valid: true, String: "plan-1"}, + sql.NullString{Valid: true, String: "My Plan"}, + 1, sql.NullString{Valid: true, String: "cloud-acct-1"}, + // revocation columns (issue #290) + nil, nil, sql.NullString{}, sql.NullString{}, + // revocation_in_flight bool at position 22 + true, + // marketplace columns (issue #292) + sql.NullString{Valid: true, String: "standard"}, + sql.NullString{Valid: true, String: "listing-1"}, + sql.NullString{Valid: true, String: "active"}) + mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg()).WillReturnRows(rows) + + record, err := store.GetPurchaseHistoryByPurchaseID(ctx, "pur-1") + require.NoError(t, err) + require.NotNil(t, record) + assert.Equal(t, "pur-1", record.PurchaseID) + assert.Equal(t, "plan-1", record.PlanID) + require.NotNil(t, record.MonthlyCost) + assert.Equal(t, 50.0, *record.MonthlyCost) + assert.True(t, record.RevocationInFlight) + // Marketplace columns must land in their own fields, not shifted by the + // extra bool. + assert.Equal(t, "standard", record.OfferingClass) + assert.Equal(t, "listing-1", record.ListingID) + assert.Equal(t, "active", record.ListingState) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +// TestPGXMock_GetPurchaseHistoryByPurchaseID_NotFound returns (nil, nil) when +// no row matches, so the revoke / marketplace handlers can distinguish "absent" +// from a scan/query error. +func TestPGXMock_GetPurchaseHistoryByPurchaseID_NotFound(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + ctx := context.Background() + + rows := pgxmock.NewRows([]string{"account_id"}) // no rows added + mock.ExpectQuery("SELECT").WithArgs(pgxmock.AnyArg()).WillReturnRows(rows) + + record, err := store.GetPurchaseHistoryByPurchaseID(ctx, "missing") + require.NoError(t, err) + assert.Nil(t, record) + assert.NoError(t, mock.ExpectationsWereMet()) +} + // ─── GetPurchaseHistoryFiltered (issue #701) ───────────────────────────────── // purchaseHistoryCols lists the SELECT columns for purchase_history rows in // the order GetPurchaseHistoryFiltered scans them. Keep in sync with // queryPurchaseHistory in store_postgres.go (issue #290 added the 4 revocation -// columns at positions 18-21). +// columns at positions 18-21; issue #292 added the 3 marketplace columns at +// positions 22-24). var purchaseHistoryCols = []string{ "account_id", "purchase_id", "timestamp", "provider", "service", "region", "resource_type", "count", "term", "payment", "upfront_cost", "monthly_cost", "estimated_savings", "plan_id", "plan_name", "ramp_step", "cloud_account_id", "revocation_window_closes_at", "revoked_at", "revoked_via", "support_case_id", + "offering_class", "listing_id", "listing_state", } // purchaseHistoryRow builds a single AddRow tuple matching purchaseHistoryCols. @@ -788,6 +874,8 @@ func purchaseHistoryRow(now time.Time, provider, acct string) []interface{} { sql.NullString{}, sql.NullString{}, 0, sql.NullString{}, // revocation columns (issue #290): all null for non-revoked rows nil, nil, sql.NullString{}, sql.NullString{}, + // marketplace columns (issue #292): all null for unlisted rows + sql.NullString{}, sql.NullString{}, sql.NullString{}, } } @@ -1973,8 +2061,9 @@ func TestPGXMock_SavePurchaseHistory_Success(t *testing.T) { store := storeWith(mock) ctx := context.Background() - // 19 columns: original 18 + revocation_window_closes_at (issue #290). - mock.ExpectExec("INSERT INTO purchase_history").WithArgs(anyArgsCfg(19)...). + // 20 columns: original 18 + revocation_window_closes_at (issue #290) + // + offering_class (issue #292). + mock.ExpectExec("INSERT INTO purchase_history").WithArgs(anyArgsCfg(20)...). WillReturnResult(pgxmock.NewResult("INSERT", 1)) err := store.SavePurchaseHistory(ctx, &PurchaseHistoryRecord{ diff --git a/internal/config/types.go b/internal/config/types.go index 75b13addc..29b8c7ddf 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -787,8 +787,35 @@ type PurchaseHistoryRecord struct { // can detect and retry the MarkPurchaseRevoked write without re-calling Azure // (preventing a duplicate-refund error). RevocationInFlight bool `json:"revocation_in_flight,omitempty" dynamodbav:"revocation_in_flight,omitempty"` + + // OfferingClass records whether this commitment is a 'standard' or + // 'convertible' RI. NULL on pre-migration rows. The Sell-on-Marketplace + // button renders only when this equals "standard" (issue #292). + // Persisted in purchase_history via migration 000087. + OfferingClass string `json:"offering_class,omitempty" dynamodbav:"offering_class,omitempty"` + // ListingID is the AWS ReservedInstancesListingId returned by + // CreateReservedInstancesListing. Empty when the RI has not been + // listed. Persisted in purchase_history via migration 000087. + ListingID string `json:"listing_id,omitempty" dynamodbav:"listing_id,omitempty"` + // ListingState mirrors the AWS marketplace listing state (see the + // ListingState* constants). Empty when not listed. Persisted in + // purchase_history via migration 000087. + ListingState string `json:"listing_state,omitempty" dynamodbav:"listing_state,omitempty"` } +// AWS EC2 ReservedInstancesListing status values, mirroring the +// ec2types.ListingStatus enum. Stored verbatim in purchase_history.listing_state +// so the strings must match AWS exactly. ListingStatePending is additionally +// written by the marketplace-list handler as a transient claim that guards +// against concurrent listing creation (issue #292); the other three come +// straight from AWS. +const ( + ListingStateActive = "active" + ListingStatePending = "pending" + ListingStateCancelled = "cancelled" //nolint:misspell // AWS ListingStatus enum literal, not prose + ListingStateClosed = "closed" +) + // RIExchangeRecord represents a record in the ri_exchange_history table. type RIExchangeRecord struct { ID string `json:"id"` diff --git a/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing.down.sql b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing.down.sql new file mode 100644 index 000000000..66e09baa2 --- /dev/null +++ b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing.down.sql @@ -0,0 +1,5 @@ +-- Revert migration 000087 +ALTER TABLE purchase_history + DROP COLUMN IF EXISTS offering_class, + DROP COLUMN IF EXISTS listing_id, + DROP COLUMN IF EXISTS listing_state; diff --git a/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing.up.sql b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing.up.sql new file mode 100644 index 000000000..5daee4c42 --- /dev/null +++ b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing.up.sql @@ -0,0 +1,25 @@ +-- Migration 000087: add RI Marketplace listing columns to purchase_history. +-- +-- offering_class: 'convertible' or 'standard'; NULL for pre-migration rows. +-- The Sell button renders only when offering_class = 'standard' and the RI is +-- active with remaining term. +-- +-- listing_id: the AWS ReservedInstancesListingId returned by +-- CreateReservedInstancesListing. NULL when not listed. +-- listing_state: mirrors the AWS listing state (active/cancelled/closed). NULL +-- when not listed. +-- +-- Only these three columns are added: they are the full set written and read by +-- the implemented list/cancel flow. The settlement/poller columns (listed_at, +-- listing_price_schedule, listing_proceeds_received, listing_fee_paid) were +-- descoped from this PR because the status poller that would populate them is +-- not implemented yet; they will land with the poller (see the #292 follow-up +-- issue) so the schema never carries columns nothing writes. +-- +-- All three columns are nullable and added with IF NOT EXISTS so the +-- migration is idempotent on re-apply. + +ALTER TABLE purchase_history + ADD COLUMN IF NOT EXISTS offering_class TEXT, + ADD COLUMN IF NOT EXISTS listing_id TEXT, + ADD COLUMN IF NOT EXISTS listing_state TEXT; diff --git a/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing_test.go b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing_test.go new file mode 100644 index 000000000..0aa2609dc --- /dev/null +++ b/internal/database/postgres/migrations/000087_purchase_history_marketplace_listing_test.go @@ -0,0 +1,66 @@ +//go:build integration +// +build integration + +package migrations_test + +import ( + "context" + "testing" + "time" + + "github.com/LeanerCloud/CUDly/internal/database/postgres/migrations" + "github.com/LeanerCloud/CUDly/internal/database/postgres/testhelpers" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMigration087_MarketplaceColumns verifies that migration 000087 creates +// the three new columns (offering_class, listing_id, listing_state) on the +// purchase_history table. The test migrates a fresh DB up through the full +// migration set and asserts that the columns are present and writable. +// +// Fail-before/pass-after: running this test against a DB at version 086 (without +// 000087 applied) would fail because the columns do not exist; after 000087 the +// test passes. +func TestMigration087_MarketplaceColumns(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + + container, err := testhelpers.SetupPostgresContainer(ctx, t) + if err != nil { + t.Skipf("Skipping test: could not setup postgres container: %v", err) + } + defer container.Cleanup(ctx) + + require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), getMigrationsPath(), "", ""), + "migrations must apply cleanly through 000087") + + // Verify all three columns exist by inserting a row that references them. + // We insert the NOT-NULL-without-default columns plus the three new + // marketplace columns; if any of the new columns is missing Postgres will + // error. plan_id is intentionally omitted (nullable UUID FK to + // purchase_plans) so the test needs no plan fixture. + const insertSQL = ` + INSERT INTO purchase_history ( + account_id, purchase_id, timestamp, provider, service, region, + resource_type, term, payment, + offering_class, listing_id, listing_state + ) VALUES ( + 'test-account', 'ri-087-test', $1, 'aws', 'ec2', 'us-east-1', + 't3.micro', 12, 'All Upfront', + 'standard', 'ril-087-test', 'active' + )` + + _, err = container.DB.Pool().Exec(ctx, insertSQL, time.Now()) + require.NoError(t, err, "INSERT referencing offering_class, listing_id, listing_state must succeed after migration 000087") + + // Verify the values round-trip correctly. + var offeringClass, listingID, listingState string + row := container.DB.Pool().QueryRow(ctx, ` + SELECT offering_class, listing_id, listing_state + FROM purchase_history WHERE purchase_id = 'ri-087-test'`) + require.NoError(t, row.Scan(&offeringClass, &listingID, &listingState)) + assert.Equal(t, "standard", offeringClass, "offering_class must round-trip") + assert.Equal(t, "ril-087-test", listingID, "listing_id must round-trip") + assert.Equal(t, "active", listingState, "listing_state must round-trip") +} diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index 90aef78c2..ed446b09a 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -389,7 +389,7 @@ func (m *MockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, filter return v, args.Error(1) } -// GetPurchaseHistoryByPurchaseID mocks the GetPurchaseHistoryByPurchaseID operation (issue #290). +// GetPurchaseHistoryByPurchaseID mocks the single-row lookup by purchase_id (issues #290, #292). func (m *MockConfigStore) GetPurchaseHistoryByPurchaseID(ctx context.Context, purchaseID string) (*config.PurchaseHistoryRecord, error) { args := m.Called(ctx, purchaseID) if args.Get(0) == nil { @@ -443,6 +443,24 @@ func (m *MockConfigStore) GetPurchaseHistoryInFlight(ctx context.Context) ([]*co return v, args.Error(1) } +// UpdatePurchaseHistoryListing mocks stamping listing_id and listing_state (issue #292). +func (m *MockConfigStore) UpdatePurchaseHistoryListing(ctx context.Context, purchaseID, listingID, listingState string) error { + args := m.Called(ctx, purchaseID, listingID, listingState) + return args.Error(0) +} + +// StampOfferingClass mocks persisting the offering_class fetched from AWS (issue #292). +func (m *MockConfigStore) StampOfferingClass(ctx context.Context, purchaseID, offeringClass string) error { + args := m.Called(ctx, purchaseID, offeringClass) + return args.Error(0) +} + +// ClaimMarketplaceListingSlot mocks the atomic listing-slot claim (issue #292). +func (m *MockConfigStore) ClaimMarketplaceListingSlot(ctx context.Context, purchaseID string) (bool, error) { + args := m.Called(ctx, purchaseID) + return args.Bool(0), args.Error(1) +} + func (m *MockConfigStore) SaveRIExchangeRecord(ctx context.Context, record *config.RIExchangeRecord) error { args := m.Called(ctx, record) return args.Error(0) diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 89ebce2fd..026f586e7 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -17,6 +17,7 @@ import ( "github.com/LeanerCloud/CUDly/pkg/provider" azureprovider "github.com/LeanerCloud/CUDly/providers/azure" gcpprovider "github.com/LeanerCloud/CUDly/providers/gcp" + ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types" "github.com/aws/aws-sdk-go-v2/service/sts" "github.com/google/uuid" ) @@ -785,6 +786,14 @@ func (m *Manager) savePurchaseHistory(ctx context.Context, exec *config.Purchase // Revoke button (issue #290). Azure-only in Phase 1; nil for AWS/GCP. RevocationWindowClosesAt: config.RevocationWindowClosesAtFor(rec.Provider, purchasedAt), } + // Stamp offering_class for AWS EC2 Reserved Instances. CUDly always + // purchases EC2 RIs with offering-class=Convertible (the EC2 client + // hard-wires OfferingClassTypeConvertible). Stamping at write time + // ensures the marketplace-sell guard can distinguish eligibility without + // an extra AWS round-trip for CUDly-purchased rows (issue #292). + if rec.Provider == string(common.ProviderAWS) && rec.Service == string(common.ServiceEC2) { + historyRecord.OfferingClass = string(ec2types.OfferingClassTypeConvertible) + } if err := m.config.SavePurchaseHistory(ctx, historyRecord); err != nil { logging.Errorf("Failed to save history: %v", err) return err diff --git a/internal/server/test_helpers_test.go b/internal/server/test_helpers_test.go index e2e304d00..52618d416 100644 --- a/internal/server/test_helpers_test.go +++ b/internal/server/test_helpers_test.go @@ -368,3 +368,13 @@ func (m *mockConfigStoreForHealth) UpdateGlobalConfigAtomic(_ context.Context, a } return cfg, nil } +func (m *mockConfigStoreForHealth) UpdatePurchaseHistoryListing(_ context.Context, _, _, _ string) error { + return nil +} +func (m *mockConfigStoreForHealth) ClaimMarketplaceListingSlot(_ context.Context, _ string) (bool, error) { + return true, nil +} + +func (m *mockConfigStoreForHealth) StampOfferingClass(_ context.Context, _, _ string) error { + return nil +} diff --git a/providers/aws/services/ec2/client.go b/providers/aws/services/ec2/client.go index 0e8166a77..0e9af15e0 100644 --- a/providers/aws/services/ec2/client.go +++ b/providers/aws/services/ec2/client.go @@ -28,6 +28,9 @@ type EC2API interface { GetReservedInstancesExchangeQuote(ctx context.Context, params *ec2.GetReservedInstancesExchangeQuoteInput, optFns ...func(*ec2.Options)) (*ec2.GetReservedInstancesExchangeQuoteOutput, error) AcceptReservedInstancesExchangeQuote(ctx context.Context, params *ec2.AcceptReservedInstancesExchangeQuoteInput, optFns ...func(*ec2.Options)) (*ec2.AcceptReservedInstancesExchangeQuoteOutput, error) CreateTags(ctx context.Context, params *ec2.CreateTagsInput, optFns ...func(*ec2.Options)) (*ec2.CreateTagsOutput, error) + CreateReservedInstancesListing(ctx context.Context, params *ec2.CreateReservedInstancesListingInput, optFns ...func(*ec2.Options)) (*ec2.CreateReservedInstancesListingOutput, error) + DescribeReservedInstancesListings(ctx context.Context, params *ec2.DescribeReservedInstancesListingsInput, optFns ...func(*ec2.Options)) (*ec2.DescribeReservedInstancesListingsOutput, error) + CancelReservedInstancesListing(ctx context.Context, params *ec2.CancelReservedInstancesListingInput, optFns ...func(*ec2.Options)) (*ec2.CancelReservedInstancesListingOutput, error) } // Client handles AWS EC2 Reserved Instances @@ -923,3 +926,159 @@ func normalizationFactorForInstanceType(instanceType string) float64 { } return exchange.NormalizationFactorForSize(parts[1]) } + +// MarketplaceListingRequest carries the parameters for +// CreateMarketplaceListing. PriceSchedule is required by the AWS API; a +// nil/empty slice is rejected before the outbound call is made. +type MarketplaceListingRequest struct { + // ReservedInstancesID is the AWS ReservedInstancesId to list. + ReservedInstancesID string + // ClientToken is a caller-supplied idempotency token (UUID). AWS dedupes + // CreateReservedInstancesListing calls sharing the same token for the same + // RI within a short window. + ClientToken string + // PriceSchedule is the per-month price schedule. At least one entry is + // required. Each entry specifies how many months the price applies and the + // list price per unit. + PriceSchedule []MarketplacePriceTier + // InstanceCount is the number of Reserved Instances to list. A purchase + // row of N RIs must list all N; this mirrors purchase_history.count. Values + // <= 0 are rejected before the outbound call so a misconfigured caller can + // never silently list a single RI from a multi-count row. + InstanceCount int32 +} + +// MarketplacePriceTier represents one tier of the AWS RI Marketplace price +// schedule. Term is the number of months this price applies, counting down +// from the remaining RI term. Price is the per-unit listing price in USD. +type MarketplacePriceTier struct { + // Term is the remaining-months count this tier covers. + Term int64 + // Price is the USD list price per unit for this tier. + Price float64 +} + +// MarketplaceListingResult is returned by CreateMarketplaceListing and +// DescribeMarketplaceListing. +type MarketplaceListingResult struct { + // ListingID is the AWS ReservedInstancesListingId. + ListingID string + // State is the AWS listing state: active, cancelled, closed, pending-fulfillment, etc. + State string +} + +// CreateMarketplaceListing calls ec2:CreateReservedInstancesListing. +// Only Standard (not Convertible) RIs can be listed; AWS rejects requests +// for Convertible RIs with an explicit API error — the caller should gate on +// offering_class == "standard" before calling this to surface a cleaner error. +// AWS also requires the seller account to have a US bank account on file; +// that precondition is checked in the API handler so the UI can show a +// tailored message before making the API call. +func (c *Client) CreateMarketplaceListing(ctx context.Context, req MarketplaceListingRequest) (MarketplaceListingResult, error) { + if len(req.PriceSchedule) == 0 { + return MarketplaceListingResult{}, fmt.Errorf("price schedule must have at least one tier") + } + if req.ReservedInstancesID == "" { + return MarketplaceListingResult{}, fmt.Errorf("reserved instances ID is required") + } + if req.InstanceCount <= 0 { + return MarketplaceListingResult{}, fmt.Errorf("instance count must be a positive integer, got %d", req.InstanceCount) + } + + awsSchedule := make([]types.PriceScheduleSpecification, 0, len(req.PriceSchedule)) + for _, tier := range req.PriceSchedule { + tier := tier // capture loop var + awsSchedule = append(awsSchedule, types.PriceScheduleSpecification{ + Term: aws.Int64(tier.Term), + Price: aws.Float64(tier.Price), + CurrencyCode: types.CurrencyCodeValuesUsd, + }) + } + + input := &ec2.CreateReservedInstancesListingInput{ + ReservedInstancesId: aws.String(req.ReservedInstancesID), + ClientToken: aws.String(req.ClientToken), + InstanceCount: aws.Int32(req.InstanceCount), + PriceSchedules: awsSchedule, + } + + out, err := c.client.CreateReservedInstancesListing(ctx, input) + if err != nil { + return MarketplaceListingResult{}, fmt.Errorf("CreateReservedInstancesListing failed: %w", err) + } + if len(out.ReservedInstancesListings) == 0 { + return MarketplaceListingResult{}, fmt.Errorf("CreateReservedInstancesListing returned empty response") + } + listing := out.ReservedInstancesListings[0] + state := "" + if listing.Status != "" { + state = string(listing.Status) + } + listingID := aws.ToString(listing.ReservedInstancesListingId) + if listingID == "" { + return MarketplaceListingResult{}, fmt.Errorf("CreateReservedInstancesListing returned a listing with an empty ID") + } + return MarketplaceListingResult{ListingID: listingID, State: state}, nil +} + +// DescribeMarketplaceListing polls the status of a single listing by ID. +// Returns an error when the listing is not found. +func (c *Client) DescribeMarketplaceListing(ctx context.Context, listingID string) (MarketplaceListingResult, error) { + out, err := c.client.DescribeReservedInstancesListings(ctx, &ec2.DescribeReservedInstancesListingsInput{ + ReservedInstancesListingId: aws.String(listingID), + }) + if err != nil { + return MarketplaceListingResult{}, fmt.Errorf("DescribeReservedInstancesListings failed: %w", err) + } + if len(out.ReservedInstancesListings) == 0 { + return MarketplaceListingResult{}, fmt.Errorf("listing %s not found", listingID) + } + listing := out.ReservedInstancesListings[0] + state := string(listing.Status) + resolvedID := aws.ToString(listing.ReservedInstancesListingId) + if resolvedID == "" { + resolvedID = listingID + } + return MarketplaceListingResult{ListingID: resolvedID, State: state}, nil +} + +// FetchOfferingClass calls ec2:DescribeReservedInstances to fetch the +// offering_class ("standard" or "convertible") for a specific Reserved +// Instance ID. Used by the marketplace-list handler to lazily populate +// offering_class for rows created before migration 000087 or for externally- +// created (pre-CUDly) Standard RIs whose offering_class was never stamped. +// Returns an error when the RI is not found (e.g. it has expired or was +// exchanged) because a missing RI cannot be listed. +func (c *Client) FetchOfferingClass(ctx context.Context, reservedInstancesID string) (string, error) { + out, err := c.client.DescribeReservedInstances(ctx, &ec2.DescribeReservedInstancesInput{ + ReservedInstancesIds: []string{reservedInstancesID}, + }) + if err != nil { + return "", fmt.Errorf("DescribeReservedInstances(%s): %w", reservedInstancesID, err) + } + if len(out.ReservedInstances) == 0 { + return "", fmt.Errorf("reserved instance %s not found (may have expired or been exchanged)", reservedInstancesID) + } + return string(out.ReservedInstances[0].OfferingClass), nil +} + +// CancelMarketplaceListing calls ec2:CancelReservedInstancesListing to +// withdraw an active listing. Returns the updated listing state. +func (c *Client) CancelMarketplaceListing(ctx context.Context, listingID string) (MarketplaceListingResult, error) { + out, err := c.client.CancelReservedInstancesListing(ctx, &ec2.CancelReservedInstancesListingInput{ + ReservedInstancesListingId: aws.String(listingID), + }) + if err != nil { + return MarketplaceListingResult{}, fmt.Errorf("CancelReservedInstancesListing failed: %w", err) + } + if len(out.ReservedInstancesListings) == 0 { + return MarketplaceListingResult{}, fmt.Errorf("CancelReservedInstancesListing returned empty response") + } + listing := out.ReservedInstancesListings[0] + state := string(listing.Status) + resolvedID := aws.ToString(listing.ReservedInstancesListingId) + if resolvedID == "" { + resolvedID = listingID + } + return MarketplaceListingResult{ListingID: resolvedID, State: state}, nil +} diff --git a/providers/aws/services/ec2/client_test.go b/providers/aws/services/ec2/client_test.go index 14c3c893f..ae621601b 100644 --- a/providers/aws/services/ec2/client_test.go +++ b/providers/aws/services/ec2/client_test.go @@ -78,6 +78,30 @@ func (m *MockEC2Client) CreateTags(ctx context.Context, params *ec2.CreateTagsIn return args.Get(0).(*ec2.CreateTagsOutput), args.Error(1) } +func (m *MockEC2Client) CreateReservedInstancesListing(ctx context.Context, params *ec2.CreateReservedInstancesListingInput, optFns ...func(*ec2.Options)) (*ec2.CreateReservedInstancesListingOutput, error) { + args := m.Called(ctx, params) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*ec2.CreateReservedInstancesListingOutput), args.Error(1) +} + +func (m *MockEC2Client) DescribeReservedInstancesListings(ctx context.Context, params *ec2.DescribeReservedInstancesListingsInput, optFns ...func(*ec2.Options)) (*ec2.DescribeReservedInstancesListingsOutput, error) { + args := m.Called(ctx, params) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*ec2.DescribeReservedInstancesListingsOutput), args.Error(1) +} + +func (m *MockEC2Client) CancelReservedInstancesListing(ctx context.Context, params *ec2.CancelReservedInstancesListingInput, optFns ...func(*ec2.Options)) (*ec2.CancelReservedInstancesListingOutput, error) { + args := m.Called(ctx, params) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(*ec2.CancelReservedInstancesListingOutput), args.Error(1) +} + func TestNewClient(t *testing.T) { t.Parallel() cfg := aws.Config{ @@ -885,3 +909,166 @@ func TestPurchaseCommitment_IdempotencySkipLogMasked(t *testing.T) { assert.Equal(t, token[:8]+"...", masked, "MaskToken shape: first 8 chars + ellipsis") assert.NotEqual(t, token, masked, "masked token must not equal raw token") } + +// --- RI Marketplace listing tests (issue #292) --- + +func TestClient_CreateMarketplaceListing_HappyPathMultiCount(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + // Capture the input so we can assert InstanceCount is the row's count, not 1. + mockEC2.On("CreateReservedInstancesListing", mock.Anything, + mock.MatchedBy(func(in *ec2.CreateReservedInstancesListingInput) bool { + return aws.ToInt32(in.InstanceCount) == 3 && + aws.ToString(in.ReservedInstancesId) == "ri-multi" && + len(in.PriceSchedules) == 1 + })). + Return(&ec2.CreateReservedInstancesListingOutput{ + ReservedInstancesListings: []types.ReservedInstancesListing{ + { + ReservedInstancesListingId: aws.String("ril-abc"), + Status: types.ListingStatusActive, + }, + }, + }, nil) + + res, err := client.CreateMarketplaceListing(context.Background(), MarketplaceListingRequest{ + ReservedInstancesID: "ri-multi", + ClientToken: "tok-1", + InstanceCount: 3, + PriceSchedule: []MarketplacePriceTier{{Term: 12, Price: 100}}, + }) + + assert.NoError(t, err) + assert.Equal(t, "ril-abc", res.ListingID) + assert.Equal(t, "active", res.State) + mockEC2.AssertExpectations(t) +} + +func TestClient_CreateMarketplaceListing_RejectsNonPositiveCount(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + _, err := client.CreateMarketplaceListing(context.Background(), MarketplaceListingRequest{ + ReservedInstancesID: "ri-1", + ClientToken: "tok", + InstanceCount: 0, + PriceSchedule: []MarketplacePriceTier{{Term: 12, Price: 100}}, + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "instance count must be a positive integer") + // No outbound call must have been made. + mockEC2.AssertNotCalled(t, "CreateReservedInstancesListing", mock.Anything, mock.Anything) + mockEC2.AssertExpectations(t) +} + +func TestClient_CreateMarketplaceListing_EmptyScheduleRejected(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + _, err := client.CreateMarketplaceListing(context.Background(), MarketplaceListingRequest{ + ReservedInstancesID: "ri-1", + InstanceCount: 1, + }) + assert.Error(t, err) + assert.Contains(t, err.Error(), "price schedule must have at least one tier") + mockEC2.AssertExpectations(t) +} + +func TestClient_CreateMarketplaceListing_EmptyListingIDRejected(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + mockEC2.On("CreateReservedInstancesListing", mock.Anything, mock.Anything). + Return(&ec2.CreateReservedInstancesListingOutput{ + ReservedInstancesListings: []types.ReservedInstancesListing{ + {ReservedInstancesListingId: aws.String(""), Status: types.ListingStatusActive}, + }, + }, nil) + + _, err := client.CreateMarketplaceListing(context.Background(), MarketplaceListingRequest{ + ReservedInstancesID: "ri-1", + InstanceCount: 1, + PriceSchedule: []MarketplacePriceTier{{Term: 12, Price: 100}}, + }) + assert.Error(t, err) + assert.Contains(t, err.Error(), "empty ID") + mockEC2.AssertExpectations(t) +} + +func TestClient_DescribeMarketplaceListing(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + mockEC2.On("DescribeReservedInstancesListings", mock.Anything, + mock.MatchedBy(func(in *ec2.DescribeReservedInstancesListingsInput) bool { + return aws.ToString(in.ReservedInstancesListingId) == "ril-xyz" + })). + Return(&ec2.DescribeReservedInstancesListingsOutput{ + ReservedInstancesListings: []types.ReservedInstancesListing{ + {ReservedInstancesListingId: aws.String("ril-xyz"), Status: types.ListingStatusClosed}, + }, + }, nil) + + res, err := client.DescribeMarketplaceListing(context.Background(), "ril-xyz") + assert.NoError(t, err) + assert.Equal(t, "ril-xyz", res.ListingID) + assert.Equal(t, "closed", res.State) + mockEC2.AssertExpectations(t) +} + +func TestClient_DescribeMarketplaceListing_NotFound(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + mockEC2.On("DescribeReservedInstancesListings", mock.Anything, mock.Anything). + Return(&ec2.DescribeReservedInstancesListingsOutput{}, nil) + + _, err := client.DescribeMarketplaceListing(context.Background(), "ril-missing") + assert.Error(t, err) + assert.Contains(t, err.Error(), "not found") + mockEC2.AssertExpectations(t) +} + +func TestClient_CancelMarketplaceListing(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + mockEC2.On("CancelReservedInstancesListing", mock.Anything, + mock.MatchedBy(func(in *ec2.CancelReservedInstancesListingInput) bool { + return aws.ToString(in.ReservedInstancesListingId) == "ril-cancel" + })). + Return(&ec2.CancelReservedInstancesListingOutput{ + ReservedInstancesListings: []types.ReservedInstancesListing{ + {ReservedInstancesListingId: aws.String("ril-cancel"), Status: types.ListingStatusCancelled}, + }, + }, nil) + + res, err := client.CancelMarketplaceListing(context.Background(), "ril-cancel") + assert.NoError(t, err) + assert.Equal(t, "ril-cancel", res.ListingID) + assert.Equal(t, "cancelled", res.State) + mockEC2.AssertExpectations(t) +} + +func TestClient_CancelMarketplaceListing_APIError(t *testing.T) { + t.Parallel() + mockEC2 := &MockEC2Client{} + client := &Client{client: mockEC2, region: "us-east-1"} + + mockEC2.On("CancelReservedInstancesListing", mock.Anything, mock.Anything). + Return(nil, fmt.Errorf("boom")) + + _, err := client.CancelMarketplaceListing(context.Background(), "ril-cancel") + assert.Error(t, err) + assert.Contains(t, err.Error(), "CancelReservedInstancesListing failed") + mockEC2.AssertExpectations(t) +}