OCPBUGS-74937: “Import from Git” should not force HTTPS/443 for Git providers (GitHub/GitLab/Bitbucket) #15978
Conversation
…roviders (GitHub/GitLab/Bitbucket)
openshift-ci-robot
added
jira/valid-reference
|
@krishagarwal278: This pull request references Jira Issue OCPBUGS-74937, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis pull request updates the Git service implementations to preserve URL scheme and port information from configured git source URLs. Previously, Bitbucket, GitHub, and GitLab service classes hard-coded HTTPS protocol when constructing API endpoints for non-standard hosts. The changes introduce runtime URL parsing to extract the protocol and port from the configured git source, enabling support for HTTP-based connections and non-default ports. Corresponding test cases are added to verify scheme and port preservation for each service implementation. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/packages/git-service/src/services/github-service.ts`:
- Around line 65-72: The current GitUrlParse handling uses protocols?.[0]
directly which can be 'ssh' and produces broken API hosts; update the protocol
selection in github-service.ts (and mirror the same change in gitlab-service.ts)
to only accept 'http' or 'https' — e.g., derive protocol from
GitUrlParse(this.gitsource.url).protocols but if protocols?.[0] is not 'http' or
'https' then set protocol to 'https' (so the protocol variable used in the host
construction always becomes 'http' or 'https'); leave the host construction and
contextDir logic (host = port ? `${protocol}://${resource}:${port}` :
`${protocol}://${resource}` and contextDir =
this.gitsource.contextDir?.replace(/\/$/, '') || '') unchanged.
In `@frontend/packages/git-service/src/services/gitlab-service.ts`:
- Around line 81-86: The protocol chosen from GitUrlParse (protocols?.[0]) can
be "ssh" and must be validated before building the API host; update the logic
around the protocol and host construction in gitlab-service.ts (the
variables/properties: GitUrlParse, protocols, protocol, host) to only accept
'http' or 'https' values and otherwise default to 'https' so SSH or other
schemes don’t become the API protocol; keep the rest of host construction (using
port/resource) unchanged and ensure any downstream GitLab API client uses this
validated protocol value.
| const { name, owner, protocols, port, resource, full_name: fullName } = GitUrlParse( | ||
| this.gitsource.url, | ||
| ); | ||
| const contextDir = removeLeadingSlash(this.gitsource.contextDir); | ||
| const host = `https://${resource}`; | ||
| const protocol = protocols?.[0] || 'https'; | ||
| const host = port ? `${protocol}://${resource}:${port}` : `${protocol}://${resource}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the file and examine the code context
fd gitlab-service.tsRepository: openshift/console
Length of output: 121
🏁 Script executed:
# Let's also check for git-url-parse in package.json files to understand the version
rg "git-url-parse" --type jsonRepository: openshift/console
Length of output: 609
🌐 Web query:
git-url-parse npm protocols field ssh url behavior
💡 Result:
In the git-url-parse (IonicaBizau) npm package, the returned fields behave like this:
protocols: an array of the URL schemes that were explicitly present in the input string (and split on+when applicable). For a normal HTTPS URL you’ll typically see one entry, e.g.["https"]. (skypack.dev)- SCP-like SSH remotes (e.g.
git@github.com:owner/repo.git) have no explicit scheme, soprotocolscomes back empty ([]), even though the URL is treated as SSH. (npm.io) protocol: the effective git transport protocol inferred/used by the parser. For SCP-like SSH inputs it returns"ssh"(despiteprotocols: []). (npm.io)
This matches Git’s own rule: the user@host:path “scp-like” form is an SSH URL syntax, just without an ssh:// scheme prefix. (git-scm.com)
Citations:
- 1: https://www.skypack.dev/view/git-url-parse?utm_source=openai
- 2: https://npm.io/package/bb-git-url-parse?utm_source=openai
- 3: https://npm.io/package/bb-git-url-parse?utm_source=openai
- 4: https://git-scm.com/docs/git-clone/2.44.0?utm_source=openai
🏁 Script executed:
# View the file with more context
cat -n frontend/packages/git-service/src/services/gitlab-service.ts | head -100Repository: openshift/console
Length of output: 3486
🏁 Script executed:
# Check for tests related to this service
find frontend/packages/git-service -name "*.test.*" -o -name "*.spec.*" | head -20Repository: openshift/console
Length of output: 760
🏁 Script executed:
# Search for how this GitLab service is instantiated/called to understand URL inputs
rg "GitLabService\|gitsource\.url" frontend/packages/git-service/src --type ts -A 2 -B 2Repository: openshift/console
Length of output: 43
🏁 Script executed:
# Check the gitlab-service test file to understand usage patterns
cat -n frontend/packages/git-service/src/services/__tests__/gitlab-service.spec.ts | head -150Repository: openshift/console
Length of output: 6275
🏁 Script executed:
# Search for GitSource definitions and how URLs are passed to the service
rg "GitSource|gitsource.*=" frontend/packages/git-service/src --type ts -B 1 -A 3 | head -60Repository: openshift/console
Length of output: 4340
🏁 Script executed:
# Check if there are any tests or usage with SSH URLs
rg "git@|ssh://" frontend/packages/git-service/src --type tsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Check other git service implementations to see if they handle this the same way
cat -n frontend/packages/git-service/src/services/github-service.ts | sed -n '60,100p'Repository: openshift/console
Length of output: 1518
🏁 Script executed:
# Look at the type definition of GitSource
rg "type GitSource|interface GitSource" frontend/packages/git-service/src --type ts -A 10Repository: openshift/console
Length of output: 790
🏁 Script executed:
# Let's check if there's any validation or documentation about accepted URL formats
rg "url.*ssh|url.*https|url.*validation" frontend/packages/git-service/src --type ts -B 2 -A 2 | head -40Repository: openshift/console
Length of output: 3470
🏁 Script executed:
# Check base-service to see if there's any URL validation there
cat -n frontend/packages/git-service/src/services/base-service.ts | head -50Repository: openshift/console
Length of output: 1532
🏁 Script executed:
# Check how git-provider-detector is used - does the system accept SSH URLs?
cat -n frontend/packages/git-service/src/utils/git-provider-detector.tsRepository: openshift/console
Length of output: 1365
🏁 Script executed:
# Look for any URL validation or normalization before passing to GitlabService
rg "GitlabService|GitlabService\(" frontend/packages/git-service/src --type ts -B 3 -A 3 | head -50Repository: openshift/console
Length of output: 4736
Validate protocol is HTTP/HTTPS before constructing the API host URL.
The current code protocols?.[0] || 'https' works for common SCP-style URLs (e.g., git@gitlab.com:org/repo.git) because git-url-parse returns an empty protocols array for those, safely defaulting to HTTPS. However, if an explicit ssh:// URL is passed, protocols[0] would be "ssh", which breaks the GitLab API client. Bitbucket service already implements this validation—apply the same pattern here:
Suggested fix
- const protocol = protocols?.[0] || 'https';
- const host = port ? `${protocol}://${resource}:${port}` : `${protocol}://${resource}`;
+ const rawProtocol = protocols?.[0]?.replace(':', '');
+ const protocol = rawProtocol === 'http' || rawProtocol === 'https' ? rawProtocol : 'https';
+ const host = port ? `${protocol}://${resource}:${port}` : `${protocol}://${resource}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { name, owner, protocols, port, resource, full_name: fullName } = GitUrlParse( | |
| this.gitsource.url, | |
| ); | |
| const contextDir = removeLeadingSlash(this.gitsource.contextDir); | |
| const host = `https://${resource}`; | |
| const protocol = protocols?.[0] || 'https'; | |
| const host = port ? `${protocol}://${resource}:${port}` : `${protocol}://${resource}`; | |
| const { name, owner, protocols, port, resource, full_name: fullName } = GitUrlParse( | |
| this.gitsource.url, | |
| ); | |
| const contextDir = removeLeadingSlash(this.gitsource.contextDir); | |
| const rawProtocol = protocols?.[0]?.replace(':', ''); | |
| const protocol = rawProtocol === 'http' || rawProtocol === 'https' ? rawProtocol : 'https'; | |
| const host = port ? `${protocol}://${resource}:${port}` : `${protocol}://${resource}`; |
🤖 Prompt for AI Agents
In `@frontend/packages/git-service/src/services/gitlab-service.ts` around lines 81
- 86, The protocol chosen from GitUrlParse (protocols?.[0]) can be "ssh" and
must be validated before building the API host; update the logic around the
protocol and host construction in gitlab-service.ts (the variables/properties:
GitUrlParse, protocols, protocol, host) to only accept 'http' or 'https' values
and otherwise default to 'https' so SSH or other schemes don’t become the API
protocol; keep the rest of host construction (using port/resource) unchanged and
ensure any downstream GitLab API client uses this validated protocol value.
|
/retest |
|
/test e2e-gcp-console |
|
/retest |
|
Checked on cluster launched against the pr.
@krishagarwal278 Your fix works for github/gitlab/bitbucket(in test result 1 to 4). But seems there is not fix for gitea type(in test result 5), need gitea type be fixed? |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@krishagarwal278: This pull request references Jira Issue OCPBUGS-74937, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Claude has additional feedback that is worth examining: Pull Request Review: OCPBUGS-74937PR: #15978 SummaryThis PR fixes a bug where "Import from Git" forces HTTPS/443 for custom Git provider instances, breaking support for HTTP-only servers or non-standard ports. Changes:
Review Findings✅ Positives
|
|
/retest |
|
@krishagarwal278: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Checked on cluster launched against the latest pr code. |
openshift-ci-robot
added
the
verified
|
@yanpzhan: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/label tide/merge-method-squash |
openshift-ci
bot
added
the
tide/merge-method-squash
|
/retest |
|
Solid work, @krishagarwal278! |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: krishagarwal278, rhamilto The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
thanks so much! @rhamilto ❤️ |
|
@krishagarwal278: Jira Issue Verification Checks: Jira Issue OCPBUGS-74937 Jira Issue OCPBUGS-74937 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
5 participants
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests