feat(gerrit): Add gitUrl support#42672
Conversation
This change adds support for the gitUrl parameter to the gerrit platform. It queries the gerrit server to check what download options are available. If ssh is selected but not available, it will throw an error.
This removes support for querying the supported download schemes and instead hard-codes the default ssh port.
| ); | ||
|
|
||
| const sshUrl = new URL( | ||
| `ssh://${endpointUrl.host}:29418${joinUrlParts(endpointUrl.pathname, repository)}`, |
There was a problem hiding this comment.
Can you please extract the port number to a constant?
| url.password = opts.password; | ||
| url.pathname = joinUrlParts( | ||
| url.pathname, | ||
| const httpUrl = new URL(endpointUrl.toString()); |
There was a problem hiding this comment.
Nitpick, but maybe using clone from util/clone.ts would be nicer.
| encodeURIComponent(repository), | ||
| ); | ||
|
|
||
| const sshUrl = new URL( |
There was a problem hiding this comment.
You should probably use parseUrl here too.
|
@spmiller, this reminded me of a very old discussion: In that discussion, the Gerrit URL would be something like Maybe you can extract the prefix from the configured platform URL and "subtract" it from the generated SSH URL? |
Hey @felipecrs, this old discussion is gold! Having some actual config examples makes it much easier to solve the problem. I think the behaviour in my PR would not work for their server. It got me thinking, maybe my original approach (query the server to get the download schemes) wasn't so bad after all? It was an extra API call ( The trade-off is more code in the client/types files. Perhaps it's worth it? This is the original commit I made in case you want to compare: |
|
No stop whatever you doing now 5019
…On Thu, Apr 16, 2026, 18:33 spmiller ***@***.***> wrote:
*spmiller* left a comment (renovatebot/renovate#42672)
<#42672 (comment)>
@spmiller <https://github.com/spmiller>, this reminded me of a very old
discussion:
Maybe you can extract the prefix from the configured platform URL and
"subtract" it from the generated SSH URL?
Hey @felipecrs <https://github.com/felipecrs>, this old discussion is
gold! Having some actual config examples makes it much easier to solve the
problem. I think the behaviour in my PR would not work for their server.
It got me thinking, maybe my original approach (query the server to get
the download schemes) wasn't so bad after all? It was an extra API call (
config/server/info) but I could Promise.all them quite easily to get it
in parallel. We wouldn't have to have extra logic to extract the prefix.
The trade-off is more code in the client/types files. Perhaps it's worth
it?
This is the original commit I made in case you want to compare:
***@***.***
#diff-9e28b3970977a05aec269cd0b4fb67427dc51bdb6a1e10281f1dc214c69c56f8
<spmiller@bb187fa#diff-9e28b3970977a05aec269cd0b4fb67427dc51bdb6a1e10281f1dc214c69c56f8>
—
Reply to this email directly, view it on GitHub
<#42672 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/CBXT7ICNFUE2GFVOFJZZRVD4WF3U7AVCNFSM6AAAAACX23P6QCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DENRUGM4DANBYGE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
@spmiller, you only need to make these changes: diff --git a/lib/modules/platform/gerrit/utils.spec.ts b/lib/modules/platform/gerrit/utils.spec.ts
index a6f7b7a72..7515b781f 100644
--- a/lib/modules/platform/gerrit/utils.spec.ts
+++ b/lib/modules/platform/gerrit/utils.spec.ts
@@ -100,7 +100,7 @@ describe('modules/platform/gerrit/utils', () => {
'https://gerrit.example.com/context',
'ssh',
);
- expect(repoUrl).toBe('ssh://gerrit.example.com:29418/context/web/apps');
+ expect(repoUrl).toBe('ssh://gerrit.example.com:29418/web/apps');
});
});
});
diff --git a/lib/modules/platform/gerrit/utils.ts b/lib/modules/platform/gerrit/utils.ts
index 5f24e3635..9d7ffde83 100644
--- a/lib/modules/platform/gerrit/utils.ts
+++ b/lib/modules/platform/gerrit/utils.ts
@@ -62,7 +62,7 @@ export function getGerritRepoUrl(
);
const sshUrl = new URL(
- `ssh://${endpointUrl.host}:29418${joinUrlParts(endpointUrl.pathname, repository)}`,
+ `ssh://${endpointUrl.host}:29418/${repository}`,
);
const url = pickUrlFromGitUrl(gitUrl, sshUrl, httpUrl);
|
Addresses feedback left in review: - constants for ports - better SSH url creation - more consistent util method usage
| const sshUrl = parseUrl( | ||
| `ssh://${endpointUrl.host}:${DEFAULT_SSH_PORT}/${repository}`, | ||
| ); | ||
| if (!sshUrl) { | ||
| throw new Error(CONFIG_GIT_URL_UNAVAILABLE); | ||
| } | ||
|
|
||
| const url = pickUrlFromGitUrl(gitUrl, sshUrl, httpUrl); | ||
| logger.trace( | ||
| { url: url.toString() }, | ||
| 'using URL based on configured endpoint', | ||
| ); |
There was a problem hiding this comment.
Coverage is now failing because of the change I asked (to use parseUrl). Sorry about that.
Feel free to change it back to new URL() for now, then let's see what the maintainers think.
There was a problem hiding this comment.
Feel free to change it back to
new URL()for now, then let's see what the maintainers think.
Thanks, I think that's the pragmatic option. I rewrote the method to allow testing the coverage and it looked dreadful!
Addresses feedback left in review: - Reverts SSH URL creation to ensure coverage target
Okay, I've made those changes and addressed your other review comments. Thanks for your help so far! |
| case 'ssh': | ||
| return sshUrl.toString(); | ||
| case 'endpoint': | ||
| return httpUrl.toString(); |
There was a problem hiding this comment.
| return httpUrl.toString(); |
let fall through when same
| url.password = opts.password; | ||
| url.pathname = joinUrlParts( | ||
| url.pathname, | ||
| const httpUrl = clone(endpointUrl); |
There was a problem hiding this comment.
why cloning? the endpointUrl is only used in this function, so use it here again?
better do the switch case here and don't compute multiple urls to later throw the other away. only compute ssh when gitUrl=ssh, use current otherwise. will make the code much simpler
Changes
This change adds support for the existing
gitUrlparameter to thegerritplatform.The existing behaviour (when
gitUrlis not specified) is left unchanged, and is mapped to thedefaultandendpointvalues ofgitUrl.Context
Please select one of the following:
AI assistance disclosure
Did you use AI tools to create any part of this pull request?
Please select one option and, if yes, briefly describe how AI was used (e.g., code, tests, docs) and which tool(s) you used.
I asked Claude which API provided the download schemes in Gerrit. No AI was used for the coding.
Documentation (please check one with an [x])
I added Gerrit as a supported platform for the parameter
gitUrlinconfig/options/index.tsHow I've tested my work (please select one)
I have verified these changes via:
My Gerrit server is not publicly available, but with my changes Renovate was able to clone the repo over SSH.