[cristim]

Closes #1462

Summary

go test runs were popping macOS keychain password prompts (naming pwsh) and issuing live network calls to Azure. Traced via process ancestry of a live prompt: Azure tests were walking the real DefaultAzureCredential fallback chain and calling real Azure endpoints.

Two atomic commits:

1. test(secrets) — azure_resolver_coverage_test.go called the real NewAzureResolver against test-vault.vault.azure.net; the Key Vault 401 challenge walked the live credential chain (IMDS probe, az account get-access-token retries, pwsh Get-AzAccessToken → keychain prompt). The t.Skipf guards never fired because credential construction always succeeds on a dev box. Rewrote the live tests onto the package's existing fake-credential httptest harness; constructor-only tests now fail loud instead of silently skipping. Added TestMain pinning AZURE_TOKEN_CREDENTIALS=EnvironmentCredential with dummy env credentials so no test in the package can reach the az/azd/pwsh/IMDS legs, plus a regression guard test that fails if the pin is removed.
2. test(azure) — the two recommendations-adapter tests built real armconsumption/armadvisor clients and issued live HTTPS requests to management.azure.com on every run, discarding both return values. Rewrote them onto the existing newXxxClientFn/getAdvisorRecsFn injection points (shared overrideServiceClientFns helper with t.Cleanup restore) with real assertions on returned recommendations.

Verification

• internal/secrets: 245/245 pass, offline in ~4s (was ~2min of network retries).
• providers/azure module: 710/710 pass, gofmt/vet clean.
• Positive proof of no credential-helper usage: a pwsh process watcher stayed silent and ~/.Azure/commands/ gained zero entries across timed runs of both suites.

Follow-ups filed (out of scope here)

• chore(api): add injection seam for Azure revoke clients (real DefaultAzureCredential built inline) #1463 — revoke handler builds real DefaultAzureCredential/armreservations clients with no injection seam
• chore(oidc): add injection seam to NewAzureKeyVaultSigner and cover fully-configured factory branch #1464 — NewAzureKeyVaultSigner no-seam construction; fully-configured factory branch untested
• chore(providers/azure): replace silent IsConfigured() test guards with injected fake credential provider #1465 — provider_test.go silent IsConfigured() guards
• chore(providers/azure): reuse shared mocks package in synapse/managedredis client tests #1466 — synapse/managedredis duplicate the shared mocks package

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Improved Azure test reliability by removing dependency on real cloud services and making resolver behavior fully deterministic via HTTP mocking.
  • Added coverage to ensure Azure credential selection is restricted to non-interactive environment-based credentials.
  • Expanded tests for error handling, cancellation, filtering, missing secrets, and proper JSON retrieval behavior.
  • Updated Azure recommendation adapter tests to avoid live API calls, using injected fakes and stronger result assertions.

[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: c8a4eea3-96a7-410c-8c0a-5cd932c8d4e5

📥 Commits

Reviewing files that changed from the base of the PR and between 13246cc and 50c56e1.

📒 Files selected for processing (1)

• internal/secrets/azure_resolver_coverage_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• internal/secrets/azure_resolver_coverage_test.go

---

📝 Walkthrough

Walkthrough

Azure tests now avoid live credential and service calls by restricting the secrets credential chain, using mocked Key Vault responses, and injecting fake recommendation clients with concrete assertions.

Changes

Azure test hermeticity

Layer / File(s)	Summary
Restrict Azure credential chains internal/secrets/azure_credential_guard_test.go	Package tests pin Azure authentication to EnvironmentCredential and verify the environment restriction.
Use mocked Key Vault responses internal/secrets/azure_resolver_coverage_test.go	Resolver tests use HTTP mocks, require successful construction, validate 404 and filtering behavior, and assert close, cancellation, and input-handling results.
Inject recommendation service clients providers/azure/recommendations_test.go	Adapter tests replace service and advisor constructors with fakes and assert returned recommendations and nil errors.

Estimated code review effort: 3 (Moderate) | ~20 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 accurately summarizes the PR's main change: mocking Azure credentials and service APIs in tests.
Linked Issues check	✅ Passed	The PR addresses #1462 by removing live credential-chain and Azure API calls, adding offline mocks and assertions.
Out of Scope Changes check	✅ Passed	The changes stay focused on Azure test isolation and regression coverage, with no unrelated code paths added.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ 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 test/azure-cred-mocking

---

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

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/secrets/azure_resolver_coverage_test.go`:
- Around line 140-142: Strengthen the canceled-context test around
resolver.GetSecret by tracking whether notFoundHandler is invoked and asserting
that it remains uncalled after the request. Keep the existing error assertion,
but ensure the test fails if cancellation is ignored and an HTTP request reaches
the server.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1139a3a2-2550-4b33-be97-d9db9c7deaca

📥 Commits

Reviewing files that changed from the base of the PR and between 5c267c5 and 13246cc.

📒 Files selected for processing (3)

• internal/secrets/azure_credential_guard_test.go
• internal/secrets/azure_resolver_coverage_test.go
• providers/azure/recommendations_test.go