[Agents Extension] Updates to pull rbac identity from the agent version#7747
[Agents Extension] Updates to pull rbac identity from the agent version#7747
Conversation
Signed-off-by: trangevi <trangevi@microsoft.com>
trangevi
requested review from
JeffreyCA,
hemarina,
jongio,
vhvb1989,
wbreza and
weikanglim
as code owners
trangevi
requested review from
rajeshkamal5050,
tg-msft,
therealjohn and
trrwilson
as code owners
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates the agents extension to source per-agent RBAC principal IDs from the agent version object (when available) to avoid Graph-based identity discovery.
Changes:
- Extends agent version API models to include instance identity, blueprint identity/reference, status, and agent GUID.
- Updates post-deploy RBAC setup to fetch agent versions and pass principal IDs through to RBAC assignment (falling back to Graph when missing).
- Enhances
showoutput/tests to surface the newly added agent version fields and switches vnextshowto the vnext API version constant.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go | Accepts name→principalID map and skips Graph discovery when principal IDs are known. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/models.go | Adds fields/types to represent identity/blueprint info on agent version objects. |
| cli/azd/extensions/azure.ai.agents/internal/pkg/agents/agent_api/models_test.go | Expands round-trip coverage for newly added agent version fields. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/show.go | Uses vnext API version for vnext show and prints new fields in table format. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/show_test.go | Validates JSON/table output includes the newly added fields. |
| cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go | Postdeploy now fetches agent versions to obtain principal IDs for RBAC assignment. |
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
Nice optimization - skipping Graph API polling is a solid improvement. A few things to sort out before this is ready:
The main concern is that the new credential + API calls in postdeployHandler run unconditionally, before the vnext gate in EnsureAgentIdentityRBAC. This means non-vnext projects pay for unnecessary HTTP calls and can fail on missing env vars. Additionally, GetAgentVersion uses the older API version while show.go was updated to the vnext version in this same PR - if InstanceIdentity requires the newer version, the optimization won't actually kick in.
Move the Conditional Access token protection (AADSTS530084) error detection from the preflight developer RBAC check into the postdeploy agent identity RBAC path, specifically in the Graph API fallback used when the agent version API does not return a principal ID. This aligns with the PR Azure#7747 changes that pull identity info from the agent API directly — the token protection check is now only needed in the edge case fallback path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…P logging
- Gate postdeployHandler on isVNextEnabled early to avoid unnecessary API
calls and credential creation for non-vnext projects
- Use DefaultVNextAgentAPIVersion for GetAgentVersion in postdeploy so
instance_identity is actually returned by the API
- Restore continue for missing/empty AGENT_{KEY}_NAME/VERSION env vars
instead of hard errors; separate RPC failures from empty values
- Remove redundant isVnextEnabled check from EnsureAgentIdentityRBAC
(now checked by caller)
- Log when Graph API discovery finds multiple service principals for
a single agent display name
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
Move the Conditional Access token protection (AADSTS530084) error detection from the preflight developer RBAC check into the postdeploy agent identity RBAC path, specifically in the Graph API fallback used when the agent version API does not return a principal ID. This aligns with the PR Azure#7747 changes that pull identity info from the agent API directly — the token protection check is now only needed in the edge case fallback path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…7713) Move the Conditional Access token protection (AADSTS530084) error detection from the preflight developer RBAC check into the postdeploy agent identity RBAC path, specifically in the Graph API fallback used when the agent version API does not return a principal ID. This aligns with the PR #7747 changes that pull identity info from the agent API directly — the token protection check is now only needed in the edge case fallback path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
All my previous concerns are addressed - the vnext gate, API version fix, soft env var handling, and multi-SP logging all look good.
The flow is clean: fetch agent version with the vnext API version to get InstanceIdentity.PrincipalID, pass it through to RBAC assignment, skip Graph entirely when the principal ID is known. The lazy Graph client init for the fallback path and the token protection error detection are solid additions.
One minor note: this bumps azidentity to v1.14.0-beta.3 - just flagging in case that wasn't intentional.
The agent identity is now available on the agent version object, updating handling to use that instead of relying on graph