fix(cmd): discover recommendation regions from Cost Explorer

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) {

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

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

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)

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", `<Response><Errors><Error><Code>UnauthorizedOperation</Code><Message>fixture denied region discovery</Message></Error></Errors></Response>`
-		}
 		return 200, "text/xml", `<DescribeRegionsResponse xmlns="http://ec2.amazonaws.com/doc/2016-11-15/"><regionInfo><item><regionName>us-east-1</regionName></item></regionInfo></DescribeRegionsResponse>`
 	}
 	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)
 }
 

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)

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)
 }