fix(azure/savingsplans): send Compute_Savings_Plan as sku.name on purchase and validate

providers/azure/services/savingsplans/client.go

@@ -328,6 +328,21 @@ func commitmentCurrency(d *common.SavingsPlanDetails) (string, error) {
 	return strings.ToUpper(d.Currency), nil
 }
 
+// savingsPlanSKUName maps a CUDly PlanType to the ARM SKU name that the
+// SavingsPlanOrderAlias and validatePurchase bodies carry in sku.name. Azure
+// names the compute savings plan SKU "Compute_Savings_Plan" (the armSkuName the
+// Benefit Recommendations API returns), while CUDly's PlanType is "Compute"
+// (issue #387). The ARM name itself is accepted as-is. Anything else fails
+// closed before any Azure call rather than sending a sku.name Azure rejects.
+func savingsPlanSKUName(planType string) (string, error) {
+	switch planType {
+	case "Compute", armSkuComputeSavingsPlan:
+		return armSkuComputeSavingsPlan, nil
+	default:
+		return "", fmt.Errorf("azure savings plan type %q has no known Azure savings plan SKU", planType)
+	}
+}
+
 // PurchaseCommitment creates an Azure Savings Plan via the OrderAlias API.
 //
 // Idempotency: the SavingsPlanOrderAlias is created with an HTTP PUT on the
@@ -354,22 +369,11 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		return result, result.Error
 	}
 
-	term, err := toAzureTerm(rec.Term)
-	if err != nil {
-		result.Error = err
-		return result, result.Error
-	}
-
-	grain := armbillingbenefits.CommitmentGrainHourly
-	billingScopeID := c.billingScopeID()
-	appliedScope := armbillingbenefits.AppliedScopeTypeShared
-	hourlyAmount := spDetails.HourlyCommitment
-	currency, err := commitmentCurrency(spDetails)
+	body, err := c.purchaseOrderBody(rec, spDetails)
 	if err != nil {
 		result.Error = err
 		return result, result.Error
 	}
-	displayName := fmt.Sprintf("cudly-%s-%s", spDetails.PlanType, rec.Term)
 	// Derive a deterministic alias name from the idempotency token when present
 	// so a re-drive PUTs the same alias (idempotent), falling back to the prior
 	// timestamp-based name on the CLI path (empty token). A malformed non-empty
@@ -381,31 +385,10 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 		return result, result.Error
 	}
 
