[JENKINS-55582] [JEP-230] Split instance-identity to a plugin

[basil]

The merge of #6543 makes it finally practical to complete what was started in #5304.

Testing done

Ran Jenkins against a clean home directory with java -jar jenkins.war and completed the setup wizard successfully. Verified there were no warnings for any plugins after completing the setup wizard.

Next steps

If approved, https://github.com/jenkins-infra/update-center2/blob/d4bcc4ce0041e00b4a6a9773d24c4cd84731c4c7/resources/artifact-ignores.properties#L527-L529= will need to be adjusted to restore instance-identity to the Update Center.

Proposed changelog entries

Convert the instance-identity module into a (detached) plugin.

Proposed upgrade guidelines

Normally the new plugin will be automatically installed. If you use an “as-code” installation mechanism like a text file with a list of plugins, you may need to add instance-identity (version 3.0.1 or later) in order to restore this functionality.

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) and are in the imperative mood. 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
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadoc, as appropriate.
• For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

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 accurate, human-readable, and in the imperative mood
• 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).

[basil]

Needed to avoid this error:

java.io.IOException: Failed to load: Jenkins GIT server Plugin (git-server 1.11)
 - Update required: SSH server (sshd 3.0.3) to be updated to 3.1.0 or higher
        at hudson.PluginWrapper.resolvePluginDependencies(PluginWrapper.java:994)
        at hudson.PluginManager.dynamicLoad(PluginManager.java:926)
Caused: java.io.IOException: Failed to install git-server plugin
        at hudson.PluginManager.dynamicLoad(PluginManager.java:940)
        at hudson.model.UpdateCenter$InstallationJob._run(UpdateCenter.java:2217)
Caused: java.io.IOException: Failed to dynamically deploy this plugin
        at hudson.model.UpdateCenter$InstallationJob._run(UpdateCenter.java:2221)
        at hudson.model.UpdateCenter$DownloadJob.run(UpdateCenter.java:1867)
        at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
        at hudson.remoting.AtmostOneThreadExecutor$Worker.run(AtmostOneThreadExecutor.java:121)
        at java.base/java.lang.Thread.run(Thread.java:829)

[jglick]

Oops, forgot about this, thanks.

[jglick]

I think we want jenkinsci/instance-identity-plugin#21 released first. 3.0 & 3.0.1 should be considered broken.

[basil]

A request made without reasoning. I will not accommodate such a request unless reasoning is provided.

[jglick]

As per https://github.com/jenkinsci/jep/blob/master/jep/230/README.adoc#backwards-compatibility the upgrade guidelines should emphasize that the newly detached plugin is required for inbound TCP agents (JNLP4-connect), as well as any other functionality that happens to expect methods in jenkins.model.identity to work, rather than compiling against org.jenkinsci.main.modules.instance_identity directly.

It would be nicer to actually delete this plugin and inline it into core. Generating an RSA keypair is easy enough; but some callers (incl. TCP agents) need an X509Certificate and, if I understand right, that is the part that is hard to do in Java without third-party libraries (JDK-8058778).

[jglick]

If approved, https://github.com/jenkins-infra/update-center2/blob/d4bcc4ce0041e00b4a6a9773d24c4cd84731c4c7/resources/artifact-ignores.properties#L527-L529= will need to be adjusted to restore instance-identity to the Update Center.

I do not think so. As in #6570 (comment) those releases should continue to be blacklisted. Any new version should automatically appear.

[basil]

As per https://github.com/jenkinsci/jep/blob/master/jep/230/README.adoc#backwards-compatibility the upgrade guidelines should emphasize that the newly detached plugin is required for inbound TCP agents (JNLP4-connect), as well as any other functionality that happens to expect methods in jenkins.model.identity to work, rather than compiling against org.jenkinsci.main.modules.instance_identity directly.

These upgrade guidelines were taken directly from your original work, Jesse. If you don't like them, please fix them up yourself. I am trying to help you finish this work. I did not ask for you to review this change, and I am not willing to do a large amount of additional work to finish this.

[jglick]

Also

jenkins/core/src/main/java/hudson/slaves/JNLPLauncher.java

Lines 251 to 253 in 62fbc3a

	if (Jenkins.get().getTcpSlaveAgentListener() == null) {
	return FormValidation.error("Either WebSocket mode is selected, or the TCP port for inbound agents must be enabled");
	}

should be extended to verify that

jenkins/core/src/main/java/jenkins/slaves/JnlpSlaveAgentProtocol4.java

Lines 104 to 111 in 62fbc3a

	X509Certificate identityCertificate = InstanceIdentityProvider.RSA.getCertificate();
	if (identityCertificate == null) {
	throw new KeyStoreException("JENKINS-41987: no X509Certificate found; perhaps instance-identity module is missing or too old");
	}
	RSAPrivateKey privateKey = InstanceIdentityProvider.RSA.getPrivateKey();
	if (privateKey == null) {
	throw new KeyStoreException("JENKINS-41987: no RSAPrivateKey found; perhaps instance-identity module is missing or too old");
	}

will not fail at runtime.

[basil]

Also […] should be extended to verify that […] will not fail at runtime.

A request made without reasoning. I will not accommodate such a request unless reasoning is provided.

[jglick]

I am not willing to do a large amount of additional work to finish this.

Understood, and I was not expecting someone else to take this over either. The original was indeed a work in progress and there are some missing pieces.

[basil]

No good deed goes unpunished.

[jglick]

Your work here was helpful and appreciated—gets JEP-230 that much closer and should make it easier for me to finish up.

[basil]

Please do not make requests of me in the future without providing the reasoning.