diff --git a/go.mod b/go.mod index dfe749e5..9d63b471 100644 --- a/go.mod +++ b/go.mod @@ -85,7 +85,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azkeys v1.4.0 github.com/Azure/azure-sdk-for-go/sdk/security/keyvault/azsecrets v1.4.0 github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260929105827-b3b4cb5e3d80 - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261002152209-006ef5c8d0a2 + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261003204812-9962786e0695 github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928214714-ce9513612901 github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928214714-ce9513612901 github.com/aws/aws-lambda-go v1.47.0 diff --git a/go.sum b/go.sum index 5fb9646e..85adcd71 100644 --- a/go.sum +++ b/go.sum @@ -94,8 +94,8 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapp github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc= github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260929105827-b3b4cb5e3d80 h1:wVKlMokfaME/Lw525Qz3F155R3nY4VmuaIyh+6d4sCU= github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20260929105827-b3b4cb5e3d80/go.mod h1:ApWBliDXe099f3oDXBz41K/I9v4bHvn1dG/BGoRmHlw= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261002152209-006ef5c8d0a2 h1:rYxXq0G0O2lkf8AB1ni9m4pVhIdb6Z0y52sTxS1RKIA= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261002152209-006ef5c8d0a2/go.mod h1:d4nsy61/Ptib0SxqMg0ble3yksVm3+QbtUtmGja82PA= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261003204812-9962786e0695 h1:DfNEBzFS7/MaBZljLGRRRUaarfnXXZyBE/6iozzp9/k= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261003204812-9962786e0695/go.mod h1:d4nsy61/Ptib0SxqMg0ble3yksVm3+QbtUtmGja82PA= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928214714-ce9513612901 h1:iSdHYdmGUjcjtSmgGZxptBzDuLrJ9KR2mh1/x7FNvSY= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20260928214714-ce9513612901/go.mod h1:zgCL/ozOkcZUDbEC7a2UwW+6MPcLoBlEU2WUZ0TORUs= github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928214714-ce9513612901 h1:i6OwXLUheudN3GfwnYXdKuEq8vPdE9qkNJ+r71LRLKA= diff --git a/internal/scheduler/recommendation_completeness_integration_test.go b/internal/scheduler/recommendation_completeness_integration_test.go index a679635c..fa8814fe 100644 --- a/internal/scheduler/recommendation_completeness_integration_test.go +++ b/internal/scheduler/recommendation_completeness_integration_test.go @@ -3,6 +3,7 @@ package scheduler import ( + "bytes" "context" "encoding/json" "fmt" @@ -15,6 +16,7 @@ import ( "testing" "time" + "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" awsprovider "github.com/LeanerCloud/cloud-commitments-go/providers/aws" "github.com/LeanerCloud/cloud-commitments-platform/internal/config" @@ -31,6 +33,8 @@ type completenessHTTP struct { details []string fallback []string failScope bool + spMode string + spFallback string rdsCalls atomic.Int32 unexpected atomic.Int32 mu sync.Mutex @@ -44,6 +48,8 @@ type completenessRequest struct { TermInYears string PaymentOption string SavingsPlansType string + NextPageToken string + AccountScope string } func (f *completenessHTTP) Do(req *http.Request) (*http.Response, error) { @@ -88,7 +94,9 @@ func (f *completenessHTTP) Do(req *http.Request) (*http.Response, error) { } } body = map[string]any{"Recommendations": []any{map[string]any{"RecommendationDetails": details}}} - case "AWSInsightsIndexService.GetSavingsPlansPurchaseRecommendation", "AWSInsightsIndexService.GetReservationCoverage": + case "AWSInsightsIndexService.GetSavingsPlansPurchaseRecommendation": + body, status = f.savingsPlansResponse(input) + case "AWSInsightsIndexService.GetReservationCoverage": default: f.unexpected.Add(1) return nil, fmt.Errorf("unexpected AWS operation %s", req.Header.Get("X-Amz-Target")) @@ -104,6 +112,37 @@ func (f *completenessHTTP) Do(req *http.Request) (*http.Response, error) { Body: io.NopCloser(strings.NewReader(string(raw))), Request: req}, nil } +func (f *completenessHTTP) savingsPlansResponse(input completenessRequest) (any, int) { + mode := f.spMode + if input.LookbackPeriodInDays == "THIRTY_DAYS" { + mode = f.spFallback + } + if mode == "failed type" && input.SavingsPlansType == "DATABASE_SP" || + mode == "late page" && input.NextPageToken == "next" { + return nil, http.StatusBadRequest + } + details := []any{} + if mode != "" && mode != "empty" { + commitments := []string{"2"} + if mode == "invalid" { + commitments = []string{"invalid"} + } else if mode == "mixed" && input.SavingsPlansType == "COMPUTE_SP" { + commitments = append(commitments, "invalid") + } + for _, commitment := range commitments { + details = append(details, map[string]any{"HourlyCommitmentToPurchase": commitment, + "EstimatedMonthlySavingsAmount": "10", "UpfrontCost": "3", + "CurrentAverageHourlyOnDemandSpend": "4", "EstimatedSavingsPercentage": "25"}) + } + } + body := map[string]any{"SavingsPlansPurchaseRecommendation": map[string]any{ + "SavingsPlansPurchaseRecommendationDetails": details}} + if mode == "late page" && input.SavingsPlansType == "COMPUTE_SP" { + body["NextPageToken"] = "next" + } + return body, http.StatusOK +} + func completenessProvider(t *testing.T, fixture *completenessHTTP) *MockProvider { t.Helper() client := awsprovider.NewRecommendationsClient(aws.Config{ @@ -124,13 +163,17 @@ func completenessProvider(t *testing.T, fixture *completenessHTTP) *MockProvider func TestAWSRecommendationCompletenessPersistence(t *testing.T) { for _, tc := range []struct { - name string - registered bool - details []string - fallback []string - failScope bool - complete bool - wantRows int + name string + registered bool + details []string + fallback []string + failScope bool + complete bool + wantRows int + spMode string + spFallback string + failedDetails int + failedScopes int }{ {name: "ambient mixed", details: []string{"2", "invalid"}, wantRows: 6}, {name: "ambient all invalid", details: []string{"invalid"}}, @@ -143,9 +186,25 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { {name: "ambient failed API scope", details: []string{"2"}, failScope: true, wantRows: 5}, {name: "registered mixed", registered: true, details: []string{"2", "invalid"}, wantRows: 6}, {name: "registered all invalid", registered: true, details: []string{"invalid"}}, + {name: "SP valid", spMode: "valid", complete: true, wantRows: 24}, + {name: "SP empty", spMode: "empty", complete: true}, + {name: "SP mixed", spMode: "mixed", wantRows: 24, failedDetails: 6}, + {name: "SP all invalid", spMode: "invalid", failedDetails: 24}, + {name: "SP failed type", spMode: "failed type", wantRows: 18, failedScopes: 6}, + {name: "SP late page", spMode: "late page", wantRows: 24, failedScopes: 6}, + {name: "SP invalid then clean fallback", spMode: "invalid", spFallback: "valid", fallback: []string{}, wantRows: 4, failedDetails: 24}, + {name: "SP empty then incomplete fallback", spMode: "empty", spFallback: "mixed", fallback: []string{}, wantRows: 4, failedDetails: 1}, + {name: "SP clean fallback", spMode: "empty", spFallback: "valid", fallback: []string{}, complete: true, wantRows: 4}, + {name: "SP empty fallback", spMode: "empty", spFallback: "empty", fallback: []string{}, complete: true}, + {name: "SP registered mixed", registered: true, spMode: "mixed", wantRows: 24, failedDetails: 6}, + {name: "SP registered failed type", registered: true, spMode: "failed type", wantRows: 18, failedScopes: 6}, + {name: "SP registered late page", registered: true, spMode: "late page", wantRows: 24, failedScopes: 6}, } { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() + var diagnostics bytes.Buffer + previous := logging.SetOutput(&diagnostics) + t.Cleanup(func() { logging.SetOutput(previous) }) container, err := testhelpers.SetupPostgresContainer(ctx, t) require.NoError(t, err) t.Cleanup(func() { container.Cleanup(ctx) }) @@ -154,11 +213,16 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { require.NoError(t, migrations.RunMigrations(ctx, container.DB.Pool(), migrationPath, "", "")) store := config.NewPostgresStore(container.DB) factory := new(MockProviderFactory) - fixture := &completenessHTTP{resourceType: "db.t3.medium", details: tc.details, fallback: tc.fallback, failScope: tc.failScope} + fixture := &completenessHTTP{resourceType: "db.t3.medium", details: tc.details, fallback: tc.fallback, failScope: tc.failScope, spMode: tc.spMode, spFallback: tc.spFallback} factory.On("CreateAndValidateProvider", mock.Anything, "aws", (*provider.ProviderConfig)(nil)). Return(completenessProvider(t, fixture), nil).Once() var accountID *string var completeAccountID string + uncollectedID := uuid.NewString() + if tc.spMode != "" && !tc.registered { + acct := config.CloudAccount{ID: uncollectedID, Name: "disabled", Provider: "aws", ExternalID: "333333333333", Enabled: false, AWSAuthMode: "role_arn"} + require.NoError(t, store.CreateCloudAccount(ctx, &acct)) + } if tc.registered { id := uuid.NewString() accountID = &id @@ -166,11 +230,16 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { for _, acct := range []config.CloudAccount{ {ID: id, Name: "incomplete", Provider: "aws", ExternalID: "111111111111", Enabled: true, AWSAuthMode: "role_arn"}, {ID: completeAccountID, Name: "complete", Provider: "aws", ExternalID: "222222222222", Enabled: true, AWSAuthMode: "role_arn", AWSRoleARN: "arn:aws:iam::222222222222:role/synthetic"}, + {ID: uncollectedID, Name: "disabled", Provider: "aws", ExternalID: "333333333333", Enabled: false, AWSAuthMode: "role_arn"}, } { require.NoError(t, store.CreateCloudAccount(ctx, &acct)) } + cleanFixture := &completenessHTTP{resourceType: "db.r5.large", details: []string{"2"}} + if tc.spMode != "" { + cleanFixture = &completenessHTTP{spMode: "valid"} + } factory.On("CreateAndValidateProvider", mock.Anything, "aws", mock.MatchedBy(func(cfg *provider.ProviderConfig) bool { return cfg != nil })). - Return(completenessProvider(t, &completenessHTTP{resourceType: "db.r5.large", details: []string{"2"}}), nil).Once() + Return(completenessProvider(t, cleanFixture), nil).Once() } seed := []config.RecommendationRecord{ {ID: "missing-offer", Provider: "aws", CloudAccountID: accountID, Service: "rds", Region: "us-east-1", ResourceType: "db.t3.large", Savings: 40, Count: 1, Term: 12, Payment: "no-upfront"}, @@ -179,6 +248,17 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { if tc.registered { seed = append(seed, config.RecommendationRecord{ID: "complete-stale", Provider: "aws", CloudAccountID: &completeAccountID, Service: "rds", Region: "us-east-1", ResourceType: "db.t3.large", Savings: 40, Count: 1, Term: 12, Payment: "no-upfront"}) + for _, scope := range []struct { + id string + account *string + }{{"uncollected", &uncollectedID}, {"ambient", nil}} { + seed = append(seed, config.RecommendationRecord{ID: scope.id, Provider: "aws", CloudAccountID: scope.account, + Service: "savings-plans-compute", Savings: 40, Count: 1, Term: 1, Payment: "no-upfront"}) + } + } + if tc.spMode != "" && !tc.registered { + seed = append(seed, config.RecommendationRecord{ID: "uncollected", Provider: "aws", CloudAccountID: &uncollectedID, + Service: "savings-plans-compute", Savings: 40, Count: 1, Term: 1, Payment: "no-upfront"}) } require.NoError(t, store.UpsertRecommendations(ctx, time.Now().Add(-time.Hour), seed, nil)) s := &Scheduler{config: store, providerFactory: factory} @@ -191,7 +271,11 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { wantRows := tc.wantRows switch { case tc.registered: - wantRows += 6 + if tc.spMode == "" { + wantRows += 6 + } else { + wantRows += 24 + } require.Equal(t, []string{completeAccountID}, ids) case tc.complete: require.Equal(t, []string{""}, ids) @@ -214,6 +298,31 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { require.Contains(t, byID, rec.ID, "surviving SDK recommendations must be persisted") require.Equal(t, rec.CloudAccountID, byID[rec.ID].CloudAccountID) } + if tc.spMode != "" { + assertSavingsPlansPersistence(t, rows, accountID, tc.wantRows, completeAccountID, tc.spMode, tc.fallback != nil) + require.Contains(t, byID, "uncollected") + if tc.failedDetails+tc.failedScopes > 0 { + require.Contains(t, diagnostics.String(), fmt.Sprintf("failed_details=%d failed_scopes=%d", tc.failedDetails, tc.failedScopes)) + } else { + require.NotContains(t, diagnostics.String(), "recommendations incomplete") + } + if tc.registered { + require.Contains(t, byID, "uncollected") + require.Contains(t, byID, "ambient") + for _, id := range []string{*accountID, completeAccountID} { + filtered, err := store.ListStoredRecommendations(ctx, config.RecommendationFilter{AccountIDs: []string{id}}) + require.NoError(t, err) + count := 24 + if id == *accountID { + count = tc.wantRows + 1 + } + require.Len(t, filtered, count) + for _, row := range filtered { + require.Equal(t, &id, row.CloudAccountID) + } + } + } + } fallbackServices := []string{} for _, request := range fixture.requests { if request.Operation == "AWSInsightsIndexService.GetReservationCoverage" { @@ -240,7 +349,107 @@ func TestAWSRecommendationCompletenessPersistence(t *testing.T) { } else { require.Empty(t, fallbackServices) } + if tc.spMode != "" { + assertSavingsPlansRequests(t, fixture.requests, tc.fallback != nil, tc.spMode == "late page") + } factory.AssertExpectations(t) }) } } + +func assertSavingsPlansPersistence(t *testing.T, rows []config.RecommendationRecord, accountID *string, want int, cleanID, mode string, fallback bool) { + t.Helper() + count := 0 + actual := []string{} + for _, row := range rows { + if !strings.HasPrefix(row.ID, "aws|") || !strings.HasPrefix(row.Service, "savings-plans-") { + continue + } + scope := "" + if row.CloudAccountID != nil { + scope = *row.CloudAccountID + } + if scope != cleanID || cleanID == "" { + count++ + } + actual = append(actual, row.ID) + require.Contains(t, []string{"savings-plans-compute", "savings-plans-ec2instance", "savings-plans-sagemaker", "savings-plans-database"}, row.Service) + require.Empty(t, row.Region) + require.Empty(t, row.ResourceType) + require.Empty(t, row.Engine) + require.Equal(t, fmt.Sprintf("aws|%s|%s||||%d|%s", scope, row.Service, row.Term, row.Payment), row.ID) + require.Contains(t, []int{1, 3}, row.Term) + require.Contains(t, []string{"no-upfront", "partial-upfront", "all-upfront"}, row.Payment) + require.Equal(t, 1, row.Count) + require.Equal(t, 10.0, row.Savings) + require.Equal(t, 3.0, row.UpfrontCost) + require.NotNil(t, row.OnDemandCost) + require.Equal(t, 2920.0, *row.OnDemandCost) + require.NotNil(t, row.SavingsPercentage) + require.Equal(t, 25.0, *row.SavingsPercentage) + require.NotNil(t, row.MonthlyCost) + monthly := 1460.0 + if row.Payment == "all-upfront" { + monthly = 0 + } + require.Equal(t, monthly, *row.MonthlyCost) + if cleanID == "" { + require.Equal(t, accountID, row.CloudAccountID) + } else { + require.Contains(t, []string{*accountID, cleanID}, scope) + } + } + require.Equal(t, want, count) + expected := []string{} + accounts := []string{""} + if accountID != nil { + accounts = []string{*accountID, cleanID} + } + for _, account := range accounts { + cleanSibling := cleanID != "" && account == cleanID + if want == 0 && !cleanSibling { + continue + } + for _, service := range []string{"savings-plans-compute", "savings-plans-ec2instance", "savings-plans-sagemaker", "savings-plans-database"} { + if mode == "failed type" && service == "savings-plans-database" && !cleanSibling { + continue + } + for _, term := range []int{1, 3} { + for _, payment := range []string{"no-upfront", "partial-upfront", "all-upfront"} { + if fallback && !cleanSibling && (term != 1 || payment != "no-upfront") { + continue + } + expected = append(expected, fmt.Sprintf("aws|%s|%s||||%d|%s", account, service, term, payment)) + } + } + } + } + require.ElementsMatch(t, expected, actual) +} + +func assertSavingsPlansRequests(t *testing.T, requests []completenessRequest, fallback, latePage bool) { + t.Helper() + actual := []string{} + for _, request := range requests { + if request.Operation != "AWSInsightsIndexService.GetSavingsPlansPurchaseRecommendation" { + continue + } + require.Equal(t, "LINKED", request.AccountScope) + actual = append(actual, strings.Join([]string{request.SavingsPlansType, request.TermInYears, request.PaymentOption, request.LookbackPeriodInDays, request.NextPageToken}, "/")) + } + expected := []string{} + for _, plan := range []string{"COMPUTE_SP", "EC2_INSTANCE_SP", "SAGEMAKER_SP", "DATABASE_SP"} { + for _, term := range []string{"ONE_YEAR", "THREE_YEARS"} { + for _, payment := range []string{"NO_UPFRONT", "PARTIAL_UPFRONT", "ALL_UPFRONT"} { + expected = append(expected, strings.Join([]string{plan, term, payment, "SEVEN_DAYS", ""}, "/")) + if latePage && plan == "COMPUTE_SP" { + expected = append(expected, strings.Join([]string{plan, term, payment, "SEVEN_DAYS", "next"}, "/")) + } + } + } + if fallback { + expected = append(expected, plan+"/ONE_YEAR/NO_UPFRONT/THIRTY_DAYS/") + } + } + require.ElementsMatch(t, expected, actual) +}