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

[JENKINS-50181] add trailing newline to private keys entered in web UI to appease ssh-add #33

Merged
merged 1 commit into from
Jul 10, 2019

Conversation

liath
Copy link
Contributor

@liath liath commented Mar 16, 2018

Per JENKINS-50181.

If there's no trailing newline ssh-add will get upset and either refuse the key with Error loading key "<path>": invalid format or prompt for a password when the key does not have one and fail.

I don't know that the ternary is idiomatic, so please let me know if there's anything smelly. Ty!

@dwnusbaum
Copy link
Member

Thanks for the PR! Changes look fine at a glance, but I think #32 needs to be finalized before the CI build will pass.

@willis7
Copy link

willis7 commented Aug 30, 2018

Keen to see this merged as hitting the same problem

@rvbmedeiros
Copy link

Does it have any estimated conclusion?

@jvz
Copy link
Member

jvz commented Oct 17, 2018

Code looks good, but needs to be rebased from master apparently. I don't have merge permissions here, though I can try to find someone who can.

@radepal
Copy link

radepal commented Oct 30, 2018

Is there any chance to solve this
It is very annoying issue to automate things with JCasC

@albertorm95
Copy link

@liath Where do I need to put the new line entry? I don't get it

@liath
Copy link
Contributor Author

liath commented Nov 5, 2018

When pasting the key into the web UI, you need to make sure there's a newline after the key.
So

---KEY---

needs to be:

---KEY---

@FnTm
Copy link

FnTm commented Apr 2, 2019

@liath do you think you'll fix this up and push for merge?
I'm only asking because I got bit by this today, and I'd really like to not have to remember this. Therefore if you've no time to wrap this up - I could take over the small bits left to finish.

@akoeplinger
Copy link

Just wasted a few hours today until I found this issue/PR, would be really nice if this got merged :)

@subourbonite
Copy link

Is there a reason this isn't merged? This is a huge pain. Do you need someone else to pick up this PR to get it over the finish line?

@jvz jvz requested a review from jglick May 7, 2019 15:33
@liath
Copy link
Contributor Author

liath commented May 8, 2019

Afaik the failing tests have nothing to do with this PR and it's ready to merge.

@lcollins
Copy link

@jglick - hi there, we have been caught by this costing us a couple of hours... wondering if this PR is in a state where it can be merged as it looks like the test has been updated per last review.

@liath
Copy link
Contributor Author

liath commented Jun 26, 2019

Oh good, the tests pass now.

@jglick
Copy link
Member

jglick commented Jun 27, 2019

This PR got damaged by some kind of Git mistakes, as can be seen in the Commits tab. Please rebase it to one commit, or just close it and file a fresh PR with the patch recommitted.

@liath
Copy link
Contributor Author

liath commented Jun 27, 2019

Yeah, that was a mess. Should be clean now :)

@jvz jvz dismissed jglick’s stale review June 28, 2019 14:46

Force push updated

@whyman
Copy link

whyman commented Jul 10, 2019

We just ran into this, please merge!

@jvz
Copy link
Member

jvz commented Jul 10, 2019

Thanks for reminding me that this was still open.

@jvz jvz merged commit 2aaa135 into jenkinsci:master Jul 10, 2019
@jvz
Copy link
Member

jvz commented Jul 10, 2019

I'll work on a 1.17.1 release later today most likely (otherwise later this week sometime).

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.