diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go index 688ecb14c0f..86507fe8ee8 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers.go @@ -10,7 +10,6 @@ import ( "fmt" "io" "log" - "maps" "net/http" "net/url" "os" @@ -24,7 +23,6 @@ import ( "azureaiagent/internal/pkg/agents/agent_api" "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/paths" - "azureaiagent/internal/pkg/projectconfig" projectpkg "azureaiagent/internal/project" "github.com/azure/azure-dev/cli/azd/pkg/azdext" @@ -941,10 +939,11 @@ func resolveAgentServiceFromProject( // ServiceRunContext holds the resolved context needed for local development. type ServiceRunContext struct { - ServiceName string // the resolved service name (from azure.yaml) - ProjectDir string // absolute path to the service source directory - StartupCommand string // startupCommand from AdditionalProperties (may be empty) - Environment map[string]string + ServiceName string // the resolved service name (from azure.yaml) + ProjectDir string // absolute path to the service source directory + StartupCommand string // startupCommand from AdditionalProperties (may be empty) + ServiceEnvironment map[string]string // values already expanded by azd core + HasServiceEnvironment bool // service declares env: even when empty // Definition is the resolved agent definition (from the inline azure.yaml // entry or a legacy agent.yaml). It is nil when no definition can be resolved. Definition *agent_yaml.ContainerAgent @@ -982,28 +981,10 @@ func resolveServiceRunContext(ctx context.Context, azdClient *azdext.AzdClient, } var startupCmd string - serviceEnv := map[string]string{} if agentConfig, cfgErr := projectpkg.LoadServiceTargetAgentConfig( svc, ); cfgErr == nil { startupCmd = agentConfig.StartupCommand - maps.Copy(serviceEnv, agentConfig.Environment) - } - serviceEnv, err = loadServiceRunEnvironment( - project.Path, - svc, - serviceEnv, - ) - if err != nil { - return nil, exterrors.Validation( - exterrors.CodeInvalidServiceConfig, - fmt.Sprintf( - "failed to load environment for %s: %s", - svc.Name, - err, - ), - "fix the service env configuration in azure.yaml", - ) } var definition *agent_yaml.ContainerAgent @@ -1014,39 +995,24 @@ func resolveServiceRunContext(ctx context.Context, azdClient *azdext.AzdClient, } } + hasServiceEnvironment := false + if resp, envErr := azdClient.Project().GetServiceConfigValue( + ctx, + &azdext.GetServiceConfigValueRequest{ServiceName: svc.Name, Path: "env"}, + ); envErr == nil { + hasServiceEnvironment = resp.GetFound() + } + return &ServiceRunContext{ - ServiceName: svc.Name, - ProjectDir: projectDir, - StartupCommand: startupCmd, - Environment: serviceEnv, - Definition: definition, + ServiceName: svc.Name, + ProjectDir: projectDir, + StartupCommand: startupCmd, + ServiceEnvironment: svc.GetEnvironment(), + HasServiceEnvironment: hasServiceEnvironment, + Definition: definition, }, nil } -func loadServiceRunEnvironment( - projectRoot string, - svc *azdext.ServiceConfig, - base map[string]string, -) (map[string]string, error) { - env := maps.Clone(base) - if env == nil { - env = map[string]string{} - } - raw, err := projectconfig.LoadServiceEnvironment( - projectRoot, - svc.GetName(), - ) - if err != nil { - return nil, err - } - if raw == nil { - maps.Copy(env, svc.GetEnvironment()) - } else { - maps.Copy(env, raw) - } - return env, nil -} - // toServiceKey converts a service name into the env var key format (uppercase, underscores). func toServiceKey(serviceName string) string { key := strings.ReplaceAll(serviceName, " ", "_") diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go index 56ce534b7d0..f1a3c5a27b4 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/helpers_test.go @@ -956,41 +956,3 @@ func TestResolveAgentProtocol_MultipleServicesPromptsOnce(t *testing.T) { require.Equal(t, int32(1), promptServer.selectCalls.Load(), "resolveAgentProtocol should trigger exactly one prompt") } - -func TestLoadServiceRunEnvironmentUsesRawValues(t *testing.T) { - t.Parallel() - - root := t.TempDir() - require.NoError(t, os.WriteFile( - filepath.Join(root, "azure.yaml"), - []byte(`services: - agent: - host: azure.ai.agent - env: - PROJECT: ${{project.endpoint}} - ENABLED: true - SHARED: direct -`), - 0o600, - )) - svc := &azdext.ServiceConfig{ - Name: "agent", - Environment: map[string]string{ - "PROJECT": "", - "ENABLED": "", - "SHARED": "expanded", - }, - } - - env, err := loadServiceRunEnvironment( - root, - svc, - map[string]string{"CONFIG_ONLY": "config", "SHARED": "config"}, - ) - - require.NoError(t, err) - require.Equal(t, "${{project.endpoint}}", env["PROJECT"]) - require.Equal(t, "true", env["ENABLED"]) - require.Equal(t, "direct", env["SHARED"]) - require.Equal(t, "config", env["CONFIG_ONLY"]) -} diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go index 0b48d8a78fb..0e102ca357f 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init.go @@ -2935,6 +2935,7 @@ func (a *InitAction) addToProject(ctx context.Context, targetDir string, agentMa if err != nil { return err } + agentEnvironment := project.AgentEnvironment(containerDef) serviceConfig := &azdext.ServiceConfig{ Name: a.serviceNameOverride, @@ -2968,6 +2969,14 @@ func (a *InitAction) addToProject(ctx context.Context, targetDir string, agentMa if _, err := a.azdClient.Project().AddService(ctx, req); err != nil { return fmt.Errorf("adding agent service to project: %w", err) } + if err := setServiceEnvironment( + ctx, + a.azdClient, + a.serviceNameOverride, + agentEnvironment, + ); err != nil { + return err + } // Emit the sibling Foundry resource services (project + deployments, // connections, toolboxes) and wire the agent's uses: to them. A selected diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go index 6f6ceef87c8..aaf579f5c2f 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_from_code.go @@ -831,6 +831,7 @@ func (a *InitFromCodeAction) addToProject( if err != nil { return err } + agentEnvironment := project.AgentEnvironment(*definition) language := "python" if !isCodeDeploy { @@ -840,8 +841,9 @@ func (a *InitFromCodeAction) addToProject( language = "csharp" } + agentServiceName := strings.ReplaceAll(agentName, " ", "") serviceConfig := &azdext.ServiceConfig{ - Name: strings.ReplaceAll(agentName, " ", ""), + Name: agentServiceName, RelativePath: targetDir, Host: AiAgentHost, Language: language, @@ -868,11 +870,18 @@ func (a *InitFromCodeAction) addToProject( if _, err := a.azdClient.Project().AddService(ctx, req); err != nil { return fmt.Errorf("adding agent service to project: %w", err) } + if err := setServiceEnvironment( + ctx, + a.azdClient, + agentServiceName, + agentEnvironment, + ); err != nil { + return err + } // Emit the sibling azure.ai.project service carrying the model deployments // and wire the agent's uses: to it. A selected existing project contributes // its endpoint so provision reuses it instead of creating a new project. - agentServiceName := strings.ReplaceAll(agentName, " ", "") if err := emitResourceServices( ctx, a.azdClient, agentServiceName, projectNameHint(ctx, a.azdClient, a.environment.Name, a.selectedFoundryProject), diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go index 7d5bf3a5bdf..21814b8b140 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/init_test.go @@ -423,6 +423,9 @@ func TestAddToProjectPreBuiltImageWritesServiceImage(t *testing.T) { Protocols: []agent_yaml.ProtocolVersionRecord{ {Protocol: "responses", Version: "2.0.0"}, }, + EnvironmentVariables: &[]agent_yaml.EnvironmentVariable{ + {Name: "LOG_LEVEL", Value: "info"}, + }, }, } @@ -446,9 +449,16 @@ func TestAddToProjectPreBuiltImageWritesServiceImage(t *testing.T) { require.Equal(t, "docker", agentService.GetLanguage()) require.NotNil(t, agentService.GetDocker()) require.NotNil(t, agentService.GetAdditionalProperties()) + require.Empty(t, agentService.GetEnvironment()) + require.Equal(t, map[string]any{ + "LOG_LEVEL": "info", + }, server.env["my-agent"]) _, hasInlineImage := agentService.GetAdditionalProperties().GetFields()["image"] require.False(t, hasInlineImage, "pre-built image must ride on the top-level service image field") + _, hasInlineEnvironment := agentService.GetAdditionalProperties(). + GetFields()["environmentVariables"] + require.False(t, hasInlineEnvironment) } func TestValidateInitAgentName(t *testing.T) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go index 92b34000836..cdd2ee2ccab 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply.go @@ -17,6 +17,7 @@ import ( "encoding/json" "fmt" "io" + "maps" "os" "path/filepath" "strings" @@ -24,6 +25,7 @@ import ( "azureaiagent/internal/pkg/agents/opt_eval" "azureaiagent/internal/pkg/agents/optimize_api" "azureaiagent/internal/pkg/paths" + "azureaiagent/internal/pkg/projectconfig" projectpkg "azureaiagent/internal/project" "github.com/azure/azure-dev/cli/azd/pkg/azdext" @@ -196,33 +198,13 @@ func (a *OptimizeApplyAction) apply( return fmt.Errorf("failed to read agent definition: %w", err) } else if found { fmt.Fprintf(out, " Updating agent definition in azure.yaml...\n") - if err := projectpkg.UpsertAgentEnvVars(svc, envUpdates); err != nil { - return fmt.Errorf("failed to update agent definition: %w", err) - } - // Read the current `uses:` value before replacing the whole service entry. - // AddService writes back the proto ServiceConfig shape, which doesn't carry - // the `uses:` field (a core azd-only field). Reading it first and restoring - // it after avoids silently dropping dependency edges that were written by - // `setServiceUses` via SetServiceConfigValue. - prevUses, err := azdClient.Project().GetServiceConfigValue(ctx, &azdext.GetServiceConfigValueRequest{ - ServiceName: svc.Name, - Path: "uses", - }) - if err != nil { - return fmt.Errorf("failed to read uses for service %q: %w", svc.Name, err) - } - if _, err := azdClient.Project().AddService(ctx, &azdext.AddServiceRequest{Service: svc}); err != nil { - return fmt.Errorf("failed to persist agent definition: %w", err) - } - // Restore `uses:` if it was set before the replacement. - if prevUses.GetFound() && prevUses.GetValue() != nil { - if _, err := azdClient.Project().SetServiceConfigValue(ctx, &azdext.SetServiceConfigValueRequest{ - ServiceName: svc.Name, - Path: "uses", - Value: prevUses.GetValue(), - }); err != nil { - return fmt.Errorf("failed to restore uses for service %q: %w", svc.Name, err) - } + if err := persistInlineAgentEnvironment( + ctx, + azdClient, + svc, + envUpdates, + ); err != nil { + return err } } else { agentYamlPath := filepath.Join(serviceDir, "agent.yaml") @@ -272,6 +254,129 @@ func (a *OptimizeApplyAction) apply( return nil } +func persistInlineAgentEnvironment( + ctx context.Context, + azdClient *azdext.AzdClient, + svc *azdext.ServiceConfig, + envUpdates map[string]string, +) error { + _, _, found, source, err := projectpkg.AgentDefinitionFromService(svc) + if err != nil { + return fmt.Errorf("failed to read agent definition: %w", err) + } + if !found { + return fmt.Errorf( + "service %q does not carry an inline agent definition", + svc.GetName(), + ) + } + + // Build the env to persist from raw templates only, never from the + // core-expanded svc.Environment. AddService escapes env values to + // literals, so routing templates through it would freeze a ${VAR} + // template (or snapshot an already-expanded value) into azure.yaml. + legacyEnv, err := projectpkg.InlineAgentEnvironmentVariables(svc) + if err != nil { + return fmt.Errorf("failed to read agent environment: %w", err) + } + existingEnv, err := getRawServiceEnv(ctx, azdClient, svc) + if err != nil { + return err + } + // Merge order mirrors deploy-time precedence in toContainerAgent: + // the top-level env overlays legacy environmentVariables, then the + // new updates win. + mergedEnv := map[string]string{} + maps.Copy(mergedEnv, legacyEnv) + maps.Copy(mergedEnv, existingEnv) + maps.Copy(mergedEnv, envUpdates) + + if err := setServiceEnvironment(ctx, azdClient, svc.Name, mergedEnv); err != nil { + return err + } + + environmentPath := "environmentVariables" + if source == projectpkg.AgentDefinitionSourceLegacyConfig { + environmentPath = "config.environmentVariables" + } + if _, err := azdClient.Project().UnsetServiceConfig( + ctx, + &azdext.UnsetServiceConfigRequest{ + ServiceName: svc.Name, + Path: environmentPath, + }, + ); err != nil { + return fmt.Errorf( + "removing deprecated environmentVariables from service %q: %w", + svc.Name, + err, + ) + } + return nil +} + +// getRawServiceEnv reads the service's existing env section as raw, +// unexpanded templates. GetServiceConfigValue returns the on-disk +// config, so callers can rewrite env without losing ${VAR}/$${{...}} +// templates. +func getRawServiceEnv( + ctx context.Context, + azdClient *azdext.AzdClient, + svc *azdext.ServiceConfig, +) (map[string]string, error) { + serviceName := svc.GetName() + resp, err := azdClient.Project().GetServiceConfigValue( + ctx, + &azdext.GetServiceConfigValueRequest{ + ServiceName: serviceName, + Path: "env", + }, + ) + if err != nil { + return nil, fmt.Errorf( + "failed to read env for service %q: %w", serviceName, err) + } + if !resp.GetFound() || resp.GetValue() == nil { + return nil, nil + } + raw, ok := resp.GetValue().AsInterface().(map[string]any) + if !ok { + return nil, nil + } + originalRaw := maps.Clone(raw) + properties := map[string]any{"env": raw} + if err := projectconfig.NormalizeEnvironment(properties); err != nil { + return nil, fmt.Errorf( + "normalizing env for service %q: %w", + serviceName, + err, + ) + } + env := make(map[string]string, len(raw)) + for key, value := range raw { + if original, wasString := originalRaw[key].(string); wasString { + env[key] = original + continue + } + if forwarded, found := svc.GetEnvironment()[key]; found { + env[key] = forwarded + continue + } + str, ok := value.(string) + if ok { + env[key] = str + continue + } + return nil, fmt.Errorf( + "normalizing env %q for service %q produced %T", + key, + serviceName, + value, + ) + } + return env, nil +} + // agentConfigMetadata is the YAML structure written as metadata.yaml in each // agent config version directory (baseline or candidate). // diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply_test.go index f84f87e372e..c03ab66de2d 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/optimize_apply_test.go @@ -11,13 +11,16 @@ import ( "path/filepath" "testing" + "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/agents/opt_eval" "azureaiagent/internal/pkg/agents/optimize_api" + projectpkg "azureaiagent/internal/project" "github.com/azure/azure-dev/cli/azd/pkg/azdext" "github.com/fatih/color" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" ) // ---- newOptimizeApplyCommand — command shape ---- @@ -47,6 +50,270 @@ func TestNewOptimizeApplyCommand_CandidateIsRequired(t *testing.T) { assert.Contains(t, err.Error(), "candidate") } +func TestPersistInlineAgentEnvironmentMigratesLegacyTemplates(t *testing.T) { + props, err := projectpkg.AgentDefinitionToServiceProperties( + agent_yaml.ContainerAgent{ + AgentDefinition: agent_yaml.AgentDefinition{ + Kind: agent_yaml.AgentKindHosted, + Name: "basic-agent", + }, + Protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: "responses", Version: "2.0.0"}, + }, + }, + nil, + ) + require.NoError(t, err) + legacyEnvironment, err := structpb.NewValue([]any{ + map[string]any{ + "name": "LEGACY_KEY", + "value": "${LEGACY_KEY}", + }, + }) + require.NoError(t, err) + props.Fields["environmentVariables"] = legacyEnvironment + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: AiAgentHost, + Config: props, + } + + server := &recordingProjectServer{} + client := newProjectRecorderClient(t, server) + require.NoError(t, persistInlineAgentEnvironment( + t.Context(), + client, + svc, + map[string]string{"OPTIMIZATION_CANDIDATE_ID": "candidate-1"}, + )) + + server.mu.Lock() + defer server.mu.Unlock() + require.Empty(t, server.added) + require.Equal( + t, + []string{"config.environmentVariables"}, + server.unsetPaths, + ) + require.Equal(t, map[string]any{ + "LEGACY_KEY": "${LEGACY_KEY}", + "OPTIMIZATION_CANDIDATE_ID": "candidate-1", + }, server.env["basic-agent"]) +} + +// TestPersistInlineAgentEnvironmentPreservesTopLevelEnv verifies a +// modern agent's top-level env templates survive the OPTIMIZATION_* +// update: they are read raw and rewritten via the env section, not +// snapshotted to expanded literals through AddService. +func TestPersistInlineAgentEnvironmentPreservesTopLevelEnv(t *testing.T) { + props, err := projectpkg.AgentDefinitionToServiceProperties( + agent_yaml.ContainerAgent{ + AgentDefinition: agent_yaml.AgentDefinition{ + Kind: agent_yaml.AgentKindHosted, + Name: "basic-agent", + }, + Protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: "responses", Version: "2.0.0"}, + }, + }, + nil, + ) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: AiAgentHost, + AdditionalProperties: props, + // Core forwards expanded values; the raw templates live on disk. + Environment: map[string]string{ + "LOG_LEVEL": "debug", + "MODEL_ENDPOINT": "https://resolved.example", + }, + } + + server := &recordingProjectServer{ + rawEnv: map[string]map[string]any{ + "basic-agent": { + "LOG_LEVEL": "${AZURE_LOG_LEVEL}", + "MODEL_ENDPOINT": "$${{project.endpoint}}", + }, + }, + } + client := newProjectRecorderClient(t, server) + require.NoError(t, persistInlineAgentEnvironment( + t.Context(), + client, + svc, + map[string]string{"OPTIMIZATION_CANDIDATE_ID": "candidate-1"}, + )) + + server.mu.Lock() + defer server.mu.Unlock() + require.Empty(t, server.added) + require.Equal( + t, + []string{"environmentVariables"}, + server.unsetPaths, + ) + require.Equal(t, map[string]any{ + "LOG_LEVEL": "${AZURE_LOG_LEVEL}", + "MODEL_ENDPOINT": "$${{project.endpoint}}", + "OPTIMIZATION_CANDIDATE_ID": "candidate-1", + }, server.env["basic-agent"]) +} + +// TestPersistInlineAgentEnvironmentEscapesLegacyFoundrySpan verifies +// a legacy environmentVariables value carrying a raw Foundry ${{...}} +// span is escaped to $${{...}} when migrated into the env section. +func TestPersistInlineAgentEnvironmentEscapesLegacyFoundrySpan(t *testing.T) { + props, err := projectpkg.AgentDefinitionToServiceProperties( + agent_yaml.ContainerAgent{ + AgentDefinition: agent_yaml.AgentDefinition{ + Kind: agent_yaml.AgentKindHosted, + Name: "basic-agent", + }, + Protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: "responses", Version: "2.0.0"}, + }, + }, + nil, + ) + require.NoError(t, err) + legacyEnvironment, err := structpb.NewValue([]any{ + map[string]any{ + "name": "SEARCH_KEY", + "value": "${{connections.search.credentials.key}}", + }, + }) + require.NoError(t, err) + props.Fields["environmentVariables"] = legacyEnvironment + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: AiAgentHost, + AdditionalProperties: props, + } + + server := &recordingProjectServer{} + client := newProjectRecorderClient(t, server) + require.NoError(t, persistInlineAgentEnvironment( + t.Context(), + client, + svc, + map[string]string{"OPTIMIZATION_CANDIDATE_ID": "candidate-1"}, + )) + + server.mu.Lock() + defer server.mu.Unlock() + require.Equal( + t, + []string{"environmentVariables"}, + server.unsetPaths, + ) + require.Equal(t, map[string]any{ + "SEARCH_KEY": "$${{connections.search.credentials.key}}", + "OPTIMIZATION_CANDIDATE_ID": "candidate-1", + }, server.env["basic-agent"]) +} + +func TestPersistInlineAgentEnvironmentNormalizesScalars(t *testing.T) { + props, err := projectpkg.AgentDefinitionToServiceProperties( + agent_yaml.ContainerAgent{ + AgentDefinition: agent_yaml.AgentDefinition{ + Kind: agent_yaml.AgentKindHosted, + Name: "basic-agent", + }, + Protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: "responses", Version: "2.0.0"}, + }, + }, + nil, + ) + require.NoError(t, err) + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: AiAgentHost, + AdditionalProperties: props, + Environment: map[string]string{ + "LARGE": "9007199254740993", + }, + } + server := &recordingProjectServer{ + rawEnv: map[string]map[string]any{ + "basic-agent": { + "ENABLED": true, + "RETRIES": float64(3), + "EMPTY": nil, + "LARGE": float64(9007199254740992), + }, + }, + } + + client := newProjectRecorderClient(t, server) + require.NoError(t, persistInlineAgentEnvironment( + t.Context(), + client, + svc, + map[string]string{"OPTIMIZATION_CANDIDATE_ID": "candidate-1"}, + )) + + server.mu.Lock() + defer server.mu.Unlock() + require.Equal(t, map[string]any{ + "ENABLED": "true", + "RETRIES": "3", + "EMPTY": "", + "LARGE": "9007199254740993", + "OPTIMIZATION_CANDIDATE_ID": "candidate-1", + }, server.env["basic-agent"]) +} + +func TestPersistInlineAgentEnvironmentKeepsLegacyOnEnvFailure( + t *testing.T, +) { + props, err := projectpkg.AgentDefinitionToServiceProperties( + agent_yaml.ContainerAgent{ + AgentDefinition: agent_yaml.AgentDefinition{ + Kind: agent_yaml.AgentKindHosted, + Name: "basic-agent", + }, + Protocols: []agent_yaml.ProtocolVersionRecord{ + {Protocol: "responses", Version: "2.0.0"}, + }, + }, + nil, + ) + require.NoError(t, err) + legacyEnvironment, err := structpb.NewValue([]any{ + map[string]any{ + "name": "LEGACY_KEY", + "value": "${LEGACY_KEY}", + }, + }) + require.NoError(t, err) + props.Fields["environmentVariables"] = legacyEnvironment + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: AiAgentHost, + AdditionalProperties: props, + } + server := &recordingProjectServer{ + setEnvironmentErr: fmt.Errorf("write failed"), + } + + client := newProjectRecorderClient(t, server) + err = persistInlineAgentEnvironment( + t.Context(), + client, + svc, + map[string]string{"OPTIMIZATION_CANDIDATE_ID": "candidate-1"}, + ) + + require.ErrorContains(t, err, "write failed") + server.mu.Lock() + defer server.mu.Unlock() + require.Empty(t, server.unsetPaths) + require.Empty(t, server.added) +} + // ---- printPreviewLines ---- func TestPrintPreviewLines(t *testing.T) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go index 0931fbd76fb..2aa7017fa1a 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services.go @@ -277,6 +277,7 @@ func addResourceService( cfg *structpb.Struct, uses []string, ) error { + environment := serviceEnvironmentTemplates(cfg) svc := &azdext.ServiceConfig{ Name: name, Host: host, @@ -287,6 +288,15 @@ func addResourceService( return fmt.Errorf("adding %s service %q: %w", host, name, err) } + if err := setServiceEnvironment( + ctx, + azdClient, + name, + environment, + ); err != nil { + return err + } + if len(uses) > 0 { if err := setServiceUses(ctx, azdClient, name, uses); err != nil { return err @@ -296,6 +306,202 @@ func addResourceService( return nil } +// serviceEnvironmentTemplates discovers client-side templates in the +// generic nested resource config emitted to azure.yaml. +func serviceEnvironmentTemplates(cfg *structpb.Struct) map[string]string { + if cfg == nil { + return nil + } + + environment := map[string]string{} + collectEnvironmentTemplates(cfg.AsMap(), environment) + if len(environment) == 0 { + return nil + } + return environment +} + +func collectEnvironmentTemplates(value any, environment map[string]string) { + switch typed := value.(type) { + case string: + collectStringEnvironmentTemplates(typed, environment) + case map[string]any: + for _, nested := range typed { + collectEnvironmentTemplates(nested, environment) + } + case []any: + for _, nested := range typed { + collectEnvironmentTemplates(nested, environment) + } + } +} + +func collectStringEnvironmentTemplates(value string, environment map[string]string) { + for offset := 0; offset < len(value); { + startOffset := strings.Index(value[offset:], "${") + if startOffset < 0 { + return + } + start := offset + startOffset + if strings.HasPrefix(value[start:], "${{") { + end := strings.Index(value[start+3:], "}}") + if end < 0 { + return + } + offset = start + end + 5 + continue + } + + name, end, found := environmentTemplateAt(value, start) + if !found { + offset = start + 2 + continue + } + // env is keyed by name, so store one canonical ${NAME}. + // A ${NAME:-default} default is re-applied by the owning + // extension against the raw config at deploy, so the env section + // only needs NAME's resolved base value. Collapsing every form of + // a var to one value also keeps collection deterministic when the + // same var appears with and without a default. + environment[name] = "${" + name + "}" + offset = end + } +} + +func environmentTemplateAt(value string, start int) (string, int, bool) { + if start > 0 && value[start-1] == '$' { + return "", 0, false + } + + index := start + 2 + if index >= len(value) || !isEnvironmentNameStart(value[index]) { + return "", 0, false + } + nameStart := index + index++ + for index < len(value) && isEnvironmentNameCharacter(value[index]) { + index++ + } + name := value[nameStart:index] + + if index < len(value) && value[index] == '}' { + return name, index + 1, true + } + if !strings.HasPrefix(value[index:], ":-") { + return "", 0, false + } + + end, found := environmentTemplateEnd(value, index+2) + if !found { + return "", 0, false + } + return name, end, true +} + +// environmentTemplateEnd skips nested Foundry expressions. +func environmentTemplateEnd(value string, index int) (int, bool) { + depth := 1 + for index < len(value) { + if strings.HasPrefix(value[index:], "${{") { + end := strings.Index(value[index+3:], "}}") + if end < 0 { + return 0, false + } + index += end + 5 + continue + } + if strings.HasPrefix(value[index:], "${") { + depth++ + index += 2 + continue + } + if value[index] == '}' { + depth-- + index++ + if depth == 0 { + return index, true + } + continue + } + index++ + } + return 0, false +} + +func isEnvironmentNameStart(value byte) bool { + return value == '_' || value >= 'A' && value <= 'Z' || + value >= 'a' && value <= 'z' +} + +func isEnvironmentNameCharacter(value byte) bool { + return isEnvironmentNameStart(value) || value >= '0' && value <= '9' +} + +// escapeFoundryTemplates escapes Foundry ${{...}} spans as $${{...}} +// so azd core's envsubst emits a literal ${{...}} for the owning +// extension to resolve. Already-escaped $${{...}} and bare ${VAR} +// are left unchanged, so it is safe on values read back from disk. +func escapeFoundryTemplates(value string) string { + if !strings.Contains(value, "${{") { + return value + } + var b strings.Builder + b.Grow(len(value) + 2) + for i := 0; i < len(value); i++ { + if value[i] == '$' && strings.HasPrefix(value[i:], "${{") && + (i == 0 || value[i-1] != '$') { + b.WriteByte('$') + } + b.WriteByte(value[i]) + } + return b.String() +} + +// setServiceEnvironment writes raw templates through the config RPC. +// AddService treats ServiceConfig.Environment as expanded literals. +func setServiceEnvironment( + ctx context.Context, + azdClient *azdext.AzdClient, + serviceName string, + environment map[string]string, +) error { + if len(environment) == 0 { + return nil + } + + sectionValues := make(map[string]any, len(environment)) + for key, value := range environment { + sectionValues[key] = escapeFoundryTemplates(value) + } + section, err := structpb.NewStruct(sectionValues) + if err != nil { + return fmt.Errorf( + "encoding env for service %q: %w", + serviceName, + err, + ) + } + + // ServiceConfig.Environment only carries expanded values. + // The config RPC preserves raw ${VAR} templates. + _, err = azdClient.Project().SetServiceConfigSection( + ctx, + &azdext.SetServiceConfigSectionRequest{ + ServiceName: serviceName, + Path: "env", + Section: section, + }, + ) + if err != nil { + return fmt.Errorf( + "setting env for service %q: %w", + serviceName, + err, + ) + } + return nil +} + // setServiceUses sets the uses: list on an existing service. uses is a real // core ServiceConfig field, so it is written via SetServiceConfigValue (a raw // map path) rather than AddService's inlined config map, which cannot carry it. diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go index d5851a0d38c..a479a247155 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/resource_services_test.go @@ -91,6 +91,108 @@ func TestReserveServiceName(t *testing.T) { assert.Contains(t, err.Error(), "agent service") } +func TestServiceEnvironmentTemplates(t *testing.T) { + t.Parallel() + + cfg, err := project.MarshalStruct(&project.Connection{ + Credentials: map[string]any{ + "key": "${SEARCH_KEY}", + }, + Metadata: map[string]string{ + "server": "${SERVER_NAME}", + "default": "${DEFAULT_NAME:-fallback}", + "foundry_default": "${EVENT_BODY:-${{event.body}}}", + "token": "${{connections.search.credentials.key}}", + "literal": "$${LITERAL}", + }, + }) + require.NoError(t, err) + + assert.Equal(t, map[string]string{ + "SEARCH_KEY": "${SEARCH_KEY}", + "SERVER_NAME": "${SERVER_NAME}", + "DEFAULT_NAME": "${DEFAULT_NAME}", + "EVENT_BODY": "${EVENT_BODY}", + }, serviceEnvironmentTemplates(cfg)) +} + +// TestServiceEnvironmentTemplatesDeterministic verifies a var +// referenced both bare and with a default collapses to the same +// canonical ${VAR}, so field order cannot change the result. +func TestServiceEnvironmentTemplatesDeterministic(t *testing.T) { + t.Parallel() + + cfg, err := project.MarshalStruct(&project.Connection{ + Metadata: map[string]string{ + "bare": "${TOPIC}", + "default": "${TOPIC:-general}", + "default2": "${TOPIC:-other}", + }, + }) + require.NoError(t, err) + + assert.Equal(t, map[string]string{ + "TOPIC": "${TOPIC}", + }, serviceEnvironmentTemplates(cfg)) +} + +func TestEscapeFoundryTemplates(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + value string + want string + }{ + {"foundry span", "${{event.body}}", "$${{event.body}}"}, + {"already escaped", "$${{event.body}}", "$${{event.body}}"}, + {"single brace untouched", "${VAR}", "${VAR}"}, + {"literal untouched", "info", "info"}, + { + "embedded span", + "prefix-${{connections.x.key}}-suffix", + "prefix-$${{connections.x.key}}-suffix", + }, + { + "span in default", + "${MISSING:-${{event.body}}}", + "${MISSING:-$${{event.body}}}", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, escapeFoundryTemplates(tt.value)) + }) + } +} + +func TestAddResourceServiceWritesEnvironment(t *testing.T) { + server := &recordingProjectServer{} + client := newProjectRecorderClient(t, server) + cfg, err := project.MarshalStruct(&project.Connection{ + Credentials: map[string]any{"key": "${SEARCH_KEY}"}, + }) + require.NoError(t, err) + + require.NoError(t, addResourceService( + t.Context(), + client, + "search", + AiConnectionHost, + cfg, + nil, + )) + + server.mu.Lock() + defer server.mu.Unlock() + require.Len(t, server.added, 1) + assert.Empty(t, server.added[0].GetEnvironment()) + assert.Equal(t, map[string]any{ + "SEARCH_KEY": "${SEARCH_KEY}", + }, server.env["search"]) +} + func TestCollectLegacyProjectDeploymentsIgnoresSplitProject( t *testing.T, ) { @@ -314,11 +416,19 @@ type recordingProjectServer struct { mu sync.Mutex added []*azdext.ServiceConfig uses map[string][]string + env map[string]map[string]any // configValues records non-"uses" SetServiceConfigValue calls keyed by path. configValues map[string]configValueRecord // existing is returned by Get to simulate services already present in the // project (e.g. a prior init's azure.ai.project service). existing map[string]*azdext.ServiceConfig + // rawEnv is returned by GetServiceConfigValue for path "env" to + // simulate a service that already carries an env section (raw, + // on-disk templates). + rawEnv map[string]map[string]any + unsetPaths []string + setEnvironmentErr error + unsetServiceConfigErr error } // configValueRecord captures a single SetServiceConfigValue call. @@ -346,6 +456,26 @@ func (s *recordingProjectServer) AddService( return &azdext.EmptyResponse{}, nil } +func (s *recordingProjectServer) GetServiceConfigValue( + _ context.Context, req *azdext.GetServiceConfigValueRequest, +) (*azdext.GetServiceConfigValueResponse, error) { + s.mu.Lock() + defer s.mu.Unlock() + if req.Path == "env" { + if raw, ok := s.rawEnv[req.ServiceName]; ok { + value, err := structpb.NewValue(raw) + if err != nil { + return nil, err + } + return &azdext.GetServiceConfigValueResponse{ + Found: true, + Value: value, + }, nil + } + } + return &azdext.GetServiceConfigValueResponse{}, nil +} + func (s *recordingProjectServer) SetServiceConfigValue( _ context.Context, req *azdext.SetServiceConfigValueRequest, ) (*azdext.EmptyResponse, error) { @@ -378,6 +508,37 @@ func (s *recordingProjectServer) SetServiceConfigValue( return &azdext.EmptyResponse{}, nil } +func (s *recordingProjectServer) SetServiceConfigSection( + _ context.Context, + req *azdext.SetServiceConfigSectionRequest, +) (*azdext.EmptyResponse, error) { + s.mu.Lock() + defer s.mu.Unlock() + if s.setEnvironmentErr != nil { + return nil, s.setEnvironmentErr + } + if s.env == nil { + s.env = map[string]map[string]any{} + } + if req.Path == "env" && req.Section != nil { + s.env[req.ServiceName] = req.Section.AsMap() + } + return &azdext.EmptyResponse{}, nil +} + +func (s *recordingProjectServer) UnsetServiceConfig( + _ context.Context, + req *azdext.UnsetServiceConfigRequest, +) (*azdext.EmptyResponse, error) { + s.mu.Lock() + defer s.mu.Unlock() + s.unsetPaths = append(s.unsetPaths, req.Path) + if s.unsetServiceConfigErr != nil { + return nil, s.unsetServiceConfigErr + } + return &azdext.EmptyResponse{}, nil +} + // newProjectRecorderClient spins up an in-process gRPC server backed by the // supplied project server stub and returns a client wired to its address. func newProjectRecorderClient(t *testing.T, server azdext.ProjectServiceServer) *azdext.AzdClient { diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go index d9a1401ac4e..07d250df570 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/run.go @@ -10,6 +10,7 @@ import ( "fmt" "io" "log" + "maps" "net" "net/http" "os" @@ -47,11 +48,6 @@ type runFlags struct { channel string } -type environmentEntry struct { - key string - value string -} - func newRunCommand(extCtx *azdext.ExtensionContext) *cobra.Command { flags := &runFlags{} extCtx = ensureExtensionContext(extCtx) @@ -191,59 +187,44 @@ func runRun(ctx context.Context, flags *runFlags, noPrompt bool) error { cmdParts = resolveVenvCommand(projectDir, cmdParts) - env := os.Environ() - env = appendPortEnvVars(env, pt, flags.port) + env := appendPortEnvVars(os.Environ(), pt, flags.port) - // Load azd environment variables (e.g., FOUNDRY_PROJECT_ENDPOINT) - // so the agent can reach Azure services during local development. - // Also translate azd env keys to FOUNDRY_* env vars so the agent code - // works identically whether running locally or in a hosted container - // (where the platform automatically injects FOUNDRY_* env vars). + // Load azd values as template inputs and legacy fallback values. var azdEnvVars map[string]string if loaded, err := loadAzdEnvironment(ctx, azdClient); err == nil { azdEnvVars = loaded - for k, v := range azdEnvVars { - env = append(env, fmt.Sprintf("%s=%s", k, v)) - } - env = appendFoundryEnvVars(env, azdEnvVars, runCtx.ServiceName) } else if shouldWarnLoadAzdEnvironmentFailure(err) { fmt.Fprintf(os.Stderr, "Warning: failed to load azd environment values: %s\n", err) } endpoint, _ := resolveAgentEndpoint(ctx, "", "") - defEnv, defErr := resolveAgentDefinitionEnvVars(ctx, runCtx.Definition, azdEnvVars, endpoint) - if defErr != nil { - fmt.Fprintf(os.Stderr, "Warning: %s\n", defErr) - } - serviceEnv, serviceEnvErr := resolveServiceEnvironmentVars( + endpoint = localProjectEndpoint( + env, + runCtx.ServiceEnvironment, + endpoint, + ) + serviceEnvironment := resolveLocalServiceEnvironment( + runCtx.ServiceEnvironment, + endpoint, + ) + defEnv, defErr := resolveAgentDefinitionEnvVars( ctx, - runCtx.Environment, + runCtx.Definition, + serviceEnvironment, azdEnvVars, endpoint, ) - if serviceEnvErr != nil { - fmt.Fprintf(os.Stderr, "Warning: %s\n", serviceEnvErr) + if defErr != nil { + fmt.Fprintf(os.Stderr, "Warning: %s\n", defErr) } - configuredEnv := mergeConfiguredEnvironmentEntries( + env = mergeAgentRunEnvironment( + env, + azdEnvVars, + serviceEnvironment, defEnv, - serviceEnv, - runtime.GOOS == "windows", + runCtx.ServiceName, + runCtx.HasServiceEnvironment, ) - keys := make([]string, 0, len(configuredEnv)) - for key := range configuredEnv { - keys = append(keys, key) - } - slices.Sort(keys) - for _, key := range keys { - entry := configuredEnv[key] - if !envSliceHasKey(env, entry.key) { - env = append( - env, - fmt.Sprintf("%s=%s", entry.key, entry.value), - ) - } - - } // Activity agents bind IPv4 and are reached at 127.0.0.1 everywhere else // (the port-readiness check and the Playground URL), because `localhost` @@ -537,6 +518,7 @@ func shouldWarnLoadAzdEnvironmentFailure(err error) bool { func resolveAgentDefinitionEnvVars( ctx context.Context, agentDef *agent_yaml.ContainerAgent, + serviceEnvironment map[string]string, azdEnvVars map[string]string, endpoint string, ) ([]string, error) { @@ -567,8 +549,15 @@ func resolveAgentDefinitionEnvVars( if _, isConn := connRefEnvNames[ev.Name]; isConn { continue } - // ExpandEnv returns the original value on error, so a failed expansion is a no-op. - resolved, _ := project.ExpandEnv(ev.Value, lookup) + resolved, err := project.ResolveAgentEnvironmentVariable( + ev.Name, + ev.Value, + serviceEnvironment, + lookup, + ) + if err != nil { + resolved = ev.Value + } result = append(result, fmt.Sprintf("%s=%s", ev.Name, resolved)) } @@ -584,43 +573,39 @@ func resolveAgentDefinitionEnvVars( return result, nil } -func resolveServiceEnvironmentVars( - ctx context.Context, - values map[string]string, - azdEnvVars map[string]string, +func resolveLocalServiceEnvironment( + environment map[string]string, endpoint string, -) ([]string, error) { - if len(values) == 0 { - return nil, nil - } - keys := make([]string, 0, len(values)) - for key := range values { - keys = append(keys, key) - } - slices.Sort(keys) - envVars := make([]agent_yaml.EnvironmentVariable, 0, len(keys)) - for _, key := range keys { - value := values[key] - if endpoint != "" { - value = strings.ReplaceAll( - value, - "${{project.endpoint}}", - endpoint, - ) - } - envVars = append(envVars, agent_yaml.EnvironmentVariable{ - Name: key, - Value: value, - }) +) map[string]string { + resolved := maps.Clone(environment) + if endpoint == "" { + return resolved + } + for key, value := range resolved { + resolved[key] = strings.ReplaceAll( + value, + "${{project.endpoint}}", + endpoint, + ) } - return resolveAgentDefinitionEnvVars( - ctx, - &agent_yaml.ContainerAgent{ - EnvironmentVariables: &envVars, - }, - azdEnvVars, - endpoint, - ) + return resolved +} + +func localProjectEndpoint( + baseEnvironment []string, + serviceEnvironment map[string]string, + fallback string, +) string { + if value, found := envSliceValue( + baseEnvironment, + "FOUNDRY_PROJECT_ENDPOINT", + ); found { + return value + } + if value, found := serviceEnvironment["FOUNDRY_PROJECT_ENDPOINT"]; found { + return value + } + return fallback } // findAgentYaml locates the agent definition file in the given directory. @@ -1032,89 +1017,142 @@ func findSystemPython() (pythonInterpreter, error) { return firstCompatiblePython(pythonCandidates(), pythonVersion) } -// appendFoundryEnvVars translates azd environment keys to FOUNDRY_* env vars that hosted -// agent containers receive automatically from the platform. This ensures the agent code -// works identically whether running locally (via azd ai agent run) or in a hosted container. +// mergeAgentRunEnvironment builds the local agent environment. +// +// baseEnvironment contains process and command-owned values. +// azdEnvironment is the full active environment for legacy fallback. +// serviceEnvironment is core-expanded services..env. +// definitionEnvironment comes from legacy agent definitions. +// hasServiceEnvironment reports whether the service declares an +// env: block (even an empty one). +func mergeAgentRunEnvironment( + baseEnvironment []string, + azdEnvironment map[string]string, + serviceEnvironment map[string]string, + definitionEnvironment []string, + serviceName string, + hasServiceEnvironment bool, +) []string { + environment := slices.Clone(baseEnvironment) + + // The full azd environment is a compatibility fallback only. + if !hasServiceEnvironment { + for key, value := range azdEnvironment { + if !envSliceHasKey(baseEnvironment, key) { + environment = append( + environment, + fmt.Sprintf("%s=%s", key, value), + ) + } + } + } else { + for key, value := range serviceEnvironment { + if !envSliceHasKey(baseEnvironment, key) { + environment = append( + environment, + fmt.Sprintf("%s=%s", key, value), + ) + } + } + } + + environment = appendFoundryEnvVars( + environment, + azdEnvironment, + serviceName, + ) + + for _, entry := range definitionEnvironment { + key, _, _ := strings.Cut(entry, "=") + _, serviceScoped := serviceEnvironment[key] + if serviceScoped { + continue + } + if !envSliceHasKey(environment, key) { + environment = append(environment, entry) + } + } + + return environment +} + +// appendFoundryEnvVars adds values injected by hosted agents. // // The mapping is: // -// AZURE_AI_PROJECT_ID → FOUNDRY_PROJECT_ARM_ID -// AGENT_{SVC}_NAME → FOUNDRY_AGENT_NAME -// AGENT_{SVC}_VERSION → FOUNDRY_AGENT_VERSION -// APPLICATIONINSIGHTS_CONNECTION_STRING (unchanged — already matches platform name) +// FOUNDRY_PROJECT_ENDPOINT -> unchanged +// AZURE_AI_PROJECT_ID -> FOUNDRY_PROJECT_ARM_ID +// AGENT_{SVC}_NAME -> FOUNDRY_AGENT_NAME +// AGENT_{SVC}_VERSION -> FOUNDRY_AGENT_VERSION +// APPLICATIONINSIGHTS_CONNECTION_STRING -> unchanged func appendFoundryEnvVars(env []string, azdEnv map[string]string, serviceName string) []string { - // Static mappings from azd env key names to FOUNDRY_* env var names - staticMappings := []struct { - azdKey string - foundryKey string - }{ - {"AZURE_AI_PROJECT_ID", "FOUNDRY_PROJECT_ARM_ID"}, - } - - for _, m := range staticMappings { - if v := azdEnv[m.azdKey]; v != "" { - if _, exists := azdEnv[m.foundryKey]; !exists && !envSliceHasKey(env, m.foundryKey) { - env = append(env, fmt.Sprintf("%s=%s", m.foundryKey, v)) - } - } + env = appendEnvValue( + env, + "FOUNDRY_PROJECT_ENDPOINT", + azdEnv["FOUNDRY_PROJECT_ENDPOINT"], + ) + + projectArmID := azdEnv["FOUNDRY_PROJECT_ARM_ID"] + if projectArmID == "" { + projectArmID = azdEnv["AZURE_AI_PROJECT_ID"] } + env = appendEnvValue(env, "FOUNDRY_PROJECT_ARM_ID", projectArmID) - // Service-specific mappings (AGENT_{SVC}_NAME → FOUNDRY_AGENT_NAME, etc.) + agentName := "" + agentVersion := "" if serviceName != "" { serviceKey := toServiceKey(serviceName) - agentMappings := []struct { - azdKeyFmt string - foundryKey string - }{ - {"AGENT_%s_NAME", "FOUNDRY_AGENT_NAME"}, - {"AGENT_%s_VERSION", "FOUNDRY_AGENT_VERSION"}, - } - - for _, m := range agentMappings { - azdKey := fmt.Sprintf(m.azdKeyFmt, serviceKey) - if v := azdEnv[azdKey]; v != "" { - if _, exists := azdEnv[m.foundryKey]; !exists && !envSliceHasKey(env, m.foundryKey) { - env = append(env, fmt.Sprintf("%s=%s", m.foundryKey, v)) - } - } - } + agentName = azdEnv[fmt.Sprintf("AGENT_%s_NAME", serviceKey)] + agentVersion = azdEnv[fmt.Sprintf("AGENT_%s_VERSION", serviceKey)] + } + if agentName == "" { + agentName = azdEnv["FOUNDRY_AGENT_NAME"] } + if agentVersion == "" { + agentVersion = azdEnv["FOUNDRY_AGENT_VERSION"] + } + env = appendEnvValue(env, "FOUNDRY_AGENT_NAME", agentName) + env = appendEnvValue(env, "FOUNDRY_AGENT_VERSION", agentVersion) + + env = appendEnvValue( + env, + "APPLICATIONINSIGHTS_CONNECTION_STRING", + azdEnv["APPLICATIONINSIGHTS_CONNECTION_STRING"], + ) return env } -func mergeConfiguredEnvironmentEntries( - definitionEnv []string, - serviceEnv []string, - caseInsensitive bool, -) map[string]environmentEntry { - configuredEnv := map[string]environmentEntry{} - for _, entry := range append(definitionEnv, serviceEnv...) { - key, value, _ := strings.Cut(entry, "=") - lookupKey := key - if caseInsensitive { - lookupKey = strings.ToUpper(key) - } - configuredEnv[lookupKey] = environmentEntry{ - key: key, - value: value, - } +func appendEnvValue(env []string, key string, value string) []string { + if value == "" || envSliceHasKey(env, key) { + return env } - return configuredEnv + return append(env, fmt.Sprintf("%s=%s", key, value)) } // envSliceHasKey reports whether env contains an entry for key. func envSliceHasKey(env []string, key string) bool { - return slices.ContainsFunc(env, func(entry string) bool { - entryKey, _, found := strings.Cut(entry, "=") + _, found := envSliceValue(env, key) + return found +} + +func envSliceValue(env []string, key string) (string, bool) { + for _, entry := range env { + entryKey, value, found := strings.Cut(entry, "=") if !found { - return false + continue } if runtime.GOOS == "windows" { - return strings.EqualFold(entryKey, key) + if strings.EqualFold(entryKey, key) { + return value, true + } + continue } - return entryKey == key - }) + if entryKey == key { + return value, true + } + } + return "", false } // loadAzdEnvironment reads all key-value pairs from the current azd environment. diff --git a/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go b/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go index 72861c8ac16..0baebe95529 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/cmd/run_test.go @@ -439,14 +439,15 @@ func createVenv(t *testing.T, projectDir string) string { func TestAppendFoundryEnvVars(t *testing.T) { t.Parallel() - t.Run("does not map FOUNDRY_PROJECT_ENDPOINT to itself", func(t *testing.T) { + t.Run("forwards FOUNDRY_PROJECT_ENDPOINT", func(t *testing.T) { t.Parallel() azdEnv := map[string]string{ "FOUNDRY_PROJECT_ENDPOINT": "https://myaccount.services.ai.azure.com/api/projects/myproject", } env := appendFoundryEnvVars(nil, azdEnv, "") - if len(env) != 0 { - t.Errorf("expected no translated env vars, got %v", env) + expected := "FOUNDRY_PROJECT_ENDPOINT=https://myaccount.services.ai.azure.com/api/projects/myproject" + if !slices.Contains(env, expected) { + t.Errorf("expected %q in env, got %v", expected, env) } }) @@ -495,12 +496,12 @@ func TestAppendFoundryEnvVars(t *testing.T) { "AGENT_AGENT1_VERSION": "v1", } env := appendFoundryEnvVars(nil, azdEnv, "agent1") - if len(env) != 3 { - t.Errorf("expected 3 env vars, got %d: %v", len(env), env) + if len(env) != 4 { + t.Errorf("expected 4 env vars, got %d: %v", len(env), env) } }) - t.Run("skips foundry key when already set in azd env", func(t *testing.T) { + t.Run("prefers service-specific agent metadata", func(t *testing.T) { t.Parallel() azdEnv := map[string]string{ "FOUNDRY_PROJECT_ENDPOINT": "https://explicit.services.ai.azure.com", @@ -509,20 +510,26 @@ func TestAppendFoundryEnvVars(t *testing.T) { } env := appendFoundryEnvVars(nil, azdEnv, "my-svc") - // Neither FOUNDRY_PROJECT_ENDPOINT nor FOUNDRY_AGENT_NAME should be - // appended because they already exist in azdEnv (and were thus already - // added to the env slice by the caller's loop over azdEnv). - for _, entry := range env { - if strings.HasPrefix(entry, "FOUNDRY_PROJECT_ENDPOINT=") || - strings.HasPrefix(entry, "FOUNDRY_AGENT_NAME=") { - t.Errorf("should not translate when foundry key already in azdEnv, got %q", entry) - } + if !slices.Contains( + env, + "FOUNDRY_PROJECT_ENDPOINT=https://explicit.services.ai.azure.com", + ) { + t.Errorf("expected project endpoint in env, got %v", env) } + if !slices.Contains(env, "FOUNDRY_AGENT_NAME=my-agent") { + t.Errorf("expected service agent name in env, got %v", env) + } + }) - // AZURE_AI_PROJECT_ID has no explicit FOUNDRY_PROJECT_ARM_ID, so it should still be skipped - // (it's not in azdEnv either, so appendFoundryEnvVars skips it because the source key is empty) - if len(env) != 0 { - t.Errorf("expected no translated env vars, got %v", env) + t.Run("forwards application insights", func(t *testing.T) { + t.Parallel() + azdEnv := map[string]string{ + "APPLICATIONINSIGHTS_CONNECTION_STRING": "InstrumentationKey=test", + } + env := appendFoundryEnvVars(nil, azdEnv, "") + expected := "APPLICATIONINSIGHTS_CONNECTION_STRING=InstrumentationKey=test" + if !slices.Contains(env, expected) { + t.Errorf("expected %q in env, got %v", expected, env) } }) @@ -572,38 +579,177 @@ func TestAppendFoundryEnvVars(t *testing.T) { }) } -func TestEnvSliceHasKeyUsesPlatformCasing(t *testing.T) { +func TestMergeAgentRunEnvironment(t *testing.T) { t.Parallel() - env := []string{"Path=process-value"} - if !envSliceHasKey(env, "Path") { - t.Fatal("expected exact-case environment key to match") + value := func(environment []string, key string) (string, bool) { + t.Helper() + for i := len(environment) - 1; i >= 0; i-- { + name, entryValue, found := strings.Cut(environment[i], "=") + if found && name == key { + return entryValue, true + } + } + return "", false } - got := envSliceHasKey(env, "PATH") - want := runtime.GOOS == "windows" - if got != want { - t.Errorf("envSliceHasKey() = %t, want %t", got, want) - } + t.Run("service env wins without leaking azd values", func(t *testing.T) { + t.Parallel() + environment := mergeAgentRunEnvironment( + []string{"FOO=process", "PORT=8088"}, + map[string]string{ + "FOO": "global", + "BAR": "global", + "UNDECLARED_SECRET": "hidden", + "FOUNDRY_PROJECT_ENDPOINT": "https://project.example", + }, + map[string]string{ + "FOO": "service", + "BAR": "service", + "EMPTY": "", + "PORT": "9000", + "SERVICE_ONLY": "service-only", + }, + []string{ + "FOO=service", + "BAR=service", + "PORT=9000", + "LEGACY=declared", + }, + "agent", + true, + ) + + if got, _ := value(environment, "FOO"); got != "process" { + t.Errorf("expected process FOO, got %q", got) + } + if got, _ := value(environment, "BAR"); got != "service" { + t.Errorf("expected service BAR, got %q", got) + } + if got, _ := value(environment, "PORT"); got != "8088" { + t.Errorf("expected command PORT, got %q", got) + } + if _, found := value(environment, "UNDECLARED_SECRET"); found { + t.Errorf("did not expect undeclared azd value in %v", environment) + } + if got, _ := value(environment, "FOUNDRY_PROJECT_ENDPOINT"); got != + "https://project.example" { + t.Errorf("expected Foundry endpoint, got %q", got) + } + if got, _ := value(environment, "LEGACY"); got != "declared" { + t.Errorf("expected declared legacy value, got %q", got) + } + if got, _ := value(environment, "SERVICE_ONLY"); got != "service-only" { + t.Errorf("expected service-only value, got %q", got) + } + if got, found := value(environment, "EMPTY"); !found || got != "" { + t.Errorf("expected empty service value, got %q, found %v", got, found) + } + }) + + t.Run("explicit empty env stays isolated", func(t *testing.T) { + t.Parallel() + environment := mergeAgentRunEnvironment( + []string{"FOO=process"}, + map[string]string{"SECRET": "leaked", "OTHER": "leaked"}, + map[string]string{}, + nil, + "agent", + true, + ) + if _, found := value(environment, "SECRET"); found { + t.Errorf("did not expect azd value in isolated env %v", environment) + } + if got, _ := value(environment, "FOO"); got != "process" { + t.Errorf("expected process FOO, got %q", got) + } + }) + + t.Run("legacy service keeps azd fallback", func(t *testing.T) { + t.Parallel() + environment := mergeAgentRunEnvironment( + []string{"FOO=process"}, + map[string]string{ + "FOO": "global", + "BAR": "global", + "PROJECT_ID": "project", + }, + nil, + []string{"BAR=inline", "BAZ=inline"}, + "agent", + false, + ) + + if got, _ := value(environment, "FOO"); got != "process" { + t.Errorf("expected process FOO, got %q", got) + } + if got, _ := value(environment, "BAR"); got != "global" { + t.Errorf("expected global BAR, got %q", got) + } + if got, _ := value(environment, "PROJECT_ID"); got != "project" { + t.Errorf("expected legacy azd value, got %q", got) + } + if got, _ := value(environment, "BAZ"); got != "inline" { + t.Errorf("expected inline BAZ, got %q", got) + } + }) } -func TestMergeConfiguredEnvironmentEntriesUsesServicePrecedence(t *testing.T) { +func TestResolveLocalServiceEnvironment(t *testing.T) { t.Parallel() - entries := mergeConfiguredEnvironmentEntries( - []string{"PATH=definition-value"}, - []string{"Path=service-value"}, - true, + original := map[string]string{ + "MODEL_ENDPOINT": "${{project.endpoint}}/models", + "LITERAL": "literal ${NOT_A_TEMPLATE}", + } + endpoint := localProjectEndpoint( + []string{"FOUNDRY_PROJECT_ENDPOINT=https://process.example"}, + map[string]string{ + "FOUNDRY_PROJECT_ENDPOINT": "https://service.example", + }, + "https://azd.example", + ) + resolved := resolveLocalServiceEnvironment( + original, + endpoint, ) - if len(entries) != 1 { - t.Fatalf("expected one entry, got %v", entries) + if got := resolved["MODEL_ENDPOINT"]; got != + "https://process.example/models" { + t.Errorf("expected resolved endpoint, got %q", got) + } + if got := resolved["LITERAL"]; got != "literal ${NOT_A_TEMPLATE}" { + t.Errorf("expected literal value, got %q", got) } - if entries["PATH"].key != "Path" { - t.Errorf("key = %q, want %q", entries["PATH"].key, "Path") + if got := original["MODEL_ENDPOINT"]; got != + "${{project.endpoint}}/models" { + t.Errorf("expected original map to stay unchanged, got %q", got) } - if entries["PATH"].value != "service-value" { - t.Errorf("value = %q, want %q", entries["PATH"].value, "service-value") + + serviceEndpoint := localProjectEndpoint( + nil, + map[string]string{ + "FOUNDRY_PROJECT_ENDPOINT": "https://service.example", + }, + "https://azd.example", + ) + if serviceEndpoint != "https://service.example" { + t.Errorf("expected service endpoint, got %q", serviceEndpoint) + } +} + +func TestEnvSliceHasKeyUsesPlatformCasing(t *testing.T) { + t.Parallel() + + env := []string{"Path=process-value"} + if !envSliceHasKey(env, "Path") { + t.Fatal("expected exact-case environment key to match") + } + + got := envSliceHasKey(env, "PATH") + want := runtime.GOOS == "windows" + if got != want { + t.Errorf("envSliceHasKey() = %t, want %t", got, want) } } @@ -929,7 +1075,7 @@ environment_variables: value: debug `) - result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, "") + result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -953,7 +1099,7 @@ environment_variables: azdEnv := map[string]string{ "FOUNDRY_PROJECT_ENDPOINT": "https://example.azure.com", } - result, err := resolveAgentDefinitionEnvVars(t.Context(), def, azdEnv, "") + result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, azdEnv, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -974,7 +1120,7 @@ environment_variables: value: hello `) - result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, "") + result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -990,7 +1136,7 @@ environment_variables: }) t.Run("returns nil for nil definition", func(t *testing.T) { - result, err := resolveAgentDefinitionEnvVars(t.Context(), nil, nil, "") + result, err := resolveAgentDefinitionEnvVars(t.Context(), nil, nil, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1002,7 +1148,7 @@ environment_variables: t.Run("returns nil for empty environment_variables", func(t *testing.T) { def := parse(t, "name: test-agent\n") - result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, "") + result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, nil, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1018,7 +1164,7 @@ environment_variables: value: ${DOES_NOT_EXIST} `) - result, err := resolveAgentDefinitionEnvVars(t.Context(), def, map[string]string{}, "") + result, err := resolveAgentDefinitionEnvVars(t.Context(), def, nil, map[string]string{}, "") if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1026,35 +1172,28 @@ environment_variables: t.Errorf("expected MISSING_REF= (empty), got %v", result) } }) -} - -func TestResolveServiceEnvironmentVars(t *testing.T) { - t.Parallel() - result, err := resolveServiceEnvironmentVars( - t.Context(), - map[string]string{ - "ENDPOINT": "${FOUNDRY_PROJECT_ENDPOINT}/agents", - "PROJECT": "${{project.endpoint}}", - "STATIC": "value", - }, - map[string]string{ - "FOUNDRY_PROJECT_ENDPOINT": "https://example", - }, - "https://example/project", - ) + t.Run("keeps forwarded core values literal", func(t *testing.T) { + def := parse(t, `name: test-agent +environment_variables: + - name: FORWARDED_VALUE + value: ${FORWARDED_VALUE} +`) - if err != nil { - t.Fatalf("resolve service environment: %v", err) - } - want := []string{ - "ENDPOINT=https://example/agents", - "PROJECT=https://example/project", - "STATIC=value", - } - if !slices.Equal(want, result) { - t.Fatalf("expected %v, got %v", want, result) - } + result, err := resolveAgentDefinitionEnvVars( + t.Context(), + def, + map[string]string{"FORWARDED_VALUE": "literal ${NOT_A_TEMPLATE}"}, + map[string]string{"NOT_A_TEMPLATE": "expanded"}, + "", + ) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !slices.Contains(result, "FORWARDED_VALUE=literal ${NOT_A_TEMPLATE}") { + t.Errorf("expected literal forwarded value, got %v", result) + } + }) } func TestVenvPip(t *testing.T) { diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go index c9e6af5a4c5..92de5663f76 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition.go @@ -7,6 +7,7 @@ import ( "fmt" "maps" "os" + "slices" "sync" "azureaiagent/internal/exterrors" @@ -84,11 +85,12 @@ func WarnLegacyAgentShape(source AgentDefinitionSource) { type AgentDefinitionInline struct { agent_yaml.AgentDefinition `json:",inline"` Protocols []agent_yaml.ProtocolVersionRecord `json:"protocols,omitempty"` - EnvironmentVariables *[]agent_yaml.EnvironmentVariable `json:"environmentVariables,omitempty"` - AgentEndpoint *agent_yaml.AgentEndpoint `json:"agentEndpoint,omitempty"` - AgentCard *agent_yaml.AgentCard `json:"agentCard,omitempty"` - CodeConfiguration *agent_yaml.CodeConfiguration `json:"codeConfiguration,omitempty"` - Policies []agent_yaml.Policy `json:"policies,omitempty"` + // EnvironmentVariables reads the deprecated inline shape. + EnvironmentVariables *[]agent_yaml.EnvironmentVariable `json:"environmentVariables,omitempty"` + AgentEndpoint *agent_yaml.AgentEndpoint `json:"agentEndpoint,omitempty"` + AgentCard *agent_yaml.AgentCard `json:"agentCard,omitempty"` + CodeConfiguration *agent_yaml.CodeConfiguration `json:"codeConfiguration,omitempty"` + Policies []agent_yaml.Policy `json:"policies,omitempty"` } // agentDefinitionToInline splits a ContainerAgent into the inline definition, @@ -97,13 +99,12 @@ type AgentDefinitionInline struct { // returned separately so the caller can place them on their respective homes. func agentDefinitionToInline(ca agent_yaml.ContainerAgent) (AgentDefinitionInline, *ContainerSettings, string) { inline := AgentDefinitionInline{ - AgentDefinition: ca.AgentDefinition, - Protocols: ca.Protocols, - EnvironmentVariables: ca.EnvironmentVariables, - AgentEndpoint: ca.AgentEndpoint, - AgentCard: ca.AgentCard, - CodeConfiguration: ca.CodeConfiguration, - Policies: ca.Policies, + AgentDefinition: ca.AgentDefinition, + Protocols: ca.Protocols, + AgentEndpoint: ca.AgentEndpoint, + AgentCard: ca.AgentCard, + CodeConfiguration: ca.CodeConfiguration, + Policies: ca.Policies, } var container *ContainerSettings @@ -119,12 +120,28 @@ func agentDefinitionToInline(ca agent_yaml.ContainerAgent) (AgentDefinitionInlin // toContainerAgent rebuilds the agent_yaml.ContainerAgent from the inline // definition, the CPU/memory carried in the `container` config, and the image // carried on the core service field. -func (d AgentDefinitionInline) toContainerAgent(container *ContainerSettings, image string) agent_yaml.ContainerAgent { +func (d AgentDefinitionInline) toContainerAgent( + container *ContainerSettings, + image string, + environment map[string]string, +) agent_yaml.ContainerAgent { + environmentVariables := d.EnvironmentVariables + if len(environment) > 0 { + legacyEnvironment := AgentEnvironment(agent_yaml.ContainerAgent{ + EnvironmentVariables: d.EnvironmentVariables, + }) + if legacyEnvironment == nil { + legacyEnvironment = map[string]string{} + } + maps.Copy(legacyEnvironment, environment) + environmentVariables = environmentVariablesFromMap(legacyEnvironment) + } + ca := agent_yaml.ContainerAgent{ AgentDefinition: d.AgentDefinition, Image: image, Protocols: d.Protocols, - EnvironmentVariables: d.EnvironmentVariables, + EnvironmentVariables: environmentVariables, AgentEndpoint: d.AgentEndpoint, AgentCard: d.AgentCard, CodeConfiguration: d.CodeConfiguration, @@ -141,6 +158,61 @@ func (d AgentDefinitionInline) toContainerAgent(container *ContainerSettings, im return ca } +// AgentEnvironment converts an agent environment list to a map. +func AgentEnvironment(ca agent_yaml.ContainerAgent) map[string]string { + if ca.EnvironmentVariables == nil || len(*ca.EnvironmentVariables) == 0 { + return nil + } + + environment := make(map[string]string, len(*ca.EnvironmentVariables)) + for _, variable := range *ca.EnvironmentVariables { + environment[variable.Name] = variable.Value + } + return environment +} + +// ResolveAgentEnvironmentVariable preserves values forwarded by core. +func ResolveAgentEnvironmentVariable( + name string, + value string, + serviceEnvironment map[string]string, + mapping func(string) string, +) (string, error) { + if environmentValue, found := serviceEnvironment[name]; found { + return environmentValue, nil + } + return ExpandEnv(value, func(variableName string) string { + if environmentValue, found := serviceEnvironment[variableName]; found { + return environmentValue + } + if mapping == nil { + return "" + } + return mapping(variableName) + }) +} + +func environmentVariablesFromMap( + environment map[string]string, +) *[]agent_yaml.EnvironmentVariable { + if len(environment) == 0 { + return nil + } + + variables := make( + []agent_yaml.EnvironmentVariable, + 0, + len(environment), + ) + for _, name := range slices.Sorted(maps.Keys(environment)) { + variables = append(variables, agent_yaml.EnvironmentVariable{ + Name: name, + Value: environment[name], + }) + } + return &variables +} + // structHasKind reports whether the struct carries a non-empty string `kind`, // the marker that an agent definition is present in a service entry's inline or // config properties. @@ -227,6 +299,7 @@ func AgentDefinitionFromResolvedService( ca, isHosted, err := agentDefinitionFromStruct( resolved, image, + svc.GetEnvironment(), ) return ca, isHosted, true, candidate.source, err } @@ -287,7 +360,11 @@ func AgentDefinitionFromService( } } - ca, isHosted, err := agentDefinitionFromStruct(inlineStruct, svc.GetImage()) + ca, isHosted, err := agentDefinitionFromStruct( + inlineStruct, + svc.GetImage(), + svc.GetEnvironment(), + ) return ca, isHosted, true, source, err } @@ -464,6 +541,7 @@ func validateRootRefCoreFields( return err } for _, field := range []string{ + "env", "project", "language", "image", @@ -479,11 +557,7 @@ func validateRootRefCoreFields( return nil } -// UpsertAgentEnvVars adds or updates environment variables on the agent -// definition carried inline on the service entry, preserving every other key. -// It is used by commands that mutate the definition (e.g. `optimize apply`). -// Returns an error when the service carries no inline definition; callers fall -// back to mutating a legacy on-disk agent.yaml in that case. +// UpsertAgentEnvVars updates the service-level environment map. func UpsertAgentEnvVars(svc *azdext.ServiceConfig, kv map[string]string) error { ca, _, found, source, err := AgentDefinitionFromService(svc) if err != nil { @@ -493,55 +567,42 @@ func UpsertAgentEnvVars(svc *azdext.ServiceConfig, kv map[string]string) error { return fmt.Errorf("service %q does not carry an inline agent definition", svc.GetName()) } - envVars := []agent_yaml.EnvironmentVariable{} - if ca.EnvironmentVariables != nil { - envVars = *ca.EnvironmentVariables - } - for key, value := range kv { - idx := -1 - for i := range envVars { - if envVars[i].Name == key { - idx = i - break - } - } - if idx >= 0 { - envVars[idx].Value = value - } else { - envVars = append(envVars, agent_yaml.EnvironmentVariable{Name: key, Value: value}) - } + environment := AgentEnvironment(ca) + if environment == nil { + environment = map[string]string{} } - ca.EnvironmentVariables = &envVars + maps.Copy(environment, kv) + svc.Environment = environment - var props *structpb.Struct + props := svc.GetAdditionalProperties() if source == AgentDefinitionSourceLegacyConfig { props = svc.GetConfig() - } else { - props = svc.GetAdditionalProperties() } - if props == nil { - return fmt.Errorf( - "service %q does not carry an inline agent definition", - svc.GetName(), - ) - } - if props.Fields == nil { - props.Fields = map[string]*structpb.Value{} + if props != nil { + delete(props.Fields, "environmentVariables") } + return nil +} - envValues := make([]*structpb.Value, 0, len(envVars)) - for _, envVar := range envVars { - envValues = append(envValues, structpb.NewStructValue( - &structpb.Struct{Fields: map[string]*structpb.Value{ - "name": structpb.NewStringValue(envVar.Name), - "value": structpb.NewStringValue(envVar.Value), - }}, - )) +// InlineAgentEnvironmentVariables returns the deprecated inline +// environmentVariables carried on the agent definition as a raw +// template map, without merging the core-forwarded (already expanded) +// service environment. Values are the templates as authored, suitable +// for migrating into the env section without losing them. +func InlineAgentEnvironmentVariables( + svc *azdext.ServiceConfig, +) (map[string]string, error) { + props := ServiceConfigProps(svc) + if props == nil || len(props.GetFields()) == 0 { + return nil, nil } - props.Fields["environmentVariables"] = structpb.NewListValue( - &structpb.ListValue{Values: envValues}, - ) - return nil + var inline AgentDefinitionInline + if err := UnmarshalStruct(props, &inline); err != nil { + return nil, err + } + return AgentEnvironment(agent_yaml.ContainerAgent{ + EnvironmentVariables: inline.EnvironmentVariables, + }), nil } // SetAgentContainerSettings writes the resolved container settings onto the @@ -582,7 +643,11 @@ func SetAgentContainerSettings(svc *azdext.ServiceConfig, container *ContainerSe // struct that carries the agent definition as service-level properties. coreImage // is the value of the service's `image` field, which is carried on the core // [azdext.ServiceConfig] rather than in the inline property bag. -func agentDefinitionFromStruct(s *structpb.Struct, coreImage string) (agent_yaml.ContainerAgent, bool, error) { +func agentDefinitionFromStruct( + s *structpb.Struct, + coreImage string, + environment map[string]string, +) (agent_yaml.ContainerAgent, bool, error) { var inline AgentDefinitionInline if err := UnmarshalStruct(s, &inline); err != nil { return agent_yaml.ContainerAgent{}, false, exterrors.Validation( @@ -608,7 +673,7 @@ func agentDefinitionFromStruct(s *structpb.Struct, coreImage string) (agent_yaml ) } - ca := inline.toContainerAgent(cfg.Container, coreImage) + ca := inline.toContainerAgent(cfg.Container, coreImage, environment) if err := validateAgentServiceDefinition(ca); err != nil { return agent_yaml.ContainerAgent{}, false, err diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go index 9316af7e4c3..f10e818124e 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/agent_definition_test.go @@ -52,11 +52,14 @@ func TestAgentDefinitionRoundTrip(t *testing.T) { props, err := AgentDefinitionToServiceProperties(ca, extra) require.NoError(t, err) + _, hasInlineEnvironment := props.GetFields()["environmentVariables"] + require.False(t, hasInlineEnvironment) svc := &azdext.ServiceConfig{ Name: "basic-agent", Host: "azure.ai.agent", AdditionalProperties: props, + Environment: AgentEnvironment(ca), } got, isHosted, found, source, err := AgentDefinitionFromService(svc) @@ -111,6 +114,118 @@ func TestAgentDefinitionFromService_LegacyConfigShape(t *testing.T) { require.Equal(t, "basic-agent", got.Name) } +func TestAgentDefinitionFromService_LegacyEnvironment(t *testing.T) { + props, err := AgentDefinitionToServiceProperties( + sampleContainerAgent(), + nil, + ) + require.NoError(t, err) + legacyEnvironment, err := structpb.NewValue([]any{ + map[string]any{ + "name": "LEGACY_KEY", + "value": "${LEGACY_KEY}", + }, + map[string]any{ + "name": "SHARED_KEY", + "value": "legacy", + }, + }) + require.NoError(t, err) + props.Fields["environmentVariables"] = legacyEnvironment + + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: "azure.ai.agent", + Config: props, + Environment: map[string]string{ + "NEW_KEY": "new", + "SHARED_KEY": "service", + }, + } + got, _, found, source, err := AgentDefinitionFromService(svc) + require.NoError(t, err) + require.True(t, found) + require.Equal(t, AgentDefinitionSourceLegacyConfig, source) + require.Equal(t, map[string]string{ + "LEGACY_KEY": "${LEGACY_KEY}", + "NEW_KEY": "new", + "SHARED_KEY": "service", + }, AgentEnvironment(got)) +} + +// TestInlineAgentEnvironmentVariables verifies the raw inline +// environmentVariables are returned as authored, without merging the +// core-forwarded (already expanded) service environment. +func TestInlineAgentEnvironmentVariables(t *testing.T) { + props, err := AgentDefinitionToServiceProperties( + sampleContainerAgent(), + nil, + ) + require.NoError(t, err) + legacyEnvironment, err := structpb.NewValue([]any{ + map[string]any{ + "name": "LEGACY_KEY", + "value": "${LEGACY_KEY}", + }, + map[string]any{ + "name": "SHARED_KEY", + "value": "legacy", + }, + }) + require.NoError(t, err) + props.Fields["environmentVariables"] = legacyEnvironment + + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: "azure.ai.agent", + AdditionalProperties: props, + // Core-forwarded env must NOT leak into the raw result. + Environment: map[string]string{ + "NEW_KEY": "new", + "SHARED_KEY": "service", + }, + } + got, err := InlineAgentEnvironmentVariables(svc) + require.NoError(t, err) + require.Equal(t, map[string]string{ + "LEGACY_KEY": "${LEGACY_KEY}", + "SHARED_KEY": "legacy", + }, got) +} +func TestResolveAgentEnvironmentVariable(t *testing.T) { + t.Parallel() + + t.Run("preserves same-name core value", func(t *testing.T) { + value, err := ResolveAgentEnvironmentVariable( + "FORWARDED_VALUE", + "${FORWARDED_VALUE}", + map[string]string{ + "FORWARDED_VALUE": "literal ${NOT_A_TEMPLATE}", + }, + func(string) string { + return "expanded" + }, + ) + require.NoError(t, err) + require.Equal(t, "literal ${NOT_A_TEMPLATE}", value) + }) + + t.Run("resolves aliases from service env first", func(t *testing.T) { + value, err := ResolveAgentEnvironmentVariable( + "TARGET", + "${SERVICE_ENDPOINT}", + map[string]string{ + "SERVICE_ENDPOINT": "https://service.example", + }, + func(string) string { + return "https://project.example" + }, + ) + require.NoError(t, err) + require.Equal(t, "https://service.example", value) + }) +} + func TestLoadAgentDefinition_UnrelatedInlineFallsBackToConfig( t *testing.T, ) { @@ -415,6 +530,7 @@ func TestResolveServiceConfigInPlaceRejectsCoreFieldsFromRootRef(t *testing.T) { name string value string }{ + {name: "env", value: "env:\n LOG_LEVEL: info\n"}, {name: "project", value: "project: src/agent\n"}, {name: "language", value: "language: docker\n"}, {name: "image", value: "image: registry.example/agent:v1\n"}, @@ -499,9 +615,15 @@ func TestAgentDefinitionUsesFileRefIgnoresInlineDefinition(t *testing.T) { // TestUpsertAgentEnvVars verifies that env vars are added/updated on the inline // definition while preserving the other definition keys. func TestUpsertAgentEnvVars(t *testing.T) { - props, err := AgentDefinitionToServiceProperties(sampleContainerAgent(), nil) + ca := sampleContainerAgent() + props, err := AgentDefinitionToServiceProperties(ca, nil) require.NoError(t, err) - svc := &azdext.ServiceConfig{Name: "basic-agent", Host: "azure.ai.agent", AdditionalProperties: props} + svc := &azdext.ServiceConfig{ + Name: "basic-agent", + Host: "azure.ai.agent", + AdditionalProperties: props, + Environment: AgentEnvironment(ca), + } require.NoError(t, UpsertAgentEnvVars(svc, map[string]string{ "FOUNDRY_MODEL_DEPLOYMENT_NAME": "gpt-4o", // update existing @@ -513,11 +635,10 @@ func TestUpsertAgentEnvVars(t *testing.T) { require.True(t, found) require.Equal(t, "basic-agent", got.Name) // other keys preserved require.NotNil(t, got.EnvironmentVariables) + _, hasInlineEnvironment := props.GetFields()["environmentVariables"] + require.False(t, hasInlineEnvironment) - values := map[string]string{} - for _, ev := range *got.EnvironmentVariables { - values[ev.Name] = ev.Value - } + values := AgentEnvironment(got) require.Equal(t, "gpt-4o", values["FOUNDRY_MODEL_DEPLOYMENT_NAME"]) require.Equal(t, "cand-1", values["OPTIMIZATION_CANDIDATE_ID"]) } diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/config.go b/cli/azd/extensions/azure.ai.agents/internal/project/config.go index 8a581fc73a8..144d25fb60a 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/config.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/config.go @@ -45,7 +45,6 @@ type ServiceTargetAgentConfig struct { // Foundry project. Its presence is the brownfield signal that makes provision // connect to that project instead of creating a new one. Endpoint string `json:"endpoint,omitempty"` - Environment map[string]string `json:"env,omitempty"` Container *ContainerSettings `json:"container,omitempty"` Deployments []Deployment `json:"deployments,omitempty"` Resources []Resource `json:"resources,omitempty"` diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/config_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/config_test.go index 33c8819ff18..ac4fa16b397 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/config_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/config_test.go @@ -147,7 +147,6 @@ func TestServiceTargetAgentConfig_MultipleToolboxes(t *testing.T) { // alongside other ServiceTargetAgentConfig fields. func TestServiceTargetAgentConfig_WithOtherFields(t *testing.T) { original := ServiceTargetAgentConfig{ - Environment: map[string]string{"KEY": "VALUE"}, Deployments: []Deployment{ { Name: "test-deployment", @@ -187,10 +186,6 @@ func TestServiceTargetAgentConfig_WithOtherFields(t *testing.T) { t.Fatalf("UnmarshalStruct failed: %v", err) } - if roundTripped.Environment["KEY"] != "VALUE" { - t.Errorf("Expected env KEY=VALUE, got '%s'", roundTripped.Environment["KEY"]) - } - if len(roundTripped.Deployments) != 1 { t.Fatalf("Expected 1 deployment, got %d", len(roundTripped.Deployments)) } diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go index c2e410aa174..c3fdc3d1cea 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent.go @@ -15,6 +15,7 @@ import ( "io" "io/fs" "log" + "maps" "net/http" "net/url" "os" @@ -31,7 +32,6 @@ import ( "azureaiagent/internal/pkg/agents/agent_yaml" "azureaiagent/internal/pkg/azure" "azureaiagent/internal/pkg/paths" - "azureaiagent/internal/pkg/projectconfig" "github.com/Azure/azure-sdk-for-go/sdk/azcore" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" @@ -1442,6 +1442,25 @@ func (p *AgentServiceTargetProvider) prepareDeploy( fmt.Fprintf(os.Stderr, "Using endpoint: %s\n", azdEnv["FOUNDRY_PROJECT_ENDPOINT"]) fmt.Fprintf(os.Stderr, "Agent Name: %s\n", agentDef.Name) + // Seed core-expanded values before resolving legacy variables. + resolvedEnvVars := maps.Clone(serviceConfig.GetEnvironment()) + if resolvedEnvVars == nil { + resolvedEnvVars = make(map[string]string) + } + if agentDef.EnvironmentVariables != nil { + for _, envVar := range *agentDef.EnvironmentVariables { + if _, found := resolvedEnvVars[envVar.Name]; found { + continue + } + resolvedEnvVars[envVar.Name] = p.resolveEnvironmentVariables( + envVar.Name, + envVar.Value, + serviceConfig.GetEnvironment(), + azdEnv, + ) + } + } + // Parse service config for container resource overrides foundryAgentConfig, err := LoadServiceTargetAgentConfig(serviceConfig) if err != nil { @@ -1451,34 +1470,6 @@ func (p *AgentServiceTargetProvider) prepareDeploy( "check the service configuration in azure.yaml", ) } - serviceEnv, err := p.serviceEnvironment(serviceConfig) - if err != nil { - return nil, exterrors.Validation( - exterrors.CodeInvalidServiceConfig, - fmt.Sprintf( - "failed to load service environment: %s", - err, - ), - "fix the service env configuration in azure.yaml", - ) - } - - resolvedEnvVars := make(map[string]string) - if agentDef.EnvironmentVariables != nil { - for _, envVar := range *agentDef.EnvironmentVariables { - resolvedEnvVars[envVar.Name] = - p.resolveEnvironmentVariables(envVar.Value, azdEnv) - } - } - for name, value := range foundryAgentConfig.Environment { - resolvedEnvVars[name] = - p.resolveEnvironmentVariables(value, azdEnv) - } - for name, value := range serviceEnv { - resolvedEnvVars[name] = - p.resolveEnvironmentVariables(value, azdEnv) - } - warnDeprecatedScaleSettings(ServiceConfigProps(serviceConfig)) var cpu, memory string @@ -1534,22 +1525,6 @@ func (p *AgentServiceTargetProvider) prepareDeploy( }, nil } -func (p *AgentServiceTargetProvider) serviceEnvironment( - serviceConfig *azdext.ServiceConfig, -) (map[string]string, error) { - raw, err := projectconfig.LoadServiceEnvironment( - p.projectPath, - serviceConfig.GetName(), - ) - if err != nil { - return nil, err - } - if raw != nil { - return raw, nil - } - return serviceConfig.GetEnvironment(), nil -} - // deployResult holds the intermediate results from a deploy method (code or container) // before the common post-deploy steps (polling, patching, finalization) are applied. type deployResult struct { @@ -2684,12 +2659,21 @@ func (p *AgentServiceTargetProvider) registerAgentEnvironmentVariables( return nil } -// resolveEnvironmentVariables resolves ${ENV_VAR} style references in value using azd environment variables. -// Supports default values (e.g., "${VAR:-default}") and multiple expressions (e.g., "${VAR1}-${VAR2}"). -func (p *AgentServiceTargetProvider) resolveEnvironmentVariables(value string, azdEnv map[string]string) string { - resolved, err := ExpandEnv(value, func(varName string) string { - return azdEnv[varName] - }) +// resolveEnvironmentVariables expands legacy inline templates. +func (p *AgentServiceTargetProvider) resolveEnvironmentVariables( + name string, + value string, + serviceEnvironment map[string]string, + azdEnv map[string]string, +) string { + resolved, err := ResolveAgentEnvironmentVariable( + name, + value, + serviceEnvironment, + func(varName string) string { + return azdEnv[varName] + }, + ) if err != nil { // If resolution fails, return original value return value diff --git a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go index 307571d6023..376f4d99e47 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/project/service_target_agent_test.go @@ -1004,6 +1004,51 @@ func TestLoadContainerAgentDefinition_MalformedYAMLReturnsError(t *testing.T) { require.Contains(t, err.Error(), "agent.yaml is not valid") } +func TestPrepareDeployIncludesServiceEnvironment(t *testing.T) { + t.Parallel() + + agentDef := sampleContainerAgent() + *agentDef.EnvironmentVariables = append( + *agentDef.EnvironmentVariables, + agent_yaml.EnvironmentVariable{ + Name: "LEGACY_ONLY", + Value: "${GLOBAL_VALUE}", + }, + agent_yaml.EnvironmentVariable{ + Name: "SHARED", + Value: "${SHARED}", + }, + ) + serviceConfig := &azdext.ServiceConfig{ + Name: "basic-agent", + Environment: map[string]string{ + "SERVICE_ONLY": "literal ${NOT_A_TEMPLATE}", + "SHARED": "service", + }, + } + + prep, err := (&AgentServiceTargetProvider{}).prepareDeploy( + serviceConfig, + agentDef, + map[string]string{ + "FOUNDRY_PROJECT_ENDPOINT": "https://project.example", + "GLOBAL_VALUE": "legacy", + "SHARED": "global", + }, + []agent_yaml.AgentBuildOption{ + agent_yaml.WithImageURL("registry.example/agent:latest"), + }, + ) + require.NoError(t, err) + require.Equal( + t, + "literal ${NOT_A_TEMPLATE}", + prep.resolvedEnvVars["SERVICE_ONLY"], + ) + require.Equal(t, "service", prep.resolvedEnvVars["SHARED"]) + require.Equal(t, "legacy", prep.resolvedEnvVars["LEGACY_ONLY"]) +} + func TestLoadContainerAgentDefinition_EnvPathOverridesInlineDefinition(t *testing.T) { t.Parallel() @@ -1097,129 +1142,6 @@ func TestPackageBuildsContainerAgent(t *testing.T) { require.Equal(t, int32(1), containerStub.packageCalls.Load()) } -func TestPrepareDeploy_MergesUnifiedEnvironment(t *testing.T) { - t.Parallel() - - agentDef := sampleContainerAgent() - agentDef.EnvironmentVariables = &[]agent_yaml.EnvironmentVariable{ - {Name: "LEGACY_ONLY", Value: "${LEGACY_VALUE}"}, - {Name: "SHARED", Value: "legacy"}, - } - props, err := AgentDefinitionToServiceProperties( - agentDef, - &ServiceTargetAgentConfig{ - Environment: map[string]string{ - "REF_ONLY": "${REF_VALUE}", - "SHARED": "ref", - }, - }, - ) - require.NoError(t, err) - svc := &azdext.ServiceConfig{ - Name: "basic-agent", - AdditionalProperties: props, - Environment: map[string]string{ - "DIRECT_ONLY": "direct", - "SHARED": "direct", - }, - } - provider := &AgentServiceTargetProvider{} - - prep, err := provider.prepareDeploy( - svc, - agentDef, - map[string]string{ - "FOUNDRY_PROJECT_ENDPOINT": "https://example", - "LEGACY_VALUE": "legacy-value", - "REF_VALUE": "ref-value", - }, - []agent_yaml.AgentBuildOption{ - agent_yaml.WithImageURL("registry.example/agent:v1"), - }, - ) - - require.NoError(t, err) - definition, ok := prep.request.Definition.(agent_api.HostedAgentDefinition) - require.True(t, ok) - require.Equal( - t, - "legacy-value", - definition.EnvironmentVariables["LEGACY_ONLY"], - ) - require.Equal( - t, - "ref-value", - definition.EnvironmentVariables["REF_ONLY"], - ) - require.Equal( - t, - "direct", - definition.EnvironmentVariables["DIRECT_ONLY"], - ) - require.Equal( - t, - "direct", - definition.EnvironmentVariables["SHARED"], - ) -} - -func TestPrepareDeployUsesRawUnifiedEnvironment(t *testing.T) { - t.Parallel() - - root := t.TempDir() - require.NoError(t, os.WriteFile( - filepath.Join(root, "azure.yaml"), - []byte(`services: - basic-agent: - host: azure.ai.agent - env: - PROJECT: ${{project.endpoint}} - ENABLED: true -`), - 0o600, - )) - agentDef := sampleContainerAgent() - props, err := AgentDefinitionToServiceProperties( - agentDef, - &ServiceTargetAgentConfig{}, - ) - require.NoError(t, err) - svc := &azdext.ServiceConfig{ - Name: "basic-agent", - AdditionalProperties: props, - Environment: map[string]string{ - "PROJECT": "", - "ENABLED": "", - }, - } - provider := &AgentServiceTargetProvider{projectPath: root} - - prep, err := provider.prepareDeploy( - svc, - agentDef, - map[string]string{ - "FOUNDRY_PROJECT_ENDPOINT": "https://example", - }, - []agent_yaml.AgentBuildOption{ - agent_yaml.WithImageURL("registry.example/agent:v1"), - }, - ) - - require.NoError(t, err) - definition, ok := prep.request.Definition.(agent_api.HostedAgentDefinition) - require.True(t, ok) - require.Equal( - t, - "${{project.endpoint}}", - definition.EnvironmentVariables["PROJECT"], - ) - require.Equal( - t, - "true", - definition.EnvironmentVariables["ENABLED"], - ) -} - func TestPrepareDeployAppliesDefaultResources(t *testing.T) { t.Parallel() diff --git a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go index 8c4e7d7baea..41781ecd0b5 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go +++ b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer.go @@ -52,12 +52,16 @@ type Input struct { // value is not checked (only existence and endpoint: are). AcceptedHosts []string - // Env maps azd environment variable names to values. Used to resolve - // ${VAR} references in network fields (subnet vnet ids, dns.subscription). - // When a referenced variable is absent here, the synthesizer falls back - // to the process environment before failing. May be nil. + // Env maps project-wide azd values. + // Network fields always use it. + // Legacy connection services use it when service env is absent. + // Missing values may fall back to the process environment. Env map[string]string + // ServiceEnvironments contains core-expanded values by service. + // Connection fields prefer these values over the legacy Env map. + ServiceEnvironments map[string]map[string]string + // PreserveVarRefs keeps ${VAR} references verbatim instead of resolving // them. Used by the eject path, where the synthesized main.parameters.json // must stay environment-portable: the on-disk provision flow resolves @@ -291,6 +295,7 @@ func Synthesize(in Input) (*Result, error) { connections, err := collectConnections( root.Services, in.Env, + in.ServiceEnvironments, !in.PreserveVarRefs, in.ProjectRoot, ) @@ -362,6 +367,7 @@ func BrownfieldDeployments( func BrownfieldConnections( raw []byte, env map[string]string, + serviceEnvironments map[string]map[string]string, projectRoot string, ) ([]Connection, error) { if len(raw) == 0 { @@ -373,7 +379,13 @@ func BrownfieldConnections( return nil, fmt.Errorf("parse azure.yaml: %w", err) } - return collectConnections(root.Services, env, true, projectRoot) + return collectConnections( + root.Services, + env, + serviceEnvironments, + true, + projectRoot, + ) } // ProjectEndpoint returns the endpoint configured on a Foundry project service. @@ -629,12 +641,13 @@ func agentNeedsAcr(a agentBlock) bool { // (the service key is the connection name) and returns them sorted by name so // the synthesized parameter is deterministic regardless of YAML map order. // -// ${VAR} in target/credentials/metadata is expanded from env when resolve is -// true (provision path) and kept verbatim when false (eject path); Foundry -// ${{...}} expressions are always preserved, mirroring synthesizeNetwork. +// Provisioning resolves ${VAR} from service env when present. +// Legacy services use project and process values. +// Eject keeps references, and Foundry ${{...}} remains unchanged. func collectConnections( services map[string]yaml.Node, env map[string]string, + serviceEnvironments map[string]map[string]string, resolve bool, projectRoot string, ) ([]Connection, error) { @@ -660,17 +673,27 @@ func collectConnections( return nil, fmt.Errorf("services.%s: decode connection: %w", name, err) } - target, err := maybeExpand(svc.Target, env, resolve) + declared := len(serviceEnvironments[name]) > 0 || connectionEnvDeclared(node) + mapping := connectionEnvironmentMapping( + env, + serviceEnvironments[name], + declared, + ) + target, err := maybeExpand(svc.Target, mapping, resolve) if err != nil { return nil, fmt.Errorf("services.%s.target: %w", name, err) } - credentials, err := expandCredentials(svc.Credentials, env, resolve) + credentials, err := expandCredentials( + svc.Credentials, + mapping, + resolve, + ) if err != nil { return nil, fmt.Errorf("services.%s.credentials: %w", name, err) } - metadata, err := expandMetadata(svc.Metadata, env, resolve) + metadata, err := expandMetadata(svc.Metadata, mapping, resolve) if err != nil { return nil, fmt.Errorf("services.%s.metadata: %w", name, err) } @@ -691,20 +714,54 @@ func collectConnections( return connections, nil } +// connectionEnvDeclared reports whether the service node +// declares an env: key, including an empty env: {}. Core +// collapses an empty env to an omitted one, so the raw node +// is the only signal that a service opted into an isolated +// (possibly empty) scope. +func connectionEnvDeclared(node yaml.Node) bool { + var fields map[string]yaml.Node + if err := node.Decode(&fields); err != nil { + return false + } + _, ok := fields["env"] + return ok +} + +// Use scoped values when the service declares env. +// Legacy services use the project and process environments. +func connectionEnvironmentMapping( + env map[string]string, + serviceEnvironment map[string]string, + declared bool, +) func(string) string { + if declared { + return func(name string) string { + return serviceEnvironment[name] + } + } + + return func(name string) string { + if value, found := env[name]; found { + return value + } + value, _ := os.LookupEnv(name) + return value + } +} + // maybeExpand expands ${VAR} references in s when resolve is true, preserving // Foundry ${{...}} expressions; when resolve is false it returns s unchanged so // the eject path keeps references verbatim. -func maybeExpand(s string, env map[string]string, resolve bool) (string, error) { +func maybeExpand( + s string, + mapping func(string) string, + resolve bool, +) (string, error) { if !resolve || s == "" { return s, nil } - return foundry.ExpandEnv(s, func(name string) string { - if v, ok := env[name]; ok { - return v - } - v, _ := os.LookupEnv(name) - return v - }) + return foundry.ExpandEnv(s, mapping) } // expandCredentials deep-copies a credentials map, expanding ${VAR} in every @@ -713,7 +770,7 @@ func maybeExpand(s string, env map[string]string, resolve bool) (string, error) // credentials entirely (e.g. None / identity auth). func expandCredentials( creds map[string]any, - env map[string]string, + mapping func(string) string, resolve bool, ) (map[string]any, error) { if creds == nil { @@ -721,7 +778,7 @@ func expandCredentials( } out := make(map[string]any, len(creds)) for k, v := range creds { - expanded, err := expandValue(v, env, resolve) + expanded, err := expandValue(v, mapping, resolve) if err != nil { return nil, err } @@ -732,14 +789,18 @@ func expandCredentials( // expandValue recursively expands ${VAR} in string values, map values, and // slice elements, leaving other types untouched. -func expandValue(v any, env map[string]string, resolve bool) (any, error) { +func expandValue( + v any, + mapping func(string) string, + resolve bool, +) (any, error) { switch val := v.(type) { case string: - return maybeExpand(val, env, resolve) + return maybeExpand(val, mapping, resolve) case map[string]any: out := make(map[string]any, len(val)) for k, inner := range val { - expanded, err := expandValue(inner, env, resolve) + expanded, err := expandValue(inner, mapping, resolve) if err != nil { return nil, err } @@ -749,7 +810,7 @@ func expandValue(v any, env map[string]string, resolve bool) (any, error) { case []any: out := make([]any, len(val)) for i, inner := range val { - expanded, err := expandValue(inner, env, resolve) + expanded, err := expandValue(inner, mapping, resolve) if err != nil { return nil, err } @@ -765,7 +826,7 @@ func expandValue(v any, env map[string]string, resolve bool) (any, error) { // A nil map returns nil so the connection omits metadata entirely. func expandMetadata( metadata map[string]string, - env map[string]string, + mapping func(string) string, resolve bool, ) (map[string]string, error) { if metadata == nil { @@ -773,7 +834,7 @@ func expandMetadata( } out := make(map[string]string, len(metadata)) for k, v := range metadata { - expanded, err := maybeExpand(v, env, resolve) + expanded, err := maybeExpand(v, mapping, resolve) if err != nil { return nil, err } diff --git a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go index d505c04da2e..72ae2c39b1d 100644 --- a/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go +++ b/cli/azd/extensions/azure.ai.agents/internal/synthesis/synthesizer_test.go @@ -743,6 +743,7 @@ services: conns, err := BrownfieldConnections( []byte(yaml), map[string]string{"SEARCH_API_KEY": "secret"}, + nil, "", ) require.NoError(t, err) @@ -760,13 +761,18 @@ services: host: azure.ai.project endpoint: https://existing.services.ai.azure.com/api/projects/p1 ` - conns, err := BrownfieldConnections([]byte(noConns), nil, "") + conns, err := BrownfieldConnections( + []byte(noConns), + nil, + nil, + "", + ) require.NoError(t, err) assert.Empty(t, conns) }) t.Run("empty raw errors", func(t *testing.T) { - _, err := BrownfieldConnections(nil, nil, "") + _, err := BrownfieldConnections(nil, nil, nil, "") require.Error(t, err) }) @@ -796,6 +802,7 @@ credentials: connections, err := BrownfieldConnections( raw, map[string]string{"SEARCH_KEY": "secret"}, + nil, root, ) diff --git a/cli/azd/extensions/azure.ai.agents/schemas/azure.ai.agent.json b/cli/azd/extensions/azure.ai.agents/schemas/azure.ai.agent.json index 1e362c72ed7..9376b3a90ba 100644 --- a/cli/azd/extensions/azure.ai.agents/schemas/azure.ai.agent.json +++ b/cli/azd/extensions/azure.ai.agents/schemas/azure.ai.agent.json @@ -4,13 +4,6 @@ "description": "Custom configuration for the Azure AI Agent Service target", "type": "object", "properties": { - "env": { - "type": "object", - "description": "Environment variables as key-value pairs", - "additionalProperties": { - "type": "string" - } - }, "container": { "$ref": "#/definitions/ContainerSettings" }, diff --git a/cli/azd/extensions/azure.ai.agents/schemas/examples/complex.azure.yaml b/cli/azd/extensions/azure.ai.agents/schemas/examples/complex.azure.yaml index 1f299dd05bb..b4165873bb3 100644 --- a/cli/azd/extensions/azure.ai.agents/schemas/examples/complex.azure.yaml +++ b/cli/azd/extensions/azure.ai.agents/schemas/examples/complex.azure.yaml @@ -46,6 +46,8 @@ services: host: azure.ai.connection uses: - ai-project + env: + SEARCH_API_KEY: ${SEARCH_API_KEY} category: CognitiveSearch target: https://my-search.search.windows.net authType: ApiKey @@ -108,7 +110,8 @@ services: - research-tools env: LOG_LEVEL: info - MODEL_ENDPOINT: ${{project.endpoint}} + # The extra $ preserves this Foundry expression through azd. + MODEL_ENDPOINT: $${{project.endpoint}} protocols: - protocol: a2a version: "0.2" @@ -143,6 +146,8 @@ services: host: azure.ai.routine uses: - researcher + env: + DIGEST_TOPIC: ${DIGEST_TOPIC} description: Summarize the day's documents every night. triggers: default: diff --git a/cli/azd/extensions/azure.ai.agents/schemas/examples/simple.azure.yaml b/cli/azd/extensions/azure.ai.agents/schemas/examples/simple.azure.yaml index 38197ca2346..6c51d4ab962 100644 --- a/cli/azd/extensions/azure.ai.agents/schemas/examples/simple.azure.yaml +++ b/cli/azd/extensions/azure.ai.agents/schemas/examples/simple.azure.yaml @@ -25,3 +25,5 @@ services: kind: hosted name: assistant description: A simple assistant. + env: + LOG_LEVEL: info diff --git a/cli/azd/extensions/azure.ai.connections/extension.yaml b/cli/azd/extensions/azure.ai.connections/extension.yaml index da3acfbc434..0c0dd40451c 100644 --- a/cli/azd/extensions/azure.ai.connections/extension.yaml +++ b/cli/azd/extensions/azure.ai.connections/extension.yaml @@ -17,4 +17,4 @@ tags: - connection usage: azd ai connection [options] version: 1.0.0-beta.3 -requiredAzdVersion: ">=1.27.0" +requiredAzdVersion: ">=1.27.1" diff --git a/cli/azd/extensions/azure.ai.connections/schemas/azure.ai.connection.json b/cli/azd/extensions/azure.ai.connections/schemas/azure.ai.connection.json index f51ea03067e..831ee68087f 100644 --- a/cli/azd/extensions/azure.ai.connections/schemas/azure.ai.connection.json +++ b/cli/azd/extensions/azure.ai.connections/schemas/azure.ai.connection.json @@ -12,7 +12,7 @@ }, "target": { "type": "string", - "description": "Target endpoint URL or ARM resource ID. May contain ${VAR} (azd env, resolved client-side)." + "description": "Target endpoint URL or ARM resource ID. May contain ${VAR}; declare each referenced variable in the service-level env object." }, "authType": { "type": "string", @@ -38,7 +38,7 @@ }, "credentials": { "type": "object", - "description": "Credentials. Values may contain ${VAR} (azd env, resolved client-side) or ${{...}} (Foundry server-side resolution, passed through untouched).", + "description": "Credentials. Use ${VAR} for azd service env substitution or ${{...}} for Foundry server-side resolution. In a service env value, use $${{...}} so azd forwards the Foundry expression unchanged.", "additionalProperties": true }, "metadata": { diff --git a/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider.go b/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider.go index df2981e15aa..65e9ffd8249 100644 --- a/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider.go +++ b/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider.go @@ -10,6 +10,7 @@ import ( "fmt" "hash/fnv" "log" + "maps" "net/url" "os" "path/filepath" @@ -65,19 +66,20 @@ type FoundryProvisioningProvider struct { azdClient *azdext.AzdClient // Populated by Initialize. - projectPath string - synthResult *synthesis.Result // nil when onDiskSource != nil - envName string - subID string - location string - rgName string - rgExplicit bool // AZURE_RESOURCE_GROUP came from env, not the rg- default - foundryName string - principalID string - credential azcore.TokenCredential - tenantID string // resolved lazily by ensureCredential; surfaced as AZURE_TENANT_ID - armTemplate map[string]any // embedded ARM JSON; nil when onDiskSource is set - onDiskSource *templateSource // non-nil when ./infra/main.{bicep,bicepparam} exists + projectPath string + synthResult *synthesis.Result // nil when onDiskSource != nil + serviceEnvironments map[string]map[string]string + envName string + subID string + location string + rgName string + rgExplicit bool // AZURE_RESOURCE_GROUP came from env, not the rg- default + foundryName string + principalID string + credential azcore.TokenCredential + tenantID string // resolved lazily by ensureCredential; surfaced as AZURE_TENANT_ID + armTemplate map[string]any // embedded ARM JSON; nil when onDiskSource is set + onDiskSource *templateSource // non-nil when ./infra/main.{bicep,bicepparam} exists // brownfieldEndpoint is the existing project endpoint when the foundry // service sets endpoint: (bring-your-own). When non-empty the provider skips @@ -124,12 +126,11 @@ func NewFoundryProvisioningProvider(azdClient *azdext.AzdClient) azdext.Provisio // and the on-disk Bicep path, and resolves required env values. It rejects // brownfield (endpoint:) and missing services with structured errors. // -// Initialize is cheap by contract: it does no network I/O and builds no -// credential. Tenant lookup and credential construction happen lazily in -// [FoundryProvisioningProvider.ensureCredential]; the bicep CLI is built -// only when an on-disk template actually needs compiling. azd-core may -// call Initialize on providers it never deploys with, so keeping it cheap -// lets pure metadata calls (Parameters, PlannedOutputs) succeed without auth. +// Initialize is cheap and performs no Azure network I/O. +// Credentials are created lazily by ensureCredential. +// The bicep CLI is built only when an on-disk template needs it. +// azd-core may initialize providers it never deploys with, so this +// keeps metadata calls unauthenticated. func (p *FoundryProvisioningProvider) Initialize( ctx context.Context, projectPath string, @@ -166,6 +167,11 @@ func (p *FoundryProvisioningProvider) Initialize( return err } + p.serviceEnvironments, err = p.projectServiceEnvironments(ctx) + if err != nil { + return err + } + // Detect on-disk Bicep before synthesizing. Stat-only; no compile here. if p.onDiskTemplatePresent() { log.Printf("[debug] foundry provider: on-disk Bicep detected under %s; "+ @@ -209,11 +215,12 @@ func (p *FoundryProvisioningProvider) Initialize( } res, err := synthesis.Synthesize(synthesis.Input{ - RawAzureYAML: rawYAML, - ServiceName: svcName, - AcceptedHosts: FoundryProvisioningServiceHosts, - Env: p.networkEnvMap(ctx), - ProjectRoot: projectPath, + RawAzureYAML: rawYAML, + ServiceName: svcName, + AcceptedHosts: FoundryProvisioningServiceHosts, + Env: p.networkEnvMap(ctx), + ServiceEnvironments: p.serviceEnvironments, + ProjectRoot: projectPath, }) switch { case errors.Is(err, synthesis.ErrEndpointBrownfield): @@ -293,6 +300,7 @@ func (p *FoundryProvisioningProvider) networkEnvMap(ctx context.Context) map[str log.Printf("[debug] foundry provider: no azd client; network ${VAR} uses process env only") return nil } + envClient := p.azdClient.Environment() if envClient == nil { log.Printf("[debug] foundry provider: no environment client; network ${VAR} uses process env only") @@ -318,6 +326,46 @@ func (p *FoundryProvisioningProvider) networkEnvMap(ctx context.Context) map[str return out } +// projectServiceEnvironments reads core-expanded service values. +// It keeps service scopes separate for connection synthesis. +func (p *FoundryProvisioningProvider) projectServiceEnvironments( + ctx context.Context, +) (map[string]map[string]string, error) { + if p.azdClient == nil { + return nil, exterrors.Dependency( + exterrors.CodeAzdClientFailed, + "read project service environments: azd client is unavailable", + "restart azd and retry", + ) + } + + response, err := p.azdClient.Project().Get( + ctx, + &azdext.EmptyRequest{}, + ) + if err != nil { + return nil, exterrors.Dependency( + exterrors.CodeAzdClientFailed, + fmt.Sprintf("read project service environments: %s", err), + "verify the azd project is accessible, then retry", + ) + } + if response.GetProject() == nil { + return nil, exterrors.Internal( + exterrors.CodeInvalidServiceConfig, + "read project service environments: project is missing", + ) + } + + environments := map[string]map[string]string{} + for name, service := range response.GetProject().GetServices() { + if len(service.GetEnvironment()) > 0 { + environments[name] = maps.Clone(service.GetEnvironment()) + } + } + return environments, nil +} + // warnNetworkIgnoredInBrownfield logs a warning when a service declares both // endpoint: (brownfield) and network:. The account's network posture is fixed // by whoever created it, so the network: block has no effect. @@ -835,6 +883,7 @@ func (p *FoundryProvisioningProvider) captureBrownfieldDeployments( connections, err := synthesis.BrownfieldConnections( rawYAML, p.networkEnvMap(ctx), + p.serviceEnvironments, p.projectPath, ) if err != nil { diff --git a/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider_test.go b/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider_test.go index 059f61b9dc7..deff0671de1 100644 --- a/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider_test.go +++ b/cli/azd/extensions/azure.ai.projects/internal/provisioning/foundry_provisioning_provider_test.go @@ -183,6 +183,101 @@ func TestFoundryProvider_ImplementsContract(t *testing.T) { assert.NotNil(t, p) } +func TestProjectServiceEnvironments(t *testing.T) { + t.Parallel() + + projectServer := &validateStubProjectServer{ + project: &azdext.ProjectConfig{ + Services: map[string]*azdext.ServiceConfig{ + "connection": { + Environment: map[string]string{ + "ENDPOINT": "https://service.example", + }, + }, + "legacy": {}, + }, + }, + } + client := newValidateTestClient( + t, + projectServer, + &validateStubEnvServer{}, + ) + provider := &FoundryProvisioningProvider{azdClient: client} + + environments, err := provider.projectServiceEnvironments(t.Context()) + require.NoError(t, err) + require.Equal( + t, + map[string]map[string]string{ + "connection": { + "ENDPOINT": "https://service.example", + }, + }, + environments, + ) +} + +func TestInitializeUsesConnectionServiceEnvironment(t *testing.T) { + t.Parallel() + + projectPath := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(projectPath, "azure.yaml"), + []byte(` +services: + project: + host: azure.ai.project + connection: + host: azure.ai.connection + uses: [project] + env: + ENDPOINT: ${SEARCH_ENDPOINT} + category: CognitiveSearch + target: ${ENDPOINT} + authType: None +`), + 0o600, + )) + + projectServer := &validateStubProjectServer{ + project: &azdext.ProjectConfig{ + Path: projectPath, + Services: map[string]*azdext.ServiceConfig{ + "connection": { + Environment: map[string]string{ + "ENDPOINT": "https://service.example", + }, + }, + }, + }, + } + client := newValidateTestClient( + t, + projectServer, + &validateStubEnvServer{ + envName: "test", + get: map[string]string{ + envKeySubscriptionID: "00000000-0000-0000-0000-000000000000", + envKeyLocation: "eastus", + }, + }, + ) + provider := &FoundryProvisioningProvider{azdClient: client} + + err := provider.Initialize( + t.Context(), + projectPath, + &azdext.ProvisioningOptions{Provider: FoundryProviderName}, + ) + require.NoError(t, err) + require.NotNil(t, provider.synthResult) + connections, ok := provider.synthResult.Parameters["connections"].([]synthesis.Connection) + require.True(t, ok) + require.Len(t, connections, 1) + require.Equal(t, "https://service.example", connections[0].Target) +} + func TestArmOutputsToProto(t *testing.T) { tests := []struct { name string diff --git a/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer.go b/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer.go index 8c4e7d7baea..41781ecd0b5 100644 --- a/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer.go +++ b/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer.go @@ -52,12 +52,16 @@ type Input struct { // value is not checked (only existence and endpoint: are). AcceptedHosts []string - // Env maps azd environment variable names to values. Used to resolve - // ${VAR} references in network fields (subnet vnet ids, dns.subscription). - // When a referenced variable is absent here, the synthesizer falls back - // to the process environment before failing. May be nil. + // Env maps project-wide azd values. + // Network fields always use it. + // Legacy connection services use it when service env is absent. + // Missing values may fall back to the process environment. Env map[string]string + // ServiceEnvironments contains core-expanded values by service. + // Connection fields prefer these values over the legacy Env map. + ServiceEnvironments map[string]map[string]string + // PreserveVarRefs keeps ${VAR} references verbatim instead of resolving // them. Used by the eject path, where the synthesized main.parameters.json // must stay environment-portable: the on-disk provision flow resolves @@ -291,6 +295,7 @@ func Synthesize(in Input) (*Result, error) { connections, err := collectConnections( root.Services, in.Env, + in.ServiceEnvironments, !in.PreserveVarRefs, in.ProjectRoot, ) @@ -362,6 +367,7 @@ func BrownfieldDeployments( func BrownfieldConnections( raw []byte, env map[string]string, + serviceEnvironments map[string]map[string]string, projectRoot string, ) ([]Connection, error) { if len(raw) == 0 { @@ -373,7 +379,13 @@ func BrownfieldConnections( return nil, fmt.Errorf("parse azure.yaml: %w", err) } - return collectConnections(root.Services, env, true, projectRoot) + return collectConnections( + root.Services, + env, + serviceEnvironments, + true, + projectRoot, + ) } // ProjectEndpoint returns the endpoint configured on a Foundry project service. @@ -629,12 +641,13 @@ func agentNeedsAcr(a agentBlock) bool { // (the service key is the connection name) and returns them sorted by name so // the synthesized parameter is deterministic regardless of YAML map order. // -// ${VAR} in target/credentials/metadata is expanded from env when resolve is -// true (provision path) and kept verbatim when false (eject path); Foundry -// ${{...}} expressions are always preserved, mirroring synthesizeNetwork. +// Provisioning resolves ${VAR} from service env when present. +// Legacy services use project and process values. +// Eject keeps references, and Foundry ${{...}} remains unchanged. func collectConnections( services map[string]yaml.Node, env map[string]string, + serviceEnvironments map[string]map[string]string, resolve bool, projectRoot string, ) ([]Connection, error) { @@ -660,17 +673,27 @@ func collectConnections( return nil, fmt.Errorf("services.%s: decode connection: %w", name, err) } - target, err := maybeExpand(svc.Target, env, resolve) + declared := len(serviceEnvironments[name]) > 0 || connectionEnvDeclared(node) + mapping := connectionEnvironmentMapping( + env, + serviceEnvironments[name], + declared, + ) + target, err := maybeExpand(svc.Target, mapping, resolve) if err != nil { return nil, fmt.Errorf("services.%s.target: %w", name, err) } - credentials, err := expandCredentials(svc.Credentials, env, resolve) + credentials, err := expandCredentials( + svc.Credentials, + mapping, + resolve, + ) if err != nil { return nil, fmt.Errorf("services.%s.credentials: %w", name, err) } - metadata, err := expandMetadata(svc.Metadata, env, resolve) + metadata, err := expandMetadata(svc.Metadata, mapping, resolve) if err != nil { return nil, fmt.Errorf("services.%s.metadata: %w", name, err) } @@ -691,20 +714,54 @@ func collectConnections( return connections, nil } +// connectionEnvDeclared reports whether the service node +// declares an env: key, including an empty env: {}. Core +// collapses an empty env to an omitted one, so the raw node +// is the only signal that a service opted into an isolated +// (possibly empty) scope. +func connectionEnvDeclared(node yaml.Node) bool { + var fields map[string]yaml.Node + if err := node.Decode(&fields); err != nil { + return false + } + _, ok := fields["env"] + return ok +} + +// Use scoped values when the service declares env. +// Legacy services use the project and process environments. +func connectionEnvironmentMapping( + env map[string]string, + serviceEnvironment map[string]string, + declared bool, +) func(string) string { + if declared { + return func(name string) string { + return serviceEnvironment[name] + } + } + + return func(name string) string { + if value, found := env[name]; found { + return value + } + value, _ := os.LookupEnv(name) + return value + } +} + // maybeExpand expands ${VAR} references in s when resolve is true, preserving // Foundry ${{...}} expressions; when resolve is false it returns s unchanged so // the eject path keeps references verbatim. -func maybeExpand(s string, env map[string]string, resolve bool) (string, error) { +func maybeExpand( + s string, + mapping func(string) string, + resolve bool, +) (string, error) { if !resolve || s == "" { return s, nil } - return foundry.ExpandEnv(s, func(name string) string { - if v, ok := env[name]; ok { - return v - } - v, _ := os.LookupEnv(name) - return v - }) + return foundry.ExpandEnv(s, mapping) } // expandCredentials deep-copies a credentials map, expanding ${VAR} in every @@ -713,7 +770,7 @@ func maybeExpand(s string, env map[string]string, resolve bool) (string, error) // credentials entirely (e.g. None / identity auth). func expandCredentials( creds map[string]any, - env map[string]string, + mapping func(string) string, resolve bool, ) (map[string]any, error) { if creds == nil { @@ -721,7 +778,7 @@ func expandCredentials( } out := make(map[string]any, len(creds)) for k, v := range creds { - expanded, err := expandValue(v, env, resolve) + expanded, err := expandValue(v, mapping, resolve) if err != nil { return nil, err } @@ -732,14 +789,18 @@ func expandCredentials( // expandValue recursively expands ${VAR} in string values, map values, and // slice elements, leaving other types untouched. -func expandValue(v any, env map[string]string, resolve bool) (any, error) { +func expandValue( + v any, + mapping func(string) string, + resolve bool, +) (any, error) { switch val := v.(type) { case string: - return maybeExpand(val, env, resolve) + return maybeExpand(val, mapping, resolve) case map[string]any: out := make(map[string]any, len(val)) for k, inner := range val { - expanded, err := expandValue(inner, env, resolve) + expanded, err := expandValue(inner, mapping, resolve) if err != nil { return nil, err } @@ -749,7 +810,7 @@ func expandValue(v any, env map[string]string, resolve bool) (any, error) { case []any: out := make([]any, len(val)) for i, inner := range val { - expanded, err := expandValue(inner, env, resolve) + expanded, err := expandValue(inner, mapping, resolve) if err != nil { return nil, err } @@ -765,7 +826,7 @@ func expandValue(v any, env map[string]string, resolve bool) (any, error) { // A nil map returns nil so the connection omits metadata entirely. func expandMetadata( metadata map[string]string, - env map[string]string, + mapping func(string) string, resolve bool, ) (map[string]string, error) { if metadata == nil { @@ -773,7 +834,7 @@ func expandMetadata( } out := make(map[string]string, len(metadata)) for k, v := range metadata { - expanded, err := maybeExpand(v, env, resolve) + expanded, err := maybeExpand(v, mapping, resolve) if err != nil { return nil, err } diff --git a/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer_test.go b/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer_test.go index 04f90f496f7..901697b8430 100644 --- a/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer_test.go +++ b/cli/azd/extensions/azure.ai.projects/internal/synthesis/synthesizer_test.go @@ -543,6 +543,14 @@ services: require.Len(t, conns, 1) return conns[0] } + getKeys := func(t *testing.T, c Connection) map[string]any { + t.Helper() + value, found := c.Credentials["keys"] + require.True(t, found, "credentials should contain keys") + keys, ok := value.(map[string]any) + require.True(t, ok, "keys should be a map, got %T", value) + return keys + } t.Run("provision path resolves ${VAR}", func(t *testing.T) { res, err := Synthesize(Input{ @@ -555,15 +563,105 @@ services: c := getConn(t, res) assert.Equal(t, "https://mcp.example.com/mcp", c.Target) - keys, ok := c.Credentials["keys"].(map[string]any) - require.True(t, ok, "keys should be a nested map, got %T", c.Credentials["keys"]) + keys := getKeys(t, c) assert.Equal(t, "secret-value", keys["x-api-key"]) assert.Equal(t, "team-ai", c.Metadata["owner"]) - publicConnections := res.Parameters["connections"].([]Connection) + publicValue, found := res.Parameters["connections"] + require.True(t, found) + publicConnections, ok := publicValue.([]Connection) + require.True(t, ok, "connections should be []Connection") + require.Len(t, publicConnections, 1) assert.Nil(t, publicConnections[0].Credentials) - secureCredentials := res.Parameters["connectionCredentials"].(map[string]map[string]any) - assert.Equal(t, "secret-value", secureCredentials["mcp-conn"]["keys"].(map[string]any)["x-api-key"]) + secureValue, found := res.Parameters["connectionCredentials"] + require.True(t, found) + secureCredentials, ok := secureValue.(map[string]map[string]any) + require.True(t, ok, "connectionCredentials should be a map") + connectionCredentials, found := secureCredentials["mcp-conn"] + require.True(t, found) + keyValue, found := connectionCredentials["keys"] + require.True(t, found) + secureKeys, ok := keyValue.(map[string]any) + require.True(t, ok, "secure keys should be a map") + assert.Equal(t, "secret-value", secureKeys["x-api-key"]) + }) + + t.Run("service env takes precedence and isolates lookup", func(t *testing.T) { + const serviceEnvYAML = ` +services: + my-project: + host: azure.ai.project + mcp-conn: + host: azure.ai.connection + uses: [my-project] + env: + ENDPOINT: ${MCP_URL} + KEY: ${MCP_KEY} + category: RemoteTool + target: ${ENDPOINT} + authType: CustomKeys + credentials: + keys: + x-api-key: ${KEY} + metadata: + owner: ${OWNER:-service-default} +` + res, err := Synthesize(Input{ + RawAzureYAML: []byte(serviceEnvYAML), + ServiceName: "my-project", + AcceptedHosts: []string{"azure.ai.project"}, + Env: map[string]string{ + "ENDPOINT": "https://wrong.example/mcp", + "KEY": "wrong-secret", + "OWNER": "wrong-owner", + }, + ServiceEnvironments: map[string]map[string]string{ + "mcp-conn": { + "ENDPOINT": "https://service.example/mcp", + "KEY": "service-secret", + }, + }, + }) + require.NoError(t, err) + + c := getConn(t, res) + assert.Equal(t, "https://service.example/mcp", c.Target) + keys := getKeys(t, c) + assert.Equal(t, "service-secret", keys["x-api-key"]) + assert.Equal(t, "service-default", c.Metadata["owner"]) + }) + + t.Run("explicit empty env isolates the connection", func(t *testing.T) { + const emptyEnvYAML = ` +services: + my-project: + host: azure.ai.project + mcp-conn: + host: azure.ai.connection + uses: [my-project] + env: {} + category: RemoteTool + target: ${MCP_URL} + authType: CustomKeys + credentials: + keys: + x-api-key: ${MCP_KEY} +` + res, err := Synthesize(Input{ + RawAzureYAML: []byte(emptyEnvYAML), + ServiceName: "my-project", + AcceptedHosts: []string{"azure.ai.project"}, + Env: map[string]string{ + "MCP_URL": "https://leak.example/mcp", + "MCP_KEY": "leaked-secret", + }, + }) + require.NoError(t, err) + + c := getConn(t, res) + assert.Equal(t, "", c.Target) + keys := getKeys(t, c) + assert.Equal(t, "", keys["x-api-key"]) }) t.Run("eject path preserves ${VAR} verbatim", func(t *testing.T) { @@ -578,8 +676,7 @@ services: c := getConn(t, res) assert.Equal(t, "${MCP_URL}", c.Target) - keys, ok := c.Credentials["keys"].(map[string]any) - require.True(t, ok) + keys := getKeys(t, c) assert.Equal(t, "${MCP_KEY}", keys["x-api-key"]) assert.Equal(t, "${MCP_OWNER}", c.Metadata["owner"]) }) @@ -608,7 +705,7 @@ services: require.NoError(t, err) c := getConn(t, res) - keys := c.Credentials["keys"].(map[string]any) + keys := getKeys(t, c) assert.Equal(t, "${{connections.other.credentials.key}}", keys["x-api-key"]) }) @@ -627,7 +724,7 @@ services: c := getConn(t, res) assert.Equal(t, "", c.Target) - keys := c.Credentials["keys"].(map[string]any) + keys := getKeys(t, c) assert.Equal(t, "", keys["x-api-key"]) }) } @@ -661,6 +758,7 @@ services: conns, err := BrownfieldConnections( []byte(yaml), map[string]string{"SEARCH_API_KEY": "secret"}, + nil, "", ) require.NoError(t, err) @@ -671,6 +769,20 @@ services: assert.Equal(t, "secret", conns[1].Credentials["key"]) }) + t.Run("service environment takes precedence", func(t *testing.T) { + conns, err := BrownfieldConnections( + []byte(yaml), + map[string]string{"SEARCH_API_KEY": "global"}, + map[string]map[string]string{ + "search-conn": {"SEARCH_API_KEY": "service"}, + }, + "", + ) + require.NoError(t, err) + require.Len(t, conns, 2) + assert.Equal(t, "service", conns[1].Credentials["key"]) + }) + t.Run("no connection services yields empty slice", func(t *testing.T) { const noConns = ` services: @@ -678,13 +790,18 @@ services: host: azure.ai.project endpoint: https://existing.services.ai.azure.com/api/projects/p1 ` - conns, err := BrownfieldConnections([]byte(noConns), nil, "") + conns, err := BrownfieldConnections( + []byte(noConns), + nil, + nil, + "", + ) require.NoError(t, err) assert.Empty(t, conns) }) t.Run("empty raw errors", func(t *testing.T) { - _, err := BrownfieldConnections(nil, nil, "") + _, err := BrownfieldConnections(nil, nil, nil, "") require.Error(t, err) }) } @@ -972,7 +1089,12 @@ services: require.Len(t, deployments, 1) assert.Equal(t, "gpt-4o", deployments[0].Name) - connections, err := BrownfieldConnections([]byte(yaml), nil, root) + connections, err := BrownfieldConnections( + []byte(yaml), + nil, + nil, + root, + ) require.NoError(t, err) require.Len(t, connections, 1) assert.Equal(t, "CognitiveSearch", connections[0].Category) diff --git a/cli/azd/extensions/azure.ai.routines/extension.yaml b/cli/azd/extensions/azure.ai.routines/extension.yaml index b799078e347..81da4c40c59 100644 --- a/cli/azd/extensions/azure.ai.routines/extension.yaml +++ b/cli/azd/extensions/azure.ai.routines/extension.yaml @@ -17,4 +17,4 @@ tags: - routine usage: azd ai routine [options] version: 1.0.0-beta.3 -requiredAzdVersion: ">=1.27.0" +requiredAzdVersion: ">=1.27.1" diff --git a/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target.go b/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target.go index 9203d8a3f34..52b2305f0ab 100644 --- a/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target.go +++ b/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target.go @@ -29,18 +29,21 @@ var _ azdext.ServiceTargetProvider = (*routineServiceTarget)(nil) // model (triggers, action, ...); the routine name is the service key. Package // and Publish are no-ops because a routine has no build artifact. type routineServiceTarget struct { - azdClient *azdext.AzdClient - serviceConfig *azdext.ServiceConfig + azdClient *azdext.AzdClient } // newRoutineServiceTarget creates the azure.ai.routine service-target provider. -func newRoutineServiceTarget(azdClient *azdext.AzdClient) azdext.ServiceTargetProvider { +func newRoutineServiceTarget( + azdClient *azdext.AzdClient, +) azdext.ServiceTargetProvider { return &routineServiceTarget{azdClient: azdClient} } -// Initialize stores the service configuration; no other setup is required. -func (p *routineServiceTarget) Initialize(ctx context.Context, serviceConfig *azdext.ServiceConfig) error { - p.serviceConfig = serviceConfig +// Initialize requires no setup. +func (p *routineServiceTarget) Initialize( + _ context.Context, + _ *azdext.ServiceConfig, +) error { return nil } @@ -111,14 +114,16 @@ func (p *routineServiceTarget) Deploy( // The service key is the routine identity; ignore any name in the body. body.Name = serviceConfig.GetName() - // Resolve ${VAR} references in the routine's action input against the azd - // environment, leaving Foundry server-side ${{...}} expressions untouched. + // Resolve ${VAR} against the service environment forwarded by azd. if body.Action != nil { - env, err := p.currentEnvValues(ctx) + environment, err := p.environmentValues(ctx, serviceConfig) if err != nil { return nil, err } - body.Action.Input = expandRoutineValue(body.Action.Input, env) + body.Action.Input = expandRoutineValue( + body.Action.Input, + environment, + ) } if progress != nil { @@ -185,16 +190,54 @@ func newRoutineServiceClient(ctx context.Context) (*routines.Client, error) { ), nil } -// currentEnvValues loads all key-value pairs from the active azd environment, used to -// resolve ${VAR} references in routine fields at deploy time. -func (p *routineServiceTarget) currentEnvValues(ctx context.Context) (map[string]string, error) { - current, err := p.azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{}) +var serviceEnvDeclared = func( + ctx context.Context, + azdClient *azdext.AzdClient, + serviceName string, +) (bool, error) { + resp, err := azdClient.Project().GetServiceConfigValue(ctx, &azdext.GetServiceConfigValueRequest{ + ServiceName: serviceName, + Path: "env", + }) + if err != nil { + return false, fmt.Errorf("reading env for service %q: %w", serviceName, err) + } + return resp.GetFound(), nil +} + +func (p *routineServiceTarget) environmentValues( + ctx context.Context, + serviceConfig *azdext.ServiceConfig, +) (map[string]string, error) { + environment := serviceConfig.GetEnvironment() + if len(environment) > 0 { + return environment, nil + } + // An explicit empty env: {} declares an isolated scope. + // Core forwards it as an empty map, indistinguishable from + // an omitted env, so consult the raw config before falling + // back to the full azd environment. + declared, err := serviceEnvDeclared(ctx, p.azdClient, serviceConfig.GetName()) + if err != nil { + return nil, err + } + if declared { + return environment, nil + } + + current, err := p.azdClient.Environment().GetCurrent( + ctx, + &azdext.EmptyRequest{}, + ) if err != nil { return nil, fmt.Errorf("resolving current azd environment: %w", err) } - resp, err := p.azdClient.Environment().GetValues(ctx, &azdext.GetEnvironmentRequest{ - Name: current.GetEnvironment().GetName(), - }) + resp, err := p.azdClient.Environment().GetValues( + ctx, + &azdext.GetEnvironmentRequest{ + Name: current.GetEnvironment().GetName(), + }, + ) if err != nil { return nil, fmt.Errorf("loading azd environment values: %w", err) } @@ -205,9 +248,7 @@ func (p *routineServiceTarget) currentEnvValues(ctx context.Context) (map[string return values, nil } -// expandRoutineValue recursively expands ${VAR} references in every string within a -// routine value (maps, slices, scalars) against the azd environment, preserving Foundry -// server-side ${{...}} expressions. +// expandRoutineValue expands ${VAR} in nested routine values. func expandRoutineValue(value any, env map[string]string) any { switch typed := value.(type) { case string: diff --git a/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target_test.go b/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target_test.go index 4df169c2cd9..b4ca8135baf 100644 --- a/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target_test.go +++ b/cli/azd/extensions/azure.ai.routines/internal/cmd/service_target_test.go @@ -4,6 +4,7 @@ package cmd import ( + "context" "testing" "github.com/azure/azure-dev/cli/azd/pkg/azdext" @@ -57,3 +58,40 @@ func TestParseRoutineServiceConfig_ConfigFallback(t *testing.T) { require.NoError(t, err) assert.Equal(t, "legacy", body.Description) } + +func TestExpandRoutineValue(t *testing.T) { + t.Parallel() + + serviceConfig := &azdext.ServiceConfig{ + Environment: map[string]string{"DIGEST_TOPIC": "weekly changes"}, + } + environment, err := (&routineServiceTarget{}).environmentValues( + t.Context(), + serviceConfig, + ) + require.NoError(t, err) + input := map[string]any{ + "topic": "${DIGEST_TOPIC}", + "secret": "${{connections.search.credentials.key}}", + } + + assert.Equal(t, map[string]any{ + "topic": "weekly changes", + "secret": "${{connections.search.credentials.key}}", + }, expandRoutineValue(input, environment)) +} + +func TestRoutineEnvironmentValuesEmptyDeclaredIsolates(t *testing.T) { + orig := serviceEnvDeclared + t.Cleanup(func() { serviceEnvDeclared = orig }) + serviceEnvDeclared = func(context.Context, *azdext.AzdClient, string) (bool, error) { + return true, nil + } + + env, err := (&routineServiceTarget{}).environmentValues( + t.Context(), + &azdext.ServiceConfig{Name: "nightly-digest"}, + ) + require.NoError(t, err) + require.Empty(t, env) +} diff --git a/cli/azd/extensions/azure.ai.routines/schemas/azure.ai.routine.json b/cli/azd/extensions/azure.ai.routines/schemas/azure.ai.routine.json index 40058c5dfaa..d4f72ec59c7 100644 --- a/cli/azd/extensions/azure.ai.routines/schemas/azure.ai.routine.json +++ b/cli/azd/extensions/azure.ai.routines/schemas/azure.ai.routine.json @@ -36,7 +36,7 @@ "properties": { "type": { "type": "string", "description": "Action variant (e.g. invoke_agent_responses_api, invoke_agent_invocations_api)." }, "agent_name": { "type": "string", "description": "Name of the azure.ai.agent service the routine invokes." }, - "input": { "description": "Static JSON input sent to the agent when the routine fires. Values may use ${VAR} or ${{...}}." } + "input": { "description": "Static JSON input sent to the agent when the routine fires. Values may use ${VAR} declared in the service-level env object or ${{...}} for Foundry server-side resolution." } } } } diff --git a/cli/azd/extensions/azure.ai.toolboxes/README.md b/cli/azd/extensions/azure.ai.toolboxes/README.md index 406294ec0ee..dd3acc39ab0 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/README.md +++ b/cli/azd/extensions/azure.ai.toolboxes/README.md @@ -16,10 +16,13 @@ services: research-tools: host: azure.ai.toolbox endpoint: ${RESEARCH_TOOLBOX_ENDPOINT} + env: + RESEARCH_TOOLBOX_ENDPOINT: ${RESEARCH_TOOLBOX_ENDPOINT} ``` Get the endpoint value from `azd ai toolbox show ` (the `Endpoint:` line). -The value may contain `${VAR}` references, which resolve against the azd -environment. Because a toolbox version is immutable, `endpoint` cannot be +The value may contain `${VAR}` references. Declare each referenced variable +in the service-level `env` object; azd falls back to the active environment +only when the service declares no `env`. Because a toolbox version is immutable, `endpoint` cannot be combined with `tools` or `description`. diff --git a/cli/azd/extensions/azure.ai.toolboxes/extension.yaml b/cli/azd/extensions/azure.ai.toolboxes/extension.yaml index 6086e624d89..a14ef500864 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/extension.yaml +++ b/cli/azd/extensions/azure.ai.toolboxes/extension.yaml @@ -17,4 +17,4 @@ tags: - toolbox usage: azd ai toolbox [options] version: 1.0.0-beta.4 -requiredAzdVersion: ">=1.27.0" +requiredAzdVersion: ">=1.27.1" diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target.go index d97321acaeb..23eef91ef86 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target.go @@ -46,19 +46,25 @@ type toolboxServiceConfig struct { // name is the service key. Package and Publish are no-ops because a toolbox has no build // artifact. type toolboxServiceTarget struct { - azdClient *azdext.AzdClient - serviceConfig *azdext.ServiceConfig - resolver connectionResolver + azdClient *azdext.AzdClient + resolver connectionResolver } // newToolboxServiceTarget creates the azure.ai.toolbox service-target provider. -func newToolboxServiceTarget(azdClient *azdext.AzdClient) azdext.ServiceTargetProvider { - return &toolboxServiceTarget{azdClient: azdClient, resolver: defaultConnectionResolver{}} +func newToolboxServiceTarget( + azdClient *azdext.AzdClient, +) azdext.ServiceTargetProvider { + return &toolboxServiceTarget{ + azdClient: azdClient, + resolver: defaultConnectionResolver{}, + } } -// Initialize stores the service configuration; no other setup is required. -func (p *toolboxServiceTarget) Initialize(ctx context.Context, serviceConfig *azdext.ServiceConfig) error { - p.serviceConfig = serviceConfig +// Initialize requires no setup. +func (p *toolboxServiceTarget) Initialize( + _ context.Context, + _ *azdext.ServiceConfig, +) error { return nil } @@ -113,8 +119,8 @@ func (p *toolboxServiceTarget) Publish( // Deploy upserts the toolbox by creating a new version from the entry's tools. Tool // entries that name a `connection` are resolved to their project_connection_id (the -// `uses:` edge guarantees the connection is reconciled first). ${VAR} references resolve -// against the azd environment; Foundry ${{...}} expressions pass through untouched. +// `uses:` edge guarantees the connection is reconciled first). ${VAR} +// references resolve from the forwarded service environment. // Removing the service from azure.yaml stops azd managing the toolbox but does not delete // it (use `azd ai toolbox delete`). // When the entry sets `endpoint` instead, azd reuses that existing @@ -135,7 +141,7 @@ func (p *toolboxServiceTarget) Deploy( // Reuse (bring-your-own): endpoint set means azd resolves ${VAR} // and publishes it for agents instead of creating a version. if strings.TrimSpace(cfg.Endpoint) != "" { - return p.deployReuse(ctx, name, cfg, progress) + return p.deployReuse(ctx, name, cfg, serviceConfig, progress) } resolved, err := projectctx.Resolve(ctx, projectctx.ResolveOpts{}) @@ -144,12 +150,16 @@ func (p *toolboxServiceTarget) Deploy( } endpoint := resolved.Endpoint - env, err := p.currentEnvValues(ctx) + environment, err := p.environmentValues(ctx, serviceConfig) if err != nil { return nil, err } - - tools, err := p.buildToolEntries(ctx, endpoint, cfg.Tools, env) + tools, err := p.buildToolEntries( + ctx, + endpoint, + cfg.Tools, + environment, + ) if err != nil { return nil, err } @@ -189,9 +199,10 @@ func (p *toolboxServiceTarget) deployReuse( ctx context.Context, name string, cfg *toolboxServiceConfig, + serviceConfig *azdext.ServiceConfig, progress azdext.ProgressReporter, ) (*azdext.ServiceDeployResult, error) { - env, err := p.currentEnvValues(ctx) + env, err := p.environmentValues(ctx, serviceConfig) if err != nil { return nil, err } @@ -323,16 +334,54 @@ func parseToolboxServiceConfig(svc *azdext.ServiceConfig) (*toolboxServiceConfig return cfg, nil } -// currentEnvValues loads all key-value pairs from the active azd environment, used to -// resolve ${VAR} references in tool fields at deploy time. -func (p *toolboxServiceTarget) currentEnvValues(ctx context.Context) (map[string]string, error) { - current, err := p.azdClient.Environment().GetCurrent(ctx, &azdext.EmptyRequest{}) +var serviceEnvDeclared = func( + ctx context.Context, + azdClient *azdext.AzdClient, + serviceName string, +) (bool, error) { + resp, err := azdClient.Project().GetServiceConfigValue(ctx, &azdext.GetServiceConfigValueRequest{ + ServiceName: serviceName, + Path: "env", + }) + if err != nil { + return false, fmt.Errorf("reading env for service %q: %w", serviceName, err) + } + return resp.GetFound(), nil +} + +func (p *toolboxServiceTarget) environmentValues( + ctx context.Context, + serviceConfig *azdext.ServiceConfig, +) (map[string]string, error) { + environment := serviceConfig.GetEnvironment() + if len(environment) > 0 { + return environment, nil + } + // An explicit empty env: {} declares an isolated scope. + // Core forwards it as an empty map, indistinguishable from + // an omitted env, so consult the raw config before falling + // back to the full azd environment. + declared, err := serviceEnvDeclared(ctx, p.azdClient, serviceConfig.GetName()) + if err != nil { + return nil, err + } + if declared { + return environment, nil + } + + current, err := p.azdClient.Environment().GetCurrent( + ctx, + &azdext.EmptyRequest{}, + ) if err != nil { return nil, fmt.Errorf("resolving current azd environment: %w", err) } - resp, err := p.azdClient.Environment().GetValues(ctx, &azdext.GetEnvironmentRequest{ - Name: current.GetEnvironment().GetName(), - }) + resp, err := p.azdClient.Environment().GetValues( + ctx, + &azdext.GetEnvironmentRequest{ + Name: current.GetEnvironment().GetName(), + }, + ) if err != nil { return nil, fmt.Errorf("loading azd environment values: %w", err) } @@ -343,9 +392,7 @@ func (p *toolboxServiceTarget) currentEnvValues(ctx context.Context) (map[string return values, nil } -// expandToolboxValue recursively expands ${VAR} references in every string within a tool -// value (maps, slices, scalars) against the azd environment, preserving Foundry -// server-side ${{...}} expressions. +// expandToolboxValue expands ${VAR} in nested toolbox values. func expandToolboxValue(value any, env map[string]string) any { switch typed := value.(type) { case string: diff --git a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target_test.go b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target_test.go index 8fece33c91a..b9e0678e68c 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target_test.go +++ b/cli/azd/extensions/azure.ai.toolboxes/internal/cmd/service_target_test.go @@ -145,6 +145,30 @@ func TestPublishReuseEndpoint_WritesExpandedEndpoint(t *testing.T) { assert.Equal(t, wantURL, (*calls)[0].value) } +func TestDeployReuseUsesServiceEnvironment(t *testing.T) { + // No t.Parallel: stubToolboxEndpointEnv swaps a package-level seam. + calls := stubToolboxEndpointEnv(t) + const wantURL = "https://mcp.example.com/toolboxes/research" + serviceConfig := &azdext.ServiceConfig{ + Environment: map[string]string{ + "RESEARCH_TOOLBOX_ENDPOINT": wantURL, + }, + } + + result, err := (&toolboxServiceTarget{}).deployReuse( + t.Context(), + "research", + &toolboxServiceConfig{Endpoint: "${RESEARCH_TOOLBOX_ENDPOINT}"}, + serviceConfig, + nil, + ) + + require.NoError(t, err) + require.NotNil(t, result) + require.Len(t, *calls, 1) + assert.Equal(t, wantURL, (*calls)[0].value) +} + func TestBuildToolEntries_ResolvesConnectionRef(t *testing.T) { t.Parallel() @@ -174,16 +198,43 @@ func TestBuildToolEntries_ResolvesConnectionRef(t *testing.T) { func TestExpandToolboxValue(t *testing.T) { t.Parallel() - env := map[string]string{"MCP_URL": "https://resolved.example.com"} + serviceConfig := &azdext.ServiceConfig{ + Environment: map[string]string{ + "MCP_URL": "https://resolved.example.com", + }, + } + environment, err := (&toolboxServiceTarget{}).environmentValues( + t.Context(), + serviceConfig, + ) + require.NoError(t, err) in := map[string]any{ "type": "mcp", "server_url": "${MCP_URL}", "headers": []any{"x-secret: ${{secrets.token}}"}, } - out, ok := expandToolboxValue(in, env).(map[string]any) + out, ok := expandToolboxValue( + in, + environment, + ).(map[string]any) require.True(t, ok) assert.Equal(t, "https://resolved.example.com", out["server_url"]) // Foundry ${{...}} passes through untouched. assert.Equal(t, []any{"x-secret: ${{secrets.token}}"}, out["headers"]) } + +func TestToolboxEnvironmentValuesEmptyDeclaredIsolates(t *testing.T) { + orig := serviceEnvDeclared + t.Cleanup(func() { serviceEnvDeclared = orig }) + serviceEnvDeclared = func(context.Context, *azdext.AzdClient, string) (bool, error) { + return true, nil + } + + env, err := (&toolboxServiceTarget{}).environmentValues( + t.Context(), + &azdext.ServiceConfig{Name: "research-tools"}, + ) + require.NoError(t, err) + require.Empty(t, env) +} diff --git a/cli/azd/extensions/azure.ai.toolboxes/schemas/azure.ai.toolbox.json b/cli/azd/extensions/azure.ai.toolboxes/schemas/azure.ai.toolbox.json index 54ae8ef04e3..08c300a8254 100644 --- a/cli/azd/extensions/azure.ai.toolboxes/schemas/azure.ai.toolbox.json +++ b/cli/azd/extensions/azure.ai.toolboxes/schemas/azure.ai.toolbox.json @@ -8,7 +8,7 @@ "properties": { "endpoint": { "type": "string", - "description": "MCP endpoint URL of an existing Foundry toolbox version, as shown by 'azd ai toolbox show' (e.g. https://my-account.services.ai.azure.com/api/projects/my-project/toolboxes/research/versions/3/mcp?api-version=v1). When set, azd reuses this existing toolbox and publishes the endpoint for agents instead of creating a new version, so 'tools' and 'description' must be omitted. May contain ${VAR} (azd env, resolved client-side)." + "description": "MCP endpoint URL of an existing Foundry toolbox version, as shown by 'azd ai toolbox show' (e.g. https://my-account.services.ai.azure.com/api/projects/my-project/toolboxes/research/versions/3/mcp?api-version=v1). When set, azd reuses this existing toolbox and publishes the endpoint for agents instead of creating a new version, so 'tools' and 'description' must be omitted. May contain ${VAR} declared in the service-level env object." }, "description": { "type": "string", @@ -16,7 +16,7 @@ }, "tools": { "type": "array", - "description": "List of tools in the toolbox.", + "description": "List of tools in the toolbox. Values may use ${VAR} declared in the service-level env object.", "items": { "type": "object", "required": ["type"],