From 920345011f3b547686687933524dcd2a1f413d07 Mon Sep 17 00:00:00 2001 From: Jonathan Griffe Date: Wed, 13 Nov 2024 15:16:23 +0100 Subject: [PATCH] feat(api-status): add api key and instance sources to the api-status command output --- ...n.griffe_improve_api_status_output_info.md | 3 + doc/schemas/api-status.json | 8 +++ ggshield/__main__.py | 2 +- ggshield/cmd/status.py | 14 ++-- ggshield/core/config/config.py | 71 ++++++++++++++++--- ggshield/core/env_utils.py | 22 ++++-- tests/unit/cmd/test_status.py | 69 +++++++++++++++++- tests/unit/core/test_env_utils.py | 17 ++++- 8 files changed, 185 insertions(+), 21 deletions(-) create mode 100644 changelog.d/20241113_152802_jonathan.griffe_improve_api_status_output_info.md diff --git a/changelog.d/20241113_152802_jonathan.griffe_improve_api_status_output_info.md b/changelog.d/20241113_152802_jonathan.griffe_improve_api_status_output_info.md new file mode 100644 index 0000000000..c4f73963f0 --- /dev/null +++ b/changelog.d/20241113_152802_jonathan.griffe_improve_api_status_output_info.md @@ -0,0 +1,3 @@ +### Added + +- The `api-status` command now returns the sources of both the api-key and instance used. diff --git a/doc/schemas/api-status.json b/doc/schemas/api-status.json index b26864d8ae..bcf53239b5 100644 --- a/doc/schemas/api-status.json +++ b/doc/schemas/api-status.json @@ -13,6 +13,14 @@ "format": "uri", "description": "URL of the GitGuardian instance" }, + "instance_source": { + "enum": ["CMD_OPTION", "DOTENV", "ENV_VAR", "USER_CONFIG", "DEFAULT"], + "description": "Source the instance was read from" + }, + "api_key_source": { + "enum": ["DOTENV", "ENV_VAR", "USER_CONFIG"], + "description": "Source the API key was read from" + }, "detail": { "type": "string", "description": "Human-readable version of the status" diff --git a/ggshield/__main__.py b/ggshield/__main__.py index be3fef13a8..6913e78dc3 100644 --- a/ggshield/__main__.py +++ b/ggshield/__main__.py @@ -107,7 +107,7 @@ def cli( if allow_self_signed: user_config.allow_self_signed = allow_self_signed - load_dot_env() + ctx_obj.config._dotenv_vars = load_dot_env() _set_color(ctx) diff --git a/ggshield/cmd/status.py b/ggshield/cmd/status.py index 54186e3011..529f9102b7 100644 --- a/ggshield/cmd/status.py +++ b/ggshield/cmd/status.py @@ -24,7 +24,7 @@ @add_common_options() @click.pass_context def status_cmd(ctx: click.Context, **kwargs: Any) -> int: - """Show API status and version.""" + """Show API status and version, along with API key and instance sources.""" ctx_obj = ContextObj.get(ctx) client = create_client_from_config(ctx_obj.config) response: HealthCheckResponse = client.health_check() @@ -32,17 +32,23 @@ def status_cmd(ctx: click.Context, **kwargs: Any) -> int: if not isinstance(response, HealthCheckResponse): raise UnexpectedError("Unexpected health check response") + instance, instance_source = ctx_obj.config.get_instance_name_and_source() + _, api_key_source = ctx_obj.config.get_api_key_and_source() if ctx_obj.use_json: json_output = response.to_dict() - json_output["instance"] = client.base_uri + json_output["instance"] = instance + json_output["instance_source"] = instance_source.name + json_output["api_key_source"] = api_key_source.name click.echo(json.dumps(json_output)) else: click.echo( - f"{format_text('API URL:', STYLE['key'])} {client.base_uri}\n" + f"{format_text('API URL:', STYLE['key'])} {instance}\n" f"{format_text('Status:', STYLE['key'])} {format_healthcheck_status(response)}\n" f"{format_text('App version:', STYLE['key'])} {response.app_version or 'Unknown'}\n" f"{format_text('Secrets engine version:', STYLE['key'])} " - f"{response.secrets_engine_version or 'Unknown'}\n" + f"{response.secrets_engine_version or 'Unknown'}\n\n" + f"{format_text('Instance source:', STYLE['key'])} {instance_source.value}\n" + f"{format_text('API key source:', STYLE['key'])} {api_key_source.value}\n" ) return 0 diff --git a/ggshield/core/config/config.py b/ggshield/core/config/config.py index 07ce9f4171..67e200b6f9 100644 --- a/ggshield/core/config/config.py +++ b/ggshield/core/config/config.py @@ -1,7 +1,8 @@ import logging import os +from enum import Enum from pathlib import Path -from typing import Any, Optional +from typing import Any, Optional, Set, Tuple import click @@ -17,6 +18,19 @@ ) +class ConfigSource(Enum): + """ + Enum of the different sources of configuration + where an API key or instance URL can come from + """ + + CMD_OPTION = "command line option" + DOTENV = ".env file" + ENV_VAR = "environment variable" + USER_CONFIG = "user config" + DEFAULT = "default" + + logger = logging.getLogger(__name__) @@ -26,7 +40,13 @@ class Config: AuthConfig. """ - __slots__ = ["user_config", "auth_config", "_cmdline_instance_name", "_config_path"] + __slots__ = [ + "user_config", + "auth_config", + "_cmdline_instance_name", + "_config_path", + "_dotenv_vars", + ] user_config: UserConfig auth_config: AuthConfig @@ -36,10 +56,16 @@ class Config: _config_path: Path + # This environment variable helps us knowing whether environment variables + # were set by the dotenv file or not + # It is used in the `api-status` command to return the API key and instance sources + _dotenv_vars: Set[str] + def __init__(self, config_path: Optional[Path] = None): self.user_config, self._config_path = UserConfig.load(config_path=config_path) self.auth_config = AuthConfig.load() self._cmdline_instance_name = None + self._dotenv_vars = set() def save(self) -> None: self.user_config.save(self._config_path) @@ -51,17 +77,23 @@ def config_path(self) -> Path: @property def instance_name(self) -> str: + return self.get_instance_name_and_source()[0] + + def get_instance_name_and_source(self) -> Tuple[str, ConfigSource]: """ + Return the instance name and source of the selected instance. + The instance name (defaulting to URL) of the selected instance priority order is: - set from the command line (by setting cmdline_instance_name) - - env var (in auth_config.current_instance) + - GITGUARDIAN_INSTANCE env var + - GITGUARDIAN_API_URL env var - in local user config (in user_config.dashboard_url) - in global user config (in user_config.dashboard_url) - the default instance """ if self._cmdline_instance_name: - return self._cmdline_instance_name + return self._cmdline_instance_name, ConfigSource.CMD_OPTION try: url = os.environ["GITGUARDIAN_INSTANCE"] @@ -70,7 +102,12 @@ def instance_name(self) -> str: pass else: validate_instance_url(url) - return remove_url_trailing_slash(url) + source = ( + ConfigSource.DOTENV + if "GITGUARDIAN_INSTANCE" in self._dotenv_vars + else ConfigSource.ENV_VAR + ) + return remove_url_trailing_slash(url), source try: name = os.environ["GITGUARDIAN_API_URL"] @@ -78,12 +115,17 @@ def instance_name(self) -> str: except KeyError: pass else: - return api_to_dashboard_url(name, warn=True) + source = ( + ConfigSource.DOTENV + if "GITGUARDIAN_API_URL" in self._dotenv_vars + else ConfigSource.ENV_VAR + ) + return api_to_dashboard_url(name, warn=True), source if self.user_config.instance: - return self.user_config.instance + return self.user_config.instance, ConfigSource.USER_CONFIG - return DEFAULT_INSTANCE_URL + return DEFAULT_INSTANCE_URL, ConfigSource.DEFAULT @property def cmdline_instance_name(self) -> Optional[str]: @@ -123,7 +165,12 @@ def dashboard_url(self) -> str: @property def api_key(self) -> str: + return self.get_api_key_and_source()[0] + + def get_api_key_and_source(self) -> Tuple[str, ConfigSource]: """ + Return the selected API key and its source + The API key to use priority order is - env var @@ -132,9 +179,15 @@ def api_key(self) -> str: try: key = os.environ["GITGUARDIAN_API_KEY"] logger.debug("Using API key from $GITGUARDIAN_API_KEY") + source = ( + ConfigSource.DOTENV + if "GITGUARDIAN_API_KEY" in self._dotenv_vars + else ConfigSource.ENV_VAR + ) except KeyError: key = self.auth_config.get_instance_token(self.instance_name) - return key + source = ConfigSource.USER_CONFIG + return key, source def add_ignored_match(self, *args: Any, **kwargs: Any) -> None: return self.user_config.secret.add_ignored_match(*args, **kwargs) diff --git a/ggshield/core/env_utils.py b/ggshield/core/env_utils.py index 4733114aa5..db6aa8d5df 100644 --- a/ggshield/core/env_utils.py +++ b/ggshield/core/env_utils.py @@ -1,15 +1,21 @@ import logging import os from pathlib import Path -from typing import Optional +from typing import Optional, Set -from dotenv import load_dotenv +from dotenv import dotenv_values, load_dotenv from ggshield.core import ui from ggshield.utils.git_shell import get_git_root, is_git_dir from ggshield.utils.os import getenv_bool +TRACKED_ENV_VARS = { + "GITGUARDIAN_INSTANCE", + "GITGUARDIAN_API_URL", + "GITGUARDIAN_API_KEY", +} + logger = logging.getLogger(__name__) @@ -39,15 +45,21 @@ def _find_dot_env() -> Optional[Path]: return None -def load_dot_env() -> None: - """Loads .env file into os.environ.""" +def load_dot_env() -> Set[str]: + """ + Loads .env file into os.environ. + Return the list of env vars that were set by the dotenv file + among env vars in TRACKED_ENV_VARS + """ dont_load_env = getenv_bool("GITGUARDIAN_DONT_LOAD_ENV") if dont_load_env: logger.debug("Not loading .env, GITGUARDIAN_DONT_LOAD_ENV is set") - return + return set() dot_env_path = _find_dot_env() if dot_env_path: dot_env_path = dot_env_path.absolute() logger.debug("Loading environment file %s", dot_env_path) load_dotenv(dot_env_path, override=True) + + return dotenv_values(dot_env_path).keys() & TRACKED_ENV_VARS diff --git a/tests/unit/cmd/test_status.py b/tests/unit/cmd/test_status.py index da69e89ff5..9e97a72d9c 100644 --- a/tests/unit/cmd/test_status.py +++ b/tests/unit/cmd/test_status.py @@ -3,10 +3,13 @@ import jsonschema import pytest +from pygitguardian.models import HealthCheckResponse from pytest_voluptuous import S -from voluptuous.validators import All, Match +from voluptuous.validators import All, In, Match from ggshield.__main__ import cli +from ggshield.core.config.config import ConfigSource +from ggshield.utils.os import cd from tests.unit.conftest import assert_invoke_ok, my_vcr @@ -40,6 +43,8 @@ def test_api_status(cli_fs_runner, api_status_json_schema): "status_code": 200, "app_version": Match(r"v\d\.\d{1,3}\.\d{1,2}(-rc\.\d)?"), "secrets_engine_version": Match(r"\d\.\d{1,3}\.\d"), + "instance_source": In(x.name for x in ConfigSource), + "api_key_source": In(x.name for x in ConfigSource), } ) ) @@ -47,6 +52,68 @@ def test_api_status(cli_fs_runner, api_status_json_schema): ) +@mock.patch( + "ggshield.core.config.auth_config.AuthConfig.get_instance_token", + return_value="token", +) +@mock.patch( + "pygitguardian.GGClient.health_check", + return_value=HealthCheckResponse(detail="", status_code=200), +) +def test_api_status_sources(_, hs_mock, cli_fs_runner, tmp_path, monkeypatch): + """ + GIVEN an api_key and an instance configured anywhere + WHEN running the api-status command + THEN the correct api key and instance source are returned + """ + (tmp_path / ".env").touch() + + def get_api_status(env, instance=None): + with cd(tmp_path): + cmd = ["api-status", "--json"] + if instance: + cmd.extend(["--instance", instance]) + result = cli_fs_runner.invoke(cli, cmd, color=False, env=env) + + json_res = json.loads(result.output) + return json_res["instance_source"], json_res["api_key_source"] + + env: dict[str, str | None] = { + "GITGUARDIAN_INSTANCE": None, + "GITGUARDIAN_URL": None, + "GITGUARDIAN_API_KEY": None, + } + instance_source, api_key_source = get_api_status(env) + assert instance_source == ConfigSource.DEFAULT.name + assert api_key_source == ConfigSource.USER_CONFIG.name + + (tmp_path / ".gitguardian.yaml").write_text( + "version: 2\ninstance: https://dashboard.gitguardian.com\n" + ) + instance_source, api_key_source = get_api_status(env) + assert instance_source == ConfigSource.USER_CONFIG.name + assert api_key_source == ConfigSource.USER_CONFIG.name + + env["GITGUARDIAN_INSTANCE"] = "https://dashboard.gitguardian.com" + env["GITGUARDIAN_API_KEY"] = "token" + instance_source, api_key_source = get_api_status(env) + assert instance_source == ConfigSource.ENV_VAR.name + assert api_key_source == ConfigSource.ENV_VAR.name + + (tmp_path / ".env").write_text( + "GITGUARDIAN_INSTANCE=https://dashboard.gitguardian.com\n" + "GITGUARDIAN_API_KEY=token" + ) + instance_source, api_key_source = get_api_status(env) + assert instance_source == ConfigSource.DOTENV.name + assert api_key_source == ConfigSource.DOTENV.name + + assert ( + get_api_status(env, instance="https://dashboard.gitguardian.com")[0] + == ConfigSource.CMD_OPTION.name + ) + + @pytest.mark.parametrize("verify", [True, False]) def test_ssl_verify(cli_fs_runner, verify): cmd = ["api-status"] if verify else ["--allow-self-signed", "api-status"] diff --git a/tests/unit/core/test_env_utils.py b/tests/unit/core/test_env_utils.py index 46ef7bd0d3..9f23e7e5e7 100644 --- a/tests/unit/core/test_env_utils.py +++ b/tests/unit/core/test_env_utils.py @@ -3,7 +3,7 @@ import pytest -from ggshield.core.env_utils import load_dot_env +from ggshield.core.env_utils import TRACKED_ENV_VARS, load_dot_env from ggshield.utils.os import cd @@ -84,3 +84,18 @@ def test_load_dot_env_loads_git_root_env( with cd(sub1_sub2_dir): load_dot_env() load_dotenv_mock.assert_called_once_with(git_root_dotenv, override=True) + + +@pytest.mark.parametrize("env_var", TRACKED_ENV_VARS) +def test_load_dot_env_returns_set_vars(env_var, tmp_path, monkeypatch): + """ + GIVEN an env var that is set, and also set with the same value in the .env + WHEN load_dot_env() is called + THEN it returns the env var + """ + monkeypatch.setenv(env_var, "value") + (tmp_path / ".env").write_text(f"{env_var}=value") + with cd(tmp_path): + set_variables = load_dot_env() + + assert set_variables == {env_var}