-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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
Feature/updated ssh cert auth retargeted to master #18896
Feature/updated ssh cert auth retargeted to master #18896
Conversation
e6c885a
to
6c46c9d
Compare
@jbardin @apparentlymart I'm back! I reopened this PR and rebased 0.12-dev |
and updated based on the old commits |
@apparentlymart i'll retarget this to master. Most of the newer work is on here. I left my last job at paxos. |
885ada5
to
8292668
Compare
@jbardin @apparentlymart anybody able to look at this? I've updated it for master and also addressed the comments. :) |
8292668
to
b680e33
Compare
@mildwonkey @apparentlymart anybody :) can i get a look at this? |
Hi @sherodtaylor, Sorry about the delay here. I do intend to review this soon. Once we get master into a stable state, then I'll be going through the backlog of pending PRs. |
@jbardin hi thanks for the response i was wondering how long until this would take? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating the PR!
This needs another rebase to resolve some conflicts. Probably because we recently made sure to remove the private key info from the error messages.
@@ -77,6 +77,11 @@ provisioner "file" { | |||
[the `file` function](/docs/configuration/functions/file.html). This takes | |||
preference over the password if provided. | |||
|
|||
* `certificate` - The contents of an signed CA Certificate. The certificate argument must be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: a signed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
* `certificate` - The contents of an signed CA Certificate. The certificate argument must be | ||
used in conjunction with a `private_key`. These can | ||
be loaded from a file on disk using the [`file()` interpolation | ||
function](/docs/configuration/interpolation.html#file_path_). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the same markdown link from above, and remove the newline in the link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
b680e33
to
c456d96
Compare
@jbardin hey I updated the pr |
Thanks @sherodtaylor! |
@jbardin np :) |
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This pull request addresses #14413. It adds certificate support for the ssh provisioner. It adds a new parameter certificate which is used to authenticate ssh sessions. I updated the provisioner_test and docs to reflect this. #18017. Also updated the communicator test to validate ssh certificates. Since this was blocked by dev I decided up create this pr for dev. Reopened this as it was closed #18421