feat(cli): do not require config file to run CLI

nejch · nejch · commit e6449d3020d3 · 2021-12-13T00:47:14.000+01:00
BREAKING CHANGE: A config file is no longer needed to run the CLI. python-gitlab will default to https://gitlab.com with no authentication if there is no config file provided. python-gitlab will now also only look for configuration in the provided PYTHON_GITLAB_CFG path, instead of merging it with user- and system-wide config files. If the environment variable is defined and the file cannot be opened, python-gitlab will now explicitly fail.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -26,6 +26,7 @@ repos:
       - id: pylint
         additional_dependencies:
           - argcomplete==1.12.3
+          - pytest==6.2.5
           - requests==2.26.0
           - requests-toolbelt==0.9.1
         files: 'gitlab/'
diff --git a/docs/cli-usage.rst b/docs/cli-usage.rst
@@ -4,7 +4,8 @@
 
 ``python-gitlab`` provides a :command:`gitlab` command-line tool to interact
 with GitLab servers. It uses a configuration file to define how to connect to
-the servers.
+the servers. Without a configuration file, ``gitlab`` will default to
+https://gitlab.com and unauthenticated requests.
 
 .. _cli_configuration:
 
@@ -16,8 +17,8 @@ Files
 
 ``gitlab`` looks up 3 configuration files by default:
 
-``PYTHON_GITLAB_CFG`` environment variable
-    An environment variable that contains the path to a configuration file
+The ``PYTHON_GITLAB_CFG`` environment variable
+    An environment variable that contains the path to a configuration file.
 
 ``/etc/python-gitlab.cfg``
     System-wide configuration file
@@ -27,6 +28,13 @@ Files
 
 You can use a different configuration file with the ``--config-file`` option.
 
+.. warning::
+    If the ``PYTHON_GITLAB_CFG`` environment variable is defined and the target
+    file exists, it will be the only configuration file parsed by ``gitlab``.  
+
+    If the environment variable is defined and the target file cannot be accessed,
+    ``gitlab`` will fail explicitly.
+
 Content
 -------
 
diff --git a/gitlab/config.py b/gitlab/config.py
@@ -20,27 +20,67 @@
 import shlex
 import subprocess
 from os.path import expanduser, expandvars
+from pathlib import Path
 from typing import List, Optional, Union
 
-from gitlab.const import USER_AGENT
+from gitlab.const import DEFAULT_URL, USER_AGENT
 
-
-def _env_config() -> List[str]:
-    if "PYTHON_GITLAB_CFG" in os.environ:
-        return [os.environ["PYTHON_GITLAB_CFG"]]
-    return []
-
-
-_DEFAULT_FILES: List[str] = _env_config() + [
+_DEFAULT_FILES: List[str] = [
     "/etc/python-gitlab.cfg",
-    os.path.expanduser("~/.python-gitlab.cfg"),
+    str(Path.home() / ".python-gitlab.cfg"),
 ]
 
 HELPER_PREFIX = "helper:"
 
 HELPER_ATTRIBUTES = ["job_token", "http_password", "private_token", "oauth_token"]
 
 
+def _resolve_file(filepath: Union[Path, str]) -> str:
+    resolved = Path(filepath).resolve(strict=True)
+    return str(resolved)
+
+
+def _get_config_files(
+    config_files: Optional[List[str]] = None,
+) -> Union[str, List[str]]:
+    """
+    Return resolved path(s) to config files if they exist, with precedence:
+    1. Files passed in config_files
+    2. File defined in PYTHON_GITLAB_CFG
+    3. User- and system-wide config files
+    """
+    resolved_files = []
+
+    if config_files:
+        for config_file in config_files:
+            try:
+                resolved = _resolve_file(config_file)
+            except OSError as e:
+                raise GitlabConfigMissingError(f"Cannot read config from file: {e}")
+            resolved_files.append(resolved)
+
+        return resolved_files
+
+    try:
+        env_config = os.environ["PYTHON_GITLAB_CFG"]
+        return _resolve_file(env_config)
+    except KeyError:
+        pass
+    except OSError as e:
+        raise GitlabConfigMissingError(
+            f"Cannot read config from PYTHON_GITLAB_CFG: {e}"
+        )
+
+    for config_file in _DEFAULT_FILES:
+        try:
+            resolved = _resolve_file(config_file)
+        except OSError:
+            continue
+        resolved_files.append(resolved)
+
+    return resolved_files
+
+
 class ConfigError(Exception):
     pass
 
@@ -66,155 +106,149 @@ def __init__(
         self, gitlab_id: Optional[str] = None, config_files: Optional[List[str]] = None
     ) -> None:
         self.gitlab_id = gitlab_id
-        _files = config_files or _DEFAULT_FILES
-        file_exist = False
-        for file in _files:
-            if os.path.exists(file):
-                file_exist = True
-        if not file_exist:
-            raise GitlabConfigMissingError(
-                "Config file not found. \nPlease create one in "
-                "one of the following locations: {} \nor "
-                "specify a config file using the '-c' parameter.".format(
-                    ", ".join(_DEFAULT_FILES)
-                )
-            )
+        self.http_username: Optional[str] = None
+        self.http_password: Optional[str] = None
+        self.job_token: Optional[str] = None
+        self.oauth_token: Optional[str] = None
+        self.private_token: Optional[str] = None
+
+        self.api_version: str = "4"
+        self.order_by: Optional[str] = None
+        self.pagination: Optional[str] = None
+        self.per_page: Optional[int] = None
+        self.retry_transient_errors: bool = False
+        self.ssl_verify: Union[bool, str] = True
+        self.timeout: int = 60
+        self.url: str = DEFAULT_URL
+        self.user_agent: str = USER_AGENT
 
-        self._config = configparser.ConfigParser()
-        self._config.read(_files)
+        self._files = _get_config_files(config_files)
+        if self._files:
+            self._parse_config()
+
+    def _parse_config(self) -> None:
+        _config = configparser.ConfigParser()
+        _config.read(self._files)
 
         if self.gitlab_id is None:
             try:
-                self.gitlab_id = self._config.get("global", "default")
+                self.gitlab_id = _config.get("global", "default")
             except Exception as e:
                 raise GitlabIDError(
                     "Impossible to get the gitlab id (not specified in config file)"
                 ) from e
 
         try:
-            self.url = self._config.get(self.gitlab_id, "url")
+            self.url = _config.get(self.gitlab_id, "url")
         except Exception as e:
             raise GitlabDataError(
                 "Impossible to get gitlab details from "
                 f"configuration ({self.gitlab_id})"
             ) from e
 
-        self.ssl_verify: Union[bool, str] = True
         try:
-            self.ssl_verify = self._config.getboolean("global", "ssl_verify")
+            self.ssl_verify = _config.getboolean("global", "ssl_verify")
         except ValueError:
             # Value Error means the option exists but isn't a boolean.
             # Get as a string instead as it should then be a local path to a
             # CA bundle.
             try:
-                self.ssl_verify = self._config.get("global", "ssl_verify")
+                self.ssl_verify = _config.get("global", "ssl_verify")
             except Exception:
                 pass
         except Exception:
             pass
         try:
-            self.ssl_verify = self._config.getboolean(self.gitlab_id, "ssl_verify")
+            self.ssl_verify = _config.getboolean(self.gitlab_id, "ssl_verify")
         except ValueError:
             # Value Error means the option exists but isn't a boolean.
             # Get as a string instead as it should then be a local path to a
             # CA bundle.
             try:
-                self.ssl_verify = self._config.get(self.gitlab_id, "ssl_verify")
+                self.ssl_verify = _config.get(self.gitlab_id, "ssl_verify")
             except Exception:
                 pass
         except Exception:
             pass
 
-        self.timeout = 60
         try:
-            self.timeout = self._config.getint("global", "timeout")
+            self.timeout = _config.getint("global", "timeout")
         except Exception:
             pass
         try:
-            self.timeout = self._config.getint(self.gitlab_id, "timeout")
+            self.timeout = _config.getint(self.gitlab_id, "timeout")
         except Exception:
             pass
 
-        self.private_token = None
         try:
-            self.private_token = self._config.get(self.gitlab_id, "private_token")
+            self.private_token = _config.get(self.gitlab_id, "private_token")
         except Exception:
             pass
 
-        self.oauth_token = None
         try:
-            self.oauth_token = self._config.get(self.gitlab_id, "oauth_token")
+            self.oauth_token = _config.get(self.gitlab_id, "oauth_token")
         except Exception:
             pass
 
-        self.job_token = None
         try:
-            self.job_token = self._config.get(self.gitlab_id, "job_token")
+            self.job_token = _config.get(self.gitlab_id, "job_token")
         except Exception:
             pass
 
-        self.http_username = None
-        self.http_password = None
         try:
-            self.http_username = self._config.get(self.gitlab_id, "http_username")
-            self.http_password = self._config.get(self.gitlab_id, "http_password")
+            self.http_username = _config.get(self.gitlab_id, "http_username")
+            self.http_password = _config.get(self.gitlab_id, "http_password")
         except Exception:
             pass
 
         self._get_values_from_helper()
 
-        self.api_version = "4"
         try:
-            self.api_version = self._config.get("global", "api_version")
+            self.api_version = _config.get("global", "api_version")
         except Exception:
             pass
         try:
-            self.api_version = self._config.get(self.gitlab_id, "api_version")
+            self.api_version = _config.get(self.gitlab_id, "api_version")
         except Exception:
             pass
         if self.api_version not in ("4",):
             raise GitlabDataError(f"Unsupported API version: {self.api_version}")
 
-        self.per_page = None
         for section in ["global", self.gitlab_id]:
             try:
-                self.per_page = self._config.getint(section, "per_page")
+                self.per_page = _config.getint(section, "per_page")
             except Exception:
                 pass
         if self.per_page is not None and not 0 <= self.per_page <= 100:
             raise GitlabDataError(f"Unsupported per_page number: {self.per_page}")
 
-        self.pagination = None
         try:
-            self.pagination = self._config.get(self.gitlab_id, "pagination")
+            self.pagination = _config.get(self.gitlab_id, "pagination")
         except Exception:
             pass
 
-        self.order_by = None
         try:
-            self.order_by = self._config.get(self.gitlab_id, "order_by")
+            self.order_by = _config.get(self.gitlab_id, "order_by")
         except Exception:
             pass
 
-        self.user_agent = USER_AGENT
         try:
-            self.user_agent = self._config.get("global", "user_agent")
+            self.user_agent = _config.get("global", "user_agent")
         except Exception:
             pass
         try:
-            self.user_agent = self._config.get(self.gitlab_id, "user_agent")
+            self.user_agent = _config.get(self.gitlab_id, "user_agent")
         except Exception:
             pass
 
-        self.retry_transient_errors = False
         try:
-            self.retry_transient_errors = self._config.getboolean(
+            self.retry_transient_errors = _config.getboolean(
                 "global", "retry_transient_errors"
             )
         except Exception:
             pass
         try:
-            self.retry_transient_errors = self._config.getboolean(
+            self.retry_transient_errors = _config.getboolean(
                 self.gitlab_id, "retry_transient_errors"
             )
         except Exception:
diff --git a/requirements-test.txt b/requirements-test.txt
@@ -1,6 +1,6 @@
 coverage
 httmock
-pytest
+pytest==6.2.5
 pytest-console-scripts==1.2.1
 pytest-cov
 responses
diff --git a/tests/functional/cli/test_cli.py b/tests/functional/cli/test_cli.py
@@ -1,8 +1,24 @@
 import json
 
+import pytest
+import responses
+
 from gitlab import __version__
 
 
+@pytest.fixture
+def resp_list_projects():
+    with responses.RequestsMock() as rsps:
+        rsps.add(
+            method=responses.GET,
+            url="https://gitlab.com/api/v4/projects",
+            json={"name": "name", "path": "test-path", "id": 1},
+            content_type="application/json",
+            status=200,
+        )
+        yield rsps
+
+
 def test_main_entrypoint(script_runner, gitlab_config):
     ret = script_runner.run("python", "-m", "gitlab", "--config-file", gitlab_config)
     assert ret.returncode == 2
@@ -13,6 +29,29 @@ def test_version(script_runner):
     assert ret.stdout.strip() == __version__
 
 
+@pytest.mark.script_launch_mode("inprocess")
+def test_defaults_to_gitlab_com(script_runner, resp_list_projects):
+    # Runs in-process to intercept requests to gitlab.com
+    ret = script_runner.run("gitlab", "project", "list")
+    assert ret.success
+    assert "id: 1" in ret.stdout
+
+
+def test_env_config_missing_file_raises(script_runner, monkeypatch):
+    monkeypatch.setenv("PYTHON_GITLAB_CFG", "non-existent")
+    ret = script_runner.run("gitlab", "project", "list")
+    assert not ret.success
+    assert ret.stderr.startswith("Cannot read config from PYTHON_GITLAB_CFG")
+
+
+def test_arg_config_missing_file_raises(script_runner):
+    ret = script_runner.run(
+        "gitlab", "--config-file", "non-existent", "project", "list"
+    )
+    assert not ret.success
+    assert ret.stderr.startswith("Cannot read config from file")
+
+
 def test_invalid_config(script_runner):
     ret = script_runner.run("gitlab", "--gitlab", "invalid")
     assert not ret.success
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
