perf(exchange): rec-driven cross-family alternatives via cached Cost Explorer recommendations

internal/api/exchange_lookup.go

@@ -0,0 +1,193 @@
+// Package api provides the HTTP API handlers for the CUDly dashboard.
+package api
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/LeanerCloud/CUDly/internal/config"
+	"github.com/LeanerCloud/CUDly/pkg/exchange"
+)
+
+// recsLister is the narrow slice of config.StoreInterface that the
+// reshape lookup needs. Scoped here so the closure stays unit-testable
+// against a tiny fake instead of the full StoreInterface mock.
+type recsLister interface {
+	ListStoredRecommendations(ctx context.Context, filter config.RecommendationFilter) ([]config.RecommendationRecord, error)
+}
+
+// purchaseRecLookupFromStore builds an exchange.PurchaseRecLookup that
+// reads the cached AWS Cost Explorer purchase recommendations out of
+// Postgres for a given (region, currency) pair. Each
+// RecommendationRecord is mapped to an OfferingOption with:
+//
+//   - InstanceType   = rec.ResourceType        (e.g. "m6i.large")
+//   - OfferingID     = rec.ID                  (UUID; the UI uses this as a stable handle)
+//   - EffectiveMonthlyCost = (UpfrontCost / termMonths) + MonthlyCost
+//   - NormalizationFactor  = exchange.NormalizationFactorForSize(size)
+//   - CurrencyCode   = currencyCode            (propagated; recs don't carry it)
+//
+// Where termMonths = rec.Term × 12 (rec.Term is in years, AWS standard
+// for RIs / Savings Plans). Term ≤ 0 means we can't amortise upfront,
+// so we fall back to MonthlyCost alone — the dollar-units check will
+// then accept or reject based on monthly recurring vs. source.
+//
+// AccountID scoping (cross-account leak guard): when accountID is
+// non-empty we restrict the query to that single CloudAccount UUID so
+// reshape can't surface another tenant's recs. Empty accountID means
+// "no filter" — used for ambient-credentials deployments where the
+// caller couldn't (or chose not to) resolve the source account.
+func purchaseRecLookupFromStore(store recsLister, accountID string) exchange.PurchaseRecLookup {
+	return func(ctx context.Context, region, currencyCode string) ([]exchange.OfferingOption, error) {
+		filter := config.RecommendationFilter{
+			Provider: "aws",
+			Service:  "ec2",
+			Region:   region,
+		}
+		if accountID != "" {
+			filter.AccountIDs = []string{accountID}
+		}
+		recs, err := store.ListStoredRecommendations(ctx, filter)
+		if err != nil {
+			return nil, err
+		}
+		out := make([]exchange.OfferingOption, 0, len(recs))
+		for _, rec := range recs {
+			out = append(out, recommendationToOffering(rec, currencyCode))
+		}
+		return out, nil
+	}
+}
+
+// recommendationToOffering maps a single cached Cost Explorer purchase
+// recommendation to the OfferingOption shape the reshape layer
+// consumes. Pulled out so the mapping can be unit-tested in isolation
+// (no DB / no ctx required).
+//
+// TermSeconds is derived from rec.Term (years) using the canonical
+// AWS RI duration constant 31_536_000s/year — the same value the AWS
+// SDK reports on ec2.ReservedInstance.Duration so the term-match guard
+// in pkg/exchange.fillAlternativesFromRecs can compare apples-to-apples
+// against RIInfo.TermSeconds populated from ec2.ConvertibleRI.Duration.
+func recommendationToOffering(rec config.RecommendationRecord, currencyCode string) exchange.OfferingOption {
+	// AWS Cost Explorer returns MonthlyCost and UpfrontCost as totals for
+	// the recommended Count of instances, not per-instance. The reshape
+	// layer compares OfferingOption.EffectiveMonthlyCost against
+	// RIInfo.MonthlyCost which is per-instance, so we normalize here.
+	// rec.Count == 0 is treated as "unknown count, fall back to total"
+	// rather than dividing by zero — practically rare since the scheduler
+	// only stores recs with Count >= 1.
+	count := float64(1)
+	if rec.Count > 0 {
+		count = float64(rec.Count)
+	}
+	monthly := rec.MonthlyCost / count
+	if rec.Term > 0 {
+		// rec.Term is in years; canonical AWS RI/SP amortisation uses
+		// 12 months per year regardless of leap years.
+		termMonths := float64(rec.Term * 12)
+		if termMonths > 0 {
+			monthly += rec.UpfrontCost / (count * termMonths)
+		}
+	}
+	_, size := splitInstanceType(rec.ResourceType)
+	var termSeconds int64
+	if rec.Term > 0 {
+		termSeconds = int64(rec.Term) * secondsPerYear
+	}
+	return exchange.OfferingOption{
+		InstanceType:         rec.ResourceType,
+		OfferingID:           rec.ID,
+		EffectiveMonthlyCost: monthly,
+		NormalizationFactor:  exchange.NormalizationFactorForSize(size),
+		CurrencyCode:         currencyCode,
+		TermSeconds:          termSeconds,
+	}
+}
+
+// secondsPerYear is the AWS-canonical RI duration constant for a 1-year
+// term (365 × 86400). Matches the value the AWS SDK reports on
+// ec2.ReservedInstance.Duration and the value
+// ec2.ConvertibleRI.Duration carries — used so OfferingOption.TermSeconds
+// can be compared directly against RIInfo.TermSeconds.
+const secondsPerYear int64 = 365 * 24 * 60 * 60
+
+// splitInstanceType splits "m5.xlarge" into ("m5", "xlarge"). Returns
+// empty strings if the format is unrecognized. Mirrors the helper in
+// pkg/exchange/reshape.go but kept local to avoid exporting a
+// general-purpose parser the caller doesn't need.
+func splitInstanceType(instanceType string) (family, size string) {
+	parts := strings.SplitN(instanceType, ".", 2)
+	if len(parts) != 2 {
+		return "", ""
+	}
+	return parts[0], parts[1]
+}
+
+// resolveAWSCloudAccountID maps the running AWS account ID (raw, from
+// STS) to the registered CloudAccount UUID so the reshape lookup can
+// scope its query against the correct row in the recommendations
+// table. Returns ("", nil) ONLY for the truly-no-scope cases:
+//
+//   - AWS SDK config could not load (deployment is running on an
+//     Azure / GCP host with no AWS context at all — resolveAWSAccountID
+//     returns ("", nil) for this case);
+//   - ListCloudAccounts returned ZERO rows (no CloudAccounts registered
+//     at all — the bootstrap-before-first-account path; the recs table
+//     is also empty so an unscoped read is harmless).
+//
+// purchaseRecLookupFromStore treats ("", nil) as "skip the AccountIDs
+// filter" so a deployment with ambient credentials and no registered
+// CloudAccounts still sees alternatives, not a permanently empty list.
+// Once the operator registers the account the filter engages.
+//
+// FAIL CLOSED on every real failure:
+//   - resolveAWSAccountID returns a non-nil error (STS GetCallerIdentity
+//     denied, transient AWS API failure, token expiry) — propagated so
+//     the caller aborts the lookup rather than silently falling through
+//     to an unscoped query that could leak another tenant's recs.
+//   - ListCloudAccounts returns an error (DB outage, permissions) —
+//     same treatment.
+//   - The running AWS account is resolved but DOES NOT match any
+//     registered CloudAccount AND there ARE registered accounts:
+//     return an error. Returning ("", nil) here would have
+//     purchaseRecLookupFromStore omit the AccountIDs filter and serve
+//     up another tenant's recs — exactly the multi-tenant leak the rest
+//     of this code path is designed to prevent.
+func (h *Handler) resolveAWSCloudAccountID(ctx context.Context) (string, error) {
+	awsAccountID, err := h.resolveAWSAccountID(ctx)
+	if err != nil {
+		// STS reachable-but-failed: must NOT fall through to an
+		// unscoped read. A transient STS error in a multi-tenant
+		// deployment would otherwise surface another tenant's recs.
+		return "", fmt.Errorf("resolve source aws account for reshape scope: %w", err)
+	}
+	if awsAccountID == "" {
+		// Genuine "no AWS context" (Azure/GCP host).
+		return "", nil
+	}
+	provider := "aws"
+	accounts, err := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{Provider: &provider})
+	if err != nil {
+		return "", fmt.Errorf("list cloud accounts for reshape scope: %w", err)
+	}
+	if len(accounts) == 0 {
+		// Bootstrap: no CloudAccounts registered at all. The recs
+		// table is necessarily empty too, so an unscoped read is a
+		// no-op rather than a leak.
+		return "", nil
+	}
+	for _, a := range accounts {
+		if a.ExternalID == awsAccountID {
+			return a.ID, nil
+		}
+	}
+	// Resolved-but-unregistered: AWS host has accounts, but the running
+	// account isn't one of them. Could be a misconfigured deployment, a
+	// fresh first-run before the operator added their own account, or
+	// (worst case) a host running in a different account than any
+	// registered tenant. Either way, returning "" here would skip the
+	// AccountIDs filter and leak other tenants' recs — fail closed.
+	return "", fmt.Errorf("running aws account %s is not registered in cloud_accounts; reshape scope cannot be resolved safely", awsAccountID)
+}

