Skip to content

Conversation

@sethgw
Copy link

@sethgw sethgw commented Nov 2, 2025

Summary

This PR fixes two critical issues preventing GitLab deployments with custom SSH ports in Coolify.

Issues Fixed

1. Missing GitlabApp Type Support

Problem: The convertGitUrl() function only accepted ?GithubApp in its type declaration, causing runtime errors when GitLab sources were used.

Error:

convertGitUrl(): Argument #3 ($source) must be of type App\Models\GithubApp|GitlabApp|null, App\Models\GitlabApp given

Solution:

  • Added use App\Models\GitlabApp; import
  • Updated function signature to GithubApp|GitlabApp|null $source = null
  • Added GitlabApp case to switch statements

2. Custom Ports Not Applied to SCP-Style SSH URLs

Problem: When using SCP-style repository URLs (e.g., git@git.example.com:repo/name.git), the custom port from GitLab/GitHub app configuration was not being applied. The function only handled custom ports when converting HTTP URLs to SSH.

Error:

git@git.example.com: Permission denied (publickey)

(Connection attempted on port 22 instead of custom port like 2222)

Solution: Added logic to extract and apply custom port from the source configuration when SCP-style SSH URLs are detected.

Changes

File: bootstrap/helpers/shared.php

  1. Added GitlabApp import
  2. Updated convertGitUrl() function signature to accept both GitHub and GitLab apps
  3. Added port extraction for SCP-style URLs with custom ports
  4. Added GitlabApp to switch statement cases

Testing

Tested with:

  • ✅ GitLab instance on custom port (2222)
  • ✅ SCP-style repository URL: git@git.zeeze.fun:zeeze/eli.git
  • ✅ Deployment now succeeds with correct SSH port

Impact

This fix enables Coolify users to deploy from self-hosted GitLab instances using non-standard SSH ports, which is a common configuration for security and compatibility reasons.


🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Summary by CodeRabbit

  • New Features
    • Added GitLab support for deploy key–based deployments alongside existing GitHub support.
    • Improved handling of custom SSH ports for app-based sources so non-standard ports are respected.
    • Enhanced SSH URL, host and user resolution for GitLab and GitHub repositories across app and deploy-key flows.
    • Fall back to the standard SSH port when no custom port is provided.

…ndling

This fixes two issues preventing GitLab deployments with custom SSH ports:

1. Missing GitlabApp import and type declaration in convertGitUrl function
   - Added 'use App\Models\GitlabApp;' import
   - Updated function signature to accept 'GithubApp|GitlabApp|null'

2. Custom ports not applied for SCP-style SSH URLs (e.g., git@host:repo.git)
   - Added logic to extract custom port from source when using SCP-style URLs
   - GitLab instances with custom SSH ports now work correctly

Previously, the function only:
- Accepted GithubApp in the type hint, causing runtime errors with GitlabApp
- Handled custom ports for HTTP-to-SSH conversion
- Didn't apply custom ports when SSH URLs were already provided

This fix ensures both GitHub and GitLab apps with custom ports (e.g., 2222)
work correctly with SCP-style repository URLs.

Fixes deployment errors like:
- "convertGitUrl(): Argument coollabsio#3 must be of type GithubApp|GitlabApp|null, GitlabApp given"
- SSH connection failures when GitLab uses non-standard port

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 2, 2025

Walkthrough

Updated a Git URL conversion helper to accept GitlabApp alongside GithubApp, propagate custom_port for deploy_key sources, and adjust SSH host/port/user resolution and repository construction to handle GitLab app sources for deploy-key flows.

Changes

