refactor(cmd): split multi_service_helpers.go by pure moves

.gitallowed

@@ -75,7 +75,7 @@
 # bracketed letter in PRIVAT[E] for the same self-scan reason as above.
 ^(SECURITY\.md|\.github/workflows/README\.md):[0-9]+:\|[-| ]+\|$
 ^cmd/[a-z_]+\.go:[0-9]+:[[:space:]]*(fmt|AppLogger)\.Println\("-+"\)$
-^cmd/multi_service(_helpers|_csv_run)?\.go:[0-9]+:[[:space:]]*AppLogger\.Printf\("(\\n)?(━)+\\n"\)$
+^cmd/multi_service(_helpers|_csv_run|_fetch_all)?\.go:[0-9]+:[[:space:]]*AppLogger\.Printf\("(\\n)?(━)+\\n"\)$
 ^cmd/configure_gcp_test\.go:[0-9]+:// --- addMemberToPolicyBinding -+$
 ^cmd/configure_test\.go:[0-9]+:[[:space:]]*assert\.Equal\(t, "-----BEGIN PRIVAT[E] KEY-----\\n\.\.\.", creds\.PrivateKey\)$
 ^cmd/configure_gcp_store_test\.go:[0-9]+:[[:space:]]*"private_key": "-----BEGIN PRIVAT[E] KEY-----\\nMIIEvQIBADANBg\.\.\.\\n-----END PRIVATE KEY-----\\n",$

cmd/multi_service_engine_data.go

