fix(deps): adopt reviewed cloud-commitments-go purchase safeguards

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)
+}

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)
+		}
+	})
+}

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