diff --git a/cmd/cancellation_prepurchase_test.go b/cmd/cancellation_prepurchase_test.go
index 9aa5bdb37..9e5c8b7f1 100644
--- a/cmd/cancellation_prepurchase_test.go
+++ b/cmd/cancellation_prepurchase_test.go
@@ -71,15 +71,55 @@ func TestFetchExistingCoverage_CanceledContextIsTerminalEvenOnDryRun(t *testing.
assert.Nil(t, got)
}
-func TestHandleRegionDiscoveryError_CanceledContextDoesNotFallBackToDiscovery(t *testing.T) {
+func TestDetermineRegionsForService_CanceledContextIsTerminal(t *testing.T) {
captureAppLog(t)
- client := &MockRecommendationsClient{} // no expectations: any call fails the test
+ client := &MockRecommendationsClient{}
+ client.On("GetRecommendations", mock.Anything, mock.Anything).Return(nil, canceledCtx().Err())
- regions, err := handleRegionDiscoveryError(canceledCtx(), client, common.ServiceRDS, errors.New("describe regions: context canceled"))
+ regions, err := determineRegionsForService(canceledCtx(), client, common.ServiceRDS, Config{TermYears: 1})
require.ErrorIs(t, err, context.Canceled)
assert.Nil(t, regions)
- client.AssertNotCalled(t, "GetRecommendationsForService", mock.Anything, mock.Anything)
+}
+
+func TestDetermineRegionsForService_DiscoversRegionsTheCallerHasNotOptedInto(t *testing.T) {
+ captureAppLog(t)
+ cfg := Config{TermYears: 1, PaymentOption: "no-upfront", Coverage: 100}
+ rec := func(region string) common.Recommendation {
+ return common.Recommendation{Service: common.ServiceEC2, Region: region, ResourceType: "m5.large", Count: 1}
+ }
+ client := &MockRecommendationsClient{}
+ // aws.Config{} has no credentials, so any ec2:DescribeRegions call would
+ // fail: the regions must come from the recommendations.
+ client.On("GetRecommendations", mock.Anything, mock.MatchedBy(func(p *common.RecommendationParams) bool { return p.Region == "" })).
+ Return([]common.Recommendation{rec("us-east-1"), rec("eu-central-2"), rec("eu-central-2")}, nil).Once()
+
+ regions, err := determineRegionsForService(context.Background(), client, common.ServiceEC2, cfg)
+
+ require.NoError(t, err)
+ assert.Equal(t, []string{"eu-central-2", "us-east-1"}, regions)
+}
+
+func TestFetchAllRecs_DefaultRunKeepsRecommendationsInRegionsOutsideTheCallersOptIns(t *testing.T) {
+ captureAppLog(t)
+ cfg := Config{TermYears: 1, PaymentOption: "no-upfront", Coverage: 100}
+ rec := func(region string) common.Recommendation {
+ return common.Recommendation{Service: common.ServiceRDS, Region: region, ResourceType: "db.t3.small", Count: 3, EstimatedSavings: 10, SavingsPercentage: 10}
+ }
+ client := &MockRecommendationsClient{}
+ // Cost Explorer answers the same account-wide list for every call; the
+ // per-region pass keeps the rows of its own region.
+ client.On("GetRecommendations", mock.Anything, mock.Anything).
+ Return([]common.Recommendation{rec("us-east-1"), rec("eu-central-2")}, nil)
+
+ recs, _, err := fetchAllRecs(context.Background(), aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
+
+ require.NoError(t, err)
+ regions := make([]string, 0, len(recs))
+ for i := range recs {
+ regions = append(regions, recs[i].Region)
+ }
+ assert.ElementsMatch(t, []string{"us-east-1", "eu-central-2"}, regions)
}
func TestFetchRecommendationsForRegion_InterruptIsNotReportedAsProviderFailure(t *testing.T) {
diff --git a/cmd/multi_service.go b/cmd/multi_service.go
index 1636cf2aa..179a27579 100644
--- a/cmd/multi_service.go
+++ b/cmd/multi_service.go
@@ -915,7 +915,7 @@ func filterAndAdjustRecommendations(ctx context.Context, recs []common.Recommend
// processService processes a single service and returns recommendations and results.
// Used by legacy callers; new code should use fetchAllRecs + executePurchasePipeline.
func processService(ctx context.Context, awsCfg aws.Config, recClient provider.RecommendationsClient, accountCache *AccountAliasCache, service common.ServiceType, isDryRun bool, cfg Config, engineData engineVersionData) ([]common.Recommendation, []common.PurchaseResult) { //nolint:unparam // engineData always nil at current callsites but param is part of the API
- regionsToProcess, err := determineRegionsForService(ctx, awsCfg, recClient, service, cfg.Regions)
+ regionsToProcess, err := determineRegionsForService(ctx, recClient, service, cfg)
if err != nil {
log.Printf("❌ Failed to determine regions: %v", err)
return nil, nil
diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go
index 87cd19f40..627932f48 100644
--- a/cmd/multi_service_helpers.go
+++ b/cmd/multi_service_helpers.go
@@ -109,19 +109,14 @@ func getAllAWSRegionsWithClient(ctx context.Context, ec2Client EC2ClientInterfac
return regions, nil
}
-// discoverRegionsForService discovers regions that have recommendations for a specific service.
-func discoverRegionsForService(ctx context.Context, client provider.RecommendationsClient, service common.ServiceType) ([]string, error) {
- recs, err := client.GetRecommendationsForService(ctx, service)
- var incomplete *recommendations.IncompleteRecommendationsError
- if errors.As(err, &incomplete) {
- AppLogger.Printf(" ⚠️ Region discovery incomplete: %v\n", incomplete)
- } else if partial := azureprovider.AsPartialSubscriptionFailure(err); partial != nil {
- // Region discovery is best-effort: the subscriptions that answered
- // still tell us where to look. Report the gap rather than dropping
- // the discovered regions or failing outright.
- AppLogger.Printf(" ⚠️ Region discovery incomplete: %d of %d Azure subscriptions succeeded\n",
- partial.Succeeded, partial.Attempted)
- } else if err != nil {
+// discoverRegionsForService returns the regions Cost Explorer has
+// recommendations in for the run's term, payment option and lookback. The
+// regions come from the recommendations themselves, not from ec2:DescribeRegions:
+// Cost Explorer returns linked-account recommendations in regions the caller
+// has not opted into, and iterating only the caller's regions dropped them.
+func discoverRegionsForService(ctx context.Context, client provider.RecommendationsClient, service common.ServiceType, cfg Config) ([]string, error) {
+ recs, err := fetchRecommendationsForRegion(ctx, client, service, "", cfg)
+ if err != nil {
return nil, err
}
@@ -316,7 +311,7 @@ func executePurchase(ctx context.Context, rec common.Recommendation, region stri
}
// determineRegionsForService determines which regions to process for a given service.
-func determineRegionsForService(ctx context.Context, awsCfg aws.Config, recClient provider.RecommendationsClient, service common.ServiceType, configuredRegions []string) ([]string, error) {
+func determineRegionsForService(ctx context.Context, recClient provider.RecommendationsClient, service common.ServiceType, cfg Config) ([]string, error) {
// Savings Plans are account-level, not regional - only query once, even
// when --regions lists several. The recommendation call returns the same
// account-wide set for every region, so honoring --regions here repeats
@@ -330,35 +325,18 @@ func determineRegionsForService(ctx context.Context, awsCfg aws.Config, recClien
}
// If regions are explicitly configured, use those
- if len(configuredRegions) > 0 {
- return configuredRegions, nil
- }
-
- // Default to all AWS regions for other services
- AppLogger.Printf("🌍 Processing all AWS regions for %s...\n", getServiceDisplayName(service))
- allRegions, err := getAllAWSRegions(ctx, awsCfg)
- if err != nil {
- return handleRegionDiscoveryError(ctx, recClient, service, err)
- }
-
- AppLogger.Printf("📍 Processing %d region(s)\n", len(allRegions))
- return allRegions, nil
-}
-
-// handleRegionDiscoveryError handles errors during region discovery by falling back to auto-discovery.
-func handleRegionDiscoveryError(ctx context.Context, recClient provider.RecommendationsClient, service common.ServiceType, originalErr error) ([]string, error) {
- if ctxErr := ctx.Err(); ctxErr != nil {
- return nil, ctxErr
+ if len(cfg.Regions) > 0 {
+ return cfg.Regions, nil
}
- AppLogger.Printf("❌ Failed to get AWS regions: %v\n", originalErr)
- AppLogger.Printf("🔍 Falling back to auto-discovery...\n")
- discoveredRegions, err := discoverRegionsForService(ctx, recClient, service)
+ AppLogger.Printf("🌍 Discovering regions with %s recommendations...\n", getServiceDisplayName(service))
+ regions, err := discoverRegionsForService(ctx, recClient, service, cfg)
if err != nil {
- return nil, fmt.Errorf("failed to discover regions: %w", err)
+ return nil, err
}
- return discoveredRegions, nil
+ AppLogger.Printf("📍 Processing %d region(s)\n", len(regions))
+ return regions, nil
}
// engineVersionData holds the results of engine version queries.
@@ -940,7 +918,7 @@ func fetchAllRecs(
AppLogger.Printf("🔍 Fetching %s recommendations\n", getServiceDisplayName(service))
AppLogger.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n")
- regions, err := determineRegionsForService(ctx, awsCfg, recClient, service, cfg.Regions)
+ regions, err := determineRegionsForService(ctx, recClient, service, cfg)
if err != nil {
if ctx.Err() != nil {
return nil, drops, nil
diff --git a/cmd/multi_service_helpers_test.go b/cmd/multi_service_helpers_test.go
index a67a19798..dfee0d931 100644
--- a/cmd/multi_service_helpers_test.go
+++ b/cmd/multi_service_helpers_test.go
@@ -161,10 +161,11 @@ func TestDiscoverRegionsForService(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
mockClient := &MockRecommendationsClient{}
- mockClient.On("GetRecommendationsForService", ctx, tt.service).Return(tt.mockReturns, nil)
+ mockClient.On("GetRecommendations", ctx, mock.MatchedBy(func(p *common.RecommendationParams) bool {
+ return p.Service == tt.service && p.Region == ""
+ })).Return(tt.mockReturns, nil)
- // Now we can use the actual function directly since it accepts an interface
- regions, err := discoverRegionsForService(ctx, mockClient, tt.service)
+ regions, err := discoverRegionsForService(ctx, mockClient, tt.service, Config{TermYears: 1})
assert.NoError(t, err)
assert.Equal(t, tt.expectedRegions, regions)
diff --git a/cmd/recommendation_completeness_proxy_test.go b/cmd/recommendation_completeness_proxy_test.go
index 8c9359a8a..d5d3a4f11 100644
--- a/cmd/recommendation_completeness_proxy_test.go
+++ b/cmd/recommendation_completeness_proxy_test.go
@@ -185,9 +185,8 @@ func (p *completenessProxy) respond(req *http.Request) *http.Response {
}
p.mu.Lock()
p.requests[op]++
- call := p.requests[op]
p.mu.Unlock()
- status, contentType, payload := p.operation(req.Host, op, call)
+ status, contentType, payload := p.operation(req.Host, op)
if p.service != "rds" && req.Host == "ce.us-east-1.amazonaws.com" && op == "AWSInsightsIndexService.GetSavingsPlansPurchaseRecommendation" {
status, contentType, payload = p.spRecommendation(body)
}
@@ -196,7 +195,7 @@ func (p *completenessProxy) respond(req *http.Request) *http.Response {
Body: io.NopCloser(strings.NewReader(payload)), ContentLength: int64(len(payload))}
}
-func (p *completenessProxy) operation(host, op string, call int) (int, string, string) {
+func (p *completenessProxy) operation(host, op string) (int, string, string) {
responses := map[string]string{
"DescribeDBInstances": "DBInstances",
"DescribeDBMajorEngineVersions": "DBMajorEngineVersions",
@@ -238,9 +237,6 @@ func (p *completenessProxy) operation(host, op string, call int) (int, string, s
return 200, "application/x-amz-json-1.1", `{"Recommendations":[{"RecommendationDetails":[` + details[p.details] + `]}]}`
}
if host == "ec2.us-east-1.amazonaws.com" && op == "DescribeRegions" {
- if p.regions == "fallback" && call == 1 {
- return 400, "text/xml", `UnauthorizedOperationfixture denied region discovery`
- }
return 200, "text/xml", `- us-east-1
`
}
p.t.Errorf("unexpected operation %s %s", host, op)
@@ -264,20 +260,16 @@ func (p *completenessProxy) assertRequests(t *testing.T) {
p.assertSPRequests(t)
return
}
+ // Region discovery is one Cost Explorer call; each discovered region is
+ // then fetched once. Discovery finds a region only when a recommendation
+ // survived parsing.
ceCalls := 1
- if p.regions == "fallback" {
- ceCalls = 6
- if p.details == "valid" || p.details == "mixed" {
- ceCalls++
- }
+ if p.regions == "default" && (p.details == "valid" || p.details == "mixed") {
+ ceCalls++
}
require.Equal(t, ceCalls, p.requests["AWSInsightsIndexService.GetReservationPurchaseRecommendation"], "operations=%v", p.requests)
- // --include-extended-support skips the RDS exclusion queries, so only region discovery lists regions.
- wantRegions := 1
- if p.regions == "explicit" {
- wantRegions = 0
- }
- require.Equal(t, wantRegions, p.requests["DescribeRegions"], "region calls")
+ // Regions come from the recommendations; DescribeRegions is never called.
+ require.Zero(t, p.requests["DescribeRegions"], "region calls")
t.Logf("actual root command, config loader, SDK and CSV; synthetic operations=%v", p.requests)
}
diff --git a/cmd/recommendation_completeness_test.go b/cmd/recommendation_completeness_test.go
index 9368d2761..4f897eb01 100644
--- a/cmd/recommendation_completeness_test.go
+++ b/cmd/recommendation_completeness_test.go
@@ -28,7 +28,7 @@ func TestRecommendationCompletenessCommand(t *testing.T) {
runCompletenessCommandChild(t)
return
}
- for _, regionMode := range []string{"explicit", "default", "fallback"} {
+ for _, regionMode := range []string{"explicit", "default"} {
for _, details := range []string{"valid", "mixed", "all-invalid", "empty", "api-error"} {
t.Run(regionMode+"/"+details, func(t *testing.T) {
runCompletenessScenario(t, "rds", regionMode, details)
@@ -90,7 +90,7 @@ func runCompletenessScenario(t *testing.T, service, regionMode, details string)
case rdsExpiryService:
assertRDSExpiryCSV(t, output, details, logs)
case "rds":
- assertCompletenessDiagnostics(t, regionMode, details, stdout.String(), stderr.String())
+ assertCompletenessDiagnostics(t, details, stdout.String(), stderr.String())
assertCompletenessCSV(t, output, details, logs)
default:
assertSPCompleteness(t, output, service, details, logs)
@@ -183,21 +183,14 @@ func assertSPCompleteness(t *testing.T, output, service, details, logs string) {
require.Contains(t, logs, "CSV report written")
}
-func assertCompletenessDiagnostics(t *testing.T, regionMode, details, stdout, stderr string) {
+func assertCompletenessDiagnostics(t *testing.T, details, stdout, stderr string) {
t.Helper()
malformed := details == "mixed" || details == "all-invalid"
if malformed {
- count := 1
- if regionMode == "fallback" {
- count = 6
- }
- want := fmt.Sprintf("incomplete AWS recommendations: %d failed details, 0 failed scopes:", count)
+ want := "incomplete AWS recommendations: 1 failed details, 0 failed scopes:"
for _, diagnostic := range []string{want, "not-a-number", "detail"} {
require.Contains(t, stdout, diagnostic, "stderr: %s", stderr)
}
- if regionMode == "fallback" {
- require.Contains(t, stdout, "Region discovery incomplete")
- }
} else {
require.NotContains(t, stdout, "incomplete AWS recommendations")
}
@@ -253,9 +246,8 @@ func TestRecommendationCompletenessHelpers(t *testing.T) {
t.Cleanup(func() { AppLogger.SetOutput(previous) })
ctx := context.Background()
client := &MockRecommendationsClient{}
- client.On("GetRecommendationsForService", ctx, common.ServiceRDS).Return(tc.recs, tc.err)
client.On("GetRecommendations", ctx, mock.Anything).Return(tc.recs, tc.err)
- regions, err := discoverRegionsForService(ctx, client, common.ServiceRDS)
+ regions, err := discoverRegionsForService(ctx, client, common.ServiceRDS, Config{TermYears: 1})
got, fetchErr := fetchRecommendationsForRegion(ctx, client, common.ServiceRDS, "us-east-1", Config{TermYears: 1})
if !tc.keep {
require.ErrorIs(t, err, tc.err)
diff --git a/cmd/savings_plan_purchase_test.go b/cmd/savings_plan_purchase_test.go
index fcb28beea..5540a8c47 100644
--- a/cmd/savings_plan_purchase_test.go
+++ b/cmd/savings_plan_purchase_test.go
@@ -24,14 +24,14 @@ func TestDetermineRegionsForService_SavingsPlanCollectedOnceWithConfiguredRegion
common.ServiceSavingsPlansCompute, common.ServiceSavingsPlansEC2Instance,
common.ServiceSavingsPlansSageMaker, common.ServiceSavingsPlansDatabase,
} {
- regions, err := determineRegionsForService(context.Background(), aws.Config{}, nil, svc, []string{"eu-west-1", "us-west-2", "ap-south-1"})
+ regions, err := determineRegionsForService(context.Background(), nil, svc, Config{Regions: []string{"eu-west-1", "us-west-2", "ap-south-1"}})
require.NoError(t, err)
assert.Equal(t, []string{spAPIRegion}, regions, svc)
}
}
func TestDetermineRegionsForService_ConfiguredRegionsKeptForRegionalServices(t *testing.T) {
- regions, err := determineRegionsForService(context.Background(), aws.Config{}, nil, common.ServiceRDS, []string{"eu-west-1", "us-west-2"})
+ regions, err := determineRegionsForService(context.Background(), nil, common.ServiceRDS, Config{Regions: []string{"eu-west-1", "us-west-2"}})
require.NoError(t, err)
assert.Equal(t, []string{"eu-west-1", "us-west-2"}, regions)
}