diff --git a/cmd/aws_fixture_stub_test.go b/cmd/aws_fixture_stub_test.go new file mode 100644 index 000000000..cb1353fe3 --- /dev/null +++ b/cmd/aws_fixture_stub_test.go @@ -0,0 +1,148 @@ +package main + +import ( + "bytes" + "fmt" + "io" + "maps" + "net/http" + "net/url" + "path" + "strings" + "sync" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// awsFixtureStub is the offline AWS endpoint behind the tests that build their +// own aws.Config. Every request must match a fixture keyed "service operation"; +// anything else fails the test, so a request that would have left the machine +// is reported instead of silently timing out. The region in the host is +// ignored for the lookup but kept in received ("host operation"). +type awsFixtureStub struct { + t *testing.T + mu sync.Mutex + received map[string]int + fixtures map[string]awsFixture + log *bytes.Buffer +} + +// awsFixture is one canned response; a zero status means 200. +type awsFixture struct { + body string + status int +} + +// awsFixtures hold the default per-operation responses: empty existing +// commitments, one SDK-parseable body per protocol (Query XML, rest-json). +var awsFixtures = map[string]awsFixture{ + "rds DescribeReservedDBInstances": {body: emptyRDSFixture}, + "elasticache DescribeReservedCacheNodes": {body: emptyElastiCacheFixture}, + "savingsplans DescribeSavingsPlans": {body: `{"savingsPlans":[]}`}, + "ec2 DescribeRegions": {body: describeRegionsFixture}, + // A non-retryable error, for tests of the fetch-failure path. + "ce GetReservationCoverage": {status: http.StatusBadRequest, body: `{"__type":"AccessDeniedException","message":"denied by fixture"}`}, +} + +const ( + emptyRDSFixture = `` + + `` + + `fixture` + emptyElastiCacheFixture = `` + + `` + + `fixture` + // Unsorted on purpose: getAllAWSRegions must sort. + describeRegionsFixture = `fixture` + + `us-west-2eu-west-1` +) + +// operation names the API call: the Query "Action", else the JSON +// X-Amz-Target suffix, else the last path segment (rest-json). +func awsOperation(r *http.Request, form url.Values) string { + if a := form.Get("Action"); a != "" { + return a + } + if tgt := r.Header.Get("X-Amz-Target"); tgt != "" { + _, op, _ := strings.Cut(tgt, ".") + return op + } + return path.Base(r.URL.Path) +} + +func (s *awsFixtureStub) RoundTrip(r *http.Request) (*http.Response, error) { + var b []byte + if r.Body != nil { + b, _ = io.ReadAll(r.Body) + } + form, _ := url.ParseQuery(string(b)) + op := awsOperation(r, form) + s.mu.Lock() + s.received[r.URL.Host+" "+op]++ + s.mu.Unlock() + svc, _, _ := strings.Cut(r.URL.Host, ".") + fx, ok := s.fixtures[svc+" "+op] + if !ok { + s.t.Errorf("unexpected AWS request %s %s %s", r.Method, r.URL.Host, op) + fx = awsFixture{status: http.StatusInternalServerError, body: "unexpected request"} + } + if fx.status == 0 { + fx.status = http.StatusOK + } + contentType := "application/x-amz-json-1.1" + if strings.HasPrefix(fx.body, "<") { + contentType = "text/xml" + } + return &http.Response{ + StatusCode: fx.status, + Header: http.Header{"Content-Type": []string{contentType}}, + Body: io.NopCloser(strings.NewReader(fx.body)), + Request: r, + }, nil +} + +// newAWSFixtureConfig returns an aws.Config whose HTTP client is the +// stub and whose credentials are anonymous, so requests are unsigned and need +// no environment. +func newAWSFixtureConfig(t *testing.T) (aws.Config, *awsFixtureStub) { + t.Helper() + stub := &awsFixtureStub{t: t, received: map[string]int{}, fixtures: maps.Clone(awsFixtures), log: captureAppLog(t)} + return aws.Config{ + Region: "us-east-1", + HTTPClient: &http.Client{Transport: stub}, + Credentials: aws.AnonymousCredentials{}, + Retryer: func() aws.Retryer { return aws.NopRetryer{} }, + }, stub +} + +// requireRequests asserts the exact "host operation" request counts the stub +// received, so a test cannot pass without its AWS calls reaching the stub. +func (s *awsFixtureStub) requireRequests(t *testing.T, want map[string]int) { + t.Helper() + s.mu.Lock() + defer s.mu.Unlock() + assert.Equal(t, want, s.received) +} + +// requireDuplicateChecks asserts the duplicate check really ran against the +// stub: one call per (service, region) pair, no other request, and every +// response parsed. +func (s *awsFixtureStub) requireDuplicateChecks(t *testing.T, pairs int) { + t.Helper() + s.mu.Lock() + defer s.mu.Unlock() + total := 0 + for k, n := range s.received { + host, op, _ := strings.Cut(k, " ") + svc, _, _ := strings.Cut(host, ".") + assert.Contains(t, s.fixtures, svc+" "+op) + assert.Equalf(t, 1, n, "duplicate-check requests for %s", k) + total += n + } + // A fixture the SDK cannot parse only logs this warning in dry-run, so the + // request count alone would not notice a broken fixture. + assert.NotContains(t, s.log.String(), "Could not check for existing RIs") + require.Equal(t, pairs, total, fmt.Sprintf("duplicate-check requests received: %v", s.received)) +} diff --git a/cmd/cancellation_prepurchase_test.go b/cmd/cancellation_prepurchase_test.go index e80de9519..683010a6a 100644 --- a/cmd/cancellation_prepurchase_test.go +++ b/cmd/cancellation_prepurchase_test.go @@ -112,9 +112,15 @@ func TestFetchAllRecs_DefaultRunKeepsRecommendationsInRegionsOutsideTheCallersOp 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) + awsCfg, awsStub := newAWSFixtureConfig(t) + recs, _, err := fetchAllRecs(context.Background(), awsCfg, client, NewAccountAliasCache(awsCfg), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil) require.NoError(t, err) + // One duplicate check per region: us-east-1 and eu-central-2. + awsStub.requireRequests(t, map[string]int{ + "rds.us-east-1.amazonaws.com DescribeReservedDBInstances": 1, + "rds.eu-central-2.amazonaws.com DescribeReservedDBInstances": 1, + }) regions := make([]string, 0, len(recs)) for i := range recs { regions = append(regions, recs[i].Region) @@ -235,6 +241,7 @@ func TestFetchAllRecs_AccountFilters(t *testing.T) { } base := Config{Regions: []string{"us-east-1"}, Coverage: 100, TermYears: 1, PaymentOption: "partial-upfront"} // Member accounts cannot call DescribeAccount: every lookup is denied. + dupChecks := 0 run := func(t *testing.T, cfg Config, accounts ...string) (accountsOut []string, orgCalls int, err error) { t.Helper() found := make([]common.Recommendation, 0, len(accounts)) @@ -245,7 +252,11 @@ func TestFetchAllRecs_AccountFilters(t *testing.T) { client.On("GetRecommendations", mock.Anything, mock.Anything).Return(found, nil) org := &MockOrganizationsClient{} org.On("DescribeAccount", mock.Anything, mock.Anything).Return(nil, errors.New("AccessDenied")) - recs, _, err := fetchAllRecs(context.Background(), aws.Config{}, client, NewAccountAliasCacheWithClient(org), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil) + awsCfg, awsStub := newAWSFixtureConfig(t) + recs, _, err := fetchAllRecs(context.Background(), awsCfg, client, NewAccountAliasCacheWithClient(org), []common.ServiceType{common.ServiceRDS}, engineVersionData{}, cfg, nil) + for _, n := range awsStub.received { + dupChecks += n + } for i := range recs { accountsOut = append(accountsOut, recs[i].Account) } @@ -296,6 +307,7 @@ func TestFetchAllRecs_AccountFilters(t *testing.T) { require.NoError(t, err) assert.Equal(t, 1, calls) }) + assert.Equal(t, 4, dupChecks, "duplicate checks served by the fixture stub") } func TestRequireResolvedAccountNames_CSVRows(t *testing.T) { diff --git a/cmd/multi_service_coverage_test.go b/cmd/multi_service_coverage_test.go index bdd7b4a68..1cb30e5e2 100644 --- a/cmd/multi_service_coverage_test.go +++ b/cmd/multi_service_coverage_test.go @@ -860,7 +860,7 @@ func TestFetchExistingCoverage_LookbackDays(t *testing.T) { // ce:GetReservationCoverage permission would. func TestFetchExistingCoverage_FetchFailure(t *testing.T) { ctx := context.Background() - awsCfg := aws.Config{Region: "us-east-1"} + awsCfg, awsStub := newAWSFixtureConfig(t) realClient := awsprovider.NewRecommendationsClientDirect(awsCfg) t.Run("purchase run aborts: returns the error and a nil map", func(t *testing.T) { @@ -876,4 +876,7 @@ func TestFetchExistingCoverage_FetchFailure(t *testing.T) { require.NoError(t, err, "a dry run must not abort; nothing is bought") assert.Nil(t, got) }) + + // Each case asked CE once; the fixture answers with a non-retryable 400. + awsStub.requireRequests(t, map[string]int{"ce.us-east-1.amazonaws.com GetReservationCoverage": 2}) } diff --git a/cmd/multi_service_helpers_test.go b/cmd/multi_service_helpers_test.go index dfee0d931..8f0a740d8 100644 --- a/cmd/multi_service_helpers_test.go +++ b/cmd/multi_service_helpers_test.go @@ -3,7 +3,6 @@ package main import ( "context" "errors" - "os" "testing" "time" @@ -89,24 +88,13 @@ func TestGetAllAWSRegions(t *testing.T) { }) } - t.Run("Integration test", func(t *testing.T) { - // This test requires actual AWS credentials - if os.Getenv("AWS_ACCESS_KEY_ID") == "" { - t.Skip("Skipping integration test: AWS credentials not present") - } - - cfg := aws.Config{Region: "us-east-1"} + t.Run("SDK client path against the fixture endpoint", func(t *testing.T) { + cfg, awsStub := newAWSFixtureConfig(t) regions, err := getAllAWSRegions(ctx, cfg) - if err == nil { - assert.NotNil(t, regions) - assert.Greater(t, len(regions), 0) - - // Verify regions are sorted - for i := 1; i < len(regions); i++ { - assert.LessOrEqual(t, regions[i-1], regions[i]) - } - } + require.NoError(t, err) + assert.Equal(t, []string{"eu-west-1", "us-west-2"}, regions, "regions are sorted") + awsStub.requireRequests(t, map[string]int{"ec2.us-east-1.amazonaws.com DescribeRegions": 1}) }) } diff --git a/cmd/multi_service_max_instances_test.go b/cmd/multi_service_max_instances_test.go index 51abe204e..1dff4d972 100644 --- a/cmd/multi_service_max_instances_test.go +++ b/cmd/multi_service_max_instances_test.go @@ -1,109 +1,16 @@ package main import ( - "bytes" "context" - "fmt" - "io" - "net/http" - "net/url" "path/filepath" - "strings" - "sync" "testing" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" - "github.com/aws/aws-sdk-go-v2/aws" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) -// maxInstancesAWSStub is the offline AWS endpoint behind the duplicate check -// (checkDuplicates -> GetExistingCommitments). Every request must match a -// fixture keyed "host Action"; anything else fails the test, so a request that -// would have left the machine is reported instead of silently timing out. -type maxInstancesAWSStub struct { - t *testing.T - mu sync.Mutex - received map[string]int - log *bytes.Buffer -} - -// maxInstancesAWSFixtures answer the Query-protocol DescribeReserved* calls -// with an empty existing-commitment list. -var maxInstancesAWSFixtures = map[string]string{ - "rds.us-east-1.amazonaws.com DescribeReservedDBInstances": maxInstancesEmptyRDS, - "rds.us-west-2.amazonaws.com DescribeReservedDBInstances": maxInstancesEmptyRDS, - "rds.eu-west-1.amazonaws.com DescribeReservedDBInstances": maxInstancesEmptyRDS, - "elasticache.us-east-1.amazonaws.com DescribeReservedCacheNodes": maxInstancesEmptyElastiCache, - "elasticache.us-west-2.amazonaws.com DescribeReservedCacheNodes": maxInstancesEmptyElastiCache, - "elasticache.eu-west-1.amazonaws.com DescribeReservedCacheNodes": maxInstancesEmptyElastiCache, -} - -const ( - maxInstancesEmptyRDS = `` + - `` + - `fixture` - maxInstancesEmptyElastiCache = `` + - `` + - `fixture` -) - -func (s *maxInstancesAWSStub) RoundTrip(r *http.Request) (*http.Response, error) { - b, _ := io.ReadAll(r.Body) - form, _ := url.ParseQuery(string(b)) - key := r.URL.Host + " " + form.Get("Action") - s.mu.Lock() - s.received[key]++ - s.mu.Unlock() - body, ok := maxInstancesAWSFixtures[key] - status := http.StatusOK - if !ok { - s.t.Errorf("unexpected AWS request %s %s%s", r.Method, key, r.URL.Path) - status, body = http.StatusInternalServerError, "unexpected request" - } - return &http.Response{ - StatusCode: status, - Header: http.Header{"Content-Type": []string{"text/xml"}}, - Body: io.NopCloser(strings.NewReader(body)), - Request: r, - }, nil -} - -// newMaxInstancesAWSConfig returns an aws.Config whose HTTP client is the -// stub and whose credentials are anonymous, so requests are unsigned and need -// no environment. -func newMaxInstancesAWSConfig(t *testing.T) (aws.Config, *maxInstancesAWSStub) { - t.Helper() - stub := &maxInstancesAWSStub{t: t, received: map[string]int{}, log: captureAppLog(t)} - return aws.Config{ - Region: "us-east-1", - HTTPClient: &http.Client{Transport: stub}, - Credentials: aws.AnonymousCredentials{}, - Retryer: func() aws.Retryer { return aws.NopRetryer{} }, - }, stub -} - -// requireDuplicateChecks asserts the duplicate check really ran against the -// stub: one call per (service, region) pair, no other request, and every -// response parsed. -func (s *maxInstancesAWSStub) requireDuplicateChecks(t *testing.T, pairs int) { - t.Helper() - s.mu.Lock() - defer s.mu.Unlock() - total := 0 - for k, n := range s.received { - assert.Contains(t, maxInstancesAWSFixtures, k) - assert.Equalf(t, 1, n, "duplicate-check requests for %s", k) - total += n - } - // A fixture the SDK cannot parse only logs this warning in dry-run, so the - // request count alone would not notice a broken fixture. - assert.NotContains(t, s.log.String(), "Could not check for existing RIs") - require.Equal(t, pairs, total, fmt.Sprintf("duplicate-check requests received: %v", s.received)) -} - // maxInstancesFixtureServices and maxInstancesFixtureRegions describe the // multi-service, multi-region fan-out the run-wide cap has to survive. A // single-service or single-region fixture stays green with the per-region bug @@ -169,7 +76,7 @@ func TestMaxInstancesCapsWholeRunAcrossServicesAndRegions(t *testing.T) { const maxInstances = 10 ctx := context.Background() - awsCfg, awsStub := newMaxInstancesAWSConfig(t) + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg t.Cleanup(func() { toolCfg = origCfg }) @@ -242,7 +149,7 @@ func TestMaxInstancesKeepsHighestSavingsNotFirstFetched(t *testing.T) { ) ctx := context.Background() - awsCfg, awsStub := newMaxInstancesAWSConfig(t) + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg t.Cleanup(func() { toolCfg = origCfg }) @@ -317,7 +224,7 @@ func TestMaxInstancesNeverPurchasesBelowMinCount(t *testing.T) { ) ctx := context.Background() - awsCfg, awsStub := newMaxInstancesAWSConfig(t) + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg t.Cleanup(func() { toolCfg = origCfg }) @@ -407,7 +314,7 @@ func TestDropTruncatedBelowMinCount(t *testing.T) { // run that never asked for one. func TestMaxInstancesNotAppliedWhenUnset(t *testing.T) { ctx := context.Background() - awsCfg, awsStub := newMaxInstancesAWSConfig(t) + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg t.Cleanup(func() { toolCfg = origCfg }) diff --git a/cmd/multi_service_test.go b/cmd/multi_service_test.go index 99644bf9b..8498296ea 100644 --- a/cmd/multi_service_test.go +++ b/cmd/multi_service_test.go @@ -7,10 +7,13 @@ import ( "encoding/json" "fmt" "log" + "net/http" + "net/http/httptest" "os" "path/filepath" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -209,7 +212,14 @@ func TestProcessService_EdgeCases(t *testing.T) { func TestProcessServiceWithMocks(t *testing.T) { useCallerAccount(t) ctx := context.Background() - awsCfg := aws.Config{Region: "us-east-1"} + awsCfg, awsStub := newAWSFixtureConfig(t) + // The real-purchase case below relies on a failed duplicate check refusing + // the purchase (fail closed); an empty fixture would instead let it reach + // the purchase call. + awsStub.fixtures["elasticache DescribeReservedCacheNodes"] = awsFixture{ + status: http.StatusForbidden, + body: `{"__type":"AccessDeniedException","message":"denied by test"}`, + } // Save original values origCfg := toolCfg @@ -312,15 +322,23 @@ func TestProcessServiceWithMocks(t *testing.T) { assert.Empty(t, recs) assert.Empty(t, results) } + if !tt.isDryRun { + assert.Empty(t, results, "a failed duplicate check must refuse a real purchase") + } mockClient.AssertExpectations(t) }) } + + awsStub.requireRequests(t, map[string]int{ + "rds.us-east-1.amazonaws.com DescribeReservedDBInstances": 1, + "elasticache.eu-west-1.amazonaws.com DescribeReservedCacheNodes": 1, + }) } func TestProcessService_SavingsPlansAccountLevel(t *testing.T) { ctx := context.Background() - awsCfg := aws.Config{Region: "us-east-1"} + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg defer func() { toolCfg = origCfg }() @@ -350,6 +368,7 @@ func TestProcessService_SavingsPlansAccountLevel(t *testing.T) { // Verify Savings Plans queried only once mockClient.AssertNumberOfCalls(t, "GetRecommendations", 1) mockClient.AssertExpectations(t) + awsStub.requireRequests(t, map[string]int{"savingsplans.amazonaws.com DescribeSavingsPlans": 1}) } func TestProcessService_WithInstanceLimit(t *testing.T) { @@ -364,7 +383,7 @@ func TestProcessService_WithInstanceLimit(t *testing.T) { // TestProcessService_InstanceLimitRefusesRealPurchase covers that. ctx := context.Background() - awsCfg := aws.Config{Region: "us-east-1"} + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg defer func() { toolCfg = origCfg }() @@ -403,6 +422,7 @@ func TestProcessService_WithInstanceLimit(t *testing.T) { "the per-region path must not apply --max-instances; the cap is run-wide") mockClient.AssertExpectations(t) + awsStub.requireDuplicateChecks(t, 1) } // TestProcessService_InstanceLimitRefusesRealPurchase pins the fail-closed @@ -454,7 +474,7 @@ func TestProcessService_InstanceLimitRefusesRealPurchase(t *testing.T) { func TestProcessService_WithOverrideCount(t *testing.T) { ctx := context.Background() - awsCfg := aws.Config{Region: "us-east-1"} + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg defer func() { toolCfg = origCfg }() @@ -482,11 +502,12 @@ func TestProcessService_WithOverrideCount(t *testing.T) { } mockClient.AssertExpectations(t) + awsStub.requireDuplicateChecks(t, 1) } func TestProcessService_MultipleRegions(t *testing.T) { ctx := context.Background() - awsCfg := aws.Config{Region: "us-east-1"} + awsCfg, awsStub := newAWSFixtureConfig(t) origCfg := toolCfg defer func() { toolCfg = origCfg }() @@ -518,6 +539,7 @@ func TestProcessService_MultipleRegions(t *testing.T) { // Verify each region was queried mockClient.AssertNumberOfCalls(t, "GetRecommendations", 3) mockClient.AssertExpectations(t) + awsStub.requireDuplicateChecks(t, 3) } // ==================== Helper Function Tests ==================== @@ -1627,11 +1649,12 @@ func TestFilterAndAdjustRecommendations(t *testing.T) { } // isolateAWSEnv pins the AWS environment to deterministic invalid static -// credentials so the runToolFromCSV tests behave identically on credentialed -// developer machines and bare CI runners: config loading succeeds, every -// outbound AWS API call fails and takes the warn-and-continue path, and no -// real account is ever touched. -func isolateAWSEnv(t *testing.T) { +// credentials and points every SDK client at a local server that answers 403 +// AccessDenied, so the runToolFromCSV tests behave identically on credentialed +// developer machines and bare CI runners: config loading succeeds, every AWS +// API call fails fast on the warn-and-continue path, and nothing leaves the +// machine. It returns the number of requests the local server has received. +func isolateAWSEnv(t *testing.T) *atomic.Int64 { t.Helper() t.Setenv("AWS_ACCESS_KEY_ID", "invalid-test-key-id") t.Setenv("AWS_SECRET_ACCESS_KEY", "invalid-test-secret") @@ -1640,6 +1663,26 @@ func isolateAWSEnv(t *testing.T) { t.Setenv("AWS_CONFIG_FILE", os.DevNull) t.Setenv("AWS_SHARED_CREDENTIALS_FILE", os.DevNull) t.Setenv("AWS_EC2_METADATA_DISABLED", "true") + + hits := &atomic.Int64{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + w.Header().Set("Content-Type", "application/x-amz-json-1.1") + w.Header().Set("X-Amzn-ErrorType", "AccessDeniedException") + w.WriteHeader(http.StatusForbidden) + _, _ = w.Write([]byte(`{"__type":"AccessDeniedException","message":"denied by test"}`)) + })) + t.Cleanup(srv.Close) + + // A developer's global endpoint overrides must not bypass the local server. + for _, kv := range os.Environ() { + if name, _, _ := strings.Cut(kv, "="); strings.HasPrefix(name, "AWS_ENDPOINT_URL_") { + t.Setenv(name, "") + } + } + t.Setenv("AWS_IGNORE_CONFIGURED_ENDPOINT_URLS", "") + t.Setenv("AWS_ENDPOINT_URL", srv.URL) + return hits } // readPurchaseReport parses the CSV purchase report written by runToolFromCSV @@ -1693,7 +1736,7 @@ func writeTestRecommendationsCSV(t *testing.T, csvData string) string { func TestRunToolFromCSV(t *testing.T) { origCfg := toolCfg defer func() { toolCfg = origCfg }() - isolateAWSEnv(t) + awsHits := isolateAWSEnv(t) // Column names match writeMultiServiceCSVReport's header (the real // round-trip shape). The previous fixture used headers the parser @@ -1750,6 +1793,8 @@ elasticache,us-west-2,cache.t3.micro,redis,1,1yr,all-upfront,111111111111 } }) } + // Without a request the env wiring in isolateAWSEnv would be vacuous. + assert.Positive(t, awsHits.Load(), "isolateAWSEnv's local endpoint must have served the duplicate checks") } // TestRunToolFromCSV_DuplicateCheckFailureRefusesToPurchase reproduces #1941: