diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index e84ed2e..ee69bfc 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -13,7 +13,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/concurrency" - "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" ) // maxRecommendationPages caps the number of pages fetched per Cost Explorer @@ -257,12 +256,7 @@ var defaultDiscoveryTerms = []string{"1yr", "3yr"} // rows and render as distinct UI rows for free. var defaultDiscoveryPaymentOptions = []string{"all-upfront", "partial-upfront", "no-upfront"} -// fetchSingleComboRecs fetches recommendations for one (term, payment) pair. -// If the context is already done before the call, it returns (nil, ctx.Err()). -// If GetRecommendations returns an error after ctx cancellation, it also -// returns (nil, ctx.Err()) so the caller exits the sweep immediately. Per-combo -// errors (throttle, 5xx) return (nil, err) with ctx.Err() == nil, signaling -// skip-and-continue tolerance in the outer loop. +// fetchSingleComboRecs retains incomplete responses but makes cancellation terminal. func (c *Client) fetchSingleComboRecs(ctx context.Context, service common.ServiceType, term, payment string) ([]common.Recommendation, error) { if ctx.Err() != nil { return nil, ctx.Err() @@ -279,20 +273,13 @@ func (c *Client) fetchSingleComboRecs(ctx context.Context, service common.Servic Region: "", } recs, err := c.GetRecommendations(ctx, ¶ms) - if err != nil { - // A canceled / deadline-exceeded ctx is NOT a per-combo - // failure to be tolerated -- every subsequent combo - // would just hit the same dead context and waste time - // while we accumulate "failures" that hide the real - // reason. Short-circuit so the caller sees the ctx - // error verbatim. Per-combo errors (throttle, 5xx) - // keep the existing skip-and-continue tolerance. - if ctx.Err() != nil { - return nil, ctx.Err() - } - return nil, err + if ctx.Err() != nil { + return nil, ctx.Err() } - return recs, nil + if canceled := collectionCancellation(err); canceled != nil { + return nil, canceled + } + return recs, err } // GetRecommendationsForService fetches recommendations for a specific @@ -303,31 +290,30 @@ func (c *Client) fetchSingleComboRecs(ctx context.Context, service common.Servic // params.PaymentOption so the resulting slice contains every combo // for the user to choose from in the UI. // -// A per-call Cost Explorer error is tolerated and skipped so a single -// throttle on one (term, payment) combo doesn't suppress the others; -// only an error where every combo fails is propagated. This mirrors -// the "continue on per-service error" tolerance in GetAllRecommendations. +// Failed details or scopes accompany surviving results as a typed diagnostic. +// If every API call fails, the ordinary error remains fatal. func (c *Client) GetRecommendationsForService(ctx context.Context, service common.ServiceType) ([]common.Recommendation, error) { allRecs := make([]common.Recommendation, 0) var lastErr error + incomplete := &IncompleteRecommendationsError{} successCount := 0 - attempts := 0 for _, term := range defaultDiscoveryTerms { for _, payment := range defaultDiscoveryPaymentOptions { - attempts++ recs, err := c.fetchSingleComboRecs(ctx, service, term, payment) if err != nil { - if ctx.Err() != nil { - return nil, ctx.Err() + if canceled := collectionCancellation(err); canceled != nil { + return nil, canceled } lastErr = err - continue + if !incomplete.addFailure(fmt.Errorf("service %s term %s payment %s: %w", service, term, payment, err)) { + continue + } } successCount++ allRecs = append(allRecs, recs...) } } - if successCount == 0 && attempts > 0 && lastErr != nil { + if successCount == 0 && lastErr != nil { return nil, fmt.Errorf("all (term, payment) variants failed for service %s: %w", service, lastErr) } // Enrich each rec with 7-day daily coverage history so the frontend @@ -335,8 +321,12 @@ func (c *Client) GetRecommendationsForService(ctx context.Context, service commo // are skipped inside AttachDailyUsageHistory (no per-SKU CE coverage // breakdown available). Errors are logged and skipped per-tuple so a // single CE failure doesn't suppress the rest of the collection. - if len(allRecs) > 0 { - c.AttachDailyUsageHistory(ctx, allRecs) + c.AttachDailyUsageHistory(ctx, allRecs) + if err := ctx.Err(); err != nil { + return nil, err + } + if len(incomplete.Causes) > 0 { + return allRecs, incomplete } return allRecs, nil } @@ -350,11 +340,8 @@ func (c *Client) GetRecommendationsForService(ctx context.Context, service commo // the canonical order EC2 → RDS → ElastiCache → OpenSearch → Redshift after // all goroutines finish so order-sensitive consumers stay stable. // -// Behavior change vs the previous sequential loop: per-service errors are -// now logged at WARN via mergeServiceResults — the previous loop swallowed -// them silently with a bare `continue`, leaving operators no signal when a -// single service was misbehaving. Mirrors the Azure parallelisation in -// providers/azure/recommendations.go (closes #258, commit b10326c5). +// Partial results retain diagnostics through mergeServiceResults; callers decide +// whether their workflow can use an incomplete collection. func (c *Client) GetAllRecommendations(ctx context.Context) ([]common.Recommendation, error) { var ( ec2Recs, rdsRecs, cacheRecs, osRecs, redshiftRecs []common.Recommendation @@ -436,41 +423,32 @@ type serviceResult struct { recs []common.Recommendation } -// mergeServiceResults logs per-service errors at WARN and appends successful -// results in the order the slice is passed — callers must preserve the -// canonical EC2 → RDS → ElastiCache → OpenSearch → Redshift → SavingsPlans -// order so that order-sensitive consumers stay stable. -// -// Partial failure is tolerated: as long as at least one service succeeded, the -// successful services' recommendations are returned with a nil error and the -// failures are logged at WARN. But when EVERY service errored (e.g. a sustained -// Cost Explorer throttle that exhausts each service's per-combo retries), the -// merge returns a wrapped error instead of an empty-but-nil-error result -// (08-H4). Returning (recs, nil) on a total failure makes a throttled run -// indistinguishable from "no savings available", which an operator can misread -// as "nothing to buy": the same hazard the per-service all-combos-failed guard -// in GetRecommendationsForService prevents one level down. +// mergeServiceResults preserves service order and incomplete responses. +// All ordinary service failures remain fatal, even when no rows were expected. func mergeServiceResults(results ...serviceResult) ([]common.Recommendation, error) { - total := 0 + var out []common.Recommendation + incomplete := &IncompleteRecommendationsError{} failures := 0 var lastErr error for i := range results { - total += len(results[i].recs) - if results[i].err != nil { - failures++ - lastErr = results[i].err - } - } - out := make([]common.Recommendation, 0, total) - for i := range results { - if results[i].err != nil { - logging.Warnf("AWS %s recommendations: %v", results[i].name, results[i].err) - continue + result := &results[i] + if result.err != nil { + if canceled := collectionCancellation(result.err); canceled != nil { + return nil, canceled + } + lastErr = result.err + if !incomplete.addFailure(fmt.Errorf("service %s: %w", result.name, result.err)) { + failures++ + continue + } } - out = append(out, results[i].recs...) + out = append(out, result.recs...) } if failures == len(results) && failures > 0 { return nil, fmt.Errorf("all %d AWS recommendation services failed: %w", failures, lastErr) } + if len(incomplete.Causes) > 0 { + return out, incomplete + } return out, nil } diff --git a/providers/aws/recommendations/client_test.go b/providers/aws/recommendations/client_test.go index fbb7e67..70867e7 100644 --- a/providers/aws/recommendations/client_test.go +++ b/providers/aws/recommendations/client_test.go @@ -499,7 +499,10 @@ func TestGetAllRecommendations(t *testing.T) { recs, err := client.GetAllRecommendations(context.Background()) - require.NoError(t, err) + var incomplete *IncompleteRecommendationsError + require.ErrorAs(t, err, &incomplete) + require.Equal(t, 24, incomplete.FailedDetails) + require.Zero(t, incomplete.FailedScopes) // Only EC2 will successfully parse since the mock returns EC2 details for all services // Other services will fail parsing and be skipped assert.NotEmpty(t, recs) @@ -536,8 +539,10 @@ func TestGetAllRecommendations_SomeServicesFail(t *testing.T) { recs, err := client.GetAllRecommendations(context.Background()) - // Should not error even if some services fail - require.NoError(t, err) + var incomplete *IncompleteRecommendationsError + require.ErrorAs(t, err, &incomplete) + require.Equal(t, 24, incomplete.FailedDetails) + require.Zero(t, incomplete.FailedScopes) // Should have recommendations from services that succeeded assert.NotEmpty(t, recs) } @@ -654,7 +659,10 @@ func TestGetAllRecommendations_IncludesSavingsPlans(t *testing.T) { client := NewClientWithAPI(mockAPI, "us-east-1") recs, err := client.GetAllRecommendations(context.Background()) - require.NoError(t, err) + var incomplete *IncompleteRecommendationsError + require.ErrorAs(t, err, &incomplete) + require.Equal(t, 24, incomplete.FailedDetails) + require.Zero(t, incomplete.FailedScopes) // At least one EC2 rec must be present (existing services not regressed). var ec2Count, spCount int @@ -925,7 +933,7 @@ func TestMergeServiceResults_AllFailIsError(t *testing.T) { assert.Contains(t, err.Error(), "all", "error should signal that every service failed") assert.Nil(t, recs) - // Partial failure is still tolerated: surviving service's recs returned, nil error. + // Partial failure retains the surviving service with an incomplete diagnostic. ec2Rec := common.Recommendation{Service: common.ServiceEC2} recs, err = mergeServiceResults( serviceResult{name: "EC2", recs: []common.Recommendation{ec2Rec}}, @@ -935,7 +943,9 @@ func TestMergeServiceResults_AllFailIsError(t *testing.T) { serviceResult{name: "Redshift", err: throttle}, serviceResult{name: "SavingsPlans", err: throttle}, ) - require.NoError(t, err, "a single surviving service must keep the run successful") + var incomplete *IncompleteRecommendationsError + require.ErrorAs(t, err, &incomplete) + assert.Equal(t, 5, incomplete.FailedScopes) assert.Len(t, recs, 1) assert.Equal(t, common.ServiceEC2, recs[0].Service) diff --git a/providers/aws/recommendations/collection_error.go b/providers/aws/recommendations/collection_error.go new file mode 100644 index 0000000..5669884 --- /dev/null +++ b/providers/aws/recommendations/collection_error.go @@ -0,0 +1,46 @@ +package recommendations + +import ( + "context" + "errors" + "fmt" +) + +// IncompleteRecommendationsError accompanies survivors of an incomplete collection. +// Each cause represents one rejected RI detail or one failed collection scope. +type IncompleteRecommendationsError struct { + FailedDetails int + FailedScopes int + Causes []error +} + +func (e *IncompleteRecommendationsError) Error() string { + return fmt.Sprintf("incomplete AWS recommendations: %d failed details, %d failed scopes: %v", + e.FailedDetails, e.FailedScopes, errors.Join(e.Causes...)) +} + +func (e *IncompleteRecommendationsError) Unwrap() []error { return e.Causes } + +// addFailure returns whether the failed scope received a usable response. +func (e *IncompleteRecommendationsError) addFailure(err error) bool { + var partial *IncompleteRecommendationsError + if errors.As(err, &partial) { + e.FailedDetails += partial.FailedDetails + e.FailedScopes += partial.FailedScopes + e.Causes = append(e.Causes, partial.Causes...) + return true + } + e.FailedScopes++ + e.Causes = append(e.Causes, err) + return false +} + +func collectionCancellation(err error) error { + if errors.Is(err, context.Canceled) { + return context.Canceled + } + if errors.Is(err, context.DeadlineExceeded) { + return context.DeadlineExceeded + } + return nil +} diff --git a/providers/aws/recommendations/collection_error_test.go b/providers/aws/recommendations/collection_error_test.go new file mode 100644 index 0000000..9da9e88 --- /dev/null +++ b/providers/aws/recommendations/collection_error_test.go @@ -0,0 +1,168 @@ +package recommendations + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/aws/aws-sdk-go-v2/service/costexplorer" + "github.com/aws/aws-sdk-go-v2/service/costexplorer/types" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" +) + +type incompleteCollectionAPI struct { + mockCostExplorerAPI + request func(*costexplorer.GetReservationPurchaseRecommendationInput) (*costexplorer.GetReservationPurchaseRecommendationOutput, error) + coverage func() +} + +func (m *incompleteCollectionAPI) GetReservationPurchaseRecommendation(_ context.Context, in *costexplorer.GetReservationPurchaseRecommendationInput, _ ...func(*costexplorer.Options)) (*costexplorer.GetReservationPurchaseRecommendationOutput, error) { + return m.request(in) +} + +func (m *incompleteCollectionAPI) GetReservationCoverage(_ context.Context, _ *costexplorer.GetReservationCoverageInput, _ ...func(*costexplorer.Options)) (*costexplorer.GetReservationCoverageOutput, error) { + if m.coverage != nil { + m.coverage() + } + return &costexplorer.GetReservationCoverageOutput{}, nil +} + +func incompleteRIDetails(valid bool, bad int) []types.ReservationPurchaseRecommendation { + var blocks []types.ReservationPurchaseRecommendation + if valid { + blocks = append(blocks, types.ReservationPurchaseRecommendation{RecommendationDetails: []types.ReservationPurchaseRecommendationDetail{{ + RecommendedNumberOfInstancesToPurchase: aws.String("2"), EstimatedMonthlyOnDemandCost: aws.String("30"), + InstanceDetails: &types.InstanceDetails{RDSInstanceDetails: &types.RDSInstanceDetails{InstanceType: aws.String("db.t3.medium"), Region: aws.String("us-east-1")}}, + }}}) + } + for range bad { + blocks = append(blocks, types.ReservationPurchaseRecommendation{RecommendationDetails: []types.ReservationPurchaseRecommendationDetail{{RecommendedNumberOfInstancesToPurchase: aws.String("bad")}}}) + } + return blocks +} + +func TestCollectionCompleteness_Combos(t *testing.T) { + apiFailure := errors.New("access denied") + for _, tc := range []struct { + name string + valid bool + bad, failed, rows, details, scopes int + fatal bool + }{ + {name: "valid", valid: true, rows: 6}, + {name: "empty"}, + {name: "mixed details", valid: true, bad: 2, rows: 6, details: 12}, + {name: "all invalid", bad: 2, details: 12}, + {name: "empty and API failure", failed: 1, scopes: 1}, + {name: "valid and API failure", valid: true, failed: 1, rows: 5, scopes: 1}, + {name: "invalid and API failure", bad: 2, failed: 5, details: 2, scopes: 5}, + {name: "total API failure", failed: 6, fatal: true}, + } { + t.Run(tc.name, func(t *testing.T) { + calls := 0 + api := &incompleteCollectionAPI{request: func(*costexplorer.GetReservationPurchaseRecommendationInput) (*costexplorer.GetReservationPurchaseRecommendationOutput, error) { + calls++ + if calls <= tc.failed { + return nil, apiFailure + } + return &costexplorer.GetReservationPurchaseRecommendationOutput{Recommendations: incompleteRIDetails(tc.valid, tc.bad)}, nil + }} + recs, err := NewClientWithAPI(api, "us-east-1").GetRecommendationsForService(context.Background(), common.ServiceRDS) + require.Equal(t, 6, calls) + require.Len(t, recs, tc.rows) + var incomplete *IncompleteRecommendationsError + if tc.fatal { + require.ErrorIs(t, err, apiFailure) + require.False(t, errors.As(err, &incomplete)) + return + } + if tc.details+tc.scopes == 0 { + require.NoError(t, err) + return + } + require.ErrorAs(t, err, &incomplete) + require.Equal(t, tc.details, incomplete.FailedDetails) + require.Equal(t, tc.scopes, incomplete.FailedScopes) + require.Len(t, incomplete.Causes, tc.details+tc.scopes) + for _, cause := range incomplete.Causes { + require.Contains(t, cause.Error(), "service rds term ") + require.Contains(t, cause.Error(), " payment ") + } + if tc.bad > 0 { + block := 0 + if tc.valid { + block = 1 + } + require.Contains(t, err.Error(), fmt.Sprintf("block %d detail 0", block)) + require.Contains(t, err.Error(), fmt.Sprintf("block %d detail 0", block+1)) + } + if tc.failed > 0 { + require.ErrorIs(t, err, apiFailure) + } + }) + } +} + +func TestCollectionCompleteness_NestedServices(t *testing.T) { + detailFailure := errors.New("bad quantity") + apiFailure := errors.New("access denied") + partial := &IncompleteRecommendationsError{FailedDetails: 1, FailedScopes: 1, Causes: []error{detailFailure, apiFailure}} + recs, err := mergeServiceResults( + serviceResult{name: "RDS", recs: []common.Recommendation{{Count: 2}}, err: fmt.Errorf("wrapped: %w", partial)}, + serviceResult{name: "EC2", err: apiFailure}, + ) + outerRecs, outerErr := mergeServiceResults(serviceResult{name: "nested", recs: recs, err: err}, serviceResult{name: "empty"}) + var incomplete *IncompleteRecommendationsError + require.ErrorAs(t, outerErr, &incomplete) + require.Len(t, outerRecs, 1) + require.Equal(t, 1, incomplete.FailedDetails) + require.Equal(t, 2, incomplete.FailedScopes) + require.Len(t, incomplete.Causes, 3) + require.ErrorIs(t, outerErr, detailFailure) + require.ErrorIs(t, outerErr, apiFailure) + for _, cause := range incomplete.Causes { + var nested *IncompleteRecommendationsError + require.False(t, errors.As(cause, &nested)) + } + _, emptyErr := mergeServiceResults(serviceResult{name: "invalid", err: partial}) + require.ErrorAs(t, emptyErr, &incomplete) + require.Equal(t, 1, incomplete.FailedDetails) +} + +func TestCollectionCompleteness_Cancellation(t *testing.T) { + for _, terminal := range []error{context.Canceled, context.DeadlineExceeded} { + t.Run(terminal.Error(), func(t *testing.T) { + calls := 0 + api := &incompleteCollectionAPI{request: func(*costexplorer.GetReservationPurchaseRecommendationInput) (*costexplorer.GetReservationPurchaseRecommendationOutput, error) { + calls++ + return nil, fmt.Errorf("SDK: %w", terminal) + }} + recs, err := NewClientWithAPI(api, "us-east-1").GetRecommendationsForService(context.Background(), common.ServiceRDS) + require.ErrorIs(t, err, terminal) + require.Nil(t, recs) + require.Equal(t, 1, calls) + recs, err = mergeServiceResults(serviceResult{name: "valid", recs: []common.Recommendation{{Count: 1}}}, serviceResult{name: "canceled", err: terminal}) + require.ErrorIs(t, err, terminal) + require.Nil(t, recs) + var incomplete *IncompleteRecommendationsError + require.False(t, errors.As(err, &incomplete)) + }) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + coverageCalls := 0 + api := &incompleteCollectionAPI{ + request: func(*costexplorer.GetReservationPurchaseRecommendationInput) (*costexplorer.GetReservationPurchaseRecommendationOutput, error) { + return &costexplorer.GetReservationPurchaseRecommendationOutput{Recommendations: incompleteRIDetails(true, 1)}, nil + }, + coverage: func() { coverageCalls++; cancel() }, + } + recs, err := NewClientWithAPI(api, "us-east-1").GetRecommendationsForService(ctx, common.ServiceRDS) + require.Positive(t, coverageCalls) + require.ErrorIs(t, err, context.Canceled) + require.Nil(t, recs) +} diff --git a/providers/aws/recommendations/parser_ri.go b/providers/aws/recommendations/parser_ri.go index 71b1861..ccb0e29 100644 --- a/providers/aws/recommendations/parser_ri.go +++ b/providers/aws/recommendations/parser_ri.go @@ -17,18 +17,23 @@ import ( // parseRecommendations converts AWS recommendations to common.Recommendation format. func (c *Client) parseRecommendations(ctx context.Context, awsRecs []types.ReservationPurchaseRecommendation, params common.RecommendationParams) ([]common.Recommendation, error) { var recommendations []common.Recommendation + incomplete := &IncompleteRecommendationsError{} for r := range awsRecs { awsRec := &awsRecs[r] for i := range awsRec.RecommendationDetails { + if err := ctx.Err(); err != nil { + return nil, err + } details := &awsRec.RecommendationDetails[i] rec, err := c.parseRecommendationDetail(ctx, details, params) if err != nil { - // log (stderr), never fmt.Print (stdout): this package is - // linked into cmd/cudly-mcp, whose stdio transport owns - // stdout for JSON-RPC framing. A warning printed here during - // a cudly_search_recommendations call would be interleaved - // into the protocol stream and corrupt the session. + if canceled := collectionCancellation(err); canceled != nil { + return nil, canceled + } + incomplete.FailedDetails++ + incomplete.Causes = append(incomplete.Causes, fmt.Errorf("service %s term %s payment %s block %d detail %d: %w", + params.Service, params.Term, params.PaymentOption, r, i, err)) log.Printf("Warning: Failed to parse recommendation detail %d: %v", i, err) continue } @@ -39,6 +44,12 @@ func (c *Client) parseRecommendations(ctx context.Context, awsRecs []types.Reser } } + if err := ctx.Err(); err != nil { + return nil, err + } + if incomplete.FailedDetails > 0 { + return recommendations, incomplete + } return recommendations, nil } diff --git a/providers/aws/recommendations/parser_ri_test.go b/providers/aws/recommendations/parser_ri_test.go index c54d164..146d6cc 100644 --- a/providers/aws/recommendations/parser_ri_test.go +++ b/providers/aws/recommendations/parser_ri_test.go @@ -443,7 +443,7 @@ func TestParseRecommendations(t *testing.T) { assert.Equal(t, "us-west-2", recs[1].Region) } -func TestParseRecommendations_SkipsInvalidDetails(t *testing.T) { +func TestParseRecommendations_ReportsInvalidDetails(t *testing.T) { client := &Client{} awsRecs := []types.ReservationPurchaseRecommendation{ @@ -498,8 +498,9 @@ func TestParseRecommendations_SkipsInvalidDetails(t *testing.T) { recs, err := client.parseRecommendations(context.Background(), awsRecs, params) - require.NoError(t, err) - // Should have 2 valid recommendations, skipping the invalid one + var incomplete *IncompleteRecommendationsError + require.ErrorAs(t, err, &incomplete) + assert.Equal(t, 1, incomplete.FailedDetails) assert.Len(t, recs, 2) assert.Equal(t, "t3.medium", recs[0].ResourceType) assert.Equal(t, "r5.xlarge", recs[1].ResourceType) @@ -611,7 +612,7 @@ func TestParseRIUtilizationSignals(t *testing.T) { // during a cudly_search_recommendations call injected a bare line of prose // into the middle of the JSON-RPC stream and broke the client session. // -// TestParseRecommendations_SkipsInvalidDetails above already drives this exact +// TestParseRecommendations_ReportsInvalidDetails above already drives this exact // code path, but it only asserts the returned recommendation count -- it stayed // green the entire time the bug was live. This test asserts the property that // actually matters: nothing reaches stdout, whatever is logged. @@ -645,7 +646,7 @@ func TestParseRecommendations_WarningsNeverGoToStdout(t *testing.T) { stdout, logged := captureStdoutAndLog(t, func() { recs, err := client.parseRecommendations(context.Background(), awsRecs, params) - require.NoError(t, err) + require.Error(t, err) assert.Empty(t, recs, "the single invalid detail must be skipped") }) diff --git a/providers/aws/service_client.go b/providers/aws/service_client.go index 33a91ef..a93cd60 100644 --- a/providers/aws/service_client.go +++ b/providers/aws/service_client.go @@ -3,6 +3,7 @@ package aws import ( "context" + "errors" "fmt" "github.com/aws/aws-sdk-go-v2/aws" @@ -77,12 +78,13 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p return nil, fmt.Errorf("params cannot be nil") } recs, err := r.client.GetRecommendations(ctx, params) - if err != nil { + var incomplete *recommendations.IncompleteRecommendationsError + if err != nil && !errors.As(err, &incomplete) { return nil, err } recs = applyRecommendationFilters(recs, *params) - return recs, nil + return recs, err } // applyRecommendationFilters applies account and region filters to recommendations. diff --git a/providers/aws/service_client_completeness_test.go b/providers/aws/service_client_completeness_test.go new file mode 100644 index 0000000..ac05112 --- /dev/null +++ b/providers/aws/service_client_completeness_test.go @@ -0,0 +1,55 @@ +package aws + +import ( + "context" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/providers/aws/recommendations" +) + +type recommendationHTTPFixture func(*http.Request) (*http.Response, error) + +func (f recommendationHTTPFixture) Do(req *http.Request) (*http.Response, error) { return f(req) } + +func TestRecommendationsClientAdapter_IncompleteSDKResponse(t *testing.T) { + calls := 0 + client := NewRecommendationsClient(aws.Config{ + Region: "us-east-1", Credentials: aws.CredentialsProviderFunc(func(context.Context) (aws.Credentials, error) { + return aws.Credentials{AccessKeyID: "synthetic", SecretAccessKey: "synthetic"}, nil + }), + HTTPClient: recommendationHTTPFixture(func(req *http.Request) (*http.Response, error) { + if req.Header.Get("X-Amz-Target") != "AWSInsightsIndexService.GetReservationPurchaseRecommendation" { + return nil, fmt.Errorf("unexpected SDK request: %s", req.Header.Get("X-Amz-Target")) + } + calls++ + body := `{"Recommendations":[{"RecommendationDetails":[ + {"RecommendedNumberOfInstancesToPurchase":"2","EstimatedMonthlyOnDemandCost":"30","AccountId":"included","InstanceDetails":{"RDSInstanceDetails":{"InstanceType":"db.t3.medium","Region":"us-east-1"}}}, + {"RecommendedNumberOfInstancesToPurchase":"1","EstimatedMonthlyOnDemandCost":"30","AccountId":"excluded","InstanceDetails":{"RDSInstanceDetails":{"InstanceType":"db.t3.medium","Region":"us-east-1"}}}, + {"RecommendedNumberOfInstancesToPurchase":"1","EstimatedMonthlyOnDemandCost":"30","AccountId":"included","InstanceDetails":{"RDSInstanceDetails":{"InstanceType":"db.t3.medium","Region":"eu-west-1"}}}, + {"RecommendedNumberOfInstancesToPurchase":"not-a-number"}]}]}` + return &http.Response{StatusCode: http.StatusOK, Header: http.Header{"Content-Type": {"application/x-amz-json-1.1"}}, Body: io.NopCloser(strings.NewReader(body)), Request: req}, nil + }), + }) + recs, err := client.GetRecommendations(context.Background(), &common.RecommendationParams{ + Service: common.ServiceRDS, Term: "1yr", PaymentOption: "no-upfront", LookbackPeriod: "7d", + Region: "us-east-1", AccountFilter: []string{"included"}, + }) + require.Error(t, err, "malformed detail must not produce a successful short menu") + var incomplete *recommendations.IncompleteRecommendationsError + require.ErrorAs(t, err, &incomplete) + require.Equal(t, 1, incomplete.FailedDetails) + require.Zero(t, incomplete.FailedScopes) + require.Len(t, incomplete.Causes, 1) + require.Contains(t, incomplete.Error(), "service rds term 1yr payment no-upfront block 0 detail 3") + require.Len(t, recs, 1, "incomplete survivors must still receive account and region filters") + require.Equal(t, 2, recs[0].Count) + require.Equal(t, 1, calls) +}