Skip to content
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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Burgestrand
Copy link

@Burgestrand Burgestrand commented Oct 17, 2024

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 a config/ssh_config with my project and have Kamal use it, instead of having to modify my global ~/.ssh/config.

There are two changes in this PR:

  • Actually pass ssh/config through to the SSH options, so that the documentation is accurate.
  • Modify the validation to allow a string or an array of string, on top of the boolean.

The second change allows passing the path to an ssh_config-file, similar to this one:

Host web
  HostName 8.8.8.8
  User kamal

Host accessories
  HostName 10.0.0.3
  User kamal
  ProxyJump web

... 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

  • Main question: is this change wanted?
  • Add tests to ensure this keeps working.

…rings

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.
@Burgestrand Burgestrand force-pushed the kbs/allow-ssh_options-in-ssh-config branch from 14cde2c to 7abd339 Compare October 17, 2024 18:08
@Burgestrand
Copy link
Author

FYI I force-pushed, because I added tests to each commit.

@Burgestrand
Copy link
Author

Burgestrand commented Oct 17, 2024

I've noticed that certain commands (e.g. app exec -i, logs -f) doesn't go through SSHKit, so this change isn't complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant