-
Notifications
You must be signed in to change notification settings - Fork 458
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
Allow ssh/config
to accept string, or array of strings
#1129
base: main
Are you sure you want to change the base?
Allow ssh/config
to accept string, or array of strings
#1129
Conversation
14cde2c
to
7abd339
Compare
FYI I force-pushed, because I added tests to each commit. |
This comment was marked as outdated.
This comment was marked as outdated.
@Burgestrand any updates on this? |
On top of that I would say this is still missing the blessing of the maintainers of Kamal, i.e. is this something they'd like to merge and ultimately support. |
I would hope it is blessed by the maintainers because @jeremy linked this PR when I raised an issue abt the incorrect documentation related to ssh config. Is the issue time or other resources? I don't have my ruby merit badge but I'd be willing to sponsor some hours if that's a motivator. |
7abd339
to
d1749ca
Compare
This option is documented as available since basecamp#908 in: https://github.com/basecamp/kamal/blob/74a06b0ccda616c86ebe1729d0795f39bcac9f00/lib/kamal/configuration/docs/ssh.yml#L65-L70 However, before this the options don't seem to pass through to SSHKit: https://github.com/basecamp/kamal/blob/74a06b0ccda616c86ebe1729d0795f39bcac9f00/lib/kamal/commander.rb#L167 Since `config` isn't actually returned in `#options`.
This is accepted by Net::SSH, research done by @jeremy in basecamp#908 (comment) This is already documented as working correctly in https://github.com/basecamp/kamal/blob/74a06b0ccda616c86ebe1729d0795f39bcac9f00/lib/kamal/configuration/docs/ssh.yml#L65-L70 However, before this change only booleans were allowed because of the example configuration file.
`ssh -F` only accepts a single config file, and it's what we use for interactive commands, e.g. `kamal app logs --follow`.
d1749ca
to
25a3f5d
Compare
Motivation and time expenditure! I've been experimenting with Kamal for future projects, but paid client work got in between and I'm not using Kamal for them in the near future 😊 Regardless, I had some time to think about the "interactive commands" problem in the car today and figured "how hard can it be?", so now this PR also has some adjustments so that e.g. In doing that work I noticed the |
Note
I made this PR rogue, in the sense that I forked and made a change that seems to have worked without discussing this change with anybody else first. I don't necessarily expect this to be merged. 🙂
On the topic of passing through
config
I would like to use this option, so that I can embed aconfig/ssh_config
with my project and have Kamal use it, instead of having to modify my global~/.ssh/config
.There are the following changes in this PR:
ssh/config
through to the SSH options, so that the documentation is accurate.run_over_ssh
(shelling out to SSH) to actually use the config file if one is passed, ornone
iffalse
is passedSupporting a string path allows support for using an
ssh_config
-file in Kamal, similar to this:... which then allows you to specify the hosts using the names, rather than IPs. The same file can also be used when SSHing in manually, using
ssh -F ssh_config web
.This is reviewable commit-by-commit.
To do