From 051639f3fa54461e88e6fd022363b13c7d20a083 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Thu, 22 Oct 2026 21:45:56 +0100 Subject: [PATCH 1/5] chore(deps): bump cloud-commitments-go providers/aws to 92722e6 for the aws-cn Savings Plans refusal Pins providers/aws to the cloud-commitments-go#566 merge commit, which makes Savings Plans offering lookups, validation and purchases fail with savingsplans.ErrUnsupportedPartition before any AWS call when a region they see is cn-* (aws-cn bills in CNY and the purchase path is USD-only). MVS moves pkg to ff6225d, the version that commit requires. Refs #1194 Co-Authored-By: Claude Opus 5.5 --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index aec966c02..11b7324e7 100644 --- a/go.mod +++ b/go.mod @@ -84,8 +84,8 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations v1.1.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.4.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261018003623-541e05e5fd29 - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261019192600-aeb8a6f7ff51 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261020053130-ff6225d5ff04 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261020071945-92722e673741 github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261017134002-3def3f898617 github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261017134002-3def3f898617 github.com/aws/aws-lambda-go v1.47.0 diff --git a/go.sum b/go.sum index 3a32f2465..cdfbbaece 100644 --- a/go.sum +++ b/go.sum @@ -92,10 +92,10 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc= -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/providers/aws v0.0.0-20261019192600-aeb8a6f7ff51 h1:fr8mS6aF7/y69cJ5UlF9MEvP15mSlH/4yKg25Cvk/YM= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261019192600-aeb8a6f7ff51/go.mod h1:p6TrYRa4+lvcdsPCAJ+sa0tI0sRfGQ/0YT1Af3fZk3w= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261020053130-ff6225d5ff04 h1:D595qtoMszl7dkETbmVFzn+WPECQ7HuiKnsMlFI7bLA= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261020053130-ff6225d5ff04/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261020071945-92722e673741 h1:AlfTl8EpfcRIWjU1FIx/XN+4ZgJjktjP/gPlRylPoF8= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261020071945-92722e673741/go.mod h1:cx+GF1J6RKy0CTOs2qhy+DM9rwFbKf3g0JIhYwqWz9k= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261017134002-3def3f898617 h1:K9wBX9ac1QwhkjW7BvDt0gl2Kt9fGdDytJyndoPRcBU= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261017134002-3def3f898617/go.mod h1:iOwl8e9EKGb3VQ4u/YjiqVzglhD6/GX1ABC3tBkqu0A= github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261017134002-3def3f898617 h1:VYVbQyoR+tIcPoXhkviclexKuU5H+A21hSXudB+kZag= From 0c4b13baf08f415f558b4f7e2d70c78f9debc920 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Thu, 22 Oct 2026 21:45:56 +0100 Subject: [PATCH 2/5] fix(accounts): return an unsupported-partition notice for aws-cn accounts aws-cn accounts were onboarded with no hint that CUDly cannot buy Savings Plans for them. Account responses (create, update, get, list and the self-account) now carry a derived, non-persisted "notices" list; an AWS account whose role ARN is in aws-cn (or a self-account when CUDly itself runs in aws-cn) gets an "unsupported_partition" notice. Onboarding is still allowed so existing aws-cn accounts are not stranded. The ARN partition parsing moves to config.AWSPartitionFromARN so the account model and the STS host-identity path share it. Refs #1194 Co-Authored-By: Claude Opus 5.5 --- internal/api/handler.go | 22 +-- internal/api/handler_accounts.go | 29 +++- .../handler_accounts_partition_notice_test.go | 148 ++++++++++++++++++ internal/config/aws_partition.go | 67 ++++++++ internal/config/aws_partition_test.go | 40 +++++ internal/config/types.go | 3 + 6 files changed, 284 insertions(+), 25 deletions(-) create mode 100644 internal/api/handler_accounts_partition_notice_test.go create mode 100644 internal/config/aws_partition.go create mode 100644 internal/config/aws_partition_test.go diff --git a/internal/api/handler.go b/internal/api/handler.go index a5a1df9e6..ed8376df6 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -1147,25 +1147,5 @@ func (h *Handler) resolveAWSCallerIdentity(ctx context.Context) (string, string, // attacker-controlled tokens into a JSON snippet the operator copy- // pastes into IAM. func parseArnPartition(arn string) string { - const prefix = "arn:" - if len(arn) <= len(prefix) || arn[:len(prefix)] != prefix { - return "" - } - rest := arn[len(prefix):] - end := -1 - for i := 0; i < len(rest); i++ { - if rest[i] == ':' { - end = i - break - } - } - if end <= 0 { - return "" - } - switch rest[:end] { - case "aws", "aws-cn", "aws-us-gov": - return rest[:end] - default: - return "" - } + return config.AWSPartitionFromARN(arn) } diff --git a/internal/api/handler_accounts.go b/internal/api/handler_accounts.go index a0e51ee1e..41adffee4 100644 --- a/internal/api/handler_accounts.go +++ b/internal/api/handler_accounts.go @@ -121,6 +121,7 @@ func (h *Handler) listAccounts(ctx context.Context, req *events.LambdaFunctionUR } // Mark the self-account (the account matching CUDly's own host identity) + // and attach the derived notices (issue #1194). h.markSelfAccount(ctx, accts) return accts, nil @@ -187,17 +188,33 @@ func (h *Handler) listAccountsMinimal(ctx context.Context, req *events.LambdaFun return summaries, nil } -// markSelfAccount sets IsSelf=true on the account matching the source identity. +// markSelfAccount sets IsSelf=true on the account matching the source +// identity, then attaches each account's derived notices. The self-account +// has no role ARN, so its notices use CUDly's own partition from STS. func (h *Handler) markSelfAccount(ctx context.Context, accts []config.CloudAccount) { si := h.resolveSourceIdentity(ctx) - if si == nil || si.ExternalID() == "" { - return + var hostPartition string + if si != nil { + hostPartition = si.Partition } for i := range accts { - if accts[i].Provider == si.Provider && accts[i].ExternalID == si.ExternalID() { + if si != nil && si.ExternalID() != "" && accts[i].Provider == si.Provider && accts[i].ExternalID == si.ExternalID() { accts[i].IsSelf = true } + annotateAccountNotices(&accts[i], hostPartition) + } +} + +// annotateAccountNotices sets the derived notices on acct, such as the aws-cn +// Savings Plans limitation (issue #1194). The partition comes from the +// account's role ARN; a self-account with no role ARN falls back to +// hostPartition, CUDly's own partition ("" when unknown). +func annotateAccountNotices(acct *config.CloudAccount, hostPartition string) { + partition := acct.AWSPartition() + if partition == "" && acct.IsSelf && acct.Provider == "aws" { + partition = hostPartition } + acct.Notices = config.AWSPartitionNotices(partition) } // buildAccountFilter constructs a CloudAccountFilter from query parameters. @@ -255,6 +272,7 @@ func (h *Handler) createSelfAccount(ctx context.Context, httpReq *events.LambdaF } account.IsSelf = true + annotateAccountNotices(account, si.Partition) return account, nil } @@ -328,6 +346,7 @@ func (h *Handler) createAccount(ctx context.Context, httpReq *events.LambdaFunct return nil, classifyStoreError(err, fmt.Sprintf("an account with external ID %q already exists for %s", req.ExternalID, req.Provider)) } + annotateAccountNotices(account, "") return account, nil } @@ -543,6 +562,7 @@ func (h *Handler) getAccount(ctx context.Context, req *events.LambdaFunctionURLR return nil, err } + annotateAccountNotices(account, "") return account, nil } @@ -583,6 +603,7 @@ func (h *Handler) updateAccount(ctx context.Context, httpReq *events.LambdaFunct return nil, classifyStoreError(err, fmt.Sprintf("an account with external ID %q already exists for %s", req.ExternalID, req.Provider)) } + annotateAccountNotices(account, "") return account, nil } diff --git a/internal/api/handler_accounts_partition_notice_test.go b/internal/api/handler_accounts_partition_notice_test.go new file mode 100644 index 000000000..5fd26a3fd --- /dev/null +++ b/internal/api/handler_accounts_partition_notice_test.go @@ -0,0 +1,148 @@ +package api + +import ( + "context" + "encoding/json" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Issue #1194: aws-cn accounts are onboarded (blocking them could strand +// existing ones), but every account response carries a visible +// unsupported_partition notice so the UI can tell the operator CUDly cannot +// buy Savings Plans for them. + +const ( + cnRoleARN = "arn:aws-cn:iam::123456789012:role/CUDly" + commercialRoleARN = "arn:aws:iam::123456789012:role/CUDly" + partitionTestID = "11111111-1111-1111-1111-111111111111" +) + +func requireUnsupportedPartitionNotice(t *testing.T, acct *config.CloudAccount) { + t.Helper() + require.Len(t, acct.Notices, 1) + assert.Equal(t, config.AccountNoticeUnsupportedPartition, acct.Notices[0].Code) + assert.Contains(t, acct.Notices[0].Message, "aws-cn") + assert.Contains(t, acct.Notices[0].Message, "Savings Plans") + + raw, err := json.Marshal(acct) + require.NoError(t, err) + assert.Contains(t, string(raw), `"notices":[{"code":"unsupported_partition"`) +} + +func requireNoNotices(t *testing.T, acct *config.CloudAccount) { + t.Helper() + assert.Empty(t, acct.Notices) + raw, err := json.Marshal(acct) + require.NoError(t, err) + assert.NotContains(t, string(raw), `"notices"`) +} + +func roleARNAccountBody(roleARN string) string { + return `{"name":"Acme","provider":"aws","external_id":"123456789012",` + + `"aws_auth_mode":"role_arn","aws_role_arn":"` + roleARN + `",` + + `"aws_external_id":"550e8400-e29b-41d4-a716-446655440000"}` +} + +func TestCreateAccount_AWSChinaRoleARN_ReturnsUnsupportedPartitionNotice(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + setupAdminAuth(ctx, mockAuth) + handler := &Handler{auth: mockAuth, config: setupAdminMock(ctx)} + + result, err := handler.createAccount(ctx, adminRequest(roleARNAccountBody(cnRoleARN))) + require.NoError(t, err, "aws-cn accounts are still onboarded") + requireUnsupportedPartitionNotice(t, result.(*config.CloudAccount)) +} + +func TestCreateAccount_CommercialRoleARN_HasNoNotice(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + setupAdminAuth(ctx, mockAuth) + handler := &Handler{auth: mockAuth, config: setupAdminMock(ctx)} + + result, err := handler.createAccount(ctx, adminRequest(roleARNAccountBody(commercialRoleARN))) + require.NoError(t, err) + requireNoNotices(t, result.(*config.CloudAccount)) +} + +func TestUpdateAccount_AWSChinaRoleARN_ReturnsUnsupportedPartitionNotice(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + setupAdminAuth(ctx, mockAuth) + existing := sampleAccount() + handler := &Handler{auth: mockAuth, config: &mockConfigStoreAccounts{MockConfigStore: setupAdminMock(ctx), getResult: &existing}} + + result, err := handler.updateAccount(ctx, adminRequest(roleARNAccountBody(cnRoleARN)), partitionTestID) + require.NoError(t, err) + requireUnsupportedPartitionNotice(t, result.(*config.CloudAccount)) +} + +func TestGetAccount_AWSChinaRoleARN_ReturnsUnsupportedPartitionNotice(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + setupAdminAuth(ctx, mockAuth) + acct := sampleAccount() + acct.AWSAuthMode = "role_arn" + acct.AWSRoleARN = cnRoleARN + handler := &Handler{auth: mockAuth, config: &mockConfigStoreAccounts{MockConfigStore: setupAdminMock(ctx), getResult: &acct}} + + result, err := handler.getAccount(ctx, adminRequest(""), partitionTestID) + require.NoError(t, err) + requireUnsupportedPartitionNotice(t, result.(*config.CloudAccount)) +} + +// TestListAccounts_PartitionNotices covers the list view, including the +// self-account, which has no role ARN and takes CUDly's own partition. +func TestListAccounts_PartitionNotices(t *testing.T) { + cases := []struct { + name string + hostPartition string + wantSelf bool + }{ + {"CUDly hosted in aws-cn flags its self-account", config.AWSPartitionChina, true}, + {"CUDly hosted in aws leaves its self-account alone", config.AWSPartitionStandard, false}, + {"unknown host partition leaves the self-account alone", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + mockAuth := new(MockAuthService) + setupAdminAuth(ctx, mockAuth) + + cn := sampleAccount() + cn.ID, cn.Name, cn.AWSAuthMode, cn.AWSRoleARN = "cn", "China", "role_arn", cnRoleARN + commercial := sampleAccount() + commercial.ID, commercial.Name, commercial.AWSAuthMode, commercial.AWSRoleARN = "com", "Commercial", "role_arn", commercialRoleARN + self := sampleAccount() + self.ID, self.Name, self.ExternalID, self.AWSAuthMode = "self", "CUDly host", "111111111111", "role_arn" + azure := config.CloudAccount{ID: "az", Name: "Azure", Provider: "azure", ExternalID: "sub-1"} + + handler := &Handler{auth: mockAuth, config: &mockConfigStoreAccounts{ + MockConfigStore: setupAdminMock(ctx), + listResult: []config.CloudAccount{cn, commercial, self, azure}, + }} + handler.sourceIdentityOnce.Do(func() { + handler.sourceID = &sourceIdentity{Provider: "aws", AccountID: "111111111111", Partition: tc.hostPartition} + }) + + result, err := handler.listAccounts(ctx, adminRequest("")) + require.NoError(t, err) + got := result.([]config.CloudAccount) + require.Len(t, got, 4) + + requireUnsupportedPartitionNotice(t, &got[0]) + requireNoNotices(t, &got[1]) + require.True(t, got[2].IsSelf) + if tc.wantSelf { + requireUnsupportedPartitionNotice(t, &got[2]) + } else { + requireNoNotices(t, &got[2]) + } + requireNoNotices(t, &got[3]) + }) + } +} diff --git a/internal/config/aws_partition.go b/internal/config/aws_partition.go new file mode 100644 index 000000000..cf189c869 --- /dev/null +++ b/internal/config/aws_partition.go @@ -0,0 +1,67 @@ +package config + +import "strings" + +// AWS partition names as they appear in the second segment of an ARN. +const ( + AWSPartitionStandard = "aws" + AWSPartitionChina = "aws-cn" + AWSPartitionGovCloud = "aws-us-gov" +) + +// AccountNoticeUnsupportedPartition is the AccountNotice code for an AWS +// account in a partition where CUDly cannot buy Savings Plans (issue #1194). +const AccountNoticeUnsupportedPartition = "unsupported_partition" + +// awsChinaNoticeMessage is the user-facing text of the aws-cn notice. +const awsChinaNoticeMessage = "This is an AWS China (aws-cn) account. CUDly cannot buy Savings Plans " + + "for it: AWS bills aws-cn Savings Plans in CNY and CUDly's purchase path is USD-only, so " + + "Savings Plans purchases for this account are refused. Recommendations may also be " + + "unavailable, because CUDly reaches Cost Explorer through us-east-1." + +// AccountNotice is a derived, non-persisted warning about a cloud account, +// returned with the account so the UI can show it. +type AccountNotice struct { + Code string `json:"code"` + Message string `json:"message"` +} + +// AWSPartitionFromARN returns the partition segment of an AWS ARN +// (arn:::...). It returns "" for inputs that are not +// ARNs or that name a partition other than aws, aws-cn and aws-us-gov. +func AWSPartitionFromARN(arn string) string { + rest, ok := strings.CutPrefix(arn, "arn:") + if !ok { + return "" + } + partition, _, ok := strings.Cut(rest, ":") + if !ok { + return "" + } + switch partition { + case AWSPartitionStandard, AWSPartitionChina, AWSPartitionGovCloud: + return partition + default: + return "" + } +} + +// AWSPartition returns the AWS partition of the account, taken from its role +// ARN. It returns "" for non-AWS accounts and for AWS accounts with no role +// ARN (access_keys mode, or a self-account on ambient credentials), whose +// partition is not known without calling AWS. +func (a *CloudAccount) AWSPartition() string { + if a == nil || a.Provider != "aws" { + return "" + } + return AWSPartitionFromARN(strings.TrimSpace(a.AWSRoleARN)) +} + +// AWSPartitionNotices returns the notices for an AWS account in the given +// partition, or nil when the partition needs none. +func AWSPartitionNotices(partition string) []AccountNotice { + if partition != AWSPartitionChina { + return nil + } + return []AccountNotice{{Code: AccountNoticeUnsupportedPartition, Message: awsChinaNoticeMessage}} +} diff --git a/internal/config/aws_partition_test.go b/internal/config/aws_partition_test.go new file mode 100644 index 000000000..9eb825577 --- /dev/null +++ b/internal/config/aws_partition_test.go @@ -0,0 +1,40 @@ +package config + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCloudAccountAWSPartition(t *testing.T) { + cases := []struct { + name string + account *CloudAccount + want string + }{ + {"nil account", nil, ""}, + {"aws-cn role ARN", &CloudAccount{Provider: "aws", AWSRoleARN: "arn:aws-cn:iam::123456789012:role/CUDly"}, AWSPartitionChina}, + {"padded aws-cn role ARN", &CloudAccount{Provider: "aws", AWSRoleARN: " arn:aws-cn:iam::123456789012:role/CUDly "}, AWSPartitionChina}, + {"GovCloud role ARN", &CloudAccount{Provider: "aws", AWSRoleARN: "arn:aws-us-gov:iam::123456789012:role/CUDly"}, AWSPartitionGovCloud}, + {"commercial role ARN", &CloudAccount{Provider: "aws", AWSRoleARN: "arn:aws:iam::123456789012:role/CUDly"}, AWSPartitionStandard}, + {"no role ARN", &CloudAccount{Provider: "aws", AWSAuthMode: "access_keys"}, ""}, + {"unknown partition", &CloudAccount{Provider: "aws", AWSRoleARN: "arn:aws-iso:iam::123456789012:role/CUDly"}, ""}, + {"non-AWS account ignores the ARN field", &CloudAccount{Provider: "azure", AWSRoleARN: "arn:aws-cn:iam::123456789012:role/CUDly"}, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, tc.account.AWSPartition()) + }) + } +} + +func TestAWSPartitionNotices(t *testing.T) { + notices := AWSPartitionNotices(AWSPartitionChina) + if assert.Len(t, notices, 1) { + assert.Equal(t, AccountNoticeUnsupportedPartition, notices[0].Code) + assert.Contains(t, notices[0].Message, "Savings Plans") + } + for _, p := range []string{"", AWSPartitionStandard, AWSPartitionGovCloud} { + assert.Nil(t, AWSPartitionNotices(p), "partition %q", p) + } +} diff --git a/internal/config/types.go b/internal/config/types.go index 18d5c1916..b479d50d8 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -1228,6 +1228,9 @@ type CloudAccount struct { CredentialsConfigured bool `json:"credentials_configured"` BastionAccountName string `json:"bastion_account_name,omitempty"` IsSelf bool `json:"is_self,omitempty"` + // Notices are warnings the API derives for the response, such as the + // aws-cn Savings Plans limitation (issue #1194). + Notices []AccountNotice `json:"notices,omitempty"` CreatedAt time.Time `json:"created_at"` UpdatedAt time.Time `json:"updated_at"` From d65cab58b468eae7b4ec3321dc8b2d32f10f0369 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Thu, 22 Oct 2026 21:45:56 +0100 Subject: [PATCH 3/5] fix(purchase): refuse aws-cn Savings Plans purchases before any AWS call A Savings Plans rec for an account whose role ARN is in aws-cn now fails with an error wrapping savingsplans.ErrUnsupportedPartition before a provider is built, on both the plan fan-out and the single-account (direct-execute) paths. Other recs in the same execution still run, so a mixed run ends partially_completed as before. The library's region-based guard stays as the backstop for accounts with no role ARN. Closes #1194 Co-Authored-By: Claude Opus 5.5 --- internal/purchase/execution.go | 71 +++--- internal/purchase/execution_test.go | 2 +- .../purchase/money_path_regression_test.go | 6 +- internal/purchase/partition_guard.go | 31 +++ internal/purchase/partition_guard_test.go | 205 ++++++++++++++++++ internal/purchase/plan_step_proofs_test.go | 2 +- 6 files changed, 286 insertions(+), 31 deletions(-) create mode 100644 internal/purchase/partition_guard.go create mode 100644 internal/purchase/partition_guard_test.go diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 4e80d4671..936bac128 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -100,7 +100,7 @@ func (m *Manager) executePurchase(ctx context.Context, exec *config.PurchaseExec // plain error when nothing committed. Split out of executePurchase to keep that // function under the gocyclo budget. func (m *Manager) executeSingleAccount(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan) error { - provCfg, targetAccountID, err := m.prepareSingleAccountSubmission(ctx, exec) + provCfg, target, err := m.prepareSingleAccountSubmission(ctx, exec) if err != nil { return err } @@ -110,7 +110,7 @@ func (m *Manager) executeSingleAccount(ctx context.Context, exec *config.Purchas // Azure / GCP). Only fall back to the ambient AWS STS identity when no // target account could be identified (truly-ambient AWS single-account // execution where credentials are inherited from the host). - accountID := targetAccountID + accountID := target.externalID if accountID == "" { // Fall back to the ambient AWS STS identity for logging/tagging. // getAWSAccountID returns ("", error) on failure so the caller @@ -124,7 +124,7 @@ func (m *Manager) executeSingleAccount(ctx context.Context, exec *config.Purchas } } - totalSavings, totalUpfront, purchaseErrors, procErr := m.processPurchaseRecommendations(ctx, exec, plan, accountID, provCfg) + totalSavings, totalUpfront, purchaseErrors, procErr := m.processPurchaseRecommendations(ctx, exec, plan, accountID, provCfg, target.awsPartition) if procErr != nil { return procErr } @@ -157,18 +157,29 @@ func (m *Manager) executeSingleAccount(ctx context.Context, exec *config.Purchas // (issue #97; see fan_out_shape.go). Every error it returns is before // submission. Split out of executeSingleAccount to keep that function under // the gocyclo budget. -func (m *Manager) prepareSingleAccountSubmission(ctx context.Context, exec *config.PurchaseExecution) (*provider.ProviderConfig, string, error) { +func (m *Manager) prepareSingleAccountSubmission(ctx context.Context, exec *config.PurchaseExecution) (*provider.ProviderConfig, purchaseTarget, error) { if err := requireRecsMatchRowAccount(exec); err != nil { - return nil, "", beforeSubmission(err) + return nil, purchaseTarget{}, beforeSubmission(err) } - provCfg, targetAccountID, err := m.resolveSingleAccountProvider(ctx, exec) + provCfg, target, err := m.resolveSingleAccountProvider(ctx, exec) if err != nil { - return nil, "", beforeSubmission(err) + return nil, purchaseTarget{}, beforeSubmission(err) } if err := m.pinLineageShape(ctx, exec, config.FanOutShapeSingle); err != nil { - return nil, "", err // already beforeSubmission + return nil, purchaseTarget{}, err // already beforeSubmission } - return provCfg, targetAccountID, nil + return provCfg, target, nil +} + +// purchaseTarget identifies the account a single-account execution buys for. +// The zero value means no target account was identified (ambient credentials). +type purchaseTarget struct { + // externalID is the provider-appropriate account identifier stamped on + // purchase_history (#646). + externalID string + // awsPartition is the account's AWS partition from its role ARN, used to + // refuse aws-cn Savings Plans before any AWS call (issue #1194). + awsPartition string } // anyRecPurchased reports whether at least one recommendation in the slice @@ -319,7 +330,7 @@ func (m *Manager) executeForAccount(ctx context.Context, baseExec *config.Purcha } accountID := account.ExternalID - totalSavings, totalUpfront, purchaseErrors, procErr := m.processPurchaseRecommendations(ctx, &acctExec, plan, accountID, provCfg) + totalSavings, totalUpfront, purchaseErrors, procErr := m.processPurchaseRecommendations(ctx, &acctExec, plan, accountID, provCfg, account.AWSPartition()) if procErr != nil { acctExec.Status = "failed" acctExec.Error = procErr.Error() @@ -406,15 +417,17 @@ func applyAccountOutcome(acctExec *config.PurchaseExecution, purchaseErrors []st } // resolveSingleAccountProvider derives per-account credentials for the -// single-account execution path. Returns (nil, "", nil) when no account can be -// identified (ambient credentials are used in that case), or when no recs are -// selected (approval-only flows where credentials are not needed). +// single-account execution path. Returns (nil, purchaseTarget{}, nil) when no +// account can be identified (ambient credentials are used in that case), or +// when no recs are selected (approval-only flows where credentials are not +// needed). // -// The second return value is the resolved target account's ExternalID — the -// provider-appropriate account identifier (AWS account number, Azure +// The second return value carries the resolved target account's ExternalID — +// the provider-appropriate account identifier (AWS account number, Azure // subscription, GCP project) that the caller stamps onto purchase_history -// (#646). It is "" when no target account could be identified, signaling the -// caller to fall back to the ambient AWS STS identity. +// (#646) — and its AWS partition (issue #1194). The ExternalID is "" when no +// target account could be identified, signaling the caller to fall back to +// the ambient AWS STS identity. // // The account is taken from exec.CloudAccountID when set (plan-with-single- // account executions), or derived from the shared cloud_account_id of the @@ -428,13 +441,13 @@ func applyAccountOutcome(acctExec *config.PurchaseExecution, purchaseErrors []st // resolved (lookup failed, the account does not exist, or credentials could // not be derived); the caller must NOT fall back to ambient credentials on // error, as that would purchase/stamp against the wrong account (#646). -func (m *Manager) resolveSingleAccountProvider(ctx context.Context, exec *config.PurchaseExecution) (*provider.ProviderConfig, string, error) { +func (m *Manager) resolveSingleAccountProvider(ctx context.Context, exec *config.PurchaseExecution) (*provider.ProviderConfig, purchaseTarget, error) { // Skip credential resolution when nothing is selected. Approval-only // flows may carry an account ID on the recs solely for the contact-email // gate; attempting resolution there would fail on accounts with no // provider field set and add a pointless DB round-trip. if len(selectedIndices(exec.Recommendations)) == 0 { - return nil, "", nil + return nil, purchaseTarget{}, nil } cloudAccountID := exec.CloudAccountID @@ -442,16 +455,16 @@ func (m *Manager) resolveSingleAccountProvider(ctx context.Context, exec *config var scopeErr error cloudAccountID, scopeErr = SingleCloudAccountIDFromRecs(exec.Recommendations) if scopeErr != nil { - return nil, "", fmt.Errorf("execution %s: %w", exec.ExecutionID, scopeErr) + return nil, purchaseTarget{}, fmt.Errorf("execution %s: %w", exec.ExecutionID, scopeErr) } } if cloudAccountID == nil { - return nil, "", nil + return nil, purchaseTarget{}, nil } account, err := m.config.GetCloudAccount(ctx, *cloudAccountID) if err != nil { - return nil, "", fmt.Errorf("credential resolution failed for account %s: %w", *cloudAccountID, err) + return nil, purchaseTarget{}, fmt.Errorf("credential resolution failed for account %s: %w", *cloudAccountID, err) } if account == nil { // A target account was identified but does not exist in config. @@ -459,13 +472,13 @@ func (m *Manager) resolveSingleAccountProvider(ctx context.Context, exec *config // credentials and purchase/stamp against the wrong account (#646), so // surface an error instead; the contract above forbids ambient // fallback once a target account ID is known. - return nil, "", fmt.Errorf("credential resolution failed for account %s: account not found", *cloudAccountID) + return nil, purchaseTarget{}, fmt.Errorf("credential resolution failed for account %s: account not found", *cloudAccountID) } provCfg, err := m.resolveAccountProvider(ctx, *account) if err != nil { - return nil, "", fmt.Errorf("credential resolution failed for account %s: %w", *cloudAccountID, err) + return nil, purchaseTarget{}, fmt.Errorf("credential resolution failed for account %s: %w", *cloudAccountID, err) } - return provCfg, account.ExternalID, nil + return provCfg, purchaseTarget{externalID: account.ExternalID, awsPartition: account.AWSPartition()}, nil } // partialPurchaseError is the sentinel returned by the single-account @@ -603,7 +616,7 @@ type recPurchaseOutcome struct { index int } -func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan, accountID string, provCfg *provider.ProviderConfig) (totalSavings, totalUpfront float64, purchaseErrors []string, procErr error) { +func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan, accountID string, provCfg *provider.ProviderConfig, awsPartition string) (totalSavings, totalUpfront float64, purchaseErrors []string, procErr error) { // ExecutionID is carried into PurchaseOptions so executeSinglePurchase // can tag every per-rec log line with the owning exec UUID. Without // this, CloudWatch filtering by exec ID returns zero hits and a stuck @@ -668,6 +681,12 @@ func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *conf return recPurchaseOutcome{}, fmt.Errorf("fan-out index %d out of range for %d recommendations", i, len(exec.Recommendations)) } rec := exec.Recommendations[i] + // Refused before any provider is built, so no AWS call is made + // for an aws-cn Savings Plans rec (issue #1194). + if partitionErr := m.rejectUnsupportedPartition(&rec, awsPartition); partitionErr != nil { + logging.Warnf("purchase[%s]: %s/%s rec %d not purchased: %v", exec.ExecutionID, rec.Provider, rec.Service, i, partitionErr) + return recPurchaseOutcome{index: i, err: partitionErr}, nil + } logging.Infof("Purchasing: %dx %s in %s (%s/%s)", rec.Count, rec.ResourceType, rec.Region, rec.Provider, rec.Service) // Derive a deterministic per-rec idempotency token from the // execution's STABLE lineage key (not its mutable ExecutionID) diff --git a/internal/purchase/execution_test.go b/internal/purchase/execution_test.go index 81b6cc931..c8428fca7 100644 --- a/internal/purchase/execution_test.go +++ b/internal/purchase/execution_test.go @@ -2214,7 +2214,7 @@ func TestProcessPurchaseRecommendations_GlobalConfigError_FailsInsteadOfDefaulti dashboardURL: "https://dashboard.example.com", } - _, _, purchaseErrors, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil) + _, _, purchaseErrors, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil, "") // The function must return an error derived from the DB failure. require.Error(t, procErr, "GlobalConfig load failure must abort processing, not silently default to convertible") diff --git a/internal/purchase/money_path_regression_test.go b/internal/purchase/money_path_regression_test.go index 5233d8e0d..b12578a02 100644 --- a/internal/purchase/money_path_regression_test.go +++ b/internal/purchase/money_path_regression_test.go @@ -71,7 +71,7 @@ func captureIdempotencyTokens(t *testing.T, exec *config.PurchaseExecution) map[ // provCfg is nil: the mock factory matches mock.Anything for it and the // real factory is never reached. - _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil) + _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, plan, "111111111111", nil, "") require.NoError(t, procErr, "processPurchaseRecommendations must not fail in the happy path") require.Empty(t, errs, "all recs must commit so the captured token reflects a real purchase") return tokens @@ -124,7 +124,7 @@ func TestOfferingClassStampedOnHistoryMatchesPurchase(t *testing.T) { }, } - _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil) + _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil, "") require.NoError(t, procErr) require.Empty(t, errs) require.NotNil(t, history) @@ -151,7 +151,7 @@ func TestUnknownOfferingClassRefusesBeforePurchase(t *testing.T) { }, } - _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil) + _, _, errs, procErr := manager.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "Direct purchase"}, "111111111111", nil, "") require.ErrorIs(t, procErr, errUnknownOfferingClass) assert.Nil(t, errs) mockFactory.AssertNotCalled(t, "CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything) diff --git a/internal/purchase/partition_guard.go b/internal/purchase/partition_guard.go new file mode 100644 index 000000000..1df4f4559 --- /dev/null +++ b/internal/purchase/partition_guard.go @@ -0,0 +1,31 @@ +package purchase + +import ( + "fmt" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/savingsplans" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// rejectUnsupportedPartition refuses a Savings Plans recommendation for an +// AWS China (aws-cn) account before any provider is built or AWS is called +// (issue #1194). AWS bills aws-cn Savings Plans in CNY and the purchase path +// is USD-only. The library refuses too, but only when a region it sees is +// cn-*; an SP rec can carry no region, and the account's role ARN is the +// reliable signal here. The error wraps savingsplans.ErrUnsupportedPartition +// so callers can match it with errors.Is. +// +// awsPartition is the target account's partition (CloudAccount.AWSPartition), +// which is only set for AWS accounts; "" (no target account, or no role ARN) +// leaves the library guard as the only check. +func (m *Manager) rejectUnsupportedPartition(rec *config.RecommendationRecord, awsPartition string) error { + if awsPartition != config.AWSPartitionChina { + return nil + } + if !common.IsSavingsPlan(m.mapServiceType(rec.Service)) { + return nil + } + return fmt.Errorf("savings plans purchase refused for this AWS China (aws-cn) account: %w", + savingsplans.ErrUnsupportedPartition) +} diff --git a/internal/purchase/partition_guard_test.go b/internal/purchase/partition_guard_test.go new file mode 100644 index 000000000..1cdeb4444 --- /dev/null +++ b/internal/purchase/partition_guard_test.go @@ -0,0 +1,205 @@ +package purchase + +import ( + "context" + "errors" + "net/http" + "sync/atomic" + "testing" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/services/savingsplans" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +const awsCNRoleARN = "arn:aws-cn:iam::123456789012:role/CUDly" + +// TestRejectUnsupportedPartition pins which recs the aws-cn guard refuses +// (issue #1194): every Savings Plans slug for an aws-cn account, and nothing +// else. +func TestRejectUnsupportedPartition(t *testing.T) { + cases := []struct { + name string + partition string + service string + refused bool + }{ + {"aws-cn umbrella SP slug", config.AWSPartitionChina, "savingsplans", true}, + {"aws-cn legacy SP slug", config.AWSPartitionChina, "savings-plans", true}, + {"aws-cn compute SP", config.AWSPartitionChina, "savings-plans-compute", true}, + {"aws-cn EC2 instance SP", config.AWSPartitionChina, "savingsplans-ec2instance", true}, + {"aws-cn SageMaker SP", config.AWSPartitionChina, "savings-plans-sagemaker", true}, + {"aws-cn database SP", config.AWSPartitionChina, "savings-plans-database", true}, + {"aws-cn EC2 RI is left to the provider", config.AWSPartitionChina, "ec2", false}, + {"commercial SP still buys", config.AWSPartitionStandard, "savings-plans-compute", false}, + {"GovCloud SP still buys", config.AWSPartitionGovCloud, "savings-plans-compute", false}, + {"unknown partition leaves the library guard", "", "savings-plans-compute", false}, + } + m := &Manager{} + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rec := config.RecommendationRecord{Provider: "aws", Service: tc.service} + err := m.rejectUnsupportedPartition(&rec, tc.partition) + if !tc.refused { + assert.NoError(t, err) + return + } + require.ErrorIs(t, err, savingsplans.ErrUnsupportedPartition) + assert.Contains(t, err.Error(), "aws-cn") + }) + } +} + +// newAWSCNManager builds a Manager whose provider factory records any call +// and fails it, so a test can assert that no provider was built (and so no +// AWS API was reached) for a refused rec. +func newAWSCNManager(store *MockConfigStore, factory *MockProviderFactory, emailSender *MockEmailSender) *Manager { + factory.On("CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything). + Return(nil, errors.New("provider must not be built for an aws-cn Savings Plans rec")).Maybe() + return &Manager{ + config: store, + email: allowFailedNotification(emailSender), + providerFactory: factory, + assumeRoleSTS: new(MockAssumeRoleSTS), + dashboardURL: "https://dashboard.example.com", + } +} + +func awsCNSPRec(accountID *string) config.RecommendationRecord { + return config.RecommendationRecord{ + Provider: "aws", Service: "savings-plans-compute", ResourceType: "compute-sp", + Count: 1, Term: 1, Payment: "no-upfront", Savings: 40, Selected: true, CloudAccountID: accountID, + } +} + +// TestExecuteForAccount_AWSChinaSavingsPlanRefusedBeforeAnyAWSCall is the +// issue #1194 regression for the plan fan-out path: an SP rec for an account +// whose role ARN is in aws-cn fails with the unsupported-partition error and +// no provider is built for it. +func TestExecuteForAccount_AWSChinaSavingsPlanRefusedBeforeAnyAWSCall(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockFactory := new(MockProviderFactory) + var saved *config.PurchaseExecution + mockStore.SavePurchaseExecutionFn = func(_ context.Context, e *config.PurchaseExecution) error { + c := *e + saved = &c + return nil + } + m := newAWSCNManager(mockStore, mockFactory, new(MockEmailSender)) + + account := config.CloudAccount{ + ID: "acct-cn", Name: "China", Provider: "aws", ExternalID: "123456789012", + AWSAuthMode: "role_arn", AWSRoleARN: awsCNRoleARN, AWSExternalID: "ext-id-placeholder", + } + baseExec := &config.PurchaseExecution{ + ExecutionID: "exec-cn", + PlanID: "plan-cn", + Source: common.PurchaseSourceWeb, + Recommendations: []config.RecommendationRecord{awsCNSPRec(nil)}, + } + + committed, err := m.executeForAccount(ctx, baseExec, &config.PurchasePlan{ID: "plan-cn", Name: "CN"}, account) + require.Error(t, err) + assert.False(t, committed) + mockFactory.AssertNotCalled(t, "CreateAndValidateProvider", mock.Anything, mock.Anything, mock.Anything) + + require.NotNil(t, saved, "the per-account row must be saved") + assert.Equal(t, "failed", saved.Status) + assert.Contains(t, saved.Error, savingsplans.ErrUnsupportedPartition.Error()) + require.Len(t, saved.Recommendations, 1) + assert.False(t, saved.Recommendations[0].Purchased) + assert.Contains(t, saved.Recommendations[0].Error, savingsplans.ErrUnsupportedPartition.Error()) +} + +// TestExecutePurchase_SingleAccount_AWSChinaSavingsPlanRefused is the issue +// #1194 regression for the direct-execute path: the target account comes from +// the recs, and the SP rec is refused before any provider is built, while a +// non-SP rec in the same execution still reaches the provider. +func TestExecutePurchase_SingleAccount_AWSChinaSavingsPlanRefused(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockFactory := new(MockProviderFactory) + mockProviderInst := new(MockProvider) + mockServiceClient := new(MockServiceClient) + + acctID := "acct-cn-direct" + mockStore.On("GetCloudAccount", ctx, acctID).Return(&config.CloudAccount{ + ID: acctID, Provider: "aws", ExternalID: "123456789012", + AWSAuthMode: "role_arn", AWSRoleARN: awsCNRoleARN, AWSExternalID: "ext-id-placeholder", + }, nil) + mockStore.On("SavePurchaseHistory", ctx, mock.AnythingOfType("*config.PurchaseHistoryRecord")).Return(nil) + // The EC2 rec is the control: it must still reach the provider. Registered + // before newAWSCNManager's catch-all so it matches first. + mockFactory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.Anything).Return(mockProviderInst, nil).Once() + mockProviderInst.On("GetServiceClient", mock.Anything, common.ServiceEC2, "cn-north-1").Return(mockServiceClient, nil).Once() + mockServiceClient.On("PurchaseCommitment", mock.Anything, + mock.MatchedBy(func(r common.Recommendation) bool { return r.Service == common.ServiceEC2 }), + mock.AnythingOfType("common.PurchaseOptions"), + ).Return(common.PurchaseResult{Success: true, CommitmentID: "ri-cn"}, nil).Once() + mockEmail := new(MockEmailSender) + mockEmail.On("SendPurchaseConfirmation", ctx, mock.AnythingOfType("email.NotificationData")).Return(nil) + m := newAWSCNManager(mockStore, mockFactory, mockEmail) + + exec := &config.PurchaseExecution{ + ExecutionID: "exec-cn-direct", + Source: common.PurchaseSourceWeb, + Recommendations: []config.RecommendationRecord{ + awsCNSPRec(&acctID), + {Provider: "aws", Service: "ec2", ResourceType: "m5.large", Region: "cn-north-1", Count: 1, UpfrontCost: 100, Selected: true, CloudAccountID: &acctID}, + }, + } + + err := m.executePurchase(ctx, exec) + var partial *partialPurchaseError + require.ErrorAs(t, err, &partial, "the EC2 rec committed, so the run is a partial success") + assert.Contains(t, err.Error(), savingsplans.ErrUnsupportedPartition.Error()) + + mockFactory.AssertNumberOfCalls(t, "CreateAndValidateProvider", 1) + mockProviderInst.AssertNotCalled(t, "GetServiceClient", mock.Anything, common.ServiceSavingsPlansCompute, mock.Anything) + assert.False(t, exec.Recommendations[0].Purchased) + assert.Contains(t, exec.Recommendations[0].Error, savingsplans.ErrUnsupportedPartition.Error()) + assert.True(t, exec.Recommendations[1].Purchased) + mockServiceClient.AssertExpectations(t) +} + +// countingHTTPClient fails every request and counts it, so a test can prove a +// real SDK client made no AWS call without touching the network. +type countingHTTPClient struct{ calls atomic.Int32 } + +func (c *countingHTTPClient) Do(*http.Request) (*http.Response, error) { + c.calls.Add(1) + return nil, errors.New("network disabled in tests") +} + +// TestPinnedSavingsPlansClientRefusesAWSCN pins the providers/aws bump +// (issue #1194): the pinned library must refuse an aws-cn Savings Plans +// purchase with ErrUnsupportedPartition before sending any request. This is +// the backstop for accounts whose partition the platform cannot read from a +// role ARN (access_keys, or a self-account on ambient credentials). +func TestPinnedSavingsPlansClientRefusesAWSCN(t *testing.T) { + httpClient := &countingHTTPClient{} + client := savingsplans.NewClient(aws.Config{ + Region: "cn-north-1", + Credentials: aws.AnonymousCredentials{}, + HTTPClient: httpClient, + }, "Compute") + rec := common.Recommendation{ + Provider: common.ProviderAWS, + Service: common.ServiceSavingsPlansCompute, + ResourceType: "Compute", + Region: "cn-north-1", + Count: 1, + Term: "1yr", + PaymentOption: "no-upfront", + Details: &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 1}, + } + + _, err := client.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{}) + require.ErrorIs(t, err, savingsplans.ErrUnsupportedPartition) + assert.Zero(t, httpClient.calls.Load(), "no AWS request may be sent for an aws-cn Savings Plans purchase") +} diff --git a/internal/purchase/plan_step_proofs_test.go b/internal/purchase/plan_step_proofs_test.go index 0c6c80c8b..c1a6ca27f 100644 --- a/internal/purchase/plan_step_proofs_test.go +++ b/internal/purchase/plan_step_proofs_test.go @@ -104,7 +104,7 @@ func tokenCapture(t *testing.T, providerName string, exec *config.PurchaseExecut mu.Unlock() }).Return(common.PurchaseResult{Success: true, CommitmentID: "c"}, nil) m := &Manager{config: store, email: sender, providerFactory: factory} - _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.ProviderConfig)(nil)) + _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.ProviderConfig)(nil), "") require.NoError(t, err) require.Empty(t, errs) return tokens From ae9819511afcf3776061bd67776fbf9e88cf38da Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 16:11:03 +0000 Subject: [PATCH 4/5] test(api): put the host account ID on an allowlisted fixture line git-secrets' AWS account-ID detector flagged the inline sourceIdentity literal in the partition notice test, which failed the pre-commit hook and the allowlist script's scan of all tracked files. Split the literal one field per line so the placeholder sits on an `AccountID: "..."` line that .gitallowed already permits in test files. Same value, same assertions. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler_accounts_partition_notice_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/api/handler_accounts_partition_notice_test.go b/internal/api/handler_accounts_partition_notice_test.go index 5fd26a3fd..3d2b9aea7 100644 --- a/internal/api/handler_accounts_partition_notice_test.go +++ b/internal/api/handler_accounts_partition_notice_test.go @@ -126,7 +126,11 @@ func TestListAccounts_PartitionNotices(t *testing.T) { listResult: []config.CloudAccount{cn, commercial, self, azure}, }} handler.sourceIdentityOnce.Do(func() { - handler.sourceID = &sourceIdentity{Provider: "aws", AccountID: "111111111111", Partition: tc.hostPartition} + handler.sourceID = &sourceIdentity{ + Provider: "aws", + AccountID: "111111111111", + Partition: tc.hostPartition, + } }) result, err := handler.listAccounts(ctx, adminRequest("")) From f28e89c1fcc669a176fdeefabfbed27d2ef1a2ae Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sun, 25 Oct 2026 21:57:10 +0000 Subject: [PATCH 5/5] fix(purchase): pass the account partition in the Azure savings-plans alias proof Main's #1305 added a test that calls processPurchaseRecommendations with the old five-argument signature. This branch adds the awsPartition argument, so after the merge the purchase test package no longer compiled. The account in that test is Azure, so it passes "" like the branch's other non-AWS call sites. Co-Authored-By: Claude Opus 5.5 --- internal/purchase/azure_savings_plans_alias_proof_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/purchase/azure_savings_plans_alias_proof_test.go b/internal/purchase/azure_savings_plans_alias_proof_test.go index a052ee750..8453c1788 100644 --- a/internal/purchase/azure_savings_plans_alias_proof_test.go +++ b/internal/purchase/azure_savings_plans_alias_proof_test.go @@ -107,7 +107,7 @@ func purchaseThroughPinnedAzureClient(t *testing.T, exec *config.PurchaseExecuti prov.On("GetServiceClient", mock.Anything, common.ServiceSavingsPlansAll, mock.Anything).Return(provider.ServiceClient(client), nil) m := &Manager{config: store, email: sender, providerFactory: factory} - _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.ProviderConfig)(nil)) + _, _, errs, err := m.processPurchaseRecommendations(ctx, exec, &config.PurchasePlan{Name: "p"}, "acct", (*provider.ProviderConfig)(nil), "") require.NoError(t, err) require.Empty(t, errs) for i, rec := range exec.Recommendations {