@@ -0,0 +1,130 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"maps"
+	"slices"
+	"sort"
+	"strings"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+// engineVersionData holds the results of engine version queries.
+// failedRegions lists regions whose RDS inventory could not be read
+// completely, mapped to the cause; instanceVersions then holds only what was
+// read, so recommendations in those regions cannot be checked (issue #2147).
+type engineVersionData struct {
+	instanceVersions map[string][]InstanceEngineVersion
+	versionInfo      map[string]MajorEngineVersionInfo
+	failedRegions    map[string]error
+}
+
+// engineVersionFetcher is the seam tests replace so they never reach AWS.
+var engineVersionFetcher = fetchEngineVersionData
+
+// extendedSupportCheckNeeded reports whether extended-support exclusion will
+// run for the given services: it is on by default and applies to RDS only.
+func extendedSupportCheckNeeded(cfg Config, services []common.ServiceType) bool {
+	if cfg.IncludeExtendedSupport {
+		return false
+	}
+	return slices.Contains(services, common.ServiceRDS)
+}
+
+// fetchEngineVersionData queries running instances and major engine versions
+// for extended-support exclusion. When needed is false nothing is queried.
+// It fails when exclusion data is unavailable as a whole (region listing or
+// any engine lifecycle query); per-region instance failures are returned in
+// failedRegions and enforced per recommendation by requireRegionInventory.
+func fetchEngineVersionData(ctx context.Context, cfg Config, needed bool) (engineVersionData, error) {
+	data := engineVersionData{
+		instanceVersions: make(map[string][]InstanceEngineVersion),
+		versionInfo:      make(map[string]MajorEngineVersionInfo),
+		failedRegions:    make(map[string]error),
+	}
+	if !needed {
+		return data, nil
+	}
+
+	var err error
+	data.instanceVersions, data.failedRegions, err = queryInstanceVersions(ctx, cfg)
+	if err != nil {
+		return engineVersionData{}, fmt.Errorf("extended-support exclusion data unavailable: %w (re-run, or pass --include-extended-support to skip the check)", err)
+	}
+	data.versionInfo, err = queryMajorVersions(ctx, cfg)
+	if err != nil {
+		return engineVersionData{}, fmt.Errorf("extended-support exclusion data unavailable: %w (re-run, or pass --include-extended-support to skip the check)", err)
+	}
+	return data, nil
+}
+
+// queryInstanceVersions queries running instances for engine version validation.
+func queryInstanceVersions(ctx context.Context, cfg Config) (instances map[string][]InstanceEngineVersion, failedRegions map[string]error, err error) {
+	AppLogger.Printf("🔍 Querying running RDS instances across all regions to validate engine versions...\n")
+	instanceVersions, failedRegions, err := queryRunningInstanceEngineVersions(ctx, cfg)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	AppLogger.Printf("✅ Found %d instance types with version information across all regions\n", len(instanceVersions))
+	if len(failedRegions) > 0 {
+		AppLogger.Printf("⚠️  Warning: RDS inventory unavailable in %s; RDS recommendations in those regions will abort the run, other regions proceed\n", describeFailedRegions(failedRegions))
+	}
+	return instanceVersions, failedRegions, nil
+}
+
+// queryMajorVersions queries major engine versions for extended support detection.
+func queryMajorVersions(ctx context.Context, cfg Config) (map[string]MajorEngineVersionInfo, error) {
+	AppLogger.Printf("🔍 Querying AWS RDS major engine versions for extended support information...\n")
+	versionInfo, err := queryMajorEngineVersions(ctx, cfg)
+	if ctxErr := ctx.Err(); ctxErr != nil {
+		return nil, ctxErr
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	AppLogger.Printf("✅ Found support information for %d major engine versions\n", len(versionInfo))
+	return versionInfo, nil
+}
+
+// describeFailedRegions renders failed regions and causes in a stable order.
+func describeFailedRegions(failed map[string]error) string {
+	names := make([]string, 0, len(failed))
+	for region := range failed {
+		names = append(names, region)
+	}
+	sort.Strings(names)
+	parts := make([]string, 0, len(names))
+	for _, region := range names {
+		parts = append(parts, fmt.Sprintf("%s (%v)", region, failed[region]))
+	}
+	return strings.Join(parts, "; ")
+}
+
+// requireRegionInventory fails when an RDS recommendation sits in a region
+// (or has no region) whose instance inventory could not be read, because the
+// extended-support exclusion cannot be applied to it. Recommendations in
+// healthy regions are unaffected.
+func requireRegionInventory(recs []common.Recommendation, cfg Config, data engineVersionData) error {
+	if cfg.IncludeExtendedSupport || len(data.failedRegions) == 0 {
+		return nil
+	}
+	affected := make(map[string]error)
+	for i := range recs {
+		if d, ok := recs[i].Details.(*common.DatabaseDetails); !ok || d == nil {
+			continue
+		}
+		if recs[i].Region == "" {
+			maps.Copy(affected, data.failedRegions)
+		} else if cause, failed := data.failedRegions[recs[i].Region]; failed {
+			affected[recs[i].Region] = cause
+		}
+	}
+	if len(affected) == 0 {
+		return nil
+	}
+	return fmt.Errorf("extended-support exclusion cannot be applied: RDS inventory unavailable in %s (re-run, or pass --include-extended-support to skip the check)", describeFailedRegions(affected))
+}

cmd/multi_service_fetch_all.go

@@ -0,0 +1,197 @@
+package main
+
+import (
+	"context"
+	"fmt"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/provider"
+	"github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations"
+	"github.com/aws/aws-sdk-go-v2/aws"
+)
+
+// checkDuplicates adjusts recommendations against already-owned RIs so the run
+// does not double-purchase existing capacity.
+//
+// It deliberately does NOT apply --max-instances. This function runs once per
+// (service, region), and capping here caps each region independently, which
+// multiplies the operator's cap by the number of service/region pairs. The cap
+// is applied once run-wide instead, after every region has been fetched
+// (applyGlobalInstanceLimit in multi_service.go).
+//
+// The duplicate check is the only guard between a re-run and a double
+// purchase, so a failed check (throttling, a describe-call AccessDenied, a
+// transient 5xx) must not fall back to the un-deduplicated counts on a
+// purchase run: that would buy reserved capacity the account already owns.
+// isDryRun selects the behavior on error: a dry run logs a loud warning and
+// continues (nothing is bought, so reporting fidelity wins), while a purchase
+// run refuses to spend and drops these recommendations entirely: an
+// over-purchase of reserved capacity is not reversible.
+//
+// drops accumulates per-reason drop counts for the end-of-run summary; pass nil to skip.
+//
+// A recommendation dropped because the check itself failed is counted under
+// dropDuplicateCheckFailed, not common.DropDuplicateDedup: the latter means
+// "an actual duplicate was found and subtracted", and reusing it for a failed
+// check would report the failure as a successful dedup in the summary.
+func checkDuplicates(
+	ctx context.Context,
+	filteredRecs []common.Recommendation,
+	serviceClient provider.ServiceClient,
+	isDryRun bool,
+	drops *common.DropSummary,
+) []common.Recommendation {
+	// Check for duplicate RIs to avoid double purchasing
+	duplicateChecker := NewDuplicateChecker(toolCfg.IdempotencyWindowHours)
+	adjustedRecs, dedupedOut, err := duplicateChecker.AdjustRecommendationsForExistingRIs(ctx, filteredRecs, serviceClient)
+	if err != nil {
+		if ctx.Err() != nil {
+			// Interrupted: terminal in dry and real runs alike, never
+			// "continue with un-deduplicated counts". The caller stops on ctx.Err().
+			return nil
+		}
+		if !isDryRun {
+			AppLogger.Printf("  ❌ Refusing to purchase %d instance(s): could not check for existing RIs (%v). Dropping these recommendations rather than risking a duplicate purchase.\n", CalculateTotalInstances(filteredRecs), err)
+			drops.Add(dropDuplicateCheckFailed, len(filteredRecs))
+			return nil
+		}
+		AppLogger.Printf("  ⚠️  Warning: Could not check for existing RIs: %v (dry run; continuing with un-deduplicated counts)\n", err)
+		// Continue with original filteredRecs on error; adjustedRecs is not used in this branch.
+	} else {
+		// Always use the adjusted recommendations (they might have different counts even if same length)
+		originalInstances := CalculateTotalInstances(filteredRecs)
+		adjustedInstances := CalculateTotalInstances(adjustedRecs)
+		if originalInstances != adjustedInstances {
+			AppLogger.Printf("  🔍 Adjusted recommendations: %d instances → %d instances to avoid duplicate purchases\n", originalInstances, adjustedInstances)
+		}
+		drops.Add(common.DropDuplicateDedup, len(dedupedOut))
+		filteredRecs = mapRecs(adjustedRecs, refreshCountLinear)
+	}
+
+	return filteredRecs
+}
+
+// fetchAndFilterRegionRecs fetches, filters, applies coverage, and deduplicates
+// recommendations for a single service+region. No purchases are made.
+// coverageMap (when non-nil) feeds existing-pool coverage into the sizing
+// step for --target-coverage. drops accumulates per-reason drop counts for
+// the end-of-run summary; pass nil to skip tracking.
+func fetchAndFilterRegionRecs(
+	ctx context.Context,
+	awsCfg aws.Config,
+	recClient provider.RecommendationsClient,
+	accountCache *AccountAliasCache,
+	service common.ServiceType,
+	region string,
+	regionIndex, totalRegions int,
+	engineData engineVersionData,
+	cfg Config,
+	coverageMap recommendations.PoolCoverageMap,
+	drops *common.DropSummary,
+) ([]common.Recommendation, error) {
+	AppLogger.Printf("\n  📍 [%d/%d] Region: %s\n", regionIndex, totalRegions, region)
+
+	recs, err := fetchRecommendationsForRegion(ctx, recClient, service, region, cfg)
+	if err != nil {
+		return nil, err
+	}
+	if len(recs) == 0 {
+		AppLogger.Printf("  ℹ️  No recommendations found\n")
+		return nil, nil
+	}
+	AppLogger.Printf("  ✅ Found %d recommendations\n", len(recs))
+
+	if err := populateRecommendationAccountNames(ctx, recs, accountCache, cfg); err != nil {
+		return nil, fmt.Errorf("%w: %w", errRecommendationFetch, err)
+	}
+	if err := requireRegionInventory(recs, cfg, engineData); err != nil {
+		return nil, err
+	}
+	recs = applyRegionFilters(recs, engineData, region, cfg, drops)
+	if len(recs) == 0 {
+		AppLogger.Printf("  ℹ️  No recommendations after applying filters\n")
+		return nil, nil
+	}
+
+	// Build the regional service client once and reuse for both expiry-aware
+	// coverage adjustment and the downstream duplicate check.
+	regionalCfg := awsCfg.Copy()
+	regionalCfg.Region = region
+	serviceClient := createServiceClient(service, regionalCfg)
+
+	// Fetch existing commitments up front when --rebuy-window-days is set so
+	// the sizing step can deduct expiring RIs from ExistingCoveragePct.
+	// Best-effort: a failure here logs and continues; expiry adjustment
+	// becomes a no-op for this region.
+	var expiringCommitments []common.Commitment
+	if cfg.RebuyWindowDays > 0 && serviceClient != nil {
+		commits, err := serviceClient.GetExistingCommitments(ctx)
+		if err != nil {
+			AppLogger.Printf("  ⚠️  Could not fetch existing commitments for expiry check: %v\n", err)
+		} else {
+			expiringCommitments = commits
+		}
+	}
+
+	recs = applyCoverageAndOverrides(recs, cfg, coverageMap, expiringCommitments, drops)
+
+	// Deduplication: skip recs matching recently-purchased commitments.
+	// --max-instances is NOT applied here; it is enforced once run-wide by the
+	// caller so the cap covers every service and region together.
+	if serviceClient != nil {
+		recs = checkDuplicates(ctx, recs, serviceClient, effectiveDryRun(cfg), drops)
+	}
+
+	return recs, nil
+}
+
+// fetchAllRecs collects recommendations from all services and regions without
+// purchasing. coverageMap (when non-nil) populates Recommendation.ExistingCoveragePct
+// on each rec before sizing, so --target-coverage can subtract what's already
+// owned in the same pool. The returned DropSummary accumulates per-reason
+// drop counts across every service and region for the end-of-run summary.
+func fetchAllRecs(
+	ctx context.Context,
+	awsCfg aws.Config,
+	recClient provider.RecommendationsClient,
+	accountCache *AccountAliasCache,
+	servicesToProcess []common.ServiceType,
+	engineData engineVersionData,
+	cfg Config,
+	coverageMap recommendations.PoolCoverageMap,
+) ([]common.Recommendation, *common.DropSummary, error) {
+	all := make([]common.Recommendation, 0)
+	drops := common.NewDropSummary()
+	for _, service := range servicesToProcess {
+		AppLogger.Printf("\n━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n")
+		AppLogger.Printf("🔍 Fetching %s recommendations\n", getServiceDisplayName(service))
+		AppLogger.Printf("━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━\n")
+
+		regions, err := determineRegionsForService(ctx, recClient, service, cfg)
+		if err != nil {
+			if ctx.Err() != nil {
+				return nil, drops, nil
+			}
+			return nil, nil, fmt.Errorf("%w: determining regions for %s: %w", errRecommendationFetch, getServiceDisplayName(service), err)
+		}
+		for i, region := range regions {
+			if ctx.Err() != nil {
+				return nil, drops, nil // partial data from an interrupted fetch is not a result
+			}
+			recs, err := fetchAndFilterRegionRecs(ctx, awsCfg, recClient, accountCache, service, region, i+1, len(regions), engineData, cfg, coverageMap, drops)
+			if ctx.Err() != nil {
+				return nil, drops, nil // interrupted mid-fetch: not a result, not a failure
+			}
+			if err != nil {
+				return nil, nil, err
+			}
+			all = append(all, recs...)
+		}
+	}
+	// Checked once over every account seen in any batch, including those the
+	// filters dropped, so an exact account in another batch is not missed.
+	if err := requireExactExcludes(accountCache.knownAccounts(), &cfg); err != nil {
+		return nil, nil, fmt.Errorf("%w: %w", errRecommendationFetch, err)
+	}
+	return all, drops, nil
+}