legacy-cli icon indicating copy to clipboard operation
legacy-cli copied to clipboard

Adding ssh-config command

Open hanoii opened this issue 7 years ago • 18 comments

I suggested this on Slack and I was told it would be a good PR.

So far it's looking good, simple and working.

I am starting the PR for an early review and any guidelines/comments.

The only extra thing I'd like to do is to also add tunnel information to the ssh config. This could be simple-ish, like providing ports for the forward, or a bit more complex and attempt to parse the actual ~/.ssh/.config to make sure local ports are not being re-used. Not sure if it's worth actually doing that.

hanoii avatar Jul 21 '16 14:07 hanoii

@pjcdawkins see an initial tunnel addition, I I can attempt to parse .ssh/config, thing is a good idea? thoughts on this?

hanoii avatar Jul 21 '16 16:07 hanoii

Another option I am considering, instead of parsing .ssh/config.. is simply storing locally the previously used ports for each combination of alias.environment, so that new calls to ssh-config for the same alias.environment would yield the same ports all the time but new tunnels would get new port numbers.

I am thinking wether I should store ports for alias.environment or better project.environment so that the same port could be used with different alias. I think this is even better.

hanoii avatar Jul 21 '16 16:07 hanoii

Well, I ended up working out the latter alternative as I think it's best. No need to parse anything out of a file that the cli uses. If there are other custom config that uses the same port, it'd be up for the user to tweak the others.

With this, the same por will be use for the same relationship until some key changes, in which case the port will change.

I actually think that loadCommandConfig and writeCommandConfig can be slightly reworked and put into CommandBase so that any command could leverage from this option. Filename could simply be the command name.

hanoii avatar Jul 21 '16 20:07 hanoii

If interested in that, I can provide either a wider PR on this one, or a separate one if this is accepted as is.

hanoii avatar Jul 21 '16 20:07 hanoii

I should also note that having this in ssh allows for bash/shell completion, which is pretty useful.

hanoii avatar Jul 25 '16 18:07 hanoii

Just bumping this one, been using it so far pretty nicely, so if possible to be accepted great, so I don't have to use my fork for when I need this.

hanoii avatar Aug 04 '16 19:08 hanoii

@pjcdawkins just picking up on this, I rebased the commits and changed the writing of the configs into the already present config.yaml file.. relying entirely on CliConfig.

Let me know wha you think.

hanoii avatar Sep 16 '16 13:09 hanoii

I'll look into the code changes.. the reason I went for making the ports used persistent is because I thought that once you get your ssh config and your tunnels setup, say with a sql client of the sort or whatever, you might want to always access the same database at that port. I also found myself branching a development environment and then maybe destroying it, to branch it again some time in the future, and I thought as a nice addition to always get the same ports for the same environment. And as I needed to at least store persistently the last used port, I thought of storing that as well.

I first did it on a per-command file, but found myself replicating a bit of the logic that was already there for storing/reading the config file, so that's why I switched to that, but a per command file would be OK as well, and I'd do it in CommandBase so that any command could use it.

hanoii avatar Sep 22 '16 18:09 hanoii

Oops, closed it accidentally.

hanoii avatar Sep 22 '16 18:09 hanoii

Hang on, I do think that the global config file is better than a per-command file, if it's decided that this info should be stored in a user-editable YAML file. I'm just not sure it should be exposed to the user like that via YAML. Also, maybe it should be overridable from a command option.

pjcdawkins avatar Sep 22 '16 19:09 pjcdawkins

In this particular command, maybe it's not that much of a big deal, I guess you could always output the same ports on the relationships and then edit it yourself manually in the ssh config, at least that makes it for a simpler addition and we can discuss/analyse the option of making the ports persistent on some other issue/PR? But sure, I'll hang and let me know what you think.

hanoii avatar Sep 22 '16 19:09 hanoii

@pjcdawkins well, for what it's worth, I changed for storing a state json file per command. Kind of what tunnel does, which could be refactor into using this same logic. I attempted to remove all of that but for me it makes sense so that the ports are not repeated.

Also addressed your reviews.

hanoii avatar Oct 08 '16 03:10 hanoii

Just rebased the code on top of the latest changes.

hanoii avatar Oct 28 '16 20:10 hanoii

I guess this PR could use a rebase, not sure if you are happy with merge commits with all intermediate ones or prefer to squash some.

hanoii avatar Oct 28 '16 20:10 hanoii

@hanoii when/if you have time could you write a short description as to how/when to use this?

BTW, I'm using the cache for the ports info, mainly because it's already a "service" and I didn't want to add to CommandBase. The disadvantages are that it can't be user-edited, and it would be flushed by platform cc. I'm not certain how bad that is... I figure the user would not want to hardcode much of this info anyway (the user would need a way to refresh it often, and so I think appending to ~/.ssh/config would at least make it difficult to use... hm...)

pjcdawkins avatar Dec 30 '16 14:12 pjcdawkins

@pjcdawkins i guess this is more a personal preference than anything else, but if it would be as temporary as a cache, you might be better off not caching it at all.

I rely on .ssh/config heavily, which means I configure shortcuts and tunnel information there rather than command line or even platform ssh or platform tunnel:open for that matter. It saves me time and keystrokes. I like to do something like ssh project.environment, and know that when logged in, I have all tunnels open. This can be used with scp as well. Then you are able to configure your local clients (mysql, redis, etc) and access them consistently. I'd say that's the use case.

I really never append it to .ssh/config, but rather copying and pasting. The reason that I wanted the ports to be always the same is because I configure, say mysql client, to access a specific port in localhost. If that port changes for whatever reason, I would have to change configuration. It's not a major thing and I don't think it will really be an issue. It's simply saving time when working with several projects and knowing each port you get from ssh-config would be unique within your workstation.

hanoii avatar Dec 30 '16 14:12 hanoii

I see. It is cached permanently, unless you run platform clear-cache... still I guess that's not quite enough.

I like to do something like ssh project.environment, and know that when logged in, I have all tunnels open.

We could implement something like:

platform ssh --local-forward --local-port-start 30000

which would run

ssh -o 'LocalForward 30000 database.internal:3306' etc.

pjcdawkins avatar Dec 30 '16 15:12 pjcdawkins

You could, but the whole point of running ssh-config and adding it your .ssh/config is not to do that and simply do:

ssh project.master

And that's it, you can then connect your local mysql client to it, your redis local client, etc. If ports overlap, you can accidentally connect to a mysql database of different project, or you might need to have different connections settings sharing the same port.

But really, it's just a personal thing.

Another option is to simplify it even more and use random ports within a range, say 30000-40000 or wider, and that's in. Fine tuning can be done on per use case basis by the end user as needed.

I actually think this last might be best, no cache, no state, random ports and manual tweaks. It's a shortcut not to have to add all this information manually to your ssh config. Specially when you have several environments per project. At least it gets the feature in, and then there's room to improve it if more use cases or requests are around that.

hanoii avatar Dec 30 '16 15:12 hanoii