Add Azure Managed Identity support for BYOK providers#1745
Closed
SteveSandersonMS wants to merge 7 commits into
Closed
Add Azure Managed Identity support for BYOK providers#1745SteveSandersonMS wants to merge 7 commits into
SteveSandersonMS wants to merge 7 commits into
Conversation
Expose managedIdentity on ProviderConfig and NamedProviderConfig so SDK consumers can authenticate a BYOK provider with an Azure managed identity instead of a static apiKey/bearerToken. Adds the ManagedIdentityConfig and ManagedIdentityOptions public types, regenerates the wire types from the schema, and exports the new types from the package entrypoint. The session client already forwards provider/providers config wholesale to the runtime, so no transport change is needed. Adds a hermetic e2e test that stands up a local mock managed-identity endpoint and a local mock model endpoint, runs a real BYOK turn, and asserts the runtime acquired the token and injected it as Authorization: Bearer, with the correct resource and identity-selector query params for both system-assigned and user-assigned (clientId) identities. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the hand-written ManagedIdentityConfig/ManagedIdentityOptions definitions in types.ts with re-exports of the codegen-generated types so they stay in sync with the runtime schema automatically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extract the Azure App Service managed identity token mock out of the Node e2e test into a standalone, language-agnostic entrypoint under test/harness so every SDK can reuse it. mockIdentityEndpoint.ts hosts the endpoint (token contract + recorded-request/reset/stop control paths) and mockIdentityServer.ts is the spawnable process that prints a parseable Listening: banner. The Node e2e test now drives it via a small MockIdentityServer wrapper, keeping only its own BYOK model mock. Kept separate from the CAPI record/replay proxy so it can be spawned on its own. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Extend the shared mock identity endpoint with configurable token lifetime and rotation (and record the issued token per request), then add two e2e tests: one proves a valid token is cached across turns (endpoint hit once), the other proves the runtime auto-refreshes once the token falls within the 5-minute refresh buffer (short expires_in + rotation -> new bearer reaches the model on the next turn). No real-time wait needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The runtime changed `managedIdentity` from a `boolean | object` union to a
single all-optional object `ManagedIdentityConfig` (an empty object now
selects the system-assigned identity, replacing the old `true`). The union
was rejected by the runtime's own SDK-mappability lint and could not be
code-generated for statically-typed SDKs.
- Regenerate the MI types in generated/rpc.ts: drop the
`boolean | ManagedIdentityOptions` alias and rename the
`ManagedIdentityOptions` interface to `ManagedIdentityConfig`.
- Drop the now-removed `ManagedIdentityOptions` re-export from types.ts and
index.ts.
- Update the system-assigned e2e config from `managedIdentity: true` to `{}`.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SteveSandersonMS
changed the title
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1745 · sonnet46 2.1M
| } from "./generated/rpc.js"; | ||
| import type { ToolSet } from "./toolSet.js"; | ||
| export type { RemoteSessionMode } from "./generated/rpc.js"; | ||
| export type { ManagedIdentityConfig } from "./generated/rpc.js"; |
Contributor
There was a problem hiding this comment.
This export makes ManagedIdentityConfig part of the Node.js public API surface. The equivalent type and field additions are still needed in the other 5 SDKs:
- Python:
managed_identity: ManagedIdentityConfig | NoneinProviderConfigandNamedProviderConfigTypedDicts (python/copilot/session.py) - .NET:
ManagedIdentityConfigclass +ManagedIdentityproperty onProviderConfigandNamedProviderConfig(dotnet/src/Types.cs) - Go:
ManagedIdentity *ManagedIdentityConfigfield inProviderConfigandNamedProviderConfig(go/rpc/zrpc.go) - Rust:
managed_identity: Option<ManagedIdentityConfig>+with_managed_identity()builder inrust/src/types.rs - Java: New
ManagedIdentityConfig.javaclass + field inProviderConfig.javaandNamedProviderConfig.java
For the auto-generated SDKs (Python, .NET, Go, Rust), the shared JSON schema should be updated and codegen re-run.
The managed-identity e2e test previously spun up its mock BYOK model endpoint inline via http.createServer, so it could not be reused by other SDK languages. Move it into the shared harness (mockModelEndpoint.ts + mockModelServer.ts entrypoint, mirroring the mock identity endpoint) and have the Node test spawn it through a MockModelServer wrapper. Every SDK language can now spawn the same endpoint and parse its Listening: banner. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Surface the BYOK managed-identity config in the .NET SDK and cover it with a hermetic e2e test that reuses the shared mock harness servers. - Regenerate Rpc.cs to add the ManagedIdentityConfig wire type (clientId / objectId / resourceId / scope) and register it in the serializer context. - Hand-add the managedIdentity property to the hand-written ProviderConfig and NamedProviderConfig in Types.cs (these public types are not codegen-owned), plus a TypesJsonContext registration. - Add npm scripts so the shared mock identity and mock model servers in test/harness can be spawned standalone, and a MockHarnessServer C# helper (with MockIdentityServer / MockModelServer) that spawns them and parses the Listening: banner, mirroring CapiProxy and the Node wrappers. - Add ManagedIdentityE2ETests covering system-assigned, user-assigned (clientId + custom scope), token caching, and automatic refresh. Passes on net8.0 (AOT-safe, reflection serializer disabled) and net472. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Cross-SDK Consistency ReviewThis PR adds However, four SDKs are missing equivalent support, creating a cross-language parity gap: What's missing in each SDK
All four SDKs' generated files ( Suggested shape for each SDKGo ( // ManagedIdentityConfig configures Azure managed identity authentication for a BYOK provider.
// Mutually exclusive with APIKey/BearerToken.
//
// Experimental.
type ManagedIdentityConfig struct {
// ClientId is the client (application) ID of a user-assigned managed identity.
ClientId string `json:"clientId,omitempty"`
// ObjectId is the object (principal) ID of a user-assigned managed identity.
ObjectId string `json:"objectId,omitempty"`
// ResourceId is the ARM resource ID of a user-assigned managed identity.
ResourceId string `json:"resourceId,omitempty"`
// Scope overrides the AAD token scope. Defaults to (cognitiveservices.azure.com/redacted)
Scope string `json:"scope,omitempty"`
}And add Python ( class ManagedIdentityConfig(TypedDict, total=False):
"""Azure managed identity authentication for a BYOK provider.
Mutually exclusive with api_key/bearer_token.
"""
client_id: str # client (application) ID of a user-assigned identity
object_id: str # object (principal) ID of a user-assigned identity
resource_id: str # ARM resource ID of a user-assigned identity
scope: str # AAD token scope; defaults to (cognitiveservices.azure.com/redacted)And add Rust ( /// Azure managed identity authentication for a BYOK provider.
/// Mutually exclusive with `api_key`/`bearer_token`.
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct ManagedIdentityConfig {
#[serde(default, skip_serializing_if = "Option::is_none")]
pub client_id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub object_id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub resource_id: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub scope: Option<String>,
}And add Java ( If the other SDKs are intentionally being deferred to a follow-up PR, it would be helpful to note that in the PR description so reviewers are aware of the phased rollout plan.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| finally | ||
| { | ||
| // disconnect may fail since the BYOK provider is a local mock | ||
| try { await session.DisposeAsync(); } catch { /* ignore */ } |
| await PostAsync(url); | ||
| } | ||
| } | ||
| catch { /* best effort; fall through to killing the process */ } |
| if (_process is { HasExited: false }) | ||
| { | ||
| try { _process.Kill(entireProcessTree: true); await _process.WaitForExitAsync(); } | ||
| catch { /* ignore */ } |
| var dir = new DirectoryInfo(AppContext.BaseDirectory); | ||
| while (dir != null) | ||
| { | ||
| if (Directory.Exists(Path.Combine(dir.FullName, "nodejs"))) |
Contributor
Author
|
Closing in favour of #1748 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Azure Managed Identity (MI) support to the Node SDK for BYOK providers, matching the runtime contract. App developers can authenticate a BYOK provider with a system- or user-assigned managed identity instead of a static
apiKey/bearerToken, and the runtime handles token acquisition, caching, and refresh automatically.Usage
managedIdentityis the generatedManagedIdentityConfigtype. It is a single all-optional object: an empty object selects the system-assigned identity; setting at most one ofclientId/objectId/resourceIdselects a user-assigned identity;scopeoverrides the token audience. It is mutually exclusive withapiKey/bearerToken.What's included
ManagedIdentityConfigtype from the public SDK surface (types.ts,index.ts).test/harness/) plus a Node entrypoint, so other-language SDK e2e suites can reuse the same fake IMDS/App-Service identity server.managed_identity.e2e.test.ts): system-assigned bearer injection, user-assignedclientIdwith custom scope, token caching across turns, and token refresh within the expiry buffer — no network access.Wire shape
managedIdentityis a single object (not aboolean | objectunion). The union shape was rejected by the runtime's own SDK-mappability lint and could not be code-generated for statically-typed SDKs, so the runtime contract uses one all-optional object;{}replaces the formertrue.Testing
npm run typecheck(both tsconfigs), prettier, eslint: greendist-cli