Merge pull request #3 from schubergphilis/feature/surface-direnv-credential-errors

feat: surface direnv stderr when .envrc credential lookup fails

Author: Yhoorneman

README.md

@@ -17,7 +17,7 @@
 [![Changelog](https://img.shields.io/badge/changelog-Keep%20a%20Changelog%201.1.0-orange)](https://keepachangelog.com/en/1.1.0/)
 [![Documentation: Diátaxis](https://img.shields.io/badge/docs-Di%C3%A1taxis-009485?logo=readthedocs&logoColor=white)](https://diataxis.fr/)
 [![Build](https://img.shields.io/badge/build-unknown-lightgrey)](https://github.com/features/actions)
-[![Coverage](https://img.shields.io/badge/coverage-60%25-orange)](https://coverage.readthedocs.io/)
+[![Coverage](https://img.shields.io/badge/coverage-61%25-orange)](https://coverage.readthedocs.io/)
 [![pyscn quality](https://img.shields.io/badge/pyscn-not%20rated-lightgrey)](https://pyscn.ludo-tech.org)
 
 CLI to set overrides idempotently for multiple SLO's

src/datadog_slo_overrides_cli/datadog_slo_overrides_cli.py

@@ -528,55 +528,66 @@ def _run_direnv_export(directory: Path) -> subprocess.CompletedProcess[str] | No
     )
 
 
-def load_direnv_env(directory: Path) -> dict[str, str]:
-    """Return the environment variables an optional ``.envrc`` produces via direnv.
+def load_direnv_env(directory: Path) -> tuple[dict[str, str], str]:
+    """Return the variables an optional ``.envrc`` produces via direnv, plus direnv's stderr.
 
-    Returns ``{}`` silently when direnv isn't installed or there's no ``.envrc``.
-    When direnv refuses an unapproved ``.envrc``, prints an actionable hint and
-    returns ``{}`` rather than crashing.
+    Returns ``({}, '')`` silently when direnv isn't installed or there's no
+    ``.envrc``. When direnv refuses an unapproved ``.envrc``, prints an actionable
+    hint and returns ``({}, <stderr>)`` rather than crashing. The stderr is
+    returned so callers can surface *why* the ``.envrc`` produced no usable values
+    (e.g. a Vault command inside it failing), since direnv runs the file itself.
 
     Args:
         directory: Directory whose ``.envrc`` should be evaluated.
 
     Returns:
-        A mapping of exported variable names to string values.
+        A ``(exported vars, direnv stderr)`` pair.
     """
     result = _run_direnv_export(directory)
     if result is None:
-        return {}
+        return {}, ''
     if result.returncode != 0 or _envrc_is_blocked(result.stderr):
         typer.echo(
             f'direnv could not load {directory}/.envrc (not approved?). Run: direnv allow {directory}',
             err=True,
         )
-        return {}
+        return {}, result.stderr
     stdout = result.stdout.strip()
     if not stdout:
-        return {}
+        return {}, result.stderr
     try:
         data = json.loads(stdout)
     except json.JSONDecodeError:
-        return {}
-    return {key: value for key, value in data.items() if isinstance(value, str)}
+        return {}, result.stderr
+    return {key: value for key, value in data.items() if isinstance(value, str)}, result.stderr
 
 
-def _warn_if_envrc_lacks_keys(directory: Path, direnv_env: dict[str, str], missing: list[str]) -> None:
-    """Warn when an evaluated ``.envrc`` didn't export the credential keys still needed.
+def _warn_if_envrc_lacks_keys(directory: Path, direnv_env: dict[str, str], missing: list[str], stderr: str) -> None:
+    """Warn when an evaluated ``.envrc`` didn't provide the credential keys still needed.
 
     ``direnv_env`` is non-empty only when direnv actually evaluated an approved
     ``.envrc`` (it always includes direnv's own bookkeeping vars), which lets us
-    tell "loaded but missing the keys" apart from "blocked" or "no .envrc".
+    tell "loaded but missing the keys" apart from "blocked" or "no .envrc". When
+    the file ran but the keys are unset/empty, direnv's stderr usually explains
+    why (e.g. a failed Vault lookup), so it is echoed back as the reason.
 
     Args:
         directory: Directory whose ``.envrc`` was evaluated.
         direnv_env: Variables direnv exported (empty if it didn't run/was blocked).
         missing: Required credential variables still unset after the merge.
+        stderr: direnv's stderr from evaluating the ``.envrc``.
     """
-    if direnv_env and missing:
-        typer.echo(
-            f'{directory}/.envrc was loaded via direnv but does not export: {", ".join(missing)}',
-            err=True,
-        )
+    if not (direnv_env and missing):
+        return
+    typer.echo(
+        f'{directory}/.envrc was loaded via direnv but did not provide: {", ".join(missing)}',
+        err=True,
+    )
+    diagnostic = stderr.strip()
+    if diagnostic:
+        typer.echo('direnv reported:', err=True)
+        for line in diagnostic.splitlines():
+            typer.echo(f'  {line}', err=True)
 
 
 def resolve_credentials(api_key: str | None, app_key: str | None, directory: Path) -> tuple[str | None, str | None]:
@@ -599,11 +610,11 @@ def resolve_credentials(api_key: str | None, app_key: str | None, directory: Pat
     """
     if api_key and app_key:
         return api_key, app_key
-    direnv_env = load_direnv_env(directory)
+    direnv_env, stderr = load_direnv_env(directory)
     api_key = api_key or direnv_env.get(API_KEY_ENV)
     app_key = app_key or direnv_env.get(APP_KEY_ENV)
     missing = [name for name, value in ((API_KEY_ENV, api_key), (APP_KEY_ENV, app_key)) if not value]
-    _warn_if_envrc_lacks_keys(directory, direnv_env, missing)
+    _warn_if_envrc_lacks_keys(directory, direnv_env, missing, stderr)
     return api_key, app_key
 
 

tests/test_datadog_slo_overrides_cli.py

@@ -197,7 +197,7 @@ def test_load_direnv_env_success(tmp_path: Path, monkeypatch: pytest.MonkeyPatch
     payload = json.dumps({'DD_API_KEY': 'key-123', 'DD_APP_KEY': 'app-456', 'DIRENV_DIFF': 'x'})
     monkeypatch.setattr(cli.subprocess, 'run', lambda *_args, **_kwargs: _completed(stdout=payload))
 
-    loaded = load_direnv_env(tmp_path)
+    loaded, _stderr = load_direnv_env(tmp_path)
     assert loaded['DD_API_KEY'] == 'key-123'
     assert loaded['DD_APP_KEY'] == 'app-456'
 
@@ -211,7 +211,8 @@ def test_load_direnv_env_unallowed(
     blocked = _completed(stderr=f'direnv: error {tmp_path}/.envrc is blocked. Run `direnv allow`.', returncode=1)
     monkeypatch.setattr(cli.subprocess, 'run', lambda *_args, **_kwargs: blocked)
 
-    assert load_direnv_env(tmp_path) == {}
+    env, _stderr = load_direnv_env(tmp_path)
+    assert env == {}
     assert 'direnv allow' in capsys.readouterr().err
 
 
@@ -225,7 +226,8 @@ def _fail(*_args: object, **_kwargs: object) -> object:
         raise AssertionError(msg)
 
     monkeypatch.setattr(cli.subprocess, 'run', _fail)
-    assert load_direnv_env(tmp_path) == {}
+    env, _stderr = load_direnv_env(tmp_path)
+    assert env == {}
 
 
 def test_resolve_credentials_prefers_explicit_values(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
@@ -244,20 +246,27 @@ def test_resolve_credentials_fills_missing_from_direnv_without_override(
     monkeypatch: pytest.MonkeyPatch,
 ) -> None:
     """Missing keys are filled from direnv; an explicit value is never overridden."""
-    monkeypatch.setattr(cli, 'load_direnv_env', lambda _d: {'DD_API_KEY': 'direnv-key', 'DD_APP_KEY': 'direnv-app'})
+    monkeypatch.setattr(
+        cli, 'load_direnv_env', lambda _d: ({'DD_API_KEY': 'direnv-key', 'DD_APP_KEY': 'direnv-app'}, '')
+    )
     assert resolve_credentials('flag-key', None, tmp_path) == ('flag-key', 'direnv-app')
 
 
-def test_resolve_credentials_warns_when_envrc_lacks_keys(
+def test_resolve_credentials_surfaces_direnv_reason_when_keys_missing(
     tmp_path: Path,
     monkeypatch: pytest.MonkeyPatch,
     capsys: pytest.CaptureFixture[str],
 ) -> None:
-    """An evaluated .envrc that exports no Datadog keys yields a hint naming the missing ones."""
-    # Non-empty dict (direnv ran) but without DD_API_KEY/DD_APP_KEY.
-    monkeypatch.setattr(cli, 'load_direnv_env', lambda _d: {'DIRENV_DIFF': 'x'})
+    """An evaluated .envrc that yields no usable keys names them and echoes direnv's stderr."""
+    # direnv ran (non-empty dict) but the Vault lookups failed, so the keys are absent
+    # and the reason is on stderr.
+    vault_error = 'Error making API request.\nCode: 403. Errors:\n* permission denied'
+    monkeypatch.setattr(cli, 'load_direnv_env', lambda _d: ({'DIRENV_DIFF': 'x'}, vault_error))
     assert resolve_credentials(None, None, tmp_path) == (None, None)
     err = capsys.readouterr().err
     assert 'DD_API_KEY' in err
     assert 'DD_APP_KEY' in err
-    assert 'does not export' in err
+    assert 'did not provide' in err
+    assert 'direnv reported:' in err
+    assert '403' in err
+    assert 'permission denied' in err