From 1bf6d0e4a54c7aaf7e72c37ff1069cf024e2c69c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 15 Oct 2026 11:31:20 +0200 Subject: [PATCH] fix(aws): scope ladder residual baselines to account Keep other accounts and incomplete daily observations out of the ladder residual percentile while preserving measured zero days. Refs #282 --- providers/aws/ladder/adapters.go | 5 +- providers/aws/ladder/adapters_test.go | 16 +- providers/aws/ladder/baseline.go | 23 ++- providers/aws/ladder/baseline_sdk_test.go | 182 ++++++++++++++++++ providers/aws/ladder/factory.go | 2 +- providers/aws/ladder/ladder_test.go | 16 +- .../aws/recommendations/ondemand_series.go | 65 +++++-- .../recommendations/ondemand_series_test.go | 41 ++++ 8 files changed, 309 insertions(+), 41 deletions(-) create mode 100644 providers/aws/ladder/baseline_sdk_test.go diff --git a/providers/aws/ladder/adapters.go b/providers/aws/ladder/adapters.go index 489aa4c0..75cb84d0 100644 --- a/providers/aws/ladder/adapters.go +++ b/providers/aws/ladder/adapters.go @@ -213,14 +213,15 @@ func parseSPDate(field string, value *string, planID string) (time.Time, error) // 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 + client *recommendations.Client + accountID string } // 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) + costs, err := a.client.GetOnDemandSeriesForAccount(ctx, a.accountID, region, lookbackDays) if err != nil { return nil, err } diff --git a/providers/aws/ladder/adapters_test.go b/providers/aws/ladder/adapters_test.go index d902a699..27de9a40 100644 --- a/providers/aws/ladder/adapters_test.go +++ b/providers/aws/ladder/adapters_test.go @@ -476,8 +476,9 @@ func (m *ladderCEMock) GetCostAndUsage( // 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 { + day, _ := time.Parse(time.DateOnly, dateStr) return cetypes.ResultByTime{ - TimePeriod: &cetypes.DateInterval{Start: aws.String(dateStr), End: aws.String(dateStr)}, + TimePeriod: &cetypes.DateInterval{Start: aws.String(dateStr), End: aws.String(day.AddDate(0, 0, 1).Format(time.DateOnly))}, Total: map[string]cetypes.MetricValue{ "UnblendedCost": {Amount: aws.String(amount)}, }, @@ -488,21 +489,22 @@ func ceDailyRow(dateStr, amount string) cetypes.ResultByTime { // []recommendations.DailyCost -> []DailyPoint mapping: dates and values are // copied index-for-index, oldest-first. func TestOnDemandSeriesAdapter_MapsDailyCostToDailyPoint(t *testing.T) { + day := time.Now().UTC().Truncate(24*time.Hour).AddDate(0, 0, -2) mock := &ladderCEMock{costAndUsage: &costexplorer.GetCostAndUsageOutput{ ResultsByTime: []cetypes.ResultByTime{ - ceDailyRow("2026-01-02", "480"), // $20/hr, newer - ceDailyRow("2026-01-01", "240"), // $10/hr, older + ceDailyRow(day.AddDate(0, 0, 1).Format(time.DateOnly), "480"), + ceDailyRow(day.Format(time.DateOnly), "240"), }, }} - adapter := &onDemandSeriesAdapter{client: recommendations.NewClientWithAPI(mock, testRegion)} + adapter := &onDemandSeriesAdapter{client: recommendations.NewClientWithAPI(mock, testRegion), accountID: "fixture-account"} 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.Equal(t, day, 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.Equal(t, day.AddDate(0, 0, 1), points[1].Date) assert.InDelta(t, 20.0, points[1].USDPerHour, 1e-9) } @@ -510,7 +512,7 @@ func TestOnDemandSeriesAdapter_MapsDailyCostToDailyPoint(t *testing.T) { // 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)} + adapter := &onDemandSeriesAdapter{client: recommendations.NewClientWithAPI(&ladderCEMock{}, testRegion), accountID: "fixture-account"} _, err := adapter.GetOnDemandSeries(context.Background(), testRegion, 7) diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go index 8c50426a..bef87186 100644 --- a/providers/aws/ladder/baseline.go +++ b/providers/aws/ladder/baseline.go @@ -141,15 +141,8 @@ func utcDay(t time.Time) time.Time { return t.UTC().Truncate(24 * time.Hour) } -// validateInWindowCoverage rejects a series that does not have enough points -// falling INSIDE the lookback window [today-lookbackDays, today]. Counting the -// total number of points is insufficient: N points smeared across more than N -// days (e.g. 27 points spread over 53 days) would pass a naive count check -// while leaving the recent window sparsely covered, so the percentile would be -// computed over data that does not represent the requested window. Every date -// is normalized to its UTC calendar day (utcDay) before comparison. -// -// today must already be normalized via utcDay by the caller. +// Only trailing reporting lag is allowed; missing interior days are not zero. +// today must be midnight UTC, matching the collector's exclusive end. func validateInWindowCoverage(points []DailyPoint, lookbackDays int, today time.Time) error { windowStart := today.AddDate(0, 0, -lookbackDays) inWindow := 0 @@ -165,6 +158,18 @@ func validateInWindowCoverage(points []DailyPoint, lookbackDays int, today time. inWindow, lookbackDays, minRequired, lookbackDays, maxMissingDays, ) } + if err := validateSeriesChronology(points); err != nil { + return err + } + for i, p := range points { + d := utcDay(p.Date) + if d.Before(windowStart) || !d.Before(today) { + return fmt.Errorf("on-demand series day %s is outside the requested window", d) + } + if !d.Equal(windowStart.AddDate(0, 0, i)) { + return fmt.Errorf("on-demand series has a missing day before %s", d) + } + } return nil } diff --git a/providers/aws/ladder/baseline_sdk_test.go b/providers/aws/ladder/baseline_sdk_test.go new file mode 100644 index 00000000..9b87168e --- /dev/null +++ b/providers/aws/ladder/baseline_sdk_test.go @@ -0,0 +1,182 @@ +package ladder_test + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "slices" + "sync/atomic" + "testing" + "time" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/credentials" + "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" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" + awsladder "github.com/LeanerCloud/cloud-commitments-go/providers/aws/ladder" +) + +// Constructed CE responses exercise the public factory without cloud access. +func TestScopedBaselineParentCompatible(t *testing.T) { + for _, mode := range []string{"account", "reverse", "zero", "lag2", "lag3", "duplicate", "page-duplicate", "hole", "leading", "outside", "future", "bad-start", "nil-start", "nil-period", "nil-end", "bad-end", "short-period", "long-period", "backward-period", "empty", "all-zero", "missing-total", "missing-metric", "missing-amount", "bad-amount", "NaN", "Inf", "negative", "stale", "few", "provider-error"} { + t.Run(mode, func(t *testing.T) { + var calls atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + calls.Add(1) + assert.Equal(t, "AWSInsightsIndexService.GetCostAndUsage", r.Header.Get("X-Amz-Target")) + var in costexplorer.GetCostAndUsageInput + if !assert.NoError(t, json.NewDecoder(r.Body).Decode(&in)) { + return + } + assert.Equal(t, types.GranularityDaily, in.Granularity) + assert.Equal(t, []string{"UnblendedCost"}, in.Metrics) + start, err := time.Parse(time.DateOnly, aws.ToString(in.TimePeriod.Start)) + assert.NoError(t, err) + assert.Equal(t, start.AddDate(0, 0, 30).Format(time.DateOnly), aws.ToString(in.TimePeriod.End)) + dimensions := map[types.Dimension][]string{} + if in.Filter != nil { + for _, clause := range in.Filter.And { + if clause.Dimensions != nil { + dimensions[clause.Dimensions.Key] = clause.Dimensions.Values + } + } + } + matched := slices.Equal(dimensions[types.DimensionLinkedAccount], []string{"fixture-account"}) && + slices.Equal(dimensions[types.DimensionRegion], []string{"eu-west-1"}) && + slices.Equal(dimensions[types.DimensionService], []string{"Amazon Elastic Compute Cloud - Compute"}) && + slices.Equal(dimensions[types.DimensionPurchaseType], []string{"On Demand Instances"}) + rows := baselineRows(start, mode) + token := "" + if mode == "account" || mode == "reverse" || mode == "page-duplicate" { + switch aws.ToString(in.NextPageToken) { + case "": + rows, token = rows[:28], "second" + case "second": + rows = rows[28:] + if mode == "page-duplicate" { + rows = append(rows, baselineRows(start, "account")[0]) + } + default: + t.Error("unexpected page token") + } + } + if mode == "reverse" { + slices.Reverse(rows) + } + if mode == "account" && !matched { + for i := range rows { + rows[i].Total["UnblendedCost"] = types.MetricValue{Amount: aws.String("2400")} + } + } + if mode == "provider-error" { + w.WriteHeader(http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/x-amz-json-1.1") + assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{"ResultsByTime": rows, "NextPageToken": token})) + })) + defer srv.Close() + endpoint, err := url.Parse(srv.URL) + require.NoError(t, err) + a, err := awsladder.NewFromConfig(aws.Config{Region: "eu-west-1", RetryMaxAttempts: 1, + Credentials: credentials.NewStaticCredentialsProvider("synthetic", "synthetic", ""), + HTTPClient: &http.Client{Transport: ceLoopback{endpoint: endpoint, transport: srv.Client().Transport}}, + }, awsladder.Config{Region: "eu-west-1", AccountID: "fixture-account"}) + require.NoError(t, err) + scope := ladder.Scope{Provider: common.ProviderAWS, AccountID: "wrong"} + _, err = a.GetUsageBaseline(context.Background(), scope, 30, 5) + require.Error(t, err) + assert.Zero(t, calls.Load()) + scope.AccountID = "fixture-account" + _, err = a.GetUsageBaseline(context.Background(), scope, 0, 5) + require.Error(t, err) + _, err = a.GetUsageBaseline(context.Background(), scope, 30, 0) + require.Error(t, err) + assert.Zero(t, calls.Load()) + bl, err := a.GetUsageBaseline(context.Background(), scope, 30, 5) + want := map[string]float64{"account": 2, "reverse": 2, "zero": 2, "lag2": 10, "lag3": 10} + if low, ok := want[mode]; ok { + require.NoError(t, err) + require.NotNil(t, bl.LowWaterUSDPerHour) + assert.InDelta(t, low, *bl.LowWaterUSDPerHour, 1e-9) + assert.Nil(t, bl.StableUSDPerHour) + assert.Equal(t, 30, bl.LookbackDays) + assert.Equal(t, 5.0, bl.Percentile) + assert.Equal(t, 10.0, bl.Series[1]) + if mode == "zero" { + assert.Zero(t, bl.Series[0]) + } + } else { + require.Error(t, err) + } + if mode == "account" || mode == "reverse" { + assert.Equal(t, int32(2), calls.Load()) + } + }) + } +} + +func baselineRows(start time.Time, mode string) []types.ResultByTime { + rows := make([]types.ResultByTime, 30) + for i := range rows { + day, amount := start.AddDate(0, 0, i), "240" + if i >= 28 { + amount = "48" + } + rows[i] = types.ResultByTime{TimePeriod: &types.DateInterval{Start: aws.String(day.Format(time.DateOnly)), End: aws.String(day.AddDate(0, 0, 1).Format(time.DateOnly))}, Total: map[string]types.MetricValue{"UnblendedCost": {Amount: aws.String(amount), Unit: aws.String("USD")}}} + } + switch mode { + case "duplicate": + rows = append(rows, rows[0]) + case "hole": + rows = append(rows[:10], rows[11:]...) + case "leading": + rows = rows[1:] + case "outside": + rows[0].TimePeriod.Start = aws.String(start.AddDate(0, 0, -1).Format(time.DateOnly)) + rows[0].TimePeriod.End = aws.String(start.Format(time.DateOnly)) + case "future": + rows[0].TimePeriod = &types.DateInterval{Start: aws.String(start.AddDate(0, 0, 30).Format(time.DateOnly)), End: aws.String(start.AddDate(0, 0, 31).Format(time.DateOnly))} + case "bad-start": + rows[0].TimePeriod.Start = aws.String("bad") + case "nil-start": + rows[0].TimePeriod.Start = nil + case "nil-period": + rows[0].TimePeriod = nil + case "nil-end": + rows[0].TimePeriod.End = nil + case "bad-end": + rows[0].TimePeriod.End = aws.String("bad") + case "short-period": + rows[0].TimePeriod.End = rows[0].TimePeriod.Start + case "long-period": + rows[0].TimePeriod.End = aws.String(start.AddDate(0, 0, 2).Format(time.DateOnly)) + case "backward-period": + rows[0].TimePeriod.End = aws.String(start.AddDate(0, 0, -1).Format(time.DateOnly)) + case "empty": + rows = nil + case "all-zero": + for i := range rows { + rows[i].Total["UnblendedCost"] = types.MetricValue{Amount: aws.String("0")} + } + case "missing-total": + rows[0].Total = nil + case "missing-metric": + rows[0].Total = map[string]types.MetricValue{} + case "missing-amount": + rows[0].Total["UnblendedCost"] = types.MetricValue{} + case "bad-amount", "NaN", "Inf", "negative", "zero": + amount := map[string]string{"bad-amount": "bad", "NaN": "NaN", "Inf": "Inf", "negative": "-1", "zero": "0"}[mode] + rows[0].Total["UnblendedCost"] = types.MetricValue{Amount: aws.String(amount)} + case "lag2", "lag3", "stale", "few": + rows = rows[:map[string]int{"lag2": 29, "lag3": 28, "stale": 27, "few": 6}[mode]] + } + return rows +} diff --git a/providers/aws/ladder/factory.go b/providers/aws/ladder/factory.go index d05d66b7..30922656 100644 --- a/providers/aws/ladder/factory.go +++ b/providers/aws/ladder/factory.go @@ -86,7 +86,7 @@ func NewFromConfig(awsCfg aws.Config, cfg Config) (*AWSLadder, error) { ec2Client, // riLister &spListerAdapter{api: spSDKClient, region: cfg.Region}, // spLister (region-scoped) recoClient, // riCoverageSource - &onDemandSeriesAdapter{client: recoClient}, // onDemandSeriesSource + &onDemandSeriesAdapter{client: recoClient, accountID: cfg.AccountID}, // onDemandSeriesSource recoClient, // utilizationSource &spCoverageAdapter{client: recoClient}, // spCoverageSource &spUtilizationAdapter{client: recoClient}, // spUtilizationSource diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go index d3cc09b7..94c92e4a 100644 --- a/providers/aws/ladder/ladder_test.go +++ b/providers/aws/ladder/ladder_test.go @@ -855,14 +855,14 @@ func TestGetUsageBaseline_WrongScope_ReturnsError(t *testing.T) { // it as though it were current. Now GetUsageBaseline must return an explicit error // naming the last date when the most-recent point is older than maxSeriesAgeDays. func TestGetUsageBaseline_StaleSeries_ReturnsError(t *testing.T) { - // Build a 30-point series whose most-recent point is maxSeriesAgeDays+1 days + // Build a 27-point series whose most-recent point is maxSeriesAgeDays+1 days // in the past, simulating a CE feed that stopped (or a wrong date range). // The date is built from the UTC calendar day so it is hour-independent. staleEnd := utcDay(time.Now()).AddDate(0, 0, -(maxSeriesAgeDays + 1)) - points := make([]DailyPoint, 30) + points := make([]DailyPoint, 27) for i := range points { points[i] = DailyPoint{ - Date: staleEnd.AddDate(0, 0, -(29 - i)), + Date: staleEnd.AddDate(0, 0, -(26 - i)), USDPerHour: float64(i + 1), } } @@ -946,9 +946,9 @@ func TestGetUsageBaseline_AgeExactlyAtBoundary_NoError(t *testing.T) { // built directly from the UTC calendar day (no hour arithmetic) so the test // does not depend on the current UTC hour. end := utcDay(time.Now()).AddDate(0, 0, -maxSeriesAgeDays) - points := make([]DailyPoint, 30) + points := make([]DailyPoint, 28) for i := range points { - points[i] = DailyPoint{Date: end.AddDate(0, 0, -(29 - i)), USDPerHour: float64(i + 1)} + points[i] = DailyPoint{Date: end.AddDate(0, 0, -(27 - i)), USDPerHour: float64(i + 1)} } cov := &fakeCoverageSource{onDemandPoints: points} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) @@ -963,10 +963,8 @@ func TestGetUsageBaseline_AgeExactlyAtBoundary_NoError(t *testing.T) { func TestGetUsageBaseline_CoverageExactlyAtBoundary_NoError(t *testing.T) { const lookbackDays = 30 n := lookbackDays - maxMissingDays // exactly the minimum required, all in-window - cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(n)} - a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) - - _, err := a.GetUsageBaseline(context.Background(), testScope(), lookbackDays, 5.0) + points := makeRecentPoints(lookbackDays)[:n] + err := validateInWindowCoverage(points, lookbackDays, utcDay(time.Now())) require.NoError(t, err, "exactly lookbackDays-maxMissingDays in-window points must pass (boundary is inWindow < minRequired)") } diff --git a/providers/aws/recommendations/ondemand_series.go b/providers/aws/recommendations/ondemand_series.go index 12ce8ada..2df38bfd 100644 --- a/providers/aws/recommendations/ondemand_series.go +++ b/providers/aws/recommendations/ondemand_series.go @@ -91,9 +91,7 @@ type DailyCost struct { // 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. +// error. Missing observations are not treated as measured zero. // - 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 @@ -101,6 +99,18 @@ type DailyCost struct { // 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) { + return c.getOnDemandSeries(ctx, "", region, lookbackDays) +} + +// GetOnDemandSeriesForAccount restricts residual observations to the usage account. +func (c *Client) GetOnDemandSeriesForAccount(ctx context.Context, accountID, region string, lookbackDays int) ([]DailyCost, error) { + if accountID == "" { + return nil, fmt.Errorf("GetOnDemandSeriesForAccount: accountID must not be empty") + } + return c.getOnDemandSeries(ctx, accountID, region, lookbackDays) +} + +func (c *Client) getOnDemandSeries(ctx context.Context, accountID, region string, lookbackDays int) ([]DailyCost, error) { if err := validateOnDemandSeriesArgs(region, lookbackDays); err != nil { return nil, err } @@ -117,6 +127,11 @@ func (c *Client) GetOnDemandSeries(ctx context.Context, region string, lookbackD Metrics: []string{onDemandMetric}, Filter: onDemandSeriesFilter(region), } + if accountID != "" { + input.Filter.And = append(input.Filter.And, types.Expression{Dimensions: &types.DimensionValues{ + Key: types.DimensionLinkedAccount, Values: []string{accountID}, + }}) + } byDate := make(map[string]float64) @@ -137,7 +152,12 @@ func (c *Client) GetOnDemandSeries(ctx context.Context, region string, lookbackD return nil, fmt.Errorf("GetOnDemandSeries: %w", err) } - if err := accumulateDailyResults(byDate, out); err != nil { + if accountID != "" { + err = accumulateScopedDailyResults(byDate, out, start, end) + } else { + err = accumulateDailyResults(byDate, out) + } + if err != nil { return nil, fmt.Errorf("GetOnDemandSeries: %w", err) } @@ -149,11 +169,33 @@ func (c *Client) GetOnDemandSeries(ctx context.Context, region string, lookbackD nextToken = out.NextPageToken } - series, err := buildDailySeries(byDate, region, lookbackDays) - if err != nil { - return nil, fmt.Errorf("GetOnDemandSeries: %w", err) + return buildDailySeries(byDate, region, lookbackDays) +} + +func accumulateScopedDailyResults(byDate map[string]float64, out *costexplorer.GetCostAndUsageOutput, start, end time.Time) error { + for _, row := range out.ResultsByTime { + if row.TimePeriod == nil { + return fmt.Errorf("CE daily result is missing its time period") + } + day, err := time.Parse(ceDateLayout, aws.ToString(row.TimePeriod.Start)) + if err != nil { + return fmt.Errorf("CE daily period start: %w", err) + } + next, err := time.Parse(ceDateLayout, aws.ToString(row.TimePeriod.End)) + if err != nil { + return fmt.Errorf("CE daily period end: %w", err) + } + if !next.Equal(day.AddDate(0, 0, 1)) || day.Before(start) || next.After(end) { + return fmt.Errorf("CE daily period %s to %s is not one day inside the requested window", day, next) + } + if _, duplicate := byDate[day.Format(ceDateLayout)]; duplicate { + return fmt.Errorf("CE returned duplicate daily period %s", day) + } + if err := accumulateDailyResults(byDate, &costexplorer.GetCostAndUsageOutput{ResultsByTime: []types.ResultByTime{row}}); err != nil { + return err + } } - return series, nil + return nil } // validateOnDemandSeriesArgs rejects out-of-range GetOnDemandSeries arguments @@ -226,10 +268,7 @@ func onDemandSeriesFilter(region string) *types.Expression { // - 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. +// - Missing metrics or amounts are insufficient observations, not zero. // - Unparseable amount strings fail loud (feedback_strict_int_parse). // // Dividing by 24 converts daily USD to USD/hr; 24 is derived from the DAILY @@ -242,7 +281,7 @@ func accumulateDailyResults(byDate map[string]float64, out *costexplorer.GetCost 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\")", + return fmt.Errorf("CE result row for day %s is missing the %q metric or amount; no observation is available", dateStr, onDemandMetric) } usd, err := strconv.ParseFloat(aws.ToString(mv.Amount), 64) diff --git a/providers/aws/recommendations/ondemand_series_test.go b/providers/aws/recommendations/ondemand_series_test.go index a5506488..44054ae0 100644 --- a/providers/aws/recommendations/ondemand_series_test.go +++ b/providers/aws/recommendations/ondemand_series_test.go @@ -79,6 +79,47 @@ func newOnDemandClient(m *mockOnDemandCE) *Client { return NewClientWithAPI(m, "us-east-1") } +func TestGetOnDemandSeriesForAccount_RejectsMissingScopeBeforeIO(t *testing.T) { + for _, args := range []struct { + account, region string + days int + }{{"", "eu-west-1", 30}, {"fixture-account", "", 30}, {"fixture-account", "eu-west-1", 0}} { + mock := &mockOnDemandCE{} + _, err := newOnDemandClient(mock).GetOnDemandSeriesForAccount(context.Background(), args.account, args.region, args.days) + require.Error(t, err) + assert.Empty(t, mock.gotInputs) + } +} + +func TestGetOnDemandSeriesForAccount_ValidatesPeriods(t *testing.T) { + end := time.Now().UTC().Truncate(24 * time.Hour) + for _, mode := range []string{"valid", "duplicate", "outside", "end"} { + t.Run(mode, func(t *testing.T) { + day := end.AddDate(0, 0, -1) + if mode == "outside" { + day = end + } + row := dailyResult(day.Format(ceDateLayout), 24) + if mode != "end" { + row.TimePeriod.End = aws.String(day.AddDate(0, 0, 1).Format(ceDateLayout)) + } + rows := []types.ResultByTime{row} + if mode == "duplicate" { + rows = append(rows, row) + } + mock := &mockOnDemandCE{pages: []*costexplorer.GetCostAndUsageOutput{{ResultsByTime: rows}}} + series, err := newOnDemandClient(mock).GetOnDemandSeriesForAccount(context.Background(), "fixture-account", "eu-west-1", 30) + if mode == "valid" { + require.NoError(t, err) + require.Len(t, series, 1) + assert.Equal(t, 1.0, series[0].USDPerHour) + } else { + require.Error(t, err) + } + }) + } +} + // 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.