Skip to content

refactor(api): inject Azure revoke client factory - #1471

Merged
cristim merged 2 commits into
mainfrom
chore/1463-azure-revoke-seam
Jul 21, 2026
Merged

cristim merged 2 commits into
mainfrom
chore/1463-azure-revoke-seam

Conversation

@cristim

@cristim cristim commented Jul 19, 2026 •

Copy link
Copy Markdown
Member

Closes #1463

Problem

calculateAzureRevoke and revokeAzurePurchase in
internal/api/handler_purchases_revoke.go each called
azidentity.NewDefaultAzureCredential(nil) and then
armreservations.NewCalculateRefundClient / armreservations.NewReturnClient
inline, with no injection seam. The injectable interfaces
(azureCalculateRefundClient / azureReturnClient) already existed and were
consumed by callAzureReturn, but any test that reached either of these two
functions directly would walk the live Azure credential chain (spawning
az/pwsh, popping macOS keychain prompts).

Fix

Added Handler.newAzureRevokeClients, an injectable factory field mirroring
the AzureSPProber.NewClient pattern (internal/commitmentopts/probe_azure.go).
A nil field means "use the production default", so all existing production
construction sites are unchanged. Both handlers now route through a new
buildAzureRevokeClients helper that returns the injected factory's result
when set, or falls back to the original DefaultAzureCredential +
armreservations construction otherwise. Error message text is preserved
exactly on both paths.

Tests added

  • TestCalculateAzureRevoke_Success / TestCalculateAzureRevoke_ClientFactoryError
  • TestRevokeAzurePurchase_Success / TestRevokeAzurePurchase_ClientFactoryError

All drive the two previously-unreachable handlers end-to-end through the new
seam using the existing stubCalcRefundClient/stubReturnClient fakes, with
no live Azure client involved.

Verification

  • gofmt -l internal/ — clean
  • go vet ./internal/api/ — clean
  • go test ./internal/api/ -count=1 — 1815 passed
  • go build ./... — clean

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved reliability of Azure purchase refund and revocation flows.
    • Azure service connection failures now produce clearer, more consistent error responses.
    • Existing refund calculations and purchase revocations continue to return the expected results when successful.
  • Tests

    • Added coverage for successful Azure refund and revocation requests, as well as service connection failures.

calculateAzureRevoke and revokeAzurePurchase each constructed
azidentity.NewDefaultAzureCredential and the armreservations clients
inline, so no test could reach either function without walking the
live Azure credential chain (az/pwsh spawn, macOS keychain prompts).

Add Handler.newAzureRevokeClients, a factory seam mirroring the
AzureSPProber.NewClient pattern: nil means "use the production
default" (unchanged credential + client construction), and tests can
override it with the existing stubCalcRefundClient/stubReturnClient
fakes. Both handlers now route through buildAzureRevokeClients, with
new tests covering the happy path and a factory-error path for each.
@cristim cristim added triaged Item has been triaged priority/p2 Backlog-worthy severity/low Minor harm urgency/this-quarter Within the quarter impact/internal Team-internal only effort/s Hours type/chore Maintenance / non-user-visible labels Jul 19, 2026
@coderabbitai

coderabbitai Bot commented Jul 19, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: afd97486-6a67-43b4-9a73-f7eeb462251a

📥 Commits

Reviewing files that changed from the base of the PR and between c848a07 and 4abfc33.

📒 Files selected for processing (3)
  • internal/api/handler.go
  • internal/api/handler_purchases_revoke.go
  • internal/api/handler_purchases_revoke_test.go

📝 Walkthrough

Walkthrough

Azure revoke client construction is centralized behind an optional injected factory. Both revoke handlers use shared builders, and tests cover successful client creation plus credential-factory error propagation.

Changes

Azure revoke injection

Layer / File(s) Summary
Factory seam and client builders
internal/api/handler.go, internal/api/handler_purchases_revoke.go
Handler accepts an optional Azure revoke factory; shared builders create credentials and Azure clients, cache the factory, and wrap construction errors.
Revoke flow integration and tests
internal/api/handler_purchases_revoke.go, internal/api/handler_purchases_revoke_test.go
Calculate and purchase revoke handlers use the shared builders, with tests covering successful construction and propagated factory errors.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: injecting an Azure revoke client factory for the API handlers.
Linked Issues check ✅ Passed The PR implements the requested injection seam for Azure revoke clients and adds tests that avoid live Azure credential calls.
Out of Scope Changes check ✅ Passed The changes stay focused on Azure revoke client injection and related tests, with no obvious unrelated additions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/1463-azure-revoke-seam

Comment @coderabbitai help to get the list of available commands.

@cristim

cristim commented Jul 19, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 19, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cristim
cristim merged commit 1efb314 into main Jul 21, 2026
19 checks passed
@cristim
cristim deleted the chore/1463-azure-revoke-seam branch July 27, 2026 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/internal Team-internal only priority/p2 Backlog-worthy severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(api): add injection seam for Azure revoke clients (real DefaultAzureCredential built inline)

1 participant