From fb8be69ea23488b87c020114f7a88ca2cbbd6ea6 Mon Sep 17 00:00:00 2001 From: atttiwari Date: Thu, 4 Jun 2026 16:08:07 +0530 Subject: [PATCH 1/4] SAC-31205: Added config to add on-premises base urls --- CHANGELOG.md | 4 +++ README.md | 14 ++++++++- setup.py | 6 ++-- tap_gitlab/client.py | 28 +++++++++++++----- tests/unittests/test_client.py | 53 ++++++++++++++++++++++++++++++++++ 5 files changed, 94 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 401a509..c4c257c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog + +# 1.1.0 + * Add optional `gitlab_url` config field to support on-premises GitLab instances; defaults to `https://gitlab.com`. [#51](https://github.com/singer-io/tap-gitlab/pull/51) + # 1.0.1 * Bump requests to 2.33.0 for security updates [#50](https://github.com/singer-io/tap-gitlab/pull/50) diff --git a/README.md b/README.md index eed8982..5cc53ea 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ This tap: Create a JSON file called `config.json` containing: - Access token you just created - - API URL for your GitLab account. If you are using the public gitlab.com this will be `https://gitlab.com/api/v3` + - *(Optional)* `gitlab_url` — the base URL of your GitLab instance. Defaults to `https://gitlab.com`. Set this when connecting to a **self-hosted / on-premises** GitLab server (e.g. `https://gitlab.mycompany.com`). The tap automatically appends `/api/v4` to whatever URL you provide. - Groups to track (space separated) - Projects to track (space separated) @@ -48,6 +48,7 @@ This tap: - 'groups' contains space separated list of groups id. - 'projects' contains space separated list of projects id. + GitLab Cloud (default): ```json { "private_token": "your-access-token", @@ -57,6 +58,17 @@ This tap: } ``` + Self-hosted / on-premises GitLab: + ```json + { + "private_token": "your-access-token", + "gitlab_url": "https://gitlab.mycompany.com", + "groups": "myorg mygroup", + "projects": "myorg/repo-a myorg/repo-b", + "start_date": "2018-01-01T00:00:00Z" + } + ``` + 4. [Optional] Create the initial state file You can provide JSON file that contains a date for the API endpoints diff --git a/setup.py b/setup.py index 65afcb4..543e125 100644 --- a/setup.py +++ b/setup.py @@ -5,14 +5,14 @@ setup( name='tap-gitlab', - version='1.0.1', + version='1.1.0', description='Singer.io tap for extracting data from the GitLab API', author='Stitch', url='https://singer.io', classifiers=['Programming Language :: Python :: 3 :: Only'], install_requires=[ - 'singer-python==6.1.1', - 'requests==2.33.0', + 'singer-python==6.8.0', + 'requests==2.34.2', 'backoff==2.2.1' ], entry_points=''' diff --git a/tap_gitlab/client.py b/tap_gitlab/client.py index 7d46b1b..a6faa62 100644 --- a/tap_gitlab/client.py +++ b/tap_gitlab/client.py @@ -1,4 +1,5 @@ from typing import Any, Dict, Mapping, Optional, Tuple +from urllib.parse import urlparse import backoff import requests @@ -75,7 +76,11 @@ class Client: def __init__(self, config: Mapping[str, Any]) -> None: self.config = config self._session = session() - self.base_url = "https://gitlab.com/api/v4" + + gitlab_url = config.get("gitlab_url", "https://gitlab.com").rstrip("/") + if not urlparse(gitlab_url).scheme: + gitlab_url = f"https://{gitlab_url}" + self.base_url = f"{gitlab_url}/api/v4" config_request_timeout = config.get("request_timeout") self.request_timeout = float(config_request_timeout) if config_request_timeout else REQUEST_TIMEOUT @@ -93,12 +98,21 @@ def check_api_credentials(self) -> None: params = {'private_token': self.config.get("private_token")} endpoint = f"{self.base_url}/user" - response = self._session.get( - endpoint, - params=params, - headers=headers, - timeout=self.request_timeout - ) + try: + response = self._session.get( + endpoint, + params=params, + headers=headers, + timeout=self.request_timeout + ) + except ConnectionError as e: + gitlab_url = self.config.get("gitlab_url", "https://gitlab.com") + raise ConnectionError( + f"Unable to reach GitLab at '{gitlab_url}'. " + "Please verify that 'gitlab_url' in your config is correct and the host is reachable. " + f"Original error: {e}" + ) from e + raise_for_error(response) user_data = response.json() diff --git a/tests/unittests/test_client.py b/tests/unittests/test_client.py index 6e613c5..c8737e6 100644 --- a/tests/unittests/test_client.py +++ b/tests/unittests/test_client.py @@ -100,3 +100,56 @@ def test_generic_http_error(self, mock_request, mock_auth, mock_check_creds, moc with self.assertRaises(Error): with Client(config) as client: client.get(endpoint="dummy", params={}, headers={}) + + +class TestClientBaseUrl(unittest.TestCase): + + def test_default_base_url_uses_gitlab_cloud(self): + """When gitlab_url is absent the client targets gitlab.com.""" + config = {"private_token": "dummy_token"} + client = Client(config) + self.assertEqual(client.base_url, "https://gitlab.com/api/v4") + + def test_custom_gitlab_url_sets_base_url(self): + """When gitlab_url is set the client targets the on-prem instance.""" + config = { + "private_token": "dummy_token", + "gitlab_url": "https://gitlab.mycompany.com", + } + client = Client(config) + self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") + + def test_trailing_slash_in_gitlab_url_is_stripped(self): + """Trailing slashes in gitlab_url must not produce a double-slash in base_url.""" + config = { + "private_token": "dummy_token", + "gitlab_url": "https://gitlab.mycompany.com/", + } + client = Client(config) + self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") + + def test_gitlab_url_without_scheme_defaults_to_https(self): + """A gitlab_url with no scheme (e.g. 'gitlab.mycompany.com') must get https:// prepended.""" + config = { + "private_token": "dummy_token", + "gitlab_url": "gitlab.mycompany.com", + } + client = Client(config) + self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") + + +class TestCheckApiCredentials(unittest.TestCase): + + @patch("requests.Session.get", side_effect=ConnectionError("Failed to resolve 'test.gitlab.com'")) + def test_unreachable_host_raises_friendly_connection_error(self, mock_get): + """A DNS/network failure in check_api_credentials raises a descriptive ConnectionError.""" + config = { + "private_token": "dummy_token", + "gitlab_url": "https://test.gitlab.com", + } + client = Client(config) + with self.assertRaises(ConnectionError) as ctx: + client.check_api_credentials() + self.assertIn("Unable to reach GitLab", str(ctx.exception)) + self.assertIn("test.gitlab.com", str(ctx.exception)) + self.assertIn("gitlab_url", str(ctx.exception)) From a84bad9a054bb2c0a7bd574c3d9d07e0983bafad Mon Sep 17 00:00:00 2001 From: atttiwari Date: Thu, 4 Jun 2026 17:12:16 +0530 Subject: [PATCH 2/4] SAC-31205: addressed minor review comments --- CHANGELOG.md | 3 ++- README.md | 4 ++-- tap_gitlab/client.py | 14 +++++++------- tests/unittests/test_client.py | 24 ++++++++++++------------ 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4c257c..a81224d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,8 @@ # 1.1.0 - * Add optional `gitlab_url` config field to support on-premises GitLab instances; defaults to `https://gitlab.com`. [#51](https://github.com/singer-io/tap-gitlab/pull/51) + * Add optional `api_url` config field to support on-premises GitLab instances; defaults to `https://gitlab.com`. [#51](https://github.com/singer-io/tap-gitlab/pull/51) + * Bump singer-python to 6.8.0. # 1.0.1 * Bump requests to 2.33.0 for security updates [#50](https://github.com/singer-io/tap-gitlab/pull/50) diff --git a/README.md b/README.md index 5cc53ea..3483262 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ This tap: Create a JSON file called `config.json` containing: - Access token you just created - - *(Optional)* `gitlab_url` — the base URL of your GitLab instance. Defaults to `https://gitlab.com`. Set this when connecting to a **self-hosted / on-premises** GitLab server (e.g. `https://gitlab.mycompany.com`). The tap automatically appends `/api/v4` to whatever URL you provide. + - *(Optional)* `api_url` — the base URL of your GitLab instance. Defaults to `https://gitlab.com`. Set this when connecting to a **self-hosted / on-premises** GitLab server (e.g. `https://gitlab.mycompany.com`). The tap automatically appends `/api/v4` to whatever URL you provide. - Groups to track (space separated) - Projects to track (space separated) @@ -62,7 +62,7 @@ This tap: ```json { "private_token": "your-access-token", - "gitlab_url": "https://gitlab.mycompany.com", + "api_url": "https://gitlab.mycompany.com", "groups": "myorg mygroup", "projects": "myorg/repo-a myorg/repo-b", "start_date": "2018-01-01T00:00:00Z" diff --git a/tap_gitlab/client.py b/tap_gitlab/client.py index a6faa62..f991c85 100644 --- a/tap_gitlab/client.py +++ b/tap_gitlab/client.py @@ -77,10 +77,10 @@ def __init__(self, config: Mapping[str, Any]) -> None: self.config = config self._session = session() - gitlab_url = config.get("gitlab_url", "https://gitlab.com").rstrip("/") - if not urlparse(gitlab_url).scheme: - gitlab_url = f"https://{gitlab_url}" - self.base_url = f"{gitlab_url}/api/v4" + api_url = config.get("api_url", "https://gitlab.com").rstrip("/") + if not urlparse(api_url).scheme: + api_url = f"https://{api_url}" + self.base_url = f"{api_url}/api/v4" config_request_timeout = config.get("request_timeout") self.request_timeout = float(config_request_timeout) if config_request_timeout else REQUEST_TIMEOUT @@ -106,10 +106,10 @@ def check_api_credentials(self) -> None: timeout=self.request_timeout ) except ConnectionError as e: - gitlab_url = self.config.get("gitlab_url", "https://gitlab.com") + api_url = self.config.get("api_url", "https://gitlab.com") raise ConnectionError( - f"Unable to reach GitLab at '{gitlab_url}'. " - "Please verify that 'gitlab_url' in your config is correct and the host is reachable. " + f"Unable to reach GitLab at '{api_url}'. " + "Please verify that 'api_url' in your config is correct and the host is reachable. " f"Original error: {e}" ) from e diff --git a/tests/unittests/test_client.py b/tests/unittests/test_client.py index c8737e6..5d0671b 100644 --- a/tests/unittests/test_client.py +++ b/tests/unittests/test_client.py @@ -105,34 +105,34 @@ def test_generic_http_error(self, mock_request, mock_auth, mock_check_creds, moc class TestClientBaseUrl(unittest.TestCase): def test_default_base_url_uses_gitlab_cloud(self): - """When gitlab_url is absent the client targets gitlab.com.""" + """When api_url is absent the client targets gitlab.com.""" config = {"private_token": "dummy_token"} client = Client(config) self.assertEqual(client.base_url, "https://gitlab.com/api/v4") - def test_custom_gitlab_url_sets_base_url(self): - """When gitlab_url is set the client targets the on-prem instance.""" + def test_custom_api_url_sets_base_url(self): + """When api_url is set the client targets the on-prem instance.""" config = { "private_token": "dummy_token", - "gitlab_url": "https://gitlab.mycompany.com", + "api_url": "https://gitlab.mycompany.com", } client = Client(config) self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") - def test_trailing_slash_in_gitlab_url_is_stripped(self): - """Trailing slashes in gitlab_url must not produce a double-slash in base_url.""" + def test_trailing_slash_in_api_url_is_stripped(self): + """Trailing slashes in api_url must not produce a double-slash in base_url.""" config = { "private_token": "dummy_token", - "gitlab_url": "https://gitlab.mycompany.com/", + "api_url": "https://gitlab.mycompany.com/", } client = Client(config) self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") - def test_gitlab_url_without_scheme_defaults_to_https(self): - """A gitlab_url with no scheme (e.g. 'gitlab.mycompany.com') must get https:// prepended.""" + def test_api_url_without_scheme_defaults_to_https(self): + """An api_url with no scheme (e.g. 'gitlab.mycompany.com') must get https:// prepended.""" config = { "private_token": "dummy_token", - "gitlab_url": "gitlab.mycompany.com", + "api_url": "gitlab.mycompany.com", } client = Client(config) self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") @@ -145,11 +145,11 @@ def test_unreachable_host_raises_friendly_connection_error(self, mock_get): """A DNS/network failure in check_api_credentials raises a descriptive ConnectionError.""" config = { "private_token": "dummy_token", - "gitlab_url": "https://test.gitlab.com", + "api_url": "https://test.gitlab.com", } client = Client(config) with self.assertRaises(ConnectionError) as ctx: client.check_api_credentials() self.assertIn("Unable to reach GitLab", str(ctx.exception)) self.assertIn("test.gitlab.com", str(ctx.exception)) - self.assertIn("gitlab_url", str(ctx.exception)) + self.assertIn("api_url", str(ctx.exception)) From 0391413d9e383f5eb4adf9b103095059650f3a64 Mon Sep 17 00:00:00 2001 From: atttiwari Date: Fri, 5 Jun 2026 12:55:27 +0530 Subject: [PATCH 3/4] SAC-31205: addressed copilot review suggestions and fix integration tests --- CHANGELOG.md | 2 +- README.md | 2 +- tap_gitlab/client.py | 7 +++++-- tests/test_all_fields.py | 6 ------ tests/unittests/test_client.py | 18 ++++++++++++++++++ 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a81224d..522064b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ # 1.1.0 * Add optional `api_url` config field to support on-premises GitLab instances; defaults to `https://gitlab.com`. [#51](https://github.com/singer-io/tap-gitlab/pull/51) - * Bump singer-python to 6.8.0. + * Bump singer-python to 6.8.0 and requests to 2.34.2. # 1.0.1 * Bump requests to 2.33.0 for security updates [#50](https://github.com/singer-io/tap-gitlab/pull/50) diff --git a/README.md b/README.md index 3483262..103cdc8 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,7 @@ This tap: Create a JSON file called `config.json` containing: - Access token you just created - - *(Optional)* `api_url` — the base URL of your GitLab instance. Defaults to `https://gitlab.com`. Set this when connecting to a **self-hosted / on-premises** GitLab server (e.g. `https://gitlab.mycompany.com`). The tap automatically appends `/api/v4` to whatever URL you provide. + - *(Optional)* `api_url` — the base URL of your GitLab instance, **without** the `/api/v4` path (e.g. `https://gitlab.mycompany.com`). Defaults to `https://gitlab.com`. Set this only when connecting to a **self-hosted / on-premises** GitLab server. The tap appends `/api/v4` automatically. - Groups to track (space separated) - Projects to track (space separated) diff --git a/tap_gitlab/client.py b/tap_gitlab/client.py index f991c85..0f1b5c6 100644 --- a/tap_gitlab/client.py +++ b/tap_gitlab/client.py @@ -1,5 +1,5 @@ from typing import Any, Dict, Mapping, Optional, Tuple -from urllib.parse import urlparse +import re import backoff import requests @@ -78,8 +78,11 @@ def __init__(self, config: Mapping[str, Any]) -> None: self._session = session() api_url = config.get("api_url", "https://gitlab.com").rstrip("/") - if not urlparse(api_url).scheme: + if not api_url.startswith(("http://", "https://")): api_url = f"https://{api_url}" + # Strip any existing /api/vN suffix so both 'https://gitlab.com' and + # 'https://gitlab.com/api/v4' are accepted without producing a double suffix. + api_url = re.sub(r"/api/v\d+$", "", api_url) self.base_url = f"{api_url}/api/v4" config_request_timeout = config.get("request_timeout") diff --git a/tests/test_all_fields.py b/tests/test_all_fields.py index 769bf4e..4d3d1bc 100644 --- a/tests/test_all_fields.py +++ b/tests/test_all_fields.py @@ -9,7 +9,6 @@ class AllFields(AllFieldsTest, BaseTest): MISSING_FIELDS = { "groups": { "repository_storage", - "ip_restriction_ranges" }, "issues": { 'assignee_id', @@ -29,11 +28,6 @@ class AllFields(AllFieldsTest, BaseTest): 'owner', 'repository_storage', 'mirror_trigger_builds', - 'ci_restrict_pipeline_cancellation_role', - 'approvals_before_merge', - 'mirror', - 'allow_pipeline_trigger_approve_deployment', - 'secret_push_protection_enabled', } } diff --git a/tests/unittests/test_client.py b/tests/unittests/test_client.py index 5d0671b..cd797c3 100644 --- a/tests/unittests/test_client.py +++ b/tests/unittests/test_client.py @@ -137,6 +137,24 @@ def test_api_url_without_scheme_defaults_to_https(self): client = Client(config) self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") + def test_api_url_with_port_and_no_scheme_gets_https(self): + """host:port without a scheme must not be mis-detected by urlparse and must get https://.""" + config = { + "private_token": "dummy_token", + "api_url": "gitlab.mycompany.com:8443", + } + client = Client(config) + self.assertEqual(client.base_url, "https://gitlab.mycompany.com:8443/api/v4") + + def test_api_url_with_existing_api_v4_suffix_is_not_doubled(self): + """Providing a full API base URL must not produce /api/v4/api/v4.""" + config = { + "private_token": "dummy_token", + "api_url": "https://gitlab.mycompany.com/api/v4", + } + client = Client(config) + self.assertEqual(client.base_url, "https://gitlab.mycompany.com/api/v4") + class TestCheckApiCredentials(unittest.TestCase): From 14cd6a2aa78aefe5bf217fe96f411de3c691fca4 Mon Sep 17 00:00:00 2001 From: atttiwari Date: Fri, 5 Jun 2026 13:43:26 +0530 Subject: [PATCH 4/4] SAC-31205: addressed review comments --- tap_gitlab/client.py | 8 ++++---- tests/unittests/test_client.py | 22 ++++++++++++++++++---- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/tap_gitlab/client.py b/tap_gitlab/client.py index 0f1b5c6..eb6b8e3 100644 --- a/tap_gitlab/client.py +++ b/tap_gitlab/client.py @@ -77,7 +77,8 @@ def __init__(self, config: Mapping[str, Any]) -> None: self.config = config self._session = session() - api_url = config.get("api_url", "https://gitlab.com").rstrip("/") + api_url = (config.get("api_url") or "").strip() or "https://gitlab.com" + api_url = api_url.rstrip("/") if not api_url.startswith(("http://", "https://")): api_url = f"https://{api_url}" # Strip any existing /api/vN suffix so both 'https://gitlab.com' and @@ -109,11 +110,10 @@ def check_api_credentials(self) -> None: timeout=self.request_timeout ) except ConnectionError as e: - api_url = self.config.get("api_url", "https://gitlab.com") raise ConnectionError( - f"Unable to reach GitLab at '{api_url}'. " + f"Unable to reach GitLab at '{self.base_url}'. " "Please verify that 'api_url' in your config is correct and the host is reachable. " - f"Original error: {e}" + f"({type(e).__name__})" ) from e raise_for_error(response) diff --git a/tests/unittests/test_client.py b/tests/unittests/test_client.py index cd797c3..72ee810 100644 --- a/tests/unittests/test_client.py +++ b/tests/unittests/test_client.py @@ -110,6 +110,14 @@ def test_default_base_url_uses_gitlab_cloud(self): client = Client(config) self.assertEqual(client.base_url, "https://gitlab.com/api/v4") + def test_empty_api_url_falls_back_to_gitlab_cloud(self): + """An empty or whitespace-only api_url must fall back to gitlab.com.""" + for value in ["", " ", None]: + with self.subTest(api_url=value): + config = {"private_token": "dummy_token", "api_url": value} + client = Client(config) + self.assertEqual(client.base_url, "https://gitlab.com/api/v4") + def test_custom_api_url_sets_base_url(self): """When api_url is set the client targets the on-prem instance.""" config = { @@ -160,7 +168,8 @@ class TestCheckApiCredentials(unittest.TestCase): @patch("requests.Session.get", side_effect=ConnectionError("Failed to resolve 'test.gitlab.com'")) def test_unreachable_host_raises_friendly_connection_error(self, mock_get): - """A DNS/network failure in check_api_credentials raises a descriptive ConnectionError.""" + """A DNS/network failure in check_api_credentials raises a descriptive ConnectionError + that does not leak the raw urllib3 message (which may contain the private_token URL).""" config = { "private_token": "dummy_token", "api_url": "https://test.gitlab.com", @@ -168,6 +177,11 @@ def test_unreachable_host_raises_friendly_connection_error(self, mock_get): client = Client(config) with self.assertRaises(ConnectionError) as ctx: client.check_api_credentials() - self.assertIn("Unable to reach GitLab", str(ctx.exception)) - self.assertIn("test.gitlab.com", str(ctx.exception)) - self.assertIn("api_url", str(ctx.exception)) + msg = str(ctx.exception) + self.assertIn("Unable to reach GitLab", msg) + # base_url (no token) is shown, not the raw original error + self.assertIn("test.gitlab.com", msg) + self.assertIn("api_url", msg) + # Raw urllib3 message must not be included to avoid token leakage + self.assertNotIn("Failed to resolve", msg) + self.assertNotIn("private_token", msg)