diff --git a/providers/aws/ladder/adapters.go b/providers/aws/ladder/adapters.go new file mode 100644 index 000000000..4c2708f56 --- /dev/null +++ b/providers/aws/ladder/adapters.go @@ -0,0 +1,271 @@ +package ladder + +import ( + "context" + "fmt" + "strconv" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + sdksp "github.com/aws/aws-sdk-go-v2/service/savingsplans" + sptypes "github.com/aws/aws-sdk-go-v2/service/savingsplans/types" + + cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + + "github.com/LeanerCloud/CUDly/providers/aws/recommendations" +) + +// activeSPListAPI is the minimal interface for listing Savings Plans. +// Only DescribeSavingsPlans is needed; the full SavingsPlansAPI from +// providers/aws/services/savingsplans includes purchase and offering methods +// the read-only lister does not require (interface-segregation principle). +// Tests inject a hermetic fake implementing this narrow interface. +type activeSPListAPI interface { + DescribeSavingsPlans(ctx context.Context, params *sdksp.DescribeSavingsPlansInput, optFns ...func(*sdksp.Options)) (*sdksp.DescribeSavingsPlansOutput, error) +} + +// spListerStates is the set of Savings Plan states counted as existing +// commitment. +// - active: currently billing. +// - payment-pending: purchase accepted, first payment in flight. A +// just-purchased SP sits in this state briefly; it MUST count as existing +// commitment immediately, or the next scheduled run would not see it and +// double-purchase. Safe default before the L6 write side lands. +// - queued (future-dated) SPs are deliberately EXCLUDED: they have not +// started, so counting them would suppress purchases the queued SP will +// not cover until its start date. Revisit with L14 (expiry alignment), +// which can reason about future-dated coverage explicitly. +var spListerStates = []sptypes.SavingsPlanState{ + sptypes.SavingsPlanStateActive, + sptypes.SavingsPlanStatePaymentPending, +} + +// spListerAdapter implements the spLister interface using the AWS Savings +// Plans SDK directly. It filters to spListerStates (active + payment-pending; +// see that var for rationale) and scopes EC2Instance SPs to the ladder's +// region. Commitment strings are parsed with strconv.ParseFloat; parse errors +// fail loud per feedback_strict_int_parse and feedback_no_silent_fallbacks. +type spListerAdapter struct { + api activeSPListAPI + // region is the ladder's configured region. DescribeSavingsPlans is an + // account-wide API, but the ladder's layer state is region-scoped: + // EC2Instance SPs from other regions must not inflate this region's + // ExistingUSDPerHour (that would understate the gap and under-purchase). + region string +} + +// ListActiveSPs lists the account's Savings Plans in spListerStates and maps +// them to the AWSLadder view, region-scoped: +// +// - EC2Instance SPs are region-bound; entries whose Region differs from the +// adapter's region are EXCLUDED (they cover other regions' usage). +// - Compute SPs are global and are counted fully in every region. This is +// deliberately conservative: attributing the full global commitment to +// this region can only make the engine see MORE existing coverage and +// purchase LESS, never over-purchase. Multi-region laddering (L18) will +// revisit this attribution. +// +// Commitment strings and SP dates are validated at the boundary and fail +// loud (feedback_strict_int_parse, feedback_no_silent_fallbacks). +// +// Pagination is fully exhausted (issue #692): DescribeSavingsPlans documents +// no page limit, so a fixed page cap would silently truncate accounts with +// more Savings Plans than the cap (understating existing commitment). The +// only loop guard needed is against a REPEATED NextToken, which indicates +// API misbehavior rather than a legitimately long listing. +func (a *spListerAdapter) ListActiveSPs(ctx context.Context) ([]ActiveSP, error) { + var sps []ActiveSP + var nextToken *string + seenTokens := make(map[string]struct{}) + page := 0 + + for { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("ListActiveSPs: context cancelled: %w", err) + } + page++ + + input := &sdksp.DescribeSavingsPlansInput{ + States: spListerStates, + NextToken: nextToken, + MaxResults: aws.Int32(100), + } + out, err := a.api.DescribeSavingsPlans(ctx, input) + if err != nil { + return nil, fmt.Errorf("ListActiveSPs: DescribeSavingsPlans page %d: %w", page, err) + } + + sps, err = appendRegionScopedSPs(sps, out.SavingsPlans, a.region) + if err != nil { + return nil, err + } + + tok := aws.ToString(out.NextToken) + if tok == "" { + break + } + if _, dup := seenTokens[tok]; dup { + return nil, fmt.Errorf("ListActiveSPs: DescribeSavingsPlans returned a repeated pagination token on page %d; aborting to avoid an infinite loop", page) + } + seenTokens[tok] = struct{}{} + nextToken = out.NextToken + } + return sps, nil +} + +// appendRegionScopedSPs maps one DescribeSavingsPlans page onto the +// accumulated slice, applying the region-scoping rule: EC2Instance SPs bound +// to another region cover that region's usage, not ours; including them +// would inflate ExistingUSDPerHour and under-purchase. Compute SPs are +// global and always kept (see ListActiveSPs doc). Extracted to keep +// ListActiveSPs under the cyclomatic complexity limit. +func appendRegionScopedSPs(sps []ActiveSP, page []sptypes.SavingsPlan, region string) ([]ActiveSP, error) { + for _, sp := range page { + entry, err := mapActiveSP(sp) + if err != nil { + return nil, err + } + if entry.PlanType == spPlanTypeEC2Instance && entry.Region != region { + continue + } + sps = append(sps, entry) + } + return sps, nil +} + +// mapActiveSP converts one DescribeSavingsPlans entry to ActiveSP. +// Fails loud on a missing SavingsPlanId (would make the entry +// un-identifiable), on a non-numeric Commitment string (money path: cannot +// represent an absent number as 0, per feedback_nullable_not_zero and +// feedback_strict_int_parse), on missing or unparseable Start/End dates +// (a silently zero EndDate would drop the SP from sumExpiringSPHourlyCost +// and understate expiring commitment), and on an EC2Instance-plan SP with +// an empty Region (AWS always populates Region for region-bound plans, so +// an empty one means corrupted data; letting it fall through to the +// region-scope filter would silently exclude it, understating existing +// commitment and over-purchasing). +func mapActiveSP(sp sptypes.SavingsPlan) (ActiveSP, error) { + if sp.SavingsPlanId == nil { + return ActiveSP{}, fmt.Errorf("ListActiveSPs: DescribeSavingsPlans returned entry with nil SavingsPlanId") + } + if string(sp.SavingsPlanType) == spPlanTypeEC2Instance && aws.ToString(sp.Region) == "" { + return ActiveSP{}, fmt.Errorf("ListActiveSPs: EC2Instance SP %s has an empty Region; AWS always populates Region for region-bound plans, so this indicates corrupted data (silently excluding it would understate existing commitment)", + *sp.SavingsPlanId) + } + commitment := aws.ToString(sp.Commitment) + hourly, err := strconv.ParseFloat(commitment, 64) + if err != nil { + return ActiveSP{}, fmt.Errorf("ListActiveSPs: cannot parse Commitment %q for SP %s: %w", + commitment, *sp.SavingsPlanId, err) + } + start, err := parseSPDate("Start", sp.Start, *sp.SavingsPlanId) + if err != nil { + return ActiveSP{}, err + } + end, err := parseSPDate("End", sp.End, *sp.SavingsPlanId) + if err != nil { + return ActiveSP{}, err + } + + return ActiveSP{ + PlanID: *sp.SavingsPlanId, + PlanType: string(sp.SavingsPlanType), + State: string(sp.State), + Region: aws.ToString(sp.Region), + StartDate: start, + EndDate: end, + HourlyCommitmentUSD: hourly, + }, nil +} + +// parseSPDate parses a DescribeSavingsPlans RFC3339 date field, failing loud +// on a nil or unparseable value. Expiry math (sumExpiringSPHourlyCost) +// depends on EndDate; a silently zero date would exclude the SP from the +// expiring sum and understate expiring commitment +// (feedback_no_silent_fallbacks). +func parseSPDate(field string, value *string, planID string) (time.Time, error) { + if value == nil { + return time.Time{}, fmt.Errorf("ListActiveSPs: SP %s has nil %s date; expiry math requires it", planID, field) + } + t, err := time.Parse(time.RFC3339, *value) + if err != nil { + return time.Time{}, fmt.Errorf("ListActiveSPs: cannot parse %s date %q for SP %s: %w", + field, *value, planID, err) + } + return t, nil +} + +// onDemandSeriesAdapter implements onDemandSeriesSource by wrapping +// *recommendations.Client and mapping its []recommendations.DailyCost to this +// package's []DailyPoint. The two types have identical fields (Date midnight +// UTC + USDPerHour); the duplication exists because recommendations cannot +// import providers/aws/ladder (ladder already imports recommendations), so +// the mapping happens here at the seam. Ordering (oldest-first, strictly +// increasing unique UTC days) is guaranteed by the recommendations contract +// and preserved verbatim by the index-for-index copy. +type onDemandSeriesAdapter struct { + client *recommendations.Client +} + +// GetOnDemandSeries fetches the daily on-demand cost series from CE and maps +// each dated entry to a DailyPoint. Errors propagate unchanged (no silent +// fallback); an empty series is already rejected inside the client. +func (a *onDemandSeriesAdapter) GetOnDemandSeries(ctx context.Context, region string, lookbackDays int) ([]DailyPoint, error) { + costs, err := a.client.GetOnDemandSeries(ctx, region, lookbackDays) + if err != nil { + return nil, err + } + points := make([]DailyPoint, len(costs)) + for i, c := range costs { + points[i] = DailyPoint{Date: c.Date, USDPerHour: c.USDPerHour} + } + return points, nil +} + +// spCoverageAdapter implements spCoverageSource by wrapping *recommendations.Client +// and mapping its richer SPCoverageSummary to the local SPCoverageSummary type. +// The mapping preserves the nil-when-Days==0 contract: if CE returned no +// coverage data the recommendations summary has Days==0 and CoveragePct==nil; +// the adapter returns an empty local summary (CoveragePct stays nil, signalling +// "not measured" to the engine rather than "0% coverage"). +type spCoverageAdapter struct { + client *recommendations.Client +} + +// GetSPCoverageSummary fetches SP coverage from CE and maps the result to the +// local SPCoverageSummary type. CE does not support plan-type filtering for +// coverage (the filter contract allows only REGION, SERVICE, LINKED_ACCOUNT, +// and INSTANCE_FAMILY per the GetSavingsPlansCoverageInput SDK doc), so the +// returned summary applies to all SP types in the region. +func (a *spCoverageAdapter) GetSPCoverageSummary(ctx context.Context, region string, lookbackDays int) (SPCoverageSummary, error) { + rich, err := a.client.GetSPCoverageSummary(ctx, region, lookbackDays) + if err != nil { + return SPCoverageSummary{}, err + } + // Preserve nil-when-Days==0: recommendations.SPCoverageSummary.CoveragePct + // is nil when Days==0 (no CE data). The local type propagates that nil so + // the engine treats it as "not yet measured" rather than "zero coverage". + return SPCoverageSummary{CoveragePct: rich.CoveragePct}, nil +} + +// spUtilizationAdapter implements spUtilizationSource by wrapping +// *recommendations.Client and mapping its SPUtilizationSummary to the local +// type. The mapping follows the same nil-propagation contract as +// spCoverageAdapter. +type spUtilizationAdapter struct { + client *recommendations.Client +} + +// GetSPUtilization fetches SP utilization from CE for the given plan type and +// maps the result to the local SPUtilizationSummary type. planType must be a +// valid cetypes.SupportedSavingsPlansType (validated upstream in toSPUtilPlanType +// before this call). region=="" means all regions (for Compute SPs). +func (a *spUtilizationAdapter) GetSPUtilization(ctx context.Context, planType cetypes.SupportedSavingsPlansType, region string, lookbackDays int) (SPUtilizationSummary, error) { + rich, err := a.client.GetSPUtilization(ctx, planType, region, lookbackDays) + if err != nil { + return SPUtilizationSummary{}, err + } + // Propagate nil UtilizationPct: nil means CE returned no data ("not yet + // measured"), which the engine distinguishes from 0% (fully idle layer). + return SPUtilizationSummary{UtilizationPct: rich.UtilizationPct}, nil +} diff --git a/providers/aws/ladder/adapters_test.go b/providers/aws/ladder/adapters_test.go new file mode 100644 index 000000000..743bedc36 --- /dev/null +++ b/providers/aws/ladder/adapters_test.go @@ -0,0 +1,603 @@ +package ladder + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/costexplorer" + sdksp "github.com/aws/aws-sdk-go-v2/service/savingsplans" + sptypes "github.com/aws/aws-sdk-go-v2/service/savingsplans/types" + + cetypes "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + pkgladder "github.com/LeanerCloud/CUDly/pkg/ladder" + "github.com/LeanerCloud/CUDly/providers/aws/recommendations" +) + +// testRegion is the ladder region used across adapter tests. +const testRegion = "us-east-1" + +// --------------------------------------------------------------------------- +// spListerAdapter helpers +// --------------------------------------------------------------------------- + +// mockDescribeSP is a hermetic fake for activeSPListAPI. +// pages[0] is served on the first call (no incoming token). +// tokens[i] is the incoming NextToken that selects pages[i+1]. apiErr is +// returned on every call if non-nil. +type mockDescribeSP struct { + pages []*sdksp.DescribeSavingsPlansOutput + tokens []string // tokens[i] = incoming token that selects pages[i+1] + apiErr error + calls int +} + +func (m *mockDescribeSP) DescribeSavingsPlans( + _ context.Context, + params *sdksp.DescribeSavingsPlansInput, + _ ...func(*sdksp.Options), +) (*sdksp.DescribeSavingsPlansOutput, error) { + m.calls++ + if m.apiErr != nil { + return nil, m.apiErr + } + idx := 0 + incoming := aws.ToString(params.NextToken) + for i, tok := range m.tokens { + if tok == incoming { + idx = i + 1 + break + } + } + if idx >= len(m.pages) { + return nil, errors.New("unexpected page token in mockDescribeSP") + } + return m.pages[idx], nil +} + +// makeSPEntry builds a minimal DescribeSavingsPlans response entry. +// region is the SP's bound region ("" for global Compute SPs). +func makeSPEntry(id, planType, commitment, region string, state sptypes.SavingsPlanState) sptypes.SavingsPlan { + return sptypes.SavingsPlan{ + SavingsPlanId: aws.String(id), + SavingsPlanType: sptypes.SavingsPlanType(planType), + Commitment: aws.String(commitment), + Region: aws.String(region), + State: state, + Start: aws.String("2025-01-01T00:00:00Z"), + End: aws.String("2026-01-01T00:00:00Z"), + } +} + +// capturingSPAPI wraps an activeSPListAPI and records the States filter from +// each DescribeSavingsPlans call. Used to assert the states filter contract. +type capturingSPAPI struct { + inner activeSPListAPI + gotStates *[][]sptypes.SavingsPlanState +} + +func (c *capturingSPAPI) DescribeSavingsPlans( + ctx context.Context, + params *sdksp.DescribeSavingsPlansInput, + optFns ...func(*sdksp.Options), +) (*sdksp.DescribeSavingsPlansOutput, error) { + *c.gotStates = append(*c.gotStates, params.States) + return c.inner.DescribeSavingsPlans(ctx, params, optFns...) +} + +// newSPLister builds a region-scoped spListerAdapter over the given mock. +func newSPLister(api activeSPListAPI) *spListerAdapter { + return &spListerAdapter{api: api, region: testRegion} +} + +// --------------------------------------------------------------------------- +// spListerAdapter unit tests +// --------------------------------------------------------------------------- + +func TestSPLister_HappyPath(t *testing.T) { + sp := makeSPEntry("sp-abc", string(sptypes.SavingsPlanTypeCompute), "1.50", "", sptypes.SavingsPlanStateActive) + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{SavingsPlans: []sptypes.SavingsPlan{sp}}}, + } + lister := newSPLister(mock) + + got, err := lister.ListActiveSPs(context.Background()) + + require.NoError(t, err) + require.Len(t, got, 1) + assert.Equal(t, "sp-abc", got[0].PlanID) + assert.Equal(t, string(sptypes.SavingsPlanTypeCompute), got[0].PlanType) + assert.InDelta(t, 1.50, got[0].HourlyCommitmentUSD, 1e-9) + assert.Equal(t, string(sptypes.SavingsPlanStateActive), got[0].State) + assert.Equal(t, time.Date(2025, 1, 1, 0, 0, 0, 0, time.UTC), got[0].StartDate) + assert.Equal(t, time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), got[0].EndDate) +} + +// TestSPLister_StatesFilter verifies that DescribeSavingsPlans is called with +// exactly the typed SDK enums for active + payment-pending. payment-pending +// must be included so a just-purchased SP counts as existing commitment +// immediately (otherwise the next run double-purchases); queued stays +// excluded because a future-dated SP does not cover usage until its start. +func TestSPLister_StatesFilter(t *testing.T) { + var gotStates [][]sptypes.SavingsPlanState + inner := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{}}, + } + capturer := &capturingSPAPI{inner: inner, gotStates: &gotStates} + lister := newSPLister(capturer) + + _, err := lister.ListActiveSPs(context.Background()) + require.NoError(t, err) + require.Len(t, gotStates, 1, "exactly one API call on an empty account") + assert.Equal(t, []sptypes.SavingsPlanState{ + sptypes.SavingsPlanStateActive, + sptypes.SavingsPlanStatePaymentPending, + }, gotStates[0]) + assert.NotContains(t, gotStates[0], sptypes.SavingsPlanStateQueued, + "queued (future-dated) SPs must not be requested") +} + +// TestSPLister_PaymentPendingIncluded verifies a payment-pending SP is mapped +// and returned as existing commitment. +func TestSPLister_PaymentPendingIncluded(t *testing.T) { + sp := makeSPEntry("sp-pending", string(sptypes.SavingsPlanTypeCompute), "3.00", "", sptypes.SavingsPlanStatePaymentPending) + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{SavingsPlans: []sptypes.SavingsPlan{sp}}}, + } + lister := newSPLister(mock) + + got, err := lister.ListActiveSPs(context.Background()) + + require.NoError(t, err) + require.Len(t, got, 1, "payment-pending SP must count as existing commitment") + assert.Equal(t, string(sptypes.SavingsPlanStatePaymentPending), got[0].State) + assert.InDelta(t, 3.00, got[0].HourlyCommitmentUSD, 1e-9) +} + +// TestSPLister_OutOfRegionEC2InstanceExcluded verifies the region-scoping +// contract: an EC2Instance SP bound to another region is excluded (it covers +// that region's usage, and including it would inflate this region's +// ExistingUSDPerHour and under-purchase), while a global Compute SP is +// always included. +func TestSPLister_OutOfRegionEC2InstanceExcluded(t *testing.T) { + inRegion := makeSPEntry("sp-ec2-local", string(sptypes.SavingsPlanTypeEc2Instance), "1.00", testRegion, sptypes.SavingsPlanStateActive) + outOfRegion := makeSPEntry("sp-ec2-remote", string(sptypes.SavingsPlanTypeEc2Instance), "2.00", "eu-west-1", sptypes.SavingsPlanStateActive) + globalCompute := makeSPEntry("sp-compute", string(sptypes.SavingsPlanTypeCompute), "4.00", "", sptypes.SavingsPlanStateActive) + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{ + SavingsPlans: []sptypes.SavingsPlan{inRegion, outOfRegion, globalCompute}, + }}, + } + lister := newSPLister(mock) + + got, err := lister.ListActiveSPs(context.Background()) + + require.NoError(t, err) + require.Len(t, got, 2, "out-of-region EC2Instance SP must be excluded") + ids := []string{got[0].PlanID, got[1].PlanID} + assert.Contains(t, ids, "sp-ec2-local", "in-region EC2Instance SP kept") + assert.Contains(t, ids, "sp-compute", "global Compute SP kept in every region") + assert.NotContains(t, ids, "sp-ec2-remote", "eu-west-1 EC2Instance SP must not leak into us-east-1") +} + +// TestSPLister_PaginationExhausted verifies that two pages are fetched and +// merged (regression against issue #692 pattern: truncation at page 1). +func TestSPLister_PaginationExhausted(t *testing.T) { + sp1 := makeSPEntry("sp-1", string(sptypes.SavingsPlanTypeCompute), "1.00", "", sptypes.SavingsPlanStateActive) + sp2 := makeSPEntry("sp-2", string(sptypes.SavingsPlanTypeEc2Instance), "2.00", testRegion, sptypes.SavingsPlanStateActive) + pages := []*sdksp.DescribeSavingsPlansOutput{ + {SavingsPlans: []sptypes.SavingsPlan{sp1}, NextToken: aws.String("tok1")}, + {SavingsPlans: []sptypes.SavingsPlan{sp2}}, + } + mock := &mockDescribeSP{pages: pages, tokens: []string{"tok1"}} + lister := newSPLister(mock) + + got, err := lister.ListActiveSPs(context.Background()) + + require.NoError(t, err) + require.Len(t, got, 2, "both pages must be merged") + assert.Equal(t, "sp-1", got[0].PlanID) + assert.Equal(t, "sp-2", got[1].PlanID) + assert.Equal(t, 2, mock.calls, "two API calls expected") +} + +// TestSPLister_DeepPaginationNoCap verifies there is NO fixed page cap: +// DescribeSavingsPlans documents no page limit, so an account with more SPs +// than any arbitrary cap must still be fully listed (a truncated listing +// would understate existing commitment). 25 pages > the removed 20-page cap. +func TestSPLister_DeepPaginationNoCap(t *testing.T) { + const n = 25 + pages := make([]*sdksp.DescribeSavingsPlansOutput, n) + tokens := make([]string, n-1) + for i := 0; i < n; i++ { + sp := makeSPEntry(fmt.Sprintf("sp-%02d", i), string(sptypes.SavingsPlanTypeCompute), "1.00", "", sptypes.SavingsPlanStateActive) + out := &sdksp.DescribeSavingsPlansOutput{SavingsPlans: []sptypes.SavingsPlan{sp}} + if i < n-1 { + tok := fmt.Sprintf("tok%02d", i+1) + out.NextToken = aws.String(tok) + tokens[i] = tok + } + pages[i] = out + } + mock := &mockDescribeSP{pages: pages, tokens: tokens} + lister := newSPLister(mock) + + got, err := lister.ListActiveSPs(context.Background()) + + require.NoError(t, err, "deep pagination must not hit an artificial page cap") + assert.Len(t, got, n, "all %d pages must be listed", n) + assert.Equal(t, n, mock.calls) +} + +// TestSPLister_RepeatedTokenFails verifies the loop guard that replaced the +// page cap: a NextToken that repeats indicates API misbehavior and must abort +// with an error instead of looping forever. +func TestSPLister_RepeatedTokenFails(t *testing.T) { + pages := []*sdksp.DescribeSavingsPlansOutput{ + {NextToken: aws.String("tok1")}, + {NextToken: aws.String("tok1")}, // same token again -> loop + } + mock := &mockDescribeSP{pages: pages, tokens: []string{"tok1"}} + lister := newSPLister(mock) + + _, err := lister.ListActiveSPs(context.Background()) + + require.Error(t, err, "a repeated pagination token must abort the loop") + assert.Contains(t, err.Error(), "repeated pagination token") +} + +// TestSPLister_InvalidCommitmentFails verifies fail-loud on a non-numeric +// Commitment string (feedback_strict_int_parse, feedback_no_silent_fallbacks). +func TestSPLister_InvalidCommitmentFails(t *testing.T) { + sp := makeSPEntry("sp-bad", string(sptypes.SavingsPlanTypeCompute), "not-a-number", "", sptypes.SavingsPlanStateActive) + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{SavingsPlans: []sptypes.SavingsPlan{sp}}}, + } + lister := newSPLister(mock) + + _, err := lister.ListActiveSPs(context.Background()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot parse Commitment") +} + +// TestSPLister_BadDatesFail verifies fail-loud on missing or unparseable +// Start/End dates: a silently zero EndDate would drop the SP from +// sumExpiringSPHourlyCost and understate expiring commitment. +func TestSPLister_BadDatesFail(t *testing.T) { + base := func() sptypes.SavingsPlan { + return makeSPEntry("sp-dates", string(sptypes.SavingsPlanTypeCompute), "1.00", "", sptypes.SavingsPlanStateActive) + } + tests := []struct { + name string + mutate func(*sptypes.SavingsPlan) + wantMsg string + }{ + {"nil Start", func(sp *sptypes.SavingsPlan) { sp.Start = nil }, "nil Start date"}, + {"nil End", func(sp *sptypes.SavingsPlan) { sp.End = nil }, "nil End date"}, + {"garbage Start", func(sp *sptypes.SavingsPlan) { sp.Start = aws.String("garbage") }, "cannot parse Start date"}, + {"garbage End", func(sp *sptypes.SavingsPlan) { sp.End = aws.String("garbage") }, "cannot parse End date"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sp := base() + tt.mutate(&sp) + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{SavingsPlans: []sptypes.SavingsPlan{sp}}}, + } + lister := newSPLister(mock) + + _, err := lister.ListActiveSPs(context.Background()) + + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantMsg) + }) + } +} + +// TestSPLister_EmptyRegionEC2InstanceFails verifies fail-loud on an +// EC2Instance-plan SP with an empty Region: AWS always populates Region for +// region-bound plans, so an empty one means corrupted data. Letting it fall +// through to the region-scope filter would silently exclude it ("" != region), +// understating existing commitment and over-purchasing. +func TestSPLister_EmptyRegionEC2InstanceFails(t *testing.T) { + sp := makeSPEntry("sp-noregion", string(sptypes.SavingsPlanTypeEc2Instance), "1.00", "", sptypes.SavingsPlanStateActive) + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{SavingsPlans: []sptypes.SavingsPlan{sp}}}, + } + lister := newSPLister(mock) + + _, err := lister.ListActiveSPs(context.Background()) + + require.Error(t, err, "empty Region on an EC2Instance SP must fail loud, not be silently excluded") + assert.Contains(t, err.Error(), "empty Region") + assert.Contains(t, err.Error(), "sp-noregion", "error must name the offending SP") +} + +// TestSPLister_APIErrorPropagated verifies that a DescribeSavingsPlans error +// is propagated to the caller (no silent swallowing). +func TestSPLister_APIErrorPropagated(t *testing.T) { + sentinel := errors.New("DescribeSavingsPlans failed") + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{}}, + apiErr: sentinel, + } + lister := newSPLister(mock) + + _, err := lister.ListActiveSPs(context.Background()) + + require.Error(t, err) + assert.ErrorIs(t, err, sentinel) +} + +// TestSPLister_EmptyResult verifies that an account with no matching SPs +// returns an empty slice without error. +func TestSPLister_EmptyResult(t *testing.T) { + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{}}, + } + lister := newSPLister(mock) + + got, err := lister.ListActiveSPs(context.Background()) + + require.NoError(t, err) + assert.Empty(t, got) +} + +// TestSPLister_ContextCancelled verifies that a cancelled context terminates +// the listing loop before the first (or any subsequent) API call. +func TestSPLister_ContextCancelled(t *testing.T) { + pages := []*sdksp.DescribeSavingsPlansOutput{ + {NextToken: aws.String("tok1")}, + {}, + } + mock := &mockDescribeSP{pages: pages, tokens: []string{"tok1"}} + lister := newSPLister(mock) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := lister.ListActiveSPs(ctx) + require.Error(t, err, "cancelled context must produce an error") +} + +// TestSPLister_NilIDFails verifies fail-loud on a SP entry with a nil +// SavingsPlanId (un-identifiable entry; feedback_no_silent_fallbacks). +func TestSPLister_NilIDFails(t *testing.T) { + sp := sptypes.SavingsPlan{ + SavingsPlanId: nil, // nil: cannot identify + Commitment: aws.String("1.00"), + } + mock := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{SavingsPlans: []sptypes.SavingsPlan{sp}}}, + } + lister := newSPLister(mock) + + _, err := lister.ListActiveSPs(context.Background()) + + require.Error(t, err) + assert.Contains(t, err.Error(), "nil SavingsPlanId") +} + +// --------------------------------------------------------------------------- +// Direct mapping tests for the recommendations-backed adapters +// --------------------------------------------------------------------------- + +// ladderCEMock implements recommendations.CostExplorerAPI with canned +// responses so the ladder-side adapters can be exercised against a real +// *recommendations.Client without AWS. +type ladderCEMock struct { + costAndUsage *costexplorer.GetCostAndUsageOutput + spCoverage *costexplorer.GetSavingsPlansCoverageOutput + spUtilization *costexplorer.GetSavingsPlansUtilizationOutput +} + +func (m *ladderCEMock) GetReservationPurchaseRecommendation( + _ context.Context, _ *costexplorer.GetReservationPurchaseRecommendationInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetReservationPurchaseRecommendationOutput, error) { + return &costexplorer.GetReservationPurchaseRecommendationOutput{}, nil +} + +func (m *ladderCEMock) GetSavingsPlansPurchaseRecommendation( + _ context.Context, _ *costexplorer.GetSavingsPlansPurchaseRecommendationInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetSavingsPlansPurchaseRecommendationOutput, error) { + return &costexplorer.GetSavingsPlansPurchaseRecommendationOutput{}, nil +} + +func (m *ladderCEMock) GetReservationUtilization( + _ context.Context, _ *costexplorer.GetReservationUtilizationInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetReservationUtilizationOutput, error) { + return &costexplorer.GetReservationUtilizationOutput{}, nil +} + +func (m *ladderCEMock) GetReservationCoverage( + _ context.Context, _ *costexplorer.GetReservationCoverageInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetReservationCoverageOutput, error) { + return &costexplorer.GetReservationCoverageOutput{}, nil +} + +func (m *ladderCEMock) GetSavingsPlansCoverage( + _ context.Context, _ *costexplorer.GetSavingsPlansCoverageInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetSavingsPlansCoverageOutput, error) { + if m.spCoverage != nil { + return m.spCoverage, nil + } + return &costexplorer.GetSavingsPlansCoverageOutput{}, nil +} + +func (m *ladderCEMock) GetSavingsPlansUtilization( + _ context.Context, _ *costexplorer.GetSavingsPlansUtilizationInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetSavingsPlansUtilizationOutput, error) { + if m.spUtilization != nil { + return m.spUtilization, nil + } + return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil +} + +func (m *ladderCEMock) GetCostAndUsage( + _ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetCostAndUsageOutput, error) { + if m.costAndUsage != nil { + return m.costAndUsage, nil + } + return &costexplorer.GetCostAndUsageOutput{}, nil +} + +// ceDailyRow builds one GetCostAndUsage daily row keyed by the CamelCase +// "UnblendedCost" metric name. The literal MUST stay CamelCase: it mirrors +// what real CE echoes back for a GetCostAndUsage request (the enum-derived +// "UNBLENDED_COST" belongs to other CE APIs). If production regressed to the +// SCREAMING_SNAKE lookup, the r.Total lookup would miss this key and the +// mapping test below would fail with a missing-metric error. +func ceDailyRow(dateStr, amount string) cetypes.ResultByTime { + return cetypes.ResultByTime{ + TimePeriod: &cetypes.DateInterval{Start: aws.String(dateStr), End: aws.String(dateStr)}, + Total: map[string]cetypes.MetricValue{ + "UnblendedCost": {Amount: aws.String(amount)}, + }, + } +} + +// TestOnDemandSeriesAdapter_MapsDailyCostToDailyPoint verifies the direct +// []recommendations.DailyCost -> []DailyPoint mapping: dates and values are +// copied index-for-index, oldest-first. +func TestOnDemandSeriesAdapter_MapsDailyCostToDailyPoint(t *testing.T) { + mock := &ladderCEMock{costAndUsage: &costexplorer.GetCostAndUsageOutput{ + ResultsByTime: []cetypes.ResultByTime{ + ceDailyRow("2026-01-02", "480"), // $20/hr, newer + ceDailyRow("2026-01-01", "240"), // $10/hr, older + }, + }} + adapter := &onDemandSeriesAdapter{client: recommendations.NewClientWithAPI(mock, testRegion)} + + points, err := adapter.GetOnDemandSeries(context.Background(), testRegion, 7) + + require.NoError(t, err) + require.Len(t, points, 2) + assert.Equal(t, time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), points[0].Date, "oldest first") + assert.InDelta(t, 10.0, points[0].USDPerHour, 1e-9) + assert.Equal(t, time.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC), points[1].Date) + assert.InDelta(t, 20.0, points[1].USDPerHour, 1e-9) +} + +// TestOnDemandSeriesAdapter_PropagatesError verifies errors pass through the +// adapter unchanged (no silent fallback). +func TestOnDemandSeriesAdapter_PropagatesError(t *testing.T) { + // Empty CE output -> the client errors on the empty series. + adapter := &onDemandSeriesAdapter{client: recommendations.NewClientWithAPI(&ladderCEMock{}, testRegion)} + + _, err := adapter.GetOnDemandSeries(context.Background(), testRegion, 7) + + require.Error(t, err) + assert.Contains(t, err.Error(), "no on-demand data") +} + +// TestSPCoverageAdapter_MapsCoveragePct verifies the direct +// recommendations.SPCoverageSummary -> local SPCoverageSummary mapping with a +// populated CE response: covered $120 + on-demand $120 over 1 day = 50%. +func TestSPCoverageAdapter_MapsCoveragePct(t *testing.T) { + mock := &ladderCEMock{spCoverage: &costexplorer.GetSavingsPlansCoverageOutput{ + SavingsPlansCoverages: []cetypes.SavingsPlansCoverage{{ + Coverage: &cetypes.SavingsPlansCoverageData{ + SpendCoveredBySavingsPlans: aws.String("120"), + OnDemandCost: aws.String("120"), + }, + }}, + }} + adapter := &spCoverageAdapter{client: recommendations.NewClientWithAPI(mock, testRegion)} + + got, err := adapter.GetSPCoverageSummary(context.Background(), testRegion, 1) + + require.NoError(t, err) + require.NotNil(t, got.CoveragePct) + assert.InDelta(t, 50.0, *got.CoveragePct, 1e-9, "covered/(covered+onDemand) = 120/240 = 50%") +} + +// TestSPCoverageAdapter_NilWhenNoData verifies the nil-when-Days==0 contract: +// an empty CE coverage response maps to CoveragePct == nil ("not measured"), +// never a fabricated 0. +func TestSPCoverageAdapter_NilWhenNoData(t *testing.T) { + adapter := &spCoverageAdapter{client: recommendations.NewClientWithAPI(&ladderCEMock{}, testRegion)} + + got, err := adapter.GetSPCoverageSummary(context.Background(), testRegion, 30) + + require.NoError(t, err) + assert.Nil(t, got.CoveragePct, "no CE data must map to nil, not 0%") +} + +// TestSPUtilizationAdapter_MapsUtilizationPct verifies the direct +// recommendations.SPUtilizationSummary -> local SPUtilizationSummary mapping. +func TestSPUtilizationAdapter_MapsUtilizationPct(t *testing.T) { + mock := &ladderCEMock{spUtilization: &costexplorer.GetSavingsPlansUtilizationOutput{ + Total: &cetypes.SavingsPlansUtilizationAggregates{ + Utilization: &cetypes.SavingsPlansUtilization{ + UtilizationPercentage: aws.String("85"), + }, + }, + }} + adapter := &spUtilizationAdapter{client: recommendations.NewClientWithAPI(mock, testRegion)} + + got, err := adapter.GetSPUtilization(context.Background(), cetypes.SupportedSavingsPlansTypeComputeSp, "", 30) + + require.NoError(t, err) + require.NotNil(t, got.UtilizationPct) + assert.InDelta(t, 85.0, *got.UtilizationPct, 1e-9) +} + +// TestSPUtilizationAdapter_NilWhenNoData verifies nil propagation on an empty +// CE utilization response. +func TestSPUtilizationAdapter_NilWhenNoData(t *testing.T) { + adapter := &spUtilizationAdapter{client: recommendations.NewClientWithAPI(&ladderCEMock{}, testRegion)} + + got, err := adapter.GetSPUtilization(context.Background(), cetypes.SupportedSavingsPlansTypeComputeSp, "", 30) + + require.NoError(t, err) + assert.Nil(t, got.UtilizationPct, "no CE data must map to nil, not 0%") +} + +// --------------------------------------------------------------------------- +// Regression test: real spListerAdapter wired into New() must produce +// non-zero ExistingUSDPerHour for an active SP. +// Pre-fix behaviour: noop stub always returned 0.0. +// --------------------------------------------------------------------------- + +// TestGetLayerStates_RealSPLister_NonZeroExisting wires a real spListerAdapter +// backed by a hermetic DescribeSavingsPlans mock that returns one active +// Compute SP at $2/hr and asserts that GetLayerStates returns +// ExistingUSDPerHour == 2.0 for LayerComputeSP. +func TestGetLayerStates_RealSPLister_NonZeroExisting(t *testing.T) { + sp := makeSPEntry("sp-real", string(sptypes.SavingsPlanTypeCompute), "2.00", "", sptypes.SavingsPlanStateActive) + mockSPAPI := &mockDescribeSP{ + pages: []*sdksp.DescribeSavingsPlansOutput{{SavingsPlans: []sptypes.SavingsPlan{sp}}}, + } + + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(7)} + a, err := New( + Config{Region: testRegion, AccountID: "123456789012"}, + &fakeRILister{}, + newSPLister(mockSPAPI), // real adapter, not the noop stub + cov, + cov, + &fakeUtilizationSource{}, + nil, // spCoverageSource: not needed for this assertion + nil, // spUtilizationSource: not needed for this assertion + ) + require.NoError(t, err) + + states, err := a.GetLayerStates(context.Background(), testScope()) + + require.NoError(t, err) + computeSP, ok := states[pkgladder.LayerComputeSP] + require.True(t, ok, "LayerComputeSP must be present") + require.NotNil(t, computeSP.ExistingUSDPerHour, + "ExistingUSDPerHour must not be nil for a layer with an active SP") + assert.InDelta(t, 2.0, *computeSP.ExistingUSDPerHour, 1e-9, + "ExistingUSDPerHour must equal the SP $2/hr commitment (was 0.0 pre-fix)") +} diff --git a/providers/aws/ladder/factory.go b/providers/aws/ladder/factory.go index 9626cdcd3..57f3b802c 100644 --- a/providers/aws/ladder/factory.go +++ b/providers/aws/ladder/factory.go @@ -4,99 +4,87 @@ import ( "context" "fmt" + awsconfig "github.com/aws/aws-sdk-go-v2/config" + sdksp "github.com/aws/aws-sdk-go-v2/service/savingsplans" + pkgladder "github.com/LeanerCloud/CUDly/pkg/ladder" "github.com/LeanerCloud/CUDly/providers/aws/recommendations" ec2svc "github.com/LeanerCloud/CUDly/providers/aws/services/ec2" ) -// NewFromAWSConfig constructs an AWSLadder for the given region and accountID. -// All read-side data-source adapters that are not yet wired in PR-2 are replaced -// by no-op stubs: -// - RI and SP listers return empty slices (no existing commitments is safe for -// a plan-only run: the engine correctly shows the full gap as unhedged). -// - RI coverage and utilization sources return empty results. -// - The on-demand series source returns an explicit error so GetUsageBaseline -// fails loud; executeLadderRun returns before persisting anything, so the -// config is counted Errored for the run (no ladder_runs row is written). -// Real wiring arrives in PR-4 (Cost Explorer data source). +// NewFromAWSConfig constructs a fully wired AWSLadder for the given region and +// accountID. All seven read-side data-source adapters are wired to real AWS +// clients so that scheduled ladder runs produce meaningful plans. +// +// Pre-L2 behaviour: every source was a no-op stub; GetUsageBaseline always +// errored; every run was recorded as Errored with no plan produced. +// +// Client wiring (7 sources): +// +// - riLister : ec2svc.Client.ListConvertibleReservedInstances +// - spLister : spListerAdapter wrapping *sdksp.Client.DescribeSavingsPlans +// - riCoverageSource : recommendations.Client.GetRICoverageMap +// - onDemandSeries : onDemandSeriesAdapter wrapping recommendations.Client.GetOnDemandSeries +// (CE GetCostAndUsage; maps recommendations.DailyCost to DailyPoint) +// - utilizationSource : recommendations.Client.GetRIUtilization +// - spCoverageSource : spCoverageAdapter wrapping recommendations.Client.GetSPCoverageSummary +// - spUtilizationSource: spUtilizationAdapter wrapping recommendations.Client.GetSPUtilization // -// The write side (PurchaseLayer / ReshapeBuffer) is not wired here; all writes -// are rejected with errWriteNotWired, which is correct for the plan-only phase. +// The write side (PurchaseLayer / ReshapeBuffer) remains unwired; all writes +// are rejected with errWriteNotWired. Write-side wiring arrives in L6. // -// NewFromAWSConfig matches the LadderCapabilityFactory signature on Application -// so it can be assigned directly: +// NewFromAWSConfig matches the LadderCapabilityFactory type on Application so +// it can be assigned directly: // // app.LadderCapabilityFactory = awsladder.NewFromAWSConfig -func NewFromAWSConfig(_ context.Context, region, accountID string) (pkgladder.LadderCapability, error) { +func NewFromAWSConfig(ctx context.Context, region, accountID string) (pkgladder.LadderCapability, error) { if region == "" { return nil, fmt.Errorf("awsladder.NewFromAWSConfig: region must not be empty") } if accountID == "" { return nil, fmt.Errorf("awsladder.NewFromAWSConfig: accountID must not be empty") } + + // Load the ambient AWS credentials (Lambda execution role or env vars). + // The ladder region is set here so EC2 and Savings Plans clients use the + // correct regional endpoint. recommendations.NewClient overrides the region + // for Cost Explorer (always us-east-1) internally. + awsCfg, err := awsconfig.LoadDefaultConfig(ctx, + awsconfig.WithRegion(region), + ) + if err != nil { + return nil, fmt.Errorf("awsladder.NewFromAWSConfig: load AWS config: %w", err) + } + + // recommendations.Client satisfies riCoverageSource and utilizationSource + // directly, and is the underlying client for the on-demand series and SP + // coverage/utilization adapters. + recoClient := recommendations.NewClient(awsCfg) + + // ec2svc.Client satisfies riLister (ListConvertibleReservedInstances). + ec2Client := ec2svc.NewClient(awsCfg) + + // sdksp.Client satisfies activeSPListAPI (DescribeSavingsPlans), which is the + // narrow interface spListerAdapter expects (interface-segregation: we do not + // need the offering and purchase methods the full SavingsPlansAPI exposes). + spSDKClient := sdksp.NewFromConfig(awsCfg) + cfg := Config{ Region: region, AccountID: accountID, } l, err := New( cfg, - noopRILister{}, - noopSPLister{}, - noopRICoverageSource{}, - noopOnDemandSeriesSource{}, - noopUtilizationSource{}, - nil, // spCoverageSource: wired when PR-4 lands - nil, // spUtilizationSource: wired when PR-4 lands + ec2Client, // riLister + &spListerAdapter{api: spSDKClient, region: region}, // spLister (region-scoped) + recoClient, // riCoverageSource + &onDemandSeriesAdapter{client: recoClient}, // onDemandSeriesSource + recoClient, // utilizationSource + &spCoverageAdapter{client: recoClient}, // spCoverageSource + &spUtilizationAdapter{client: recoClient}, // spUtilizationSource ) if err != nil { return nil, fmt.Errorf("awsladder.NewFromAWSConfig: %w", err) } return l, nil } - -// noopRILister satisfies riLister by reporting no active convertible RIs. -// Returning an empty slice is safe: the engine counts ExistingUSDPerHour for -// RI layers as zero and plans the full gap. -type noopRILister struct{} - -func (noopRILister) ListConvertibleReservedInstances(_ context.Context) ([]ec2svc.ConvertibleRI, error) { - return []ec2svc.ConvertibleRI{}, nil -} - -// noopSPLister satisfies spLister by reporting no active Savings Plans. -// Returning an empty slice is safe for the same reason as noopRILister. -type noopSPLister struct{} - -func (noopSPLister) ListActiveSPs(_ context.Context) ([]ActiveSP, error) { - return []ActiveSP{}, nil -} - -// noopRICoverageSource satisfies riCoverageSource with an empty coverage map. -// The engine sets CoveragePct to nil for all RI pools when no coverage data is -// available, which is treated as "not yet measured" (not "zero coverage"). -type noopRICoverageSource struct{} - -func (noopRICoverageSource) GetRICoverageMap(_ context.Context, _ int, _ []string) (recommendations.PoolCoverageMap, error) { - return recommendations.PoolCoverageMap{}, nil -} - -// noopOnDemandSeriesSource satisfies onDemandSeriesSource by returning an -// explicit error. GetUsageBaseline requires a non-empty daily spend series to -// compute the low-water baseline; without it the engine cannot produce a -// meaningful plan. executeLadderRun returns early on the GetUsageBaseline -// error, so the config is counted Errored and no ladder_runs row is persisted. -// Real wiring (Cost Explorer adapter) arrives in PR-4. -type noopOnDemandSeriesSource struct{} - -func (noopOnDemandSeriesSource) GetOnDemandSeries(_ context.Context, _ string, _ int) ([]DailyPoint, error) { - return nil, fmt.Errorf("on-demand series source not yet wired: the Cost Explorer adapter is connected in PR-4; until then plan runs will be recorded as failed") -} - -// noopUtilizationSource satisfies utilizationSource by returning an empty -// slice. The engine treats nil/empty utilization as "not yet measured" and -// leaves UtilizationPct nil for RI layers. -type noopUtilizationSource struct{} - -func (noopUtilizationSource) GetRIUtilization(_ context.Context, _ int) ([]recommendations.RIUtilization, error) { - return []recommendations.RIUtilization{}, nil -} diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index 95102165f..7a1274f7f 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -30,6 +30,10 @@ type CostExplorerAPI interface { GetReservationCoverage(ctx context.Context, params *costexplorer.GetReservationCoverageInput, optFns ...func(*costexplorer.Options)) (*costexplorer.GetReservationCoverageOutput, error) GetSavingsPlansCoverage(ctx context.Context, params *costexplorer.GetSavingsPlansCoverageInput, optFns ...func(*costexplorer.Options)) (*costexplorer.GetSavingsPlansCoverageOutput, error) GetSavingsPlansUtilization(ctx context.Context, params *costexplorer.GetSavingsPlansUtilizationInput, optFns ...func(*costexplorer.Options)) (*costexplorer.GetSavingsPlansUtilizationOutput, error) + // GetCostAndUsage fetches cost and usage data for arbitrary time periods + // and granularities. Added for the daily on-demand series adapter that + // powers GetUsageBaseline (L2). + GetCostAndUsage(ctx context.Context, params *costexplorer.GetCostAndUsageInput, optFns ...func(*costexplorer.Options)) (*costexplorer.GetCostAndUsageOutput, error) } // Client wraps the AWS Cost Explorer client for RI recommendations diff --git a/providers/aws/recommendations/client_test.go b/providers/aws/recommendations/client_test.go index 2d83b44ab..54beed2de 100644 --- a/providers/aws/recommendations/client_test.go +++ b/providers/aws/recommendations/client_test.go @@ -60,6 +60,10 @@ func (m *mockCostExplorerAPI) GetSavingsPlansUtilization(ctx context.Context, pa return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil } +func (m *mockCostExplorerAPI) GetCostAndUsage(_ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options)) (*costexplorer.GetCostAndUsageOutput, error) { + return &costexplorer.GetCostAndUsageOutput{}, nil +} + func TestNewClient(t *testing.T) { cfg := aws.Config{ Region: "us-west-2", @@ -701,6 +705,12 @@ func (m *multiPageRIMock) GetSavingsPlansUtilization( return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil } +func (m *multiPageRIMock) GetCostAndUsage( + _ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetCostAndUsageOutput, error) { + return &costexplorer.GetCostAndUsageOutput{}, nil +} + // riDetail returns a minimal ReservationPurchaseRecommendation with n EC2 details. func riDetail(n int) types.ReservationPurchaseRecommendation { details := make([]types.ReservationPurchaseRecommendationDetail, n) @@ -833,6 +843,12 @@ func (m *alwaysNextPageRIMock) GetSavingsPlansUtilization( return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil } +func (m *alwaysNextPageRIMock) GetCostAndUsage( + _ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetCostAndUsageOutput, error) { + return &costexplorer.GetCostAndUsageOutput{}, nil +} + // TestGetRecommendations_RI_PaginationCapError asserts that exceeding // maxRecommendationPages returns a diagnostic error (issue #692). func TestGetRecommendations_RI_PaginationCapError(t *testing.T) { diff --git a/providers/aws/recommendations/ondemand_series.go b/providers/aws/recommendations/ondemand_series.go new file mode 100644 index 000000000..abbac66b4 --- /dev/null +++ b/providers/aws/recommendations/ondemand_series.go @@ -0,0 +1,302 @@ +package recommendations + +import ( + "context" + "fmt" + "sort" + "strconv" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/costexplorer" + "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" +) + +// ec2ComputeService is the CE SERVICE dimension value for EC2 compute. +// Matches the exact string CE uses; deviations silently return empty results. +const ec2ComputeService = "Amazon Elastic Compute Cloud - Compute" + +// purchaseTypeOnDemand is the CE PURCHASE_TYPE dimension value for on-demand +// instances. Filters to usage billed at on-demand rates (excludes RI-covered, +// SP-covered, and Spot). Using the uncovered on-demand spend as the baseline +// is conservative: existing coverage is separately counted by GetLayerStates, +// so the engine correctly nets it into the gap calculation. +const purchaseTypeOnDemand = "On Demand Instances" + +// onDemandMetric is the CE metric name for GetCostAndUsage. UnblendedCost is +// the actual charge to the account at the billed rate; for on-demand +// instances this equals on-demand rate x hours. +// +// The valid Metrics vocabulary for THIS operation is CamelCase +// ("AmortizedCost", "BlendedCost", "NetAmortizedCost", "NetUnblendedCost", +// "NormalizedUsageAmount", "UnblendedCost", "UsageQuantity") per the +// GetCostAndUsageInput.Metrics SDK doc (api_op_GetCostAndUsage.go). Do NOT +// substitute string(types.MetricUnblendedCost): that enum yields +// SCREAMING_SNAKE "UNBLENDED_COST", which belongs to other CE APIs +// (recommendation/utilization), and GetCostAndUsage rejects it with a +// ValidationException on every call (feedback_verify_api_filter_contracts: +// verify against the actual API op's doc, not a same-SDK enum). The same +// name keys the per-day r.Total lookup because CE echoes request metric +// names as response keys. +const onDemandMetric = "UnblendedCost" + +// ceDateLayout is the calendar-day format CE uses for DateInterval boundaries +// and ResultByTime period starts ("YYYY-MM-DD"). time.Parse with this layout +// yields midnight UTC, which is the normalization the ladder baseline expects. +const ceDateLayout = "2006-01-02" + +// maxOnDemandSeriesPages caps the GetCostAndUsage NextPageToken loop. +// Daily granularity over a 30-day window produces at most 1 page; the cap +// guards against a runaway token loop or API misbehavior and returns a +// diagnostic error instead of billing $0.01/call indefinitely. +const maxOnDemandSeriesPages = 20 + +// DailyCost is one calendar day of on-demand cost from CE GetCostAndUsage. +// Date is midnight UTC of the calendar day the entry covers; USDPerHour is +// the day's total unblended cost divided by 24. The ladder package maps this +// to its own DailyPoint type (identical fields) via a thin adapter; the type +// is duplicated because recommendations cannot import providers/aws/ladder +// (ladder already imports recommendations). +type DailyCost struct { + // Date is the UTC calendar day (midnight UTC) this entry covers. + Date time.Time + // USDPerHour is the on-demand-equivalent spend averaged over the day. + USDPerHour float64 +} + +// GetOnDemandSeries fetches daily on-demand EC2 compute cost for the given +// region over the past lookbackDays days from CE GetCostAndUsage and returns +// one DailyCost per returned calendar day, ordered oldest-to-newest with +// strictly increasing unique UTC days (map accumulation dedupes by date; +// lexicographic sort of "YYYY-MM-DD" keys is chronological). The last element +// is the most recent day CE has data for, which the ladder baseline's +// freshness check relies on. +// +// CE query parameters: +// - Granularity: DAILY (one entry per calendar day) +// - Metric: UnblendedCost (actual on-demand billed rate; see onDemandMetric) +// - Filter: SERVICE = EC2 compute AND PURCHASE_TYPE = On Demand Instances +// AND REGION = region (three-clause AND) +// - Time window: [now-lookbackDays, now) exclusive end +// +// Each day's total USD is divided by 24.0 to yield USD/hr. Dividing by 24 is +// derived from the DAILY granularity contract (one day = 24 hours), not a +// magic constant. +// +// CE typically lags ~24-48h, so the returned series may be shorter than +// lookbackDays. The caller (baseline.GetUsageBaseline) enforces minimum +// length, in-window coverage, and freshness. +// +// Fail-loud conditions (feedback_no_silent_fallbacks): +// - Empty result: error. +// - A result row missing the requested metric key (or with a nil Amount): +// error. CE echoes every requested metric on every row (genuine $0 days +// arrive as Amount:"0"), so a missing key means the request vocabulary is +// wrong, and fabricating a 0 would silently corrupt the baseline. +// - An all-zero series: error. When a filter or metric name is wrong, CE +// returns a complete, fresh, chronological series of $0 rows that would +// pass every downstream validation and make the engine size purchases +// from fabricated data. An account with genuinely zero on-demand EC2 +// spend for the whole lookback has nothing to ladder, so erroring is +// correct there too. +func (c *Client) GetOnDemandSeries(ctx context.Context, region string, lookbackDays int) ([]DailyCost, error) { + if err := validateOnDemandSeriesArgs(region, lookbackDays); err != nil { + return nil, err + } + + end := time.Now().UTC().Truncate(24 * time.Hour) // midnight today (exclusive end for CE) + start := end.AddDate(0, 0, -lookbackDays) + + input := &costexplorer.GetCostAndUsageInput{ + TimePeriod: &types.DateInterval{ + Start: aws.String(start.Format(ceDateLayout)), + End: aws.String(end.Format(ceDateLayout)), + }, + Granularity: types.GranularityDaily, + Metrics: []string{onDemandMetric}, + Filter: onDemandSeriesFilter(region), + } + + byDate := make(map[string]float64) + + var nextToken *string + page := 0 + for { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("GetOnDemandSeries: context cancelled during pagination: %w", err) + } + page++ + if page > maxOnDemandSeriesPages { + return nil, fmt.Errorf("GetOnDemandSeries: exceeded %d page cap; possible CE token loop", maxOnDemandSeriesPages) + } + input.NextPageToken = nextToken + + out, err := c.fetchOnDemandPage(ctx, input) + if err != nil { + return nil, fmt.Errorf("GetOnDemandSeries: %w", err) + } + + if err := accumulateDailyResults(byDate, out); err != nil { + return nil, fmt.Errorf("GetOnDemandSeries: %w", err) + } + + // aws.ToString returns "" for nil, so this single check covers both nil + // and empty-string tokens. + if aws.ToString(out.NextPageToken) == "" { + break + } + nextToken = out.NextPageToken + } + + series, err := buildDailySeries(byDate, region, lookbackDays) + if err != nil { + return nil, fmt.Errorf("GetOnDemandSeries: %w", err) + } + return series, nil +} + +// validateOnDemandSeriesArgs rejects out-of-range GetOnDemandSeries arguments +// at the boundary. Extracted to keep GetOnDemandSeries under the cyclomatic +// complexity limit. +func validateOnDemandSeriesArgs(region string, lookbackDays int) error { + if lookbackDays <= 0 { + return fmt.Errorf("GetOnDemandSeries: lookbackDays must be > 0, got %d", lookbackDays) + } + if region == "" { + return fmt.Errorf("GetOnDemandSeries: region must not be empty") + } + return nil +} + +// fetchOnDemandPage calls GetCostAndUsage with rate-limit retry, mirroring +// the fetchCoveragePage / fetchUtilizationPage pattern in coverage.go and +// utilization.go. Each attempt acquires one rate-limiter slot at the API call +// site (not at goroutine creation) per feedback_semaphore_at_api_call. +func (c *Client) fetchOnDemandPage(ctx context.Context, input *costexplorer.GetCostAndUsageInput) (*costexplorer.GetCostAndUsageOutput, error) { + c.rateLimiter.Reset() + for { + if waitErr := c.rateLimiter.Wait(ctx); waitErr != nil { + return nil, fmt.Errorf("rate limiter wait: %w", waitErr) + } + out, err := c.costExplorerClient.GetCostAndUsage(ctx, input) + if !c.rateLimiter.ShouldRetry(err) { + if err != nil { + return nil, fmt.Errorf("GetCostAndUsage: %w", err) + } + return out, nil + } + } +} + +// onDemandSeriesFilter builds the three-clause AND filter for +// GetCostAndUsage: EC2 compute service, on-demand purchase type, and the +// given region. All three clauses are required: +// - SERVICE scopes to EC2 compute (excludes RDS, ElastiCache, etc.). +// - PURCHASE_TYPE scopes to on-demand charges (excludes RI-covered, +// SP-covered, and Spot; those are accounted for via GetLayerStates). +// - REGION scopes to the ladder's configured region. +// +// Per feedback_verify_api_filter_contracts: GetCostAndUsage supports SERVICE, +// PURCHASE_TYPE, and REGION as valid filter dimensions (CE API reference, +// GetCostAndUsageInput.Filter). SAVINGS_PLANS_TYPE is NOT supported here +// (that is a GetSavingsPlansCoverage/Utilization dimension only). +func onDemandSeriesFilter(region string) *types.Expression { + return &types.Expression{ + And: []types.Expression{ + {Dimensions: &types.DimensionValues{ + Key: types.DimensionService, + Values: []string{ec2ComputeService}, + }}, + {Dimensions: &types.DimensionValues{ + Key: types.DimensionPurchaseType, + Values: []string{purchaseTypeOnDemand}, + }}, + {Dimensions: &types.DimensionValues{ + Key: types.DimensionRegion, + Values: []string{region}, + }}, + }, + } +} + +// accumulateDailyResults extracts daily USD/hr values from one page of +// GetCostAndUsage results and merges them into byDate. All malformed rows +// fail loud (feedback_no_silent_fallbacks): +// - A row missing its TimePeriod/Start cannot be dated; silently skipping +// it would leave an incomplete series that can still pass downstream +// minimum-length checks and produce an incorrect baseline. +// - A row missing the requested metric key (or carrying a nil Amount) +// signals a request-vocabulary bug: CE echoes every requested metric on +// every returned row, with genuine $0 days arriving as Amount:"0"; +// writing a fabricated 0 would silently corrupt the baseline. +// - Unparseable amount strings fail loud (feedback_strict_int_parse). +// +// Dividing by 24 converts daily USD to USD/hr; 24 is derived from the DAILY +// granularity contract (one day = 24 hours), not a magic constant. +func accumulateDailyResults(byDate map[string]float64, out *costexplorer.GetCostAndUsageOutput) error { + for i, r := range out.ResultsByTime { + if r.TimePeriod == nil || r.TimePeriod.Start == nil { + return fmt.Errorf("CE result row %d is missing its period start; a row that cannot be dated would leave an undetectable gap in the daily series", i) + } + dateStr := aws.ToString(r.TimePeriod.Start) + mv, ok := r.Total[onDemandMetric] + if !ok || mv.Amount == nil { + return fmt.Errorf("CE result row for day %s is missing the %q metric; CE echoes every requested metric on every row, so this indicates a request-vocabulary bug (genuine $0 days arrive as Amount:\"0\")", + dateStr, onDemandMetric) + } + usd, err := strconv.ParseFloat(aws.ToString(mv.Amount), 64) + if err != nil { + return fmt.Errorf("cannot parse CE amount %q for day %s: %w", + aws.ToString(mv.Amount), dateStr, err) + } + // Divide total daily USD by 24 to get USD/hr. Derived from DAILY + // granularity: 1 day = 24 hours; not a magic constant. + byDate[dateStr] = usd / 24.0 + } + return nil +} + +// buildDailySeries converts the date-keyed cost map into the final validated +// []DailyCost: +// - Empty map: error (no data from CE). +// - Sorted oldest-to-newest (lexicographic sort on "2006-01-02" strings is +// chronological). Map keys are unique, so the result has strictly +// increasing unique UTC days, which the ladder baseline's chronology +// check requires. +// - A date string that does not parse fails loud: it indicates a CE +// contract violation the caller must see (feedback_no_silent_fallbacks). +// - An all-zero series fails loud: a wrong filter or metric name makes CE +// return a complete series of $0 rows that would pass every downstream +// validation and let the engine size purchases from fabricated data. An +// account with genuinely zero on-demand EC2 spend over the whole window +// has nothing to ladder, so erroring is correct in that case too. +func buildDailySeries(byDate map[string]float64, region string, lookbackDays int) ([]DailyCost, error) { + if len(byDate) == 0 { + return nil, fmt.Errorf("CE returned no on-demand data for region %q over the past %d days (account may have no on-demand EC2 spend, or CE data not yet available)", + region, lookbackDays) + } + + dates := make([]string, 0, len(byDate)) + for d := range byDate { + dates = append(dates, d) + } + sort.Strings(dates) + + allZero := true + series := make([]DailyCost, len(dates)) + for i, d := range dates { + day, err := time.Parse(ceDateLayout, d) + if err != nil { + return nil, fmt.Errorf("cannot parse CE period start date %q: %w", d, err) + } + if byDate[d] != 0 { + allZero = false + } + series[i] = DailyCost{Date: day, USDPerHour: byDate[d]} + } + if allZero { + return nil, fmt.Errorf("CE returned an all-zero on-demand series for region %q over the past %d days; either the account has no on-demand EC2 spend to ladder, or the CE filter/metric vocabulary is wrong (a bad filter yields complete $0 rows, not an empty result)", + region, lookbackDays) + } + return series, nil +} diff --git a/providers/aws/recommendations/ondemand_series_test.go b/providers/aws/recommendations/ondemand_series_test.go new file mode 100644 index 000000000..b5f8d582c --- /dev/null +++ b/providers/aws/recommendations/ondemand_series_test.go @@ -0,0 +1,435 @@ +package recommendations + +import ( + "context" + "errors" + "fmt" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/costexplorer" + "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// mockOnDemandCE is a hermetic mock for GetCostAndUsage. Only GetCostAndUsage +// is behaviourally wired; all other CostExplorerAPI methods are stubs that +// return empty non-nil outputs (the base mockCostExplorerAPI already provides +// those stubs via embedding). +type mockOnDemandCE struct { + mockCostExplorerAPI // provides all stub methods except GetCostAndUsage + pages []*costexplorer.GetCostAndUsageOutput + tokens []string // tokens[i] triggers pages[i+1] + gotInputs []*costexplorer.GetCostAndUsageInput + apiErr error +} + +func (m *mockOnDemandCE) GetCostAndUsage( + _ context.Context, + params *costexplorer.GetCostAndUsageInput, + _ ...func(*costexplorer.Options), +) (*costexplorer.GetCostAndUsageOutput, error) { + m.gotInputs = append(m.gotInputs, params) + if m.apiErr != nil { + return nil, m.apiErr + } + // Determine which page to return based on the incoming NextPageToken. + idx := 0 + incoming := aws.ToString(params.NextPageToken) + for i, tok := range m.tokens { + if tok == incoming { + idx = i + 1 + break + } + } + if idx >= len(m.pages) { + return nil, fmt.Errorf("unexpected page token %q", incoming) + } + return m.pages[idx], nil +} + +// dailyResult builds one ResultByTime entry for testing. +// dateStr must be in "2006-01-02" format; totalUSD is the day's UnblendedCost. +func dailyResult(dateStr string, totalUSD float64) types.ResultByTime { + return types.ResultByTime{ + TimePeriod: &types.DateInterval{ + Start: aws.String(dateStr), + End: aws.String(dateStr), // end is exclusive, not relevant for mock + }, + Total: map[string]types.MetricValue{ + onDemandMetric: {Amount: aws.String(fmt.Sprintf("%.10f", totalUSD))}, + }, + } +} + +// dailyResultNoMetric builds a ResultByTime entry with a missing metric key, +// exercising the missing-metric error path (production fails loud rather +// than fabricating a $0 day). +func dailyResultNoMetric(dateStr string) types.ResultByTime { + return types.ResultByTime{ + TimePeriod: &types.DateInterval{Start: aws.String(dateStr)}, + Total: map[string]types.MetricValue{}, + } +} + +// newOnDemandClient creates a recommendations.Client wired to the given mock. +func newOnDemandClient(m *mockOnDemandCE) *Client { + return NewClientWithAPI(m, "us-east-1") +} + +// utcDate parses a "2006-01-02" string into midnight UTC, failing the test +// on malformed input. Mirrors the production ceDateLayout parse so date +// assertions compare like with like. +func utcDate(t *testing.T, dateStr string) time.Time { + t.Helper() + d, err := time.Parse(ceDateLayout, dateStr) + require.NoError(t, err) + return d +} + +// requireStrictlyIncreasingDays asserts the series dates are unique ascending +// UTC calendar days (the ladder baseline's chronology contract). +func requireStrictlyIncreasingDays(t *testing.T, series []DailyCost) { + t.Helper() + for i := 1; i < len(series); i++ { + require.True(t, series[i].Date.After(series[i-1].Date), + "series dates must be strictly increasing: index %d (%s) not after index %d (%s)", + i, series[i].Date.Format(ceDateLayout), i-1, series[i-1].Date.Format(ceDateLayout)) + } +} + +// generate30DayPage builds a single-page mock returning 30 daily entries, +// each billing totalPerDay USD total (i.e. totalPerDay/24 USD/hr per entry). +func generate30DayPage(startDate time.Time, totalPerDay float64) []*costexplorer.GetCostAndUsageOutput { + results := make([]types.ResultByTime, 30) + for i := range results { + day := startDate.AddDate(0, 0, i) + results[i] = dailyResult(day.Format(ceDateLayout), totalPerDay) + } + return []*costexplorer.GetCostAndUsageOutput{ + {ResultsByTime: results}, + } +} + +// TestGetOnDemandSeries_HappyPath verifies that 30 daily entries at +// $240/day each return 30 dated points of $10/hr ($240/24h) with strictly +// increasing unique UTC days and a fresh (most recent CE day) tail. +func TestGetOnDemandSeries_HappyPath(t *testing.T) { + start := time.Now().UTC().Truncate(24*time.Hour).AddDate(0, 0, -30) + mock := &mockOnDemandCE{pages: generate30DayPage(start, 240.0)} + client := newOnDemandClient(mock) + + series, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 30) + + require.NoError(t, err) + require.Len(t, series, 30, "series must have one element per returned day") + for i, p := range series { + assert.InDelta(t, 10.0, p.USDPerHour, 1e-9, "day %d: expected $10/hr ($240/24h)", i) + assert.Equal(t, start.AddDate(0, 0, i), p.Date, + "day %d: Date must be the UTC day from the CE period start", i) + } + requireStrictlyIncreasingDays(t, series) + // Freshness contract: the last point is the most recent CE day (start+29 + // = yesterday), within the baseline's maxSeriesAgeDays. + assert.Equal(t, start.AddDate(0, 0, 29), series[len(series)-1].Date, + "last point must be the most recent CE day") +} + +// TestGetOnDemandSeries_OldestFirst verifies that days are returned in +// chronological (oldest-to-newest) order by Date regardless of the order CE +// returned them in. +func TestGetOnDemandSeries_OldestFirst(t *testing.T) { + // Provide 3 days with distinct costs so order is detectable. + pages := []*costexplorer.GetCostAndUsageOutput{{ + ResultsByTime: []types.ResultByTime{ + dailyResult("2026-01-03", 72.0), // $3/hr + dailyResult("2026-01-01", 24.0), // $1/hr + dailyResult("2026-01-02", 48.0), // $2/hr + }, + }} + mock := &mockOnDemandCE{pages: pages} + client := newOnDemandClient(mock) + + series, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.NoError(t, err) + require.Len(t, series, 3) + assert.Equal(t, utcDate(t, "2026-01-01"), series[0].Date, "oldest day first") + assert.InDelta(t, 1.0, series[0].USDPerHour, 1e-9) + assert.Equal(t, utcDate(t, "2026-01-02"), series[1].Date, "middle day second") + assert.InDelta(t, 2.0, series[1].USDPerHour, 1e-9) + assert.Equal(t, utcDate(t, "2026-01-03"), series[2].Date, "newest day last") + assert.InDelta(t, 3.0, series[2].USDPerHour, 1e-9) + requireStrictlyIncreasingDays(t, series) +} + +// TestGetOnDemandSeries_Paginated verifies that two pages are fetched and +// their results merged into a single date-sorted series. +func TestGetOnDemandSeries_Paginated(t *testing.T) { + pages := []*costexplorer.GetCostAndUsageOutput{ + { + ResultsByTime: []types.ResultByTime{ + dailyResult("2026-01-01", 24.0), + dailyResult("2026-01-02", 48.0), + }, + NextPageToken: aws.String("tok1"), + }, + { + ResultsByTime: []types.ResultByTime{ + dailyResult("2026-01-03", 72.0), + }, + }, + } + mock := &mockOnDemandCE{pages: pages, tokens: []string{"tok1"}} + client := newOnDemandClient(mock) + + series, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.NoError(t, err) + require.Len(t, series, 3, "both pages must be merged") + assert.Equal(t, utcDate(t, "2026-01-01"), series[0].Date) + assert.InDelta(t, 1.0, series[0].USDPerHour, 1e-9) + assert.Equal(t, utcDate(t, "2026-01-02"), series[1].Date) + assert.InDelta(t, 2.0, series[1].USDPerHour, 1e-9) + assert.Equal(t, utcDate(t, "2026-01-03"), series[2].Date) + assert.InDelta(t, 3.0, series[2].USDPerHour, 1e-9) + requireStrictlyIncreasingDays(t, series) + // Verify exactly 2 CE calls were made (one per page). + assert.Len(t, mock.gotInputs, 2, "should have fetched 2 pages") + // Second call must carry the token from the first response. + assert.Equal(t, "tok1", aws.ToString(mock.gotInputs[1].NextPageToken)) +} + +// TestGetOnDemandSeries_EmptyResultErrors verifies a hard error when CE +// returns zero daily entries (no on-demand spend or CE not yet available). +func TestGetOnDemandSeries_EmptyResultErrors(t *testing.T) { + mock := &mockOnDemandCE{pages: []*costexplorer.GetCostAndUsageOutput{{}}} + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 30) + + require.Error(t, err) + assert.Contains(t, err.Error(), "no on-demand data", "error should describe the empty-data condition") +} + +// TestGetOnDemandSeries_APIErrorPropagated verifies that a CE API error is +// returned to the caller without swallowing. +func TestGetOnDemandSeries_APIErrorPropagated(t *testing.T) { + sentinel := errors.New("CE rate limit") + mock := &mockOnDemandCE{ + pages: []*costexplorer.GetCostAndUsageOutput{{}}, + apiErr: sentinel, + } + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 30) + + require.Error(t, err) + assert.ErrorIs(t, err, sentinel) +} + +// TestGetOnDemandSeries_ParseErrorFails verifies that a non-numeric Amount +// from CE triggers a fail-loud error (feedback_strict_int_parse). +func TestGetOnDemandSeries_ParseErrorFails(t *testing.T) { + pages := []*costexplorer.GetCostAndUsageOutput{{ + ResultsByTime: []types.ResultByTime{{ + TimePeriod: &types.DateInterval{Start: aws.String("2026-01-01")}, + Total: map[string]types.MetricValue{onDemandMetric: {Amount: aws.String("not-a-number")}}, + }}, + }} + mock := &mockOnDemandCE{pages: pages} + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot parse CE amount") +} + +// TestGetOnDemandSeries_BadDateFails verifies that a malformed CE period-start +// date fails loud instead of being skipped or misdated +// (feedback_no_silent_fallbacks). +func TestGetOnDemandSeries_BadDateFails(t *testing.T) { + pages := []*costexplorer.GetCostAndUsageOutput{{ + ResultsByTime: []types.ResultByTime{ + dailyResult("not-a-date", 24.0), + }, + }} + mock := &mockOnDemandCE{pages: pages} + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.Error(t, err) + assert.Contains(t, err.Error(), "cannot parse CE period start date") +} + +// TestGetOnDemandSeries_CorrectFilterParams verifies the CE query uses the +// correct granularity, metric, and three-clause AND filter +// (feedback_verify_api_filter_contracts). +func TestGetOnDemandSeries_CorrectFilterParams(t *testing.T) { + start := time.Now().UTC().Truncate(24*time.Hour).AddDate(0, 0, -7) + mock := &mockOnDemandCE{pages: generate30DayPage(start, 24.0)} + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "eu-west-1", 7) + require.NoError(t, err) + require.NotEmpty(t, mock.gotInputs, "at least one CE call must have been made") + + input := mock.gotInputs[0] + + assert.Equal(t, types.GranularityDaily, input.Granularity, + "granularity must be DAILY for the daily series") + // The literal CamelCase value is asserted on purpose: GetCostAndUsage's + // Metrics vocabulary is CamelCase ("UnblendedCost"), NOT the + // types.MetricUnblendedCost enum's "UNBLENDED_COST" (that enum belongs to + // other CE APIs and triggers a ValidationException here). Locking the + // literal prevents a "cleanup" back to the enum from passing tests. + assert.Equal(t, []string{"UnblendedCost"}, input.Metrics, + "metric must be the CamelCase UnblendedCost accepted by GetCostAndUsage") + + // TimePeriod contract: [today-lookbackDays, today) with an exclusive end + // at midnight UTC today, so today's partial data is never included. + wantEnd := time.Now().UTC().Truncate(24 * time.Hour) + wantStart := wantEnd.AddDate(0, 0, -7) + require.NotNil(t, input.TimePeriod) + assert.Equal(t, wantStart.Format(ceDateLayout), aws.ToString(input.TimePeriod.Start), + "start must be today-lookbackDays (UTC)") + assert.Equal(t, wantEnd.Format(ceDateLayout), aws.ToString(input.TimePeriod.End), + "end must be midnight UTC today (exclusive: today's partial day excluded)") + + require.NotNil(t, input.Filter, "filter must be present") + require.Len(t, input.Filter.And, 3, "filter must be a three-clause AND") + + // Extract dimension keys from the AND clauses. + dimKeys := make([]types.Dimension, 0, 3) + dimVals := make(map[types.Dimension][]string, 3) + for _, clause := range input.Filter.And { + require.NotNil(t, clause.Dimensions) + dimKeys = append(dimKeys, clause.Dimensions.Key) + dimVals[clause.Dimensions.Key] = clause.Dimensions.Values + } + assert.Contains(t, dimKeys, types.DimensionService, "SERVICE dimension required") + assert.Contains(t, dimKeys, types.DimensionPurchaseType, "PURCHASE_TYPE dimension required") + assert.Contains(t, dimKeys, types.DimensionRegion, "REGION dimension required") + + assert.Equal(t, []string{ec2ComputeService}, dimVals[types.DimensionService]) + assert.Equal(t, []string{purchaseTypeOnDemand}, dimVals[types.DimensionPurchaseType]) + assert.Equal(t, []string{"eu-west-1"}, dimVals[types.DimensionRegion]) +} + +// TestGetOnDemandSeries_ContextCancelled verifies that a cancelled context is +// propagated before the first CE call (ctx-cancel-is-terminal rule). +func TestGetOnDemandSeries_ContextCancelled(t *testing.T) { + mock := &mockOnDemandCE{pages: generate30DayPage(time.Now(), 24.0)} + client := newOnDemandClient(mock) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() // cancel immediately before the call + + _, err := client.GetOnDemandSeries(ctx, "us-east-1", 30) + + require.Error(t, err) +} + +// TestGetOnDemandSeries_MissingMetricKeyFails verifies that a result row +// missing the requested metric key fails loud instead of fabricating a $0 +// day. CE echoes every requested metric on every row (genuine $0 days arrive +// as Amount:"0"), so a missing key means the request vocabulary is wrong; +// silently writing 0 would corrupt the baseline +// (feedback_no_silent_fallbacks). +func TestGetOnDemandSeries_MissingMetricKeyFails(t *testing.T) { + pages := []*costexplorer.GetCostAndUsageOutput{{ + ResultsByTime: []types.ResultByTime{ + dailyResult("2026-01-01", 24.0), + dailyResultNoMetric("2026-01-02"), // missing metric -> hard error + dailyResult("2026-01-03", 48.0), + }, + }} + mock := &mockOnDemandCE{pages: pages} + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.Error(t, err, "a missing metric key must fail loud, not fabricate $0") + assert.Contains(t, err.Error(), "missing the \"UnblendedCost\" metric") + assert.Contains(t, err.Error(), "2026-01-02", "error must name the offending day") +} + +// TestGetOnDemandSeries_MissingPeriodStartFails verifies that a result row +// missing its TimePeriod/Start fails loud instead of being silently skipped: +// an undatable row dropped from the series leaves a gap that can still pass +// downstream minimum-length checks and produce an incorrect baseline. +func TestGetOnDemandSeries_MissingPeriodStartFails(t *testing.T) { + pages := []*costexplorer.GetCostAndUsageOutput{{ + ResultsByTime: []types.ResultByTime{ + dailyResult("2026-01-01", 24.0), + { // undatable row: nil TimePeriod + TimePeriod: nil, + Total: map[string]types.MetricValue{onDemandMetric: {Amount: aws.String("48")}}, + }, + }, + }} + mock := &mockOnDemandCE{pages: pages} + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.Error(t, err, "a row missing its period start must fail loud, not be skipped") + assert.Contains(t, err.Error(), "missing its period start") + assert.Contains(t, err.Error(), "row 1", "error must name the offending row index") +} + +// TestGetOnDemandSeries_AllZeroSeriesFails verifies that a complete series of +// $0 rows is rejected. When a filter or metric name is wrong, CE returns +// every day as a $0 row (not an empty result), producing a fresh, +// chronological all-zero series that would pass every downstream validation +// and make the engine size purchases from fabricated data. An account with +// genuinely zero on-demand spend all window has nothing to ladder, so +// erroring is correct there too. +func TestGetOnDemandSeries_AllZeroSeriesFails(t *testing.T) { + pages := []*costexplorer.GetCostAndUsageOutput{{ + ResultsByTime: []types.ResultByTime{ + dailyResult("2026-01-01", 0), + dailyResult("2026-01-02", 0), + dailyResult("2026-01-03", 0), + }, + }} + mock := &mockOnDemandCE{pages: pages} + client := newOnDemandClient(mock) + + _, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.Error(t, err, "an all-zero series must be rejected") + assert.Contains(t, err.Error(), "all-zero") +} + +// TestGetOnDemandSeries_MixedZeroDaysOK verifies that genuine $0 days +// (Amount:"0") interleaved with non-zero days are accepted and keep their +// calendar dates: only the ALL-zero case is rejected. +func TestGetOnDemandSeries_MixedZeroDaysOK(t *testing.T) { + pages := []*costexplorer.GetCostAndUsageOutput{{ + ResultsByTime: []types.ResultByTime{ + dailyResult("2026-01-01", 24.0), + dailyResult("2026-01-02", 0), // genuine $0 day: Amount:"0" + dailyResult("2026-01-03", 48.0), + }, + }} + mock := &mockOnDemandCE{pages: pages} + client := newOnDemandClient(mock) + + series, err := client.GetOnDemandSeries(context.Background(), "us-east-1", 7) + + require.NoError(t, err, "genuine zero days mixed with spend must be accepted") + require.Len(t, series, 3) + assert.Equal(t, utcDate(t, "2026-01-01"), series[0].Date) + assert.InDelta(t, 1.0, series[0].USDPerHour, 1e-9) + assert.Equal(t, utcDate(t, "2026-01-02"), series[1].Date, + "the zero-spend day must keep its calendar date") + assert.InDelta(t, 0.0, series[1].USDPerHour, 1e-9) + assert.Equal(t, utcDate(t, "2026-01-03"), series[2].Date) + assert.InDelta(t, 2.0, series[2].USDPerHour, 1e-9) + requireStrictlyIncreasingDays(t, series) +} diff --git a/providers/aws/recommendations/parser_sp_additional_test.go b/providers/aws/recommendations/parser_sp_additional_test.go index 25252d13f..70f864d22 100644 --- a/providers/aws/recommendations/parser_sp_additional_test.go +++ b/providers/aws/recommendations/parser_sp_additional_test.go @@ -253,6 +253,10 @@ func (m *mockCostExplorerForSP) GetSavingsPlansUtilization(_ context.Context, _ return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil } +func (m *mockCostExplorerForSP) GetCostAndUsage(_ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options)) (*costexplorer.GetCostAndUsageOutput, error) { + return &costexplorer.GetCostAndUsageOutput{}, nil +} + func TestGetSavingsPlansRecommendations_WithFilters(t *testing.T) { mockAPI := &mockCostExplorerForSP{ responses: map[types.SupportedSavingsPlansType]*costexplorer.GetSavingsPlansPurchaseRecommendationOutput{ @@ -454,6 +458,12 @@ func (m *multiPageSPMock) GetSavingsPlansUtilization( return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil } +func (m *multiPageSPMock) GetCostAndUsage( + _ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetCostAndUsageOutput, error) { + return &costexplorer.GetCostAndUsageOutput{}, nil +} + // alwaysNextPageSPMock returns pages each carrying a non-nil non-empty NextPageToken. type alwaysNextPageSPMock struct { calls int @@ -498,6 +508,12 @@ func (m *alwaysNextPageSPMock) GetSavingsPlansUtilization( return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil } +func (m *alwaysNextPageSPMock) GetCostAndUsage( + _ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options), +) (*costexplorer.GetCostAndUsageOutput, error) { + return &costexplorer.GetCostAndUsageOutput{}, nil +} + // TestGetSavingsPlansRecommendations_Paginates asserts multi-page accumulation (issue #692). func TestGetSavingsPlansRecommendations_Paginates(t *testing.T) { mock := &multiPageSPMock{ diff --git a/providers/aws/service_client_test.go b/providers/aws/service_client_test.go index 35ae91c18..8df394714 100644 --- a/providers/aws/service_client_test.go +++ b/providers/aws/service_client_test.go @@ -44,6 +44,10 @@ func (m *mockCostExplorerClient) GetSavingsPlansUtilization(ctx context.Context, return &costexplorer.GetSavingsPlansUtilizationOutput{}, nil } +func (m *mockCostExplorerClient) GetCostAndUsage(_ context.Context, _ *costexplorer.GetCostAndUsageInput, _ ...func(*costexplorer.Options)) (*costexplorer.GetCostAndUsageOutput, error) { + return &costexplorer.GetCostAndUsageOutput{}, nil +} + // newTestRecommendationsClient creates a recommendations client with a mock CE client func newTestRecommendationsClient(ce *mockCostExplorerClient) *recommendations.Client { return recommendations.NewClientWithAPI(ce, "us-east-1")