diff --git a/internal/api/handler_history.go b/internal/api/handler_history.go index 13c674fb0..72c06674a 100644 --- a/internal/api/handler_history.go +++ b/internal/api/handler_history.go @@ -412,14 +412,25 @@ func executionToHistoryRow(exec config.PurchaseExecution, approver, createdByEma // fixing the config makes Retry work). Same call and input as the // retry handler's gate so the two cannot disagree (issue #161). if reason := purchase.RedriveRefusalReason(&exec); reason != "" { - row.OpsHint = reason row.RedriveUnsafe = true + // Only a proven pre-send AWS refusal on every rec means nothing was + // bought; for any other outcome the duplicate-purchase warning stays. + // ops_hint stays set either way: the History UI hides Retry on it. + if purchase.IsDefiniteAWSRefusal(exec.Recommendations, exec.Error) { + row.OpsHint = definiteRefusalOpsHint + } else { + row.OpsHint = reason + } } } annotateHistoryRowByStatus(&row, exec, approver) return row } +// definiteRefusalOpsHint replaces the duplicate-purchase warning when AWS +// refused the purchase before making it. +const definiteRefusalOpsHint = "AWS refused this purchase before it was made; start a new purchase after fixing the cause" + // annotateHistoryRowByStatus fills in Approver + StatusDescription on the // row based on exec.Status. Split from executionToHistoryRow to keep the // switch below under the gocyclo threshold. @@ -428,7 +439,7 @@ func annotateHistoryRowByStatus(row *config.PurchaseHistoryRecord, exec config.P case "pending", "notified": row.Approver = approver case "failed": - row.StatusDescription = exec.Error + row.StatusDescription = purchase.SanitizeStoredError(exec.Error) case "expired": row.StatusDescription = "approval link expired (not approved within 7 days)" case config.StatusCanceled, config.LegacyStatusCanceled: @@ -482,7 +493,7 @@ func annotateInFlightOrAuditGapRow(row *config.PurchaseHistoryRecord, exec confi // double-count guard as the audit-gap completed case below. row.IsAuditGap = true annotateApproved(row, exec, approver) - row.StatusDescription = "partially completed — some commitments succeeded, others failed: " + exec.Error + row.StatusDescription = "partially completed — some commitments succeeded, others failed: " + purchase.SanitizeStoredError(exec.Error) case "completed": // Only audit-gap completed executions reach here (fetchExecutionsAsHistory // skips clean completed rows). exec.Error carries why the history write @@ -494,7 +505,7 @@ func annotateInFlightOrAuditGapRow(row *config.PurchaseHistoryRecord, exec confi row.IsAuditGap = true annotateApproved(row, exec, approver) if exec.Error != "" { - row.StatusDescription = "purchase completed but its history record could not be saved: " + exec.Error + row.StatusDescription = "purchase completed but its history record could not be saved: " + purchase.SanitizeStoredError(exec.Error) } } } diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index 764f38756..0fbe3328d 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -527,18 +527,40 @@ func executeFailureError(executionID, action string, final *config.PurchaseExecu } switch { case errors.Is(err, config.ErrAuditLoss): - return NewClientError(500, fmt.Sprintf("execution %s ran but its final status could not be saved: %v", executionID, err)) + logRawExecuteFailure(executionID, err) + return NewClientError(500, fmt.Sprintf("execution %s ran but its final status could not be saved (reference: execution %s)", executionID, executionID)) case errors.Is(err, purchase.ErrClaimedRowRefused): return claimedRowRefusalError(final, err) case errors.Is(err, purchase.ErrStaleAzurePricing): - return NewClientError(409, err.Error()) + return NewClientError(409, purchase.ErrStaleAzurePricing.Error()) case purchase.IsCommittedPartialRun(final, err): - return NewClientErrorWithDetails(502, fmt.Sprintf("execution %s partially completed: some commitments were purchased and can be revoked, others failed: %v", executionID, err), - map[string]any{"execution_id": executionID, "status": final.Status}) + category, msg := summarizeExecuteFailure(executionID, err) + return NewClientErrorWithDetails(502, fmt.Sprintf("execution %s partially completed: some commitments were purchased and can be revoked, others failed: %s (reference: execution %s)", executionID, msg, executionID), + map[string]any{"execution_id": executionID, "status": final.Status, "category": category}) default: - return NewClientErrorWithDetails(502, fmt.Sprintf("execution %s failed: %v", executionID, err), - map[string]any{"execution_id": executionID, "status": final.Status}) + category, msg := summarizeExecuteFailure(executionID, err) + return NewClientErrorWithDetails(502, fmt.Sprintf("execution %s failed: %s (reference: execution %s)", executionID, msg, executionID), + map[string]any{"execution_id": executionID, "status": final.Status, "category": category}) + } +} + +// logRawExecuteFailure keeps the provider's own error text in the server log +// only; responses carry a category message and the execution ID. +func logRawExecuteFailure(executionID string, err error) { + logging.Errorf("purchase[%s]: execution failed: %v", executionID, err) +} + +func summarizeExecuteFailure(executionID string, err error) (category, message string) { + logRawExecuteFailure(executionID, err) + category, message = purchase.SummarizeProviderError(err) + if category == purchase.ErrCategoryUnknown { + // Only known server-authored refusals (for example "re-fetch the + // recommendation") stay readable; any other text is the category message. + if text, ok := purchase.ServerNoteMessage(err); ok { + message = text + } } + return category, message } // unclaimedExecuteError answers a failure where the funnel returned no row: @@ -1059,7 +1081,7 @@ func tokenActionError(err error) error { case errors.Is(err, purchase.ErrClaimedRowRefused): return claimedRowRefusalError(nil, err) case errors.Is(err, purchase.ErrStaleAzurePricing): - return NewClientError(409, err.Error()) + return NewClientError(409, purchase.ErrStaleAzurePricing.Error()) case errors.Is(err, purchase.ErrInvalidApprovalToken), errors.Is(err, config.ErrApprovalTokenChanged): return NewClientError(403, "invalid approval token") case errors.Is(err, purchase.ErrApprovalTokenExpired): @@ -2638,7 +2660,7 @@ func checkRetryEligibilityGates(failedExec *config.PurchaseExecution, req *event if hint := resolveOpsHint(failedExec.Error); hint != "" { return NewClientErrorWithDetails(409, "this failure is operator-fixable; retrying without changing configuration will fail again", - map[string]any{"ops_hint": hint, "failure_reason": failedExec.Error}) + map[string]any{"ops_hint": hint, "failure_reason": purchase.SanitizeStoredError(failedExec.Error)}) } // Threshold soft-block (Q2). force=true (set by the frontend @@ -3076,6 +3098,17 @@ func (h *Handler) tryGetSession(ctx context.Context, req *events.LambdaFunctionU return session } +// sanitizedRecommendationErrors copies recs with provider text in Error +// replaced, leaving the stored slice untouched. +func sanitizedRecommendationErrors(recs []config.RecommendationRecord) []config.RecommendationRecord { + out := make([]config.RecommendationRecord, len(recs)) + copy(out, recs) + for i := range out { + out[i].Error = purchase.SanitizeStoredError(out[i].Error) + } + return out +} + // buildPurchaseDetailsResponse builds the response map for a purchase execution. func buildPurchaseDetailsResponse(execution *config.PurchaseExecution, planName string) map[string]any { response := map[string]any{ @@ -3087,7 +3120,7 @@ func buildPurchaseDetailsResponse(execution *config.PurchaseExecution, planName "scheduled_date": execution.ScheduledDate.Format("2006-01-02"), "total_upfront_cost": execution.TotalUpfrontCost, "estimated_savings": execution.EstimatedSavings, - "recommendations": execution.Recommendations, + "recommendations": sanitizedRecommendationErrors(execution.Recommendations), } if execution.NotificationSent != nil { @@ -3097,7 +3130,7 @@ func buildPurchaseDetailsResponse(execution *config.PurchaseExecution, planName response["completed_at"] = execution.CompletedAt.Format("2006-01-02T15:04:05Z") } if execution.Error != "" { - response["error"] = execution.Error + response["error"] = purchase.SanitizeStoredError(execution.Error) } return response } diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index 51220cb5a..9faa713b9 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -2771,7 +2771,7 @@ func TestHandler_getPurchaseDetails_WithTimestamps(t *testing.T) { ScheduledDate: scheduledDate, NotificationSent: ¬ificationSent, CompletedAt: &completedAt, - Error: "some error", + Error: "ramp not advanced: some error", } mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) @@ -2793,7 +2793,7 @@ func TestHandler_getPurchaseDetails_WithTimestamps(t *testing.T) { assert.Equal(t, "completed", resultMap["status"]) assert.NotNil(t, resultMap["notification_sent"]) assert.NotNil(t, resultMap["completed_at"]) - assert.Equal(t, "some error", resultMap["error"]) + assert.Equal(t, "ramp not advanced", resultMap["error"]) } // Tests for executePurchase diff --git a/internal/api/history_duplicate_hint_test.go b/internal/api/history_duplicate_hint_test.go new file mode 100644 index 000000000..d145463f4 --- /dev/null +++ b/internal/api/history_duplicate_hint_test.go @@ -0,0 +1,53 @@ +package api + +import ( + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" +) + +func TestHistoryRow_DuplicateGuardHint(t *testing.T) { + ec2 := []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}} + denied := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: fakeAWSDenied, Recommendations: ec2}, "", "") + assert.False(t, strings.Contains(denied.OpsHint, "duplicate guard"), "definite refusal hides the warning") + unknown := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: "purchase outcome unknown", Recommendations: ec2}, "", "") + assert.Contains(t, unknown.OpsHint, "duplicate guard") + assert.True(t, unknown.RedriveUnsafe) + + // One refused rec does not prove that another, whose outcome is unknown, bought nothing. + mixed := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: "t4g.nano: purchase outcome unknown; t4g.micro: " + fakeAWSDenied, Recommendations: ec2}, "", "") + assert.Contains(t, mixed.OpsHint, "duplicate guard", "a mixed failure keeps the warning") + allDenied := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: "t4g.nano: " + fakeAWSDenied + "; t4g.micro: " + fakeAWSDenied, Recommendations: ec2}, "", "") + assert.NotContains(t, allDenied.OpsHint, "duplicate guard", "every rec refused hides the warning") +} + +func TestHistoryRow_DuplicateHintKeptOutsideDefiniteAWSRefusals(t *testing.T) { + const gcpRate = "commitment creation failed: googleapi: Error 403: Rate Limit Exceeded, rateLimitExceeded" + const gcpNotFound = "commitment creation failed: googleapi: Error 404: operations/op-1 was not found, notFound" + azure500 := azureHTTPError(t, 500, "/providers/Microsoft.Capacity/reservationOrders/x").Error() + azure429 := "PUT https://management.azure.com/x\nRESPONSE 429: 429 Too Many\nERROR CODE: TooManyRequests" + aws503 := strings.Replace(fakeAWSDenied, "StatusCode: 403", "StatusCode: 503", 1) + ec2 := config.RecommendationRecord{Provider: "aws", Service: "ec2"} + azSP := config.RecommendationRecord{Provider: "azure", Service: "savingsplans"} + gcp := config.RecommendationRecord{Provider: "gcp", Service: "compute"} + cases := []struct { + name string + recs []config.RecommendationRecord + err string + }{ + {"gcp rate limit", []config.RecommendationRecord{gcp, ec2}, gcpRate}, + {"gcp not found", []config.RecommendationRecord{gcp, ec2}, gcpNotFound}, + {"azure 500 on a capacity url", []config.RecommendationRecord{azSP}, azure500}, + {"azure 429", []config.RecommendationRecord{azSP}, azure429}, + {"aws 5xx", []config.RecommendationRecord{ec2}, aws503}, + {"aws without a code", []config.RecommendationRecord{ec2}, "operation error EC2: X, https response error StatusCode: 403"}, + {"non-aws rec beside a definite aws refusal", []config.RecommendationRecord{ec2, gcp}, fakeAWSDenied}, + } + for _, c := range cases { + row := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: c.err, Recommendations: c.recs}, "", "") + assert.True(t, row.RedriveUnsafe, c.name) + assert.Contains(t, row.OpsHint, "duplicate guard", c.name) + } +} diff --git a/internal/api/purchase_failure_notes_test.go b/internal/api/purchase_failure_notes_test.go new file mode 100644 index 000000000..38d0d83e7 --- /dev/null +++ b/internal/api/purchase_failure_notes_test.go @@ -0,0 +1,96 @@ +package api + +import ( + "encoding/json" + "errors" + "fmt" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/purchase" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const fakeConnectErr = "failed to connect to `host=db.internal.example user=cudly_app database=cudly`: " + + "dial error (dial tcp 10.0.0.5:5432: connect: connection refused)" + +func assertNoConnectText(t *testing.T, where, s string) { + t.Helper() + for _, n := range []string{"db.internal.example", "cudly_app", "10.0.0.5", "dial tcp"} { + assert.NotContains(t, s, n, where) + } +} + +func TestTailedNotesWithStoreErrors_NeverReachUserSurfaces(t *testing.T) { + notes := []string{ + "ramp not advanced: " + fakeConnectErr, + "refusing to execute: ladder tranche lookup failed: " + fakeConnectErr, + "refusing to execute ladder purchase: global config unreadable: " + fakeConnectErr, + purchase.ErrStaleAzurePricing.Error() + ": cannot read the latest collection state to confirm Azure prices are current: " + fakeConnectErr, + } + for _, note := range notes { + name := note[:strings.Index(note, ":")] + t.Run(name, func(t *testing.T) { + row := executionToHistoryRow(config.PurchaseExecution{ExecutionID: "e-1", Status: "failed", Error: note}, "", "") + assertNoConnectText(t, "status_description", row.StatusDescription) + assert.NotEmpty(t, row.StatusDescription) + + exec := &config.PurchaseExecution{ExecutionID: "e-1", Status: "failed", Error: note} + body, err := json.Marshal(buildPurchaseDetailsResponse(exec, "plan")) + require.NoError(t, err) + assertNoConnectText(t, "details", string(body)) + + creator := retryCallerID + failed := &config.PurchaseExecution{ExecutionID: retryExecID, Status: "failed", CreatedByUserID: &creator, + Error: "FROM_EMAIL not configured for this deployment; " + note} + handler, _, _ := buildSessionRetryHandler(t, failed, &Session{UserID: retryCallerID}, false, true) + _, rErr := handler.retryPurchase(t.Context(), sessionRetryReq(), retryExecID) + ce, ok := IsClientError(rErr) + require.True(t, ok) + assert.Equal(t, 409, ce.code) + raw, mErr := json.Marshal(ce.Details()) + require.NoError(t, mErr) + assertNoConnectText(t, "retry 409", string(raw)+ce.Error()) + assert.NotEmpty(t, ce.Details()["failure_reason"]) + }) + } +} + +func TestExecuteFailure_SentinelWrappingProviderErrorShowsOnlySentinelText(t *testing.T) { + for _, sentinel := range []error{purchase.ErrStaleAzurePricing, purchase.ErrClaimedRowRefused} { + err := fmt.Errorf("%w: %w", sentinel, errors.New(fakeConnectErr)) + _, msg := summarizeExecuteFailure("e-1", err) + assert.Equal(t, sentinel.Error(), msg) + out := executeFailureError("e-1", "executed", &config.PurchaseExecution{Status: "failed"}, err) + require.Error(t, out) + // Non-client errors render as a generic 500 (handler.go), so only + // structured client errors can carry text to the caller. + if ce, ok := IsClientError(out); ok { + assertNoConnectText(t, "live", ce.Error()) + } + } +} + +func TestTokenActionError_StalePricingShowsOnlySentinelText(t *testing.T) { + err := tokenActionError(fmt.Errorf("%w: %w", purchase.ErrStaleAzurePricing, errors.New(fakeConnectErr))) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, purchase.ErrStaleAzurePricing.Error(), ce.Error()) +} + +// A refused AWS purchase keeps ops_hint (the History UI hides Retry on it) but +// carries no duplicate-purchase warning. +func TestHistoryRow_DefiniteAWSRefusalKeepsOpsHintWithoutDuplicateWarning(t *testing.T) { + denied := "operation error EC2: X, https response error StatusCode: 403, RequestID: r, api error UnauthorizedOperation: msg" + row := executionToHistoryRow(config.PurchaseExecution{ + ExecutionID: "e-1", Status: "failed", Error: denied, + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, "", "") + assert.True(t, row.RedriveUnsafe) + assert.NotEmpty(t, row.OpsHint) + assert.NotContains(t, strings.ToLower(row.OpsHint), "duplicate guard") + assert.NotEqual(t, purchase.RedriveRefusalReason(&config.PurchaseExecution{ + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}}), row.OpsHint) +} diff --git a/internal/api/purchase_failure_summary_test.go b/internal/api/purchase_failure_summary_test.go new file mode 100644 index 000000000..666466f1e --- /dev/null +++ b/internal/api/purchase_failure_summary_test.go @@ -0,0 +1,165 @@ +package api + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + fakeEncodedBlob = "AbCdEfGhIjKlMnOpQrStUvWxYz0123456789LongEncodedBlob" + fakeAWSDenied = "operation error EC2: PurchaseReservedInstancesOffering, https response error StatusCode: 403, " + + "RequestID: 11111111-2222-3333-4444-555555555555, api error UnauthorizedOperation: You are not authorized. " + + "User: arn:aws:sts::123456789012:assumed-role/cudly-role/session. " + + "Encoded authorization failure message: " + fakeEncodedBlob +) + +var providerTextNeedles = []string{"arn:aws", "123456789012", "Encoded authorization", fakeEncodedBlob, "RequestID"} + +func assertNoProviderText(t *testing.T, where, s string) { + t.Helper() + for _, n := range providerTextNeedles { + assert.NotContains(t, s, n, where) + } +} + +// One realistic provider denial must stay out of every user-facing surface. +func TestProviderDenial_NeverReachesUserSurfaces(t *testing.T) { + const execID = "11111111-1111-1111-1111-111111111111" + raw := errors.New(fakeAWSDenied) + rec := config.RecommendationRecord{Provider: "aws", Service: "rds", Error: fakeAWSDenied} + + t.Run("direct execute 502", func(t *testing.T) { + err := executeFailureError(execID, "executed", &config.PurchaseExecution{Status: "failed"}, raw) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 502, ce.code) + body, mErr := json.Marshal(ce.Details()) + require.NoError(t, mErr) + assertNoProviderText(t, "message", ce.Error()) + assertNoProviderText(t, "details", string(body)) + assert.Contains(t, ce.Error(), execID) + assert.Equal(t, "access_denied", ce.Details()["category"]) + }) + + t.Run("history failed row", func(t *testing.T) { + row := executionToHistoryRow(config.PurchaseExecution{ + ExecutionID: execID, Status: "failed", Error: "IAM denied: " + fakeAWSDenied, + }, "", "") + assertNoProviderText(t, "status_description", row.StatusDescription) + assertNoProviderText(t, "ops_hint", row.OpsHint) + assert.Equal(t, "Grant the deploy role missing IAM permission, then retry", row.OpsHint, + "the hint still resolves from the raw text") + }) + + t.Run("partially completed and audit-gap rows", func(t *testing.T) { + for _, status := range []string{"partially_completed", "completed"} { + row := executionToHistoryRow(config.PurchaseExecution{ + ExecutionID: execID, Status: status, Error: "t4g.nano: " + fakeAWSDenied, + }, "", "") + assert.NotEmpty(t, row.StatusDescription, status) + assertNoProviderText(t, status+" status_description", row.StatusDescription) + } + }) + + t.Run("purchase details", func(t *testing.T) { + exec := &config.PurchaseExecution{ExecutionID: execID, Status: "failed", Error: fakeAWSDenied, + Recommendations: []config.RecommendationRecord{rec}} + resp := buildPurchaseDetailsResponse(exec, "plan") + body, err := json.Marshal(resp) + require.NoError(t, err) + assertNoProviderText(t, "details body", string(body)) + assert.Equal(t, fakeAWSDenied, exec.Error, "stored row is not mutated") + assert.Equal(t, fakeAWSDenied, exec.Recommendations[0].Error, "stored rec is not mutated") + }) + + t.Run("retry 409 failure_reason", func(t *testing.T) { + creator := retryCallerID + failed := &config.PurchaseExecution{ExecutionID: retryExecID, Status: "failed", + Error: "AssumeRole error: IAM denied: " + fakeAWSDenied, CreatedByUserID: &creator} + handler, _, _ := buildSessionRetryHandler(t, failed, &Session{UserID: retryCallerID}, false, true) + _, err := handler.retryPurchase(t.Context(), sessionRetryReq(), retryExecID) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 409, ce.code) + body, mErr := json.Marshal(ce.Details()) + require.NoError(t, mErr) + assertNoProviderText(t, "retry details", string(body)+ce.Error()) + assert.NotEmpty(t, ce.Details()["failure_reason"]) + }) +} + +func TestExecuteFailureError_PartialAndAuditLossHideProviderText(t *testing.T) { + for _, status := range []string{"partially_completed", "failed"} { + err := executeFailureError("e1", "executed", &config.PurchaseExecution{Status: status}, + errors.New("some purchases failed (partial success): "+fakeAWSDenied)) + ce, ok := IsClientError(err) + require.True(t, ok) + assertNoProviderText(t, status, ce.Error()) + } + err := executeFailureError("e1", "executed", &config.PurchaseExecution{Status: "failed"}, + errors.Join(config.ErrAuditLoss, errors.New(fakeAWSDenied))) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 500, ce.code) + assertNoProviderText(t, "audit loss", ce.Error()) +} + +const ( + fakeSubID = "00000000-0000-0000-0000-000000000000" + fakeTenant = "11111111-1111-1111-1111-111111111111" +) + +func azureHTTPError(t *testing.T, status int, path string) error { + t.Helper() + req, err := http.NewRequest(http.MethodPut, "https://management.azure.com"+path, nil) + require.NoError(t, err) + return runtime.NewResponseError(&http.Response{ + StatusCode: status, Status: http.StatusText(status), Request: req, + Header: http.Header{"Content-Type": {"text/html"}}, + Body: io.NopCloser(strings.NewReader("gateway error")), + }) +} + +func assertNoAzureText(t *testing.T, where, s string) { + t.Helper() + for _, n := range []string{fakeSubID, fakeTenant, "management.azure.com", "gateway", "ERROR CODE"} { + assert.NotContains(t, s, n, where) + } +} + +func TestCodelessAzureError_NeverReachesUserSurfaces(t *testing.T) { + for _, status := range []int{400, 409, 412} { + raw := azureHTTPError(t, status, "/subscriptions/"+fakeSubID+"/providers/Microsoft.BillingBenefits/savingsPlanOrderAliases/x") + rec := config.RecommendationRecord{Provider: "azure", Service: "savingsplans", Error: raw.Error()} + ce, ok := IsClientError(executeFailureError("e1", "executed", &config.PurchaseExecution{Status: "failed"}, raw)) + require.True(t, ok) + assertNoAzureText(t, "live", ce.Error()) + row := executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: raw.Error(), Recommendations: []config.RecommendationRecord{rec}}, "", "") + assert.NotEmpty(t, row.StatusDescription) + assertNoAzureText(t, "history", row.StatusDescription) + resp, err := json.Marshal(buildPurchaseDetailsResponse(&config.PurchaseExecution{Status: "failed", Error: raw.Error(), Recommendations: []config.RecommendationRecord{rec}}, "plan")) + require.NoError(t, err) + assertNoAzureText(t, "details", string(resp)) + } +} + +func TestAzureIdentityText_NeverReachesUserSurfaces(t *testing.T) { + raw := "ClientSecretCredential authentication failed. POST https://login.microsoftonline.com/" + fakeTenant + "/oauth2/v2.0/token\n" + + "RESPONSE 400: 400 Bad Request\n{\"error_description\":\"AADSTS700016: Application not found. Trace ID: 22222222 Correlation ID: 33333333\"}" + ce, ok := IsClientError(executeFailureError("e1", "executed", &config.PurchaseExecution{Status: "failed"}, errors.New(raw))) + require.True(t, ok) + for _, s := range []string{ce.Error(), executionToHistoryRow(config.PurchaseExecution{Status: "failed", Error: raw}, "", "").StatusDescription} { + for _, n := range []string{fakeTenant, "login.microsoftonline.com", "AADSTS", "Trace ID", "Correlation ID"} { + assert.NotContains(t, s, n) + } + } +} diff --git a/internal/api/token_approve_failure_message_test.go b/internal/api/token_approve_failure_message_test.go index 2cbd78f95..ff9ac3816 100644 --- a/internal/api/token_approve_failure_message_test.go +++ b/internal/api/token_approve_failure_message_test.go @@ -101,9 +101,9 @@ func TestTokenApprove_FailureBodyOmitsRunErrorText(t *testing.T) { } } -// The session caller holds approve-* scoped to the execution and keeps the -// detailed message. -func TestSessionApprove_FailureKeepsRunErrorText(t *testing.T) { +// The session caller gets the execution reference and a category message; the +// provider's own text (ARNs, account IDs) stays in the server log. +func TestSessionApprove_FailureShowsSummaryNotRunErrorText(t *testing.T) { ctx := context.Background() execID := "c1234567-c123-c123-c123-c1234567abcd" for _, tc := range tokenFailureCases()[:3] { @@ -113,7 +113,9 @@ func TestSessionApprove_FailureKeepsRunErrorText(t *testing.T) { ce, ok := IsClientError(err) require.True(t, ok, "expected a client error, got %v", err) assert.Equal(t, tc.code, ce.code) - assert.Contains(t, ce.message, otherAcctName) + assert.NotContains(t, ce.message, "arn:aws") + assert.NotContains(t, ce.message, otherAcctID) + assert.Contains(t, ce.message, execID) }) } } diff --git a/internal/purchase/errnotes.go b/internal/purchase/errnotes.go new file mode 100644 index 000000000..e7b4d070b --- /dev/null +++ b/internal/purchase/errnotes.go @@ -0,0 +1,107 @@ +package purchase + +import ( + "errors" + "regexp" + "strings" +) + +// serverNotePatterns lists every server-authored string stored into +// purchase_executions.error or a recommendation's error (enumerated from the +// assignments in purchase/execution.go, manager.go, reaper.go, recover_reaped.go, +// notifications.go, plan_step_attach.go, stranded_redrive.go, step_cap.go, +// due_age_cap.go, claim.go, ladder_execution_recheck.go and the api retry +// handler's email reasons). Anything else in a stored error is treated as +// provider text. Each pattern is anchored and closed: no pattern matches free text or a newline. +// The em dash in the reaper note is written as \x{2014}. +var serverNotePatterns = []string{ + `history_write_failed: commitment [^;\s]+ purchased but its history record failed to save`, + `recovered after reap; provider purchase succeeded`, + `reaped after \d+m in [a-z_]+ state \x{2014} executor did not complete(?:; safe to retry)?`, + `execution was approved but its purchase run was interrupted before completing and never finalized; failed by the recovery sweep so it is not silently stuck \(issue #632\)\. (?:Verify on the cloud provider that no commitment was created, then Retry\.|Retry is refused for this purchase because a repeat could buy a second commitment\. Verify on the cloud provider whether the commitment was created; if it was not, start a new purchase\.)`, + `refusing to execute retry attempt \d+: (?:` + redriveReasonPattern + `)`, + `stale Azure recommendation pricing; refresh recommendations and submit a new purchase(?:: the latest collection state is missing, so Azure prices cannot be confirmed current)?`, + `(?:execution [^:\s]+: )?plan step has no recommendations attached, so it cannot buy anything \(platform#631: plan steps cannot yet carry recommendations\)`, + `plan step refused by commitment cap(?:: plan has no max_step_commitment set \(auto-purchase of steps with recommendations is refused until one is set\)|: step total commitment \d+\.\d{2} exceeds max_step_commitment \d+\.\d{2})?`, + `scheduled purchase skipped: it was due too long ago(?:: it was due at \d{4}-\d{2}-\d{2}T[0-9:.]+Z, [0-9a-z.]+ ago, which is more than the [0-9a-z.]+ limit \(SCHEDULED_PURCHASE_MAX_OVERDUE\); nothing was submitted, retry it if the purchase is still wanted)?`, + `purchase refused after claim: the claimed execution no longer passes the authorization checks`, + `refusing to execute ladder purchase: (?:global config unavailable|ladder planning is switched off|ladder execution is switched off|its tranche status is unknown|its tranche is [a-z_]+|its ladder config no longer exists|its ladder config is disabled|ladder config mode is "[a-z_]*", not auto_approve|its ladder run no longer exists|its ladder run was canceled)`, + `scheduled plan step reached its scheduled date without recommendations; no purchase was submitted`, + `no current recommendation matched plan [^;\s]+ step \d+ by \d{4}-\d{2}-\d{2} \(\d+-day grace after the plan date\)(?:; ` + skippedNotePattern + `)?`, + `target already met: target \d+, committed \d+(?:; ` + skippedNotePattern + `)?; \d+ failed row\(s\) of unknown outcome counted as committed`, + `selected recommendations do not resolve to a single cloud account: (?:\d+ selected recommendation\(s\) target \d+ cloud accounts \(` + idListPattern + `\); submit one purchase per account|\d+ selected recommendation\(s\) carry no cloud_account_id while \d+ target account ` + idPattern + `)`, + `purchase was not successful`, + `EC2 recommendation for \S+ has zonal scope but no AvailabilityZone: refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation`, + `zonal RI offering needs an availability zone; re-fetch the recommendation`, + `email notifier not configured for this deployment`, + `no notification email set in Settings \x{2192} General(?: and no account contact emails configured)?`, + `FROM_EMAIL not configured for this deployment`, +} + +const ( + idPattern = `[^;\s,()]+` + idListPattern = idPattern + `(?:, ` + idPattern + `)*` + skippedNotePattern = `\d+ recommendation\(s\) without a frozen baseline skipped \(under-buy\): ` + idListPattern +) + +// serverNoteHeads are server notes whose tail can carry a wrapped store or +// config error. Only the fixed head is ever shown; the tail is dropped. +var serverNoteHeads = []string{ + `ramp not advanced`, + `refusing to execute: ladder tranche lookup failed`, + `refusing to execute ladder purchase: global config unreadable`, + `stale Azure recommendation pricing; refresh recommendations and submit a new purchase`, + `plan step refused by commitment cap`, + `purchase refused after claim: the claimed execution no longer passes the authorization checks`, + `scheduled purchase skipped: it was due too long ago`, +} + +var redriveReasonPattern = regexp.QuoteMeta(redriveReasonAWSNoGuard) + `|` + regexp.QuoteMeta(redriveReasonAzureSavingsPlan) + + `|provider "[^"]*" is not known to reject a duplicate purchase, so re-driving this could buy a second commitment` + +// A rec's ": " head, as the writer's per-rec join adds it. +var serverNoteRE = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9._-]*: )?(?:` + strings.Join(serverNotePatterns, "|") + `)(?:; |$)`) + +var serverNoteHeadRE = regexp.MustCompile(`^((?:[A-Za-z0-9][A-Za-z0-9._-]*: )?(?:` + strings.Join(serverNoteHeads, "|") + `))(?:: [^;]*)?(?:; |$)`) + +// renderServerNotes returns s with every "; "-separated item either kept +// verbatim (a closed allowlisted note) or cut to its fixed head (a known note +// whose tail is not closed). ok is false when any item is neither. Matching +// consumes whole notes, so a "; " inside a provider message cannot smuggle a +// tail through. +func renderServerNotes(s string) (string, bool) { + var out strings.Builder + for s != "" { + if m := serverNoteRE.FindString(s); m != "" { + out.WriteString(m) + s = s[len(m):] + continue + } + loc := serverNoteHeadRE.FindStringSubmatchIndex(s) + if loc == nil { + return "", false + } + out.WriteString(s[loc[2]:loc[3]]) + if strings.HasSuffix(s[:loc[1]], "; ") { + out.WriteString("; ") + } + s = s[loc[1]:] + } + return out.String(), true +} + +// ServerNoteMessage returns fixed server text for a known server-authored +// refusal so live responses can show it. A sentinel yields only its own text, +// never the wrapped chain; any other error must be allowlisted notes. +func ServerNoteMessage(err error) (string, bool) { + if err == nil { + return "", false + } + for _, sentinel := range []error{ErrPlanStepNoRecommendations, ErrStaleAzurePricing, ErrStepCommitmentCap, + ErrDueRowTooOld, ErrClaimedRowRefused, errAmbiguousAccountScope} { + if errors.Is(err, sentinel) { + return sentinel.Error(), true + } + } + return renderServerNotes(err.Error()) +} diff --git a/internal/purchase/errnotes_tail_test.go b/internal/purchase/errnotes_tail_test.go new file mode 100644 index 000000000..851d9cbd0 --- /dev/null +++ b/internal/purchase/errnotes_tail_test.go @@ -0,0 +1,73 @@ +package purchase + +import ( + "errors" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +const fakeConnectError = "failed to connect to `host=db.internal.example user=cudly_app database=cudly`: " + + "dial error (dial tcp 10.0.0.5:5432: connect: connection refused)" + +var connectNeedles = []string{"db.internal.example", "cudly_app", "10.0.0.5", "dial", "connect"} + +func assertNoConnectDetail(t *testing.T, where, s string) { + t.Helper() + for _, n := range connectNeedles { + assert.NotContains(t, s, n, where) + } +} + +// A note whose tail is not a closed format shows only its fixed head. +func TestSanitizeStoredError_TailedNotesShowOnlyTheirHead(t *testing.T) { + cases := []struct{ in, want string }{ + {"ramp not advanced: " + fakeConnectError, "ramp not advanced"}, + {"ramp not advanced: " + fakeConnectError + "\nsecond line", "ramp not advanced"}, + {"refusing to execute: ladder tranche lookup failed: " + fakeConnectError, "refusing to execute: ladder tranche lookup failed"}, + {"refusing to execute ladder purchase: global config unreadable: " + fakeConnectError, "refusing to execute ladder purchase: global config unreadable"}, + {ErrStaleAzurePricing.Error() + ": cannot read the latest collection state to confirm Azure prices are current: " + fakeConnectError, ErrStaleAzurePricing.Error()}, + {ErrStaleAzurePricing.Error() + ": Azure recommendation \"r-1\" no longer has the approved price", ErrStaleAzurePricing.Error()}, + {ErrStepCommitmentCap.Error() + ": recommendation \"r-1\" is not priced in USD, so it cannot be compared against the USD cap", ErrStepCommitmentCap.Error()}, + {ErrClaimedRowRefused.Error() + ": " + fakeConnectError, ErrClaimedRowRefused.Error()}, + {"x: ramp not advanced: " + fakeConnectError + "; recovered after reap; provider purchase succeeded", "x: ramp not advanced; recovered after reap; provider purchase succeeded"}, + {partialRunPrefix + "ramp not advanced: " + fakeConnectError, partialRunPrefix + "ramp not advanced"}, + } + for _, c := range cases { + out := SanitizeStoredError(c.in) + assertNoConnectDetail(t, c.in, out) + assert.Equal(t, c.want, out, c.in) + } +} + +func TestSanitizeStoredError_NoPatternMatchesANewline(t *testing.T) { + for _, in := range []string{ + "purchase refused after claim: the claimed execution no longer passes the authorization checks\narn:aws:iam::123456789012:role/x", + "refusing to execute ladder purchase: its tranche is paused\narn:aws:iam::123456789012:role/x", + "selected recommendations do not resolve to a single cloud account: 1 selected recommendation(s) carry no cloud_account_id while 2 target account a\nb", + "no current recommendation matched plan p step 2 by 2026-01-02\n(3-day grace after the plan date)", + } { + assert.Equal(t, ErrCategoryMessage(ErrCategoryUnknown), SanitizeStoredError(in), in) + } +} + +// A sentinel yields only its own text, never the wrapped chain. +func TestServerNoteMessage_SentinelReturnsOnlyItsOwnText(t *testing.T) { + for _, sentinel := range []error{ErrPlanStepNoRecommendations, ErrStaleAzurePricing, ErrStepCommitmentCap, + ErrDueRowTooOld, ErrClaimedRowRefused, errAmbiguousAccountScope} { + for _, err := range []error{ + fmt.Errorf("%w: %w", sentinel, errors.New(fakeConnectError)), + fmt.Errorf("%w: cannot read the latest collection state to confirm Azure prices are current: %w", sentinel, errors.New(fakeConnectError)), + fmt.Errorf("execution e-1: %w: %s", sentinel, fakeConnectError), + } { + msg, ok := ServerNoteMessage(err) + assert.True(t, ok) + assert.Equal(t, sentinel.Error(), msg) + assertNoConnectDetail(t, sentinel.Error(), msg) + } + } + msg, ok := ServerNoteMessage(errors.New("ramp not advanced: " + fakeConnectError)) + assert.True(t, ok) + assert.Equal(t, "ramp not advanced", msg) +} diff --git a/internal/purchase/errnotes_test.go b/internal/purchase/errnotes_test.go new file mode 100644 index 000000000..a8e366396 --- /dev/null +++ b/internal/purchase/errnotes_test.go @@ -0,0 +1,79 @@ +package purchase + +import ( + "fmt" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" +) + +// Every server-authored string stored into purchase_executions.error or a +// recommendation's error must come back verbatim. +func TestSanitizeStoredError_AllowlistedNotesPassThroughVerbatim(t *testing.T) { + fresh := &config.PurchaseExecution{} + recordHistoryAuditGap(fresh, "c-1", fmt.Errorf("db down")) + retry := &config.PurchaseExecution{RetryAttemptN: 2, Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}} + azSP := &config.PurchaseExecution{RetryAttemptN: 1, Recommendations: []config.RecommendationRecord{{Provider: "azure", Service: "savingsplans"}}} + gcpRetry := &config.PurchaseExecution{RetryAttemptN: 1, Recommendations: []config.RecommendationRecord{{Provider: "weird"}}} + notes := []string{ + fresh.Error, + recoveredAfterReapNote, + "ramp not advanced", + "reaped after 30m in running state — executor did not complete", + "reaped after 30m in approved state — executor did not complete; safe to retry", + strandedRecoveryError(&config.PurchaseExecution{}), + strandedRecoveryError(retry), + armedRedriveRefusal(retry).Error(), + armedRedriveRefusal(azSP).Error(), + armedRedriveRefusal(gcpRetry).Error(), + ErrStaleAzurePricing.Error(), + ErrPlanStepNoRecommendations.Error(), + fmt.Errorf("execution %s: %w", "00000000-0000-0000-0000-000000000000", ErrPlanStepNoRecommendations).Error(), + fmt.Errorf("%w: plan has no max_step_commitment set (auto-purchase of steps with recommendations is refused until one is set)", ErrStepCommitmentCap).Error(), + fmt.Errorf("%w: step total commitment 10.00 exceeds max_step_commitment 5.00", ErrStepCommitmentCap).Error(), + dueRowAgeRefusal(&config.PurchaseExecution{ScheduledDate: time.Now().Add(-100 * time.Hour)}, time.Hour, time.Now()).Error(), + ErrClaimedRowRefused.Error(), + "refusing to execute ladder purchase: its ladder run was canceled", + "refusing to execute ladder purchase: its tranche is paused", + "refusing to execute: ladder tranche lookup failed", + "refusing to execute ladder purchase: global config unreadable", + "scheduled plan step reached its scheduled date without recommendations; no purchase was submitted", + "no current recommendation matched plan 00000000-0000-0000-0000-000000000000 step 2 by 2026-01-02 (3-day grace after the plan date)", + "no current recommendation matched plan p step 2 by 2026-01-02 (3-day grace after the plan date); " + skippedNote([]string{"a\x1fb", "c"}), + "target already met: target 10, committed 12; 0 failed row(s) of unknown outcome counted as committed", + "target already met: target 10, committed 12; " + skippedNote([]string{"a"}) + "; 1 failed row(s) of unknown outcome counted as committed", + fmt.Errorf("%w: 2 selected recommendation(s) target 2 cloud accounts (a, b); submit one purchase per account", errAmbiguousAccountScope).Error(), + fmt.Errorf("%w: 1 selected recommendation(s) carry no cloud_account_id while 2 target account a", errAmbiguousAccountScope).Error(), + "purchase was not successful", + "EC2 recommendation for m5.large has zonal scope but no AvailabilityZone: refusing to buy a zonal RI in an arbitrary AZ; re-fetch the recommendation", + "zonal RI offering needs an availability zone; re-fetch the recommendation", + "email notifier not configured for this deployment", + "no notification email set in Settings → General and no account contact emails configured", + "no notification email set in Settings → General", + "FROM_EMAIL not configured for this deployment", + } + for _, n := range notes { + assert.NotEmpty(t, n) + assert.Equal(t, n, SanitizeStoredError(n), n) + assert.Equal(t, n, SanitizeStoredError(partialRunPrefix + "x: " + n)[len(partialRunPrefix)+3:], "behind a resource head: "+n) + } + joined := notes[0] + "; " + notes[1] + "; " + notes[6] + assert.Equal(t, joined, SanitizeStoredError(joined)) + assert.Equal(t, partialRunPrefix+joined, SanitizeStoredError(partialRunPrefix+joined)) +} + +func TestSanitizeStoredError_UnknownTextFailsClosed(t *testing.T) { + for _, in := range []string{ + "some unlisted text", + "history_write_failed: commitment c purchased; and then provider text", + "ramp not advanced: x; some tail", + "purchase failed: boom", + "send failed: SES said no", + "failed to get plan: db", + } { + out := SanitizeStoredError(in) + assert.Equal(t, ErrCategoryMessage(ErrCategoryUnknown), out, in) + } +} diff --git a/internal/purchase/errsummary.go b/internal/purchase/errsummary.go new file mode 100644 index 000000000..c2edf57fa --- /dev/null +++ b/internal/purchase/errsummary.go @@ -0,0 +1,248 @@ +package purchase + +import ( + "errors" + "net/http" + "regexp" + "strings" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "google.golang.org/api/googleapi" +) + +// Closed set of purchase failure categories shown to users in place of the +// provider's own error text, which carries ARNs, account IDs and request IDs. +const ( + ErrCategoryAccessDenied = "access_denied" + ErrCategoryUnavailable = "not_available" + ErrCategoryLimit = "limit_reached" + ErrCategoryUnknown = "unknown" +) + +var errCategoryMessages = map[string]string{ + ErrCategoryAccessDenied: "the cloud provider refused the purchase: CUDly's role is not allowed to make it in this account", + ErrCategoryUnavailable: "the requested commitment is not available from the provider", + ErrCategoryLimit: "a provider limit was reached", + ErrCategoryUnknown: "the provider returned an error", +} + +// outcomeUnconfirmedMessage replaces the category message when the provider had +// already accepted the request and only the wait for its result failed. +const outcomeUnconfirmedMessage = "the provider accepted the request but its outcome could not be confirmed; check the account before trying again" + +// postInsertWaitMarker is the cloud-commitments-go GCP wrapper around a failed +// wait on an operation whose Insert was accepted. +const postInsertWaitMarker = "commitment creation failed: " + +const partialRunPrefix = "some purchases failed (partial success): " + +// ErrCategoryMessage returns the fixed user message for a category. +func ErrCategoryMessage(category string) string { + if m, ok := errCategoryMessages[category]; ok { + return m + } + return errCategoryMessages[ErrCategoryUnknown] +} + +// SummarizeProviderError classifies a live error by its type or explicit code +// fields and returns its category and fixed message. +func SummarizeProviderError(err error) (category, message string) { + if err != nil && strings.Contains(err.Error(), postInsertWaitMarker) { + return ErrCategoryUnknown, outcomeUnconfirmedMessage + } + category = classifyTypedError(err) + if category == ErrCategoryUnknown && err != nil { + category = classifyStoredText(err.Error()) + } + return category, ErrCategoryMessage(category) +} + +func classifyTypedError(err error) string { + var apiErr interface{ ErrorCode() string } + var azErr *azcore.ResponseError + var gErr *googleapi.Error + var authErr *azidentity.AuthenticationFailedError + switch { + case errors.As(err, &authErr): + return ErrCategoryAccessDenied + case errors.As(err, &apiErr): + if c := classifyCode(apiErr.ErrorCode()); c != ErrCategoryUnknown { + return c + } + case errors.As(err, &azErr): + if c := classifyCode(azErr.ErrorCode); c != ErrCategoryUnknown { + return c + } + return categoryForStatus(azErr.StatusCode) + case errors.As(err, &gErr): + for _, item := range gErr.Errors { + if c := classifyCode(item.Reason); c != ErrCategoryUnknown { + return c + } + } + return categoryForStatus(gErr.Code) + } + var httpErr interface{ HTTPStatusCode() int } + if errors.As(err, &httpErr) { + return categoryForStatus(httpErr.HTTPStatusCode()) + } + return ErrCategoryUnknown +} + +func categoryForStatus(code int) string { + switch code { + case http.StatusUnauthorized, http.StatusForbidden: + return ErrCategoryAccessDenied + case http.StatusNotFound: + return ErrCategoryUnavailable + case http.StatusTooManyRequests: + return ErrCategoryLimit + } + return ErrCategoryUnknown +} + +var ( + deniedCodes = []string{"accessdenied", "unauthorized", "authorizationfailed", "forbidden"} + limitCodes = []string{"limitexceeded", "quota", "throttl", "toomanyrequests", "ratelimit"} + unavailCodes = []string{"capacity", "offeringnotfound", "notfound", "notavailable"} +) + +func containsAny(lower string, needles []string) bool { + for _, n := range needles { + if strings.Contains(lower, n) { + return true + } + } + return false +} + +// classifyCode classifies one short error code or reason, never free text. +func classifyCode(code string) string { + lower := strings.ToLower(code) + switch { + case containsAny(lower, deniedCodes): + return ErrCategoryAccessDenied + case containsAny(lower, limitCodes): + return ErrCategoryLimit + case containsAny(lower, unavailCodes): + return ErrCategoryUnavailable + } + return ErrCategoryUnknown +} + +// Explicit code fields in the providers' own error formats. Words elsewhere +// (URLs, resource names, bodies) never count. +var ( + awsCodeRE = regexp.MustCompile(`api error ([A-Za-z0-9_.]+):`) + awsStatusRE = regexp.MustCompile(`StatusCode: (\d{3})`) + azCodeRE = regexp.MustCompile(`ERROR CODE: ([A-Za-z0-9_.-]+)`) + azStatusRE = regexp.MustCompile(`RESPONSE (\d{3}):`) + gcpReasonRE = regexp.MustCompile(`Reason: ([A-Za-z]+), Message:`) + gcpErrorRE = regexp.MustCompile(`googleapi: Error (\d{3}): [^\n]*?(?:, ([A-Za-z]+))?(?:\n|$)`) + definiteAWSRE = regexp.MustCompile(`^(?:[A-Za-z0-9][A-Za-z0-9._-]*: )?operation error [^\n]*StatusCode: (4\d{2}),[^\n]*api error ([A-Za-z0-9_.]+): [^\n]*$`) +) + +func statusOf(re *regexp.Regexp, s string) int { + m := re.FindStringSubmatch(s) + if m == nil { + return 0 + } + n := 0 + for _, c := range m[1] { + n = n*10 + int(c-'0') + } + return n +} + +// classifyStoredText classifies a stored error string from its explicit code +// fields only; no code field means unknown. +func classifyStoredText(s string) string { + if strings.Contains(s, postInsertWaitMarker) { + return ErrCategoryUnknown + } + for _, re := range []*regexp.Regexp{awsCodeRE, azCodeRE, gcpErrorRE, gcpReasonRE} { + for _, code := range lastGroups(re, s) { + if c := classifyCode(code); c != ErrCategoryUnknown { + return c + } + } + } + for _, re := range []*regexp.Regexp{awsStatusRE, azStatusRE, gcpErrorRE} { + if c := categoryForStatus(statusOf(re, s)); c != ErrCategoryUnknown { + return c + } + } + return ErrCategoryUnknown +} + +// lastGroups returns the last capture group of every match of re in s. +func lastGroups(re *regexp.Regexp, s string) []string { + matches := re.FindAllStringSubmatch(s, -1) + out := make([]string, 0, len(matches)) + for _, m := range matches { + out = append(out, m[len(m)-1]) + } + return out +} + +// definiteAWSCodes are the AWS error codes returned before anything is +// created: refused credentials, throttling and rejected parameters. +var definiteAWSCodes = map[string]bool{ + "AccessDenied": true, "AccessDeniedException": true, "UnauthorizedOperation": true, "AuthFailure": true, + "UnrecognizedClientException": true, "InvalidClientTokenId": true, "ExpiredToken": true, "ExpiredTokenException": true, + "Throttling": true, "ThrottlingException": true, "RequestLimitExceeded": true, "TooManyRequestsException": true, + "LimitExceeded": true, "ReservedInstanceLimitExceeded": true, "ReservedInstancesLimitExceeded": true, + "ReservedDBInstanceQuotaExceeded": true, "ReservedCacheNodeQuotaExceeded": true, + "InvalidParameterValue": true, "InvalidParameterCombination": true, "InvalidParameter": true, + "ValidationError": true, "ValidationException": true, "MissingParameter": true, + "InvalidReservedInstancesOfferingId": true, "ReservedInstancesOfferingNotFound": true, + "ReservedDBInstancesOfferingNotFound": true, "ReservedCacheNodesOfferingNotFound": true, +} + +func definiteAWSSegment(seg string) bool { + if strings.Contains(seg, "outcome unknown") { + return false + } + m := definiteAWSRE.FindStringSubmatch(seg) + return len(m) == 3 && definiteAWSCodes[m[2]] +} + +// IsDefiniteAWSRefusal reports whether a stored error proves nothing was +// bought: every rec is AWS and every "; "-separated segment is an AWS 4xx +// with a listed pre-send code. Azure, GCP, 5xx, a missing code or a server +// note anywhere means the outcome is not proven, so the answer is false. +func IsDefiniteAWSRefusal(recs []config.RecommendationRecord, stored string) bool { + if len(recs) == 0 { + return false + } + for i := range recs { + if recs[i].Provider != "" && recs[i].Provider != "aws" { + return false + } + } + stored = strings.TrimPrefix(stored, partialRunPrefix) + for _, seg := range strings.Split(stored, "; ") { + if !definiteAWSSegment(seg) { + return false + } + } + return stored != "" +} + +// SanitizeStoredError turns a stored error string into text safe to show: it +// passes through only when every item is an allowlisted server note, and +// otherwise the whole remainder becomes one category message. The partial-run +// prefix is kept. +func SanitizeStoredError(s string) string { + rest := strings.TrimPrefix(s, partialRunPrefix) + prefix := s[:len(s)-len(rest)] + if out, ok := renderServerNotes(rest); ok { + return prefix + out + } + if strings.Contains(rest, postInsertWaitMarker) { + return prefix + outcomeUnconfirmedMessage + } + return prefix + ErrCategoryMessage(classifyStoredText(rest)) +} diff --git a/internal/purchase/errsummary_provider_test.go b/internal/purchase/errsummary_provider_test.go new file mode 100644 index 000000000..70486e485 --- /dev/null +++ b/internal/purchase/errsummary_provider_test.go @@ -0,0 +1,209 @@ +package purchase + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/api/googleapi" +) + +const ( + fakeSub = "00000000-0000-0000-0000-000000000000" + fakeTenant = "11111111-1111-1111-1111-111111111111" +) + +var providerDetailNeedles = []string{fakeSub, fakeTenant, "management.azure.com", "login.microsoftonline.com", + "Trace ID", "Correlation ID", "AADSTS", "123456789012", "gateway"} + +func assertNoProviderDetail(t *testing.T, where, s string) { + t.Helper() + for _, n := range providerDetailNeedles { + assert.NotContains(t, s, n, where) + } +} + +func azureResponseError(t *testing.T, status int, path, contentType, body string) error { + t.Helper() + req, err := http.NewRequest(http.MethodPut, "https://management.azure.com"+path, nil) + require.NoError(t, err) + resp := &http.Response{ + StatusCode: status, Status: http.StatusText(status), Request: req, + Header: http.Header{"Content-Type": {contentType}}, + Body: io.NopCloser(strings.NewReader(body)), + } + return runtime.NewResponseError(resp) +} + +type fakeTransport struct{ status int } + +func (f fakeTransport) Do(r *http.Request) (*http.Response, error) { + body := `{"error":"unauthorized_client","error_description":"AADSTS700016: Application not found in tenant ` + fakeTenant + `. Trace ID: 22222222 Correlation ID: 33333333"}` + return &http.Response{StatusCode: f.status, Status: "400 Bad Request", Request: r, + Header: http.Header{"Content-Type": {"application/json"}}, Body: io.NopCloser(strings.NewReader(body))}, nil +} + +func aadError(t *testing.T) error { + t.Helper() + cred, err := azidentity.NewClientSecretCredential(fakeTenant, fakeSub, "fake-secret", &azidentity.ClientSecretCredentialOptions{ + ClientOptions: azcore.ClientOptions{Transport: fakeTransport{status: 400}}, + DisableInstanceDiscovery: true, + }) + require.NoError(t, err) + _, err = cred.GetToken(context.Background(), policy.TokenRequestOptions{Scopes: []string{"https://management.azure.com/.default"}}) + require.Error(t, err) + return err +} + +func TestAzureCodelessErrors_StoredAndLiveAreSanitized(t *testing.T) { + for _, status := range []int{400, 409, 412} { + err := azureResponseError(t, status, "/subscriptions/"+fakeSub+"/providers/Microsoft.Capacity/reservationOrders", "text/html", "gateway error") + require.Contains(t, err.Error(), "ERROR CODE UNAVAILABLE") + stored := SanitizeStoredError(err.Error()) + assertNoProviderDetail(t, "stored", stored) + assert.NotEmpty(t, stored) + _, live := SummarizeProviderError(err) + assertNoProviderDetail(t, "live", live) + _, liveStored := SummarizeProviderError(err) + assert.Equal(t, ErrCategoryMessage(ErrCategoryUnknown), liveStored) + } +} + +func TestAzureIdentityFailure_StoredAndLiveAreSanitized(t *testing.T) { + err := aadError(t) + var afe *azidentity.AuthenticationFailedError + require.ErrorAs(t, err, &afe) + require.Contains(t, err.Error(), "AADSTS700016") + assertNoProviderDetail(t, "stored", SanitizeStoredError(err.Error())) + cat, msg := SummarizeProviderError(err) + assert.Equal(t, ErrCategoryAccessDenied, cat) + assertNoProviderDetail(t, "live", msg) +} + +// A 500 whose URL names Microsoft.Capacity is an unknown outcome, never "not available". +func TestAzure500OnCapacityURL_IsUnknown(t *testing.T) { + err := azureResponseError(t, 500, "/providers/Microsoft.Capacity/reservationOrders/x", "application/json", `{"error":{"code":"InternalServerError","message":"boom"}}`) + cat, _ := SummarizeProviderError(err) + assert.Equal(t, ErrCategoryUnknown, cat) + assert.Equal(t, ErrCategoryUnknown, classifyStoredText(err.Error())) +} + +func TestClassifyStoredText_IgnoresWordsOutsideCodeFields(t *testing.T) { + for _, s := range []string{ + "PUT https://management.azure.com/providers/Microsoft.Capacity/x\nRESPONSE 500: 500 Internal\nERROR CODE: InternalServerError", + "something about quota and forbidden and notfound", + "operation error EC2: X, https response error StatusCode: 500, api error InternalError: capacity forbidden", + } { + assert.Equal(t, ErrCategoryUnknown, classifyStoredText(s), s) + } +} + +func TestGoogleAPIErrors_ClassifyByCodeAndReason(t *testing.T) { + rate := &googleapi.Error{Code: 403, Message: "Rate Limit Exceeded", Errors: []googleapi.ErrorItem{{Reason: "rateLimitExceeded", Message: "Rate Limit Exceeded"}}} + cat, _ := SummarizeProviderError(rate) + assert.Equal(t, ErrCategoryLimit, cat) + assert.Equal(t, ErrCategoryLimit, classifyStoredText(rate.Error())) + nf := &googleapi.Error{Code: 404, Message: "operations/op-1 was not found", Errors: []googleapi.ErrorItem{{Reason: "notFound", Message: "operations/op-1 was not found"}}} + assert.Equal(t, ErrCategoryUnavailable, classifyStoredText(nf.Error())) + multi := &googleapi.Error{Code: 403, Message: "m", Errors: []googleapi.ErrorItem{{Reason: "rateLimitExceeded", Message: "a"}, {Reason: "x", Message: "b"}}} + assert.Equal(t, ErrCategoryLimit, classifyStoredText(multi.Error())) +} + +func TestSanitizeStoredError_WholeRemainderReplaced(t *testing.T) { + const awsWithSemicolon = "operation error EC2: X, https response error StatusCode: 400, api error InvalidParameterValue: offering rejected; reserved for account 123456789012 in us-east-1" + for _, in := range []string{ + awsWithSemicolon, + "t4g.nano: " + awsWithSemicolon, + "t4g.nano: " + awsWithSemicolon + "; t4g.micro: " + awsWithSemicolon, + "history_write_failed: commitment c-1 purchased but its history record failed to save; " + awsWithSemicolon, + "plain provider words with no marker at all", + } { + out := SanitizeStoredError(in) + assert.NotContains(t, out, "123456789012", in) + assert.NotContains(t, out, "reserved for", in) + assert.NotEmpty(t, out) + assert.Equal(t, 1, strings.Count(out, "the provider"), in) + } + out := SanitizeStoredError(partialRunPrefix + "t4g.nano: " + awsWithSemicolon) + assert.True(t, strings.HasPrefix(out, partialRunPrefix)) + assert.NotContains(t, out, "123456789012") + assert.Equal(t, "", SanitizeStoredError("")) +} + +func TestIsDefiniteAWSRefusal(t *testing.T) { + aws := func(status, code string) string { + return "operation error EC2: X, https response error StatusCode: " + status + ", RequestID: r, api error " + code + ": msg" + } + azure429 := "PUT https://management.azure.com/x\nRESPONSE 429: 429 Too Many\nERROR CODE: TooManyRequests" + gcp := "commitment creation failed: googleapi: Error 403: Rate Limit Exceeded, rateLimitExceeded" + cases := []struct { + name string + recs []string + err string + want bool + }{ + {"aws 403 denied", []string{"aws"}, aws("403", "UnauthorizedOperation"), true}, + {"legacy empty provider", []string{""}, aws("403", "AccessDenied"), true}, + {"aws throttle 400", []string{"aws"}, aws("400", "Throttling"), true}, + {"aws invalid offering", []string{"aws"}, aws("400", "InvalidParameterValue"), true}, + {"per-rec segments", []string{"aws", "aws"}, "a: " + aws("403", "AccessDenied") + "; b: " + aws("400", "Throttling"), true}, + {"partial prefix", []string{"aws"}, partialRunPrefix + "a: " + aws("403", "AccessDenied"), true}, + {"5xx", []string{"aws"}, aws("503", "AccessDenied"), false}, + {"no code", []string{"aws"}, "operation error EC2: X, https response error StatusCode: 403", false}, + {"unlisted code", []string{"aws"}, aws("400", "SomethingOdd"), false}, + {"azure rec", []string{"azure"}, azure429, false}, + {"azure text on aws rec", []string{"aws"}, azure429, false}, + {"gcp rec", []string{"gcp"}, gcp, false}, + {"mixed recs", []string{"aws", "gcp"}, aws("403", "AccessDenied"), false}, + {"server note", []string{"aws"}, "history_write_failed: commitment c purchased but its history record failed to save", false}, + {"unknown outcome wrapper", []string{"aws"}, "purchase outcome unknown: the request may have been sent; reconcile before retrying: " + aws("400", "Throttling"), false}, + {"one unknown segment", []string{"aws", "aws"}, "a: " + aws("403", "AccessDenied") + "; b: purchase outcome unknown", false}, + {"semicolon in message", []string{"aws"}, aws("400", "InvalidParameterValue") + "; reserved elsewhere", false}, + {"no recs", nil, aws("403", "AccessDenied"), false}, + {"empty", []string{"aws"}, "", false}, + } + for _, c := range cases { + var recs []config.RecommendationRecord + for _, p := range c.recs { + recs = append(recs, config.RecommendationRecord{Provider: p}) + } + assert.Equal(t, c.want, IsDefiniteAWSRefusal(recs, c.err), c.name) + } +} + +func TestClassifyStoredText_ReadsEveryGoogleReason(t *testing.T) { + multi := &googleapi.Error{Code: 403, Message: "m", Errors: []googleapi.ErrorItem{{Reason: "x", Message: "a"}, {Reason: "rateLimitExceeded", Message: "b"}}} + assert.Contains(t, multi.Error(), "Reason: rateLimitExceeded") + assert.Equal(t, ErrCategoryLimit, classifyStoredText(multi.Error())) + cat, _ := SummarizeProviderError(multi) + assert.Equal(t, ErrCategoryLimit, cat) +} + +// After an accepted Insert the commitment may exist, so a failed wait is never +// a definite category. +func TestPostInsertWaitFailure_IsUnconfirmed(t *testing.T) { + for _, g := range []*googleapi.Error{ + {Code: 404, Message: "operations/op-1 was not found", Errors: []googleapi.ErrorItem{{Reason: "notFound", Message: "operations/op-1 was not found"}}}, + {Code: 403, Message: "denied", Errors: []googleapi.ErrorItem{{Reason: "forbidden", Message: "denied"}}}, + } { + err := fmt.Errorf("%w: commitment creation failed: %w", errors.New("retry: permanent error, do not retry"), g) + cat, msg := SummarizeProviderError(err) + assert.Equal(t, ErrCategoryUnknown, cat) + assert.Contains(t, msg, "outcome could not be confirmed") + assert.Equal(t, ErrCategoryUnknown, classifyStoredText(err.Error())) + assert.Contains(t, SanitizeStoredError(err.Error()), "outcome could not be confirmed") + assert.Contains(t, SanitizeStoredError(partialRunPrefix+"x: "+err.Error()), "outcome could not be confirmed") + assert.NotContains(t, SanitizeStoredError(err.Error()), "not available") + } +} diff --git a/internal/purchase/errsummary_test.go b/internal/purchase/errsummary_test.go new file mode 100644 index 000000000..409d70cef --- /dev/null +++ b/internal/purchase/errsummary_test.go @@ -0,0 +1,69 @@ +package purchase + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/aws/smithy-go" + "github.com/stretchr/testify/assert" + "google.golang.org/api/googleapi" +) + +const fakeAWS403 = "operation error EC2: PurchaseReservedInstancesOffering, https response error StatusCode: 403, " + + "RequestID: 11111111-2222-3333-4444-555555555555, api error UnauthorizedOperation: You are not authorized to perform this operation. " + + "User: arn:aws:sts::123456789012:assumed-role/cudly-role/session is not authorized. " + + "Encoded authorization failure message: AbCdEfGhIjKlMnOpQrStUvWxYz0123456789" + +func TestSanitizeStoredError_AWS403(t *testing.T) { + out := SanitizeStoredError(fakeAWS403) + assert.Equal(t, ErrCategoryMessage(ErrCategoryAccessDenied), out) + for _, bad := range []string{"arn:", "123456789012", "RequestID", "Encoded"} { + assert.NotContains(t, out, bad) + } +} + +func TestSanitizeStoredError_Samples(t *testing.T) { + cases := map[string]string{ + "RESPONSE 403: 403 Forbidden\nERROR CODE: AuthorizationFailed": ErrCategoryAccessDenied, + "googleapi: Error 403: Required permission denied, forbidden": ErrCategoryAccessDenied, + "operation error EC2: X, api error InsufficientInstanceCapacity: none": ErrCategoryUnavailable, + "operation error RDS: X, api error ReservedDBInstanceQuotaExceeded: no": ErrCategoryLimit, + "operation error EC2: X, api error SomethingOdd: boom": ErrCategoryUnknown, + } + for in, cat := range cases { + assert.Equal(t, ErrCategoryMessage(cat), SanitizeStoredError(in), in) + } +} + +func TestSanitizeStoredError_PartialRun(t *testing.T) { + in := partialRunPrefix + "rec a: " + fakeAWS403 + "; rec b failed locally; " + "api error LimitExceeded: x arn:aws:iam::123456789012:role/r" + out := SanitizeStoredError(in) + assert.True(t, strings.HasPrefix(out, partialRunPrefix)) + assert.Equal(t, partialRunPrefix+ErrCategoryMessage(ErrCategoryAccessDenied), out) + assert.NotContains(t, out, "123456789012") +} + +func TestSummarizeProviderError_Typed(t *testing.T) { + cases := []struct { + err error + cat string + }{ + {&smithy.GenericAPIError{Code: "UnauthorizedOperation"}, ErrCategoryAccessDenied}, + {&smithy.GenericAPIError{Code: "AccessDenied"}, ErrCategoryAccessDenied}, + {&smithy.GenericAPIError{Code: "ReservedInstancesLimitExceeded"}, ErrCategoryLimit}, + {&smithy.GenericAPIError{Code: "InsufficientInstanceCapacity"}, ErrCategoryUnavailable}, + {&azcore.ResponseError{ErrorCode: "AuthorizationFailed", StatusCode: 403}, ErrCategoryAccessDenied}, + {&googleapi.Error{Code: 404}, ErrCategoryUnavailable}, + {fmt.Errorf("wrapped: %w", &googleapi.Error{Code: 403}), ErrCategoryAccessDenied}, + {errors.New("boom"), ErrCategoryUnknown}, + } + for _, c := range cases { + cat, msg := SummarizeProviderError(c.err) + assert.Equal(t, c.cat, cat, c.err.Error()) + assert.Equal(t, ErrCategoryMessage(c.cat), msg) + assert.NotEmpty(t, msg) + } +} diff --git a/internal/purchase/manager.go b/internal/purchase/manager.go index c3630c68d..1f75a81b0 100644 --- a/internal/purchase/manager.go +++ b/internal/purchase/manager.go @@ -587,6 +587,12 @@ func RedriveRefusalReason(exec *config.PurchaseExecution) string { return "" } +// Fixed refusal texts, shared with the stored-error allowlist (errnotes.go). +const ( + redriveReasonAWSNoGuard = "AWS EC2 and Redshift purchases have no provider-side duplicate guard, and a lost response means the earlier attempt may already have bought the commitment. Check EC2 Reserved Instances or Redshift Reserved Nodes in the target account and region; if it is missing, start a new purchase instead of retrying" + redriveReasonAzureSavingsPlan = "Azure savings plans have no provider-side duplicate guard, so re-driving this purchase would buy a second savings plan that cannot be canceled" +) + // recRedriveRefusalReason returns the reason a single recommendation cannot be // safely re-driven, or "" when it can. Extracted from RedriveRefusalReason to // keep that function under the gocyclo budget and to make per-rec exclusions @@ -601,7 +607,7 @@ func recRedriveRefusalReason(rec config.RecommendationRecord) string { // "purchase outcome unknown". The commitment may exist, so a second // attempt must not be made until someone has checked the account. if awsPurchaseHasNoDuplicateGuard(rec.Service) { - return "AWS EC2 and Redshift purchases have no provider-side duplicate guard, and a lost response means the earlier attempt may already have bought the commitment. Check EC2 Reserved Instances or Redshift Reserved Nodes in the target account and region; if it is missing, start a new purchase instead of retrying" + return redriveReasonAWSNoGuard } return "" case "azure": @@ -610,7 +616,7 @@ func recRedriveRefusalReason(rec config.RecommendationRecord) string { // re-drive may create a duplicate. All other Azure services use // DoIdempotentPurchaseTwoStep (#729). if rec.Service == "savingsplans" || rec.Service == "savings-plans" { - return "Azure savings plans have no provider-side duplicate guard, so re-driving this purchase would buy a second savings plan that cannot be canceled" + return redriveReasonAzureSavingsPlan } return "" case "gcp":