fix: skip resources with empty IDs from conditional env vars in config processing

Added handling in replace_env_vars() to skip registered resources when their
ID fields (model_id, shield_id, dataset_id, scoring_fn_id, benchmark_id,
toolgroup_id) resolve to empty/None from conditional env var syntax like
${env.VAR:+value}.

Signed-off-by: Mustafa Elbehery <melbeher@redhat.com>

Author: Elbehery

src/llama_stack/core/stack.py

@@ -110,6 +110,10 @@ class LlamaStack(
 REGISTRY_REFRESH_TASK = None
 TEST_RECORDING_CONTEXT = None
 
+# ID fields for registered resources that should trigger skipping
+# when they resolve to empty/None (from conditional env vars like :+)
+RESOURCE_ID_FIELDS = ["model_id", "shield_id", "dataset_id", "scoring_fn_id", "benchmark_id", "toolgroup_id"]
+
 
 def is_request_model(t: Any) -> bool:
     """Check if a type is a request model (Pydantic BaseModel).
@@ -346,15 +350,31 @@ def replace_env_vars(config: Any, path: str = "") -> Any:
                             logger.debug(
                                 f"Skipping config env variable expansion for disabled provider: {v.get('provider_id', '')}"
                             )
-                            # Create a copy with resolved provider_id but original config
-                            disabled_provider = v.copy()
-                            disabled_provider["provider_id"] = resolved_provider_id
                             continue
                     except EnvVarError:
                         # If we can't resolve the provider_id, continue with normal processing
                         pass
 
-                # Normal processing for non-disabled providers
+                # Special handling for registered resources: check if ID field resolves to empty/None
+                # from conditional env vars (e.g., ${env.VAR:+value}) and skip the entry if so
+                if isinstance(v, dict):
+                    should_skip = False
+                    for id_field in RESOURCE_ID_FIELDS:
+                        if id_field in v:
+                            try:
+                                resolved_id = replace_env_vars(v[id_field], f"{path}[{i}].{id_field}")
+                                if resolved_id is None or resolved_id == "":
+                                    logger.debug(
+                                        f"Skipping resource with empty {id_field} (conditional env var not set)"
+                                    )
+                                    should_skip = True
+                                    break
+                            except EnvVarError:
+                                pass
+                    if should_skip:
+                        continue
+
+                # Normal processing
                 result.append(replace_env_vars(v, f"{path}[{i}]"))
             except EnvVarError as e:
                 raise EnvVarError(e.var_name, e.path) from None

tests/unit/server/test_replace_env_vars.py

@@ -95,3 +95,86 @@ def test_explicit_strings_preserved(setup_env_vars):
     data = {"port": "8080", "enabled": "true", "count": "123", "ratio": "3.14"}
     expected = {"port": "8080", "enabled": "true", "count": "123", "ratio": "3.14"}
     assert replace_env_vars(data) == expected
+
+
+def test_resource_with_empty_benchmark_id_skipped(setup_env_vars):
+    """Test that resources with empty benchmark_id from conditional env vars are skipped."""
+    data = {
+        "benchmarks": [
+            {"benchmark_id": "${env.BENCHMARK_ID:+my-benchmark}", "dataset_id": "test-dataset"},
+            {"benchmark_id": "always-present", "dataset_id": "another-dataset"},
+        ]
+    }
+    # BENCHMARK_ID is not set, so first benchmark should be skipped
+    result = replace_env_vars(data)
+    assert len(result["benchmarks"]) == 1
+    assert result["benchmarks"][0]["benchmark_id"] == "always-present"
+
+
+def test_resource_with_set_benchmark_id_not_skipped(setup_env_vars):
+    """Test that resources with set benchmark_id are not skipped."""
+    os.environ["BENCHMARK_ID"] = "enabled"
+    try:
+        data = {
+            "benchmarks": [
+                {"benchmark_id": "${env.BENCHMARK_ID:+my-benchmark}", "dataset_id": "test-dataset"},
+                {"benchmark_id": "always-present", "dataset_id": "another-dataset"},
+            ]
+        }
+        result = replace_env_vars(data)
+        assert len(result["benchmarks"]) == 2
+        assert result["benchmarks"][0]["benchmark_id"] == "my-benchmark"
+        assert result["benchmarks"][1]["benchmark_id"] == "always-present"
+    finally:
+        del os.environ["BENCHMARK_ID"]
+
+
+def test_resource_with_empty_model_id_skipped(setup_env_vars):
+    """Test that resources with empty model_id from conditional env vars are skipped."""
+    data = {
+        "models": [
+            {"model_id": "${env.MODEL_ID:+my-model}", "provider_id": "test-provider"},
+            {"model_id": "always-present", "provider_id": "another-provider"},
+        ]
+    }
+    # MODEL_ID is not set, so first model should be skipped
+    result = replace_env_vars(data)
+    assert len(result["models"]) == 1
+    assert result["models"][0]["model_id"] == "always-present"
+
+
+def test_resource_with_empty_shield_id_skipped(setup_env_vars):
+    """Test that resources with empty shield_id from conditional env vars are skipped."""
+    data = {
+        "shields": [
+            {"shield_id": "${env.SHIELD_ID:+my-shield}", "provider_id": "test-provider"},
+            {"shield_id": "always-present", "provider_id": "another-provider"},
+        ]
+    }
+    # SHIELD_ID is not set, so first shield should be skipped
+    result = replace_env_vars(data)
+    assert len(result["shields"]) == 1
+    assert result["shields"][0]["shield_id"] == "always-present"
+
+
+def test_multiple_resources_with_conditional_ids(setup_env_vars):
+    """Test that multiple resource types with conditional IDs are handled correctly."""
+    os.environ["INCLUDE_BENCHMARK"] = "yes"
+    try:
+        data = {
+            "benchmarks": [
+                {"benchmark_id": "${env.INCLUDE_BENCHMARK:+included-benchmark}", "dataset_id": "ds1"},
+                {"benchmark_id": "${env.EXCLUDE_BENCHMARK:+excluded-benchmark}", "dataset_id": "ds2"},
+            ],
+            "models": [
+                {"model_id": "${env.EXCLUDE_MODEL:+excluded-model}", "provider_id": "p1"},
+            ],
+        }
+        result = replace_env_vars(data)
+        # Only the benchmark with INCLUDE_BENCHMARK set should remain
+        assert len(result["benchmarks"]) == 1
+        assert result["benchmarks"][0]["benchmark_id"] == "included-benchmark"
+        # Model with unset env var should be skipped
+        assert len(result["models"]) == 0
+    finally:
+        del os.environ["INCLUDE_BENCHMARK"]