[JENKINS-64107] Convert SSHD module to a plugin

[kuisathaverat]

See JENKINS-55582 JENKINS-64107 JENKINS-64104.

Proposed changelog entries

• Major RFE: Turn SSHD module into a plugin, SSHD module is no longer bundle in Jenkins Core, it is provided as a plugin for further details see JENKINS-55582 and JENKINS-64107

Proposed upgrade guidelines

N/A

Submitter checklist

• (If applicable) Jira issue is well described
• Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
  • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
• Appropriate autotests or explanation to why this change has no tests
• [ ] For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@jenkinsci/code-reviewers

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least 2 approvals for the pull request and no outstanding requests for change
• Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
• Changelog entries in the PR title and/or Proposed changelog entries are correct
• Proper changelog labels are set so that the changelog can be generated automatically
• If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[jglick]

Will need to add a test-scoped dep to the test module since some functional tests in core do try to use java -jar jenkins-cli.jar -ssh.

[kuisathaverat]

the publish stages failed due to the issues on the infra these last two days, all the tests and the ATH passes.

[jglick]

The JEP proposes inlining ssh-cli-auth but I do not see that here.

[kuisathaverat]

I was thinking of splitting the change into two PRs, but if you think I can include the ssh-cli-auth change here, I'd include it.

[jglick]

I think you have to. Otherwise we could release a split of a plugin before that plugin is actually ready.

[kuisathaverat]

@jglick I checked the ssh-cli-auth-module and it depends on sshd-module for the tests, if I include the ssh-cli-auth-module as a dependency inside sshd-module, we will have a circular dependency for testing, so I wonder if we should move the code in ssh-cli-auth-module repo to the sshd-module repo and archive the ssh-cli-auth-module repo.

[jglick]

move the code in ssh-cli-auth-module repo to the sshd-module repo and archive the ssh-cli-auth-module repo

That is precisely what I advocated in the JEP. Perhaps I did not word it clearly enough?

[oleg-nenashev]

+1 for having a single plugin BTW. I am a bit concerned about having no SSH CLI support on startup without a plugin, but I think we can live without it.

[jglick]

Approving conditional on upstream.

[kuisathaverat]

I have added the test on jenkinsci/sshd-plugin#40 and update the PR with the master changes

What are the next steps? I guess we have to merge jenkinsci/sshd-plugin#38 after this we can decide the final Jenkins Core version we will include this PR, and after that, we can finish and merge jenkinsci/sshd-plugin#40

finally, if everything is fine we can test and merge jenkinsci/sshd-plugin#37

[jglick]

jenkins/test/src/test/java/jenkins/security/ClassFilterImplTest.java

Line 160 in 2ddf358

filter.check("org.jenkinsci.main.modules.cli.auth.ssh.UserPropertyImpl");

can be deleted

[jglick]

Note that automated test coverage is not going to be of much help here. You need to take the actual generated jenkins.war and try to run it to see what happens.

[timja]

I tested this manually, setup wizard worked fine, but update center data for the plugin seems missing, cc @danielbeck

wget http://updates.jenkins.io/current/update-center.actual.json
 cat update-center.actual.json | jq '.plugins["sshd"]'

instance-identity worked fine

[timja]

jenkins-infra/update-center2#484

[timja]

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

[jglick]

Suggest #5049 (comment) though it is not a blocker.

[jglick]

I still do not see this on the UC?

[timja]

?

➜  wars cat update-center.actual.json | jq '.plugins["sshd"]'
{
  "buildDate": "Feb 23, 2021",
  "dependencies": [],
  "developers": [
    {
      "developerId": "timja"
    }
  ],
  "excerpt": "Adds SSH server functionality to Jenkins, exposing CLI commands through it",
  "gav": "org.jenkins-ci.modules:sshd:3.0.1",
  "labels": [],
  "minimumJavaVersion": "1.8",
  "name": "sshd",
  "popularity": 0,
  "previousTimestamp": "2021-02-22T19:33:24.00Z",
  "previousVersion": "3.0",
  "releaseTimestamp": "2021-02-23T15:53:28.00Z",
  "requiredCore": "2.282-rc30905.2ddf35821c11",
  "scm": "https://github.com/jenkinsci/sshd-plugin",
  "sha1": "xX2ALuJ47hl9sGVwrc6WptJTj+g=",
  "sha256": "Jmpl9bTjReS3qrwwTf3d0CTR8RyB/bZS3u+IQVzvEJg=",
  "title": "SSH server",
  "url": "https://updates.jenkins.io/download/plugins/sshd/3.0.1/sshd.hpi",
  "version": "3.0.1",
  "wiki": "https://plugins.jenkins.io/sshd"
}

[jglick]

$ curl -sL 'https://updates.jenkins-ci.org/update-center.json?version=2.282' | head -n-1 | tail -n+2 | jq -r '.plugins | .["sshd"] | .version'
3.0

[timja]

seems not portable command?

head: illegal line count -- -1

[timja]

hmm weird, maybe update center is ignoring the version in the dynamic tiers, 'latest' has it

[jglick]

Maybe does not refresh until after 2.282 actually comes out. If so, fine.

[daniel-beck]

Why are we considering this a split/detached plugin, when the functionality wasn't in core before?

It makes some sense from a configuration/functionality POV (but seems separate enough that functionality would easily be restored after installing the plugin, similar to https://plugins.jenkins.io/cctray-xml/ ), but needlessly adds a dependency from plugins with slightly older core baselines.

[kuisathaverat]

I think that the main reason to consider it a detached plugin is that is a feature you do not want to be installed. the use of Apache Mina SSH library in it is another good reason, it forces all plugins to use a version highly attached to the Jenkins Core, it difficult to bump that library version. As a plugin allows more flexibility to change the library version and helps other plugins to depend on a plugin and not on a Jenkins Core.

[daniel-beck]

Well, yeah, but those dependencies on plugins need to be explicit. Adding it to the split-plugins file doesn't accomplish that, because that only matters on cores newer than what the downstream plugin was developed against.

I think that the main reason to consider it a detached plugin is that is a feature you do not want to be installed.

Well, the alternative is to just split it off and add a changelog entry, like we did for cctray-xml. Otherwise it'll just be installed automatically all the time because tons of plugins (almost all of them at this point) would get it as an implied dependency.

[kuisathaverat]

Oh!! I get you now, I simply put it in detached for backward compatibility as I did with the trilead plugin, to avoid friction.

[daniel-beck]

Well, trilead was on the core classpath, this one was not (AFAIUI).

[oleg-nenashev]

IIUC upgrade guideline is not required here simnce the plugin is detached, may have been confused by the changelog entry. Am I right or not?

[kuisathaverat]

yes, it is detached so nothing to do on the user side.