diff --git a/cmd/main_test.go b/cmd/main_test.go index c685a034c..3473675da 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -2,6 +2,8 @@ package main import ( "fmt" + "os" + "path/filepath" "strings" "testing" @@ -10,6 +12,23 @@ import ( "github.com/stretchr/testify/assert" ) +// TestMain overrides toolCfg.AuditLog's cobra-registered default +// ("./cudly-audit.jsonl", relative to the process working directory) before +// any test in this package runs. Since #1609, processPurchaseLoop (shared by +// the --input-csv path and the legacy per-region purchase path) writes a real +// audit record via cfg.AuditLog on every dry-run and real purchase attempt. +// Without this override, any test that reaches that loop without setting its +// own AuditLog would silently create/append to a stray cmd/cudly-audit.jsonl +// file in the repo working directory on every `go test` run. Tests that need +// to assert on audit-log contents still set their own t.TempDir()-scoped +// 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())) + code := m.Run() + _ = os.Remove(toolCfg.AuditLog) + os.Exit(code) +} + func TestParseServices(t *testing.T) { tests := []struct { name string diff --git a/cmd/multi_service.go b/cmd/multi_service.go index c3ed439eb..9cd3cbb82 100644 --- a/cmd/multi_service.go +++ b/cmd/multi_service.go @@ -421,10 +421,7 @@ func executePurchasePipeline(ctx context.Context, awsCfg aws.Config, recs []comm } result, status := purchaseSingleRec(ctx, awsCfg, rec, i+1, isDryRun, cfg) results = append(results, result) - auditRec := common.NewAuditRecord(runID, rec, result, status, isDryRun, common.PurchaseSourceCLI) - if err := common.WriteAuditRecord(auditRec, cfg.AuditLog); err != nil { - log.Printf("Warning: failed to write audit record: %v", err) - } + writePurchaseAuditRecord(runID, rec, result, status, isDryRun, cfg.AuditLog) if !isDryRun && i < len(recs)-1 && os.Getenv("DISABLE_PURCHASE_DELAY") != "true" { time.Sleep(PurchaseDelaySeconds * time.Second) } @@ -432,6 +429,32 @@ func executePurchasePipeline(ctx context.Context, awsCfg aws.Config, recs []comm return results } +// writePurchaseAuditRecord writes a single purchase's audit record. Shared by +// both purchase entry points -- executePurchasePipeline (the main pipeline) +// and processPurchaseLoop (the --input-csv path) -- so every recommendation +// that reaches a purchase attempt, dry-run or real, is recorded to +// cfg.AuditLog regardless of which one produced it. Before #1609, +// processPurchaseLoop never wrote a record at all, so CSV-mode purchases left +// no audit trail: on a partial failure there was no durable, per-recommendation +// record of which rows succeeded, so an operator could only re-run the whole +// file, which is a double purchase for the rows that already succeeded. +func writePurchaseAuditRecord(runID string, rec common.Recommendation, result common.PurchaseResult, status string, isDryRun bool, auditLogPath string) { + auditRec := common.NewAuditRecord(runID, rec, result, status, isDryRun, common.PurchaseSourceCLI) + if err := common.WriteAuditRecord(auditRec, auditLogPath); err != nil { + log.Printf("Warning: failed to write audit record: %v", err) + } +} + +// purchaseAuditStatus derives the audit status for a completed (non-dry-run) +// purchase attempt. Callers handle the dry-run ("skipped") case separately, +// since that never reaches a PurchaseResult from an actual API call. +func purchaseAuditStatus(result common.PurchaseResult) string { + if result.Success { + return "success" + } + return "error" +} + // purchaseSingleRec executes or dry-runs a single purchase and returns the result + audit status. func purchaseSingleRec(ctx context.Context, awsCfg aws.Config, rec common.Recommendation, index int, isDryRun bool, cfg Config) (purchaseResult common.PurchaseResult, auditStatus string) { AppLogger.Printf(" [%d] %s %s %s (count=%d)\n", index, rec.Service, rec.Region, rec.ResourceType, rec.Count) @@ -450,12 +473,11 @@ func purchaseSingleRec(ctx context.Context, awsCfg aws.Config, rec common.Recomm } result := executePurchase(ctx, rec, rec.Region, index, serviceClient, cfg) - status := "success" - if !result.Success { - status = "error" - AppLogger.Printf(" ❌ %v\n", result.Error) - } else { + status := purchaseAuditStatus(result) + if result.Success { AppLogger.Printf(" ✅ %s\n", result.CommitmentID) + } else { + AppLogger.Printf(" ❌ %v\n", result.Error) } return result, status } @@ -489,6 +511,47 @@ func runCSVPathOrFatal(ctx context.Context, cfg Config) { } } +// prepareCSVPurchaseRun validates and loads everything runToolFromCSV needs +// before the per-service purchase loop: the audit log writability, the CSV +// file, filtering/sizing, and the AWS config. Extracted to keep +// runToolFromCSV under the project's gocyclo budget. +// +// The audit-log check runs first and before any cloud API call, matching the +// non-CSV path (CheckAuditLogWritable in runToolMultiService). Before #1609 +// this check ran only on the non-CSV path, so a CSV-mode purchase run could +// reach real purchase calls with no way to have written a durable, +// per-recommendation audit record even in principle. +// +// A nil recs with a nil error means "nothing to process after filtering", +// which the caller treats as success rather than an error. +func prepareCSVPurchaseRun(ctx context.Context, cfg Config, csvModeCoverage float64) (recs []common.Recommendation, awsCfg aws.Config, runID string, err error) { + if err = CheckAuditLogWritable(cfg.AuditLog); err != nil { + return nil, aws.Config{}, "", fmt.Errorf("cannot write audit log: %w", err) + } + + AppLogger.Printf("📄 Reading recommendations from CSV: %s\n", cfg.CSVInput) + recs, err = loadRecommendationsFromCSV(cfg.CSVInput) + if err != nil { + return nil, aws.Config{}, "", fmt.Errorf("failed to read CSV file: %w", err) + } + AppLogger.Printf("✅ Loaded %d recommendations from CSV\n", len(recs)) + + recs, err = filterAndAdjustRecommendations(recs, csvModeCoverage, cfg) + if err != nil { + return nil, aws.Config{}, "", err + } + if len(recs) == 0 { + return nil, aws.Config{}, "", nil + } + + awsCfg, err = loadAWSConfig(ctx, cfg) + if err != nil { + return nil, aws.Config{}, "", fmt.Errorf("failed to load AWS config: %w", err) + } + + return recs, awsCfg, uuid.New().String(), nil +} + // runToolFromCSV processes recommendations from a CSV input file. // It returns an error instead of exiting so the orchestration glue is // unit-testable; the caller (runCSVPathOrFatal) turns errors fatal. @@ -498,32 +561,15 @@ func runToolFromCSV(ctx context.Context, cfg Config) error { csvModeCoverage := determineCSVCoverage(cfg) - AppLogger.Printf("📄 Reading recommendations from CSV: %s\n", cfg.CSVInput) - - // Read recommendations from CSV - recs, err := loadRecommendationsFromCSV(cfg.CSVInput) - if err != nil { - return fmt.Errorf("failed to read CSV file: %w", err) - } - - AppLogger.Printf("✅ Loaded %d recommendations from CSV\n", len(recs)) - - // Filter and adjust recommendations - recs, err = filterAndAdjustRecommendations(recs, csvModeCoverage, cfg) + recs, awsCfg, runID, err := prepareCSVPurchaseRun(ctx, cfg, csvModeCoverage) if err != nil { return err } - if len(recs) == 0 { AppLogger.Println("⚠️ No recommendations to process after filtering") return nil } - awsCfg, err := loadAWSConfig(ctx, cfg) - if err != nil { - return fmt.Errorf("failed to load AWS config: %w", err) - } - // Create account alias cache for lookup accountCache := NewAccountAliasCache(awsCfg) @@ -581,7 +627,7 @@ func runToolFromCSV(ctx context.Context, cfg Config) error { allAdjustedRecs = append(allAdjustedRecs, recs...) // Process purchases for this region - regionResults := processPurchaseLoop(ctx, recs, region, isDryRun, serviceClient, cfg) + regionResults := processPurchaseLoop(ctx, recs, region, isDryRun, serviceClient, cfg, runID) serviceResults = append(serviceResults, regionResults...) } @@ -721,8 +767,11 @@ func processService(ctx context.Context, awsCfg aws.Config, recClient provider.R return serviceRecs, serviceResults } -// processPurchaseLoop processes purchases for a single region (used by CSV mode). -func processPurchaseLoop(ctx context.Context, recs []common.Recommendation, region string, isDryRun bool, serviceClient provider.ServiceClient, cfg Config) []common.PurchaseResult { +// processPurchaseLoop processes purchases for a single region (used by CSV +// mode). runID groups every recommendation processed across the whole CSV +// run into one audit trail, matching how executePurchasePipeline (the main +// pipeline) generates one runID per invocation. +func processPurchaseLoop(ctx context.Context, recs []common.Recommendation, region string, isDryRun bool, serviceClient provider.ServiceClient, cfg Config, runID string) []common.PurchaseResult { results := make([]common.PurchaseResult, 0, len(recs)) for j := range recs { @@ -731,8 +780,10 @@ func processPurchaseLoop(ctx context.Context, recs []common.Recommendation, regi AppLogger.Printf(" 💳 Purchasing %d instances\n", rec.Count) var result common.PurchaseResult + var status string if isDryRun { result = createDryRunResult(rec, region, j+1, cfg) + status = "skipped" } else { // Ask for confirmation before proceeding with purchases (only on first item) if j == 0 { @@ -744,13 +795,17 @@ func processPurchaseLoop(ctx context.Context, recs []common.Recommendation, regi } if !ConfirmPurchase(totalInstances, totalSavings, cfg.SkipConfirmation) { - // User canceled - return canceled results for all + // User canceled - return canceled results for all. No audit + // record is written for a declined run, matching the + // non-CSV path: runPurchaseAndReport returns before ever + // calling executePurchasePipeline when the user declines. return createCancelledResults(recs, region, cfg) } } // Execute actual purchase result = executePurchase(ctx, rec, region, j+1, serviceClient, cfg) + status = purchaseAuditStatus(result) // Add delay between purchases to avoid rate limiting if j < len(recs)-1 && os.Getenv("DISABLE_PURCHASE_DELAY") != "true" { @@ -758,6 +813,7 @@ func processPurchaseLoop(ctx context.Context, recs []common.Recommendation, regi } } + writePurchaseAuditRecord(runID, rec, result, status, isDryRun, cfg.AuditLog) results = append(results, result) if result.Success { diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go index fe45c6509..b537fdfcb 100644 --- a/cmd/multi_service_helpers.go +++ b/cmd/multi_service_helpers.go @@ -14,6 +14,7 @@ import ( azureprovider "github.com/LeanerCloud/cloud-commitments-go/providers/azure" "github.com/aws/aws-sdk-go-v2/aws" awsec2 "github.com/aws/aws-sdk-go-v2/service/ec2" + "github.com/google/uuid" ) // EC2ClientInterface defines the interface for EC2 operations. @@ -436,8 +437,13 @@ func processRegionRecommendations( // Check for duplicate RIs. Drop tracking skipped (nil). adjustedRecs := checkDuplicates(ctx, filteredRecs, serviceClient, isDryRun, nil) - // Process purchases - regionResults := processPurchaseLoop(ctx, adjustedRecs, region, isDryRun, serviceClient, cfg) + // Process purchases. This legacy per-region entry point has no run-wide + // runID of its own (unlike runToolMultiService/runToolFromCSV, which mint + // one per invocation), so each call gets its own -- every recommendation + // it processes still ends up in cfg.AuditLog with a durable, groupable + // record; it is simply not grouped with a sibling region's run. + runID := uuid.New().String() + regionResults := processPurchaseLoop(ctx, adjustedRecs, region, isDryRun, serviceClient, cfg, runID) result.results = regionResults return result diff --git a/cmd/multi_service_max_instances_test.go b/cmd/multi_service_max_instances_test.go index 7c11bd54d..0da06c8f1 100644 --- a/cmd/multi_service_max_instances_test.go +++ b/cmd/multi_service_max_instances_test.go @@ -638,6 +638,7 @@ rds,us-east-1,db.t3.large,postgres,6,100.00,1yr,All Upfront,123456789012 reportPath := filepath.Join(t.TempDir(), "report.csv") toolCfg.CSVInput = csvPath toolCfg.CSVOutput = reportPath + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.ActualPurchase = false toolCfg.Coverage = 100.0 toolCfg.TargetCoverage = 0 diff --git a/cmd/multi_service_test.go b/cmd/multi_service_test.go index e12a60208..c774ad032 100644 --- a/cmd/multi_service_test.go +++ b/cmd/multi_service_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/csv" + "encoding/json" "fmt" "log" "os" @@ -1261,6 +1262,7 @@ func TestProcessPurchaseLoopPurchaseFailure(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 80.0 toolCfg.SkipConfirmation = true @@ -1281,7 +1283,7 @@ func TestProcessPurchaseLoopPurchaseFailure(t *testing.T) { t.Setenv("DISABLE_PURCHASE_DELAY", "true") - results := processPurchaseLoop(ctx, recs, "ap-south-1", false, mockClient, toolCfg) + results := processPurchaseLoop(ctx, recs, "ap-south-1", false, mockClient, toolCfg, "test-run") assert.Len(t, results, 1) assert.False(t, results[0].Success) @@ -1296,6 +1298,7 @@ func TestProcessPurchaseLoopUserCancellation(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 90.0 toolCfg.SkipConfirmation = false // User will be prompted @@ -1324,7 +1327,7 @@ func TestProcessPurchaseLoopUserCancellation(t *testing.T) { t.Setenv("DISABLE_PURCHASE_DELAY", "true") - results := processPurchaseLoop(ctx, recs, "eu-central-1", false, mockClient, toolCfg) + results := processPurchaseLoop(ctx, recs, "eu-central-1", false, mockClient, toolCfg, "test-run") assert.Len(t, results, 2) for _, result := range results { @@ -1343,7 +1346,7 @@ func TestProcessPurchaseLoopEmptyRecommendations(t *testing.T) { mockClient := &MockServiceClient{} - results := processPurchaseLoop(ctx, []common.Recommendation{}, "us-east-1", false, mockClient, toolCfg) + results := processPurchaseLoop(ctx, []common.Recommendation{}, "us-east-1", false, mockClient, toolCfg, "test-run") assert.Empty(t, results) mockClient.AssertNotCalled(t, "PurchaseCommitment", mock.Anything, mock.Anything, mock.Anything) @@ -1354,6 +1357,7 @@ func TestProcessServicePurchasesUserCancellation(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 85.0 toolCfg.SkipConfirmation = true // Skip for testing @@ -1372,7 +1376,7 @@ func TestProcessServicePurchasesUserCancellation(t *testing.T) { t.Setenv("DISABLE_PURCHASE_DELAY", "true") - results := processPurchaseLoop(ctx, recs, "us-west-1", false, mockClient, toolCfg) + results := processPurchaseLoop(ctx, recs, "us-west-1", false, mockClient, toolCfg, "test-run") assert.Len(t, results, 1) assert.True(t, results[0].Success) @@ -1386,6 +1390,7 @@ func TestProcessServicePurchasesDryRunMultiple(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 100.0 recs := []common.Recommendation{ @@ -1397,7 +1402,7 @@ func TestProcessServicePurchasesDryRunMultiple(t *testing.T) { mockClient := &MockServiceClient{} // Dry run should not call PurchaseCommitment - results := processPurchaseLoop(ctx, recs, "ap-northeast-1", true, mockClient, toolCfg) + results := processPurchaseLoop(ctx, recs, "ap-northeast-1", true, mockClient, toolCfg, "test-run") assert.Len(t, results, 3) for i, result := range results { @@ -1461,6 +1466,7 @@ func TestProcessPurchaseLoopDryRun(t *testing.T) { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 75.0 recs := []common.Recommendation{ @@ -1472,7 +1478,7 @@ func TestProcessPurchaseLoopDryRun(t *testing.T) { // Logger output disabled for testing - results := processPurchaseLoop(ctx, recs, "us-east-1", true, mockClient, toolCfg) + results := processPurchaseLoop(ctx, recs, "us-east-1", true, mockClient, toolCfg, "test-run") assert.Len(t, results, 2) for _, result := range results { @@ -1495,6 +1501,7 @@ func TestProcessPurchaseLoopActualPurchase(t *testing.T) { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 80.0 toolCfg.SkipConfirmation = true // Skip confirmation for testing @@ -1520,7 +1527,7 @@ func TestProcessPurchaseLoopActualPurchase(t *testing.T) { // Disable purchase delay for testing t.Setenv("DISABLE_PURCHASE_DELAY", "true") - results := processPurchaseLoop(ctx, recs, "eu-west-1", false, mockClient, toolCfg) + results := processPurchaseLoop(ctx, recs, "eu-west-1", false, mockClient, toolCfg, "test-run") assert.Len(t, results, 2) for i, result := range results { @@ -1540,6 +1547,7 @@ func TestProcessPurchaseLoopWithConfirmation(t *testing.T) { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.Coverage = 80.0 toolCfg.SkipConfirmation = true // Skip confirmation to proceed with purchase @@ -1563,7 +1571,7 @@ func TestProcessPurchaseLoopWithConfirmation(t *testing.T) { // Disable purchase delay for testing t.Setenv("DISABLE_PURCHASE_DELAY", "true") - results := processPurchaseLoop(ctx, recs, "us-west-2", false, mockClient, toolCfg) + results := processPurchaseLoop(ctx, recs, "us-west-2", false, mockClient, toolCfg, "test-run") assert.Len(t, results, 1) assert.True(t, results[0].Success) @@ -1763,6 +1771,7 @@ elasticache,us-west-2,cache.t3.micro,redis,1,1yr,All Upfront,123456789012 reportPath := filepath.Join(t.TempDir(), "report.csv") toolCfg.CSVInput = csvPath toolCfg.CSVOutput = reportPath + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.ActualPurchase = false toolCfg.Coverage = tt.coverage toolCfg.TargetCoverage = 0 @@ -1831,6 +1840,7 @@ func TestRunToolFromCSV_NonExistentFile(t *testing.T) { defer func() { toolCfg = origCfg }() toolCfg.CSVInput = filepath.Join(t.TempDir(), "does-not-exist.csv") + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.ActualPurchase = false err := runToolFromCSV(context.Background(), toolCfg) @@ -1846,6 +1856,7 @@ func TestRunToolFromCSV_EmptyFile(t *testing.T) { defer func() { toolCfg = origCfg }() toolCfg.CSVInput = writeTestRecommendationsCSV(t, "") + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.ActualPurchase = false err := runToolFromCSV(context.Background(), toolCfg) @@ -1872,6 +1883,7 @@ rds,us-east-1,db.t3.large,postgres,10,300.00,1yr,All Upfront,123456789012 reportPath := filepath.Join(t.TempDir(), "report.csv") toolCfg.CSVInput = csvPath toolCfg.CSVOutput = reportPath + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.ActualPurchase = false toolCfg.Coverage = 100.0 toolCfg.TargetCoverage = 0 @@ -1905,6 +1917,7 @@ rds,us-east-1,db.t3.medium,mysql,5,1yr,All Upfront,123456789012 reportPath := filepath.Join(t.TempDir(), "report.csv") toolCfg.CSVInput = csvPath toolCfg.CSVOutput = reportPath + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") toolCfg.ActualPurchase = false toolCfg.Coverage = 100.0 toolCfg.TargetCoverage = 0 @@ -1922,6 +1935,147 @@ rds,us-east-1,db.t3.medium,mysql,5,1yr,All Upfront,123456789012 } } +// TestRunToolFromCSV_WritesAuditRecordsOnDryRun reproduces #1609 through the +// public --input-csv entry point: before the fix, processPurchaseLoop never +// wrote an audit record at all, on a dry run or a real purchase. A dry run is +// used here (rather than ActualPurchase=true with invalid credentials) +// because #1941 made the duplicate check fail closed on a real purchase run: +// with no valid AWS credentials the check now refuses the region before ever +// reaching a purchase attempt, and writes no audit record at all by design +// (see TestCheckDuplicates_ErrorOnPurchaseRun_DropsRecsRatherThanFallingBack) +// -- so there is no way to drive an un-mocked real-purchase attempt through +// this entry point in a test. A dry run still exercises the exact wiring +// #1609 fixes (prepareCSVPurchaseRun's runID -> processPurchaseLoop -> +// writePurchaseAuditRecord), since the duplicate check only warns and +// continues on a dry run rather than refusing. +// TestProcessPurchaseLoop_WritesAuditRecordForRealPurchase below covers the +// "success"/"error" statuses a real purchase attempt gets audited with. +func TestRunToolFromCSV_WritesAuditRecordsOnDryRun(t *testing.T) { + origCfg := toolCfg + defer func() { toolCfg = origCfg }() + isolateAWSEnv(t) + + csvPath := writeTestRecommendationsCSV(t, `Service,Region,ResourceType,Engine,Count,Term,PaymentOption,Account +rds,us-east-1,db.t3.small,postgres,2,1yr,All Upfront,123456789012 +`) + auditPath := filepath.Join(t.TempDir(), "audit.jsonl") + + toolCfg.CSVInput = csvPath + toolCfg.CSVOutput = filepath.Join(t.TempDir(), "report.csv") + toolCfg.AuditLog = auditPath + toolCfg.ActualPurchase = false + toolCfg.Coverage = 100.0 + toolCfg.TargetCoverage = 0 + toolCfg.MaxInstances = 0 + toolCfg.OverrideCount = 0 + + err := runToolFromCSV(context.Background(), toolCfg) + require.NoError(t, err) + + data, readErr := os.ReadFile(auditPath) // #nosec G304 -- test-owned tempdir path + require.NoError(t, readErr, "a purchase run through --input-csv must write an audit log") + require.NotEmpty(t, data, "the audit log must not be empty -- an empty file would make the next length check pass vacuously") + + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + require.Len(t, lines, 1, "one audit record per recommendation") + + var rec map[string]any + require.NoError(t, json.Unmarshal([]byte(lines[0]), &rec)) + assert.NotEmpty(t, rec["run_id"], "the audit record must carry a run ID grouping the CSV run") + assert.Equal(t, common.PurchaseSourceCLI, rec["source"]) + assert.Equal(t, "db.t3.small", rec["resource_type"]) + assert.Equal(t, true, rec["dry_run"]) + assert.Equal(t, "skipped", rec["status"]) +} + +// TestProcessPurchaseLoop_WritesAuditRecordForRealPurchase reproduces the +// other half of #1609 at the processPurchaseLoop level: a real (non-dry-run) +// purchase attempt must be audited as "success" or "error", never silently +// dropped. createServiceClient is not injectable (see the comment on +// TestApplyMinCountFloorAfterDuplicateAdjustment), so this exercises +// processPurchaseLoop directly with a mocked provider.ServiceClient rather +// than through runToolFromCSV -- the same technique +// TestProcessPurchaseLoopActualPurchase already uses to test this loop's +// purchase behavior without live AWS credentials. +func TestProcessPurchaseLoop_WritesAuditRecordForRealPurchase(t *testing.T) { + tests := []struct { + result common.PurchaseResult + name string + wantStatus string + }{ + { + name: "success", + result: common.PurchaseResult{Success: true, CommitmentID: "test-purchase-id", Timestamp: time.Now()}, + wantStatus: "success", + }, + { + name: "error", + result: common.PurchaseResult{Success: false, Error: fmt.Errorf("API error: quota exceeded"), Timestamp: time.Now()}, + wantStatus: "error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + origCfg := toolCfg + defer func() { toolCfg = origCfg }() + toolCfg.AuditLog = filepath.Join(t.TempDir(), "audit.jsonl") + toolCfg.SkipConfirmation = true + t.Setenv("DISABLE_PURCHASE_DELAY", "true") + + recs := []common.Recommendation{ + {Service: common.ServiceRDS, ResourceType: "db.t3.small", Count: 2, EstimatedSavings: 100}, + } + result := tt.result + result.Recommendation = recs[0] + + mockClient := &MockServiceClient{} + mockClient.On("PurchaseCommitment", ctx, recs[0], mock.MatchedBy(func(o common.PurchaseOptions) bool { return o.Source == common.PurchaseSourceCLI })).Return(result, nil) + + processPurchaseLoop(ctx, recs, "us-east-1", false /* isDryRun */, mockClient, toolCfg, "test-run-id") + + data, readErr := os.ReadFile(toolCfg.AuditLog) // #nosec G304 -- test-owned tempdir path + require.NoError(t, readErr, "a real purchase attempt must write an audit log") + require.NotEmpty(t, data, "the audit log must not be empty -- an empty file would make the next length check pass vacuously") + + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + require.Len(t, lines, 1, "one audit record per recommendation") + + var rec map[string]any + require.NoError(t, json.Unmarshal([]byte(lines[0]), &rec)) + assert.Equal(t, "test-run-id", rec["run_id"]) + assert.Equal(t, common.PurchaseSourceCLI, rec["source"]) + assert.Equal(t, false, rec["dry_run"]) + assert.Equal(t, tt.wantStatus, rec["status"]) + + mockClient.AssertExpectations(t) + }) + } +} + +// TestRunToolFromCSV_ChecksAuditLogWritability reproduces the second half of +// #1609: the --input-csv path never verified the audit log was writable +// before touching AWS, unlike the non-CSV path (CheckAuditLogWritable in +// runToolMultiService). An unwritable audit-log directory must be rejected +// before the CSV is even read, on both dry-run and purchase invocations. +func TestRunToolFromCSV_ChecksAuditLogWritability(t *testing.T) { + origCfg := toolCfg + defer func() { toolCfg = origCfg }() + + csvPath := writeTestRecommendationsCSV(t, `Service,Region,ResourceType,Engine,Count,Term,PaymentOption,Account +rds,us-east-1,db.t3.small,postgres,2,1yr,All Upfront,123456789012 +`) + + toolCfg.CSVInput = csvPath + toolCfg.AuditLog = filepath.Join(t.TempDir(), "does-not-exist-dir", "audit.jsonl") + toolCfg.ActualPurchase = false + + err := runToolFromCSV(context.Background(), toolCfg) + require.Error(t, err) + assert.Contains(t, err.Error(), "audit log") +} + // ==================== Tests for adjustRecommendationForExcludedVersions ==================== // Helper to create test version info with extended support dates.