Uh oh!
There was an error while loading. Please reload this page.
Support managed identity auth in AzureFoundry provider - #9707
Conversation
The AzureFoundry chat client provider now authenticates with DefaultAzureCredential (Entra ID / managed identity) when no AZURE_OPENAI_API_KEY is set, keeping the provider secure-by-default for Azure-hosted scenarios. The API key remains an explicit fallback, and IsAvailable no longer requires an API key. Fixes#8412 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR makes the Microsoft.Testing.Extensions.AzureFoundry provider support Microsoft Entra ID / managed-identity authentication instead of hard-requiring AZURE_OPENAI_API_KEY, closing the secure-by-default gap described in issue #8412. The provider now reports itself available whenever the endpoint and deployment name are configured, and it authenticates via DefaultAzureCredential when no API key is supplied, keeping the explicit API-key path as an override.
Changes:
IsAvailableno longer requiresAZURE_OPENAI_API_KEY(only endpoint + deployment name).CreateChatClientAsyncusesApiKeyCredentialwhen a key is present, otherwise falls back toDefaultAzureCredential.- Adds the
Azure.Identitydependency (version pinned via CPM) and documents both auth modes inPACKAGE.md.
Show a summary per file
| File | Description |
|---|---|
| src/Platform/Microsoft.Testing.Extensions.AzureFoundry/OpenAIChatClientProvider.cs | Drops the mandatory API-key check and selects DefaultAzureCredential vs ApiKeyCredential based on whether the key is set. |
| src/Platform/Microsoft.Testing.Extensions.AzureFoundry/PACKAGE.md | Adds a configuration table and prose documenting the managed-identity and API-key authentication modes. |
| src/Platform/Microsoft.Testing.Extensions.AzureFoundry/Microsoft.Testing.Extensions.AzureFoundry.csproj | References the new Azure.Identity package. |
| Directory.Packages.props | Pins Azure.Identity version 1.14.2 under central package management. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Medium
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
| # | Dimension | Verdict |
|---|---|---|
| 13 | Test Completeness | 🟡 1 MODERATE |
| 20 | Build Infrastructure & Dependencies | 🟡 1 MODERATE |
| 17 | Documentation Accuracy | ⚪ 1 NIT |
✅ 19/22 dimensions clean (dimensions 10–12, 14, 18–19, 22 skipped as N/A).
- Test Completeness — no unit tests cover the new
DefaultAzureCredentialbranch (or any path ofAzureOpenAIChatClientProvider) - Build Infrastructure —
Azure.Identityadds a heavyweight transitive dependency graph; acceptable whilealpha/non-shipping, but worth revisiting before GA - Documentation — consider noting local-dev latency from
DefaultAzureCredentialcredential chain traversal
Overall: The implementation is clean, correct, and well-documented. The ternary conditional for credential selection is straightforward. The IsAvailable change correctly reflects the new optionality of the API key. No correctness, threading, security, or API compatibility concerns.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Reuse a single DefaultAzureCredential instance (Lazy) so the token cache and credential-chain discovery are shared across calls instead of being rebuilt on every CreateChatClientAsync invocation. - Add unit tests for the auth-selection logic (IsAvailable, ModelName, API-key vs Entra path, missing-variable failures) in Microsoft.Testing.Extensions.UnitTests, with env-var snapshot/restore and DoNotParallelize; grant InternalsVisibleTo to the test assembly. - Document AZURE_CLIENT_ID (user-assigned managed identity) and the deferred authentication-failure behavior in PACKAGE.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
- Bump Azure.Identity 1.14.2 -> 1.21.0. The 1.14.x line is deprecated (depends on a deprecated MSAL); 1.21.0 is the current supported release. Verified clean restore/build across all TFMs (no NU1605, no audit warnings) and compatible with Azure.AI.OpenAI 2.1.0. - Extract an internal AuthenticationMode + GetAuthenticationMode seam so the API-key-vs-DefaultAzureCredential selection is deterministically unit-tested (upgrades the two 'only IsNotNull' tests and adds three dedicated selection tests). - Document DefaultAzureCredential local-dev credential-chain latency in PACKAGE.md. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🧪 Test quality grade — PR #970714 new tests graded across 1 file. 12 earn A for clean AAA structure, proper env-var isolation via
This advisory comment was generated automatically. Grades are heuristic
|
Uh oh!
There was an error while loading. Please reload this page.
- AzureFoundry: add DefaultAzureCredential/managed identity auth details and required environment variables (PR #9707) - MSTestTestFramework: new entry documenting the native MTP ITestFramework for MSTest introduced by RFC 018 (PRs #9706, #9743, #9748, #9755) - VSTestBridge: note MSTest no longer depends on it on the MTP path as of MSTest 4.3 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- AzureFoundry: add DefaultAzureCredential/managed identity auth details and required environment variables (PR #9707) - MSTestTestFramework: new entry documenting the native MTP ITestFramework for MSTest introduced by RFC 018 (PRs #9706, #9743, #9748, #9755) - VSTestBridge: note MSTest no longer depends on it on the MTP path as of MSTest 4.3 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes#8412
Summary
Microsoft.Testing.Extensions.AzureFoundrypreviously hard-requiredAZURE_OPENAI_API_KEYand always constructedAzureOpenAIClientwithApiKeyCredential, with no managed-identity / Entra path. This made the only built-in AI provider not secure-by-default for Azure-hosted scenarios.Changes
IsAvailablenow only requiresAZURE_OPENAI_ENDPOINTandAZURE_OPENAI_DEPLOYMENT_NAME; the API key is no longer mandatory.CreateChatClientAsyncprefers an explicitAZURE_OPENAI_API_KEYwhen present, otherwise falls back toDefaultAzureCredential(managed identity, workload identity, Azure CLI, Visual Studio, …).Azure.Identitypackage reference (version pinned inDirectory.Packages.props).PACKAGE.mdwith a configuration table documenting both authentication modes.Notes
.resx/.xlfchanges were needed — the existingEnvironmentVariableNotSetstring is still used for the endpoint/deployment checks.