[ROB-3981] Don't force Azure Prometheus auth from partial env vars (HolmesGPT#2101)

## Summary

- `AzurePrometheusConfig.is_azure_config()` previously selected the
Azure variant whenever `AZURE_CLIENT_ID` or `AZURE_TENANT_ID` was set on
the pod — even when the user had a self-hosted `prometheus_url`
configured and `AZURE_CLIENT_SECRET` was missing. This is common when
Azure AI Foundry is the LLM provider: the `AZURE_*` env vars exist for
an unrelated subsystem, and Holmes failed the Prometheus toolset with a
confusing missing-credentials error.
- `is_azure_config` now mirrors the same completeness rule that
`AzurePrometheusConfig.__init__` already enforces: Azure mode is
selected only when `azure_use_managed_id` is enabled OR all three of
`azure_client_id` / `azure_tenant_id` / `azure_client_secret` are
resolvable (config field or upper-cased `AZURE_*` env var).
- A *partial* Azure signal (some creds present, others missing) now logs
a `WARNING` naming what was found vs. missing and falls back to plain
Prometheus, so the user immediately sees why Azure auth was skipped.
- New `get_config_field(config, field)` module-level helper centralizes
the "config dict, else upper-cased env var" lookup pattern.
- The credential field list is sourced from
`AzurePrometheusConfig._ui_required_fields` so the auto-detect check and
the UI form requirement stay in lockstep.
- The explicit `subtype: azure-managed-prometheus` opt-in path is
untouched — it still selects `AzurePrometheusConfig` directly and raises
the existing `ValueError` if credentials are incomplete.

## Test plan

- [x] `poetry run pytest tests/plugins/toolsets/test_prometheus_unit.py
--no-cov` — 28 passed (14 existing + 7 new `TestIsAzureConfig` + 7 new
`TestGetConfigField`).
- [x] Regression test asserts the user's exact scenario: with
`AZURE_CLIENT_ID` + `AZURE_TENANT_ID` exported (no secret) and a plain
`prometheus_url` in config, `determine_prometheus_class` returns
`PrometheusConfig` (not `AzurePrometheusConfig`) and emits a "Partial
Azure" warning.
- [x] Positive cases still pass: all three env vars set →
`AzurePrometheusConfig`; `azure_use_managed_id: true` in config or env →
`AzurePrometheusConfig`.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved Azure authentication validation to detect incomplete
credential configurations and log informative warnings instead of
silently failing.
* Enhanced configuration field resolution with environment variable
fallback support.

* **Tests**
* Expanded test coverage for configuration field handling and Azure
authentication detection scenarios.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/HolmesGPT/holmesgpt/pull/2101?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: avi@robusta.dev <avi@robusta.dev>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: Avi-Robusta

holmes/plugins/toolsets/prometheus/prometheus.py

@@ -77,6 +77,14 @@ class PrometheusSubtype(str, Enum):
     AZURE_MANAGED_PROMETHEUS = "azure-managed-prometheus"
 
 
