-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix: Add GitLab support to convertGitUrl with custom port handling #7087
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v4.x
Are you sure you want to change the base?
Conversation
…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>
WalkthroughUpdated a Git URL conversion helper to accept Changes
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, ... }
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. Comment |
There was a problem hiding this 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
📒 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>
There was a problem hiding this 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
📒 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>
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?GithubAppin its type declaration, causing runtime errors when GitLab sources were used.Error:
Solution:
use App\Models\GitlabApp;importGithubApp|GitlabApp|null $source = null2. 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:
(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.phpconvertGitUrl()function signature to accept both GitHub and GitLab appsTesting
Tested with:
git@git.zeeze.fun:zeeze/eli.gitImpact
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