Skip to content

fix(providers/azure): restore IdempotencyToken threading in DoPurchaseTwoStep #729

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cristim merged 1 commit into from
May 26, 2026
Merged

fix(providers/azure): restore IdempotencyToken threading in DoPurchaseTwoStep #729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 8 additions & 20 deletions providers/azure/services/cache/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,13 @@ func (c *CacheClient) PurchaseCommitment(ctx context.Context, rec common.Recomme
Timestamp: time.Now(),
}

// Azure's reservation API mints the order ID server-side in calculatePrice,
// so the only stable dedupe signal we control is the purchase-automation tag
// derived from opts.Source. Without a non-empty Source the tag is dropped and
// a re-driven purchase cannot recognise the prior attempt, producing
// duplicate reservations. Fail fast at function entry rather than allowing
// an un-tagged, non-idempotent purchase to hit the cloud.
// 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 idempotent Azure reservation purchases")
result.Error = fmt.Errorf("purchase source is required for Azure reservation purchases")
return result, result.Error
}

Expand Down Expand Up @@ -312,7 +311,7 @@ func (c *CacheClient) PurchaseCommitment(ctx context.Context, rec common.Recomme
"renew": false,
},
}
applyPurchaseAutomationTag(requestBody, opts.Source)
reservations.ApplyPurchaseTags(requestBody, opts.Source, opts.IdempotencyToken)

bodyBytes, err := json.Marshal(requestBody)
if err != nil {
Expand All @@ -328,7 +327,7 @@ func (c *CacheClient) PurchaseCommitment(ctx context.Context, rec common.Recomme
return result, result.Error
}

reservationOrderID, err := reservations.DoPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token)
reservationOrderID, err := reservations.DoIdempotentPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token, opts.IdempotencyToken)
if err != nil {
result.Error = err
return result, result.Error
Expand Down Expand Up @@ -677,14 +676,3 @@ func (c *CacheClient) fetchSKUCatalogue(ctx context.Context) map[string]redisSKU
}
return out
}

// applyPurchaseAutomationTag attaches the purchase-automation tag to an Azure
// reservation request body when source is non-empty. Extracted out of
// PurchaseCommitment to keep the function under the cyclomatic-complexity
// threshold enforced by the pre-commit hook.
func applyPurchaseAutomationTag(body map[string]interface{}, source string) {
if source == "" {
return
}
body["tags"] = map[string]string{common.PurchaseTagKey: source}
}
37 changes: 19 additions & 18 deletions providers/azure/services/compute/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,9 +363,11 @@ func (c *ComputeClient) checkAndRegisterCapacityProvider(ctx context.Context) er