+def get_config_field(config: dict[str, Any], field: str) -> Any:
+    """Return the value of ``field`` from ``config``, falling back to the
+    matching upper-cased environment variable. Empty/None config values fall
+    through to the env var; missing or empty env vars return ``None``.
+    """
+    return config.get(field) or os.environ.get(field.upper()) or None
+
+
 def format_ssl_error_message(prometheus_url: str, error: SSLError) -> str:
     """Format a clear SSL error message with remediation steps."""
     return (
@@ -536,19 +544,44 @@ def __init__(self, **data):
 
     @staticmethod
     def is_azure_config(config: dict[str, Any]) -> bool:
-        """Check if config dict or environment variables indicate Azure Prometheus config."""
-        # Check for explicit Azure fields in config
-        if (
-            "azure_client_id" in config
-            or "azure_tenant_id" in config
-            or "azure_use_managed_id" in config
-        ):
-            return True
+        """Return True if the config (dict + AZURE_* env vars) is complete enough
+        to actually use Azure Managed Prometheus auth.
+
+        Mirrors the completeness rules enforced in ``__init__``: either
+        ``azure_use_managed_id`` is enabled, or all three of
+        ``azure_client_id`` / ``azure_tenant_id`` / ``azure_client_secret`` are
+        resolvable. A partial Azure signal (e.g. ``AZURE_CLIENT_ID`` set on the
+        pod for an unrelated subsystem like Azure AI Foundry) logs a warning
+        explaining why Azure auth was skipped, then returns ``False`` so the
+        toolset falls back to plain Prometheus instead of failing on a missing
+        client secret.
+        """
+        creds = {
+            field: get_config_field(config, field)
+            for field in AzurePrometheusConfig._ui_required_fields
+        }
+        use_managed_id = str(
+            get_config_field(config, "azure_use_managed_id") or ""
+        ).lower() in ("true", "1")
 
-        # Check for Azure environment variables
-        if os.environ.get("AZURE_CLIENT_ID") or os.environ.get("AZURE_TENANT_ID"):
+        if use_managed_id or all(creds.values()):
             return True
 
+        found = [f"{f}/{f.upper()}" for f, v in creds.items() if v]
+        missing = [f"{f}/{f.upper()}" for f, v in creds.items() if not v]
+
+        if found:
+            logging.warning(
+                "Partial Azure Managed Prometheus credentials detected "
+                "(found: %s; missing: %s). Not using Azure auth — falling back "
+                "to plain Prometheus. To enable Azure, provide all of "
+                "azure_client_id / azure_tenant_id / azure_client_secret "
+                "(in the toolset config or as AZURE_* env vars), or set "
+                "azure_use_managed_id: true for managed identity.",
+                ", ".join(found),
+                ", ".join(missing),
+            )
+
         return False
 
     def is_amp(self) -> bool:

tests/plugins/toolsets/test_prometheus_unit.py

@@ -1,7 +1,13 @@
+import logging
+
 import pytest
 
 from holmes.plugins.toolsets.prometheus.prometheus import (
+    AzurePrometheusConfig,
+    PrometheusConfig,
+    PrometheusToolset,
     adjust_step_for_max_points,
+    get_config_field,
 )
 
 
@@ -192,3 +198,134 @@ def test_override_with_explicit_step_adjusts_if_needed(self, monkeypatch):
             max_points_override=1000,
         )
         assert result == 3.6
