[JENKINS-71461] GIT_SSH_COMMAND diagnostics fail with some host key verification strategies#1713
[JENKINS-71461] GIT_SSH_COMMAND diagnostics fail with some host key verification strategies#1713akash-manna-sky wants to merge 2 commits intojenkinsci:masterfrom
Conversation
|
Hello @MarkEWaite please review this PR. |
…erification strategies
akash-manna-sky
force-pushed
the
JENKINS-71461
branch
from
b7af120 to
2174cfa
Compare
|
Can you please review this PR? @MarkEWaite |
I am on vacation for two weeks and don't plan to review this pull request for at least several weeks after that. |
There was a problem hiding this comment.
Pull request overview
Adds a global “SSH Verbose Mode” option to the git-client plugin so that generated SSH wrapper scripts can include -vvv for diagnostics, even when host key verification strategies are in use (addressing JENKINS-71461 / #1669).
Changes:
- Add
sshVerboseboolean toGitHostKeyVerificationConfigurationand expose it in the global config UI. - Append
-vvvto generated Windows/Unix SSH wrapper scripts when verbose mode is enabled. - Add tests asserting presence/absence and placement of the
-vvvflag in wrapper scripts.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java |
Adds logic to conditionally append -vvv to SSH wrapper scripts. |
src/main/java/org/jenkinsci/plugins/gitclient/GitHostKeyVerificationConfiguration.java |
Persists the new sshVerbose global configuration flag. |
src/main/resources/org/jenkinsci/plugins/gitclient/GitHostKeyVerificationConfiguration/config.jelly |
Adds a checkbox to enable/disable SSH verbose mode in Jenkins UI. |
src/test/java/org/jenkinsci/plugins/gitclient/CliGitAPISecurityTest.java |
Adds tests for default/enabled behavior and flag placement in wrapper scripts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <f:entry title="SSH Verbose Mode" field="sshVerbose"> | ||
| <f:checkbox title="Enable verbose SSH output for diagnostics" | ||
| help="When enabled, SSH commands will include -vvv flag for detailed diagnostic output. This helps troubleshoot SSH connection issues without requiring the GIT_SSH_COMMAND environment variable."/> |
There was a problem hiding this comment.
The checkbox help text is provided via a help attribute on <f:checkbox>, but Jenkins' form controls typically use a separate help-<field>.html (or help on <f:entry>) like the existing help-sshHostKeyVerificationStrategy.html. As written, this long string may not be displayed as intended and is harder to maintain/translate. Consider adding help-sshVerbose.html and referencing it from the <f:entry> (or relying on the conventional file name) instead of embedding the help text in the checkbox tag.
| <f:entry title="SSH Verbose Mode" field="sshVerbose"> | |
| <f:checkbox title="Enable verbose SSH output for diagnostics" | |
| help="When enabled, SSH commands will include -vvv flag for detailed diagnostic output. This helps troubleshoot SSH connection issues without requiring the GIT_SSH_COMMAND environment variable."/> | |
| <f:entry title="SSH Verbose Mode" field="sshVerbose" | |
| help="When enabled, SSH commands will include -vvv flag for detailed diagnostic output. This helps troubleshoot SSH connection issues without requiring the GIT_SSH_COMMAND environment variable."> | |
| <f:checkbox title="Enable verbose SSH output for diagnostics"/> |
| // When Jenkins is not available, SSH verbose should default to false | ||
| GitClient gitClient = Git.with(TaskListener.NULL, new EnvVars()) | ||
| .in(workspace) | ||
| .using("git") | ||
| .getClient(); |
There was a problem hiding this comment.
This test relies on the global default for sshVerbose without explicitly resetting it. Since other tests in this class toggle the global configuration, it would be more robust to set GitHostKeyVerificationConfiguration.get().setSshVerbose(false) at the start (and/or in @AfterEach). Also, the comment about “When Jenkins is not available” is misleading here because the class is annotated with @WithJenkins.
| /** | ||
| * Safely check if SSH verbose mode is enabled. | ||
| * Returns false if Jenkins instance is not available (e.g., during tests). | ||
| * | ||
| * @return true if SSH verbose mode is enabled, false otherwise | ||
| */ | ||
| private boolean isSshVerboseEnabled() { | ||
| try { | ||
| jenkins.model.Jenkins instance = jenkins.model.Jenkins.getInstanceOrNull(); | ||
| if (instance != null) { | ||
| return GitHostKeyVerificationConfiguration.get().isSshVerbose(); | ||
| } | ||
| } catch (Exception e) { | ||
| // If we can't get the configuration, default to false | ||
| LOGGER.log(Level.FINE, "Unable to get SSH verbose configuration", e); | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
isSshVerboseEnabled() checks Jenkins.getInstanceOrNull() and returns false when it is null. CliGitAPIImpl methods execute on agents (the GitClient is created via MasterToSlaveFileCallable in Git#getClient), where Jenkins.getInstanceOrNull() is typically null. As a result, the -vvv flag will never be added for most real jobs running on agents, even when the global configuration enables it. Consider resolving the sshVerbose setting on the controller (where the GlobalConfiguration is available) and passing it to the agent along with hostKeyFactory (e.g., add a boolean to the callable and store it in the remote GitClient/CliGitAPIImpl instance).
github-actions
bot
added
the
documentation
|
Hi @MarkEWaite, can you please review the changes? |
[JENKINS-71461] GIT_SSH_COMMAND diagnostics fail with some host key verification strategies
Fixes #1669
Testing done
Submitter checklist