[cristim]

Summary

• Replaces azidentity.NewDefaultAzureCredential in buildAzureExchangeClient with credentials.ResolveAzureTokenCredentialWithOpts, scoped to the registered CloudAccount whose ExternalID matches the requested subscription_id query param.
• Mirrors the exact credential-resolution pattern used by scheduler.collectAzureForAccount and purchase/execution.go resolveAzureProvider — no new auth path invented.
• Graceful empty state: when no Azure CloudAccount is registered for the requested subscription, the handler returns an empty reservations list rather than an opaque 500.
• Error classification: DB/credential failures are server-side (5xx, no change); missing account is graceful empty state (not an error).

Closes #932

Test plan

• go build ./internal/api/... passes
• go test ./internal/api/... -run "TestListExchangeableAzureRIs|TestBuildAzureExchangeClient" — all 10 tests pass (5 pre-existing + 5 new)
• go test ./internal/api/... ./internal/credentials/... — 1483 tests pass
• gofmt -l produces no output on touched files
• go vet ./internal/api/... produces no output

New tests cover: no Azure account registered (graceful empty), empty subscription with no accounts, DB lookup error (5xx, not 4xx), credential resolution failure (5xx), resolver called with correct (provider, subscriptionID) arguments.

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of Azure Reserved Instances queries when no Azure account is registered, now gracefully returns empty results instead of errors.
  • Enhanced error handling for Azure credential resolution failures.

• Tests

  • Added comprehensive test coverage for Azure credential resolution and account lookup scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 17ce722f-6eb7-45e9-8699-8e4b37e03912

📥 Commits

Reviewing files that changed from the base of the PR and between 56a7797 and a2674b7.

📒 Files selected for processing (2)

• internal/api/handler_ri_exchange.go
• internal/api/handler_ri_exchange_test.go

---

📝 Walkthrough

Walkthrough

The PR refactors Azure credential resolution in the RI exchange handler to use per-subscription CloudAccount lookups and the project's credential resolver instead of Azure SDK's default credential chain. The handler now returns graceful empty-state responses when no Azure account is registered, and propagates errors from account lookup or credential resolution failures.

Changes

Azure RI Exchange Credential Resolution

Layer / File(s)	Summary
Azure client builder with credential resolution internal/api/handler_ri_exchange.go	Removed Azure default credential import and added internal credential resolver. Refactored buildAzureExchangeClient to accept context, look up registered Azure CloudAccount by subscription ID via GetCloudAccountByExternalID, resolve credentials with ResolveAzureTokenCredentialWithOpts (signer/issuer wired), and return (nil, nil) when no account is configured.
Handler integration and empty-account guard internal/api/handler_ri_exchange.go	Updated listExchangeableAzureRIs to call the refactored buildAzureExchangeClient(ctx, subscriptionID) and added nil-client guard that returns empty ExchangeableAzureRIsResponse instead of attempting to list reservations.
Comprehensive test coverage for credential resolution and error paths internal/api/handler_ri_exchange_test.go	Added five test functions: TestListExchangeableAzureRIs_NoAzureAccountRegistered (graceful empty when no account), TestListExchangeableAzureRIs_EmptySubscriptionNoAccounts (empty response when subscription_id omitted and no accounts), TestListExchangeableAzureRIs_AccountLookupError (DB error propagation), TestListExchangeableAzureRIs_CredentialResolutionError (credential resolution error propagation), and TestBuildAzureExchangeClient_UsesResolver (wiring verification).

Sequence Diagram

sequenceDiagram
  participant Handler as listExchangeableAzureRIs
  participant ClientBuilder as buildAzureExchangeClient
  participant ConfigStore as CloudAccount Lookup
  participant CredResolver as ResolveAzureTokenCredentialWithOpts
  participant ARMClient as armreservations.ClientFactory

  Handler->>ClientBuilder: ctx, subscriptionID
  ClientBuilder->>ConfigStore: GetCloudAccountByExternalID("azure", subscriptionID)
  alt Account found
    ConfigStore-->>ClientBuilder: CloudAccount
    ClientBuilder->>CredResolver: Resolve credentials with signer/issuer
    alt Credentials resolved
      CredResolver-->>ClientBuilder: Azure token credential
      ClientBuilder->>ARMClient: Create reservations client
      ARMClient-->>ClientBuilder: Client
      ClientBuilder-->>Handler: Client, nil error
      Handler->>ARMClient: List reservations
    else Credential resolution fails
      CredResolver-->>ClientBuilder: Error
      ClientBuilder-->>Handler: nil, wrapped error
      Handler->>Handler: Return error
    end
  else No account registered
    ConfigStore-->>ClientBuilder: nil, nil
    ClientBuilder-->>Handler: nil, nil
    Handler->>Handler: Return empty Reservations list
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• LeanerCloud/CUDly#595: Previously added the Azure RI exchange endpoint logic that this PR refactors to fix credential resolution in the buildAzureExchangeClient path.

Suggested labels

effort/s

Poem

🐰 Hops past the Azure SDK's defaults today,
Credential resolver shows the CloudAccount way,
No registered account? No 500 in sight,
Just empty Reservations, graceful and bright!

🚥 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 'fix(ri-exchange): resolve Azure credentials via per-subscription resolver' accurately and concisely summarizes the main change: replacing direct Azure credential creation with per-subscription credential resolution.
Linked Issues check	✅ Passed	The PR fully addresses issue #932: replaces azidentity.NewDefaultAzureCredential with credentials.ResolveAzureTokenCredentialWithOpts scoped to subscription_id, returns graceful empty state when no account exists, and properly classifies errors.
Out of Scope Changes check	✅ Passed	All changes are strictly scoped to the Azure RI Exchange credential resolution: handler refactoring, list function updates, import adjustments, and corresponding test coverage. No extraneous changes detected.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 fix/871-azure-ri-credentials

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cristim]

@coderabbitai review

[coderabbitai]

✅ 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]

@coderabbitai review

[coderabbitai]

✅ 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]

@coderabbitai review

[coderabbitai]

✅ 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]

@coderabbitai review

[coderabbitai]

✅ 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.