From 37131464e21b3f7e88ecb361c836f639907fb6d1 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 00:30:52 +0200 Subject: [PATCH 1/4] feat(aws): add an RI account scope option for recommendations GetReservationPurchaseRecommendation was hardcoded to AccountScope=LINKED. Add RecommendationParams.RIAccountScope (linked default, payer), validated by the same converter as SPAccountScope, so a management account can request payer-scope RI recommendations. Empty keeps linked; unknown values error. Closes #508 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++ pkg/common/types.go | 4 ++ providers/aws/recommendations/client.go | 7 ++- .../client_ri_account_scope_test.go | 61 +++++++++++++++++++ .../aws/recommendations/collection_sp.go | 8 ++- 5 files changed, 82 insertions(+), 2 deletions(-) create mode 100644 providers/aws/recommendations/client_ri_account_scope_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a53450a..4ef694d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Added +- **`RecommendationParams.RIAccountScope`:** selects the Cost Explorer account + scope (`linked` default, or `payer`) for Reserved Instance recommendations, + mirroring `SPAccountScope`. Empty keeps the previous linked behaviour; an + unknown value is rejected (#508). - **`recommendations.AdjustExistingCoverageForExpiringCommitmentsWithCoverageOptions`:** the per-pool (EC2 and other non-family) expiry deduction, plus credit for replacements already bought. Coverage is a lookback average, so RIs bought diff --git a/pkg/common/types.go b/pkg/common/types.go index ab7ad6bb..489d6fc4 100644 --- a/pkg/common/types.go +++ b/pkg/common/types.go @@ -554,6 +554,10 @@ type RecommendationParams struct { // recommendations: "linked" or "payer" (case-insensitive). Empty means // linked. Payer scope only applies to a management account. SPAccountScope string + // RIAccountScope selects the Cost Explorer account scope for Reserved + // Instance recommendations, with the same values as SPAccountScope: + // "linked" or "payer" (case-insensitive). Empty means linked. + RIAccountScope string // ExperimentalGCPPlanTerm (GCP Compute Engine only, ignored elsewhere) // takes the CUD term from the recommendation's root Commitment plan // instead of Term. EXPERIMENTAL: the payload shape is unverified against a diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index 08256220..3007a1a8 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -145,12 +145,17 @@ func (c *Client) GetRecommendations(ctx context.Context, params *common.Recommen return c.getSavingsPlansRecommendations(ctx, params) } + scope, err := convertAccountScope(params.RIAccountScope, "RI") + if err != nil { + return nil, err + } + input := &costexplorer.GetReservationPurchaseRecommendationInput{ Service: aws.String(getServiceStringForCostExplorer(params.Service)), PaymentOption: convertPaymentOption(params.PaymentOption), TermInYears: convertTermInYears(params.Term), LookbackPeriodInDays: convertLookbackPeriod(params.LookbackPeriod), - AccountScope: types.AccountScopeLinked, + AccountScope: scope, } if isEC2Service(params.Service) { diff --git a/providers/aws/recommendations/client_ri_account_scope_test.go b/providers/aws/recommendations/client_ri_account_scope_test.go new file mode 100644 index 00000000..601e89aa --- /dev/null +++ b/providers/aws/recommendations/client_ri_account_scope_test.go @@ -0,0 +1,61 @@ +package recommendations_test + +import ( + "context" + "encoding/json" + "io" + "net/http" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" +) + +type riScopeFixture struct{ scopes []string } + +func (f *riScopeFixture) Do(req *http.Request) (*http.Response, error) { + var input map[string]any + if err := json.NewDecoder(req.Body).Decode(&input); err != nil { + return nil, err + } + scope, _ := input["AccountScope"].(string) + f.scopes = append(f.scopes, scope) + return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/x-amz-json-1.1"}}, + Body: io.NopCloser(strings.NewReader(`{"Recommendations":[]}`)), Request: req}, nil +} + +func riScopeClient(f *riScopeFixture) *recommendations.Client { + return recommendations.NewClient(&aws.Config{Region: "us-east-1", HTTPClient: f, RetryMaxAttempts: 1, + Credentials: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { + return aws.Credentials{AccessKeyID: "synthetic", SecretAccessKey: "synthetic"}, nil + })}) +} + +func TestRIAccountScope(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ in, want string }{ + {"", "LINKED"}, {"linked", "LINKED"}, {"payer", "PAYER"}, {"PAYER", "PAYER"}, + } { + t.Run(tc.in, func(t *testing.T) { + t.Parallel() + f := &riScopeFixture{} + _, err := riScopeClient(f).GetRecommendations(context.Background(), &common.RecommendationParams{ + Service: common.ServiceRDS, Term: "1yr", PaymentOption: "no-upfront", LookbackPeriod: "7d", RIAccountScope: tc.in}) + require.NoError(t, err) + require.Equal(t, []string{tc.want}, f.scopes) + }) + } +} + +func TestRIAccountScopeRejectsUnknown(t *testing.T) { + t.Parallel() + f := &riScopeFixture{} + _, err := riScopeClient(f).GetRecommendations(context.Background(), &common.RecommendationParams{ + Service: common.ServiceRDS, Term: "1yr", PaymentOption: "no-upfront", LookbackPeriod: "7d", RIAccountScope: "org"}) + require.ErrorContains(t, err, "invalid RI account scope") + require.Empty(t, f.scopes) +} diff --git a/providers/aws/recommendations/collection_sp.go b/providers/aws/recommendations/collection_sp.go index 96ed4b23..e6981b64 100644 --- a/providers/aws/recommendations/collection_sp.go +++ b/providers/aws/recommendations/collection_sp.go @@ -62,13 +62,19 @@ func (c *Client) getSavingsPlansRecommendations(ctx context.Context, params *com // the Cost Explorer scope. Empty is linked, the long-standing default; an // unknown value is an error rather than a silent fallback. func convertSavingsPlansAccountScope(s string) (types.AccountScope, error) { + return convertAccountScope(s, "Savings Plans") +} + +// convertAccountScope is shared by the Savings Plans and RI recommendation +// paths so both accept the same values. +func convertAccountScope(s, kind string) (types.AccountScope, error) { switch strings.ToLower(s) { case "", "linked": return types.AccountScopeLinked, nil case "payer": return types.AccountScopePayer, nil } - return "", fmt.Errorf("invalid Savings Plans account scope %q: want linked or payer", s) + return "", fmt.Errorf("invalid %s account scope %q: want linked or payer", kind, s) } func (c *Client) fetchSPAllPages( From 0013799bbf12d1b02bc24982273a82d74ac21fb6 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 00:39:51 +0200 Subject: [PATCH 2/4] chore(aws): bump pkg for RIAccountScope Co-Authored-By: Claude Opus 5.5 --- providers/aws/go.mod | 2 +- providers/aws/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/providers/aws/go.mod b/providers/aws/go.mod index 4dabcff9..a1df881b 100644 --- a/providers/aws/go.mod +++ b/providers/aws/go.mod @@ -3,7 +3,7 @@ module github.com/LeanerCloud/cloud-commitments-go/providers/aws go 1.26.9 require ( - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261018003623-541e05e5fd29 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261019223052-37131464e21b github.com/aws/aws-sdk-go-v2 v1.41.5 github.com/aws/aws-sdk-go-v2/config v1.29.12 github.com/aws/aws-sdk-go-v2/credentials v1.17.65 diff --git a/providers/aws/go.sum b/providers/aws/go.sum index 22694c7a..8c9ae9ec 100644 --- a/providers/aws/go.sum +++ b/providers/aws/go.sum @@ -1,5 +1,5 @@ -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261018003623-541e05e5fd29 h1:QJkoZBHkwjvMGplDb1TcuBy3HdYPxlZ+AyR/7QEU8Hc= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261018003623-541e05e5fd29/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261019223052-37131464e21b h1:0JK0YAYhPL3nOD+UKZZlcCtsvJ3yhjj1JeHZWgAll8g= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261019223052-37131464e21b/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo= From f84cff411c26cf42c4cc8573452d9b40f21538be Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 01:06:10 +0200 Subject: [PATCH 3/4] fix(aws): thread RI account scope through GetRecommendationsForService Add Client.SetRIAccountScope, mirroring SetRecLookbackPeriod, so the per-service discovery path no longer always requests LINKED. Co-Authored-By: Claude Opus 5.5 --- providers/aws/recommendations/client.go | 10 +++++++++ .../client_ri_account_scope_test.go | 22 ++++++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index 3007a1a8..26bbdbe0 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -70,6 +70,9 @@ type Client struct { // recLookbackPeriod is forwarded to GetReservationPurchaseRecommendation // as LookbackPeriodInDays. Defaults to "7d" when empty. recLookbackPeriod string + // riAccountScope ("linked" or "payer"; empty is linked) is forwarded by + // GetRecommendationsForService, which takes no params. + riAccountScope string } // NewClient creates a new recommendations client. @@ -131,6 +134,12 @@ func (c *Client) SetRecLookbackPeriod(period string) { c.recLookbackPeriod = period } +// SetRIAccountScope configures the Cost Explorer account scope used by +// GetRecommendationsForService: "linked" (default) or "payer". +func (c *Client) SetRIAccountScope(scope string) { + c.riAccountScope = scope +} + // GetRecommendations fetches Reserved Instance recommendations for any service. func (c *Client) GetRecommendations(ctx context.Context, params *common.RecommendationParams) ([]common.Recommendation, error) { if params == nil { @@ -277,6 +286,7 @@ func (c *Client) fetchSingleComboRecs(ctx context.Context, service common.Servic Term: term, LookbackPeriod: lookback, Region: "", + RIAccountScope: c.riAccountScope, } recs, err := c.GetRecommendations(ctx, ¶ms) if ctx.Err() != nil { diff --git a/providers/aws/recommendations/client_ri_account_scope_test.go b/providers/aws/recommendations/client_ri_account_scope_test.go index 601e89aa..3e30cdc1 100644 --- a/providers/aws/recommendations/client_ri_account_scope_test.go +++ b/providers/aws/recommendations/client_ri_account_scope_test.go @@ -23,7 +23,9 @@ func (f *riScopeFixture) Do(req *http.Request) (*http.Response, error) { return nil, err } scope, _ := input["AccountScope"].(string) - f.scopes = append(f.scopes, scope) + if scope != "" { + f.scopes = append(f.scopes, scope) + } return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/x-amz-json-1.1"}}, Body: io.NopCloser(strings.NewReader(`{"Recommendations":[]}`)), Request: req}, nil } @@ -59,3 +61,21 @@ func TestRIAccountScopeRejectsUnknown(t *testing.T) { require.ErrorContains(t, err, "invalid RI account scope") require.Empty(t, f.scopes) } + +func TestRIAccountScopeForService(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ in, want string }{{"", "LINKED"}, {"payer", "PAYER"}} { + t.Run(tc.in, func(t *testing.T) { + t.Parallel() + f := &riScopeFixture{} + c := riScopeClient(f) + c.SetRIAccountScope(tc.in) + _, err := c.GetRecommendationsForService(context.Background(), common.ServiceRDS) + require.NoError(t, err) + require.Len(t, f.scopes, 6) + for _, s := range f.scopes { + require.Equal(t, tc.want, s) + } + }) + } +} From 06f0eaf9668a0db4affd2531c2c2b2204989f9f6 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 01:07:32 +0200 Subject: [PATCH 4/4] revert: drop SetRIAccountScope until a consumer needs it No reachable caller: the adapter does not wrap it and the cli path uses GetRecommendations with params. Co-Authored-By: Claude Opus 5.5 --- providers/aws/recommendations/client.go | 10 --------- .../client_ri_account_scope_test.go | 22 +------------------ 2 files changed, 1 insertion(+), 31 deletions(-) diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index 26bbdbe0..3007a1a8 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -70,9 +70,6 @@ type Client struct { // recLookbackPeriod is forwarded to GetReservationPurchaseRecommendation // as LookbackPeriodInDays. Defaults to "7d" when empty. recLookbackPeriod string - // riAccountScope ("linked" or "payer"; empty is linked) is forwarded by - // GetRecommendationsForService, which takes no params. - riAccountScope string } // NewClient creates a new recommendations client. @@ -134,12 +131,6 @@ func (c *Client) SetRecLookbackPeriod(period string) { c.recLookbackPeriod = period } -// SetRIAccountScope configures the Cost Explorer account scope used by -// GetRecommendationsForService: "linked" (default) or "payer". -func (c *Client) SetRIAccountScope(scope string) { - c.riAccountScope = scope -} - // GetRecommendations fetches Reserved Instance recommendations for any service. func (c *Client) GetRecommendations(ctx context.Context, params *common.RecommendationParams) ([]common.Recommendation, error) { if params == nil { @@ -286,7 +277,6 @@ func (c *Client) fetchSingleComboRecs(ctx context.Context, service common.Servic Term: term, LookbackPeriod: lookback, Region: "", - RIAccountScope: c.riAccountScope, } recs, err := c.GetRecommendations(ctx, ¶ms) if ctx.Err() != nil { diff --git a/providers/aws/recommendations/client_ri_account_scope_test.go b/providers/aws/recommendations/client_ri_account_scope_test.go index 3e30cdc1..601e89aa 100644 --- a/providers/aws/recommendations/client_ri_account_scope_test.go +++ b/providers/aws/recommendations/client_ri_account_scope_test.go @@ -23,9 +23,7 @@ func (f *riScopeFixture) Do(req *http.Request) (*http.Response, error) { return nil, err } scope, _ := input["AccountScope"].(string) - if scope != "" { - f.scopes = append(f.scopes, scope) - } + f.scopes = append(f.scopes, scope) return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/x-amz-json-1.1"}}, Body: io.NopCloser(strings.NewReader(`{"Recommendations":[]}`)), Request: req}, nil } @@ -61,21 +59,3 @@ func TestRIAccountScopeRejectsUnknown(t *testing.T) { require.ErrorContains(t, err, "invalid RI account scope") require.Empty(t, f.scopes) } - -func TestRIAccountScopeForService(t *testing.T) { - t.Parallel() - for _, tc := range []struct{ in, want string }{{"", "LINKED"}, {"payer", "PAYER"}} { - t.Run(tc.in, func(t *testing.T) { - t.Parallel() - f := &riScopeFixture{} - c := riScopeClient(f) - c.SetRIAccountScope(tc.in) - _, err := c.GetRecommendationsForService(context.Background(), common.ServiceRDS) - require.NoError(t, err) - require.Len(t, f.scopes, 6) - for _, s := range f.scopes { - require.Equal(t, tc.want, s) - } - }) - } -}