Skip to content

Commit 359ef73

Browse files
cristimcristim
committed
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
1 parent d484c9d commit 359ef73

9 files changed

Lines changed: 122 additions & 6 deletions

File tree

‎providers/azure/services/cache/client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1717
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
1818
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis/v3"
19+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
1920

2021
"github.com/LeanerCloud/CUDly/pkg/common"
2122
"github.com/LeanerCloud/CUDly/pkg/logging"
@@ -320,7 +321,7 @@ func (c *CacheClient) PurchaseCommitment(ctx context.Context, rec common.Recomme
320321
},
321322
"location": c.region,
322323
"properties": map[string]interface{}{
323-
"reservedResourceType": "RedisCache",
324+
"reservedResourceType": string(armreservations.ReservedResourceTypeRedisCache),
324325
"billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID),
325326
"term": fmt.Sprintf("P%dY", termYears),
326327
"quantity": rec.Count,

‎providers/azure/services/compute/client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1919
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v5"
2020
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
21+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
2122

2223
"github.com/LeanerCloud/CUDly/pkg/common"
2324
"github.com/LeanerCloud/CUDly/pkg/logging"
@@ -427,7 +428,7 @@ func (c *ComputeClient) buildReservationBody(rec common.Recommendation, source,
427428
"sku": map[string]string{"name": rec.ResourceType},
428429
"location": c.region,
429430
"properties": map[string]interface{}{
430-
"reservedResourceType": "VirtualMachines",
431+
"reservedResourceType": string(armreservations.ReservedResourceTypeVirtualMachines),
431432
"billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID),
432433
"term": fmt.Sprintf("P%dY", termYears),
433434
"quantity": rec.Count,

‎providers/azure/services/cosmosdb/client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1818
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
1919
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/cosmos/armcosmos/v2"
20+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
2021

2122
"github.com/LeanerCloud/CUDly/pkg/common"
2223
"github.com/LeanerCloud/CUDly/pkg/logging"
@@ -313,7 +314,7 @@ func (c *CosmosDBClient) PurchaseCommitment(ctx context.Context, rec common.Reco
313314
},
314315
"location": c.region,
315316
"properties": map[string]interface{}{
316-
"reservedResourceType": "CosmosDb",
317+
"reservedResourceType": string(armreservations.ReservedResourceTypeCosmosDb),
317318
"billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID),
318319
"term": fmt.Sprintf("P%dY", termYears),
319320
"quantity": rec.Count,

‎providers/azure/services/database/client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
1616
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1717
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
18+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
1819
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql"
1920

2021
"github.com/LeanerCloud/CUDly/pkg/common"
@@ -318,7 +319,7 @@ func (c *DatabaseClient) PurchaseCommitment(ctx context.Context, rec common.Reco
318319
},
319320
"location": c.region,
320321
"properties": map[string]interface{}{
321-
"reservedResourceType": "SqlDatabase",
322+
"reservedResourceType": string(armreservations.ReservedResourceTypeSQLDatabases),
322323
"billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID),
323324
"term": fmt.Sprintf("P%dY", termYears),
324325
"quantity": rec.Count,

‎providers/azure/services/database/client_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
1414
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1515
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
16+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
1617
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/sql/armsql"
1718
"github.com/stretchr/testify/assert"
1819
"github.com/stretchr/testify/mock"
@@ -1246,3 +1247,54 @@ func TestDatabaseClient_PurchaseCommitment_DisplayNameConformsToAzureAllowlist(t
12461247
assert.Regexp(t, `^sql-`, capturedDisplayName)
12471248
assert.Contains(t, capturedDisplayName, "GP_Gen5_2")
12481249
}
1250+
1251+
// TestDatabaseClient_PurchaseCommitment_CanonicalReservedResourceType guards
1252+
// against regression of issue #1189: the calculatePrice/purchase body must
1253+
// send the canonical SDK enum value "SqlDatabases", not the hand-written
1254+
// "SqlDatabase" Azure rejects as an invalid reservedResourceType.
1255+
func TestDatabaseClient_PurchaseCommitment_CanonicalReservedResourceType(t *testing.T) {
1256+
ctx := context.Background()
1257+
mockHTTP := &MockHTTPClient{}
1258+
mockCred := &MockTokenCredential{token: "test-token"}
1259+
client := NewClientWithHTTP(mockCred, "test-subscription", "eastus", mockHTTP)
1260+
1261+
const orderID = "azure-db-rrt"
1262+
var capturedRRT string
1263+
mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool {
1264+
if r.Method != http.MethodPost || r.URL.Path != "/providers/Microsoft.Capacity/calculatePrice" {
1265+
return false
1266+
}
1267+
if r.Body == nil {
1268+
return true
1269+
}
1270+
bodyBytes, _ := io.ReadAll(r.Body)
1271+
r.Body = io.NopCloser(bytes.NewReader(bodyBytes))
1272+
var body map[string]interface{}
1273+
if err := json.Unmarshal(bodyBytes, &body); err == nil {
1274+
if props, ok := body["properties"].(map[string]interface{}); ok {
1275+
if rrt, ok := props["reservedResourceType"].(string); ok {
1276+
capturedRRT = rrt
1277+
}
1278+
}
1279+
}
1280+
return true
1281+
})).Return(createMockHTTPResponse(http.StatusOK, calcPriceRespJSON(orderID)), nil).Once()
1282+
mockHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool {
1283+
return r.Method == http.MethodPost &&
1284+
r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/"+orderID+"/purchase"
1285+
})).Return(createMockHTTPResponse(http.StatusOK, `{}`), nil).Once()
1286+
1287+
rec := common.Recommendation{
1288+
ResourceType: "GP_Gen5_2",
1289+
Term: "1yr",
1290+
Count: 1,
1291+
CommitmentCost: 1500.0,
1292+
}
1293+
_, err := client.PurchaseCommitment(ctx, rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI})
1294+
require.NoError(t, err)
1295+
assert.Equal(t, string(armreservations.ReservedResourceTypeSQLDatabases), capturedRRT)
1296+
assert.Contains(t, armreservations.PossibleReservedResourceTypeValues(),
1297+
armreservations.ReservedResourceType(capturedRRT),
1298+
"reservedResourceType %q must be a member of armreservations.PossibleReservedResourceTypeValues()", capturedRRT)
1299+
mockHTTP.AssertExpectations(t)
1300+
}