// buildReservationBody builds the JSON body for a reservation purchase request.
// The same body is sent to both calculatePrice and purchase endpoints (issue #677).
// When source is non-empty, a top-level tags map carrying purchase-automation
// is attached so the resulting reservation is identifiable in the portal.
func (c *ComputeClient) buildReservationBody(rec common.Recommendation, source string) ([]byte, error) {
// The purchase-automation and cudly-idempotency-token tags are attached via
// reservations.ApplyPurchaseTags so the resulting reservation is identifiable
// in the portal AND a re-driven purchase can find it via tag lookup before
// buying a duplicate (issue #721).
func (c *ComputeClient) buildReservationBody(rec common.Recommendation, source, idempotencyToken string) ([]byte, error) {
termYears := 1
if rec.Term == "3yr" || rec.Term == "3" {
termYears = 3
Expand All @@ -391,9 +393,7 @@ func (c *ComputeClient) buildReservationBody(rec common.Recommendation, source s
"renew": false,
},
}
if source != "" {
requestBody["tags"] = map[string]string{common.PurchaseTagKey: source}
}
reservations.ApplyPurchaseTags(requestBody, source, idempotencyToken)
return json.Marshal(requestBody)
}

Expand All @@ -407,9 +407,11 @@ func (c *ComputeClient) buildReservationBody(rec common.Recommendation, source s
// 2. POST reservationOrders/{id}/purchase -- commits the order.
//
// Idempotency: Azure mints the order ID in step 1, so client-supplied IDs are
// no longer used. Re-drives are idempotent via tag-based deduplication at the
// caller level (purchase execution checks for existing reservations tagged with
// the (executionID, recIndex) identity before calling PurchaseCommitment).
// no longer used. Re-drives are idempotent via tag-based deduplication
// performed inside reservations.DoIdempotentPurchaseTwoStep: every purchase
// body carries the cudly-idempotency-token tag derived from opts.IdempotencyToken,
// and a re-drive lists existing reservation orders and short-circuits when an
// order already carries the same tag (issue #721).
func (c *ComputeClient) PurchaseCommitment(ctx context.Context, rec common.Recommendation, opts common.PurchaseOptions) (common.PurchaseResult, error) {
result := common.PurchaseResult{
Recommendation: rec,
Expand All @@ -418,21 +420,20 @@ func (c *ComputeClient) PurchaseCommitment(ctx context.Context, rec common.Recom
Timestamp: time.Now(),
}

// Azure's reservation API mints the order ID server-side in calculatePrice,
// so the only stable dedupe signal we control is the purchase-automation tag
// derived from opts.Source. Without a non-empty Source the tag is dropped and
// a re-driven purchase cannot recognise the prior attempt, producing
// duplicate reservations. Fail fast at function entry rather than allowing
// an un-tagged, non-idempotent purchase to hit the cloud.
// 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 idempotent Azure reservation purchases")
result.Error = fmt.Errorf("purchase source is required for Azure reservation purchases")
return result, result.Error
}

// Ensure Microsoft.Capacity provider is registered (cached after first call).
c.ensureCapacityProviderRegistered(ctx)

bodyBytes, err := c.buildReservationBody(rec, opts.Source)
bodyBytes, err := c.buildReservationBody(rec, opts.Source, opts.IdempotencyToken)
if err != nil {
result.Error = fmt.Errorf("failed to marshal request: %w", err)
return result, result.Error
Expand All @@ -446,7 +447,7 @@ func (c *ComputeClient) PurchaseCommitment(ctx context.Context, rec common.Recom
return result, result.Error
}

reservationOrderID, err := reservations.DoPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token)
reservationOrderID, err := reservations.DoIdempotentPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token, opts.IdempotencyToken)
if err != nil {
result.Error = err
return result, result.Error
Expand Down
29 changes: 25 additions & 4 deletions providers/azure/services/compute/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ func TestBuildReservationBody_IncludesPurchaseAutomationTag(t *testing.T) {
c := &ComputeClient{region: "eastus", subscriptionID: "sub-abc"}
rec := common.Recommendation{ResourceType: "Standard_D2s_v3", Count: 1, Term: "1yr"}

body, err := c.buildReservationBody(rec, common.PurchaseSourceWeb)
body, err := c.buildReservationBody(rec, common.PurchaseSourceWeb, "")
require.NoError(t, err)

var got map[string]interface{}
Expand All @@ -935,17 +935,38 @@ func TestBuildReservationBody_IncludesPurchaseAutomationTag(t *testing.T) {
assert.Equal(t, common.PurchaseSourceWeb, tags[common.PurchaseTagKey])
}

func TestBuildReservationBody_OmitsTagsWhenSourceEmpty(t *testing.T) {
func TestBuildReservationBody_OmitsTagsWhenSourceAndTokenEmpty(t *testing.T) {
c := &ComputeClient{region: "eastus", subscriptionID: "sub-abc"}
rec := common.Recommendation{ResourceType: "Standard_D2s_v3", Count: 1, Term: "1yr"}

body, err := c.buildReservationBody(rec, "")
body, err := c.buildReservationBody(rec, "", "")
require.NoError(t, err)

var got map[string]interface{}
require.NoError(t, json.Unmarshal(body, &got))
_, present := got["tags"]
assert.False(t, present, "tags must be absent when source is empty")
assert.False(t, present, "tags must be absent when both source and idempotency token are empty")
}

// TestBuildReservationBody_IncludesIdempotencyTokenTag pins the issue #721
// fix: the cudly-idempotency-token tag MUST ride along with the
// purchase-automation tag in the reservation body so a re-driven purchase
// can find the prior reservation via FindReservationOrderByIdempotencyToken
// and skip the duplicate buy.
func TestBuildReservationBody_IncludesIdempotencyTokenTag(t *testing.T) {
c := &ComputeClient{region: "eastus", subscriptionID: "sub-abc"}
rec := common.Recommendation{ResourceType: "Standard_D2s_v3", Count: 1, Term: "1yr"}
token := common.DeriveIdempotencyToken("exec-721-compute", 0)

body, err := c.buildReservationBody(rec, common.PurchaseSourceWeb, token)
require.NoError(t, err)

var got map[string]interface{}
require.NoError(t, json.Unmarshal(body, &got))
tags, ok := got["tags"].(map[string]interface{})
require.True(t, ok, "tags map missing from reservation body")
assert.Equal(t, common.PurchaseSourceWeb, tags[common.PurchaseTagKey])
assert.Equal(t, token, tags[common.IdempotencyTagKey], "idempotency tag must be stamped when token is supplied")
}

// --- Issue #148: VCPU/MemoryGB enrichment via cached SKU catalogue ---
Expand Down
28 changes: 8 additions & 20 deletions providers/azure/services/cosmosdb/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,13 @@ func (c *CosmosDBClient) PurchaseCommitment(ctx context.Context, rec common.Reco
Timestamp: time.Now(),
}

// Azure's reservation API mints the order ID server-side in calculatePrice,
// so the only stable dedupe signal we control is the purchase-automation tag
// derived from opts.Source. Without a non-empty Source the tag is dropped and
// a re-driven purchase cannot recognise the prior attempt, producing
// duplicate reservations. Fail fast at function entry rather than allowing
// an un-tagged, non-idempotent purchase to hit the cloud.
// 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 idempotent Azure reservation purchases")
result.Error = fmt.Errorf("purchase source is required for Azure reservation purchases")
return result, result.Error
}

Expand Down Expand Up @@ -305,7 +304,7 @@ func (c *CosmosDBClient) PurchaseCommitment(ctx context.Context, rec common.Reco
"renew": false,
},
}
applyPurchaseAutomationTag(requestBody, opts.Source)
reservations.ApplyPurchaseTags(requestBody, opts.Source, opts.IdempotencyToken)

bodyBytes, err := json.Marshal(requestBody)
if err != nil {
Expand All @@ -321,7 +320,7 @@ func (c *CosmosDBClient) PurchaseCommitment(ctx context.Context, rec common.Reco
return result, result.Error
}

reservationOrderID, err := reservations.DoPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token)
reservationOrderID, err := reservations.DoIdempotentPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token, opts.IdempotencyToken)
if err != nil {
result.Error = err
return result, result.Error
Expand Down Expand Up @@ -751,14 +750,3 @@ func detailsFromCosmosSKU(sku string) common.NoSQLDetails {
}
return d
}

// applyPurchaseAutomationTag attaches the purchase-automation tag to an Azure
// reservation request body when source is non-empty. Extracted out of
// PurchaseCommitment to keep the function under the cyclomatic-complexity
// threshold enforced by the pre-commit hook.
func applyPurchaseAutomationTag(body map[string]interface{}, source string) {
if source == "" {
return
}
body["tags"] = map[string]string{common.PurchaseTagKey: source}
}
28 changes: 8 additions & 20 deletions providers/azure/services/database/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,14 +274,13 @@ func (c *DatabaseClient) PurchaseCommitment(ctx context.Context, rec common.Reco
Timestamp: time.Now(),
}

// Azure's reservation API mints the order ID server-side in calculatePrice,
// so the only stable dedupe signal we control is the purchase-automation tag
// derived from opts.Source. Without a non-empty Source the tag is dropped and
// a re-driven purchase cannot recognise the prior attempt, producing
// duplicate reservations. Fail fast at function entry rather than allowing
// an un-tagged, non-idempotent purchase to hit the cloud.
// 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 idempotent Azure reservation purchases")
result.Error = fmt.Errorf("purchase source is required for Azure reservation purchases")
return result, result.Error
}

Expand Down Expand Up @@ -313,7 +312,7 @@ func (c *DatabaseClient) PurchaseCommitment(ctx context.Context, rec common.Reco
"renew": false,
},
}
applyPurchaseAutomationTag(requestBody, opts.Source)
reservations.ApplyPurchaseTags(requestBody, opts.Source, opts.IdempotencyToken)

bodyBytes, err := json.Marshal(requestBody)
if err != nil {
Expand All @@ -329,7 +328,7 @@ func (c *DatabaseClient) PurchaseCommitment(ctx context.Context, rec common.Reco
return result, result.Error
}

reservationOrderID, err := reservations.DoPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token)
reservationOrderID, err := reservations.DoIdempotentPurchaseTwoStep(ctx, c.httpClient, reservations.CalculatePriceURL(), bodyBytes, token.Token, opts.IdempotencyToken)
if err != nil {
result.Error = err
return result, result.Error
Expand Down Expand Up @@ -696,14 +695,3 @@ func detailsFromSQLSKU(sku string) common.DatabaseDetails {
}
return d
}

// applyPurchaseAutomationTag attaches the purchase-automation tag to an Azure
// reservation request body when source is non-empty. Extracted out of
// PurchaseCommitment to keep the function under the cyclomatic-complexity
// threshold enforced by the pre-commit hook.
func applyPurchaseAutomationTag(body map[string]interface{}, source string) {
if source == "" {
return
}
body["tags"] = map[string]string{common.PurchaseTagKey: source}
}
Loading
Loading