diff --git a/providers/gcp/go.mod b/providers/gcp/go.mod index 58bbf9a2..9380eeb0 100644 --- a/providers/gcp/go.mod +++ b/providers/gcp/go.mod @@ -8,7 +8,7 @@ require ( cloud.google.com/go/redis v1.18.3 cloud.google.com/go/resourcemanager v1.10.7 cloud.google.com/go/storage v1.59.2 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015085730-498b74f31f01 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261017012250-2bf23fbd8c2a github.com/googleapis/gax-go/v2 v2.21.0 github.com/stretchr/testify v1.11.1 golang.org/x/oauth2 v0.36.0 diff --git a/providers/gcp/go.sum b/providers/gcp/go.sum index cb059dab..9983261f 100644 --- a/providers/gcp/go.sum +++ b/providers/gcp/go.sum @@ -36,8 +36,8 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0 github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/cloudmock v0.54.0/go.mod h1:vB2GH9GAYYJTO3mEn8oYwzEdhlayZIdQz6zdzgUIRvA= github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.54.0 h1:s0WlVbf9qpvkh1c/uDAPElam0WrL7fHRIidgZJ7UqZI= 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-20261015085730-498b74f31f01 h1:R2EQAvwLAxPrdlZXmF+bI7ioQYh1CxBkethx5hq6WWA= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261015085730-498b74f31f01/go.mod h1:8RTmzPazA8IeVZfn7wNaU7ILx0VK5eHZQozO21Mvtq8= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261017012250-2bf23fbd8c2a h1:TggoJtZRb3MHRRhJ8Q1uu5SGGmhL1YbHiG08Xv9hqDw= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261017012250-2bf23fbd8c2a/go.mod h1:SCDEgHG6evjPh0RsoXof+J3aMmGXNFMrwbJIle7rMpQ= 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/smithy-go v1.24.2 h1:FzA3bu/nt/vDvmnkg+R8Xl46gmzEDam6mZ1hzmwXFng= diff --git a/providers/gcp/services/computeengine/client.go b/providers/gcp/services/computeengine/client.go index 7aae6db9..addb2791 100644 --- a/providers/gcp/services/computeengine/client.go +++ b/providers/gcp/services/computeengine/client.go @@ -870,7 +870,11 @@ func (c *Client) buildInsertRequest(rec common.Recommendation, opts common.Purch Region: c.region, CommitmentResource: commitment, } - if requestID := common.IdempotencyGUID(opts.IdempotencyToken); requestID != "" { + requestID, err := common.OrderIDFromToken(opts.IdempotencyToken, "") + if err != nil { + return nil, "", fmt.Errorf("derive GCP request ID: %w", err) + } + if requestID != "" { insertReq.RequestId = stringPtr(requestID) log.Printf("GCP CUD purchase using idempotent request ID for token %s (commitment %s); a re-drive will not double-purchase (issue #654)", common.MaskToken(opts.IdempotencyToken), commitmentName) @@ -1438,7 +1442,7 @@ func idempotentCommitmentName(token string) (string, error) { return fmt.Sprintf("cud-%d", time.Now().Unix()), nil } if !idempotencyTokenPattern.MatchString(token) { - return "", fmt.Errorf("%w: must be a lowercase hex digest to derive a GCP commitment name (got %d chars)", errInvalidIdempotencyToken, len(token)) + return "", fmt.Errorf("%w: must be a lowercase hex digest to derive a GCP commitment name (got %d chars)", common.ErrInvalidIdempotencyToken, len(token)) } if len(token) > idempotentNameTokenLen { token = token[:idempotentNameTokenLen] @@ -1446,8 +1450,6 @@ func idempotentCommitmentName(token string) (string, error) { return "cud-" + token, nil } -var errInvalidIdempotencyToken = errors.New("invalid idempotency token") - var idempotencyTokenPattern = regexp.MustCompile(`^[0-9a-f]+$`) // Helper functions. diff --git a/providers/gcp/services/computeengine/client_test.go b/providers/gcp/services/computeengine/client_test.go index 1fb69143..b96299b7 100644 --- a/providers/gcp/services/computeengine/client_test.go +++ b/providers/gcp/services/computeengine/client_test.go @@ -2442,7 +2442,7 @@ func TestPurchaseCommitment_RejectsNonHexIdempotencyToken(t *testing.T) { result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{IdempotencyToken: token}) require.Error(t, err) - assert.ErrorIs(t, err, errInvalidIdempotencyToken) + assert.ErrorIs(t, err, common.ErrInvalidIdempotencyToken) assert.Contains(t, err.Error(), "lowercase hex digest") assert.False(t, result.Success) assert.Empty(t, mockSvc.insertReqs, "Insert must not be called for an invalid token") @@ -2450,6 +2450,31 @@ func TestPurchaseCommitment_RejectsNonHexIdempotencyToken(t *testing.T) { } } +// A short or partly non-hex token passes the name check but cannot form a +// RequestId; it must fail before Insert instead of dropping idempotency. +func TestPurchaseCommitment_UnformableRequestIDTokenFailsLoud(t *testing.T) { + ctx := context.Background() + rec := common.Recommendation{ + ResourceType: "n1-standard-1", + Term: "1yr", + Count: 5, + Details: common.ComputeDetails{MemoryGB: 20.0}, + } + for _, token := range []string{"abc", strings.Repeat("a", 31)} { + t.Run(token, func(t *testing.T) { + client, _ := NewClient(ctx, "test-project", "us-central1") + mockSvc := &MockCommitmentsService{operation: &MockOperation{}} + client.SetCommitmentsService(mockSvc) + + result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{IdempotencyToken: token}) + require.Error(t, err) + assert.ErrorIs(t, err, common.ErrInvalidIdempotencyToken) + assert.False(t, result.Success) + assert.Empty(t, mockSvc.insertReqs, "Insert must not be called") + }) + } +} + func TestBuildInsertRequest_RejectedTokenWrapsSentinel(t *testing.T) { client, _ := NewClient(context.Background(), "test-project", "us-central1") rec := common.Recommendation{ @@ -2459,7 +2484,7 @@ func TestBuildInsertRequest_RejectedTokenWrapsSentinel(t *testing.T) { Details: common.ComputeDetails{MemoryGB: 20.0}, } _, _, err := client.buildInsertRequest(rec, common.PurchaseOptions{IdempotencyToken: "NOT-HEX"}) - require.ErrorIs(t, err, errInvalidIdempotencyToken) + require.ErrorIs(t, err, common.ErrInvalidIdempotencyToken) } // A full 64-char SHA-256 digest must be truncated so the name stays within