From d6647f43700fcdd3400283b3fe91d1a2f6e8af37 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Wed, 10 Jun 2026 12:28:11 -0700 Subject: [PATCH] fix(providers/azure): send canonical reservedResourceType enum values The Azure SQL Database and Synapse purchase bodies sent hand-written reservedResourceType literals ("SqlDatabase", "SqlDW") that are not members of the armreservations ReservedResourceType enum; Azure would reject calculatePrice with an invalid-value error once non-VM purchases become reachable (known-issues #731). Replace them with the canonical SDK constants ReservedResourceTypeSQLDatabases ("SqlDatabases") and ReservedResourceTypeSQLDataWarehouse ("SqlDataWarehouse"). Also convert the already-correct literals in cache, cosmosdb, compute, and managedredis to their SDK constants so all purchase bodies are compile-time-checked enum members, and document that search's "SearchService" has no SDK enum counterpart (v1.1.0 and v2.0.0). Regression tests capture the calculatePrice request body and assert the sent value equals the SDK constant and is a member of PossibleReservedResourceTypeValues(); both fail against the old literals. Closes #1189 --- providers/azure/services/cache/client.go | 3 +- providers/azure/services/compute/client.go | 3 +- providers/azure/services/cosmosdb/client.go | 3 +- providers/azure/services/database/client.go | 3 +- .../azure/services/database/client_test.go | 52 ++++++++++++++++++ .../azure/services/managedredis/client.go | 3 +- providers/azure/services/search/client.go | 5 ++ providers/azure/services/synapse/client.go | 3 +- .../azure/services/synapse/client_test.go | 53 +++++++++++++++++++ 9 files changed, 122 insertions(+), 6 deletions(-) diff --git a/providers/azure/services/cache/client.go b/providers/azure/services/cache/client.go index 266e27816..5145fb5e8 100644 --- a/providers/azure/services/cache/client.go +++ b/providers/azure/services/cache/client.go @@ -16,6 +16,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "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/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/pkg/logging" @@ -320,7 +321,7 @@ func (c *CacheClient) PurchaseCommitment(ctx context.Context, rec common.Recomme }, "location": c.region, "properties": map[string]interface{}{ - "reservedResourceType": "RedisCache", + "reservedResourceType": string(armreservations.ReservedResourceTypeRedisCache), "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), "term": fmt.Sprintf("P%dY", termYears), "quantity": rec.Count, diff --git a/providers/azure/services/compute/client.go b/providers/azure/services/compute/client.go index 7062f54a0..6ef720dcd 100644 --- a/providers/azure/services/compute/client.go +++ b/providers/azure/services/compute/client.go @@ -18,6 +18,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/pkg/logging" @@ -427,7 +428,7 @@ func (c *ComputeClient) buildReservationBody(rec common.Recommendation, source, "sku": map[string]string{"name": rec.ResourceType}, "location": c.region, "properties": map[string]interface{}{ - "reservedResourceType": "VirtualMachines", + "reservedResourceType": string(armreservations.ReservedResourceTypeVirtualMachines), "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), "term": fmt.Sprintf("P%dY", termYears), "quantity": rec.Count, diff --git a/providers/azure/services/cosmosdb/client.go b/providers/azure/services/cosmosdb/client.go index 5c619e3b5..5c8cad242 100644 --- a/providers/azure/services/cosmosdb/client.go +++ b/providers/azure/services/cosmosdb/client.go @@ -17,6 +17,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cosmos/armcosmos/v2" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/pkg/logging" @@ -313,7 +314,7 @@ func (c *CosmosDBClient) PurchaseCommitment(ctx context.Context, rec common.Reco }, "location": c.region, "properties": map[string]interface{}{ - "reservedResourceType": "CosmosDb", + "reservedResourceType": string(armreservations.ReservedResourceTypeCosmosDb), "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), "term": fmt.Sprintf("P%dY", termYears), "quantity": rec.Count, diff --git a/providers/azure/services/database/client.go b/providers/azure/services/database/client.go index acd598904..28842f4c1 100644 --- a/providers/azure/services/database/client.go +++ b/providers/azure/services/database/client.go @@ -15,6 +15,7 @@ import ( "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/resourcemanager/consumption/armconsumption" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql" "github.com/LeanerCloud/CUDly/pkg/common" @@ -318,7 +319,7 @@ func (c *DatabaseClient) PurchaseCommitment(ctx context.Context, rec common.Reco }, "location": c.region, "properties": map[string]interface{}{ - "reservedResourceType": "SqlDatabase", + "reservedResourceType": string(armreservations.ReservedResourceTypeSQLDatabases), "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), "term": fmt.Sprintf("P%dY", termYears), "quantity": rec.Count, diff --git a/providers/azure/services/database/client_test.go b/providers/azure/services/database/client_test.go index 95d2d8351..a2d9f2a84 100644 --- a/providers/azure/services/database/client_test.go +++ b/providers/azure/services/database/client_test.go @@ -13,6 +13,7 @@ import ( "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/resourcemanager/consumption/armconsumption" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -1246,3 +1247,54 @@ func TestDatabaseClient_PurchaseCommitment_DisplayNameConformsToAzureAllowlist(t assert.Regexp(t, `^sql-`, capturedDisplayName) assert.Contains(t, capturedDisplayName, "GP_Gen5_2") } + +// TestDatabaseClient_PurchaseCommitment_CanonicalReservedResourceType guards +// against regression of issue #1189: the calculatePrice/purchase body must +// send the canonical SDK enum value "SqlDatabases", not the hand-written +// "SqlDatabase" Azure rejects as an invalid reservedResourceType. +func TestDatabaseClient_PurchaseCommitment_CanonicalReservedResourceType(t *testing.T) { + ctx := context.Background() + mockHTTP := &MockHTTPClient{} + mockCred := &MockTokenCredential{token: "test-token"} + client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP) + + const orderID = "azure-db-rrt" + var capturedRRT string + 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 rrt, ok := props["reservedResourceType"].(string); ok { + capturedRRT = rrt + } + } + } + return true + })).Return(createMockHTTPResponse(http.StatusOK, calcPriceRespJSON(orderID)), nil).Once() + 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(createMockHTTPResponse(http.StatusOK, `{}`), nil).Once() + + rec := common.Recommendation{ + ResourceType: "GP_Gen5_2", + Term: "1yr", + Count: 1, + CommitmentCost: 1500.0, + } + _, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) + require.NoError(t, err) + assert.Equal(t, string(armreservations.ReservedResourceTypeSQLDatabases), capturedRRT) + assert.Contains(t, armreservations.PossibleReservedResourceTypeValues(), + armreservations.ReservedResourceType(capturedRRT), + "reservedResourceType %q must be a member of armreservations.PossibleReservedResourceTypeValues()", capturedRRT) + mockHTTP.AssertExpectations(t) +} diff --git a/providers/azure/services/managedredis/client.go b/providers/azure/services/managedredis/client.go index e38722c8c..809cdbf15 100644 --- a/providers/azure/services/managedredis/client.go +++ b/providers/azure/services/managedredis/client.go @@ -16,6 +16,7 @@ import ( "github.com/Azure/azure-sdk-for-go/sdk/azcore/policy" "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/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient" @@ -260,7 +261,7 @@ func (c *ManagedRedisClient) PurchaseCommitment(ctx context.Context, rec common. }, "location": c.region, "properties": map[string]interface{}{ - "reservedResourceType": "RedisCache", + "reservedResourceType": string(armreservations.ReservedResourceTypeRedisCache), "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), "term": fmt.Sprintf("P%dY", termYears), "quantity": rec.Count, diff --git a/providers/azure/services/search/client.go b/providers/azure/services/search/client.go index 0ec7fb77b..07dce3006 100644 --- a/providers/azure/services/search/client.go +++ b/providers/azure/services/search/client.go @@ -294,6 +294,11 @@ func (c *SearchClient) PurchaseCommitment(ctx context.Context, rec common.Recomm }, "location": c.region, "properties": map[string]interface{}{ + // "SearchService" has no counterpart in the armreservations + // ReservedResourceType enum (checked v1.1.0 and v2.0.0), so it + // cannot be expressed as an SDK constant like the other service + // clients do; the literal is kept until verified against the + // live reservation catalog (see issue #1189). "reservedResourceType": "SearchService", "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), "term": fmt.Sprintf("P%dY", termYears), diff --git a/providers/azure/services/synapse/client.go b/providers/azure/services/synapse/client.go index 44110ac88..7a7a915fc 100644 --- a/providers/azure/services/synapse/client.go +++ b/providers/azure/services/synapse/client.go @@ -17,6 +17,7 @@ import ( "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/resourcemanager/consumption/armconsumption" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/LeanerCloud/CUDly/pkg/common" "github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient" @@ -268,7 +269,7 @@ func (c *SynapseClient) PurchaseCommitment(ctx context.Context, rec common.Recom }, "location": c.region, "properties": map[string]interface{}{ - "reservedResourceType": "SqlDW", + "reservedResourceType": string(armreservations.ReservedResourceTypeSQLDataWarehouse), "billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID), "term": fmt.Sprintf("P%dY", termYears), "quantity": rec.Count, diff --git a/providers/azure/services/synapse/client_test.go b/providers/azure/services/synapse/client_test.go index a3c8c49c6..30d67d9ed 100644 --- a/providers/azure/services/synapse/client_test.go +++ b/providers/azure/services/synapse/client_test.go @@ -3,6 +3,7 @@ package synapse import ( "bytes" "context" + "encoding/json" "errors" "io" "net/http" @@ -12,6 +13,7 @@ import ( "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/resourcemanager/consumption/armconsumption" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -852,3 +854,54 @@ func TestGetOfferingDetails_noReservationPrice(t *testing.T) { // helper's contract is now exercised in providers/azure/services/internal/reservations // package tests; the synapse-side coverage is via the executor-level // PurchaseCommitment tests above. + +// TestPurchaseCommitment_canonicalReservedResourceType guards against +// regression of issue #1189: the calculatePrice/purchase body must send the +// canonical SDK enum value "SqlDataWarehouse", not the hand-written "SqlDW" +// Azure rejects as an invalid reservedResourceType. +func TestPurchaseCommitment_canonicalReservedResourceType(t *testing.T) { + mHTTP := &mockHTTPClient{} + t.Cleanup(func() { mHTTP.AssertExpectations(t) }) + + const orderID = "syn-order-rrt" + var capturedRRT string + mHTTP.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 rrt, ok := props["reservedResourceType"].(string); ok { + capturedRRT = rrt + } + } + } + return true + })).Return(newHTTPResponse(http.StatusOK, calcPriceRespJSON(orderID)), nil).Once() + mHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool { + return r.Method == http.MethodPost && + r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/"+orderID+"/purchase" + })).Return(newHTTPResponse(http.StatusOK, `{}`), nil).Once() + + cred := &mockTokenCredential{token: "test-token"} + c := NewClientWithHTTP(cred, "sub-123", "eastus", mHTTP) + + rec := common.Recommendation{ + ResourceType: "DW1000c", + Term: "1yr", + Count: 1, + CommitmentCost: 5000.0, + } + _, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI}) + require.NoError(t, err) + assert.Equal(t, string(armreservations.ReservedResourceTypeSQLDataWarehouse), capturedRRT) + assert.Contains(t, armreservations.PossibleReservedResourceTypeValues(), + armreservations.ReservedResourceType(capturedRRT), + "reservedResourceType %q must be a member of armreservations.PossibleReservedResourceTypeValues()", capturedRRT) +}