Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use SSHUserPrivateKey instead of BasicSSHUserPrivateKey #520

Merged
merged 3 commits into from
Nov 5, 2020

Conversation

chriskilding
Copy link
Contributor

Use the general SSHUserPrivateKey type when looking up SSH credentials instead of the BasicSSHUserPrivateKey concrete subtype. This will allow the plugin to use any compatible SSH key, not just BasicSSHUserPrivateKey instances.

Fixes JENKINS-63986

@chriskilding
Copy link
Contributor Author

Can't proceed on this until we have a way to test the change. What are the relevant tests to modify?

@chriskilding
Copy link
Contributor Author

@res0nance I believe you're one of the maintainers - would you be able to advise where the tests are for using SSH keys from the credentials API, and thus which tests I should modify to ensure this fix works?

@res0nance
Copy link
Contributor

Hey Chris, just saw this, give me a bit to double check this bit.

@res0nance
Copy link
Contributor

Hey Chris i added a test in #523 which should help you out

@chriskilding chriskilding marked this pull request as ready for review October 28, 2020 16:10
@chriskilding
Copy link
Contributor Author

I've pinged the reporter to ask if he'd like to test a build of the fix in his local Jenkins

@chriskilding
Copy link
Contributor Author

Mark has confirmed that it works.

He also said it might be worth putting out a notice of a breaking change for JCasC, since we have to change the config or our clouds will be unable to start.

@johnl2323
Copy link

johnl2323 commented Oct 28, 2020 via email

@chriskilding
Copy link
Contributor Author

The plugin build with the fix can be found at:

https://ci.jenkins.io/job/Plugins/job/ec2-plugin/job/PR-520/5/artifact/org/jenkins-ci/plugins/ec2/1.54-rc1218.bd9a8dd69948/ec2-1.54-rc1218.bd9a8dd69948.hpi

We've had a confirmation that it works so it should be good to merge, but if you need the fix in the meantime, here it is :)

@res0nance res0nance added enhancement Feature additions or enhancements bugfix labels Oct 29, 2020
@johnl2323
Copy link

johnl2323 commented Nov 4, 2020 via email

@res0nance res0nance merged commit 2ef79b6 into jenkinsci:master Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix enhancement Feature additions or enhancements
Projects
None yet
3 participants