-
Notifications
You must be signed in to change notification settings - Fork 669
OCPBUGS-74387: "Import from Git" forces HTTPS, port 443 #15975
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: main
Are you sure you want to change the base?
Conversation
Make sure "Import from Git" works fine if we specify anything other than HTTPS and port 443 when using Gitea Fixes https://issues.redhat.com/browse/OCPBUGS-74387
|
@QuadJust: This pull request references Jira Issue OCPBUGS-74387, 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: QuadJust The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @QuadJust. Thanks for your PR. I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
📝 WalkthroughWalkthroughThe gitea-service now dynamically constructs repository host URLs from parsed Git URL components. Previously, the host was hardcoded with 🚥 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 |
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
🤖 Fix all issues with AI agents
In `@frontend/packages/git-service/src/services/gitea-service.ts`:
- Around line 65-71: The host construction uses protocols[0] from GitUrlParse
without guarding for an empty protocols array, causing "undefined://..." for SSH
shorthand URLs; update the logic around the GitUrlParse result (the const
deconstruction and host computation) to use a fallback protocol (e.g., const
protocol = protocols && protocols[0] ? protocols[0] : 'ssh' or another chosen
default) and then build host with protocol (i.e., use protocol instead of
protocols[0]) while keeping the existing port-aware interpolation.
🧹 Nitpick comments (1)
frontend/packages/git-service/src/services/gitea-service.ts (1)
64-82: Consider adding a unit test for non-standard protocol/port combinations.This fix addresses a real bug (OCPBUGS-74387), but the
getRepoMetadatamethod now handles more complex URL parsing logic. Adding test coverage for edge cases—HTTP on custom ports, SSH URLs, and protocol-less shorthand—would prevent regressions.Would you like me to help draft test cases covering:
http://gitea.example.com:3000/owner/repo.githttps://gitea.example.com:8443/owner/repo.gitgit@gitea.example.com:owner/repo.git(SSH shorthand)- Standard
https://gitea.example.com/owner/repo.gitI can open an issue to track this if preferred.
| const { name, owner, protocols, port, resource, full_name: fullName } = GitUrlParse( | ||
| this.gitsource.url, | ||
| ); | ||
| const contextDir = this.gitsource.contextDir?.replace(/\/$/, '') || ''; | ||
| const host = `https://${resource}`; | ||
| const host = _.isEmpty(port) | ||
| ? `${protocols[0]}://${resource}` | ||
| : `${protocols[0]}://${resource}:${port}`; |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify git-url-parse output for different URL patterns
cat << 'EOF' > /tmp/test-parse.js
const GitUrlParse = require('git-url-parse');
const urls = [
'https://gitea.example.com:8443/owner/repo.git',
'http://gitea.example.com/owner/repo.git',
'git@gitea.example.com:owner/repo.git',
'ssh://git@gitea.example.com:22/owner/repo.git',
'gitea.example.com/owner/repo',
];
urls.forEach(url => {
const parsed = GitUrlParse(url);
console.log(`URL: ${url}`);
console.log(` protocols: ${JSON.stringify(parsed.protocols)}`);
console.log(` port: "${parsed.port}"`);
console.log(` resource: ${parsed.resource}`);
console.log('');
});
EOF
cd frontend && npm install --silent git-url-parse 2>/dev/null && node /tmp/test-parse.jsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Find the git-service package and check its dependencies
find . -type f -name "package.json" | grep -E "(git-service|frontend)" | head -20Repository: openshift/console
Length of output: 1464
🏁 Script executed:
# Check git-service package.json for git-url-parse dependency
cat ./frontend/packages/git-service/package.jsonRepository: openshift/console
Length of output: 883
🏁 Script executed:
#!/bin/bash
# Verify git-url-parse output from git-service package directory
cat << 'EOF' > /tmp/test-parse.js
const GitUrlParse = require('git-url-parse');
const urls = [
'https://gitea.example.com:8443/owner/repo.git',
'http://gitea.example.com/owner/repo.git',
'git@gitea.example.com:owner/repo.git',
'ssh://git@gitea.example.com:22/owner/repo.git',
'gitea.example.com/owner/repo',
];
urls.forEach(url => {
const parsed = GitUrlParse(url);
console.log(`URL: ${url}`);
console.log(` protocols: ${JSON.stringify(parsed.protocols)}`);
console.log(` protocols.length: ${parsed.protocols.length}`);
console.log(` protocols[0]: ${parsed.protocols[0]}`);
console.log(` port: "${parsed.port}"`);
console.log(` resource: ${parsed.resource}`);
console.log('');
});
EOF
cd ./frontend/packages/git-service && npm install --silent 2>&1 | head -5 && node /tmp/test-parse.jsRepository: openshift/console
Length of output: 930
🏁 Script executed:
#!/bin/bash
# Install dependencies in git-service and verify
cd ./frontend/packages/git-service
npm install 2>&1 | tail -20
ls -la node_modules/git-url-parse 2>&1 | head -10Repository: openshift/console
Length of output: 1196
🏁 Script executed:
#!/bin/bash
# Install from root and run test
cd ./frontend
npm install --legacy-peer-deps 2>&1 | tail -10
cat << 'EOF' > /tmp/test-parse.js
const GitUrlParse = require('git-url-parse');
const urls = [
'https://gitea.example.com:8443/owner/repo.git',
'http://gitea.example.com/owner/repo.git',
'git@gitea.example.com:owner/repo.git',
'ssh://git@gitea.example.com:22/owner/repo.git',
'gitea.example.com/owner/repo',
];
urls.forEach(url => {
const parsed = GitUrlParse(url);
console.log(`URL: ${url}`);
console.log(` protocols: ${JSON.stringify(parsed.protocols)}`);
console.log(` protocols.length: ${parsed.protocols.length}`);
console.log(` protocols[0]: ${parsed.protocols[0]}`);
console.log(` port: "${parsed.port}"`);
console.log(` resource: ${parsed.resource}`);
console.log('');
});
EOF
node /tmp/test-parse.jsRepository: openshift/console
Length of output: 147
🏁 Script executed:
# Find all uses of GitUrlParse in the codebase
rg "GitUrlParse" -A 5 -B 2 frontend/packages/git-service/Repository: openshift/console
Length of output: 4337
🏁 Script executed:
# Check the complete gitea-service getRepoMetadata method and any error handling
cat -n frontend/packages/git-service/src/services/gitea-service.ts | sed -n '60,85p'Repository: openshift/console
Length of output: 1050
🏁 Script executed:
# Look for tests related to gitea-service or git-url-parse behavior
find frontend/packages/git-service -name "*test*" -o -name "*spec*" | head -10Repository: openshift/console
Length of output: 780
🏁 Script executed:
# Check the gitea-service tests to understand expected behavior
cat frontend/packages/git-service/src/services/__tests__/gitea-service.spec.tsRepository: openshift/console
Length of output: 9406
🏁 Script executed:
# Search for any other URL patterns or protocol handling in the codebase
rg "protocols" frontend/packages/git-service/ -B 3 -A 3Repository: openshift/console
Length of output: 2445
🏁 Script executed:
# Check if there are any TypeScript type definitions for git-url-parse
rg "git-url-parse" frontend/packages/git-service/src -A 5 -B 2 --type ts | head -50Repository: openshift/console
Length of output: 2005
🌐 Web query:
git-url-parse protocols array empty SSH shorthand URL behavior
💡 Result:
In git-url-parse, the protocols array represents only the protocol tokens that are explicitly present in the input string (the parts before ://, and split by + for things like git+ssh). So:
git+ssh://git@host.xz/path/name.git→protocols: ["git","ssh"](explicit) andprotocol: "git"(the “first” protocol) [1][2]- SSH shorthand / scp-like URLs such as
git@github.com:owner/repo.githave no scheme section at all (nossh://), soprotocolsis empty ([]) even though the library still infers the effective transport as SSH and setsprotocol: "ssh"[1][3].
This matches Git’s own “scp-like” SSH syntax ([user@]host:path/to/repo.git), which is SSH despite not being a formal URI with a scheme. [3][4]
Sources: [1] git-url-parse example output, [2] parse-url/git protocol tokenization examples, [3] Git “scp-like” SSH syntax description, [4] explanation of scp-like parsing behavior.
🏁 Script executed:
# Search for any use of singular "protocol" (not protocols) from GitUrlParse
rg "\.protocol[^s]" frontend/packages/git-service/src --type tsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Check test fixtures to see what URL patterns are tested
find frontend/packages/git-service/src/services/__tests__/__nock-fixtures__/gitea -name "*.json" | head -5Repository: openshift/console
Length of output: 524
🏁 Script executed:
# Check if any test cases use SSH shorthand URLs
grep -r "git@" frontend/packages/git-service/src/services/__tests__/gitea-service.spec.tsRepository: openshift/console
Length of output: 43
Add defensive fallback for empty protocols array.
The git-url-parse library returns an empty protocols array for SSH shorthand URLs (e.g., git@gitea.example.com:owner/repo.git). Accessing protocols[0] without a guard produces undefined, resulting in malformed host strings like undefined://resource.
Add a fallback to a sensible default:
Proposed fix
const { name, owner, protocols, port, resource, full_name: fullName } = GitUrlParse(
this.gitsource.url,
);
const contextDir = this.gitsource.contextDir?.replace(/\/$/, '') || '';
+ const protocol = protocols?.[0] || 'https';
- const host = _.isEmpty(port)
- ? `${protocols[0]}://${resource}`
- : `${protocols[0]}://${resource}:${port}`;
+ const host = _.isEmpty(port)
+ ? `${protocol}://${resource}`
+ : `${protocol}://${resource}:${port}`;📝 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 = this.gitsource.contextDir?.replace(/\/$/, '') || ''; | |
| const host = `https://${resource}`; | |
| const host = _.isEmpty(port) | |
| ? `${protocols[0]}://${resource}` | |
| : `${protocols[0]}://${resource}:${port}`; | |
| const { name, owner, protocols, port, resource, full_name: fullName } = GitUrlParse( | |
| this.gitsource.url, | |
| ); | |
| const contextDir = this.gitsource.contextDir?.replace(/\/$/, '') || ''; | |
| const protocol = protocols?.[0] || 'https'; | |
| const host = _.isEmpty(port) | |
| ? `${protocol}://${resource}` | |
| : `${protocol}://${resource}:${port}`; |
🤖 Prompt for AI Agents
In `@frontend/packages/git-service/src/services/gitea-service.ts` around lines 65
- 71, The host construction uses protocols[0] from GitUrlParse without guarding
for an empty protocols array, causing "undefined://..." for SSH shorthand URLs;
update the logic around the GitUrlParse result (the const deconstruction and
host computation) to use a fallback protocol (e.g., const protocol = protocols
&& protocols[0] ? protocols[0] : 'ssh' or another chosen default) and then build
host with protocol (i.e., use protocol instead of protocols[0]) while keeping
the existing port-aware interpolation.
|
/jira refresh |
|
@QuadJust: This pull request references Jira Issue OCPBUGS-74387, 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. |
|
PR needs rebase. 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. |
Make sure "Import from Git" works fine if we specify anything other than HTTPS and port 443 when using Gitea
Fixes https://issues.redhat.com/browse/OCPBUGS-74387
Summary by CodeRabbit