-	body := armbillingbenefits.SavingsPlanOrderAliasModel{
-		SKU: &armbillingbenefits.SKU{Name: toPtr(spDetails.PlanType)},
-		Properties: &armbillingbenefits.SavingsPlanOrderAliasProperties{
-			DisplayName:      &displayName,
-			BillingScopeID:   &billingScopeID,
-			Term:             &term,
-			AppliedScopeType: &appliedScope,
-			Commitment: &armbillingbenefits.Commitment{
-				Amount:       &hourlyAmount,
-				CurrencyCode: &currency,
-				Grain:        &grain,
-			},
-		},
-	}
-
-	var aliasClient SavingsPlanOrderAliasAPI
-	if c.orderAliasClient != nil {
-		aliasClient = c.orderAliasClient
-	} else {
-		client, clientErr := armbillingbenefits.NewSavingsPlanOrderAliasClient(c.cred, nil)
-		if clientErr != nil {
-			result.Error = fmt.Errorf("failed to create order alias client: %w", clientErr)
-			return result, result.Error
-		}
-		aliasClient = &realOrderAliasClient{client: client}
+	aliasClient, err := c.orderAlias()
+	if err != nil {
+		result.Error = err
+		return result, result.Error
 	}
 
 	poller, err := aliasClient.BeginCreate(ctx, aliasName, body, nil)
@@ -431,6 +414,56 @@ func (c *Client) PurchaseCommitment(ctx context.Context, rec common.Recommendati
 	return result, nil
 }
 
+// purchaseOrderBody builds the order-alias request for a savings plan
+// purchase. The term, currency and SKU name are validated here, so an invalid
+// recommendation fails before any Azure client exists.
+func (c *Client) purchaseOrderBody(rec common.Recommendation, spDetails *common.SavingsPlanDetails) (armbillingbenefits.SavingsPlanOrderAliasModel, error) {
+	term, err := toAzureTerm(rec.Term)
+	if err != nil {
+		return armbillingbenefits.SavingsPlanOrderAliasModel{}, err
+	}
+	currency, err := commitmentCurrency(spDetails)
+	if err != nil {
+		return armbillingbenefits.SavingsPlanOrderAliasModel{}, err
+	}
+	skuName, err := savingsPlanSKUName(spDetails.PlanType)
+	if err != nil {
+		return armbillingbenefits.SavingsPlanOrderAliasModel{}, err
+	}
+	grain := armbillingbenefits.CommitmentGrainHourly
+	billingScopeID := c.billingScopeID()
+	appliedScope := armbillingbenefits.AppliedScopeTypeShared
+	hourlyAmount := spDetails.HourlyCommitment
+	displayName := fmt.Sprintf("cudly-%s-%s", spDetails.PlanType, rec.Term)
+	return armbillingbenefits.SavingsPlanOrderAliasModel{
+		SKU: &armbillingbenefits.SKU{Name: toPtr(skuName)},
+		Properties: &armbillingbenefits.SavingsPlanOrderAliasProperties{
+			DisplayName:      &displayName,
+			BillingScopeID:   &billingScopeID,
+			Term:             &term,
+			AppliedScopeType: &appliedScope,
+			Commitment: &armbillingbenefits.Commitment{
+				Amount:       &hourlyAmount,
+				CurrencyCode: &currency,
+				Grain:        &grain,
+			},
+		},
+	}, nil
+}
+
+// orderAlias returns the injected order-alias client, or a real one built
+// from the client's credential.
+func (c *Client) orderAlias() (SavingsPlanOrderAliasAPI, error) {
+	if c.orderAliasClient != nil {
+		return c.orderAliasClient, nil
+	}
+	client, err := armbillingbenefits.NewSavingsPlanOrderAliasClient(c.cred, nil)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create order alias client: %w", err)
+	}
+	return &realOrderAliasClient{client: client}, nil
+}
+
 // ValidateOffering checks whether the Savings Plan configuration is valid.
 func (c *Client) ValidateOffering(ctx context.Context, rec common.Recommendation) error {
 	validateBody, err := c.buildValidateBody(rec)
@@ -471,12 +504,16 @@ func (c *Client) buildValidateBody(rec common.Recommendation) (armbillingbenefit
 	if err != nil {
 		return armbillingbenefits.SavingsPlanPurchaseValidateRequest{}, err
 	}
+	skuName, err := savingsPlanSKUName(spDetails.PlanType)
+	if err != nil {
+		return armbillingbenefits.SavingsPlanPurchaseValidateRequest{}, err
+	}
 	displayName := "cudly-validate"
 
 	return armbillingbenefits.SavingsPlanPurchaseValidateRequest{
 		Benefits: []*armbillingbenefits.SavingsPlanOrderAliasModel{
 			{
-				SKU: &armbillingbenefits.SKU{Name: toPtr(spDetails.PlanType)},
+				SKU: &armbillingbenefits.SKU{Name: toPtr(skuName)},
 				Properties: &armbillingbenefits.SavingsPlanOrderAliasProperties{
 					DisplayName:      &displayName,
 					BillingScopeID:   &billingScopeID,

providers/azure/services/savingsplans/sku_name_test.go

@@ -0,0 +1,104 @@
+package savingsplans
+
+import (
+	"context"
+	"net/http"
+	"testing"
+
+	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/billingbenefits/armbillingbenefits"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+// Issue #387: the SavingsPlanOrderAlias and validatePurchase bodies must carry
+// the ARM SKU name "Compute_Savings_Plan" (the armSkuName the Benefit
+// Recommendations API returns and the SDK's SavingsPlanOrderAliasCreate example
+// sends), not the CUDly PlanType "Compute". Before the fix both bodies sent
+// sku.name "Compute".
+
+func skuRec(planType string) common.Recommendation {
+	rec := makeRec("1yr")
+	rec.Details = &common.SavingsPlanDetails{PlanType: planType, HourlyCommitment: 1.5, Currency: "USD"}
+	return rec
+}
+
+func okAlias() *mockOrderAliasClient {
+	orderID := "/providers/Microsoft.BillingBenefits/savingsPlanOrders/order1"
+	return &mockOrderAliasClient{createPoller: &mockPoller{resp: armbillingbenefits.SavingsPlanOrderAliasClientCreateResponse{
+		SavingsPlanOrderAliasModel: armbillingbenefits.SavingsPlanOrderAliasModel{ID: &orderID},
+	}}}
+}
+
+func capturedSKUs(t *testing.T, rec common.Recommendation) (purchase, validate string) {
+	t.Helper()
+	alias := okAlias()
+	rp := &mockRPValidate{}
+	c := NewClient(nil, "sub", "eastus")
+	c.SetOrderAliasClient(alias)
+	c.SetRPValidateClient(rp)
+
+	_, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
+	require.NoError(t, err)
+	require.NoError(t, c.ValidateOffering(context.Background(), rec))
+
+	require.NotNil(t, alias.capturedBody.SKU)
+	require.NotNil(t, alias.capturedBody.SKU.Name)
+	require.Len(t, rp.captured.Benefits, 1)
+	require.NotNil(t, rp.captured.Benefits[0].SKU)
+	require.NotNil(t, rp.captured.Benefits[0].SKU.Name)
+	return *alias.capturedBody.SKU.Name, *rp.captured.Benefits[0].SKU.Name
+}
+
+func TestSKUName_ComputePlanTypeSendsARMSKU(t *testing.T) {
+	for _, planType := range []string{"Compute", "Compute_Savings_Plan"} {
+		t.Run(planType, func(t *testing.T) {
+			purchase, validate := capturedSKUs(t, skuRec(planType))
+			assert.Equal(t, "Compute_Savings_Plan", purchase)
+			assert.Equal(t, "Compute_Savings_Plan", validate)
+		})
+	}
+}
+
+// A recommendation produced by GetRecommendations from a fixture row must
+// purchase under the same SKU the row named.
+func TestSKUName_RecommendationRoundTrip(t *testing.T) {
+	h := &routeHTTP{reply: func(*http.Request) (int, string) {
+		return 200, `{"value":[` + benefitRow("P1Y", nil) + `]}`
+	}}
+	recs, err := newTestClient(h).GetRecommendations(context.Background(), &common.RecommendationParams{Term: "1yr", LookbackPeriod: "30d"})
+	require.NoError(t, err)
+	require.Len(t, recs, 1)
+
+	purchase, validate := capturedSKUs(t, recs[0])
+	assert.Equal(t, armSkuComputeSavingsPlan, purchase)
+	assert.Equal(t, armSkuComputeSavingsPlan, validate)
+}
+
+// A PlanType with no known ARM SKU fails before any Azure call instead of
+// sending a sku.name the API would reject.
+func TestSKUName_UnknownPlanTypeFailsClosed(t *testing.T) {
+	for _, planType := range []string{"", "MachineLearning", "compute", " Compute", "Compute_Savings_Plan "} {
+		t.Run(planType, func(t *testing.T) {
+			alias := okAlias()
+			rp := &mockRPValidate{}
+			c := NewClient(nil, "sub", "eastus")
+			c.SetOrderAliasClient(alias)
+			c.SetRPValidateClient(rp)
+			rec := skuRec(planType)
+
+			result, err := c.PurchaseCommitment(context.Background(), rec, common.PurchaseOptions{})
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "no known Azure savings plan SKU")
+			assert.False(t, result.Success)
+			assert.Equal(t, err, result.Error)
+			assert.Zero(t, alias.calls)
+
+			err = c.ValidateOffering(context.Background(), rec)
+			require.Error(t, err)
+			assert.Contains(t, err.Error(), "no known Azure savings plan SKU")
+			assert.Zero(t, rp.calls)
+		})
+	}
+}