From 2e2629036f3645e7849815e0e5d9a86e7fee3bfe Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 19:43:06 +0100 Subject: [PATCH 1/4] build(azure): pin pkg at the version that has ErrRefusedBeforePurchase providers/azure pinned pkg at 6c5673cd, which predates common.ErrRefusedBeforePurchase (#673). Move it to 3efa339480ac, the version providers/aws already pins, so azure can wrap the sentinel. The pkg range adds the sentinel, field reorders in ladder and recfilter types (no behaviour change) and the /pkg dependency group bump, which also moves the indirect aws-sdk-go-v2 and smithy-go to match aws. Refs #538 --- providers/azure/go.mod | 6 +++--- providers/azure/go.sum | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/providers/azure/go.mod b/providers/azure/go.mod index 09507a8f..a30de384 100644 --- a/providers/azure/go.mod +++ b/providers/azure/go.mod @@ -15,7 +15,7 @@ require ( github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions v1.3.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/search/armsearch v1.4.0 github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql v1.2.0 - github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261026141505-6c5673cd5518 + github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104063122-3efa339480ac github.com/stretchr/testify v1.12.1 golang.org/x/sync v0.24.0 ) @@ -23,8 +23,8 @@ require ( require ( github.com/Azure/azure-sdk-for-go/sdk/internal v1.13.0 // indirect github.com/AzureAD/microsoft-authentication-library-for-go v1.10.1 // indirect - github.com/aws/aws-sdk-go-v2 v1.41.5 // indirect - github.com/aws/smithy-go v1.24.2 // indirect + github.com/aws/aws-sdk-go-v2 v1.47.4 // indirect + github.com/aws/smithy-go v1.28.6 // indirect github.com/golang-jwt/jwt/v5 v5.3.1 // indirect github.com/google/uuid v1.6.0 // indirect github.com/kylelemons/godebug v1.1.0 // indirect diff --git a/providers/azure/go.sum b/providers/azure/go.sum index 63e53b60..cfd17080 100644 --- a/providers/azure/go.sum +++ b/providers/azure/go.sum @@ -36,12 +36,12 @@ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1 h1:WJ github.com/AzureAD/microsoft-authentication-extensions-for-go/cache v0.1.1/go.mod h1:tCcJZ0uHAmvjsVYzEFivsRTN00oz5BEsRgQHu5JZ9WE= github.com/AzureAD/microsoft-authentication-library-for-go v1.10.1 h1:0O6j18nQDoIff6/mtosT5LNU2WehEwp+LqfX6699oqY= github.com/AzureAD/microsoft-authentication-library-for-go v1.10.1/go.mod h1:xdYAf5bjkOpsd3auA8riiJW4vneBubt9caavL7626jQ= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261026141505-6c5673cd5518 h1:2wyUUf+lBH+Qo3ULGiHOhn9drt+WyqLoR8fV/3q21a0= -github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261026141505-6c5673cd5518/go.mod h1:rEJErEsL40Np/ttf0RDoNNC2XQsN1GmWyFf34y0JLD8= -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= -github.com/aws/smithy-go v1.24.2/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104063122-3efa339480ac h1:EvEUCt1uH5oBtPcJJPt1mX6XYQWAKtL9QBSo9CfeaOw= +github.com/LeanerCloud/cloud-commitments-go/pkg v0.0.0-20261104063122-3efa339480ac/go.mod h1:pseCFRgm1rzBKomktjw6E0ayxmVE7XfHHiOmew4ka+g= +github.com/aws/aws-sdk-go-v2 v1.47.4 h1:bTvk6UdSSSvaQsfLVj1JRDEEqj/PDRwGf1KanaRzau0= +github.com/aws/aws-sdk-go-v2 v1.47.4/go.mod h1:XkrjAZvr705Nytv6PkPUhoSB2W9+vVHBp5oAWpkh8Ak= +github.com/aws/smithy-go v1.28.6 h1:Pj+/sUviNYjow8ELRtNwKHZ9ECeF+7pATX3+shjLIrw= +github.com/aws/smithy-go v1.28.6/go.mod h1:YE2RhdIuDbA5E5bTdciG9KrW3+TiEONeUWCqxX9i1Fc= github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY= github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= From 9628c2b52009398c8f5575b8bc8532d61bb1b3ba Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 19:43:14 +0100 Subject: [PATCH 2/4] test(azure): drop cache purchase tests ahead of its purchase refusal The cache service stops buying Azure Redis in the next commit, so its purchase tests no longer describe a supported path: - the cache re-drive tests (existing_commitment_test.go) - the cache rows of the shared purchase body and golden matrices - the first cache PurchaseCommitment success and error tests managedredis keeps the same RedisCache purchase coverage (success, 3yr, accepted, token, HTTP and status errors, billing plan, tags, display name, re-drive). The rest of the cache purchase tests go with the refusal. Refs #538 --- providers/azure/services/cache/client_test.go | 192 ------------------ .../cache/existing_commitment_test.go | 135 ------------ providers/azure/services/purchasebody_test.go | 2 - .../azure/services/purchasegolden_test.go | 4 - 4 files changed, 333 deletions(-) delete mode 100644 providers/azure/services/cache/existing_commitment_test.go diff --git a/providers/azure/services/cache/client_test.go b/providers/azure/services/cache/client_test.go index 29941057..9e6ed766 100644 --- a/providers/azure/services/cache/client_test.go +++ b/providers/azure/services/cache/client_test.go @@ -1066,198 +1066,6 @@ func calcPriceRespJSON(orderID string) string { return `{"properties":{"billingCurrencyTotal":{"currencyCode":"USD"},"reservationOrderId":"` + orderID + `"}}` } -func TestCacheClient_PurchaseCommitment_Success(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - calculateResp7 := mocks.CreateMockHTTPResponse(http.StatusOK, calcPriceRespJSON("cache-order-001")) - t.Cleanup(func() { - require.NoError(t, calculateResp7.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/calculatePrice" - })).Return(calculateResp7, nil).Once() - purchaseResp8 := mocks.CreateMockHTTPResponse(http.StatusOK, `{}`) - t.Cleanup(func() { - require.NoError(t, purchaseResp8.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/cache-order-001/purchase" - })).Return(purchaseResp8, nil).Once() - - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "1yr", - Count: 1, - CommitmentCost: 1000.0, - PaymentOption: "no-upfront", - } - - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.NoError(t, err) - assert.True(t, result.Success) - assert.Equal(t, "cache-order-001", result.CommitmentID) - require.NotNil(t, result.Cost) - assert.Zero(t, *result.Cost) - mockHTTP.AssertExpectations(t) -} - -func TestCacheClient_PurchaseCommitment_3YearTerm(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - calculateResp9 := mocks.CreateMockHTTPResponse(http.StatusOK, calcPriceRespJSON("cache-order-3yr")) - t.Cleanup(func() { - require.NoError(t, calculateResp9.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/calculatePrice" - })).Return(calculateResp9, nil).Once() - purchaseResp10 := mocks.CreateMockHTTPResponse(http.StatusCreated, `{}`) - t.Cleanup(func() { - require.NoError(t, purchaseResp10.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/cache-order-3yr/purchase" - })).Return(purchaseResp10, nil).Once() - - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "3yr", - Count: 1, - CommitmentCost: 2500.0, - PaymentOption: "no-upfront", - } - - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.NoError(t, err) - assert.True(t, result.Success) - assert.Equal(t, "cache-order-3yr", result.CommitmentID) - mockHTTP.AssertExpectations(t) -} - -func TestCacheClient_PurchaseCommitment_Accepted(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - calculateResp11 := mocks.CreateMockHTTPResponse(http.StatusOK, calcPriceRespJSON("cache-order-202")) - t.Cleanup(func() { - require.NoError(t, calculateResp11.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/calculatePrice" - })).Return(calculateResp11, nil).Once() - purchaseResp12 := mocks.CreateMockHTTPResponse(http.StatusAccepted, `{}`) - t.Cleanup(func() { - require.NoError(t, purchaseResp12.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/cache-order-202/purchase" - })).Return(purchaseResp12, nil).Once() - - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "1yr", - Count: 1, - CommitmentCost: 1000.0, - PaymentOption: "no-upfront", - } - - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.NoError(t, err) - assert.True(t, result.Success) - mockHTTP.AssertExpectations(t) -} - -func TestCacheClient_PurchaseCommitment_TokenError(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{err: errors.New("token error")} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "1yr", - Count: 1, - PaymentOption: "no-upfront", - } - - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.Error(t, err) - assert.False(t, result.Success) - assert.Contains(t, err.Error(), "failed to get access token") -} - -func TestCacheClient_PurchaseCommitment_HTTPError(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/calculatePrice" - })).Return(nil, errors.New("network error")).Once() - - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "1yr", - Count: 1, - PaymentOption: "no-upfront", - } - - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.Error(t, err) - assert.False(t, result.Success) - assert.Contains(t, err.Error(), "calculatePrice HTTP call") -} - -func TestCacheClient_PurchaseCommitment_BadStatus(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - calculateResp13 := mocks.CreateMockHTTPResponse(http.StatusOK, calcPriceRespJSON("cache-order-bad")) - t.Cleanup(func() { - require.NoError(t, calculateResp13.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/calculatePrice" - })).Return(calculateResp13, nil).Once() - purchaseResp14 := mocks.CreateMockHTTPResponse(http.StatusBadRequest, `{"error": "invalid request"}`) - t.Cleanup(func() { - require.NoError(t, purchaseResp14.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/cache-order-bad/purchase" - })).Return(purchaseResp14, nil).Once() - - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "1yr", - Count: 1, - PaymentOption: "no-upfront", - } - - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.Error(t, err) - assert.False(t, result.Success) - assert.Contains(t, err.Error(), "reservation purchase failed with status 400") - mockHTTP.AssertExpectations(t) -} - // TestCacheClient_PurchaseCommitment_TagInjection verifies that the // purchase-automation tag carrying opts.Source is present in the // calculatePrice request body. Without this regression test the dedupe diff --git a/providers/azure/services/cache/existing_commitment_test.go b/providers/azure/services/cache/existing_commitment_test.go deleted file mode 100644 index d8990103..00000000 --- a/providers/azure/services/cache/existing_commitment_test.go +++ /dev/null @@ -1,135 +0,0 @@ -package cache - -import ( - "context" - "io" - "net/http" - "strings" - "testing" - "time" - - "github.com/Azure/azure-sdk-for-go/sdk/azcore" - "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/LeanerCloud/cloud-commitments-go/pkg/common" -) - -// Issue #211: PurchaseResult.ExistingCommitment must be true only when the -// shared guard adopted an order already tagged with the idempotency token. - -type existingCommitmentCred struct{} - -func (existingCommitmentCred) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) { - return azcore.AccessToken{Token: "tok", ExpiresOn: time.Now().Add(time.Hour)}, nil -} - -// existingCommitmentHTTP is a stateless ARM stub: the reservation-orders list -// returns listBody, calculatePrice mints "minted-order", and the purchase POST -// answers purchaseStatus. posts counts calculatePrice and purchase calls. -type existingCommitmentHTTP struct { - listBody string - purchaseStatus int - posts int -} - -func (h *existingCommitmentHTTP) Do(req *http.Request) (*http.Response, error) { - respond := func(status int, body string) (*http.Response, error) { - return &http.Response{StatusCode: status, Body: io.NopCloser(strings.NewReader(body)), Header: http.Header{}}, nil - } - path := req.URL.Path - switch { - case req.Method == http.MethodGet && strings.HasSuffix(path, "/reservationOrders"): - return respond(http.StatusOK, h.listBody) - case req.Method == http.MethodGet: - return respond(http.StatusOK, `{"registrationState":"Registered"}`) - case strings.HasSuffix(path, "/calculatePrice"): - h.posts++ - return respond(http.StatusOK, `{"properties":{"billingCurrencyTotal":{"currencyCode":"USD"},"reservationOrderId":"minted-order"}}`) - case strings.HasSuffix(path, "/purchase"): - h.posts++ - return respond(h.purchaseStatus, `{}`) - } - return respond(http.StatusNotFound, `{}`) -} - -func existingCommitmentRun(t *testing.T, h *existingCommitmentHTTP) (common.PurchaseResult, error) { - t.Helper() - return existingCommitmentRunCurrency(t, h, "") -} - -func existingCommitmentRunCurrency(t *testing.T, h *existingCommitmentHTTP, currency string) (common.PurchaseResult, error) { - t.Helper() - c := NewClientWithHTTP(existingCommitmentCred{}, "sub", "eastus", h) - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "1yr", - Count: 1, - CommitmentCost: 1000.0, - PaymentOption: "no-upfront", - - CommitmentCurrency: currency, - } - return c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI, IdempotencyToken: "tok-211"}) -} - -func TestPurchaseCommitment_ExistingCommitment_FreshPurchaseNotFlagged(t *testing.T) { - t.Parallel() - h := &existingCommitmentHTTP{listBody: `{"value":[]}`, purchaseStatus: http.StatusOK} - result, err := existingCommitmentRun(t, h) - require.NoError(t, err) - assert.True(t, result.Success) - assert.False(t, result.ExistingCommitment) - assert.Equal(t, "minted-order", result.CommitmentID) - assert.Equal(t, 2, h.posts) -} - -func TestPurchaseCommitment_ExistingCommitment_FailedPurchaseNotFlagged(t *testing.T) { - t.Parallel() - h := &existingCommitmentHTTP{listBody: `{"value":[]}`, purchaseStatus: http.StatusInternalServerError} - result, err := existingCommitmentRun(t, h) - require.Error(t, err) - assert.False(t, result.Success) - assert.False(t, result.ExistingCommitment) -} - -func TestPurchaseCommitment_ExistingCommitment_RedriveAdoptsAndFlags(t *testing.T) { - t.Parallel() - h := &existingCommitmentHTTP{ - listBody: `{"value":[{"name":"adopted-order","properties":{"provisioningState":"Succeeded"},"tags":{"cudly-idempotency-token":"tok-211"}}]}`, - purchaseStatus: http.StatusOK, - } - result, err := existingCommitmentRun(t, h) - require.NoError(t, err) - assert.True(t, result.Success) - assert.True(t, result.ExistingCommitment) - assert.Equal(t, "adopted-order", result.CommitmentID) - assert.Zero(t, h.posts, "an adopted order must not trigger a second purchase") -} - -// A terminal-failed order carrying the token is not adopted, so the purchase -// proceeds and is not flagged. -func TestPurchaseCommitment_ExistingCommitment_FailedOrderNotAdopted(t *testing.T) { - t.Parallel() - h := &existingCommitmentHTTP{ - listBody: `{"value":[{"name":"dead-order","properties":{"provisioningState":"Failed"},"tags":{"cudly-idempotency-token":"tok-211"}}]}`, - purchaseStatus: http.StatusOK, - } - result, err := existingCommitmentRun(t, h) - require.NoError(t, err) - assert.False(t, result.ExistingCommitment) - assert.Equal(t, "minted-order", result.CommitmentID) -} - -// The recommendation currency must reach the shared purchase check: a EUR -// recommendation against a USD calculatePrice is refused before the purchase. -func TestPurchaseCommitment_RecommendationCurrencyMismatchRefused(t *testing.T) { - t.Parallel() - h := &existingCommitmentHTTP{listBody: `{"value":[]}`, purchaseStatus: http.StatusOK} - result, err := existingCommitmentRunCurrency(t, h, "EUR") - require.Error(t, err) - assert.Contains(t, err.Error(), "refusing to purchase") - assert.False(t, result.Success) - assert.Equal(t, 1, h.posts, "only calculatePrice; no purchase request") -} diff --git a/providers/azure/services/purchasebody_test.go b/providers/azure/services/purchasebody_test.go index e23a06b3..a45f5bac 100644 --- a/providers/azure/services/purchasebody_test.go +++ b/providers/azure/services/purchasebody_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/require" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" - "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cache" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cosmosdb" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/database" @@ -34,7 +33,6 @@ func TestPurchaseBodies(t *testing.T) { name, resourceType string build func(h *reservationstest.Capture) purchaser }{ - {"cache", "RedisCache", func(h *reservationstest.Capture) purchaser { return cache.NewClientWithHTTP(cred, "sub", "eastus", h) }}, {"compute", "VirtualMachines", func(h *reservationstest.Capture) purchaser { return compute.NewClientWithHTTP(cred, "sub", "eastus", h) }}, diff --git a/providers/azure/services/purchasegolden_test.go b/providers/azure/services/purchasegolden_test.go index 548e1f63..e7520842 100644 --- a/providers/azure/services/purchasegolden_test.go +++ b/providers/azure/services/purchasegolden_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" - "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cache" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/compute" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/cosmosdb" "github.com/LeanerCloud/cloud-commitments-go/providers/azure/services/database" @@ -52,9 +51,6 @@ type goldenCase struct { func goldenCases() []goldenCase { return []goldenCase{ - {"cache", "RedisCache", func(c *reservationstest.CountingCred, h *reservationstest.Capture) purchaser { - return cache.NewClientWithHTTP(c, "sub", "eastus", h) - }, "redis-eastus-GP_Gen5_8-2x-1yr-monthly-" + goldenMarker, regexp.MustCompile(`^redis-eastus-GP_Gen5_8-2x-1yr-monthly-\d{8}T\d{6}-[0-9a-f]{8}$`), nil}, {"compute", "VirtualMachines", func(c *reservationstest.CountingCred, h *reservationstest.Capture) purchaser { return compute.NewClientWithHTTP(c, "sub", "eastus", h) }, "vm-eastus-GP_Gen5_8-2x-1yr-monthly-" + goldenMarker, regexp.MustCompile(`^vm-eastus-GP_Gen5_8-2x-1yr-monthly-\d{8}T\d{6}-[0-9a-f]{8}$`), []string{methodGet + " " + urlProvider}}, From 135ffc1b47efe2f00f89eaec60b30b643cb2b635 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 5 Nov 2026 19:43:43 +0100 Subject: [PATCH 3/4] fix(azure/cache): refuse Redis purchases so only managedredis buys them cache and managedredis both purchased the RedisCache reservation type, so one Redis recommendation could be bought once through each service. Per the owner decision on #538 (option A), managedredis is the only buyer. cache.PurchaseCommitment now refuses before any credential or HTTP call, wrapping common.ErrRefusedBeforePurchase. - A test pins the refusal: the sentinel, the result fields, no HTTP call and no credential use. It failed against the old purchase flow. - The remaining cache purchase tests and their helpers go. - CHANGELOG: a Changed entry, and a Notice that the recommendations adapter still labels Redis recommendations ServiceCache, so consumers must route Azure Redis purchases to managedredis. Refs #538 --- CHANGELOG.md | 12 + providers/azure/services/cache/client.go | 80 +----- providers/azure/services/cache/client_test.go | 240 ------------------ .../services/cache/purchase_owner_test.go | 44 ++++ 4 files changed, 62 insertions(+), 314 deletions(-) create mode 100644 providers/azure/services/cache/purchase_owner_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 210d11d9..ac86ccc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Notices +- **Azure Redis purchases must go through managedredis (`ServiceMemoryDB`).** + The Azure recommendations adapter still labels Redis recommendations + `ServiceCache`, and the cache client now refuses every purchase (#538), so + route Azure Redis purchases to the managedredis client. The refusal wraps + `common.ErrRefusedBeforePurchase` (nothing was sent), not an upfront-approval + failure, so approving and retrying does not help. - **Federation IaC bundles downloaded before 2026-04-22 need to be re-downloaded** to get zero-touch registration. Older bundles silently skip auto-registration unless manually edited (Terraform `registration.tf` @@ -172,6 +178,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Changed +- **`azure`:** the cache service no longer purchases Azure Redis reservations; + managedredis (`ServiceMemoryDB`) is the only service that buys `RedisCache`, + so one Redis recommendation cannot be bought once through each service. + `cache.PurchaseCommitment` now refuses before any API call with an error + wrapping `common.ErrRefusedBeforePurchase`. Consumers that buy an Azure + `ServiceCache` recommendation must route it to managedredis (#538). - `ListTargetOfferings` now walks up to 50 pages (about 5000 offerings) instead of 10, and returns an error wrapping the new `ec2.ErrTargetOfferingsTruncated` (with no list) when that bound is hit, diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index 15650282..b11cfe45 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -13,7 +13,6 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis/v3" - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/LeanerCloud/cloud-commitments-go/pkg/common" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" @@ -300,79 +299,12 @@ func (c *Client) convertRedisReservation(detail *armconsumption.ReservationDetai return commitment } -// PurchaseCommitment purchases Redis Cache reserved capacity using the two-step -// calculatePrice->purchase flow required by Azure's Reservations API (issue #677). -func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) { - result := common.PurchaseResult{ - Recommendation: rec, - DryRun: false, - Success: false, - Timestamp: time.Now(), - } - - // Source is required so the resulting reservation is attributable to CUDly - // in the portal via the purchase-automation tag. The dedupe key for - // idempotent re-drives is now opts.IdempotencyToken (issue #721, applied - // in reservations.DoIdempotentPurchaseTwoStep); source remains mandatory - // for attribution. - if opts.Source == "" { - result.Error = fmt.Errorf("purchase source is required for Azure reservation purchases") - return result, result.Error - } - if strings.TrimSpace(rec.ResourceType) == "" { - result.Error = fmt.Errorf("resource type is required") - return result, result.Error - } - if rec.Count <= 0 { - result.Error = fmt.Errorf("quantity must be greater than zero, got %d", rec.Count) - return result, result.Error - } - - termYears, termErr := reservations.ParseTermYears(rec.Term) - if termErr != nil { - result.Error = termErr - return result, result.Error - } - if _, termEnumErr := reservations.ReservationTermForYears(termYears); termEnumErr != nil { - result.Error = termEnumErr - return result, result.Error - } - billingPlan, billingPlanErr := reservations.BillingPlanForPaymentOption(rec.PaymentOption) - if billingPlanErr != nil { - result.Error = billingPlanErr - return result, result.Error - } - - bodyBytes, err := reservations.BuildPurchaseBody(reservations.PurchaseParams{ - ReservedResourceType: string(armreservations.ReservedResourceTypeRedisCache), - DisplayName: func(r common.Recommendation) string { - return reservations.BuildDisplayName(reservations.DisplayNameFields{ - Service: "redis", - Region: c.region, - ResourceType: r.ResourceType, - Count: r.Count, - Term: r.Term, - Payment: r.PaymentOption, - Now: time.Now(), - }) - }, - }, rec, opts, c.region, c.subscriptionID, billingPlan) - if err != nil { - result.Error = fmt.Errorf("failed to marshal request: %w", err) - return result, result.Error - } - - reservationOrderID, existing, err := reservations.Submit(ctx, c.cred, c.httpClient, bodyBytes, opts, rec.CommitmentCurrency) - if err != nil { - result.Error = err - return result, result.Error - } - - result.Success = true - result.ExistingCommitment = existing - result.CommitmentID = reservationOrderID - result.Cost = reservations.UpfrontCostForBillingPlan(billingPlan) - return result, nil +// PurchaseCommitment refuses: managedredis (ServiceMemoryDB) is the only Azure service that +// buys RedisCache reservations (#538), so one Redis recommendation cannot be bought twice. +func (c *Client) PurchaseCommitment(_ context.Context, rec common.Recommendation, _ common.PurchaseOptions) (common.PurchaseResult, error) { + err := fmt.Errorf("%w: Azure Redis reservations are purchased through the %s service, not %s", + common.ErrRefusedBeforePurchase, common.ServiceMemoryDB, common.ServiceCache) + return common.PurchaseResult{Recommendation: rec, Timestamp: time.Now(), Error: err}, err } // ValidateOffering validates that a Redis Cache SKU exists. diff --git a/providers/azure/services/cache/client_test.go b/providers/azure/services/cache/client_test.go index 9e6ed766..69ccbf21 100644 --- a/providers/azure/services/cache/client_test.go +++ b/providers/azure/services/cache/client_test.go @@ -1,17 +1,11 @@ package cache import ( - "bytes" "context" - "encoding/json" "errors" - "io" "net/http" "testing" - "time" - "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/to" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis/v3" @@ -1045,240 +1039,6 @@ func TestCacheClient_CachedSKULookup_FetchedOnce(t *testing.T) { // Test the to package is properly imported (used in tests). var _ = to.Ptr("test") -// MockTokenCredential for testing PurchaseCommitment. -type MockTokenCredential struct { - token string - err error -} - -func (m *MockTokenCredential) GetToken(ctx context.Context, options policy.TokenRequestOptions) (azcore.AccessToken, error) { - if m.err != nil { - return azcore.AccessToken{}, m.err - } - return azcore.AccessToken{ - Token: m.token, - ExpiresOn: time.Now().Add(time.Hour), - }, nil -} - -// calcPriceRespJSON returns a minimal calculatePrice response JSON for tests. -func calcPriceRespJSON(orderID string) string { - return `{"properties":{"billingCurrencyTotal":{"currencyCode":"USD"},"reservationOrderId":"` + orderID + `"}}` -} - -// TestCacheClient_PurchaseCommitment_TagInjection verifies that the -// purchase-automation tag carrying opts.Source is present in the -// calculatePrice request body. Without this regression test the dedupe -// guard introduced for the Azure two-step flow could regress silently: -// the call would succeed without the tag and re-driven purchases would -// duplicate reservations server-side. -func TestCacheClient_PurchaseCommitment_TagInjection(t *testing.T) { - t.Parallel() - const orderID = "cache-tag-test" - const source = common.PurchaseSourceWeb - - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - var capturedBody []byte - calculateResp15 := mocks.CreateMockHTTPResponse(http.StatusOK, calcPriceRespJSON(orderID)) - t.Cleanup(func() { - require.NoError(t, calculateResp15.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - if r.URL.Path != "/providers/Microsoft.Capacity/calculatePrice" { - return false - } - capturedBody, _ = io.ReadAll(r.Body) - r.Body = io.NopCloser(bytes.NewReader(capturedBody)) - return true - })).Return(calculateResp15, nil).Once() - purchaseResp16 := mocks.CreateMockHTTPResponse(http.StatusOK, `{}`) - t.Cleanup(func() { - require.NoError(t, purchaseResp16.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/"+orderID+"/purchase" - })).Return(purchaseResp16, nil).Once() - - rec := common.Recommendation{ResourceType: "Premium_P1", Term: "1yr", Count: 1, CommitmentCost: 1000.0, PaymentOption: "no-upfront"} - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: source}) - require.NoError(t, err) - assert.True(t, result.Success) - - var body map[string]interface{} - require.NoError(t, json.Unmarshal(capturedBody, &body)) - tags, hasTags := body["tags"].(map[string]interface{}) - require.True(t, hasTags, "tags field must be present in calculatePrice body when Source is set") - assert.Equal(t, source, tags[common.PurchaseTagKey], "tag value must match opts.Source") - mockHTTP.AssertExpectations(t) -} - -// TestCacheClient_PurchaseCommitment_BillingPlan pins the billingPlan wiring -// (issue #1502, mirroring PR #1495's fix for compute): Azure Cache for Redis -// reserved capacity supports both Upfront and Monthly billing -// (learn.microsoft.com/azure/azure-cache-for-redis/cache-reserved-pricing), -// so rec.PaymentOption must map onto the correct armreservations -// ReservationBillingPlan value in the purchase body rather than silently -// defaulting to Azure's Upfront behavior for a no-upfront/monthly rec. -func TestCacheClient_PurchaseCommitment_BillingPlan(t *testing.T) { - t.Parallel() - cases := []struct { - name string - paymentOption string - wantPlan string - wantErrSub string - }{ - {name: "all-upfront maps to Upfront", paymentOption: "all-upfront", wantPlan: "Upfront"}, - {name: "no-upfront maps to Monthly", paymentOption: "no-upfront", wantPlan: "Monthly"}, - {name: "partial-upfront is rejected", paymentOption: "partial-upfront", wantErrSub: "partial-upfront has no azure equivalent"}, - {name: "empty payment option is rejected", paymentOption: "", wantErrSub: "azure reservations support only upfront or monthly billing"}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - rec := common.Recommendation{ResourceType: "Premium_P1", Term: "1yr", Count: 1, PaymentOption: tc.paymentOption} - - if tc.wantErrSub != "" { - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.Error(t, err) - assert.False(t, result.Success) - assert.Contains(t, err.Error(), tc.wantErrSub) - mockHTTP.AssertNotCalled(t, "Do", mock.Anything) - return - } - - const orderID = "cache-billingplan-test" - var capturedBody []byte - calculateResp17 := mocks.CreateMockHTTPResponse(http.StatusOK, calcPriceRespJSON(orderID)) - t.Cleanup(func() { - require.NoError(t, calculateResp17.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - if r.URL.Path != "/providers/Microsoft.Capacity/calculatePrice" { - return false - } - capturedBody, _ = io.ReadAll(r.Body) - r.Body = io.NopCloser(bytes.NewReader(capturedBody)) - return true - })).Return(calculateResp17, nil).Once() - purchaseResp18 := mocks.CreateMockHTTPResponse(http.StatusOK, `{}`) - t.Cleanup(func() { - require.NoError(t, purchaseResp18.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/"+orderID+"/purchase" - })).Return(purchaseResp18, nil).Once() - - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.NoError(t, err) - assert.True(t, result.Success) - - var body map[string]interface{} - require.NoError(t, json.Unmarshal(capturedBody, &body)) - props, ok := body["properties"].(map[string]interface{}) - require.True(t, ok, "properties map missing from reservation body") - assert.Equal(t, tc.wantPlan, props["billingPlan"]) - if tc.wantPlan == "Monthly" { - require.NotNil(t, result.Cost) - assert.Zero(t, *result.Cost) - } else { - assert.Nil(t, result.Cost) - } - mockHTTP.AssertExpectations(t) - }) - } -} - -// TestCacheClient_PurchaseCommitment_RequiresSource pins the dedupe guard: -// PurchaseCommitment must reject an empty opts.Source before issuing any HTTP -// call. Azure mints the reservation order ID server-side, so the -// purchase-automation tag derived from Source is the only stable dedupe -// signal CUDly controls -- proceeding without it would allow a re-driven -// purchase to create a duplicate reservation. -func TestCacheClient_PurchaseCommitment_RequiresSource(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - rec := common.Recommendation{ResourceType: "Premium_P1", Term: "1yr", Count: 1} - result, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{}) - require.Error(t, err) - assert.False(t, result.Success) - assert.Contains(t, err.Error(), "purchase source is required") - // No HTTP call may be issued when the guard rejects the request. - mockHTTP.AssertNotCalled(t, "Do", mock.Anything) -} - -// TestCacheClient_PurchaseCommitment_DisplayNameConformsToAzureAllowlist guards -// against regression: displayName in the calculatePrice body must match -// [A-Za-z0-9_-]{1,64} (Azure rejects DisplayNameInvalid otherwise). -func TestCacheClient_PurchaseCommitment_DisplayNameConformsToAzureAllowlist(t *testing.T) { - t.Parallel() - ctx := context.Background() - mockHTTP := &mocks.MockHTTPClient{} - mockCred := &MockTokenCredential{token: "test-token"} - client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) - - const orderID = "azure-cache-displayname" - var capturedDisplayName string - calculateResp19 := mocks.CreateMockHTTPResponse(http.StatusOK, calcPriceRespJSON(orderID)) - t.Cleanup(func() { - require.NoError(t, calculateResp19.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - if r.Method != http.MethodPost || r.URL.Path != "/providers/Microsoft.Capacity/calculatePrice" { - return false - } - if r.Body == nil { - return true - } - bodyBytes, _ := io.ReadAll(r.Body) - r.Body = io.NopCloser(bytes.NewReader(bodyBytes)) - var body map[string]interface{} - if err := json.Unmarshal(bodyBytes, &body); err == nil { - if props, ok := body["properties"].(map[string]interface{}); ok { - if dn, ok := props["displayName"].(string); ok { - capturedDisplayName = dn - } - } - } - return true - })).Return(calculateResp19, nil).Once() - purchaseResp20 := mocks.CreateMockHTTPResponse(http.StatusOK, `{}`) - t.Cleanup(func() { - require.NoError(t, purchaseResp20.Body.Close()) - }) - mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { - return r.Method == http.MethodPost && - r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/"+orderID+"/purchase" - })).Return(purchaseResp20, nil).Once() - - rec := common.Recommendation{ - ResourceType: "Premium_P1", - Term: "1yr", - Count: 1, - CommitmentCost: 1000.0, - PaymentOption: "no-upfront", - } - _, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) - require.NoError(t, err) - assert.NotEmpty(t, capturedDisplayName) - assert.Regexp(t, `^[A-Za-z0-9_-]{1,64}$`, capturedDisplayName) - // Rich-format guards: service code is correct and SKU is preserved - // (see providers/azure/services/internal/reservations/displayname.go). - assert.Regexp(t, `^redis-`, capturedDisplayName) - assert.Contains(t, capturedDisplayName, "Premium_P1") -} - // infiniteRedisCachesPager is a pager that always reports More()=true, // used to exercise the maxCachesPages budget cap. type infiniteRedisCachesPager struct{} diff --git a/providers/azure/services/cache/purchase_owner_test.go b/providers/azure/services/cache/purchase_owner_test.go new file mode 100644 index 00000000..2004a6b9 --- /dev/null +++ b/providers/azure/services/cache/purchase_owner_test.go @@ -0,0 +1,44 @@ +package cache + +import ( + "context" + "errors" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/providers/azure/mocks" +) + +type unusedCred struct{ t *testing.T } + +func (c unusedCred) GetToken(context.Context, policy.TokenRequestOptions) (azcore.AccessToken, error) { + c.t.Error("the refusal must not use the credential") + return azcore.AccessToken{}, errors.New("credential used") +} + +// Only managedredis buys RedisCache reservations (#538), so the same Redis +// capacity cannot be bought once through each service. +func TestCacheClient_PurchaseCommitment_RefusesRedisPurchases(t *testing.T) { + t.Parallel() + mockHTTP := &mocks.MockHTTPClient{} + client := NewClientWithHTTP(unusedCred{t}, "test-subscription", "eastus", mockHTTP) + rec := common.Recommendation{ResourceType: "Premium_P1", Term: "1yr", Count: 1, PaymentOption: "no-upfront"} + + result, err := client.PurchaseCommitment(context.Background(), rec, + common.PurchaseOptions{Source: common.PurchaseSourceCLI, IdempotencyToken: "token-1"}) + + require.Error(t, err) + assert.True(t, errors.Is(err, common.ErrRefusedBeforePurchase), "refusal must say nothing was sent: %v", err) + assert.Contains(t, err.Error(), string(common.ServiceMemoryDB)) + assert.False(t, result.Success) + assert.Empty(t, result.CommitmentID) + assert.Equal(t, err, result.Error) + assert.Equal(t, rec, result.Recommendation) + mockHTTP.AssertNotCalled(t, "Do", mock.Anything) +} From c4f5ed5019310b38c8d19a201bcc90d6e8863631 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 6 Nov 2026 06:27:11 +0100 Subject: [PATCH 4/4] test(azure/cache): refuse a ServiceCache-labelled Redis recommendation The refusal test used a recommendation with no Service, but the Azure recommendations adapter labels Redis recommendations ServiceCache. Use that label so a regression that branched on rec.Service would fail. Refs #538 --- providers/azure/services/cache/purchase_owner_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/azure/services/cache/purchase_owner_test.go b/providers/azure/services/cache/purchase_owner_test.go index 2004a6b9..b33fb746 100644 --- a/providers/azure/services/cache/purchase_owner_test.go +++ b/providers/azure/services/cache/purchase_owner_test.go @@ -28,7 +28,7 @@ func TestCacheClient_PurchaseCommitment_RefusesRedisPurchases(t *testing.T) { t.Parallel() mockHTTP := &mocks.MockHTTPClient{} client := NewClientWithHTTP(unusedCred{t}, "test-subscription", "eastus", mockHTTP) - rec := common.Recommendation{ResourceType: "Premium_P1", Term: "1yr", Count: 1, PaymentOption: "no-upfront"} + rec := common.Recommendation{Service: common.ServiceCache, ResourceType: "Premium_P1", Term: "1yr", Count: 1, PaymentOption: "no-upfront"} result, err := client.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI, IdempotencyToken: "token-1"})