From 2952371300c47f4d4215cba984d7dc6418d479b0 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 21 Jul 2026 21:55:22 +0200 Subject: [PATCH 1/2] fix(aws/ladder): scope RI utilization query to EC2+region for reshape decisions GetRIUtilization built GetReservationUtilization with no Filter at all, so it blended utilization across every reserved-resource type in the account (RDS, ElastiCache, OpenSearch, Redshift, standard EC2 RIs) and every region into one SUBSCRIPTION_ID-grouped number. The ladder ConvertibleRI (buffer) layer reads this as if it were EC2-convertible utilization for one account/region, then uses it to decide whether to trigger a real RI reshape/exchange -- an underutilized RI in an unrelated service or region could trigger a reshape, or mask a genuinely poorly-utilized convertible RI. Add a SERVICE+REGION Filter to the CE call, mirroring the same pattern already used by GetRICoverageMap in coverage.go. Also intersect the (now EC2+region-scoped) response against the account's own convertible RI IDs before aggregating in GetLayerStates, since a standard (non-convertible) EC2 RI in the same account/region would otherwise still pass the SERVICE+REGION filter and blend into the layer's UtilizationPct. Threads a region parameter through GetRIUtilization's callers (providers/aws/ladder, providers/aws/service_client.go, internal/server, internal/api). Follow-up to #1361. --- internal/api/handler_ri_exchange.go | 18 ++- .../handler_ri_exchange_integration_test.go | 2 +- internal/api/handler_ri_exchange_test.go | 2 +- internal/server/handler_ri_exchange.go | 2 +- internal/server/ladder_write.go | 2 +- providers/aws/ladder/interfaces.go | 2 +- providers/aws/ladder/ladder_test.go | 46 ++++++- providers/aws/ladder/layer_states.go | 30 ++++- providers/aws/recommendations/utilization.go | 33 ++++- .../aws/recommendations/utilization_test.go | 116 ++++++++++++++++++ providers/aws/service_client.go | 7 +- 11 files changed, 243 insertions(+), 17 deletions(-) create mode 100644 providers/aws/recommendations/utilization_test.go diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go index bdaada9eb..5ac566ba5 100644 --- a/internal/api/handler_ri_exchange.go +++ b/internal/api/handler_ri_exchange.go @@ -48,7 +48,7 @@ type reshapeEC2Client interface { // adapter that getReshapeRecommendations needs (the utilization // fetcher injected into the cache wrapper). Scoped identically. type reshapeRecsClient interface { - GetRIUtilization(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) + GetRIUtilization(ctx context.Context, lookbackDays int, region string) ([]recommendations.RIUtilization, error) } // buildReshapeEC2Client honors the injected factory when set, falling @@ -399,9 +399,18 @@ func (h *Handler) getRIUtilization(ctx context.Context, req *events.LambdaFuncti if err != nil { return nil, fmt.Errorf("failed to load AWS config: %w", err) } + // Adopt the resolved region when the caller omits ?region= so the CE + // utilization filter and the cache key both scope to the region the + // AWS client is actually talking to (mirrors getReshapeRecommendations). + if region == "" { + region = cfg.Region + } recsAdapter := awsprovider.NewRecommendationsClientDirect(cfg) - utilization, err := h.getRIUtilizationCache().getOrFetch(ctx, region, lookbackDays, riUtilizationCacheTTL, riUtilizationCacheStaleTTL, recsAdapter.GetRIUtilization) + fetch := func(fetchCtx context.Context, days int) ([]recommendations.RIUtilization, error) { + return recsAdapter.GetRIUtilization(fetchCtx, days, region) + } + utilization, err := h.getRIUtilizationCache().getOrFetch(ctx, region, lookbackDays, riUtilizationCacheTTL, riUtilizationCacheStaleTTL, fetch) if err != nil { return nil, fmt.Errorf("failed to get RI utilization: %w", err) } @@ -559,7 +568,10 @@ func (h *Handler) getReshapeRecommendations(ctx context.Context, req *events.Lam } recsAdapter := h.buildReshapeRecsClient(cfg) - utilData, err := h.getRIUtilizationCache().getOrFetch(ctx, p.region, p.lookbackDays, riUtilizationCacheTTL, riUtilizationCacheStaleTTL, recsAdapter.GetRIUtilization) + fetch := func(fetchCtx context.Context, days int) ([]recommendations.RIUtilization, error) { + return recsAdapter.GetRIUtilization(fetchCtx, days, p.region) + } + utilData, err := h.getRIUtilizationCache().getOrFetch(ctx, p.region, p.lookbackDays, riUtilizationCacheTTL, riUtilizationCacheStaleTTL, fetch) if err != nil { return nil, fmt.Errorf("failed to get RI utilization: %w", err) } diff --git a/internal/api/handler_ri_exchange_integration_test.go b/internal/api/handler_ri_exchange_integration_test.go index da56b9410..ae0a1c1ac 100644 --- a/internal/api/handler_ri_exchange_integration_test.go +++ b/internal/api/handler_ri_exchange_integration_test.go @@ -42,7 +42,7 @@ type fakeReshapeRecs struct { calls atomic.Int32 } -func (f *fakeReshapeRecs) GetRIUtilization(_ context.Context, _ int) ([]recommendations.RIUtilization, error) { +func (f *fakeReshapeRecs) GetRIUtilization(_ context.Context, _ int, _ string) ([]recommendations.RIUtilization, error) { f.calls.Add(1) return f.utilization, nil } diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go index 08d674568..db29a4efd 100644 --- a/internal/api/handler_ri_exchange_test.go +++ b/internal/api/handler_ri_exchange_test.go @@ -585,7 +585,7 @@ type fakeReshapeRecsStub struct { utilization []recommendations.RIUtilization } -func (f *fakeReshapeRecsStub) GetRIUtilization(_ context.Context, _ int) ([]recommendations.RIUtilization, error) { +func (f *fakeReshapeRecsStub) GetRIUtilization(_ context.Context, _ int, _ string) ([]recommendations.RIUtilization, error) { return f.utilization, nil } diff --git a/internal/server/handler_ri_exchange.go b/internal/server/handler_ri_exchange.go index adb692a1f..eddaf9a89 100644 --- a/internal/server/handler_ri_exchange.go +++ b/internal/server/handler_ri_exchange.go @@ -56,7 +56,7 @@ func (app *Application) handleRIExchangeReshape(ctx context.Context) (*exchange. }, getRIUtilization: func(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) { recsClient := awsprovider.NewRecommendationsClientDirect(awsCfg) - return recsClient.GetRIUtilization(ctx, lookbackDays) + return recsClient.GetRIUtilization(ctx, lookbackDays, awsCfg.Region) }, exchangeClient: exchange.NewExchangeClient(awsCfg), lookupOffering: func(ctx context.Context, instanceType, productDesc, tenancy, scope string, duration int64) (string, error) { diff --git a/internal/server/ladder_write.go b/internal/server/ladder_write.go index 97f716238..7f95850ce 100644 --- a/internal/server/ladder_write.go +++ b/internal/server/ladder_write.go @@ -52,7 +52,7 @@ func (a *exchangeRunnerAdapter) RunAutoExchange(ctx context.Context, cfg exchang if err != nil { return nil, fmt.Errorf("exchangeRunnerAdapter: list convertible RIs: %w", err) } - utilData, err := recsClient.GetRIUtilization(ctx, cfg.LookbackDays) + utilData, err := recsClient.GetRIUtilization(ctx, cfg.LookbackDays, a.region) if err != nil { return nil, fmt.Errorf("exchangeRunnerAdapter: get RI utilization: %w", err) } diff --git a/providers/aws/ladder/interfaces.go b/providers/aws/ladder/interfaces.go index ccac6dfdf..1a503dd34 100644 --- a/providers/aws/ladder/interfaces.go +++ b/providers/aws/ladder/interfaces.go @@ -104,7 +104,7 @@ type onDemandSeriesSource interface { // utilizationSource is the narrow interface for RI utilization data. // The real implementation is RecommendationsClientAdapter.GetRIUtilization. type utilizationSource interface { - GetRIUtilization(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) + GetRIUtilization(ctx context.Context, lookbackDays int, region string) ([]recommendations.RIUtilization, error) } // SPCoverageSummary carries the Savings Plans coverage result from the CE API. diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go index 7f971f0ec..8ba6c3a59 100644 --- a/providers/aws/ladder/ladder_test.go +++ b/providers/aws/ladder/ladder_test.go @@ -66,7 +66,7 @@ type fakeUtilizationSource struct { utils []recommendations.RIUtilization } -func (f *fakeUtilizationSource) GetRIUtilization(_ context.Context, _ int) ([]recommendations.RIUtilization, error) { +func (f *fakeUtilizationSource) GetRIUtilization(_ context.Context, _ int, _ string) ([]recommendations.RIUtilization, error) { return f.utils, f.err } @@ -507,8 +507,12 @@ func TestGetLayerStates_RILayer_CoverageAndUtilization(t *testing.T) { "us-east-1:m5.large": {Pct: 80.0, AvgInstancesPerHour: 10.0}, "us-east-1:m5.xlarge": {Pct: 60.0, AvgInstancesPerHour: 5.0}, } + // ReservedInstanceID must match the convertible RI's ID: GetLayerStates + // intersects CE utilization entries against the account's convertible + // RIs before aggregating, so an entry for an untracked ID would be + // dropped (issue #1461). utils := []recommendations.RIUtilization{ - {PurchasedHours: 100, TotalActualHours: 90}, + {ReservedInstanceID: "ri-1", PurchasedHours: 100, TotalActualHours: 90}, } a := newTestLadder(t, @@ -529,6 +533,44 @@ func TestGetLayerStates_RILayer_CoverageAndUtilization(t *testing.T) { assert.InDelta(t, 90.0, *ri.UtilizationPct, 1e-6) } +// TestGetLayerStates_RILayer_UtilizationExcludesUnrelatedReservations is the +// GetLayerStates-side regression test for issue #1461. GetRIUtilization's +// SERVICE+REGION filter narrows the CE response to EC2 RIs in-region, but a +// standard (non-convertible) EC2 RI in the same account/region -- or, before +// the fix, an RDS/ElastiCache/OpenSearch/Redshift reservation blended in by +// an unfiltered query -- would still land in the utils slice this test +// fakes. Without the ID intersection, the layer's UtilizationPct would blend +// in "other-ri"'s poor utilization and trigger a reshape decision that has +// nothing to do with this account's convertible RIs. +func TestGetLayerStates_RILayer_UtilizationExcludesUnrelatedReservations(t *testing.T) { + ris := []ec2svc.ConvertibleRI{ + makeRI("ri-1", "m5.large", 1, 0.50, time.Now().Add(365*24*time.Hour)), + } + utils := []recommendations.RIUtilization{ + // Tracked convertible RI: well utilized. + {ReservedInstanceID: "ri-1", PurchasedHours: 100, TotalActualHours: 90}, + // Untracked reservation (e.g. RDS, or a standard non-convertible EC2 + // RI) blended into the same CE response: poorly utilized. Pre-fix, + // this drags the aggregate down to (90+5)/(100+100)=47.5%; post-fix + // it must be excluded entirely. + {ReservedInstanceID: "other-ri", PurchasedHours: 100, TotalActualHours: 5}, + } + + a := newTestLadder(t, + &fakeRILister{ris: ris}, + &fakeSPLister{}, + &fakeCoverageSource{}, + &fakeUtilizationSource{utils: utils}, + ) + states, err := a.GetLayerStates(context.Background(), testScope()) + require.NoError(t, err) + + ri := states[ladder.LayerConvertibleRI] + require.NotNil(t, ri.UtilizationPct) + assert.InDelta(t, 90.0, *ri.UtilizationPct, 1e-6, + "UtilizationPct must reflect only ri-1 (90%%), not the blended 47.5%% from the untracked reservation") +} + func TestGetLayerStates_CoverageError_DegradesToNil(t *testing.T) { ris := []ec2svc.ConvertibleRI{ makeRI("ri-1", "m5.large", 1, 0.50, time.Now().Add(365*24*time.Hour)), diff --git a/providers/aws/ladder/layer_states.go b/providers/aws/ladder/layer_states.go index 54c828022..2d1f68591 100644 --- a/providers/aws/ladder/layer_states.go +++ b/providers/aws/ladder/layer_states.go @@ -54,7 +54,7 @@ func (a *AWSLadder) GetLayerStates(ctx context.Context, scope ladder.Scope) (map // covErr is checked per-layer below; a coverage failure does not fail the // whole snapshot — it degrades CoveragePct to nil. - utils, utilErr := a.utilization.GetRIUtilization(ctx, a.cfg.lookbackDays()) + utils, utilErr := a.utilization.GetRIUtilization(ctx, a.cfg.lookbackDays(), a.cfg.Region) // utilErr is handled the same way: degrade UtilizationPct to nil. now := time.Now() @@ -105,7 +105,12 @@ func (a *AWSLadder) riLayerState( log.Printf("WARNING: AWSLadder GetLayerStates: RI utilization degraded to nil (layer=%s, source=GetRIUtilization, region=%s): %v", ladder.LayerConvertibleRI, a.cfg.Region, utilErr) } else { - state.UtilizationPct = computeRIUtilizationPct(utils) + // GetRIUtilization is already scoped to EC2+region, but that still + // blends in standard (non-convertible) EC2 RIs alongside the + // convertible RIs this buffer layer tracks. Intersect by ID so + // UtilizationPct reflects only the reservations riLayerState + // reasons about. + state.UtilizationPct = computeRIUtilizationPct(utilsForConvertibleRIs(utils, ris)) } return state @@ -323,6 +328,27 @@ func computeEC2CoveragePct(coverageMap recommendations.PoolCoverageMap, region s return ptr(result) } +// utilsForConvertibleRIs filters CE utilization entries down to the IDs +// present in ris. GetRIUtilization's SERVICE+REGION filter already narrows +// the CE response to EC2 RIs in this region, but a standard (non-convertible) +// EC2 RI in the same account/region would still pass that filter; the ID +// intersection restricts the aggregate to exactly the convertible RIs this +// buffer layer tracks (issue #1461). +func utilsForConvertibleRIs(utils []recommendations.RIUtilization, ris []ec2svc.ConvertibleRI) []recommendations.RIUtilization { + ids := make(map[string]struct{}, len(ris)) + for i := range ris { + ids[ris[i].ReservedInstanceID] = struct{}{} + } + filtered := make([]recommendations.RIUtilization, 0, len(utils)) + for i := range utils { + u := utils[i] + if _, ok := ids[u.ReservedInstanceID]; ok { + filtered = append(filtered, u) + } + } + return filtered +} + // computeRIUtilizationPct aggregates per-RI utilization data from the CE // GetReservationUtilization response into a single percentage. // diff --git a/providers/aws/recommendations/utilization.go b/providers/aws/recommendations/utilization.go index 26090d6f3..797fb5e2e 100644 --- a/providers/aws/recommendations/utilization.go +++ b/providers/aws/recommendations/utilization.go @@ -29,8 +29,19 @@ type riAccumulator struct { unusedHours float64 } -// GetRIUtilization fetches per-RI utilization from Cost Explorer for the last N days. -func (c *Client) GetRIUtilization(ctx context.Context, lookbackDays int) ([]RIUtilization, error) { +// GetRIUtilization fetches per-RI utilization from Cost Explorer for the +// last N days, scoped to EC2 RIs in region. +// +// Without a Filter, GetReservationUtilization blends utilization across +// every reserved-resource type in the account (RDS, ElastiCache, +// OpenSearch, Redshift, standard EC2 RIs) and every region into one +// SUBSCRIPTION_ID-grouped number. Callers that read this as "EC2 +// convertible-RI utilization for this region" (e.g. the ladder +// ConvertibleRI layer) would then trigger real reshape/exchange +// decisions off an unrelated RI's utilization (issue #1461). region +// is optional: an empty string omits the REGION dimension, matching +// callers that haven't resolved a specific region. +func (c *Client) GetRIUtilization(ctx context.Context, lookbackDays int, region string) ([]RIUtilization, error) { if lookbackDays <= 0 { lookbackDays = 30 } @@ -49,6 +60,7 @@ func (c *Client) GetRIUtilization(ctx context.Context, lookbackDays int) ([]RIUt Key: aws.String("SUBSCRIPTION_ID"), }, }, + Filter: ec2UtilizationFilter(region), } agg := make(map[string]*riAccumulator) @@ -80,6 +92,23 @@ func (c *Client) GetRIUtilization(ctx context.Context, lookbackDays int) ([]RIUt return buildUtilizations(agg), nil } +// ec2UtilizationFilter builds the CE Filter expression scoping +// GetReservationUtilization to EC2 RIs, optionally narrowed to one +// region. Mirrors serviceRegionFilter in coverage.go so both CE +// query paths agree on how a (service, region) scope is expressed. +func ec2UtilizationFilter(region string) *types.Expression { + svc := types.Expression{Dimensions: &types.DimensionValues{Key: types.DimensionService, Values: []string{ec2ComputeService}}} + if region == "" { + return &svc + } + return &types.Expression{ + And: []types.Expression{ + svc, + {Dimensions: &types.DimensionValues{Key: types.DimensionRegion, Values: []string{region}}}, + }, + } +} + func buildUtilizations(agg map[string]*riAccumulator) []RIUtilization { utilizations := make([]RIUtilization, 0, len(agg)) for id, a := range agg { diff --git a/providers/aws/recommendations/utilization_test.go b/providers/aws/recommendations/utilization_test.go new file mode 100644 index 000000000..d4901b4a6 --- /dev/null +++ b/providers/aws/recommendations/utilization_test.go @@ -0,0 +1,116 @@ +package recommendations + +import ( + "context" + "testing" + + "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" +) + +// mockUtilizationCE extends the test mock with a configurable +// GetReservationUtilization response and captures the Filter from the most +// recent call so tests can assert the CE query was scoped correctly +// (issue #1461). +type mockUtilizationCE struct { + mockCostExplorerAPI + utilizationOutput *costexplorer.GetReservationUtilizationOutput + lastFilter *types.Expression + calls int +} + +func (m *mockUtilizationCE) GetReservationUtilization(ctx context.Context, params *costexplorer.GetReservationUtilizationInput, optFns ...func(*costexplorer.Options)) (*costexplorer.GetReservationUtilizationOutput, error) { + m.calls++ + if params != nil { + m.lastFilter = params.Filter + } + return m.utilizationOutput, nil +} + +// TestGetRIUtilization_ScopesToEC2AndRegion is the primary regression test +// for issue #1461: without a Filter, GetReservationUtilization blends +// utilization across every reserved-resource type (RDS, EC2, ...) and every +// region into one SUBSCRIPTION_ID-grouped number. It replicates the real +// failing scenario -- an RDS reservation with low utilization and an EC2 +// convertible RI with high utilization returned side by side under the +// same account -- and asserts: +// 1. the outgoing CE request carries a Filter scoping SERVICE=EC2 and +// REGION=region (pre-fix code sent no Filter at all). +// 2. the blended response is still parsed into per-RI entries so a +// caller doing its own ID intersection (ladder layer_states.go) can +// recover the EC2-only aggregate even though the mock doesn't +// actually filter server-side. +func TestGetRIUtilization_ScopesToEC2AndRegion(t *testing.T) { + mock := &mockUtilizationCE{ + utilizationOutput: &costexplorer.GetReservationUtilizationOutput{ + UtilizationsByTime: []types.UtilizationByTime{ + { + Groups: []types.ReservationUtilizationGroup{ + { + // RDS reservation: poorly utilized. Pre-fix, this + // blends into the same aggregate an EC2-only + // caller reads as "its own" utilization. + Key: aws.String("rds-ri-1"), + Utilization: &types.ReservationAggregates{ + PurchasedHours: aws.String("100"), + TotalActualHours: aws.String("10"), + }, + }, + { + // EC2 convertible RI: highly utilized. + Key: aws.String("ec2-ri-1"), + Utilization: &types.ReservationAggregates{ + PurchasedHours: aws.String("100"), + TotalActualHours: aws.String("95"), + }, + }, + }, + }, + }, + }, + } + client := NewClientWithAPI(mock, "us-east-1") + + got, err := client.GetRIUtilization(context.Background(), 30, "us-east-1") + require.NoError(t, err) + + require.NotNil(t, mock.lastFilter, "GetRIUtilization must send a Filter (issue #1461: an absent Filter blends every reserved-resource type and region into one number)") + require.NotNil(t, mock.lastFilter.And, "Filter must And together SERVICE and REGION dimensions") + require.Len(t, mock.lastFilter.And, 2) + assert.Equal(t, types.DimensionService, mock.lastFilter.And[0].Dimensions.Key) + assert.Equal(t, []string{ec2ComputeService}, mock.lastFilter.And[0].Dimensions.Values) + assert.Equal(t, types.DimensionRegion, mock.lastFilter.And[1].Dimensions.Key) + assert.Equal(t, []string{"us-east-1"}, mock.lastFilter.And[1].Dimensions.Values) + + // Both entries still parse -- the ID intersection that scopes an EC2-only + // caller down to its own convertible RIs happens one layer up in + // providers/aws/ladder (utilsForConvertibleRIs), not inside this client. + require.Len(t, got, 2) + byID := make(map[string]RIUtilization, len(got)) + for _, u := range got { + byID[u.ReservedInstanceID] = u + } + assert.InDelta(t, 10.0, byID["rds-ri-1"].UtilizationPercent, 0.001) + assert.InDelta(t, 95.0, byID["ec2-ri-1"].UtilizationPercent, 0.001) +} + +// TestGetRIUtilization_EmptyRegionOmitsRegionFilter confirms that an empty +// region argument (ambient-credentials callers that haven't resolved a +// specific region) still scopes to EC2 via SERVICE alone, rather than +// erroring or building a Filter with an empty REGION value. +func TestGetRIUtilization_EmptyRegionOmitsRegionFilter(t *testing.T) { + mock := &mockUtilizationCE{utilizationOutput: &costexplorer.GetReservationUtilizationOutput{}} + client := NewClientWithAPI(mock, "us-east-1") + + _, err := client.GetRIUtilization(context.Background(), 30, "") + require.NoError(t, err) + + require.NotNil(t, mock.lastFilter) + assert.Nil(t, mock.lastFilter.And, "no region supplied -- Filter must be SERVICE-only, not an And with an empty REGION value") + require.NotNil(t, mock.lastFilter.Dimensions) + assert.Equal(t, types.DimensionService, mock.lastFilter.Dimensions.Key) + assert.Equal(t, []string{ec2ComputeService}, mock.lastFilter.Dimensions.Values) +} diff --git a/providers/aws/service_client.go b/providers/aws/service_client.go index 42cf84fef..8e5aa1f8d 100644 --- a/providers/aws/service_client.go +++ b/providers/aws/service_client.go @@ -163,9 +163,10 @@ func (r *RecommendationsClientAdapter) GetAllRecommendations(ctx context.Context return r.client.GetAllRecommendations(ctx) } -// GetRIUtilization gets per-RI utilization from Cost Explorer. -func (r *RecommendationsClientAdapter) GetRIUtilization(ctx context.Context, lookbackDays int) ([]recommendations.RIUtilization, error) { - return r.client.GetRIUtilization(ctx, lookbackDays) +// GetRIUtilization gets per-RI utilization from Cost Explorer, scoped to +// EC2 RIs in region. +func (r *RecommendationsClientAdapter) GetRIUtilization(ctx context.Context, lookbackDays int, region string) ([]recommendations.RIUtilization, error) { + return r.client.GetRIUtilization(ctx, lookbackDays, region) } // GetRICoverageMap returns the per-pool RI coverage % over the last From b60f2a922365faed5cd0e91d4be2227e93cf7d87 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 21 Jul 2026 21:57:58 +0200 Subject: [PATCH 2/2] fix(aws/ladder): correct comment reference from #1461 to #1361 The prior commit's code comments cited "#1461" as the tracking issue for the RI utilization filter bug. #1461 is an unrelated PR (NaN/Inf rejection in SP money parsing). The bug this fix addresses was introduced by PR #1361, per the review that surfaced it; correct all comment references accordingly. --- providers/aws/ladder/ladder_test.go | 4 ++-- providers/aws/ladder/layer_states.go | 2 +- providers/aws/recommendations/utilization.go | 2 +- providers/aws/recommendations/utilization_test.go | 6 +++--- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go index 8ba6c3a59..73b6d85cf 100644 --- a/providers/aws/ladder/ladder_test.go +++ b/providers/aws/ladder/ladder_test.go @@ -510,7 +510,7 @@ func TestGetLayerStates_RILayer_CoverageAndUtilization(t *testing.T) { // ReservedInstanceID must match the convertible RI's ID: GetLayerStates // intersects CE utilization entries against the account's convertible // RIs before aggregating, so an entry for an untracked ID would be - // dropped (issue #1461). + // dropped (PR #1361). utils := []recommendations.RIUtilization{ {ReservedInstanceID: "ri-1", PurchasedHours: 100, TotalActualHours: 90}, } @@ -534,7 +534,7 @@ func TestGetLayerStates_RILayer_CoverageAndUtilization(t *testing.T) { } // TestGetLayerStates_RILayer_UtilizationExcludesUnrelatedReservations is the -// GetLayerStates-side regression test for issue #1461. GetRIUtilization's +// GetLayerStates-side regression test for PR #1361. GetRIUtilization's // SERVICE+REGION filter narrows the CE response to EC2 RIs in-region, but a // standard (non-convertible) EC2 RI in the same account/region -- or, before // the fix, an RDS/ElastiCache/OpenSearch/Redshift reservation blended in by diff --git a/providers/aws/ladder/layer_states.go b/providers/aws/ladder/layer_states.go index 2d1f68591..37b339493 100644 --- a/providers/aws/ladder/layer_states.go +++ b/providers/aws/ladder/layer_states.go @@ -333,7 +333,7 @@ func computeEC2CoveragePct(coverageMap recommendations.PoolCoverageMap, region s // the CE response to EC2 RIs in this region, but a standard (non-convertible) // EC2 RI in the same account/region would still pass that filter; the ID // intersection restricts the aggregate to exactly the convertible RIs this -// buffer layer tracks (issue #1461). +// buffer layer tracks (PR #1361). func utilsForConvertibleRIs(utils []recommendations.RIUtilization, ris []ec2svc.ConvertibleRI) []recommendations.RIUtilization { ids := make(map[string]struct{}, len(ris)) for i := range ris { diff --git a/providers/aws/recommendations/utilization.go b/providers/aws/recommendations/utilization.go index 797fb5e2e..5e78afc12 100644 --- a/providers/aws/recommendations/utilization.go +++ b/providers/aws/recommendations/utilization.go @@ -38,7 +38,7 @@ type riAccumulator struct { // SUBSCRIPTION_ID-grouped number. Callers that read this as "EC2 // convertible-RI utilization for this region" (e.g. the ladder // ConvertibleRI layer) would then trigger real reshape/exchange -// decisions off an unrelated RI's utilization (issue #1461). region +// decisions off an unrelated RI's utilization (PR #1361). region // is optional: an empty string omits the REGION dimension, matching // callers that haven't resolved a specific region. func (c *Client) GetRIUtilization(ctx context.Context, lookbackDays int, region string) ([]RIUtilization, error) { diff --git a/providers/aws/recommendations/utilization_test.go b/providers/aws/recommendations/utilization_test.go index d4901b4a6..3ec1718cf 100644 --- a/providers/aws/recommendations/utilization_test.go +++ b/providers/aws/recommendations/utilization_test.go @@ -14,7 +14,7 @@ import ( // mockUtilizationCE extends the test mock with a configurable // GetReservationUtilization response and captures the Filter from the most // recent call so tests can assert the CE query was scoped correctly -// (issue #1461). +// (PR #1361). type mockUtilizationCE struct { mockCostExplorerAPI utilizationOutput *costexplorer.GetReservationUtilizationOutput @@ -31,7 +31,7 @@ func (m *mockUtilizationCE) GetReservationUtilization(ctx context.Context, param } // TestGetRIUtilization_ScopesToEC2AndRegion is the primary regression test -// for issue #1461: without a Filter, GetReservationUtilization blends +// for PR #1361: without a Filter, GetReservationUtilization blends // utilization across every reserved-resource type (RDS, EC2, ...) and every // region into one SUBSCRIPTION_ID-grouped number. It replicates the real // failing scenario -- an RDS reservation with low utilization and an EC2 @@ -77,7 +77,7 @@ func TestGetRIUtilization_ScopesToEC2AndRegion(t *testing.T) { got, err := client.GetRIUtilization(context.Background(), 30, "us-east-1") require.NoError(t, err) - require.NotNil(t, mock.lastFilter, "GetRIUtilization must send a Filter (issue #1461: an absent Filter blends every reserved-resource type and region into one number)") + require.NotNil(t, mock.lastFilter, "GetRIUtilization must send a Filter (PR #1361: an absent Filter blends every reserved-resource type and region into one number)") require.NotNil(t, mock.lastFilter.And, "Filter must And together SERVICE and REGION dimensions") require.Len(t, mock.lastFilter.And, 2) assert.Equal(t, types.DimensionService, mock.lastFilter.And[0].Dimensions.Key)