From e221eacebf39a8e538db7b269bdf502eef4ec7f4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 26 Oct 2026 11:13:25 +0100 Subject: [PATCH 1/3] chore(deps): bump cloud-commitments-go to 9cec521 Move pkg, providers/aws, providers/azure and providers/gcp to the same pseudo-version v0.0.0-20261026094505-9cec521d6531. Azure recommendations carry their currency (go#625). Co-Authored-By: Claude Opus 5.5 --- go.mod | 8 ++++---- go.sum | 16 ++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/go.mod b/go.mod index 0844b6cb..c266062f 100644 --- a/go.mod +++ b/go.mod @@ -84,10 +84,10 @@ 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-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/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261026094505-9cec521d6531 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261026094505-9cec521d6531 + github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261026094505-9cec521d6531 + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261026094505-9cec521d6531 github.com/aws/aws-lambda-go v1.47.0 github.com/aws/aws-sdk-go-v2/service/kms v1.50.4 github.com/aws/aws-sdk-go-v2/service/lambda v1.89.0 diff --git a/go.sum b/go.sum index 021297bb..bb96a5b8 100644 --- a/go.sum +++ b/go.sum @@ -92,14 +92,14 @@ 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-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= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261017134002-3def3f898617/go.mod h1:03vjHt5wlHtZEFAFOhosA+paDSqacWnKg9cVa68QnJ0= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261026094505-9cec521d6531 h1:x7bdeCDu+GdtVQ5Mcx8j0u09qiCssTPKtjru5nVAzQ0= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261026094505-9cec521d6531/go.mod h1:rEJErEsL40Np/ttf0RDoNNC2XQsN1GmWyFf34y0JLD8= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261026094505-9cec521d6531 h1:aZhRGSwaGxZ3gnPuR9iqHo069z0o0rJSoA7cgs9xqbg= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261026094505-9cec521d6531/go.mod h1:yVHEL5ceNS/CsDatzLH0+Qmwtoi7yY4tIHvqyvhdtig= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261026094505-9cec521d6531 h1:cWndlu++2ZWeSxvZthojAsTEHOdGU7BlIufJ60ssfAY= +github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261026094505-9cec521d6531/go.mod h1:zA51HKXtBvHHMYd+owRDmn4p8ialZrLFzI9I052uUmo= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261026094505-9cec521d6531 h1:Y7mztemGOd3kChFN236+zG4XLQ72E/MNiEdh+SU3Sv8= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261026094505-9cec521d6531/go.mod h1:/HxBqXBb5v69+EHLsibHnekkEYRP8TvyCmT6d9QRxt0= github.com/Microsoft/go-winio v0.6.2 h1:F2VQgta7ecxGYO8k3ZZz3RS8fVIXVxONVUPlNERoyfY= github.com/Microsoft/go-winio v0.6.2/go.mod h1:yd8OoFMLzJbo9gZq8j5qaps8bJ9aShtEA8Ipt1oGCvU= github.com/agext/levenshtein v1.2.1 h1:QmvMAjj2aEICytGiWzmxoE0x2KZvE0fvmqMOfy2tjT8= From bd33dd3ed0b574335c4105dd0bd3a91755ff5d86 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 26 Oct 2026 11:14:51 +0100 Subject: [PATCH 2/3] refactor: adapt to cloud-commitments-go 9cec521 API changes computeengine.NewClient no longer takes a context (go#575), and provider.ProviderConfig is deprecated in favour of provider.Config (an alias, so behaviour is unchanged). Update the one call site and rename the type references so golangci-lint stays clean. Co-Authored-By: Claude Opus 5.5 --- ...combined_plan_purchase_integration_test.go | 2 +- ..._purchases_pause_claim_integration_test.go | 2 +- .../handler_purchases_retry_fanout_test.go | 2 +- ...dler_purchases_run_now_integration_test.go | 2 +- ...dations_global_filters_integration_test.go | 2 +- ...ecommendation_identity_integration_test.go | 2 +- internal/providerfactory/providerfactory.go | 6 ++--- .../providerfactory/providerfactory_test.go | 8 +++---- .../azure_savings_plans_alias_proof_test.go | 2 +- internal/purchase/execution.go | 24 +++++++++---------- internal/purchase/execution_test.go | 6 ++--- internal/purchase/gcp_history_cost_test.go | 2 +- internal/purchase/mocks_test.go | 2 +- internal/purchase/plan_step_proofs_test.go | 2 +- ...mendation_completeness_integration_test.go | 4 ++-- internal/scheduler/scheduler.go | 4 ++-- internal/scheduler/scheduler_test.go | 10 ++++---- internal/scheduler/zonal_recs_test.go | 6 ++--- 18 files changed, 44 insertions(+), 44 deletions(-) diff --git a/internal/api/combined_plan_purchase_integration_test.go b/internal/api/combined_plan_purchase_integration_test.go index c35a15ce..f3078c80 100644 --- a/internal/api/combined_plan_purchase_integration_test.go +++ b/internal/api/combined_plan_purchase_integration_test.go @@ -86,7 +86,7 @@ func (p *combinedProvider) GetServiceClient(_ context.Context, service common.Se type combinedFactory struct{ provider *combinedProvider } -func (f combinedFactory) CreateAndValidateProvider(_ context.Context, name string, _ *provider.ProviderConfig) (provider.Provider, error) { +func (f combinedFactory) CreateAndValidateProvider(_ context.Context, name string, _ *provider.Config) (provider.Provider, error) { if name != "azure" { return nil, fmt.Errorf("fixture refuses unexpected provider %q", name) } diff --git a/internal/api/handler_purchases_pause_claim_integration_test.go b/internal/api/handler_purchases_pause_claim_integration_test.go index f327b5e4..4c8ba032 100644 --- a/internal/api/handler_purchases_pause_claim_integration_test.go +++ b/internal/api/handler_purchases_pause_claim_integration_test.go @@ -52,7 +52,7 @@ func (p *pauseClaimProvider) GetServiceClient(context.Context, common.ServiceTyp type pauseClaimFactory struct{ provider *pauseClaimProvider } -func (f pauseClaimFactory) CreateAndValidateProvider(context.Context, string, *provider.ProviderConfig) (provider.Provider, error) { +func (f pauseClaimFactory) CreateAndValidateProvider(context.Context, string, *provider.Config) (provider.Provider, error) { return f.provider, nil } diff --git a/internal/api/handler_purchases_retry_fanout_test.go b/internal/api/handler_purchases_retry_fanout_test.go index 4e6d2f6f..829a1317 100644 --- a/internal/api/handler_purchases_retry_fanout_test.go +++ b/internal/api/handler_purchases_retry_fanout_test.go @@ -162,7 +162,7 @@ func (p *fanoutProvider) GetRecommendationsClient(_ context.Context) (provider.R type fanoutProviderFactory struct{ prov *fanoutProvider } -func (f *fanoutProviderFactory) CreateAndValidateProvider(_ context.Context, _ string, _ *provider.ProviderConfig) (provider.Provider, error) { +func (f *fanoutProviderFactory) CreateAndValidateProvider(_ context.Context, _ string, _ *provider.Config) (provider.Provider, error) { return f.prov, nil } diff --git a/internal/api/handler_purchases_run_now_integration_test.go b/internal/api/handler_purchases_run_now_integration_test.go index 0660e806..572efed0 100644 --- a/internal/api/handler_purchases_run_now_integration_test.go +++ b/internal/api/handler_purchases_run_now_integration_test.go @@ -51,7 +51,7 @@ func (p *runNowProvider) GetServiceClient(context.Context, common.ServiceType, s type runNowFactory struct{ provider *runNowProvider } -func (f runNowFactory) CreateAndValidateProvider(context.Context, string, *provider.ProviderConfig) (provider.Provider, error) { +func (f runNowFactory) CreateAndValidateProvider(context.Context, string, *provider.Config) (provider.Provider, error) { return f.provider, nil } diff --git a/internal/api/handler_recommendations_global_filters_integration_test.go b/internal/api/handler_recommendations_global_filters_integration_test.go index 4ef76c2d..9eca9f71 100644 --- a/internal/api/handler_recommendations_global_filters_integration_test.go +++ b/internal/api/handler_recommendations_global_filters_integration_test.go @@ -29,7 +29,7 @@ type globalFilterProvider struct { provider.RecommendationsClient } -func (p globalFilterProvider) CreateAndValidateProvider(_ context.Context, name string, cfg *provider.ProviderConfig) (provider.Provider, error) { +func (p globalFilterProvider) CreateAndValidateProvider(_ context.Context, name string, cfg *provider.Config) (provider.Provider, error) { if name != "aws" || cfg != nil { return nil, fmt.Errorf("unexpected provider request: %s", name) } diff --git a/internal/api/recommendation_identity_integration_test.go b/internal/api/recommendation_identity_integration_test.go index 5f563627..84b1c64f 100644 --- a/internal/api/recommendation_identity_integration_test.go +++ b/internal/api/recommendation_identity_integration_test.go @@ -44,7 +44,7 @@ type identityProviderFactory struct { recs map[string][]common.Recommendation } -func (f identityProviderFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.ProviderConfig) (provider.Provider, error) { +func (f identityProviderFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.Config) (provider.Provider, error) { if name != "aws" || cfg == nil || cfg.AWSCredentialsProvider == nil { return nil, fmt.Errorf("expected registered AWS fixture credentials") } diff --git a/internal/providerfactory/providerfactory.go b/internal/providerfactory/providerfactory.go index bc351ef8..516d5040 100644 --- a/internal/providerfactory/providerfactory.go +++ b/internal/providerfactory/providerfactory.go @@ -59,7 +59,7 @@ func NewGCP(projectID string, ts oauth2.TokenSource) (provider.Provider, error) func newAzure(reg *provider.Registry, subscriptionID string, cred azcore.TokenCredential) (provider.Provider, error) { // A nil cred converts to a nil `any`, which the Azure factory treats as // "no credential supplied". - return build(reg, azureName, &provider.ProviderConfig{ + return build(reg, azureName, &provider.Config{ Name: azureName, AzureSubscriptionID: subscriptionID, AzureTokenCredential: cred, @@ -73,14 +73,14 @@ func newGCP(reg *provider.Registry, projectID string, ts oauth2.TokenSource) (pr if ts == nil { return nil, ErrMissingGCPTokenSource } - return build(reg, gcpName, &provider.ProviderConfig{ + return build(reg, gcpName, &provider.Config{ Name: gcpName, GCPProjectID: projectID, GCPTokenSource: ts, }) } -func build(reg *provider.Registry, name string, cfg *provider.ProviderConfig) (provider.Provider, error) { +func build(reg *provider.Registry, name string, cfg *provider.Config) (provider.Provider, error) { prov, err := reg.GetProviderWithConfig(name, cfg) if err != nil { return nil, fmt.Errorf("providerfactory: build %s provider: %w", name, err) diff --git a/internal/providerfactory/providerfactory_test.go b/internal/providerfactory/providerfactory_test.go index 063ee506..7d4e28a0 100644 --- a/internal/providerfactory/providerfactory_test.go +++ b/internal/providerfactory/providerfactory_test.go @@ -36,11 +36,11 @@ func (f *fakeTokenSource) Token() (*oauth2.Token, error) { // recordingRegistry returns a private registry whose name factory records // the config it receives and returns want. -func recordingRegistry(t *testing.T, name string, want provider.Provider, wantErr error) (*provider.Registry, *[]*provider.ProviderConfig) { +func recordingRegistry(t *testing.T, name string, want provider.Provider, wantErr error) (*provider.Registry, *[]*provider.Config) { t.Helper() reg := provider.NewRegistry() - var got []*provider.ProviderConfig - require.NoError(t, reg.Register(name, func(cfg *provider.ProviderConfig) (provider.Provider, error) { + var got []*provider.Config + require.NoError(t, reg.Register(name, func(cfg *provider.Config) (provider.Provider, error) { got = append(got, cfg) return want, wantErr })) @@ -147,7 +147,7 @@ func TestBuild_PropagatesFactoryErrorsAndNilProviders(t *testing.T) { func TestNewAzure_MatchesLegacyConstruction(t *testing.T) { cred := &fakeAzureCred{id: "legacy"} - legacy, err := azureprovider.NewAzureProvider(&provider.ProviderConfig{Profile: testSubscriptionID}) //nolint:staticcheck // SA1019: this is the legacy baseline being compared against + legacy, err := azureprovider.NewAzureProvider(&provider.Config{Profile: testSubscriptionID}) //nolint:staticcheck // SA1019: this is the legacy baseline being compared against require.NoError(t, err) legacy.SetCredential(cred) diff --git a/internal/purchase/azure_savings_plans_alias_proof_test.go b/internal/purchase/azure_savings_plans_alias_proof_test.go index 8453c178..68150e22 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.Config)(nil), "") require.NoError(t, err) require.Empty(t, errs) for i, rec := range exec.Recommendations { diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index 16679db4..09a0ed5b 100644 --- a/internal/purchase/execution.go +++ b/internal/purchase/execution.go @@ -159,7 +159,7 @@ 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, purchaseTarget, error) { +func (m *Manager) prepareSingleAccountSubmission(ctx context.Context, exec *config.PurchaseExecution) (*provider.Config, purchaseTarget, error) { if err := requireRecsMatchRowAccount(exec); err != nil { return nil, purchaseTarget{}, beforeSubmission(err) } @@ -445,7 +445,7 @@ 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, purchaseTarget, error) { +func (m *Manager) resolveSingleAccountProvider(ctx context.Context, exec *config.PurchaseExecution) (*provider.Config, 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 @@ -502,11 +502,11 @@ func (e *partialPurchaseError) Error() string { // resolveAccountProvider returns a *ProviderConfig with a pre-authenticated provider // for the given account. Returns an error if credential resolution fails -- callers // must NOT fall back to ambient credentials on error. -func (m *Manager) resolveAccountProvider(ctx context.Context, account config.CloudAccount) (*provider.ProviderConfig, error) { +func (m *Manager) resolveAccountProvider(ctx context.Context, account config.CloudAccount) (*provider.Config, error) { t0 := time.Now() logging.Infof("purchase[resolveAccountProvider]: resolving credentials for provider=%s account=%s", account.Provider, account.ID) - var cfg *provider.ProviderConfig + var cfg *provider.Config var err error switch account.Provider { case "aws": @@ -528,7 +528,7 @@ func (m *Manager) resolveAccountProvider(ctx context.Context, account config.Clo return cfg, nil } -func (m *Manager) resolveAWSProvider(ctx context.Context, account config.CloudAccount) (*provider.ProviderConfig, error) { +func (m *Manager) resolveAWSProvider(ctx context.Context, account config.CloudAccount) (*provider.Config, error) { t0 := time.Now() logging.Infof("purchase[resolveAWSProvider]: resolving AWS credentials for account=%s authMode=%s", account.ID, account.AWSAuthMode) @@ -544,10 +544,10 @@ func (m *Manager) resolveAWSProvider(ctx context.Context, account config.CloudAc } logging.Infof("purchase[resolveAWSProvider]: AWS credentials resolved for account=%s in %s", account.ID, time.Since(t0)) - return &provider.ProviderConfig{Name: "aws", AWSCredentialsProvider: awsCreds}, nil + return &provider.Config{Name: "aws", AWSCredentialsProvider: awsCreds}, nil } -func (m *Manager) resolveAzureProvider(ctx context.Context, account config.CloudAccount) (*provider.ProviderConfig, error) { +func (m *Manager) resolveAzureProvider(ctx context.Context, account config.CloudAccount) (*provider.Config, error) { t0 := time.Now() logging.Infof("purchase[resolveAzureProvider]: resolving Azure credentials for account=%s authMode=%s", account.ID, account.AzureAuthMode) @@ -571,10 +571,10 @@ func (m *Manager) resolveAzureProvider(ctx context.Context, account config.Cloud } logging.Infof("purchase[resolveAzureProvider]: Azure credentials resolved for account=%s in %s", account.ID, time.Since(t0)) - return &provider.ProviderConfig{ProviderOverride: azProv}, nil + return &provider.Config{ProviderOverride: azProv}, nil } -func (m *Manager) resolveGCPProvider(ctx context.Context, account config.CloudAccount) (*provider.ProviderConfig, error) { +func (m *Manager) resolveGCPProvider(ctx context.Context, account config.CloudAccount) (*provider.Config, error) { t0 := time.Now() logging.Infof("purchase[resolveGCPProvider]: resolving GCP credentials for account=%s authMode=%s", account.ID, account.GCPAuthMode) @@ -604,7 +604,7 @@ func (m *Manager) resolveGCPProvider(ctx context.Context, account config.CloudAc } logging.Infof("purchase[resolveGCPProvider]: GCP credentials resolved for account=%s in %s", account.ID, time.Since(t0)) - return &provider.ProviderConfig{ProviderOverride: gcpProv}, nil + return &provider.Config{ProviderOverride: gcpProv}, nil } // getMaxAccountParallelism is a thin alias over the shared @@ -624,7 +624,7 @@ type recPurchaseOutcome struct { index int } -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) { +func (m *Manager) processPurchaseRecommendations(ctx context.Context, exec *config.PurchaseExecution, plan *config.PurchasePlan, accountID string, provCfg *provider.Config, 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 @@ -1111,7 +1111,7 @@ func logRecCtxErr(executionID, recTuple string, elapsed time.Duration, recCtxErr // provCfg carries optional per-account credentials; pass nil to use ambient credentials. // opts carries execution-level metadata (the source surface) that providers stamp // onto the commitment they create. -func (m *Manager) executeSinglePurchase(ctx context.Context, rec config.RecommendationRecord, provCfg *provider.ProviderConfig, opts common.PurchaseOptions) (common.PurchaseResult, error) { +func (m *Manager) executeSinglePurchase(ctx context.Context, rec config.RecommendationRecord, provCfg *provider.Config, opts common.PurchaseOptions) (common.PurchaseResult, error) { // Per-purchase Info logs tagged with the owning execution ID so a // CloudWatch filter on the execution UUID surfaces every step of the // purchase attempt -- provider construction, service-client lookup, diff --git a/internal/purchase/execution_test.go b/internal/purchase/execution_test.go index 3a75bf06..1728fcac 100644 --- a/internal/purchase/execution_test.go +++ b/internal/purchase/execution_test.go @@ -1322,7 +1322,7 @@ func TestExecuteMultiAccount_RunsAccountsInParallel(t *testing.T) { // // After the fix, singleCloudAccountIDFromRecs derives the account from the recs, // GetCloudAccount fetches the CloudAccount record, and resolveAccountProvider -// constructs a provider.ProviderConfig with explicit creds. The factory receives +// constructs a provider.Config with explicit creds. The factory receives // a non-nil provCfg, which the assertion captures via MatchedBy. func TestExecutePurchase_SingleAccount_AzureUsesResolvedCreds(t *testing.T) { ctx := context.Background() @@ -1376,7 +1376,7 @@ func TestExecutePurchase_SingleAccount_AzureUsesResolvedCreds(t *testing.T) { // Before the fix, nil was passed and Azure SDK fell back to DefaultAzureCredential. // After the fix, resolveAzureProvider populates ProviderOverride on the config. mockFactory.On("CreateAndValidateProvider", mock.MatchedBy(hasPerRecDeadline(30*time.Second)), "azure", - mock.MatchedBy(func(cfg *provider.ProviderConfig) bool { + mock.MatchedBy(func(cfg *provider.Config) bool { return cfg != nil && cfg.ProviderOverride != nil }), ).Return(mockProviderInst, nil) @@ -1505,7 +1505,7 @@ func TestExecutePurchase_AzureCanonicalServiceTypes(t *testing.T) { }, nil) mockFactory.On("CreateAndValidateProvider", mock.MatchedBy(hasPerRecDeadline(30*time.Second)), "azure", - mock.MatchedBy(func(cfg *provider.ProviderConfig) bool { + mock.MatchedBy(func(cfg *provider.Config) bool { return cfg != nil && cfg.ProviderOverride != nil }), ).Return(mockProviderInst, nil) diff --git a/internal/purchase/gcp_history_cost_test.go b/internal/purchase/gcp_history_cost_test.go index 89bd76ec..85a80bd9 100644 --- a/internal/purchase/gcp_history_cost_test.go +++ b/internal/purchase/gcp_history_cost_test.go @@ -45,7 +45,7 @@ func (fakeCommitmentsService) Close() error { return nil } // cloud-commitments-go pin before a32fd1a returned the term total (1000 here). func TestSavePurchaseHistory_GCPRecordsNoUpfront(t *testing.T) { ctx := context.Background() - client, err := computeengine.NewClient(ctx, "project", "us-central1") + client, err := computeengine.NewClient("project", "us-central1") require.NoError(t, err) client.SetCommitmentsService(fakeCommitmentsService{}) diff --git a/internal/purchase/mocks_test.go b/internal/purchase/mocks_test.go index 729a0fc3..8d6b9263 100644 --- a/internal/purchase/mocks_test.go +++ b/internal/purchase/mocks_test.go @@ -22,7 +22,7 @@ type MockProviderFactory struct { mock.Mock } -func (m *MockProviderFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.ProviderConfig) (provider.Provider, error) { +func (m *MockProviderFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.Config) (provider.Provider, error) { args := m.Called(ctx, name, cfg) if args.Get(0) == nil { return nil, args.Error(1) diff --git a/internal/purchase/plan_step_proofs_test.go b/internal/purchase/plan_step_proofs_test.go index c1a6ca27..1c9c2033 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.Config)(nil), "") require.NoError(t, err) require.Empty(t, errs) return tokens diff --git a/internal/scheduler/recommendation_completeness_integration_test.go b/internal/scheduler/recommendation_completeness_integration_test.go index 93bc7278..34c9b07d 100644 --- a/internal/scheduler/recommendation_completeness_integration_test.go +++ b/internal/scheduler/recommendation_completeness_integration_test.go @@ -233,7 +233,7 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { store := config.NewPostgresStore(container.DB) factory := new(MockProviderFactory) fixture := &completenessHTTP{resourceType: "db.t3.medium", details: tc.details, fallback: tc.fallback, failScope: tc.failScope, spMode: tc.spMode, spFallback: tc.spFallback} - factory.On("CreateAndValidateProvider", mock.Anything, "aws", (*provider.ProviderConfig)(nil)). + factory.On("CreateAndValidateProvider", mock.Anything, "aws", (*provider.Config)(nil)). Return(completenessProvider(t, fixture), nil).Once() var accountID *string var completeAccountID string @@ -257,7 +257,7 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { if tc.spMode != "" { cleanFixture = &completenessHTTP{spMode: "valid"} } - factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.MatchedBy(func(cfg *provider.ProviderConfig) bool { return cfg != nil })). + factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.MatchedBy(func(cfg *provider.Config) bool { return cfg != nil })). Return(completenessProvider(t, cleanFixture), nil).Once() } seed := []config.RecommendationRecord{ diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 6e67335e..76331a70 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -786,7 +786,7 @@ func (s *Scheduler) resolveAmbientAccountID(ctx context.Context, providerName, e // subscriptionID is passed explicitly to avoid an unnecessary Azure API round-trip // to auto-discover subscriptions — the caller already resolved it from env. func (s *Scheduler) collectAzureAmbient(ctx context.Context, subscriptionID string) ([]config.RecommendationRecord, *sweepGap, error) { - prov, err := s.providerFactory.CreateAndValidateProvider(ctx, "azure", &provider.ProviderConfig{ + prov, err := s.providerFactory.CreateAndValidateProvider(ctx, "azure", &provider.Config{ AzureSubscriptionID: subscriptionID, }) if err != nil { @@ -851,7 +851,7 @@ func (s *Scheduler) collectAWSForAccount(ctx context.Context, globalCfg *config. if err != nil { return nil, nil, fmt.Errorf("resolve credentials: %w", err) } - cfg := &provider.ProviderConfig{Name: "aws", AWSCredentialsProvider: awsCreds} + cfg := &provider.Config{Name: "aws", AWSCredentialsProvider: awsCreds} prov, err := s.providerFactory.CreateAndValidateProvider(ctx, "aws", cfg) if err != nil { return nil, nil, fmt.Errorf("create provider: %w", err) diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index 937109f5..69f4d547 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -33,7 +33,7 @@ type MockProviderFactory struct { mock.Mock } -func (m *MockProviderFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.ProviderConfig) (provider.Provider, error) { +func (m *MockProviderFactory) CreateAndValidateProvider(ctx context.Context, name string, cfg *provider.Config) (provider.Provider, error) { args := m.Called(ctx, name, cfg) if args.Get(0) == nil { return nil, args.Error(1) @@ -2123,7 +2123,7 @@ func TestScheduler_CollectAWSForAccount_SelfAccountRoleARNModeUsesAmbient(t *tes EstimatedSavings: 100.0, }, } - mockFactory.On("CreateAndValidateProvider", ctx, "aws", (*provider.ProviderConfig)(nil)).Return(mockProvider, nil) + mockFactory.On("CreateAndValidateProvider", ctx, "aws", (*provider.Config)(nil)).Return(mockProvider, nil) mockProvider.On("GetRecommendationsClient", ctx).Return(mockRecClient, nil) mockRecClient.On("GetAllRecommendations", ctx).Return(recommendations, nil) @@ -2592,7 +2592,7 @@ func TestScheduler_CollectAzureRecommendations_AmbientTagging_HappyPath(t *testi EstimatedSavings: 20.0, }, } - mockFactory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.MatchedBy(func(cfg *provider.ProviderConfig) bool { + mockFactory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.MatchedBy(func(cfg *provider.Config) bool { return cfg != nil && cfg.AzureSubscriptionID == "sub-abc-123" })).Return(mockProvider, nil) mockProvider.On("GetRecommendationsClient", mock.Anything).Return(mockRecClient, nil) @@ -2639,7 +2639,7 @@ func TestScheduler_CollectAzureRecommendations_AmbientTagging_NoRegisteredAccoun recommendations := []common.Recommendation{ {Provider: common.ProviderAzure, Service: common.ServiceCompute, Region: "westus", ResourceType: "Standard_B2s", Count: 1, Term: "1yr"}, } - mockFactory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.MatchedBy(func(cfg *provider.ProviderConfig) bool { + mockFactory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.MatchedBy(func(cfg *provider.Config) bool { return cfg != nil && cfg.AzureSubscriptionID == "sub-unregistered" })).Return(mockProvider, nil) mockProvider.On("GetRecommendationsClient", mock.Anything).Return(mockRecClient, nil) @@ -2676,7 +2676,7 @@ func TestScheduler_CollectAzureRecommendations_AmbientTagging_StoreError(t *test recommendations := []common.Recommendation{ {Provider: common.ProviderAzure, Service: common.ServiceCompute, Region: "eastus", ResourceType: "Standard_D2s_v3", Count: 1, Term: "1yr"}, } - mockFactory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.MatchedBy(func(cfg *provider.ProviderConfig) bool { + mockFactory.On("CreateAndValidateProvider", mock.Anything, "azure", mock.MatchedBy(func(cfg *provider.Config) bool { return cfg != nil && cfg.AzureSubscriptionID == "sub-abc-123" })).Return(mockProvider, nil) mockProvider.On("GetRecommendationsClient", mock.Anything).Return(mockRecClient, nil) diff --git a/internal/scheduler/zonal_recs_test.go b/internal/scheduler/zonal_recs_test.go index 8fd3f4ee..e9087a51 100644 --- a/internal/scheduler/zonal_recs_test.go +++ b/internal/scheduler/zonal_recs_test.go @@ -142,7 +142,7 @@ func TestCollectAWSAmbient_PassesHostAccount(t *testing.T) { factory := new(MockProviderFactory) client := new(MockRecommendationsClient) client.On("GetAllRecommendations", mock.Anything).Return(mixedAccountRecs("111111111111"), nil) - factory.On("CreateAndValidateProvider", ctx, "aws", (*provider.ProviderConfig)(nil)).Return(newRecProvider(client), nil) + factory.On("CreateAndValidateProvider", ctx, "aws", (*provider.Config)(nil)).Return(newRecProvider(client), nil) s := &Scheduler{providerFactory: factory, stsClient: &fakeSTSClient{accountID: "111111111111"}} recs, gap, err := s.collectAWSAmbient(ctx, nil) require.NoError(t, err) @@ -155,7 +155,7 @@ func TestCollectAWSForAccount_SelfAccountPassesExternalID(t *testing.T) { factory := new(MockProviderFactory) client := new(MockRecommendationsClient) client.On("GetAllRecommendations", mock.Anything).Return(mixedAccountRecs("111111111111"), nil) - factory.On("CreateAndValidateProvider", ctx, "aws", (*provider.ProviderConfig)(nil)).Return(newRecProvider(client), nil) + factory.On("CreateAndValidateProvider", ctx, "aws", (*provider.Config)(nil)).Return(newRecProvider(client), nil) s := &Scheduler{config: new(MockConfigStore), providerFactory: factory, stsClient: &fakeSTSClient{accountID: "111111111111"}} recs, gap, err := s.collectAWSForAccount(ctx, nil, config.CloudAccount{ ID: "self", Provider: "aws", AWSAuthMode: "role_arn", ExternalID: "111111111111", Enabled: true, @@ -171,7 +171,7 @@ func TestCollectAWSForAccount_AssumedRolePassesExternalID(t *testing.T) { factory := new(MockProviderFactory) client := new(MockRecommendationsClient) client.On("GetAllRecommendations", mock.Anything).Return(mixedAccountRecs("333333333333"), nil) - factory.On("CreateAndValidateProvider", ctx, "aws", mock.AnythingOfType("*provider.ProviderConfig")).Return(newRecProvider(client), nil) + factory.On("CreateAndValidateProvider", ctx, "aws", mock.AnythingOfType("*provider.Config")).Return(newRecProvider(client), nil) s := &Scheduler{config: new(MockConfigStore), providerFactory: factory, stsClient: &fakeSTSClient{accountID: "111111111111"}} recs, gap, err := s.collectAWSForAccount(ctx, nil, config.CloudAccount{ ID: "member", Provider: "aws", AWSAuthMode: "role_arn", AWSRoleARN: "arn:aws:iam::333333333333:role/cudly", From b4851b41d60a9c032bb00b5591ec823d7b4085e5 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 26 Oct 2026 11:34:06 +0100 Subject: [PATCH 3/3] test(providerfactory): keep the bypass guard working after the provider.Config rename The guard only matched composite literals named ProviderConfig, so provider.Config{Profile: ...} was no longer caught after the rename. Match both names on the provider package, add a table test covering both and an unrelated foo.Config, and update the guard messages. Also correct two comments: since go#578 pkg/exchange refuses an unknown mode instead of treating it as auto. Co-Authored-By: Claude Opus 5.5 --- internal/api/handler.go | 14 ++++---- internal/providerfactory/bypass_guard_test.go | 34 ++++++++++++++++--- internal/server/ri_exchange_four_eyes.go | 14 ++++---- 3 files changed, 43 insertions(+), 19 deletions(-) diff --git a/internal/api/handler.go b/internal/api/handler.go index 63d2da75..f9639dfc 100644 --- a/internal/api/handler.go +++ b/internal/api/handler.go @@ -579,14 +579,14 @@ func riExchangeArmStateOf(cfg *config.GlobalConfig) riExchangeArmState { // armed reports whether this state will make the scheduled TaskRIExchangeReshape // execute exchanges against the provider without a human approval step. // -// The predicate mirrors the CONSUMER exactly. pkg/exchange/auto.go's -// processRecommendation routes to processManualExchange only on the literal -// "manual" and sends every other value -- including "", "Auto" and any typo -- -// to processAutoExchange. Defining "armed" as mode == "auto" would therefore -// leave the gate below open on precisely the route that matters: PUT /api/config +// The predicate is deliberately wider than the consumer. pkg/exchange's +// RunAutoExchange executes without approval only for the literal "auto" and +// refuses any other unknown mode (including "") with an error, so a typo +// fails the scheduled run loudly instead of executing. PUT /api/config // unmarshals onto GlobalConfig wholesale and GlobalConfig.Validate never -// constrains RIExchangeMode, so any non-"manual" string reaches the scheduler. -// Both axes of the guard must see what the scheduler sees. +// constrains RIExchangeMode, so any non-"manual" string can be stored; treating +// every such value as armed keeps the grant check closed if the consumer's +// mode handling ever changes. func (s riExchangeArmState) armed() bool { return s.enabled && s.mode != "manual" } diff --git a/internal/providerfactory/bypass_guard_test.go b/internal/providerfactory/bypass_guard_test.go index 78e6f9fc..0c4c30c4 100644 --- a/internal/providerfactory/bypass_guard_test.go +++ b/internal/providerfactory/bypass_guard_test.go @@ -29,7 +29,7 @@ var bypassConstructors = map[string]map[string]bool{ } // No non-test file under cmd/ or internal/ may call the concrete Azure/GCP -// provider constructors or set the deprecated ProviderConfig.Profile field. +// provider constructors or set the deprecated provider.Config.Profile field. func TestNoRegistryBypassInProductionCode(t *testing.T) { root := filepath.Join("..", "..") var violations []string @@ -50,15 +50,21 @@ func TestNoRegistryBypassInProductionCode(t *testing.T) { } require.Greater(t, scanned, 100, "the walk must actually reach the production sources") assert.Empty(t, violations, - "build credentialed Azure/GCP providers with providerfactory.NewAzure/NewGCP and use the typed ProviderConfig fields") + "build credentialed Azure/GCP providers with providerfactory.NewAzure/NewGCP and use the typed provider.Config fields") } // registryBypasses returns one entry per bypassing call or deprecated // Profile key in the file at path. func registryBypasses(t *testing.T, path string) []string { + t.Helper() + return bypassesInSource(t, path, nil) +} + +// bypassesInSource is registryBypasses over src (or the file at path when src is nil). +func bypassesInSource(t *testing.T, path string, src any) []string { t.Helper() fset := token.NewFileSet() - f, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) + f, err := parser.ParseFile(fset, path, src, parser.SkipObjectResolution) require.NoError(t, err, path) imports := map[string]string{} // local name -> import path @@ -80,7 +86,7 @@ func registryBypasses(t *testing.T, path string) []string { out = append(out, fset.Position(node.Pos()).String()+": "+node.Sel.Name) } case *ast.CompositeLit: - if sel, ok := node.Type.(*ast.SelectorExpr); ok && sel.Sel.Name == "ProviderConfig" && selectorPkg(sel, imports) == providerPkg { + if sel, ok := node.Type.(*ast.SelectorExpr); ok && (sel.Sel.Name == "Config" || sel.Sel.Name == "ProviderConfig") && selectorPkg(sel, imports) == providerPkg { out = append(out, profileKeys(fset, node)...) } } @@ -105,8 +111,26 @@ func profileKeys(fset *token.FileSet, lit *ast.CompositeLit) []string { continue } if id, ok := kv.Key.(*ast.Ident); ok && id.Name == "Profile" { - out = append(out, fset.Position(kv.Pos()).String()+": ProviderConfig.Profile") + out = append(out, fset.Position(kv.Pos()).String()+": provider.Config.Profile") } } return out } + +func TestBypassGuardMatchesConfigNames(t *testing.T) { + const providerImport = `import "github.com/LeanerCloud/cloud-commitments-go/pkg/provider"` + cases := map[string]struct { + src string + want int + }{ + "provider.Config": {providerImport + "\nvar _ = provider.Config{Profile: \"x\"}\n", 1}, + "provider.ProviderConfig": {providerImport + "\nvar _ = provider.ProviderConfig{Profile: \"x\"}\n", 1}, + "unrelated foo.Config": {"import foo \"example.com/foo\"\nvar _ = foo.Config{Profile: \"x\"}\n", 0}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + got := bypassesInSource(t, "x.go", "package x\n"+tc.src) + assert.Len(t, got, tc.want) + }) + } +} diff --git a/internal/server/ri_exchange_four_eyes.go b/internal/server/ri_exchange_four_eyes.go index fac7a741..9fea9dd4 100644 --- a/internal/server/ri_exchange_four_eyes.go +++ b/internal/server/ri_exchange_four_eyes.go @@ -11,13 +11,13 @@ import ( // scheduled reshape task or a ladder reshape) may use under the current // 4-eyes policy (issue #679). // -// In any mode but "manual", pkg/exchange executes exchanges on the spot with -// no human approval. That bypasses RequireDifferentApprover, which the direct -// execute endpoints already refuse (#517). So with 4-eyes on, every mode but -// "manual" is downgraded to "manual": the run only raises pending records, -// and those go through the approve endpoint and its 4-eyes check. "auto", "" -// and typos all get the downgrade, because pkg/exchange treats every -// non-"manual" value as auto. +// In "auto" mode, pkg/exchange executes exchanges on the spot with no human +// approval. That bypasses RequireDifferentApprover, which the direct execute +// endpoints already refuse (#517). So with 4-eyes on, every mode but "manual" +// is downgraded to "manual": the run only raises pending records, and those go +// through the approve endpoint and its 4-eyes check. "" and typos get the +// downgrade too; pkg/exchange would otherwise refuse them as unknown modes, so +// the downgrade keeps the scheduled run working rather than failing it. // // A nil global config fails closed: it is treated as 4-eyes on, never as off. func fourEyesExchangeMode(cfg *config.GlobalConfig, mode string) string {