+
+
+AZURE_ENV_VARS = (
+    "AZURE_CLIENT_ID",
+    "AZURE_TENANT_ID",
+    "AZURE_CLIENT_SECRET",
+    "AZURE_USE_MANAGED_ID",
+)
+
+
+@pytest.fixture
+def clean_azure_env(monkeypatch):
+    for name in AZURE_ENV_VARS:
+        monkeypatch.delenv(name, raising=False)
+    return monkeypatch
+
+
+class TestGetConfigField:
+    """get_config_field reads ``field`` from the config dict, falling back to
+    the upper-cased env var (e.g. ``AZURE_CLIENT_ID``)."""
+
+    def test_returns_config_value_when_present(self, clean_azure_env):
+        assert (
+            get_config_field({"azure_client_id": "from-config"}, "azure_client_id")
+            == "from-config"
+        )
+
+    def test_falls_back_to_env_var(self, clean_azure_env):
+        clean_azure_env.setenv("AZURE_CLIENT_ID", "from-env")
+        assert get_config_field({}, "azure_client_id") == "from-env"
+
+    def test_config_takes_precedence_over_env_var(self, clean_azure_env):
+        clean_azure_env.setenv("AZURE_CLIENT_ID", "from-env")
+        assert (
+            get_config_field({"azure_client_id": "from-config"}, "azure_client_id")
+            == "from-config"
+        )
+
+    def test_empty_config_value_falls_through_to_env(self, clean_azure_env):
+        """Empty string in YAML (e.g. ``azure_client_id: ""``) should not
+        suppress the env-var fallback — the empty value is no value."""
+        clean_azure_env.setenv("AZURE_CLIENT_ID", "from-env")
+        assert (
+            get_config_field({"azure_client_id": ""}, "azure_client_id") == "from-env"
+        )
+
+    def test_missing_returns_none(self, clean_azure_env):
+        assert get_config_field({}, "azure_client_id") is None
+
+    def test_empty_env_var_returns_none(self, clean_azure_env):
+        clean_azure_env.setenv("AZURE_CLIENT_ID", "")
+        assert get_config_field({}, "azure_client_id") is None
+
+    def test_non_string_config_value_passes_through(self, clean_azure_env):
+        """Bool/int config values (e.g. ``azure_use_managed_id: true``) must
+        not be coerced — callers handle the type-specific normalization."""
+        assert (
+            get_config_field({"azure_use_managed_id": True}, "azure_use_managed_id")
+            is True
+        )
+
+
+class TestIsAzureConfig:
+    """is_azure_config must match the completeness check inside
+    AzurePrometheusConfig.__init__ — a partial signal (e.g. AZURE_CLIENT_ID
+    alone, common when the LLM uses Azure AI Foundry) should NOT select the
+    Azure variant, and the user should be warned why."""
+
+    def test_no_signals_returns_false(self, clean_azure_env, caplog):
+        with caplog.at_level(logging.WARNING):
+            assert AzurePrometheusConfig.is_azure_config({}) is False
+        assert "Azure" not in caplog.text
+
+    def test_partial_env_vars_warns_and_returns_false(self, clean_azure_env, caplog):
+        """Regression: user had AZURE_CLIENT_ID + AZURE_TENANT_ID from their
+        Azure AI Foundry LLM setup, and an internal prometheus_url. Holmes
+        must NOT hijack into Azure auth."""
+        clean_azure_env.setenv("AZURE_CLIENT_ID", "from-llm")
+        clean_azure_env.setenv("AZURE_TENANT_ID", "from-llm")
+        with caplog.at_level(logging.WARNING):
+            assert AzurePrometheusConfig.is_azure_config({}) is False
+        assert "Partial Azure" in caplog.text
+        assert "azure_client_secret" in caplog.text
+
+    def test_partial_config_field_warns_and_returns_false(
+        self, clean_azure_env, caplog
+    ):
+        with caplog.at_level(logging.WARNING):
+            assert (
+                AzurePrometheusConfig.is_azure_config({"azure_client_id": "x"})
+                is False
+            )
+        assert "Partial Azure" in caplog.text
+
+    def test_full_env_vars_returns_true_no_warning(self, clean_azure_env, caplog):
+        clean_azure_env.setenv("AZURE_CLIENT_ID", "x")
+        clean_azure_env.setenv("AZURE_TENANT_ID", "y")
+        clean_azure_env.setenv("AZURE_CLIENT_SECRET", "z")
+        with caplog.at_level(logging.WARNING):
+            assert AzurePrometheusConfig.is_azure_config({}) is True
+        assert "Partial Azure" not in caplog.text
+
+    def test_managed_id_in_config_returns_true(self, clean_azure_env, caplog):
+        with caplog.at_level(logging.WARNING):
+            assert (
+                AzurePrometheusConfig.is_azure_config({"azure_use_managed_id": True})
+                is True
+            )
+        assert "Partial Azure" not in caplog.text
+
+    def test_managed_id_env_var_returns_true(self, clean_azure_env, caplog):
+        clean_azure_env.setenv("AZURE_USE_MANAGED_ID", "true")
+        with caplog.at_level(logging.WARNING):
+            assert AzurePrometheusConfig.is_azure_config({}) is True
+        assert "Partial Azure" not in caplog.text
+
+    def test_determine_prometheus_class_with_partial_azure_env(
+        self, clean_azure_env, caplog
+    ):
+        """End-to-end: the user's exact scenario — partial Azure env vars on
+        the pod plus a plain prometheus_url config — must resolve to the
+        generic PrometheusConfig, not AzurePrometheusConfig."""
+        clean_azure_env.setenv("AZURE_CLIENT_ID", "from-llm")
+        clean_azure_env.setenv("AZURE_TENANT_ID", "from-llm")
+        toolset = PrometheusToolset()
+        with caplog.at_level(logging.WARNING):
+            cls = toolset.determine_prometheus_class(
+                {"prometheus_url": "https://internal.example:8030", "verify_ssl": True}
+            )
+        assert cls is PrometheusConfig
+        assert "Partial Azure" in caplog.text