From 4902f83e0bf485d0e2f30e5038d7b690a66b056b Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 17:21:52 -0400 Subject: [PATCH 1/6] Implement developer RBAC pre-flight checks for hosted agent deployments and enhance agent identity RBAC management. Fixes #7637 --- .../azure.ai.agents/internal/cmd/listen.go | 61 +++- .../internal/exterrors/codes.go | 13 + .../internal/project/agent_identity_rbac.go | 155 ++++------ .../project/agent_identity_rbac_test.go | 39 +-- .../internal/project/developer_rbac_check.go | 283 ++++++++++++++++++ .../project/developer_rbac_check_test.go | 78 +++++ 6 files changed, 516 insertions(+), 113 deletions(-) create mode 100644 cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go create mode 100644 cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go index b60aa3efc9f..94d6380f890 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go @@ -83,6 +83,7 @@ func preprovisionHandler(ctx context.Context, azdClient *azdext.AzdClient, args } func predeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *azdext.ProjectEventArgs) error { + hasHostedAgent := false for _, svc := range args.Project.Services { switch svc.Host { case AiAgentHost: @@ -92,27 +93,73 @@ func predeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *az if err := envUpdate(ctx, azdClient, args.Project, svc); err != nil { return fmt.Errorf("failed to update environment for service %q: %w", svc.Name, err) } + if isHostedAgentService(svc, args.Project) { + hasHostedAgent = true + } + } + } + + // Run developer RBAC pre-flight checks for hosted agent deployments. + if hasHostedAgent { + if err := project.CheckDeveloperRBAC(ctx, azdClient); err != nil { + return err } } return nil } +// isHostedAgentService checks if a service is a hosted (container) agent by reading +// the agent.yaml kind from the service directory. +func isHostedAgentService(svc *azdext.ServiceConfig, proj *azdext.ProjectConfig) bool { + agentYamlPath := filepath.Join(proj.Path, svc.RelativePath, "agent.yaml") + data, err := os.ReadFile(agentYamlPath) //nolint:gosec // path from azd project config + if err != nil { + return false + } + var generic map[string]any + if err := yaml.Unmarshal(data, &generic); err != nil { + return false + } + kind, ok := generic["kind"].(string) + return ok && kind == string(agent_yaml.AgentKindHosted) +} + func postdeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *azdext.ProjectEventArgs) error { - hasAgent := false + // Collect agent names from hosted agent services that were deployed. + // After deploy, each hosted agent's name is stored as AGENT_{SERVICE_KEY}_NAME. + envResp, err := azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{}) + if err != nil { + return nil // No environment — nothing to check. + } + + var agentNames []string for _, svc := range args.Project.Services { - if svc.Host == AiAgentHost { - hasAgent = true - break + if svc.Host != AiAgentHost { + continue + } + serviceKey := strings.ReplaceAll(svc.Name, " ", "_") + serviceKey = strings.ReplaceAll(serviceKey, "-", "_") + serviceKey = strings.ToUpper(serviceKey) + nameKey := fmt.Sprintf("AGENT_%s_NAME", serviceKey) + + valResp, err := azdClient.Environment().GetValue(ctx, &azdext.GetEnvRequest{ + EnvName: envResp.Environment.Name, + Key: nameKey, + }) + if err != nil || valResp.Value == "" { + continue } + agentNames = append(agentNames, valResp.Value) } - if !hasAgent { + + if len(agentNames) == 0 { return nil } - // Ensure agent identity RBAC is configured when vnext is enabled. + // Ensure per-agent identity RBAC is configured when vnext is enabled. // Runs post-deploy because the platform provisions the identity during agent deployment. - if err := project.EnsureAgentIdentityRBAC(ctx, azdClient); err != nil { + if err := project.EnsureAgentIdentityRBAC(ctx, azdClient, agentNames); err != nil { return fmt.Errorf("agent identity RBAC setup failed: %w", err) } diff --git a/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go b/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go index fe700c880e6..59917130f05 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go +++ b/cli/azd/extensions/azure.ai.agents/internal/exterrors/codes.go @@ -87,6 +87,19 @@ const ( CodeInvalidFilePath = "invalid_file_path" ) +// Error codes for agent identity RBAC operations. +const ( + CodeAgentIdentityNotFound = "agent_identity_not_found" + CodeAgentIdentityRBACFailed = "agent_identity_rbac_failed" +) + +// Error codes for developer RBAC pre-flight checks. +const ( + CodeDeveloperMissingAIUserRole = "developer_missing_ai_user_role" + CodeDeveloperMissingACRRole = "developer_missing_acr_role" + CodeACRResolutionFailed = "acr_resolution_failed" +) + // Error codes commonly used for internal errors. // // These are usually paired with [Internal] for unexpected failures diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go index 458553740d2..9949bb8dde3 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go @@ -9,7 +9,6 @@ import ( "os" "strconv" "strings" - "sync" "time" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -20,10 +19,8 @@ import ( ) const ( - roleAzureAIUser = "53ca6127-db72-4b80-b1b0-d745d6d5456d" - roleCognitiveServicesOpenAIUser = "5e0bd9bd-7b93-4f28-af87-19fc36ad61bd" - roleMonitoringMetricsPublisher = "3913510d-42f4-4e42-8a64-420c390055eb" - agentIdentitySuffix = "AgentIdentity" + roleAzureAIUser = "53ca6127-db72-4b80-b1b0-d745d6d5456d" + agentIdentitySuffix = "AgentIdentity" rbacVerifyMaxAttempts = 12 rbacVerifyPollInterval = 5 * time.Second @@ -35,7 +32,8 @@ const ( type agentIdentityInfo struct { AccountName string ProjectName string - AccountScope string // AI account resource ID (scope for role assignments) + ProjectScope string // Full project resource ID (scope for role assignments) + AccountScope string // AI account resource ID SubscriptionID string ResourceGroup string } @@ -84,6 +82,9 @@ func parseAgentIdentityInfo(projectResourceID string) (*agentIdentityInfo, error "could not extract all required fields from project resource ID: %s", projectResourceID) } + // Project scope is the full resource ID + info.ProjectScope = projectResourceID + // AI account scope is the project resource ID up to (but not including) "/projects/{project}" before, _, ok := strings.Cut(projectResourceID, "/projects/") if !ok { @@ -94,18 +95,25 @@ func parseAgentIdentityInfo(projectResourceID string) (*agentIdentityInfo, error return info, nil } -// agentIdentityDisplayName returns the expected display name for the agent identity SP. -func agentIdentityDisplayName(accountName, projectName string) string { - return fmt.Sprintf("%s-%s-%s", accountName, projectName, agentIdentitySuffix) +// agentIdentityDisplayName returns the expected display name for a per-agent identity SP. +// The platform creates an identity named {account}-{project}-{agentName}-AgentIdentity +// for each deployed hosted agent. +func agentIdentityDisplayName(accountName, projectName, agentName string) string { + return fmt.Sprintf("%s-%s-%s-%s", accountName, projectName, agentName, agentIdentitySuffix) } -// EnsureAgentIdentityRBAC looks up the agent identity service principal in Azure AD +// EnsureAgentIdentityRBAC looks up the per-agent identity service principals in Azure AD // and assigns the required RBAC roles. This is designed to be called from the postdeploy // handler when the vnext experience is enabled. // -// The platform provisions the agent identity automatically when an agent is deployed. -// This function assumes the identity already exists and assigns permissions to it. -func EnsureAgentIdentityRBAC(ctx context.Context, azdClient *azdext.AzdClient) error { +// Each deployed hosted agent gets a platform-created Entra service principal named +// {account}-{project}-{agentName}-AgentIdentity. This function looks up each identity +// and assigns Azure AI User scoped to the Foundry Project. +func EnsureAgentIdentityRBAC(ctx context.Context, azdClient *azdext.AzdClient, agentNames []string) error { + if len(agentNames) == 0 { + return nil + } + azdEnvClient := azdClient.Environment() cEnvResponse, err := azdEnvClient.GetCurrent(ctx, &azdext.EmptyRequest{}) if err != nil { @@ -152,34 +160,57 @@ func EnsureAgentIdentityRBAC(ctx context.Context, azdClient *azdext.AzdClient) e return fmt.Errorf("failed to create Azure credential: %w", err) } - return ensureAgentIdentityRBACWithCred(ctx, cred, azdEnv, info) + return ensureAgentIdentityRBACWithCred(ctx, cred, info, agentNames) } -// ensureAgentIdentityRBACWithCred performs the core agent identity RBAC logic using -// the provided credential and pre-loaded environment values. +// ensureAgentIdentityRBACWithCred performs the core per-agent identity RBAC logic using +// the provided credential. For each agent name, it looks up the per-agent identity +// and assigns Azure AI User scoped to the Foundry Project. func ensureAgentIdentityRBACWithCred( ctx context.Context, cred *azidentity.AzureDeveloperCLICredential, - azdEnv map[string]string, info *agentIdentityInfo, + agentNames []string, ) error { fmt.Println() fmt.Println("Agent Identity RBAC") fmt.Printf(" AI Account: %s\n", info.AccountName) fmt.Printf(" Project: %s\n", info.ProjectName) + fmt.Printf(" Agents: %d\n", len(agentNames)) - // Step 1: Look up agent identity in Azure AD - fmt.Println("[1/2] Looking up agent identity...") - - displayName := agentIdentityDisplayName(info.AccountName, info.ProjectName) graphClient, err := graphsdk.NewGraphClient(cred, nil) if err != nil { return fmt.Errorf("failed to create Graph client: %w", err) } + for i, agentName := range agentNames { + fmt.Println() + fmt.Printf("[%d/%d] Processing agent: %s\n", i+1, len(agentNames), agentName) + + if err := ensureSingleAgentRBAC(ctx, cred, graphClient, info, agentName); err != nil { + return err + } + } + + fmt.Println() + fmt.Println("✓ Agent identity RBAC complete") + return nil +} + +// ensureSingleAgentRBAC handles identity lookup and role assignment for a single agent. +func ensureSingleAgentRBAC( + ctx context.Context, + cred *azidentity.AzureDeveloperCLICredential, + graphClient *graphsdk.GraphClient, + info *agentIdentityInfo, + agentName string, +) error { + displayName := agentIdentityDisplayName(info.AccountName, info.ProjectName, agentName) + // Poll for the identity — the platform provisions it asynchronously during agent deployment, // so it may not be visible in Azure AD immediately after deploy completes. var agentIdentities []graphsdk.ServicePrincipal + var err error for attempt := range identityLookupMaxAttempts { agentIdentities, err = discoverAgentIdentity(ctx, graphClient, displayName) if err != nil { @@ -203,10 +234,7 @@ func ensureAgentIdentityRBACWithCred( } fmt.Println(" ✓ Agent identity found in Azure AD") - // Step 2: Assign RBAC roles in parallel - fmt.Println() - fmt.Println("[2/2] Assigning RBAC to agent identity...") - + // Assign Azure AI User role scoped to the Foundry Project for _, sp := range agentIdentities { principalID := "" if sp.Id != nil { @@ -218,76 +246,25 @@ func ensureAgentIdentityRBACWithCred( fmt.Printf(" Agent identity: %s (%s)\n", sp.DisplayName, principalID) - // Build the list of role assignments to perform - type roleAssignment struct { - roleID string - roleName string - scope string - } - - assignments := []roleAssignment{ - {roleAzureAIUser, "Azure AI User → AI account", info.AccountScope}, - {roleCognitiveServicesOpenAIUser, "Cognitive Services OpenAI User → AI account", info.AccountScope}, - } - - appInsightsRID := azdEnv["APPLICATIONINSIGHTS_RESOURCE_ID"] - if appInsightsRID != "" { - assignments = append(assignments, - roleAssignment{roleMonitoringMetricsPublisher, "Monitoring Metrics Publisher → App Insights", appInsightsRID}) - } else { - fmt.Println(" ⚠ APPLICATIONINSIGHTS_RESOURCE_ID not set — skipping Monitoring Metrics Publisher") - } - - // Run all assignments in parallel - type assignResult struct { - created bool - err error - } - - results := make([]assignResult, len(assignments)) - var wg sync.WaitGroup - - for i, a := range assignments { - wg.Add(1) - go func(i int, a roleAssignment) { - defer wg.Done() - created, err := assignRoleToIdentity(ctx, cred, principalID, a.roleID, a.roleName, a.scope) - results[i] = assignResult{created: created, err: err} - }(i, a) - } - - wg.Wait() - - // Report results in order and check for errors - for i, r := range results { - if r.err != nil { - return fmt.Errorf("failed to assign %s role: %w", assignments[i].roleName, r.err) - } - if r.created { - fmt.Printf(" ✓ %s (created)\n", assignments[i].roleName) - } else { - fmt.Printf(" ✓ %s (already assigned)\n", assignments[i].roleName) - } + created, err := assignRoleToIdentity( + ctx, cred, principalID, roleAzureAIUser, "Azure AI User → Foundry Project", info.ProjectScope, + ) + if err != nil { + return fmt.Errorf("failed to assign Azure AI User role: %w", err) } - // Verify newly created assignments have propagated - for i, r := range results { - if !r.created { - continue - } - a := assignments[i] - fmt.Printf(" ⏳ Verifying %s...\n", a.roleName) - if err := verifyRoleAssignment( - ctx, cred, principalID, a.roleID, a.scope, - ); err != nil { - return fmt.Errorf("failed to verify %s role assignment: %w", a.roleName, err) + if created { + fmt.Println(" ✓ Azure AI User → Foundry Project (created)") + fmt.Println(" ⏳ Verifying Azure AI User...") + if err := verifyRoleAssignment(ctx, cred, principalID, roleAzureAIUser, info.ProjectScope); err != nil { + return fmt.Errorf("failed to verify Azure AI User role assignment: %w", err) } - fmt.Printf(" ✓ %s (verified)\n", a.roleName) + fmt.Println(" ✓ Azure AI User → Foundry Project (verified)") + } else { + fmt.Println(" ✓ Azure AI User → Foundry Project (already assigned)") } } - fmt.Println() - fmt.Println("✓ Agent identity RBAC complete") return nil } diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go index 2500bbaef4f..cbd642198b0 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go @@ -12,14 +12,15 @@ import ( func TestParseAgentIdentityInfo(t *testing.T) { tests := []struct { - name string - resourceID string - wantAccount string - wantProject string - wantSubID string - wantRG string - wantScope string - wantErr bool + name string + resourceID string + wantAccount string + wantProject string + wantSubID string + wantRG string + wantScope string + wantProjScope string + wantErr bool }{ { name: "valid resource ID", @@ -31,6 +32,8 @@ func TestParseAgentIdentityInfo(t *testing.T) { wantRG: "rg-test", wantScope: "/subscriptions/sub-123/resourceGroups/rg-test/providers/" + "Microsoft.CognitiveServices/accounts/my-account", + wantProjScope: "/subscriptions/sub-123/resourceGroups/rg-test/providers/" + + "Microsoft.CognitiveServices/accounts/my-account/projects/my-project", wantErr: false, }, { @@ -43,6 +46,8 @@ func TestParseAgentIdentityInfo(t *testing.T) { wantRG: "my-rg", wantScope: "/subscriptions/aaaa-bbbb/resourceGroups/my-rg/providers/" + "Microsoft.CognitiveServices/accounts/acct-name", + wantProjScope: "/subscriptions/aaaa-bbbb/resourceGroups/my-rg/providers/" + + "Microsoft.CognitiveServices/accounts/acct-name/projects/proj-name/extraSegment/value", wantErr: false, }, { @@ -77,24 +82,26 @@ func TestParseAgentIdentityInfo(t *testing.T) { assert.Equal(t, tt.wantSubID, info.SubscriptionID) assert.Equal(t, tt.wantRG, info.ResourceGroup) assert.Equal(t, tt.wantScope, info.AccountScope) + assert.Equal(t, tt.wantProjScope, info.ProjectScope) }) } } func TestAgentIdentityDisplayName(t *testing.T) { tests := []struct { - account string - project string - want string + account string + project string + agentName string + want string }{ - {"my-account", "my-project", "my-account-my-project-AgentIdentity"}, - {"acct", "proj", "acct-proj-AgentIdentity"}, - {"a-b-c", "x-y-z", "a-b-c-x-y-z-AgentIdentity"}, + {"my-account", "my-project", "my-agent", "my-account-my-project-my-agent-AgentIdentity"}, + {"acct", "proj", "agent1", "acct-proj-agent1-AgentIdentity"}, + {"a-b-c", "x-y-z", "test-agent", "a-b-c-x-y-z-test-agent-AgentIdentity"}, } for _, tt := range tests { t.Run(tt.want, func(t *testing.T) { - got := agentIdentityDisplayName(tt.account, tt.project) + got := agentIdentityDisplayName(tt.account, tt.project, tt.agentName) assert.Equal(t, tt.want, got) }) } @@ -195,6 +202,4 @@ func TestIsVnextEnabled(t *testing.T) { func TestConstants(t *testing.T) { assert.Equal(t, "53ca6127-db72-4b80-b1b0-d745d6d5456d", roleAzureAIUser) - assert.Equal(t, "5e0bd9bd-7b93-4f28-af87-19fc36ad61bd", roleCognitiveServicesOpenAIUser) - assert.Equal(t, "3913510d-42f4-4e42-8a64-420c390055eb", roleMonitoringMetricsPublisher) } diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go new file mode 100644 index 00000000000..df59713dd11 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go @@ -0,0 +1,283 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "context" + "fmt" + "strings" + + "azureaiagent/internal/exterrors" + + "github.com/Azure/azure-sdk-for-go/sdk/azidentity" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v3" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/containerregistry/armcontainerregistry" + "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/graphsdk" +) + +const ( + // Broad roles that imply full access (superset of any specific role). + roleOwner = "8e3af657-a8ff-443c-a75c-2fe8c4bcb635" + roleContributor = "b24988ac-6180-42a0-ab88-20f7382dd24c" + + // ACR-specific roles that grant push/build access. + roleAcrPush = "8311e382-0749-4cb8-b61a-304f252e45ec" + roleContainerRegistryTasksContributor = "fb382eab-e894-4461-af04-94435c366c3f" + roleContainerRegistryRepositoryContributor = "2efddaa5-3f1f-4df3-97df-af3f13818f4c" + + // AI-specific roles that grant agent management access. + roleAzureAIDeveloper = "64702f94-c441-49e6-a78b-ef80e0188fee" +) + +// sufficientACRRoles lists every role that grants enough ACR access to build +// and push container images. Order: broadest first for early exit. +var sufficientACRRoles = []string{ + roleOwner, + roleContributor, + roleAcrPush, + roleContainerRegistryTasksContributor, + roleContainerRegistryRepositoryContributor, +} + +// sufficientAIUserRoles lists every role that grants enough Foundry Project +// access to create and run agents. +var sufficientAIUserRoles = []string{ + roleOwner, + roleContributor, + roleAzureAIUser, + roleAzureAIDeveloper, +} + +// CheckDeveloperRBAC verifies that the currently authenticated developer has the required +// RBAC roles for deploying hosted agents: +// - Azure AI User on the Foundry Project (to create and run agents) +// - Container Registry Tasks Contributor OR Container Registry Repository Contributor +// on the ACR (to build images via remote build and push container images) +// +// Returns nil if all checks pass, or a structured error with suggestions on failure. +func CheckDeveloperRBAC(ctx context.Context, azdClient *azdext.AzdClient) error { + azdEnvClient := azdClient.Environment() + cEnvResponse, err := azdEnvClient.GetCurrent(ctx, &azdext.EmptyRequest{}) + if err != nil { + return fmt.Errorf("failed to get current environment: %w", err) + } + envResponse, err := azdEnvClient.GetValues(ctx, &azdext.GetEnvironmentRequest{ + Name: cEnvResponse.Environment.Name, + }) + if err != nil { + return fmt.Errorf("failed to get environment values: %w", err) + } + + azdEnv := make(map[string]string, len(envResponse.KeyValues)) + for _, kv := range envResponse.KeyValues { + azdEnv[kv.Key] = kv.Value + } + + if !isVnextEnabled(azdEnv) { + return nil + } + + projectResourceID := azdEnv["AZURE_AI_PROJECT_ID"] + if projectResourceID == "" { + // Can't check RBAC without the project ID; deployment will fail later with a clearer message. + return nil + } + + info, err := parseAgentIdentityInfo(projectResourceID) + if err != nil { + return nil // Non-critical: let deployment handle parse errors. + } + + tenantResponse, err := azdClient.Account().LookupTenant(ctx, &azdext.LookupTenantRequest{ + SubscriptionId: info.SubscriptionID, + }) + if err != nil { + return nil // Non-critical: can't resolve tenant for pre-flight check. + } + + cred, err := azidentity.NewAzureDeveloperCLICredential(&azidentity.AzureDeveloperCLICredentialOptions{ + TenantID: tenantResponse.TenantId, + AdditionallyAllowedTenants: []string{"*"}, + }) + if err != nil { + return nil // Non-critical: auth issues will surface during deploy. + } + + fmt.Println() + fmt.Println("Developer RBAC pre-flight check") + + // Get the developer's principal ID via Graph API. + graphClient, err := graphsdk.NewGraphClient(cred, nil) + if err != nil { + fmt.Println(" ⚠ Could not create Graph client — skipping RBAC pre-flight check") + return nil + } + + userProfile, err := graphClient.Me().Get(ctx) + if err != nil { + fmt.Println(" ⚠ Could not retrieve user profile — skipping RBAC pre-flight check") + return nil + } + + principalID := userProfile.Id + fmt.Printf(" Developer: %s (%s)\n", userProfile.DisplayName, principalID) + + // Check 1: Azure AI User (or superset role) on Foundry Project scope. + hasAIAccess, err := hasAnyRoleAssignment(ctx, cred, principalID, sufficientAIUserRoles, info.ProjectScope) + if err != nil { + fmt.Printf(" ⚠ Could not check AI User role: %s\n", err) + } else if !hasAIAccess { + return exterrors.Auth( + exterrors.CodeDeveloperMissingAIUserRole, + fmt.Sprintf( + "your identity (%s) does not have the 'Azure AI User' role on the Foundry Project %s/%s", + userProfile.DisplayName, info.AccountName, info.ProjectName, + ), + fmt.Sprintf( + "ask a subscription Owner or User Access Administrator to assign the 'Azure AI User' role "+ + "to your identity on the Foundry Project scope:\n"+ + " az role assignment create --assignee %s --role \"Azure AI User\" --scope %q", + principalID, info.ProjectScope, + ), + ) + } else { + fmt.Println(" ✓ Azure AI User on Foundry Project") + } + + // Check 2: ACR role — any role that grants push/build access. + acrEndpoint := azdEnv["AZURE_CONTAINER_REGISTRY_ENDPOINT"] + if acrEndpoint == "" { + fmt.Println(" ⚠ AZURE_CONTAINER_REGISTRY_ENDPOINT not set — skipping ACR role check") + return nil + } + + acrResourceID, err := resolveACRResourceID(ctx, cred, info.SubscriptionID, acrEndpoint) + if err != nil { + fmt.Printf(" ⚠ Could not resolve ACR resource ID: %s — skipping ACR role check\n", err) + return nil + } + + hasACRAccess, err := hasAnyRoleAssignment(ctx, cred, principalID, sufficientACRRoles, acrResourceID) + if err != nil { + fmt.Printf(" ⚠ Could not check ACR role: %s\n", err) + return nil + } + + if !hasACRAccess { + acrName := strings.TrimSuffix(acrEndpoint, ".azurecr.io") + return exterrors.Auth( + exterrors.CodeDeveloperMissingACRRole, + fmt.Sprintf( + "your identity (%s) does not have the required role on the Container Registry '%s' "+ + "to build and push container images", + userProfile.DisplayName, acrName, + ), + fmt.Sprintf( + "ask a subscription Owner or User Access Administrator to assign one of these roles "+ + "to your identity on the Container Registry scope:\n"+ + " • Container Registry Tasks Contributor (for remote build)\n"+ + " • Container Registry Repository Contributor (for image push)\n\n"+ + " az role assignment create --assignee %s --role \"Container Registry Tasks Contributor\" --scope %q", + principalID, acrResourceID, + ), + ) + } + + fmt.Println(" ✓ Container Registry role on ACR") + fmt.Println() + return nil +} + +// hasAnyRoleAssignment checks whether the given principal has any of the specified roles +// at the given scope (including inherited assignments from parent scopes). +// This performs a single pass over all role assignments instead of making separate API +// calls per role, which is both faster and avoids redundant network requests. +func hasAnyRoleAssignment( + ctx context.Context, + cred *azidentity.AzureDeveloperCLICredential, + principalID string, + roleIDs []string, + scope string, +) (bool, error) { + subscriptionID := extractSubscriptionID(scope) + if subscriptionID == "" { + return false, fmt.Errorf("could not extract subscription ID from scope: %s", scope) + } + + client, err := armauthorization.NewRoleAssignmentsClient(subscriptionID, cred, nil) + if err != nil { + return false, fmt.Errorf("failed to create role assignments client: %w", err) + } + + // Build a set of suffixes to match against. + suffixes := make(map[string]bool, len(roleIDs)) + for _, id := range roleIDs { + suffixes[fmt.Sprintf("/roleDefinitions/%s", id)] = true + } + + pager := client.NewListForScopePager(scope, nil) + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + return false, fmt.Errorf("failed to list role assignments: %w", err) + } + for _, assignment := range page.Value { + if assignment.Properties == nil || + assignment.Properties.PrincipalID == nil || + assignment.Properties.RoleDefinitionID == nil { + continue + } + if *assignment.Properties.PrincipalID != principalID { + continue + } + roleDefID := *assignment.Properties.RoleDefinitionID + for suffix := range suffixes { + if strings.HasSuffix(roleDefID, suffix) { + return true, nil + } + } + } + } + + return false, nil +} + +// resolveACRResourceID finds the ARM resource ID for an Azure Container Registry +// given its login server endpoint (e.g., "myregistry.azurecr.io"). +func resolveACRResourceID( + ctx context.Context, + cred *azidentity.AzureDeveloperCLICredential, + subscriptionID string, + loginServer string, +) (string, error) { + // Normalize: ensure no protocol prefix and lowercase comparison. + loginServer = strings.TrimPrefix(loginServer, "https://") + loginServer = strings.TrimPrefix(loginServer, "http://") + loginServer = strings.ToLower(strings.TrimSuffix(loginServer, "/")) + + client, err := armcontainerregistry.NewRegistriesClient(subscriptionID, cred, nil) + if err != nil { + return "", fmt.Errorf("failed to create ACR client: %w", err) + } + + pager := client.NewListPager(nil) + for pager.More() { + page, err := pager.NextPage(ctx) + if err != nil { + return "", fmt.Errorf("failed to list container registries: %w", err) + } + for _, registry := range page.Value { + if registry.Properties != nil && + registry.Properties.LoginServer != nil && + strings.EqualFold(*registry.Properties.LoginServer, loginServer) { + if registry.ID != nil { + return *registry.ID, nil + } + } + } + } + + return "", fmt.Errorf("container registry with login server '%s' not found in subscription %s", loginServer, subscriptionID) +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go new file mode 100644 index 00000000000..91e70130bf5 --- /dev/null +++ b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package project + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestResolveACRResourceID_NormalizesLoginServer(t *testing.T) { + // Test that the login server normalization handles various input formats. + // We can't test the full resolution without a live Azure connection, + // but we can verify the normalization logic. + tests := []struct { + name string + input string + want string // expected normalized form + }{ + {"plain login server", "crfoo.azurecr.io", "crfoo.azurecr.io"}, + {"with https prefix", "https://crfoo.azurecr.io", "crfoo.azurecr.io"}, + {"with http prefix", "http://crfoo.azurecr.io", "crfoo.azurecr.io"}, + {"with trailing slash", "crfoo.azurecr.io/", "crfoo.azurecr.io"}, + {"uppercase", "CrFoo.AzureCR.io", "crfoo.azurecr.io"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // We test the normalization by checking that various inputs + // produce consistent lowercase, no-prefix, no-suffix forms. + // The actual resolution requires live Azure and is tested via integration tests. + normalized := normalizeLoginServer(tt.input) + assert.Equal(t, tt.want, normalized) + }) + } +} + +func TestDeveloperRBACRoleConstants(t *testing.T) { + // ACR roles + assert.Equal(t, "fb382eab-e894-4461-af04-94435c366c3f", roleContainerRegistryTasksContributor) + assert.Equal(t, "2efddaa5-3f1f-4df3-97df-af3f13818f4c", roleContainerRegistryRepositoryContributor) + assert.Equal(t, "8311e382-0749-4cb8-b61a-304f252e45ec", roleAcrPush) + + // Superset roles + assert.Equal(t, "8e3af657-a8ff-443c-a75c-2fe8c4bcb635", roleOwner) + assert.Equal(t, "b24988ac-6180-42a0-ab88-20f7382dd24c", roleContributor) + + // AI roles + assert.Equal(t, "64702f94-c441-49e6-a78b-ef80e0188fee", roleAzureAIDeveloper) +} + +func TestSufficientRoleLists(t *testing.T) { + // Verify the sufficient role lists contain expected entries. + assert.Contains(t, sufficientACRRoles, roleOwner) + assert.Contains(t, sufficientACRRoles, roleContributor) + assert.Contains(t, sufficientACRRoles, roleAcrPush) + assert.Contains(t, sufficientACRRoles, roleContainerRegistryTasksContributor) + assert.Contains(t, sufficientACRRoles, roleContainerRegistryRepositoryContributor) + + assert.Contains(t, sufficientAIUserRoles, roleOwner) + assert.Contains(t, sufficientAIUserRoles, roleContributor) + assert.Contains(t, sufficientAIUserRoles, roleAzureAIUser) + assert.Contains(t, sufficientAIUserRoles, roleAzureAIDeveloper) +} + +// normalizeLoginServer is extracted for testability. +func normalizeLoginServer(loginServer string) string { + s := loginServer + for _, prefix := range []string{"https://", "http://"} { + if len(s) > len(prefix) && strings.EqualFold(s[:len(prefix)], prefix) { + s = s[len(prefix):] + } + } + s = strings.TrimSuffix(s, "/") + return strings.ToLower(s) +} From 5e129d743ca6bbaa316f0db7f581bcebe9025102 Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 17:42:07 -0400 Subject: [PATCH 2/6] Add support for skipping RBAC role assignments via environment variable --- .../internal/project/agent_identity_rbac.go | 19 +++++++ .../project/agent_identity_rbac_test.go | 56 +++++++++++++++++++ .../internal/project/developer_rbac_check.go | 5 ++ 3 files changed, 80 insertions(+) diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go index 9949bb8dde3..e974d93cad7 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go @@ -50,6 +50,20 @@ func isVnextEnabled(azdEnv map[string]string) bool { return false } +// isRoleAssignmentsSkipped checks if AZD_AGENT_SKIP_ROLE_ASSIGNMENTS is set to a truthy value +// in the azd environment or OS environment. When true, both developer RBAC pre-flight checks +// and per-agent identity RBAC assignments are skipped. +func isRoleAssignmentsSkipped(azdEnv map[string]string) bool { + value := azdEnv["AZD_AGENT_SKIP_ROLE_ASSIGNMENTS"] + if value == "" { + value = os.Getenv("AZD_AGENT_SKIP_ROLE_ASSIGNMENTS") + } + if skip, err := strconv.ParseBool(value); err == nil && skip { + return true + } + return false +} + // parseAgentIdentityInfo extracts account name, project name, subscription, resource group, // and the AI account scope from the full project resource ID. // @@ -134,6 +148,11 @@ func EnsureAgentIdentityRBAC(ctx context.Context, azdClient *azdext.AzdClient, a return nil } + if isRoleAssignmentsSkipped(azdEnv) { + fmt.Println(" (-) Skipping agent identity RBAC (AZD_AGENT_SKIP_ROLE_ASSIGNMENTS is set)") + return nil + } + projectResourceID := azdEnv["AZURE_AI_PROJECT_ID"] if projectResourceID == "" { return fmt.Errorf("AZURE_AI_PROJECT_ID not set, unable to ensure agent identity RBAC") diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go index cbd642198b0..669ae11e0e8 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac_test.go @@ -203,3 +203,59 @@ func TestIsVnextEnabled(t *testing.T) { func TestConstants(t *testing.T) { assert.Equal(t, "53ca6127-db72-4b80-b1b0-d745d6d5456d", roleAzureAIUser) } + +func TestIsRoleAssignmentsSkipped(t *testing.T) { + tests := []struct { + name string + azdEnv map[string]string + osEnv string + setOsEnv bool + want bool + }{ + { + name: "enabled via azd env true", + azdEnv: map[string]string{"AZD_AGENT_SKIP_ROLE_ASSIGNMENTS": "true"}, + want: true, + }, + { + name: "enabled via azd env 1", + azdEnv: map[string]string{"AZD_AGENT_SKIP_ROLE_ASSIGNMENTS": "1"}, + want: true, + }, + { + name: "disabled via azd env false", + azdEnv: map[string]string{"AZD_AGENT_SKIP_ROLE_ASSIGNMENTS": "false"}, + want: false, + }, + { + name: "not set", + azdEnv: map[string]string{}, + want: false, + }, + { + name: "fallback to os env", + azdEnv: map[string]string{}, + osEnv: "true", + setOsEnv: true, + want: true, + }, + { + name: "invalid value", + azdEnv: map[string]string{"AZD_AGENT_SKIP_ROLE_ASSIGNMENTS": "notabool"}, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.setOsEnv { + t.Setenv("AZD_AGENT_SKIP_ROLE_ASSIGNMENTS", tt.osEnv) + } else { + t.Setenv("AZD_AGENT_SKIP_ROLE_ASSIGNMENTS", "") + } + + got := isRoleAssignmentsSkipped(tt.azdEnv) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go index df59713dd11..83c57512363 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go @@ -79,6 +79,11 @@ func CheckDeveloperRBAC(ctx context.Context, azdClient *azdext.AzdClient) error return nil } + if isRoleAssignmentsSkipped(azdEnv) { + fmt.Println(" (-) Skipping developer RBAC pre-flight check (AZD_AGENT_SKIP_ROLE_ASSIGNMENTS is set)") + return nil + } + projectResourceID := azdEnv["AZURE_AI_PROJECT_ID"] if projectResourceID == "" { // Can't check RBAC without the project ID; deployment will fail later with a clearer message. From 226d19b3810484f6a2029d87ab2a36c325eaacae Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 17:48:29 -0400 Subject: [PATCH 3/6] Enhance login server normalization to handle case-insensitive prefixes in tests --- .../internal/project/developer_rbac_check.go | 9 +++++++-- .../internal/project/developer_rbac_check_test.go | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go index 83c57512363..2cab3a22ec1 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go @@ -258,8 +258,13 @@ func resolveACRResourceID( loginServer string, ) (string, error) { // Normalize: ensure no protocol prefix and lowercase comparison. - loginServer = strings.TrimPrefix(loginServer, "https://") - loginServer = strings.TrimPrefix(loginServer, "http://") + // Use case-insensitive prefix removal since endpoints may arrive as "HTTPS://...". + for _, prefix := range []string{"https://", "http://"} { + if len(loginServer) > len(prefix) && strings.EqualFold(loginServer[:len(prefix)], prefix) { + loginServer = loginServer[len(prefix):] + break + } + } loginServer = strings.ToLower(strings.TrimSuffix(loginServer, "/")) client, err := armcontainerregistry.NewRegistriesClient(subscriptionID, cred, nil) diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go index 91e70130bf5..5b50d270d5d 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go @@ -23,7 +23,9 @@ func TestResolveACRResourceID_NormalizesLoginServer(t *testing.T) { {"with https prefix", "https://crfoo.azurecr.io", "crfoo.azurecr.io"}, {"with http prefix", "http://crfoo.azurecr.io", "crfoo.azurecr.io"}, {"with trailing slash", "crfoo.azurecr.io/", "crfoo.azurecr.io"}, - {"uppercase", "CrFoo.AzureCR.io", "crfoo.azurecr.io"}, + {"uppercase domain", "CrFoo.AzureCR.io", "crfoo.azurecr.io"}, + {"uppercase HTTPS prefix", "HTTPS://crfoo.azurecr.io", "crfoo.azurecr.io"}, + {"mixed case prefix and domain", "Https://CrFoo.AzureCR.io/", "crfoo.azurecr.io"}, } for _, tt := range tests { From 7a734ad6050e6a52ab95394365bc842dac8d8cb0 Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 17:55:05 -0400 Subject: [PATCH 4/6] azure ad > entra id --- .../internal/project/agent_identity_rbac.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go index e974d93cad7..34f9064f3d1 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go @@ -116,7 +116,7 @@ func agentIdentityDisplayName(accountName, projectName, agentName string) string return fmt.Sprintf("%s-%s-%s-%s", accountName, projectName, agentName, agentIdentitySuffix) } -// EnsureAgentIdentityRBAC looks up the per-agent identity service principals in Azure AD +// EnsureAgentIdentityRBAC looks up the per-agent identity service principals in Entra ID // and assigns the required RBAC roles. This is designed to be called from the postdeploy // handler when the vnext experience is enabled. // @@ -227,7 +227,7 @@ func ensureSingleAgentRBAC( displayName := agentIdentityDisplayName(info.AccountName, info.ProjectName, agentName) // Poll for the identity — the platform provisions it asynchronously during agent deployment, - // so it may not be visible in Azure AD immediately after deploy completes. + // so it may not be visible in Entra ID immediately after deploy completes. var agentIdentities []graphsdk.ServicePrincipal var err error for attempt := range identityLookupMaxAttempts { @@ -239,7 +239,7 @@ func ensureSingleAgentRBAC( break } if attempt < identityLookupMaxAttempts-1 { - fmt.Printf(" Identity not yet visible in Azure AD, retrying in %s (%d/%d)...\n", + fmt.Printf(" Identity not ready yet in Entra ID, retrying in %s (%d/%d)...\n", identityLookupPollInterval, attempt+1, identityLookupMaxAttempts) time.Sleep(identityLookupPollInterval) } @@ -247,11 +247,11 @@ func ensureSingleAgentRBAC( if len(agentIdentities) == 0 { return fmt.Errorf( - "agent identity '%s' not found in Azure AD — "+ + "agent identity '%s' not found in Entra ID — "+ "the platform may not have provisioned it yet, wait a few minutes and re-run: azd deploy", displayName) } - fmt.Println(" ✓ Agent identity found in Azure AD") + fmt.Println(" ✓ Agent identity found in Entra ID") // Assign Azure AI User role scoped to the Foundry Project for _, sp := range agentIdentities { @@ -287,7 +287,7 @@ func ensureSingleAgentRBAC( return nil } -// discoverAgentIdentity searches Azure AD for service principals matching the given display name. +// discoverAgentIdentity searches Entra ID for service principals matching the given display name. func discoverAgentIdentity( ctx context.Context, graphClient *graphsdk.GraphClient, From 0b73b2711fb70f9717381c7a3196bebe8518a0d3 Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 18:49:48 -0400 Subject: [PATCH 5/6] Refactor ACR resource ID resolution and normalize login server handling in RBAC checks --- .../azure.ai.agents/internal/cmd/listen.go | 8 +-- .../internal/project/developer_rbac_check.go | 61 +++++++++++-------- .../project/developer_rbac_check_test.go | 27 ++------ 3 files changed, 42 insertions(+), 54 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go index 94d6380f890..79c2e6faf9b 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go @@ -128,6 +128,7 @@ func isHostedAgentService(svc *azdext.ServiceConfig, proj *azdext.ProjectConfig) func postdeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *azdext.ProjectEventArgs) error { // Collect agent names from hosted agent services that were deployed. // After deploy, each hosted agent's name is stored as AGENT_{SERVICE_KEY}_NAME. + // Only hosted agents get platform-created per-agent identities; prompt agents do not. envResp, err := azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{}) if err != nil { return nil // No environment — nothing to check. @@ -135,13 +136,10 @@ func postdeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *a var agentNames []string for _, svc := range args.Project.Services { - if svc.Host != AiAgentHost { + if svc.Host != AiAgentHost || !isHostedAgentService(svc, args.Project) { continue } - serviceKey := strings.ReplaceAll(svc.Name, " ", "_") - serviceKey = strings.ReplaceAll(serviceKey, "-", "_") - serviceKey = strings.ToUpper(serviceKey) - nameKey := fmt.Sprintf("AGENT_%s_NAME", serviceKey) + nameKey := fmt.Sprintf("AGENT_%s_NAME", toServiceKey(svc.Name)) valResp, err := azdClient.Environment().GetValue(ctx, &azdext.GetEnvRequest{ EnvName: envResp.Environment.Name, diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go index 2cab3a22ec1..c156a68a8e8 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check.go @@ -158,10 +158,14 @@ func CheckDeveloperRBAC(ctx context.Context, azdClient *azdext.AzdClient) error return nil } - acrResourceID, err := resolveACRResourceID(ctx, cred, info.SubscriptionID, acrEndpoint) - if err != nil { - fmt.Printf(" ⚠ Could not resolve ACR resource ID: %s — skipping ACR role check\n", err) - return nil + // Prefer the persisted ARM resource ID (set during init); fall back to listing registries. + acrResourceID := azdEnv["AZURE_CONTAINER_REGISTRY_RESOURCE_ID"] + if acrResourceID == "" { + acrResourceID, err = resolveACRResourceID(ctx, cred, info.SubscriptionID, acrEndpoint) + if err != nil { + fmt.Printf(" ⚠ Could not resolve ACR resource ID: %s — skipping ACR role check\n", err) + return nil + } } hasACRAccess, err := hasAnyRoleAssignment(ctx, cred, principalID, sufficientACRRoles, acrResourceID) @@ -171,7 +175,7 @@ func CheckDeveloperRBAC(ctx context.Context, azdClient *azdext.AzdClient) error } if !hasACRAccess { - acrName := strings.TrimSuffix(acrEndpoint, ".azurecr.io") + acrName := strings.TrimSuffix(normalizeLoginServer(acrEndpoint), ".azurecr.io") return exterrors.Auth( exterrors.CodeDeveloperMissingACRRole, fmt.Sprintf( @@ -182,9 +186,11 @@ func CheckDeveloperRBAC(ctx context.Context, azdClient *azdext.AzdClient) error fmt.Sprintf( "ask a subscription Owner or User Access Administrator to assign one of these roles "+ "to your identity on the Container Registry scope:\n"+ - " • Container Registry Tasks Contributor (for remote build)\n"+ - " • Container Registry Repository Contributor (for image push)\n\n"+ - " az role assignment create --assignee %s --role \"Container Registry Tasks Contributor\" --scope %q", + " • Owner or Contributor (broad access)\n"+ + " • AcrPush (push and pull images)\n"+ + " • Container Registry Tasks Contributor (remote build)\n"+ + " • Container Registry Repository Contributor (repository operations)\n\n"+ + " az role assignment create --assignee %s --role \"AcrPush\" --scope %q", principalID, acrResourceID, ), ) @@ -197,8 +203,7 @@ func CheckDeveloperRBAC(ctx context.Context, azdClient *azdext.AzdClient) error // hasAnyRoleAssignment checks whether the given principal has any of the specified roles // at the given scope (including inherited assignments from parent scopes). -// This performs a single pass over all role assignments instead of making separate API -// calls per role, which is both faster and avoids redundant network requests. +// Uses server-side filtering by principal to reduce API load on large subscriptions. func hasAnyRoleAssignment( ctx context.Context, cred *azidentity.AzureDeveloperCLICredential, @@ -222,19 +227,18 @@ func hasAnyRoleAssignment( suffixes[fmt.Sprintf("/roleDefinitions/%s", id)] = true } - pager := client.NewListForScopePager(scope, nil) + // Use server-side assignedTo filter to only return assignments for this principal. + filter := fmt.Sprintf("assignedTo('%s')", principalID) + pager := client.NewListForScopePager(scope, &armauthorization.RoleAssignmentsClientListForScopeOptions{ + Filter: &filter, + }) for pager.More() { page, err := pager.NextPage(ctx) if err != nil { return false, fmt.Errorf("failed to list role assignments: %w", err) } for _, assignment := range page.Value { - if assignment.Properties == nil || - assignment.Properties.PrincipalID == nil || - assignment.Properties.RoleDefinitionID == nil { - continue - } - if *assignment.Properties.PrincipalID != principalID { + if assignment.Properties == nil || assignment.Properties.RoleDefinitionID == nil { continue } roleDefID := *assignment.Properties.RoleDefinitionID @@ -249,6 +253,19 @@ func hasAnyRoleAssignment( return false, nil } +// normalizeLoginServer strips protocol prefixes, trailing slashes, and lowercases +// an ACR login server endpoint for consistent comparison. +func normalizeLoginServer(loginServer string) string { + s := loginServer + for _, prefix := range []string{"https://", "http://"} { + if len(s) > len(prefix) && strings.EqualFold(s[:len(prefix)], prefix) { + s = s[len(prefix):] + break + } + } + return strings.ToLower(strings.TrimSuffix(s, "/")) +} + // resolveACRResourceID finds the ARM resource ID for an Azure Container Registry // given its login server endpoint (e.g., "myregistry.azurecr.io"). func resolveACRResourceID( @@ -257,15 +274,7 @@ func resolveACRResourceID( subscriptionID string, loginServer string, ) (string, error) { - // Normalize: ensure no protocol prefix and lowercase comparison. - // Use case-insensitive prefix removal since endpoints may arrive as "HTTPS://...". - for _, prefix := range []string{"https://", "http://"} { - if len(loginServer) > len(prefix) && strings.EqualFold(loginServer[:len(prefix)], prefix) { - loginServer = loginServer[len(prefix):] - break - } - } - loginServer = strings.ToLower(strings.TrimSuffix(loginServer, "/")) + loginServer = normalizeLoginServer(loginServer) client, err := armcontainerregistry.NewRegistriesClient(subscriptionID, cred, nil) if err != nil { diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go index 5b50d270d5d..233f314e279 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/developer_rbac_check_test.go @@ -4,20 +4,16 @@ package project import ( - "strings" "testing" "github.com/stretchr/testify/assert" ) -func TestResolveACRResourceID_NormalizesLoginServer(t *testing.T) { - // Test that the login server normalization handles various input formats. - // We can't test the full resolution without a live Azure connection, - // but we can verify the normalization logic. +func TestNormalizeLoginServer(t *testing.T) { tests := []struct { name string input string - want string // expected normalized form + want string }{ {"plain login server", "crfoo.azurecr.io", "crfoo.azurecr.io"}, {"with https prefix", "https://crfoo.azurecr.io", "crfoo.azurecr.io"}, @@ -30,11 +26,8 @@ func TestResolveACRResourceID_NormalizesLoginServer(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // We test the normalization by checking that various inputs - // produce consistent lowercase, no-prefix, no-suffix forms. - // The actual resolution requires live Azure and is tested via integration tests. - normalized := normalizeLoginServer(tt.input) - assert.Equal(t, tt.want, normalized) + got := normalizeLoginServer(tt.input) + assert.Equal(t, tt.want, got) }) } } @@ -66,15 +59,3 @@ func TestSufficientRoleLists(t *testing.T) { assert.Contains(t, sufficientAIUserRoles, roleAzureAIUser) assert.Contains(t, sufficientAIUserRoles, roleAzureAIDeveloper) } - -// normalizeLoginServer is extracted for testability. -func normalizeLoginServer(loginServer string) string { - s := loginServer - for _, prefix := range []string{"https://", "http://"} { - if len(s) > len(prefix) && strings.EqualFold(s[:len(prefix)], prefix) { - s = s[len(prefix):] - } - } - s = strings.TrimSuffix(s, "/") - return strings.ToLower(s) -} From 7b56d742ecb746b06e2d8668606d2e352b1b3009 Mon Sep 17 00:00:00 2001 From: John Miller Date: Fri, 10 Apr 2026 18:57:11 -0400 Subject: [PATCH 6/6] Improve error handling in agent identity RBAC and process agents in parallel --- .../azure.ai.agents/internal/cmd/listen.go | 7 +++-- .../internal/project/agent_identity_rbac.go | 31 ++++++++++++++++--- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go index 79c2e6faf9b..fa747f884c6 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/listen.go @@ -131,7 +131,7 @@ func postdeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *a // Only hosted agents get platform-created per-agent identities; prompt agents do not. envResp, err := azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{}) if err != nil { - return nil // No environment — nothing to check. + return fmt.Errorf("failed to get current environment for agent identity RBAC: %w", err) } var agentNames []string @@ -145,7 +145,10 @@ func postdeployHandler(ctx context.Context, azdClient *azdext.AzdClient, args *a EnvName: envResp.Environment.Name, Key: nameKey, }) - if err != nil || valResp.Value == "" { + if err != nil { + return fmt.Errorf("failed to read %s from environment: %w", nameKey, err) + } + if valResp.Value == "" { continue } agentNames = append(agentNames, valResp.Value) diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go index 34f9064f3d1..2554780a92c 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_identity_rbac.go @@ -9,6 +9,7 @@ import ( "os" "strconv" "strings" + "sync" "time" "github.com/Azure/azure-sdk-for-go/sdk/azidentity" @@ -185,6 +186,7 @@ func EnsureAgentIdentityRBAC(ctx context.Context, azdClient *azdext.AzdClient, a // ensureAgentIdentityRBACWithCred performs the core per-agent identity RBAC logic using // the provided credential. For each agent name, it looks up the per-agent identity // and assigns Azure AI User scoped to the Foundry Project. +// Agents are processed in parallel since each has an independent identity. func ensureAgentIdentityRBACWithCred( ctx context.Context, cred *azidentity.AzureDeveloperCLICredential, @@ -202,12 +204,33 @@ func ensureAgentIdentityRBACWithCred( return fmt.Errorf("failed to create Graph client: %w", err) } + // Process agents in parallel — each has an independent identity and the + // identity lookup polling (up to ~3 min per agent) dominates wall-clock time. + type agentResult struct { + name string + err error + } + + results := make([]agentResult, len(agentNames)) + var wg sync.WaitGroup + for i, agentName := range agentNames { - fmt.Println() - fmt.Printf("[%d/%d] Processing agent: %s\n", i+1, len(agentNames), agentName) + wg.Add(1) + go func(i int, name string) { + defer wg.Done() + results[i] = agentResult{ + name: name, + err: ensureSingleAgentRBAC(ctx, cred, graphClient, info, name), + } + }(i, agentName) + } + + wg.Wait() - if err := ensureSingleAgentRBAC(ctx, cred, graphClient, info, agentName); err != nil { - return err + // Report results in order and return first error. + for _, r := range results { + if r.err != nil { + return fmt.Errorf("agent identity RBAC failed for %q: %w", r.name, r.err) } }