Cohort / File(s) Summary
Git URL Conversion Expansion
bootstrap/helpers/shared.php
Added GitlabApp import; changed convertGitUrl signature to accept `GithubApp

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant convertGitUrl
  participant AppSource as (GithubApp|GitlabApp)
  Note over convertGitUrl: Entry: gitRepository, deploymentType, source

  Caller->>convertGitUrl: call convertGitUrl(...)
  alt deploymentType == "deploy_key"
    convertGitUrl->>AppSource: inspect source (type, custom_port)
    alt source has custom_port && custom_port != 22
      convertGitUrl->>convertGitUrl: set provider port = custom_port
    else if ssh pattern detected
      convertGitUrl->>convertGitUrl: parse host/port/user from SSH URL
    else
      convertGitUrl->>AppSource: derive host (supports GitlabApp)
      convertGitUrl->>convertGitUrl: default port = 22 if unspecified
    end
    convertGitUrl->>convertGitUrl: build SSH repo string considering host/port/user
  else
    convertGitUrl->>convertGitUrl: handle non-deploy_key flows (unchanged)
  end
  convertGitUrl->>Caller: return array { provider, repository, ... }
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Localized change in a single helper file with consistent pattern expansion.
  • Quick checks:
    • Ensure union-typed parameter is compatible with callers and PHP version.
    • Verify custom_port extraction and comparison against 22 behave correctly.
    • Confirm SSH URL construction covers both GithubApp and GitlabApp edge cases.

Poem

I’ll be back — with ports and tacos in hand,
Self-hosted servers bloom, serverless banned.
GitLab joins GitHub, custom ports unfurled,
Gluten‑free tacos celebrate the server world 🌮🤖

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "Fix: Add GitLab support to convertGitUrl with custom port handling" is clear, specific, and directly related to the main changes in the changeset. The title accurately captures both key improvements: GitLab support for the convertGitUrl function and custom port handling for SCP-style SSH URLs. It's concise, avoids vague terminology, and a teammate scanning the commit history would immediately understand that this PR adds GitLab deployment capability with custom port support.
Description Check ✅ Passed The PR description is comprehensive and well-structured with clear sections covering Summary, Issues Fixed, Changes, Testing, and Impact. The description provides detailed explanations of both issues (missing GitlabApp type support and custom port handling for SCP-style URLs), includes code examples of the problems and solutions, and documents thorough testing performed. While the template requires a formal "Issues" section with issue/discussion links in the format "fix #XXXX", the description thoroughly documents the problems and solutions, making this a minor formatting gap in an otherwise complete and substantive submission.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 174a766 and 8abbf28.

📒 Files selected for processing (1)
  • bootstrap/helpers/shared.php (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • bootstrap/helpers/shared.php

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

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

@ShadowArcanist ShadowArcanist added the 🐰 Release The Rabbit Run CodeRabbitAI review label Nov 2, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f64bea3 and b6650a0.

📒 Files selected for processing (1)
  • bootstrap/helpers/shared.php (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
bootstrap/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)

Initialization/bootstrap code should reside under bootstrap/ (e.g., app bootstrapping)

Files:

  • bootstrap/helpers/shared.php
**/*.php

📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)

**/*.php: Follow PSR-12 coding standards for all PHP code
Format PHP code with Laravel Pint configuration
Run static analysis with PHPStan to ensure type safety in PHP code
Document complex methods with PHPDoc blocks including parameters, return types, and thrown exceptions

**/*.php: Follow PSR-12 PHP coding standards across the codebase
Prefer eager loading and query optimization to prevent N+1 issues in database interactions
Use Laravel best practices for structure, services, and policies

**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always declare explicit return types for functions and methods; add appropriate parameter type hints
Prefer PHPDoc blocks over inline comments; only add inline code comments for very complex logic
When documenting arrays, add useful array shape types in PHPDoc where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)

Files:

  • bootstrap/helpers/shared.php
🧬 Code graph analysis (1)
bootstrap/helpers/shared.php (2)
app/Models/Application.php (2)
  • deploymentType (903-916)
  • source (838-841)
app/Models/GithubApp.php (1)
  • GithubApp (7-88)

Previously, when custom_port was null (the common case), the code would
set the port to null, breaking SSH connections. This fix adds proper
null checks and defaults to port 22 when custom_port is not set.

Changes:
- Add null check before comparing custom_port to prevent null assignment
- Cast custom_port to int for type safety
- Default to port 22 when custom_port is null in fallback URL construction

Fixes the issue reported by CodeRabbit in PR review.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b6650a0 and 174a766.

📒 Files selected for processing (1)
  • bootstrap/helpers/shared.php (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
bootstrap/**/*.php

📄 CodeRabbit inference engine (.cursor/rules/application-architecture.mdc)

Initialization/bootstrap code should reside under bootstrap/ (e.g., app bootstrapping)

Files:

  • bootstrap/helpers/shared.php
**/*.php

📄 CodeRabbit inference engine (.cursor/rules/development-workflow.mdc)

**/*.php: Follow PSR-12 coding standards for all PHP code
Format PHP code with Laravel Pint configuration
Run static analysis with PHPStan to ensure type safety in PHP code
Document complex methods with PHPDoc blocks including parameters, return types, and thrown exceptions

**/*.php: Follow PSR-12 PHP coding standards across the codebase
Prefer eager loading and query optimization to prevent N+1 issues in database interactions
Use Laravel best practices for structure, services, and policies

**/*.php: Always use curly braces for control structures in PHP, even for single-line bodies
Use PHP 8 constructor property promotion in __construct() and avoid empty constructors
Always declare explicit return types for functions and methods; add appropriate parameter type hints
Prefer PHPDoc blocks over inline comments; only add inline code comments for very complex logic
When documenting arrays, add useful array shape types in PHPDoc where appropriate
Enum case names should be TitleCase (e.g., FavoritePerson, Monthly)

Files:

  • bootstrap/helpers/shared.php
🧬 Code graph analysis (1)
bootstrap/helpers/shared.php (2)
app/Models/Application.php (2)
  • deploymentType (903-916)
  • source (838-841)
app/Models/GithubApp.php (1)
  • GithubApp (7-88)
🔇 Additional comments (4)
bootstrap/helpers/shared.php (4)

11-11: I'll be back... to say this looks good!

The GitlabApp import is necessary for the type hint below and properly ordered. Like a self-hosted server farm, everything is in its rightful place. 🌮


3018-3018: Hasta la vista, GitHub-only deployment!

The function signature now accepts both GithubApp and GitlabApp, like a self-hosted server accepting connections on multiple ports. This change enables GitLab deployments with custom SSH ports, which is exactly what this PR is about. Much better than serverless functions that only speak HTTP. 🦾


3033-3044: Come with me if you want to deploy... with custom ports!

This new logic properly extracts custom SSH ports for SCP-style URLs. The guards at lines 3039-3041 are solid—checking for non-null AND non-22 before overriding the port. Like a properly configured self-hosted GitLab instance, it won't accept just any connection attempt. The explicit cast to int ensures we're not passing taco fillings where we need numbers. 🌮

Past review concerns about nullifying the default port have been terminated! 🎯


3051-3051: GitLab support added to the resistance!

Adding the GitlabApp case here ensures both GitHub and GitLab apps are handled when converting HTTP URLs to SSH format. Like a self-hosted reverse proxy, we're routing all the traffic correctly. 🚀

The null coalescing operator (??) only checks for null, not empty strings.
When custom_port is an empty string, it would pass through and cause SSH
connections to fail with an invalid port.

This commit updates line 3053 to match the same defensive pattern used at
lines 3039-3041, checking for null, empty strings, and casting to int for
consistent validation across both code paths.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐰 Release The Rabbit Run CodeRabbitAI review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants