feat(cli): do not require config file to run CLIfeat/cli-without-config-file

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.

6 files changed, 234 insertions, 116 deletions

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 66bf045..0b1fe78 100644
--- 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
index ea10f93..50fac6d 100644
--- 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
index 6c75d0a..154f063 100644
--- a/gitlab/config.py
+++ b/gitlab/config.py
@@ -20,20 +20,14 @@ import os
 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:"
@@ -41,6 +35,52 @@ 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 @@ class GitlabConfigParser(object):
         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
index 9f9df61..dd03716 100644
--- 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
index c4e76a7..2384563 100644
--- 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_get_project():
+    with responses.RequestsMock() as rsps:
+        rsps.add(
+            method=responses.GET,
+            url="https://gitlab.com/api/v4/projects/1",
+            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_get_project):
+    # Runs in-process to intercept requests to gitlab.com
+    ret = script_runner.run("gitlab", "project", "get", "--id", "1")
+    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
index ffd67c4..c589564 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -16,15 +16,15 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import io
-import os
 import sys
+from pathlib import Path
 from textwrap import dedent
 from unittest import mock
 
 import pytest
 
 import gitlab
-from gitlab import config
+from gitlab import config, const
 
 custom_user_agent = "my-package/1.0.0"
 
@@ -107,69 +107,96 @@ def global_and_gitlab_retry_transient_errors(
     retry_transient_errors={gitlab_value}"""
 
 
-@mock.patch.dict(os.environ, {"PYTHON_GITLAB_CFG": "/some/path"})
-def test_env_config_present():
-    assert ["/some/path"] == config._env_config()
+def _mock_nonexistent_file(*args, **kwargs):
+    raise OSError
 
 
-@mock.patch.dict(os.environ, {}, clear=True)
-def test_env_config_missing():
-    assert [] == config._env_config()
+def _mock_existent_file(path, *args, **kwargs):
+    return path
 
 
-@mock.patch("os.path.exists")
-def test_missing_config(path_exists):
-    path_exists.return_value = False
+@pytest.fixture
+def mock_clean_env(monkeypatch):
+    monkeypatch.delenv("PYTHON_GITLAB_CFG", raising=False)
+
+
+def test_env_config_missing_file_raises(monkeypatch):
+    monkeypatch.setenv("PYTHON_GITLAB_CFG", "/some/path")
     with pytest.raises(config.GitlabConfigMissingError):
-        config.GitlabConfigParser("test")
+        config._get_config_files()
+
+
+def test_env_config_not_defined_does_not_raise(mock_clean_env):
+    assert config._get_config_files() == []
+
+
+def test_default_config(mock_clean_env, monkeypatch):
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_nonexistent_file)
+        cp = config.GitlabConfigParser()
+
+    assert cp.gitlab_id is None
+    assert cp.http_username is None
+    assert cp.http_password is None
+    assert cp.job_token is None
+    assert cp.oauth_token is None
+    assert cp.private_token is None
+    assert cp.api_version == "4"
+    assert cp.order_by is None
+    assert cp.pagination is None
+    assert cp.per_page is None
+    assert cp.retry_transient_errors is False
+    assert cp.ssl_verify is True
+    assert cp.timeout == 60
+    assert cp.url == const.DEFAULT_URL
+    assert cp.user_agent == const.USER_AGENT
 
 
-@mock.patch("os.path.exists")
 @mock.patch("builtins.open")
-def test_invalid_id(m_open, path_exists):
+def test_invalid_id(m_open, mock_clean_env, monkeypatch):
     fd = io.StringIO(no_default_config)
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
-    path_exists.return_value = True
-    config.GitlabConfigParser("there")
-    with pytest.raises(config.GitlabIDError):
-        config.GitlabConfigParser()
-
-    fd = io.StringIO(valid_config)
-    fd.close = mock.Mock(return_value=None)
-    m_open.return_value = fd
-    with pytest.raises(config.GitlabDataError):
-        config.GitlabConfigParser(gitlab_id="not_there")
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        config.GitlabConfigParser("there")
+        with pytest.raises(config.GitlabIDError):
+            config.GitlabConfigParser()
+        fd = io.StringIO(valid_config)
+        fd.close = mock.Mock(return_value=None)
+        m_open.return_value = fd
+        with pytest.raises(config.GitlabDataError):
+            config.GitlabConfigParser(gitlab_id="not_there")
 
 
-@mock.patch("os.path.exists")
 @mock.patch("builtins.open")
-def test_invalid_data(m_open, path_exists):
+def test_invalid_data(m_open, monkeypatch):
     fd = io.StringIO(missing_attr_config)
     fd.close = mock.Mock(return_value=None, side_effect=lambda: fd.seek(0))
     m_open.return_value = fd
-    path_exists.return_value = True
 
-    config.GitlabConfigParser("one")
-    config.GitlabConfigParser("one")
-    with pytest.raises(config.GitlabDataError):
-        config.GitlabConfigParser(gitlab_id="two")
-    with pytest.raises(config.GitlabDataError):
-        config.GitlabConfigParser(gitlab_id="three")
-    with pytest.raises(config.GitlabDataError) as emgr:
-        config.GitlabConfigParser("four")
-    assert "Unsupported per_page number: 200" == emgr.value.args[0]
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        config.GitlabConfigParser("one")
+        config.GitlabConfigParser("one")
+        with pytest.raises(config.GitlabDataError):
+            config.GitlabConfigParser(gitlab_id="two")
+        with pytest.raises(config.GitlabDataError):
+            config.GitlabConfigParser(gitlab_id="three")
+        with pytest.raises(config.GitlabDataError) as emgr:
+            config.GitlabConfigParser("four")
+        assert "Unsupported per_page number: 200" == emgr.value.args[0]
 
 
-@mock.patch("os.path.exists")
 @mock.patch("builtins.open")
-def test_valid_data(m_open, path_exists):
+def test_valid_data(m_open, monkeypatch):
     fd = io.StringIO(valid_config)
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
-    path_exists.return_value = True
 
-    cp = config.GitlabConfigParser()
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser()
     assert "one" == cp.gitlab_id
     assert "http://one.url" == cp.url
     assert "ABCDEF" == cp.private_token
@@ -181,7 +208,9 @@ def test_valid_data(m_open, path_exists):
     fd = io.StringIO(valid_config)
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
-    cp = config.GitlabConfigParser(gitlab_id="two")
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser(gitlab_id="two")
     assert "two" == cp.gitlab_id
     assert "https://two.url" == cp.url
     assert "GHIJKL" == cp.private_token
@@ -192,7 +221,9 @@ def test_valid_data(m_open, path_exists):
     fd = io.StringIO(valid_config)
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
-    cp = config.GitlabConfigParser(gitlab_id="three")
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser(gitlab_id="three")
     assert "three" == cp.gitlab_id
     assert "https://three.url" == cp.url
     assert "MNOPQR" == cp.private_token
@@ -204,7 +235,9 @@ def test_valid_data(m_open, path_exists):
     fd = io.StringIO(valid_config)
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
-    cp = config.GitlabConfigParser(gitlab_id="four")
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser(gitlab_id="four")
     assert "four" == cp.gitlab_id
     assert "https://four.url" == cp.url
     assert cp.private_token is None
@@ -213,10 +246,9 @@ def test_valid_data(m_open, path_exists):
     assert cp.ssl_verify is True
 
 
-@mock.patch("os.path.exists")
 @mock.patch("builtins.open")
 @pytest.mark.skipif(sys.platform.startswith("win"), reason="Not supported on Windows")
-def test_data_from_helper(m_open, path_exists, tmp_path):
+def test_data_from_helper(m_open, monkeypatch, tmp_path):
     helper = tmp_path / "helper.sh"
     helper.write_text(
         dedent(
@@ -243,14 +275,15 @@ def test_data_from_helper(m_open, path_exists, tmp_path):
 
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
-    cp = config.GitlabConfigParser(gitlab_id="helper")
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser(gitlab_id="helper")
     assert "helper" == cp.gitlab_id
     assert "https://helper.url" == cp.url
     assert cp.private_token is None
     assert "secret" == cp.oauth_token
 
 
-@mock.patch("os.path.exists")
 @mock.patch("builtins.open")
 @pytest.mark.parametrize(
     "config_string,expected_agent",
@@ -259,16 +292,17 @@ def test_data_from_helper(m_open, path_exists, tmp_path):
         (custom_user_agent_config, custom_user_agent),
     ],
 )
-def test_config_user_agent(m_open, path_exists, config_string, expected_agent):
+def test_config_user_agent(m_open, monkeypatch, config_string, expected_agent):
     fd = io.StringIO(config_string)
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
 
-    cp = config.GitlabConfigParser()
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser()
     assert cp.user_agent == expected_agent
 
 
-@mock.patch("os.path.exists")
 @mock.patch("builtins.open")
 @pytest.mark.parametrize(
     "config_string,expected",
@@ -303,11 +337,13 @@ def test_config_user_agent(m_open, path_exists, config_string, expected_agent):
     ],
 )
 def test_config_retry_transient_errors_when_global_config_is_set(
-    m_open, path_exists, config_string, expected
+    m_open, monkeypatch, config_string, expected
 ):
     fd = io.StringIO(config_string)
     fd.close = mock.Mock(return_value=None)
     m_open.return_value = fd
 
-    cp = config.GitlabConfigParser()
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser()
     assert cp.retry_transient_errors == expected