-
Notifications
You must be signed in to change notification settings - Fork 177
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
[WIP] Allow ssh key override #87
base: master
Are you sure you want to change the base?
Conversation
@aelsabbahy well done, this is a very good contribution! Thank you. Just a couple of naming nits and we can merge this.. |
@@ -22,6 +22,7 @@ Stack Up is a simple deployment tool that performs given set of commands on mult | |||
| Option | Description | | |||
|-------------------|----------------------------------| | |||
| `-f Supfile` | Custom path to Supfile | | |||
| `-i`, `sshKey` | Set the the ssh key to use | |
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.
They call it identity_file
in ssh command. I'm thinking if we should be consistent with them.
Want it just changed in the docs or do you want the variable to also be renamed in the code? Also, I take it |
@@ -26,6 +26,7 @@ type Network struct { | |||
Env EnvList `yaml:"env"` | |||
Inventory string `yaml:"inventory"` | |||
Hosts []string `yaml:"hosts"` | |||
SSHKey string `yaml:"ssh-key"` |
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.
We're trying to avoid dashes and underscores in the Supfile API. Can we think of one word here?
identity ... or sshkey ... any other suggestions?
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.
IdentityFile
is more verbose, but would match 1-1 with ~/.ssh/config
syntax, thoughts?
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.
+1
Sorry for the delay on my end.. been pretty busy and going on vacation.. I'll pick this back up in a few weeks. |
@aelsabbahy no worries, thanks a lot! |
Anyone wants to finish this PR based on the above comments? btw: Related PR: #123 |
@VojtechVitek Could I send a new PR to finish it (based on the PR and above comments)? Currently, I want to add some features (#128):
Could you give some advice? thank you! |
@kadefor would be great if you could split it into separate PRs :) Thanks! |
@kadefor Just a heads up if considering updating the /x/crypto/ssh package. In 2017 the Go team made a breaking change to ssh.ClientConfig. (tl;dr, must explicitly specify ssh.HostKeyCallback) |
Implement the "easy" solution requested in #86.
This adds two features:
-i
flag