Refactor GitLab registry client tests and improve API base URL extrac…

[jnathangreeg]

…tion

• Updated test cases in gitlab_test.go to use example.com for consistency.
• Introduced a new helper function, hostLooksLikeGitLab, to determine if a host resembles a GitLab instance.
• Removed outdated comments in gitlab.go for clarity and maintainability.

Summary by CodeRabbit

• Refactor

  • Improved GitLab API URL handling logic for better code maintainability.

• Tests

  • Expanded test coverage for GitLab API base URL detection, including edge cases for on-premises instances, hyphenated hostnames, port preservation, and protocol normalization.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The refactoring extracts GitLab hostname detection logic into a reusable hostLooksLikeGitLab() helper function, replacing inline hostname-prefix checks. Corresponding test cases are expanded with new hostname variants and adjusted expected outputs to validate the updated behavior.

Changes

Cohort / File(s)	Summary
Implementation registryclients/gitlab.go	Introduced hostLooksLikeGitLab() helper function to determine GitLab-like hosts. Reworked API base URL extraction to use this helper instead of direct hostname checks. Removed inline comments describing fallback behavior without altering control flow.
Tests registryclients/gitlab_test.go	Expanded test cases for GitLab API base URL computation with new hostname variants (hyphenated hosts, registry-prefixed hosts, ".gitlab." subdomains, mixed-case names, port preservation, HTTP-to-HTTPS normalization). Updated expected outputs to match new test hostnames and schemes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A helper born to spot GitLab's name,
Through hostnames we now search the same.
No more repetition cluttering the code—
Tests bloom with cases down the road!
Refactored clean, the logic shines,
GitLab detection draws new lines. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main changes: refactoring GitLab registry client tests and improving API base URL extraction through the new hostLooksLikeGitLab helper function.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-gitlab-url-trim

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

registryclients/gitlab.go (2)

113-117: ⚠️ Potential issue | 🟠 Major

Pre-existing: defer resp.Body.Close() inside a loop leaks response bodies.

Each iteration defers the close, but all defers only execute when the enclosing function returns. In a paginated loop, this means all response bodies remain open until the loop completes. Extract the loop body into a helper function, or close the body explicitly before continuing.

Proposed fix

 		resp, err := httpClient.Do(req)
 		if err != nil {
 			return nil, err
 		}
-		defer resp.Body.Close()
 
 		if resp.StatusCode != http.StatusOK {
 			body, _ := io.ReadAll(resp.Body)
+			resp.Body.Close()
 			return nil, fmt.Errorf("GitLab API error (status %d): %s", resp.StatusCode, string(body))
 		}
 
 		var projects []gitLabProject
 		if err := json.NewDecoder(resp.Body).Decode(&projects); err != nil {
+			resp.Body.Close()
 			return nil, fmt.Errorf("failed to decode projects: %w", err)
 		}
+		resp.Body.Close()

Or more idiomatically, extract the per-page fetch into a separate function so defer works as expected.

---

65-86: ⚠️ Potential issue | 🟡 Minor

getGitLabAPIBaseURL always emits https:// — HTTP-origin registries will silently upgrade.

The method strips both http:// and https:// schemes and unconditionally reconstructs the URL with https://. If a user's on-prem GitLab is HTTP-only (common in air-gapped or dev environments), the API calls will fail. This is the existing behavior, but with the expanded on-prem hostname support in this PR, it becomes more relevant.

Consider preserving the original scheme when it was explicitly http://.

registryclients/gitlab_test.go (1)

87-100: ⚠️ Potential issue | 🟡 Minor

IP-address test cases produce invalid hostnames like gitlab.192.168.1.100.

Prepending gitlab. to an IP address creates an invalid hostname that will never resolve. This is the existing behavior (these test cases aren't new), but now that hostLooksLikeGitLab is being introduced as a dedicated helper, it might be worth detecting IP addresses and handling them differently (e.g., skipping the gitlab. prepend or returning an error).

🧹 Nitpick comments (1)

registryclients/gitlab.go (1)

88-94: hostLooksLikeGitLab uses a very broad substring match.

strings.Contains(host, "gitlab") will match any host with "gitlab" anywhere in its name, including unintended cases like "notgitlabatall.example.com" or a domain where "gitlab" appears in an unrelated context. A slightly more structured check (e.g., verifying it appears as a domain label boundary using .gitlab., gitlab., gitlab-, or equals "gitlab") would reduce false positives while still covering on-prem variants.

That said, the current test suite covers the intended cases well, so this is a low-risk observation.

[Copilot]

Pull request overview

This PR refactors how the GitLab registry client decides whether to prepend gitlab. when deriving the GitLab API base URL from a configured registry URL, and expands/normalizes the related test cases.

Changes:

• Updated gitlab_test.go test inputs to use example.com domains and added coverage for more hostname variants (e.g., gitlab-*, .gitlab., -gitlab.).
• Introduced hostLooksLikeGitLab to decide when to treat a host as an existing GitLab instance vs. prepending gitlab..
• Removed several inline comments in gitlab.go.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
registryclients/gitlab_test.go	Normalizes domains to example.com and adds many new base-URL derivation test cases.
registryclients/gitlab.go	Replaces the previous gitlab. prefix check with hostLooksLikeGitLab and removes some comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.