internal/api/exchange_lookup_test.go

@@ -0,0 +1,205 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/LeanerCloud/CUDly/internal/config"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// fakeRecsLister captures the filter passed to ListStoredRecommendations
+// so tests can assert region / account / provider scoping landed in the
+// SQL query. Returns a configurable result set or error.
+type fakeRecsLister struct {
+	gotFilter config.RecommendationFilter
+	calls     int
+	out       []config.RecommendationRecord
+	err       error
+}
+
+func (f *fakeRecsLister) ListStoredRecommendations(_ context.Context, filter config.RecommendationFilter) ([]config.RecommendationRecord, error) {
+	f.calls++
+	f.gotFilter = filter
+	return f.out, f.err
+}
+
+// TestPurchaseRecLookupFromStore_RegionFilter pins that the closure
+// pushes the requested region into the SQL filter so Postgres prunes
+// rows by region rather than the Go layer doing it after the fact.
+func TestPurchaseRecLookupFromStore_RegionFilter(t *testing.T) {
+	t.Parallel()
+	store := &fakeRecsLister{}
+	lookup := purchaseRecLookupFromStore(store, "")
+	_, err := lookup(context.Background(), "eu-west-1", "USD")
+	require.NoError(t, err)
+	assert.Equal(t, 1, store.calls, "lookup must invoke the store exactly once")
+	assert.Equal(t, "aws", store.gotFilter.Provider, "must scope to AWS recs")
+	assert.Equal(t, "ec2", store.gotFilter.Service, "must scope to EC2 recs (no RDS / opensearch leakage)")
+	assert.Equal(t, "eu-west-1", store.gotFilter.Region, "region must thread through to SQL")
+}
+
+// TestPurchaseRecLookupFromStore_AccountFilter pins the cross-account
+// leak guard: when an account UUID is supplied, the filter restricts
+// the query to that single account so the reshape page can't surface
+// another tenant's recommendations.
+func TestPurchaseRecLookupFromStore_AccountFilter(t *testing.T) {
+	t.Parallel()
+	store := &fakeRecsLister{}
+	lookup := purchaseRecLookupFromStore(store, "acct-uuid-123")
+	_, err := lookup(context.Background(), "us-east-1", "USD")
+	require.NoError(t, err)
+	require.Len(t, store.gotFilter.AccountIDs, 1, "non-empty account UUID must populate AccountIDs filter")
+	assert.Equal(t, "acct-uuid-123", store.gotFilter.AccountIDs[0])
+}
+
+// TestPurchaseRecLookupFromStore_NoAccountFilterWhenEmpty pins the
+// degraded-mode contract: when the caller can't resolve an account
+// UUID (ambient credentials, account not registered yet), the lookup
+// returns whatever recs exist in the region rather than blanking the
+// page. The operator can register the account later to engage scoping.
+func TestPurchaseRecLookupFromStore_NoAccountFilterWhenEmpty(t *testing.T) {
+	t.Parallel()
+	store := &fakeRecsLister{}
+	lookup := purchaseRecLookupFromStore(store, "")
+	_, err := lookup(context.Background(), "us-east-1", "USD")
+	require.NoError(t, err)
+	assert.Empty(t, store.gotFilter.AccountIDs, "empty account UUID must NOT add an AccountIDs filter")
+}
+
+// TestPurchaseRecLookupFromStore_NoRecsReturnsEmpty pins the
+// cold-cache contract: zero recs in the region → empty slice (not
+// nil-error). The downstream AnalyzeReshapingWithRecs treats an empty
+// slice the same as "no alternatives, primary target intact".
+func TestPurchaseRecLookupFromStore_NoRecsReturnsEmpty(t *testing.T) {
+	t.Parallel()
+	store := &fakeRecsLister{out: nil}
+	lookup := purchaseRecLookupFromStore(store, "")
+	got, err := lookup(context.Background(), "us-east-1", "USD")
+	require.NoError(t, err)
+	assert.Empty(t, got, "empty recs → empty offerings, no error")
+}
+
+// TestPurchaseRecLookupFromStore_StoreErrorPropagates pins the error
+// path: an underlying SQL failure surfaces back to the caller.
+// AnalyzeReshapingWithRecs handles the error by falling back to base
+// recs (graceful degradation), so the closure just needs to forward
+// the error verbatim.
+func TestPurchaseRecLookupFromStore_StoreErrorPropagates(t *testing.T) {
+	t.Parallel()
+	store := &fakeRecsLister{err: errors.New("postgres timeout")}
+	lookup := purchaseRecLookupFromStore(store, "")
+	_, err := lookup(context.Background(), "us-east-1", "USD")
+	require.Error(t, err, "store errors must propagate so the caller can fall back")
+}
+
+// TestPurchaseRecLookupFromStore_MapsFields pins the
+// RecommendationRecord → OfferingOption mapping shape so the dollar-
+// units pre-filter and the UI both see consistent data:
+//   - InstanceType comes from ResourceType.
+//   - OfferingID = rec.ID (stable handle).
+//   - EffectiveMonthlyCost = UpfrontCost / (Term * 12) + MonthlyCost.
+//   - NormalizationFactor resolved from the size (here "large" → 4).
+//   - CurrencyCode propagated from the lookup's currencyCode arg.
+func TestPurchaseRecLookupFromStore_MapsFields(t *testing.T) {
+	t.Parallel()
+	store := &fakeRecsLister{
+		out: []config.RecommendationRecord{
+			{
+				ID:           "rec-1",
+				Provider:     "aws",
+				Service:      "ec2",
+				Region:       "us-east-1",
+				ResourceType: "m6i.large",
+				Term:         1,   // 1 year term
+				UpfrontCost:  120, // 120 / 12 = 10/mo amortised
+				MonthlyCost:  20,  // + 20/mo recurring = 30
+			},
+			{
+				// Term=0 → no upfront amortisation; effective = MonthlyCost only.
+				ID:           "rec-2",
+				Provider:     "aws",
+				Service:      "ec2",
+				Region:       "us-east-1",
+				ResourceType: "c5.xlarge",
+				Term:         0,
+				UpfrontCost:  500, // ignored when Term == 0
+				MonthlyCost:  50,
+			},
+		},
+	}
+	lookup := purchaseRecLookupFromStore(store, "")
+	got, err := lookup(context.Background(), "us-east-1", "EUR")
+	require.NoError(t, err)
+	require.Len(t, got, 2)
+
+	assert.Equal(t, "m6i.large", got[0].InstanceType)
+	assert.Equal(t, "rec-1", got[0].OfferingID)
+	assert.InDelta(t, 30.0, got[0].EffectiveMonthlyCost, 0.001,
+		"upfront 120 over 12 months = 10/mo + recurring 20/mo = 30/mo")
+	assert.InDelta(t, 4.0, got[0].NormalizationFactor, 0.001, "large → NF 4")
+	assert.Equal(t, "EUR", got[0].CurrencyCode, "currency must be propagated from caller")
+
+	assert.Equal(t, "c5.xlarge", got[1].InstanceType)
+	assert.InDelta(t, 50.0, got[1].EffectiveMonthlyCost, 0.001,
+		"Term==0 means upfront cannot be amortised; fall back to MonthlyCost")
+	assert.InDelta(t, 8.0, got[1].NormalizationFactor, 0.001, "xlarge → NF 8")
+
+	// Term plumbing: 1y rec → 31_536_000 seconds (AWS canonical RI
+	// duration); Term==0 → TermSeconds==0 (the reshape term-match guard
+	// then falls back to "skip the gate" rather than blocking the rec).
+	assert.Equal(t, int64(365*24*60*60), got[0].TermSeconds,
+		"1-year rec must serialise to 31_536_000s for the term-match guard")
+	assert.Equal(t, int64(0), got[1].TermSeconds,
+		"Term==0 rec must not synthesise a fake duration — TermSeconds stays zero")
+}
+
+// TestPurchaseRecLookupFromStore_ThreeYearTerm pins the multi-year
+// path: rec.Term=3 must serialise to exactly 3 × 31_536_000s so the
+// reshape term-match guard treats it as 3y rather than rounding it
+// onto a 1y surface.
+func TestPurchaseRecLookupFromStore_ThreeYearTerm(t *testing.T) {
+	t.Parallel()
+	store := &fakeRecsLister{
+		out: []config.RecommendationRecord{
+			{
+				ID: "rec-3y", Provider: "aws", Service: "ec2", Region: "us-east-1",
+				ResourceType: "m5.large", Term: 3, MonthlyCost: 10,
+			},
+		},
+	}
+	lookup := purchaseRecLookupFromStore(store, "")
+	got, err := lookup(context.Background(), "us-east-1", "USD")
+	require.NoError(t, err)
+	require.Len(t, got, 1)
+	assert.Equal(t, int64(3*365*24*60*60), got[0].TermSeconds,
+		"3-year rec must serialise to 3 × 31_536_000s for the term-match guard")
+}
+
+// TestSplitInstanceType pins the local instance-type parser used by
+// the mapping helper. Mirrors the pkg/exchange parser to avoid
+// exporting a general-purpose helper this package doesn't need.
+func TestSplitInstanceType(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		in         string
+		wantFamily string
+		wantSize   string
+	}{
+		{"m5.large", "m5", "large"},
+		{"m7g.metal", "m7g", "metal"},
+		{"r6i.16xlarge", "r6i", "16xlarge"},
+		{"", "", ""},
+		{"malformed", "", ""},
+	}
+	for _, c := range cases {
+		t.Run(c.in, func(t *testing.T) {
+			t.Parallel()
+			f, s := splitInstanceType(c.in)
+			assert.Equal(t, c.wantFamily, f)
+			assert.Equal(t, c.wantSize, s)
+		})
+	}
+}