fix(cmd): fail when extended-support exclusion data is unavailable

CHANGELOG.md

@@ -7,6 +7,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/).
 
 ### Fixed
 
+- Stop silently disabling the extended-support exclusion when AWS queries
+  fail. A failed engine lifecycle query or region listing now aborts the run
+  before any purchase; an RDS recommendation in a region whose instance
+  inventory could not be read aborts it too, while other regions proceed with
+  a warning. The queries are skipped entirely under `--include-extended-support`
+  or when no RDS recommendations are in scope (#2147).
 - Ctrl-C now cancels the whole invocation instead of only setting a flag that
   was polled between purchases. The engine-version lifecycle queries, region
   discovery, recommendation fetch, coverage fetch and duplicate checks treat a

cmd/cancellation_prepurchase_test.go

@@ -32,7 +32,7 @@ func expiredCtx(t *testing.T) context.Context {
 func TestFetchEngineVersionData_CancellationAndDeadlineAreTerminal(t *testing.T) {
 	captureAppLog(t)
 	for name, ctx := range map[string]context.Context{"canceled": canceledCtx(), "deadline": expiredCtx(t)} {
-		_, err := fetchEngineVersionData(ctx, Config{})
+		_, err := fetchEngineVersionData(ctx, Config{}, true)
 		require.Error(t, err, name)
 		assert.ErrorIs(t, err, ctx.Err(), name)
 	}
@@ -105,7 +105,7 @@ func TestFetchAllRecs_InterruptStopsFanOutAndDiscardsPartialData(t *testing.T) {
 		Run(func(mock.Arguments) { cancel() }). // Ctrl-C after the first region answered
 		Return([]common.Recommendation{rec}, nil).Once()
 
-	recs, _ := fetchAllRecs(ctx, aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
+	recs, _, _ := fetchAllRecs(ctx, aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
 
 	assert.Empty(t, recs, "partial data from an interrupted fetch is not a result")
 	client.AssertNumberOfCalls(t, "GetRecommendations", 1) // later regions are never queried
@@ -121,7 +121,7 @@ func TestFetchAllRecs_ActiveContextProviderErrorStaysRecoverable(t *testing.T) {
 	client.On("GetRecommendations", mock.Anything, mock.MatchedBy(func(p *common.RecommendationParams) bool { return p.Region == "us-west-2" })).
 		Return([]common.Recommendation{rec}, nil).Once()
 
-	recs, _ := fetchAllRecs(context.Background(), aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
+	recs, _, _ := fetchAllRecs(context.Background(), aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
 
 	assert.Len(t, recs, 1, "a provider error with an active context only loses that region")
 	assert.Contains(t, out.String(), "Failed to fetch recommendations")
@@ -168,7 +168,7 @@ func TestFetchAllRecs_InterruptDuringLastRegionDiscardsItsData(t *testing.T) {
 		Run(func(mock.Arguments) { cancel() }).
 		Return([]common.Recommendation{rec}, nil).Once()
 
-	recs, _ := fetchAllRecs(ctx, aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
+	recs, _, _ := fetchAllRecs(ctx, aws.Config{}, client, NewAccountAliasCache(aws.Config{}), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil)
 
 	assert.Empty(t, recs)
 }

cmd/main_test.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -25,6 +26,11 @@ import (
 // AuditLog, which takes precedence within that test.
 func TestMain(m *testing.M) {
 	toolCfg.AuditLog = filepath.Join(os.TempDir(), fmt.Sprintf("cudly-test-audit-%d.jsonl", os.Getpid()))
+	// Keep tests off AWS: the real fetcher is exercised explicitly against a
+	// local stub (useRealEngineVersionFetcher).
+	engineVersionFetcher = func(context.Context, Config, bool) (engineVersionData, error) {
+		return engineVersionData{}, nil
+	}
 	code := m.Run()
 	_ = os.Remove(toolCfg.AuditLog)
 	os.Exit(code)

cmd/multi_service.go

@@ -174,7 +174,7 @@ func runToolMultiService(ctx context.Context, cfg Config) {
 	if adapter, ok := recClient.(*awsprovider.RecommendationsClientAdapter); ok && cfg.RecLookbackPeriod != "" {
 		adapter.SetRecLookbackPeriod(cfg.RecLookbackPeriod)
 	}
-	engineData := mustFetchEngineVersionData(ctx, cfg)
+	engineData := mustFetchEngineVersionData(ctx, cfg, extendedSupportCheckNeeded(cfg, servicesToProcess))
 
 	// Fetch existing-RI coverage so --target-coverage can subtract what
 	// the user already owns. On a dry run, a failure logs a warning and
@@ -190,7 +190,8 @@ func runToolMultiService(ctx context.Context, cfg Config) {
 
 	// Phase 1: collect all recommendations without purchasing.
 	AppLogger.Printf("\n📥 Fetching recommendations from all services...\n")
-	allRecs, drops := fetchAllRecs(ctx, awsCfg, recClient, accountCache, servicesToProcess, engineData, cfg, coverageMap)
+	allRecs, drops, err := fetchAllRecs(ctx, awsCfg, recClient, accountCache, servicesToProcess, engineData, cfg, coverageMap)
+	exitOnExclusionError(err)
 	exitIfInterrupted(ctx, "fetching recommendations")
 
 	// Phase 2: score, enforce the run-wide instance cap, and display.
@@ -206,13 +207,15 @@ func runToolMultiService(ctx context.Context, cfg Config) {
 }
 
 // mustFetchEngineVersionData is fetchEngineVersionData for the main pipeline:
-// an interrupt during the lifecycle queries ends the run, since sizing and
-// offering purchases on missing lifecycle data would be wrong.
-func mustFetchEngineVersionData(ctx context.Context, cfg Config) engineVersionData {
-	data, err := fetchEngineVersionData(ctx, cfg)
-	if err != nil {
+// an interrupt or unavailable exclusion data during the lifecycle queries ends
+// the run, since sizing and offering purchases on missing lifecycle data would
+// be wrong.
+func mustFetchEngineVersionData(ctx context.Context, cfg Config, needed bool) engineVersionData {
+	data, err := engineVersionFetcher(ctx, cfg, needed)
+	if ctx.Err() != nil {
 		log.Fatalf("Interrupted while querying engine versions (%v); nothing was purchased", err)
 	}
+	exitOnExclusionError(err)
 	return data
 }
 
@@ -832,38 +835,21 @@ func checkDuplicatesForCSVRegion(ctx context.Context, recs []common.Recommendati
 // It returns an error when the cap cannot be enforced honestly; see
 // requireRankingSignal.
 func filterAndAdjustRecommendations(ctx context.Context, recs []common.Recommendation, csvModeCoverage float64, cfg Config) ([]common.Recommendation, error) {
-	// Query running instances for engine version validation
-	log.Printf("🔍 Querying running RDS instances across all regions to validate engine versions...")
-	instanceVersions, err := queryRunningInstanceEngineVersions(ctx, cfg)
-	if ctxErr := ctx.Err(); ctxErr != nil {
-		return nil, ctxErr
+	if err := ctx.Err(); err != nil {
+		return nil, err
 	}
+	engineData, err := engineVersionFetcher(ctx, cfg, !cfg.IncludeExtendedSupport && hasDatabaseRecs(recs))
 	if err != nil {
-		log.Printf("⚠️  Warning: Failed to query running instances for engine version validation: %v", err)
-		log.Printf("   Continuing without engine version filtering")
-		instanceVersions = make(map[string][]InstanceEngineVersion)
-	} else {
-		log.Printf("✅ Found %d instance types with version information across all regions", len(instanceVersions))
+		return nil, err
 	}
-
-	// Query major engine versions for extended support detection
-	log.Printf("🔍 Querying AWS RDS major engine versions for extended support information...")
-	versionInfo, err := queryMajorEngineVersions(ctx, cfg)
-	if ctxErr := ctx.Err(); ctxErr != nil {
-		return nil, ctxErr
-	}
-	if err != nil {
-		log.Printf("⚠️  Warning: Failed to query major engine versions: %v", err)
-		log.Printf("   Continuing without extended support detection")
-		versionInfo = make(map[string]MajorEngineVersionInfo)
-	} else {
-		log.Printf("✅ Found support information for %d major engine versions", len(versionInfo))
+	if err := requireRegionInventory(recs, cfg, engineData); err != nil {
+		return nil, err
 	}
 
 	// Apply filters (empty currentRegion since we're processing from CSV, not iterating regions).
 	// Drop tracking is skipped on the CSV path (nil drops).
 	originalCount := len(recs)
-	recs = applyFilters(recs, &cfg, instanceVersions, versionInfo, "", nil)
+	recs = applyFilters(recs, &cfg, engineData.instanceVersions, engineData.versionInfo, "", nil)
 	if len(recs) < originalCount {
 		AppLogger.Printf("🔍 After filters: %d recs (filtered out %d)\n", len(recs), originalCount-len(recs))
 	}
@@ -980,3 +966,22 @@ func processPurchaseLoop(ctx context.Context, recs []common.Recommendation, regi
 
 	return results
 }
+
+// hasDatabaseRecs reports whether any recommendation is one the extended-support
+// exclusion applies to (the same *common.DatabaseDetails test the adjuster uses).
+func hasDatabaseRecs(recs []common.Recommendation) bool {
+	for i := range recs {
+		if d, ok := recs[i].Details.(*common.DatabaseDetails); ok && d != nil {
+			return true
+		}
+	}
+	return false
+}
+
+// exitOnExclusionError aborts the run before any purchase when the
+// extended-support exclusion cannot be applied (issue #2147).
+func exitOnExclusionError(err error) {
+	if err != nil {
+		log.Fatalf("Cannot apply extended-support exclusion: %v", err)
+	}
+}

cmd/multi_service_coverage_test.go

@@ -76,9 +76,10 @@ func TestQueryMajorEngineVersionsWithClient_Success(t *testing.T) {
 		"lifecycle data must be carried through from the API response")
 }
 
-// TestQueryMajorEngineVersionsWithClient_EngineErrorContinues asserts the
-// warn-and-continue contract: one engine failing must not drop the results
-// of the others, and the overall call still succeeds.
+// TestQueryMajorEngineVersionsWithClient_EngineErrorContinues asserts that one
+// engine failing does not stop the remaining engine queries, but the overall
+// call fails: missing lifecycle data must not silently disable the
+// extended-support exclusion (#2147).
 func TestQueryMajorEngineVersionsWithClient_EngineErrorContinues(t *testing.T) {
 	stub := &engineKeyedRDSMajorVersionsStub{
 		versionsByEngine: map[string][]rdstypes.DBMajorEngineVersion{
@@ -90,14 +91,13 @@ func TestQueryMajorEngineVersionsWithClient_EngineErrorContinues(t *testing.T) {
 	}
 
 	result, err := queryMajorEngineVersionsWithClient(context.Background(), stub)
-	require.NoError(t, err, "per-engine API failures are warn-and-continue")
+	require.Error(t, err, "an unavailable engine lifecycle query must fail the call")
+	assert.Nil(t, result)
 
 	assert.ElementsMatch(t,
 		[]string{"mysql", "postgres", "aurora-mysql", "aurora-postgresql"},
 		stub.enginesQueried,
 		"a failing engine must not stop the remaining engine queries")
-	require.Len(t, result, 1)
-	assert.Equal(t, "15", result["postgres:15"].MajorEngineVersion)
 }
 
 // TestQueryMajorEngineVersions_ProfileSelection asserts validation-profile

cmd/multi_service_engine_versions.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"log"
 	"runtime"
@@ -41,15 +42,20 @@ type MajorEngineVersionInfo struct {
 }
 
 // queryRunningInstanceEngineVersions queries all running RDS instances and returns their engine versions.
-func queryRunningInstanceEngineVersions(ctx context.Context, cfg Config) (map[string][]InstanceEngineVersion, error) {
+//
+// The returned failedRegions maps each region whose inventory could not be read
+// completely to the cause; the instance map still holds whatever was read. A
+// non-nil error means the inventory is unusable as a whole (config, region
+// listing or canceled context).
+func queryRunningInstanceEngineVersions(ctx context.Context, cfg Config) (instances map[string][]InstanceEngineVersion, failedRegions map[string]error, err error) {
 	awsCfg, err := loadValidationAWSConfig(ctx, cfg)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	regions, err := getAWSRegions(ctx, awsCfg)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	return queryRDSInstancesInRegions(ctx, awsCfg, regions)
@@ -100,8 +106,12 @@ type RDSMajorVersionsClient interface {
 }
 
 // queryRDSInstancesInRegions queries RDS instances in all regions concurrently.
-func queryRDSInstancesInRegions(ctx context.Context, awsCfg aws.Config, regions []ec2types.Region) (map[string][]InstanceEngineVersion, error) {
-	instanceVersions := make(map[string][]InstanceEngineVersion)
+// A region that fails (API error on any page, or a worker panic) is reported in
+// failedRegions rather than silently treated as an empty inventory. A canceled
+// ctx is returned as the error.
+func queryRDSInstancesInRegions(ctx context.Context, awsCfg aws.Config, regions []ec2types.Region) (instanceVersions map[string][]InstanceEngineVersion, failedRegions map[string]error, err error) {
+	instanceVersions = make(map[string][]InstanceEngineVersion)
+	failedRegions = make(map[string]error)
 	var mu sync.Mutex
 	var wg sync.WaitGroup
 
@@ -118,18 +128,29 @@ func queryRDSInstancesInRegions(ctx context.Context, awsCfg aws.Config, regions
 					buf := make([]byte, 4096)
 					n := runtime.Stack(buf, false)
 					log.Printf("ERROR: panic in region worker (region=%s): %v\n%s", regionName, r, buf[:n])
+					mu.Lock()
+					failedRegions[regionName] = fmt.Errorf("panic in region worker: %v", r)
+					mu.Unlock()
 				}
 			}()
-			queryRDSInstancesInRegion(ctx, awsCfg, regionName, instanceVersions, &mu)
+			if regionErr := queryRDSInstancesInRegion(ctx, awsCfg, regionName, instanceVersions, &mu); regionErr != nil {
+				mu.Lock()
+				failedRegions[regionName] = regionErr
+				mu.Unlock()
+			}
 		}(aws.ToString(region.RegionName))
 	}
 
 	wg.Wait()
-	return instanceVersions, nil
+	if ctxErr := ctx.Err(); ctxErr != nil {
+		return nil, nil, ctxErr
+	}
+	return instanceVersions, failedRegions, nil
 }
 
-// queryRDSInstancesInRegion queries RDS instances in a single region.
-func queryRDSInstancesInRegion(ctx context.Context, awsCfg aws.Config, regionName string, instanceVersions map[string][]InstanceEngineVersion, mu *sync.Mutex) {
+// queryRDSInstancesInRegion queries RDS instances in a single region and
+// returns the first page error, leaving whatever earlier pages merged in place.
+func queryRDSInstancesInRegion(ctx context.Context, awsCfg aws.Config, regionName string, instanceVersions map[string][]InstanceEngineVersion, mu *sync.Mutex) error {
 	regionCfg := awsCfg.Copy()
 	regionCfg.Region = regionName
 	rdsClient := awsrds.NewFromConfig(regionCfg)
@@ -138,8 +159,7 @@ func queryRDSInstancesInRegion(ctx context.Context, awsCfg aws.Config, regionNam
 	for {
 		localVersions, nextMarker, err := queryRDSInstancesPage(ctx, rdsClient, marker, regionName)
 		if err != nil {
-			log.Printf("⚠️  Warning: Failed to describe RDS instances in %s: %v", regionName, err)
-			break
+			return fmt.Errorf("failed to describe RDS instances in %s: %w", regionName, err)
 		}
 
 		// Merge into shared map with mutex protection
@@ -150,7 +170,7 @@ func queryRDSInstancesInRegion(ctx context.Context, awsCfg aws.Config, regionNam
 		mu.Unlock()
 
 		if nextMarker == nil {
-			break
+			return nil
 		}
 		marker = nextMarker
 	}
@@ -216,24 +236,31 @@ func queryMajorEngineVersionsWithClient(ctx context.Context, rdsClient RDSMajorV
 
 	// Query all engine types we care about
 	engines := []string{"mysql", "postgres", "aurora-mysql", "aurora-postgresql"}
+	var engineErrs []error
 
 	for _, engine := range engines {
 		if err := ctx.Err(); err != nil {
 			return nil, err
 		}
 		if err := fetchMajorEngineVersionsForEngine(ctx, rdsClient, engine, versionInfo); err != nil {
 			// Canceled caller context is terminal (issue #1325); check ctx.Err(),
-			// not the wrapped API error, so SDK-internal timeouts stay warnings.
+			// not the wrapped API error, so SDK-internal timeouts are reported as engine failures.
 			if ctxErr := ctx.Err(); ctxErr != nil {
 				return nil, ctxErr
 			}
-			log.Printf("Warning: Failed to describe major engine versions for %s: %v", engine, err)
+			// Lifecycle data for an engine is required to tell whether its
+			// instances are on extended support; keep trying the remaining
+			// engines so the error reports every unavailable one (issue #2147).
+			engineErrs = append(engineErrs, fmt.Errorf("major engine versions for %s: %w", engine, err))
 		}
 	}
 
 	if err := ctx.Err(); err != nil {
 		return nil, err
 	}
+	if len(engineErrs) > 0 {
+		return nil, errors.Join(engineErrs...)
+	}
 	return versionInfo, nil
 }
 

cmd/multi_service_engine_versions_test.go

@@ -818,7 +818,7 @@ func TestQueryMajorEngineVersions_ErrorHandling(t *testing.T) {
 			// We expect an error in test environment (no real AWS creds)
 			// The important thing is that the function doesn't panic
 			if err != nil {
-				assert.Contains(t, err.Error(), "failed to load AWS config")
+				assert.Regexp(t, "failed to load AWS config|major engine versions for", err.Error())
 			}
 		})
 	}
@@ -863,7 +863,7 @@ func TestQueryRunningInstanceEngineVersions_ErrorHandling(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			// This will likely fail in test environment without real AWS credentials
 			// but it validates the function signature and basic logic paths
-			_, err := queryRunningInstanceEngineVersions(ctx, tt.cfg)
+			_, _, err := queryRunningInstanceEngineVersions(ctx, tt.cfg)
 			// We expect an error in test environment (no real AWS creds)
 			// The important thing is that the function doesn't panic
 			if err != nil {