diff --git a/cmd/gcp_cud_dedupe_2121_test.go b/cmd/gcp_cud_dedupe_2121_test.go new file mode 100644 index 000000000..4b4297885 --- /dev/null +++ b/cmd/gcp_cud_dedupe_2121_test.go @@ -0,0 +1,102 @@ +package main + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" + "github.com/LeanerCloud/cloud-commitments-go/providers/gcp/services/computeengine" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Probe the library interface at runtime so this test also compiles with the old pins. +type gcpRecentCommitmentFilter interface { + FilterRecommendationsForRecentCommitments(recs []common.Recommendation, existing []common.Commitment) (passed, filtered []common.Recommendation, err error) +} + +type fakeGCPServiceClient struct { + libraryClient any + commitments []common.Commitment +} + +func (f *fakeGCPServiceClient) GetServiceType() common.ServiceType { return common.ServiceCompute } +func (f *fakeGCPServiceClient) GetRegion() string { return "us-central1" } + +func (f *fakeGCPServiceClient) GetRecommendations(context.Context, *common.RecommendationParams) ([]common.Recommendation, error) { + return nil, errors.New("not used by this test") +} + +func (f *fakeGCPServiceClient) GetExistingCommitments(context.Context) ([]common.Commitment, error) { + return f.commitments, nil +} + +func (f *fakeGCPServiceClient) PurchaseCommitment(context.Context, common.Recommendation, common.PurchaseOptions) (common.PurchaseResult, error) { + return common.PurchaseResult{}, errors.New("purchase must never be attempted by the duplicate checker") +} + +func (f *fakeGCPServiceClient) ValidateOffering(context.Context, common.Recommendation) error { + return errors.New("not used by this test") +} + +func (f *fakeGCPServiceClient) GetOfferingDetails(context.Context, common.Recommendation) (*common.OfferingDetails, error) { + return nil, errors.New("not used by this test") +} + +func (f *fakeGCPServiceClient) GetValidResourceTypes(context.Context) ([]string, error) { + return nil, errors.New("not used by this test") +} + +func (f *fakeGCPServiceClient) FilterRecommendationsForRecentCommitments(recs []common.Recommendation, existing []common.Commitment) ([]common.Recommendation, []common.Recommendation, error) { + filter, ok := f.libraryClient.(gcpRecentCommitmentFilter) + if !ok { + return nil, nil, errors.New("pinned gcp provider client has no recent-commitment filter (pre-#155 library)") + } + return filter.FilterRecommendationsForRecentCommitments(recs, existing) +} + +var _ provider.ServiceClient = (*fakeGCPServiceClient)(nil) + +func TestDuplicateChecker_RecentGCPCUDSuppressesFamilyRetry_2121(t *testing.T) { + ctx := context.Background() + previousWindow := toolCfg.IdempotencyWindowHours + toolCfg.IdempotencyWindowHours = 0 + t.Cleanup(func() { toolCfg.IdempotencyWindowHours = previousWindow }) + + recentCUD := common.Commitment{ + Provider: common.ProviderGCP, + Account: "proj-1", + CommitmentID: "cud-recent-1", + CommitmentType: common.CommitmentCUD, + Service: common.ServiceCompute, + Region: "us-central1", + ResourceType: "GENERAL_PURPOSE_N2", // what GetExistingCommitments reports: the commitment Type + Count: 8, + State: common.CommitmentStateActive, + StartDate: time.Now().Add(-2 * time.Hour), + } + cudRec := func(machineType string) common.Recommendation { + return common.Recommendation{ + Provider: common.ProviderGCP, + Account: "proj-1", + Service: common.ServiceCompute, + CommitmentType: common.CommitmentCUD, + Region: "us-central1", + ResourceType: machineType, + Count: 2, + } + } + client := &fakeGCPServiceClient{ + libraryClient: &computeengine.Client{}, // zero value: the filter uses only its arguments + commitments: []common.Commitment{recentCUD}, + } + recs := []common.Recommendation{cudRec("n2-standard-4"), cudRec("n4-standard-4")} + drops := common.NewDropSummary() + adjusted := checkDuplicates(ctx, recs, client, false, drops) + assert.Equal(t, "Dropped 1 recs: duplicate-dedup=1", drops.FormatOneLine()) + require.Len(t, adjusted, 1, "a different commitment family must not be suppressed") + assert.Equal(t, "n4-standard-4", adjusted[0].ResourceType) +} diff --git a/cmd/purchase_safeguards_2121_test.go b/cmd/purchase_safeguards_2121_test.go new file mode 100644 index 000000000..e1d4ed8e1 --- /dev/null +++ b/cmd/purchase_safeguards_2121_test.go @@ -0,0 +1,270 @@ +package main + +import ( + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/aws/aws-sdk-go-v2/aws" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestApplyTargetCoverage_NonPositiveCountDropped_2121(t *testing.T) { + for _, count := range []int{0, -1} { + rec := common.Recommendation{ + Provider: common.ProviderAWS, + Service: common.ServiceRDS, + Region: "us-east-1", + ResourceType: "db.r6g.large", + CommitmentType: common.CommitmentReservedInstance, + Count: count, + AverageInstancesUsedPerHour: 4, + ExistingCoveragePct: 0, + CommitmentCost: 1000, + OnDemandCost: 2000, + EstimatedSavings: 500, + } + drops := common.NewDropSummary() + out := ApplyTargetCoverage([]common.Recommendation{rec}, 75, drops) + assert.Empty(t, out, "count=%d must be dropped, not scaled", count) + assert.Contains(t, drops.FormatOneLine(), "target-input-invalid=1", "count=%d", count) + } +} + +func TestApplyTargetCoverage_InvalidInputsDropped_2121(t *testing.T) { + rec := common.Recommendation{ + Provider: common.ProviderAWS, + Service: common.ServiceRDS, + Region: "us-east-1", + ResourceType: "db.r6g.large", + CommitmentType: common.CommitmentReservedInstance, + Count: 2, + AverageInstancesUsedPerHour: 4, + ExistingCoveragePct: -10, + CommitmentCost: 1000, + OnDemandCost: 2000, + EstimatedSavings: 500, + } + drops := common.NewDropSummary() + out := ApplyTargetCoverage([]common.Recommendation{rec}, 75, drops) + assert.Empty(t, out, "negative existing coverage must drop the rec") + assert.Contains(t, drops.FormatOneLine(), "target-input-invalid=1") +} + +func TestDuplicateChecker_ValkeyDoesNotCoverRedis_2121(t *testing.T) { + ctx := context.Background() + recent := time.Now().Add(-1 * time.Hour) + commitment := func(engine string) common.Commitment { + return common.Commitment{ + Provider: common.ProviderAWS, + Service: common.ServiceCache, + ResourceType: "cache.r6g.large", + Region: "us-east-1", + Engine: engine, + Count: 1, + State: common.CommitmentStateActive, + StartDate: recent, + } + } + rec := func(engine string) common.Recommendation { + return common.Recommendation{ + Provider: common.ProviderAWS, + Service: common.ServiceElastiCache, + ResourceType: "cache.r6g.large", + Region: "us-east-1", + Count: 1, + Details: &common.CacheDetails{Engine: engine}, + } + } + t.Run("recent valkey reservation keeps redis recommendation", func(t *testing.T) { + mockClient := &MockServiceClient{} + mockClient.On("GetExistingCommitments", ctx). + Return([]common.Commitment{commitment("valkey")}, nil) + passed, filtered, err := NewDuplicateChecker(0). + AdjustRecommendationsForExisting(ctx, []common.Recommendation{rec("redis")}, mockClient) + require.NoError(t, err) + require.Len(t, passed, 1, "valkey reservation must not suppress a redis recommendation") + assert.Equal(t, 1, passed[0].Count) + assert.Empty(t, filtered) + }) + t.Run("recent redis reservation still suppresses valkey recommendation", func(t *testing.T) { + mockClient := &MockServiceClient{} + mockClient.On("GetExistingCommitments", ctx). + Return([]common.Commitment{commitment("redis")}, nil) + passed, filtered, err := NewDuplicateChecker(0). + AdjustRecommendationsForExisting(ctx, []common.Recommendation{rec("valkey")}, mockClient) + require.NoError(t, err) + assert.Empty(t, passed, "redis OSS reservations cover valkey nodes") + require.Len(t, filtered, 1) + }) + t.Run("recent reservation with missing engine still wildcards", func(t *testing.T) { + mockClient := &MockServiceClient{} + mockClient.On("GetExistingCommitments", ctx). + Return([]common.Commitment{commitment("")}, nil) + passed, filtered, err := NewDuplicateChecker(0). + AdjustRecommendationsForExisting(ctx, []common.Recommendation{rec("redis")}, mockClient) + require.NoError(t, err) + assert.Empty(t, passed, "a reservation with unknown engine matches any cache engine") + require.Len(t, filtered, 1) + }) +} + +func TestDuplicateChecker_UnknownReservationStateRemainsOwned_2121(t *testing.T) { + ctx := context.Background() + recs := []common.Recommendation{{ + Provider: common.ProviderAWS, + Service: common.ServiceRDS, + ResourceType: "db.r6g.large", + Region: "us-east-1", + Count: 1, + Details: &common.DatabaseDetails{Engine: "mysql"}, + }} + commitment := func(state common.CommitmentState) common.Commitment { + return common.Commitment{ + Provider: common.ProviderAWS, + Service: common.ServiceRDS, + ResourceType: "db.r6g.large", + Region: "us-east-1", + Engine: "mysql", + Count: 1, + State: state, + StartDate: time.Now().Add(-1 * time.Hour), + } + } + t.Run("unrecognized state suppresses", func(t *testing.T) { + mockClient := &MockServiceClient{} + mockClient.On("GetExistingCommitments", ctx). + Return([]common.Commitment{commitment("some-future-state")}, nil) + passed, filtered, err := NewDuplicateChecker(0). + AdjustRecommendationsForExisting(ctx, recs, mockClient) + require.NoError(t, err) + assert.Empty(t, passed, "unknown states stay owned: skipping a purchase is recoverable, a duplicate is not") + require.Len(t, filtered, 1) + }) + t.Run("retired state does not suppress", func(t *testing.T) { + mockClient := &MockServiceClient{} + mockClient.On("GetExistingCommitments", ctx). + Return([]common.Commitment{commitment(common.CommitmentStateRetired)}, nil) + passed, filtered, err := NewDuplicateChecker(0). + AdjustRecommendationsForExisting(ctx, recs, mockClient) + require.NoError(t, err) + require.Len(t, passed, 1) + assert.Empty(t, filtered) + }) +} + +func TestExecutePurchase_PreservesExplicitZeroVsAbsentCost_2121(t *testing.T) { + ctx := context.Background() + rec := common.Recommendation{ + Provider: common.ProviderAWS, + Service: common.ServiceEC2, + ResourceType: "m6i.large", + Region: "us-east-1", + Count: 2, + } + t.Run("explicit zero upfront cost stays a non-nil zero", func(t *testing.T) { + zero := 0.0 + mockClient := &MockServiceClient{} + mockClient.On("PurchaseCommitment", ctx, rec, mock.Anything). + Return(common.PurchaseResult{Recommendation: rec, Success: true, Cost: &zero}, nil) + result := executePurchase(ctx, rec, rec.Region, 1, mockClient, toolCfg) + require.True(t, result.Success) + require.NotNil(t, result.Cost, "explicit zero must not collapse to absent") + assert.Equal(t, 0.0, *result.Cost) + data, err := json.Marshal(result) + require.NoError(t, err) + assert.Contains(t, string(data), `"cost":0`) + }) + t.Run("absent upfront cost stays nil", func(t *testing.T) { + mockClient := &MockServiceClient{} + mockClient.On("PurchaseCommitment", ctx, rec, mock.Anything). + Return(common.PurchaseResult{Recommendation: rec, Success: true, Cost: nil}, nil) + result := executePurchase(ctx, rec, rec.Region, 1, mockClient, toolCfg) + require.True(t, result.Success) + assert.Nil(t, result.Cost, "unknown cost must not be invented as zero") + data, err := json.Marshal(result) + require.NoError(t, err) + assert.Contains(t, string(data), `"cost":null`) + }) +} + +func TestWritePurchaseAuditRecord_ZeroAndUnknownCost_2121(t *testing.T) { + auditPath := filepath.Join(t.TempDir(), "audit.jsonl") + rec := common.Recommendation{ + Provider: common.ProviderAWS, + Service: common.ServiceRDS, + ResourceType: "db.r6g.large", + Region: "us-east-1", + Count: 1, + } + zero := 0.0 + + cases := []struct { + name string + result common.PurchaseResult + status string + }{ + {"explicit zero cost success", common.PurchaseResult{Recommendation: rec, Success: true, CommitmentID: "ri-zero", Cost: &zero}, "success"}, + {"absent cost success", common.PurchaseResult{Recommendation: rec, Success: true, CommitmentID: "ri-unknown"}, "success"}, + {"provider error stays fail-closed", common.PurchaseResult{Recommendation: rec, Success: false, Error: errors.New("throttling")}, "error"}, + } + + for _, tc := range cases { + writePurchaseAuditRecord("run-2121", rec, tc.result, tc.status, false, auditPath) + } + data, err := os.ReadFile(auditPath) //nolint:gosec // G304: test-controlled temp path + require.NoError(t, err) + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + require.Len(t, lines, len(cases)) + for i, tc := range cases { + var record common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(lines[i]), &record), tc.name) + assert.Equal(t, tc.status, record.Status, tc.name) + assert.Equal(t, "run-2121", record.RunID, tc.name) + } +} + +// Dry-run mode creates no service clients and makes no purchases. +func TestExecutePurchasePipeline_InterruptsFailClosed_2121(t *testing.T) { + ctx := context.Background() + recs := []common.Recommendation{ + {Provider: common.ProviderAWS, Service: common.ServiceRDS, ResourceType: "db.r6g.large", Region: "us-east-1", Count: 1}, + {Provider: common.ProviderAWS, Service: common.ServiceRDS, ResourceType: "db.r6g.xlarge", Region: "us-east-1", Count: 2}, + } + t.Run("shutdown before start purchases nothing", func(t *testing.T) { + auditPath := filepath.Join(t.TempDir(), "audit.jsonl") + shutdownRequested.Store(true) + defer shutdownRequested.Store(false) + results := executePurchasePipeline(ctx, aws.Config{}, recs, true, "run-2121-int", Config{AuditLog: auditPath}) + assert.Empty(t, results, "a requested shutdown must stop the pipeline before the first purchase") + _, err := os.Stat(auditPath) + assert.True(t, os.IsNotExist(err), "no audit records without purchase attempts") + }) + t.Run("uninterrupted dry run audits every rec as skipped", func(t *testing.T) { + auditPath := filepath.Join(t.TempDir(), "audit.jsonl") + results := executePurchasePipeline(ctx, aws.Config{}, recs, true, "run-2121-dry", Config{AuditLog: auditPath}) + require.Len(t, results, len(recs)) + for _, result := range results { + assert.True(t, result.Success) + assert.True(t, result.DryRun) + } + data, err := os.ReadFile(auditPath) //nolint:gosec // G304: test-controlled temp path + require.NoError(t, err) + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + require.Len(t, lines, len(recs)) + for _, line := range lines { + var record common.AuditRecord + require.NoError(t, json.Unmarshal([]byte(line), &record)) + assert.Equal(t, "skipped", record.Status) + assert.True(t, record.DryRun) + } + }) +} diff --git a/go.mod b/go.mod index 649c56986..f37b27dac 100644 --- a/go.mod +++ b/go.mod @@ -83,9 +83,9 @@ require ( require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2 v2.2.0 github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261006104817-90e61e668b99 - github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261005004231-945a4045d11f + github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261006205158-7ff8c1aee1bb github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261007133317-58c25f04c49b - github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928214714-ce9513612901 + github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261006205158-7ff8c1aee1bb github.com/aws/aws-sdk-go-v2/service/organizations v1.45.3 github.com/aws/aws-sdk-go-v2/service/secretsmanager v1.40.3 github.com/google/uuid v1.6.0 diff --git a/go.sum b/go.sum index 4a8d06f5c..26dd82266 100644 --- a/go.sum +++ b/go.sum @@ -80,12 +80,12 @@ 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-20261006104817-90e61e668b99 h1:inu/nYSAVY6cLsTn4qc64RIbCsymURLlEdgkhha79cw= github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261006104817-90e61e668b99/go.mod h1:ApWBliDXe099f3oDXBz41K/I9v4bHvn1dG/BGoRmHlw= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261005004231-945a4045d11f h1:c3K60juwiY6t25Gr0HWAxajIDsXH4LzDO4e3ENGAsAM= -github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261005004231-945a4045d11f/go.mod h1:wpW9/TvGFUUOqBEleJL639loh6Ue5aVIcZKdubRnYPQ= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261006205158-7ff8c1aee1bb h1:gtfWXNBATJDSS+q3+5Jxt2+HYijOSVEnpGkXmeCkudY= +github.com/LeanerCloud/cloud-commitments-go/providers/aws v0.0.0-20261006205158-7ff8c1aee1bb/go.mod h1:zgv5QhLOIk9Hqw1xcmr1lSFg4ZeChwTCMwxETFVcfj4= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261007133317-58c25f04c49b h1:yurugmfdZjf+dqjfqfDyfoBR3IAiFUpMKQ8URnZnJzQ= github.com/LeanerCloud/cloud-commitments-go/providers/azure v0.0.0-20261007133317-58c25f04c49b/go.mod h1:iIvPuWLWd+SXqLHAQNdodYMUAAcq7PnwfScXWgARt5A= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928214714-ce9513612901 h1:i6OwXLUheudN3GfwnYXdKuEq8vPdE9qkNJ+r71LRLKA= -github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20260928214714-ce9513612901/go.mod h1:UnMqDe2mBUHHF6gnKAgJn9/JeXa7WZqQxfWPs4unGrg= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261006205158-7ff8c1aee1bb h1:AJ4nKDtttjiTDemo9PDp2esoc35ymVaiCSr0zIewwpY= +github.com/LeanerCloud/cloud-commitments-go/providers/gcp v0.0.0-20261006205158-7ff8c1aee1bb/go.mod h1:vLtjCWuAcgFc6vlY4l4wpWOYvlj4itk25qjreylWpDQ= github.com/aws/aws-sdk-go-v2 v1.41.5 h1:dj5kopbwUsVUVFgO4Fi5BIT3t4WyqIDjGKCangnV/yY= github.com/aws/aws-sdk-go-v2 v1.41.5/go.mod h1:mwsPRE8ceUUpiTgF7QmQIJ7lgsKUPQOUl3o72QBrE1o= github.com/aws/aws-sdk-go-v2/config v1.29.12 h1:Y/2a+jLPrPbHpFkpAAYkVEtJmxORlXoo5k2g1fa2sUo=