-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adds ssm:copy command to allow for scp over ssm #13
base: master
Are you sure you want to change the base?
Conversation
@kubatek94 :) |
Sorry @spamoom, somehow I missed it. I will review early next week. |
use Symfony\Component\Console\Input\InputOption; | ||
use Symfony\Component\Console\Output\OutputInterface; | ||
|
||
class AwsSsmCopy extends AwsSsmConnect |
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.
I can see few improvements that could be made.
To reduce the copy-pasta, I think the main AwsSssConnect command could use a template method pattern. We can have a protected method "composeProcessComand" which returns the "command array" (passed to the withCommand method) that is called from within handle method. This way AwsSsmCopy could just override the configure + composeProcessCommand with the common code staying in one place. However since we plan to rewrite in Go anyway, I think it's less of a concern and we can focus on making the command user friendly, which brings me to the next point.
I think this command should be as close to actual scp as possible to make it as intuitive (and familiar) as can be. Instead of asking for direction, the direction would be from left to right, source to destination, just like scp and cp. Given that it uses scp underneath, I think we could just proxy arguments and options down to the scp command. In my mind, we can use it just like:
## copy local to remote
netsells aws:ssm:copy ~/Desktop/hello.txt ec2-user@i-e123412:/var/www/
## copy remote to local
netsells aws:ssm:copy ec2-user@i-e123412:/var/www/hello.txt ~/Desktop/
If we proxy the arguments and options, the above should "just work". It will also support options such as '-r' for recursive copying - so bigs wins straightaway.
The only problem remaining, is that we need to know the username and host/instance. The username and instance should be optional, allowing the user to pick them from a list (as is currently). In order to do that, I think we can parse the arguments using parse_url and determine the source/destination + user/host. Few examples:
parse_url('file://ec2-user@i-e123412:/var/www/')
=> [
"scheme" => "file",
"host" => "i-e123412",
"user" => "ec2-user",
"path" => "/var/www/",
]
parse_url('file://:/var/www/')
=> false
parse_url('file://~/Desktop/hello.txt')
=> [
"scheme" => "file",
"host" => "~",
"path" => "/Desktop/hello.txt",
]
Algorithm:
- take argument, prepend 'file://' to it and pass to parse_url
- if result is false, we interactively ask for username + instance, and then know that full path is $username@$hostname$argument
- if result has user + host keys, we set username + instance to these, and proxy the argument as is
- otherwise we just proxy the argument as is
- once we have gone through all arguments, we check if username + instance is set - if not, we error out as we can't determine where to connect to
The above will allow for doing this, when we don't know the server:
## copy local to remote
netsells aws:ssm:copy ~/Desktop/hello.txt :/var/www/
## copy remote to local
netsells aws:ssm:copy :/var/www/hello.txt ~/Desktop/
And of course it will allow for the first version, where the username + host is explicitly passed in.
Perhaps we could do the parsing in a different way, that's just an idea I wanted to put forward. I think starting the path with just colon makes sense to denote remote server, and still is very similar to usual scp.
Please let me know what you think @spamoom
Needed for a few times to do a scp over SSM.
Added a command that takes 90% of stuff from the ssm command and allows for a copy interface of either direction. Also allows for a copy from another server to the SSM server.
This is the first pass so go easy :D
I'm not super happy with the
__localhost__
hack, but I needed a non null value to that the rebuild option had a value to prevent it being asked every time - thoughts?