Skip to content

Merge JSch auth classes and bump dependencies#2

Merged
ljader merged 3 commits intojenkinsci:masterfrom
ljader:master
Nov 24, 2016
Merged

Merge JSch auth classes and bump dependencies#2
ljader merged 3 commits intojenkinsci:masterfrom
ljader:master

Conversation

@ljader
Copy link
Member

@ljader ljader commented Oct 9, 2016

This PR is a first step to release new version of the plugin.

The context:
The intent is to move built-in JSch implmentations from ssh-credentials-plugin to jsch-plugin, to isolate JSch as a dependency plugin and prevent ClassNotFoundException, triggered currently when depending on ssh-credentials-plugin
1st reference - jenkinsci/ssh-credentials-plugin#19
2nd reference - #1

The release plan

  1. Merge this PD and release new vesion of jsch-plugin (0.1.54.0)
  2. Rely on updated jsch-plugin and release ssh-plugin (probably major release 3.x)
  3. Remove JSch specific stuff and release ssh-credentials-plugin (probably major 2.x release as advised by stephenc)
  4. Update Confluence pages and JIRA tickets

Purpose of this PR
Why this PR and not #1 ? - this PR doesn't rely on unreleased 2.x version of ssh-credentials-plugin, so it didn't need coordinated release between plugins - it's easier to release plugin-by-plugin.
It's easier because JSch auth classes has been moved to different package in jsch-plugin than in ssh-credentials-plugin, so they will be treated like separate JSch auth 'techniques' - there is no need for coordinate release.
The duplication will be removed by Step3 of the plan above.

bpedman and others added 2 commits October 9, 2016 17:01
Move the implementation of the JSch SSHAuthenticatorFactory from the
ssh-credential plugin to this plugin. This enables other plugins to
depend on this plugin which has JSch on its classpath. Using the ssh-
credential plugin's SSHAuthenticator class caused problems in some cases
because the JSch library was not available on the classpath.

This provides a cleaner implementation of the SSHAuthenticatorFactory
extension point. Other libraries may do the same thing by providing
their own plugin which registers SSHAuthenticatorFactory
implementations.

This also bumps the version of JSch used, mainly because there are newer
things in this version that some users may need access to.

This also sets the version scheme to <jsch-version>.<package-revision>
so that the primary version can be based on the JSch version used but
allow us to rev the version in case there are other implementation or
package changes.
The JSch 0.1.54 fixes CVS-2016-5725

ssh-credentials was set to 1.12, since JSch auth classes
has been moved to different package,
so they will be treated like separate JSch auth 'techniques'
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code itself looks good, but I'm not sure if it is a full copy-paste from somewhere. If yes, maybe there is a better way.

CC @stephenc and @jenkinsci/code-reviewers , who may have more context


/**
* @author stephenc
* @since 25/10/2012 15:14
Copy link
Member

@oleg-nenashev oleg-nenashev Oct 9, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐜 Wrong Javadoc. At least since if you copy-pasted the code from somewhere.
CC @stephenc


/**
* @author stephenc
* @since 25/10/2012 13:57
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same


/**
* @author stephenc
* @since 25/10/2012 14:49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

The original code for 3 files was copied from
https://github.com/jenkinsci/ssh-credentials-plugin/
from tree 121df4268f7e31617003b2d2a6e6097741536a7f
(ssh-credentials-1.12)
@ljader
Copy link
Member Author

ljader commented Oct 10, 2016

I've provided a PR context and fixed Javadoc since annotations to use versions.
This PR has been reported to jenkinsci-dev mailing list.

/CC @jenkinsci/code-reviewers @oleg-nenashev @stephenc
and previously interested @bpedman @zregvart @edmund-wagner @johnny-b-goode

@ljader
Copy link
Member Author

ljader commented Oct 17, 2016

Ping for review
/CC @jenkinsci/code-reviewers @oleg-nenashev @stephenc

@daniel-beck
Copy link
Member

@zregvart Are you still maintaining this plugin?

@ljader
Copy link
Member Author

ljader commented Nov 4, 2016

@daniel-beck or @oleg-nenashev , since there is no answer from @zregvart , can you give me the push access for this plugin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants