-
Notifications
You must be signed in to change notification settings - Fork 57
Conversation
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.
Please, let's focus on the sprint goals. I don't think this is necessary at the moment.
Well, this is necessary for deploying the cluster in my environment. If you have any comments relative to the code please go ahead... |
How so? Just do a |
I'm automating things and that workflow does not match that automation. So do you think there is something wrong with this code? |
Well, sure:
But most important of all, you don't clearly state why an SSH agent is not enough for your needs. We are trying to keep things as simple as possible and stick to the sprint goals. Having these flags is something we explicitly tried to avoid unless we had a valid story of why they are needed, and so far I haven't read anything like that. |
Signed-off-by: Alvaro Saurin <alvaro.saurin@gmail.com>
It is not, password uses capital
My intention was not to ducument this as, as you said, the main workflow is with
I've already told you that I have my automation scripts (that I will not dump in this discussion for sake of brevity) that do not match that workflow.
There are always changes that appear as side-effects of the work in a sprint, things that must be fixed or small improvements that make things easier when developing. It is a developer's dutty to keep focus on his main task, so they don't need to be babysitted. |
You changed this after my comment 🚀 (SUSE/caaspctl@c1b4d93 - SUSE/caaspctl@74746dc) Cannot you do ssh agent forwarding? I still don't see enough evidence to not block this, I'm sorry. |
Maybe @flavio could break the tie here. |
@inercia please pick up the tasks that were assigned to you during sprint planning. |
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.
It's fine to work on little things that were not planned as part of the sprint as long as the end goals of the sprint are matches.
I like the idea to be able to specify the ssh key via cli, I've run into the lack of this feature at the SUSECON booth and I wanted to file an issue about it.
I don't like the idea to supply a password via the command line. ssh authentication via password is a bad security practice and we should not encourage it. Plus it would add issues on our side like, the password would be visible inside of the history and via ps
.
Just remove the code that deals with providing the password from the cli.
Just to have more context; why was this required? What use cases would this cover that the use of a |
I believe this adds extra complexity on our side, e.g. if the private key has a passphrase this change is not enough and Using an I'm a -1 to this change altogether. |
All right, I didn't know about the limitations of Go when it comes to managing ssh keys. Let's leave everything aside until we get clear feedback from our stakeholders. @inercia let's close this PR and re-open it if someone asks for one of these features. In the meantime I've pinged @r0ckarong to understand how much we document about |
This PR adds a
-P<password>
and-i<key>
arguments for using alternative authentication methods for ssh.