diff --git a/src/agentex/lib/cli/handlers/deploy_handlers.py b/src/agentex/lib/cli/handlers/deploy_handlers.py index e1cd1965c..71b6394fc 100644 --- a/src/agentex/lib/cli/handlers/deploy_handlers.py +++ b/src/agentex/lib/cli/handlers/deploy_handlers.py @@ -26,6 +26,11 @@ TEMPORAL_WORKER_KEY = "temporal-worker" DEFAULT_HELM_CHART_VERSION = "0.1.9" +CHART_IDENTITY_ENV_PATHS = { + EnvVarKeys.AGENT_NAME.value: ("agent", "name"), + EnvVarKeys.WORKFLOW_NAME.value: ("workflow", "name"), + EnvVarKeys.WORKFLOW_TASK_QUEUE.value: ("workflow", "taskQueue"), +} class InputDeployOverrides(BaseModel): @@ -236,6 +241,69 @@ def convert_env_vars_dict_to_list(env_vars: dict[str, str]) -> list[dict[str, st return [{"name": key, "value": value} for key, value in env_vars.items()] +def _chart_identity_names(env_vars: Any) -> set[str]: + if not isinstance(env_vars, list): + return set() + return { + str(env_var["name"]) + for env_var in env_vars + if isinstance(env_var, dict) and env_var.get("name") in CHART_IDENTITY_ENV_PATHS + } + + +def _normalize_chart_identity_env( + helm_values: dict[str, Any], + env_vars: dict[str, str], + secret_env_vars: list[dict[str, str]], + environment_identity_env_names: set[str], + explicit_global_identity_names: set[str], +) -> None: + """Move legacy identity env values into the chart globals that render them.""" + global_values = helm_values.get("global") + if not isinstance(global_values, dict): + raise DeploymentError("helm_overrides.global must be a mapping") + + temporal_worker_values = helm_values.get(TEMPORAL_WORKER_KEY, {}) + if not isinstance(temporal_worker_values, dict): + raise DeploymentError(f"helm_overrides.{TEMPORAL_WORKER_KEY} must be a mapping") + + secret_identity_names = sorted( + _chart_identity_names(secret_env_vars) + | _chart_identity_names(helm_values.get("secretEnvVars")) + | _chart_identity_names(global_values.get("secretEnvVars")) + | _chart_identity_names(temporal_worker_values.get("secretEnvVars")) + ) + if secret_identity_names: + names = ", ".join(secret_identity_names) + raise DeploymentError( + f"Chart-owned identity variables cannot come from credentials: {names}. " + "Configure them under helm_overrides.global instead." + ) + + unsupported_plain_identity_names = sorted( + _chart_identity_names(global_values.get("env")) | _chart_identity_names(temporal_worker_values.get("env")) + ) + if unsupported_plain_identity_names: + names = ", ".join(unsupported_plain_identity_names) + raise DeploymentError( + f"Chart-owned identity variables cannot be set in global.env or {TEMPORAL_WORKER_KEY}.env: {names}. " + "Configure them under helm_overrides.global instead." + ) + + for env_name, (group_name, value_name) in CHART_IDENTITY_ENV_PATHS.items(): + group = global_values.get(group_name) + if group_name in global_values and not isinstance(group, dict): + raise DeploymentError(f"helm_overrides.global.{group_name} must be a mapping") + + if env_name in env_vars: + if group is None: + group = {} + global_values[group_name] = group + legacy_value = env_vars.pop(env_name) + if env_name in environment_identity_env_names or env_name not in explicit_global_identity_names: + group[value_name] = legacy_value + + def add_acp_command_to_helm_values(helm_values: dict[str, Any], manifest: AgentManifest, manifest_path: str) -> None: """Add dynamic ACP command to helm values based on manifest configuration""" try: @@ -324,6 +392,8 @@ def merge_deployment_configs( # Priority: manifest -> environments.yaml -> secrets (highest) all_env_vars: dict[str, str] = {} secret_env_vars: list[dict[str, str]] = [] + environment_identity_env_names: set[str] = set() + explicit_global_identity_names: set[str] = set() # Start with agent_config env vars from manifest if agent_config.env: @@ -339,6 +409,17 @@ def merge_deployment_configs( if isinstance(env_var, dict) and "name" in env_var and "value" in env_var: env_override_dict[str(env_var["name"])] = str(env_var["value"]) all_env_vars.update(env_override_dict) + environment_identity_env_names = set(env_override_dict) & CHART_IDENTITY_ENV_PATHS.keys() + + if agent_env_config and agent_env_config.helm_overrides: + global_overrides = agent_env_config.helm_overrides.get("global") + if isinstance(global_overrides, dict): + explicit_global_identity_names = { + env_name + for env_name, (group_name, value_name) in CHART_IDENTITY_ENV_PATHS.items() + if isinstance(global_overrides.get(group_name), dict) + and value_name in global_overrides[group_name] + } # Handle credentials and check for conflicts if agent_config.credentials: @@ -389,12 +470,21 @@ def merge_deployment_configs( _deep_merge(helm_values, agent_env_config.helm_overrides) logger.info(f"After-merge helm values: {helm_values}") + _normalize_chart_identity_env( + helm_values, + all_env_vars, + secret_env_vars, + environment_identity_env_names, + explicit_global_identity_names, + ) _stamp_agent_version(helm_values, set(all_env_vars) | {var["name"] for var in secret_env_vars}) # Set final environment variables # Environment variable precedence: manifest -> environments.yaml -> secrets (highest) if all_env_vars: helm_values["env"] = convert_env_vars_dict_to_list(all_env_vars) + else: + helm_values.pop("env", None) if secret_env_vars: helm_values["secretEnvVars"] = secret_env_vars diff --git a/tests/lib/cli/test_deploy_handlers.py b/tests/lib/cli/test_deploy_handlers.py index 835b56ae8..67235e08b 100644 --- a/tests/lib/cli/test_deploy_handlers.py +++ b/tests/lib/cli/test_deploy_handlers.py @@ -4,20 +4,43 @@ from typing import Any +import pytest + +from agentex.config.credentials import CredentialMapping from agentex.config.agent_config import AgentConfig from agentex.config.build_config import BuildConfig, BuildContext +from agentex.config.agent_configs import TemporalConfig, TemporalWorkflowConfig from agentex.config.agent_manifest import AgentManifest from agentex.config.deployment_config import ImageConfig, DeploymentConfig +from agentex.lib.cli.utils.exceptions import DeploymentError from agentex.config.environment_config import AgentAuthConfig, AgentEnvironmentConfig from agentex.lib.cli.handlers.deploy_handlers import InputDeployOverrides, merge_deployment_configs MANIFEST_TAG = "sha-manifest" -def _manifest(env: dict[str, str] | None = None) -> AgentManifest: +def _manifest( + env: dict[str, str] | None = None, + credentials: list[CredentialMapping] | None = None, + temporal: bool = False, +) -> AgentManifest: return AgentManifest( build=BuildConfig(context=BuildContext(root=".", dockerfile="Dockerfile", dockerignore=None)), - agent=AgentConfig(name="emu-tax", description="Files emu taxes", acp_type="async", env=env), + agent=AgentConfig( + name="emu-tax", + description="Files emu taxes", + acp_type="async", + env=env, + credentials=credentials, + temporal=( + TemporalConfig( + enabled=True, + workflows=[TemporalWorkflowConfig(name="tax-workflow", queue_name="tax-queue")], + ) + if temporal + else None + ), + ), deployment=DeploymentConfig(image=ImageConfig(repository="registry.example.com/emu-tax", tag=MANIFEST_TAG)), ) @@ -62,3 +85,133 @@ def test_skipped_when_the_environment_env_declares_agent_version(self): values = _merge(_manifest(), _env_config({"env": [{"name": "AGENT_VERSION", "value": "v9"}]})) assert "version" not in values["global"]["agent"] + + +class TestChartIdentityEnvironment: + def test_legacy_env_identity_is_moved_to_chart_globals(self): + manifest = _manifest( + env={ + "AGENT_NAME": "manifest-agent", + "WORKFLOW_NAME": "manifest-workflow", + "WORKFLOW_TASK_QUEUE": "manifest-queue", + "CUSTOM": "manifest", + }, + temporal=True, + ) + env_config = _env_config( + { + "global": { + "agent": {"name": "global-agent"}, + "workflow": {"name": "global-workflow", "taskQueue": "global-queue"}, + }, + "env": [ + {"name": "AGENT_NAME", "value": "staging-agent"}, + {"name": "WORKFLOW_NAME", "value": "staging-workflow"}, + {"name": "WORKFLOW_TASK_QUEUE", "value": "staging-queue"}, + {"name": "CUSTOM", "value": "staging"}, + ], + } + ) + + values = _merge(manifest, env_config) + + assert values["global"]["agent"]["name"] == "staging-agent" + assert values["global"]["workflow"] == { + "name": "staging-workflow", + "taskQueue": "staging-queue", + } + env_by_name = {item["name"]: item["value"] for item in values["env"]} + assert env_by_name["CUSTOM"] == "staging" + assert set(env_by_name).isdisjoint({"AGENT_NAME", "WORKFLOW_NAME", "WORKFLOW_TASK_QUEUE"}) + assert values["temporal-worker"]["env"] == values["env"] + + def test_environment_globals_override_legacy_manifest_identity(self): + values = _merge( + _manifest( + env={ + "AGENT_NAME": "manifest-agent", + "WORKFLOW_NAME": "manifest-workflow", + "WORKFLOW_TASK_QUEUE": "manifest-queue", + }, + temporal=True, + ), + _env_config( + { + "global": { + "agent": {"name": "environment-agent"}, + "workflow": {"name": "environment-workflow", "taskQueue": "environment-queue"}, + } + } + ), + ) + + assert values["global"]["agent"]["name"] == "environment-agent" + assert values["global"]["workflow"] == { + "name": "environment-workflow", + "taskQueue": "environment-queue", + } + identity_names = {"AGENT_NAME", "WORKFLOW_NAME", "WORKFLOW_TASK_QUEUE"} + assert identity_names.isdisjoint(item["name"] for item in values["env"]) + assert identity_names.isdisjoint(item["name"] for item in values["temporal-worker"]["env"]) + + def test_identity_credentials_are_rejected(self): + credential = CredentialMapping( + env_var_name="AGENT_NAME", + secret_name="identity", + secret_key="agent-name", + ) + + with pytest.raises(DeploymentError, match="AGENT_NAME"): + _merge(_manifest(credentials=[credential])) + + @pytest.mark.parametrize( + "helm_overrides", + [ + { + "secretEnvVars": [ + {"name": "AGENT_NAME", "secretName": "identity", "secretKey": "agent-name"} + ] + }, + { + "global": { + "secretEnvVars": [ + {"name": "WORKFLOW_NAME", "secretName": "identity", "secretKey": "workflow-name"} + ] + } + }, + { + "temporal-worker": { + "secretEnvVars": [ + { + "name": "WORKFLOW_TASK_QUEUE", + "secretName": "identity", + "secretKey": "task-queue", + } + ] + } + }, + ], + ) + def test_identity_secret_helm_overrides_are_rejected(self, helm_overrides: dict[str, Any]): + with pytest.raises(DeploymentError, match="Chart-owned identity variables"): + _merge(_manifest(temporal=True), _env_config(helm_overrides)) + + @pytest.mark.parametrize("path", ["global", "temporal-worker"]) + def test_identity_container_specific_env_is_rejected(self, path: str): + with pytest.raises(DeploymentError, match="Configure them under helm_overrides.global"): + _merge( + _manifest(temporal=True), + _env_config({path: {"env": [{"name": "WORKFLOW_NAME", "value": "legacy"}]}}), + ) + + @pytest.mark.parametrize("group_name", ["agent", "workflow"]) + @pytest.mark.parametrize("group_value", [None, "invalid"]) + def test_identity_global_groups_must_be_mappings(self, group_name: str, group_value: Any): + with pytest.raises( + DeploymentError, + match=rf"helm_overrides\.global\.{group_name} must be a mapping", + ): + _merge( + _manifest(temporal=True), + _env_config({"global": {group_name: group_value}}), + )