[JENKINS-58942] Honor -Djenkins.hook.url=… if defined#240
[JENKINS-58942] Honor -Djenkins.hook.url=… if defined#240bitwiseman merged 4 commits intojenkinsci:masterfrom
Conversation
bitwiseman
left a comment
bitwiseman
left a comment
There was a problem hiding this comment.
There's ways to WireMock this, right?
Let's get an automated test that ensures this doesn't regress.
|
There are some WireMock tests in this repo. I am not sure how to write new ones using the current idiom of record/playback. Did not see anything related to hook registration. |
jglick
changed the title
|
|
||
| public static void register(GitHub hub, String orgName) throws IOException { | ||
| String rootUrl = Jenkins.getActiveInstance().getRootUrl(); | ||
| String rootUrl = System.getProperty("jenkins.hook.url"); |
There was a problem hiding this comment.
@jglick Does that automatically convert from the JENKINS_HOOK_URL env value as well?
There was a problem hiding this comment.
No. I switched to the system property
- for consistency with other configuration in Jenkins
- because for my purposes it must be mutable inside the JVM, which environment variables are not (AFAIK)
In the long term, this probably belongs in JenkinsLocationConfiguration, but requiring such a core dependency would make this potentially unmergeable for months, so I thought it would be acceptable to start with an experimental system property.
bitwiseman
left a comment
bitwiseman
left a comment
There was a problem hiding this comment.
One questions, otherwise looks good.
|
see #310 |
3 participants
See JENKINS-58942 for discussion. Open to alternate configuration mechanisms; this is just the simplest thing I could think of. I have tested this using a webhook forwarding service by creating an organization folder, verifying that the alternate base URL was used when registering the hook, and verifying that hook deliveries succeed and trigger responses such as starting builds.