‎providers/azure/services/managedredis/client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1717
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
1818
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/redis/armredis/v3"
19+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
1920

2021
"github.com/LeanerCloud/CUDly/pkg/common"
2122
"github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient"
@@ -260,7 +261,7 @@ func (c *ManagedRedisClient) PurchaseCommitment(ctx context.Context, rec common.
260261
},
261262
"location": c.region,
262263
"properties": map[string]interface{}{
263-
"reservedResourceType": "RedisCache",
264+
"reservedResourceType": string(armreservations.ReservedResourceTypeRedisCache),
264265
"billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID),
265266
"term": fmt.Sprintf("P%dY", termYears),
266267
"quantity": rec.Count,

‎providers/azure/services/search/client.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,11 @@ func (c *SearchClient) PurchaseCommitment(ctx context.Context, rec common.Recomm
294294
},
295295
"location": c.region,
296296
"properties": map[string]interface{}{
297+
// "SearchService" has no counterpart in the armreservations
298+
// ReservedResourceType enum (checked v1.1.0 and v2.0.0), so it
299+
// cannot be expressed as an SDK constant like the other service
300+
// clients do; the literal is kept until verified against the
301+
// live reservation catalog (see issue #1189).
297302
"reservedResourceType": "SearchService",
298303
"billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID),
299304
"term": fmt.Sprintf("P%dY", termYears),

‎providers/azure/services/synapse/client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
1818
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1919
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
20+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
2021

2122
"github.com/LeanerCloud/CUDly/pkg/common"
2223
"github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient"
@@ -268,7 +269,7 @@ func (c *SynapseClient) PurchaseCommitment(ctx context.Context, rec common.Recom
268269
},
269270
"location": c.region,
270271
"properties": map[string]interface{}{
271-
"reservedResourceType": "SqlDW",
272+
"reservedResourceType": string(armreservations.ReservedResourceTypeSQLDataWarehouse),
272273
"billingScopeId": fmt.Sprintf("/subscriptions/%s", c.subscriptionID),
273274
"term": fmt.Sprintf("P%dY", termYears),
274275
"quantity": rec.Count,

‎providers/azure/services/synapse/client_test.go‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package synapse
33
import (
44
"bytes"
55
"context"
6+
"encoding/json"
67
"errors"
78
"io"
89
"net/http"
@@ -12,6 +13,7 @@ import (
1213
"github.com/Azure/azure-sdk-for-go/sdk/azcore"
1314
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
1415
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/consumption/armconsumption"
16+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/reservations/armreservations"
1517
"github.com/stretchr/testify/assert"
1618
"github.com/stretchr/testify/mock"
1719
"github.com/stretchr/testify/require"
@@ -852,3 +854,54 @@ func TestGetOfferingDetails_noReservationPrice(t *testing.T) {
852854
// helper's contract is now exercised in providers/azure/services/internal/reservations
853855
// package tests; the synapse-side coverage is via the executor-level
854856
// PurchaseCommitment tests above.
857+
858+
// TestPurchaseCommitment_canonicalReservedResourceType guards against
859+
// regression of issue #1189: the calculatePrice/purchase body must send the
860+
// canonical SDK enum value "SqlDataWarehouse", not the hand-written "SqlDW"
861+
// Azure rejects as an invalid reservedResourceType.
862+
func TestPurchaseCommitment_canonicalReservedResourceType(t *testing.T) {
863+
mHTTP := &mockHTTPClient{}
864+
t.Cleanup(func() { mHTTP.AssertExpectations(t) })
865+
866+
const orderID = "syn-order-rrt"
867+
var capturedRRT string
868+
mHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool {
869+
if r.Method != http.MethodPost || r.URL.Path != "/providers/Microsoft.Capacity/calculatePrice" {
870+
return false
871+
}
872+
if r.Body == nil {
873+
return true
874+
}
875+
bodyBytes, _ := io.ReadAll(r.Body)
876+
r.Body = io.NopCloser(bytes.NewReader(bodyBytes))
877+
var body map[string]interface{}
878+
if err := json.Unmarshal(bodyBytes, &body); err == nil {
879+
if props, ok := body["properties"].(map[string]interface{}); ok {
880+
if rrt, ok := props["reservedResourceType"].(string); ok {
881+
capturedRRT = rrt
882+
}
883+
}
884+
}
885+
return true
886+
})).Return(newHTTPResponse(http.StatusOK, calcPriceRespJSON(orderID)), nil).Once()
887+
mHTTP.On("Do", mock.MatchedBy(func(r *http.Request) bool {
888+
return r.Method == http.MethodPost &&
889+
r.URL.Path == "/providers/Microsoft.Capacity/reservationOrders/"+orderID+"/purchase"
890+
})).Return(newHTTPResponse(http.StatusOK, `{}`), nil).Once()
891+
892+
cred := &mockTokenCredential{token: "test-token"}
893+
c := NewClientWithHTTP(cred, "sub-123", "eastus", mHTTP)
894+
895+
rec := common.Recommendation{
896+
ResourceType: "DW1000c",
897+
Term: "1yr",
898+
Count: 1,
899+
CommitmentCost: 5000.0,
900+
}
901+
_, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{Source: common.PurchaseSourceCLI})
902+
require.NoError(t, err)
903+
assert.Equal(t, string(armreservations.ReservedResourceTypeSQLDataWarehouse), capturedRRT)
904+
assert.Contains(t, armreservations.PossibleReservedResourceTypeValues(),
905+
armreservations.ReservedResourceType(capturedRRT),
906+
"reservedResourceType %q must be a member of armreservations.PossibleReservedResourceTypeValues()", capturedRRT)
907+
}

0 commit comments

Comments
 (0)