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

Configurable SSH port #508

Merged
merged 2 commits into from
Nov 6, 2023
Merged

Configurable SSH port #508

merged 2 commits into from
Nov 6, 2023

Conversation

leonvogt
Copy link
Contributor

@leonvogt leonvogt commented Oct 1, 2023

One of the machines I wanted to deploy to had a custom SSH port.
I think it would be handy if you could easily specify the SSH port, similar to the SSH user.
This PR would allow a port configuration like this:

ssh:
  port: 2222

@djmb
Copy link
Collaborator

djmb commented Oct 30, 2023

Thanks for the PR @leonvogt!

There's one failing test, could you resolve it? 🙏

2023-10-30T08:41:42.1135161Z Failure:
2023-10-30T08:41:42.1136000Z MainTest#test_config [/home/runner/work/kamal/kamal/test/integration/main_test.rb:56]:
2023-10-30T08:41:42.1137067Z --- expected
2023-10-30T08:41:42.1137860Z +++ actual
2023-10-30T08:41:42.1138420Z @@ -1 +1 @@
2023-10-30T08:41:42.1139101Z -{:user=>"root", :keepalive=>true, :keepalive_interval=>30, :log_level=>:fatal}
2023-10-30T08:41:42.1140078Z +{:user=>"root", :port=>22, :keepalive=>true, :keepalive_interval=>30, :log_level=>:fatal}

@leonvogt
Copy link
Contributor Author

leonvogt commented Nov 3, 2023

@djmb
My bad, the integration tests didn't work properly on my machine.
Should be fixed now.

@djmb
Copy link
Collaborator

djmb commented Nov 6, 2023

Thanks for the PR @leonvogt! Tests are all passing now.

If you had an insight into why the integrations test don't run for you, I'd be interested to know!

@djmb djmb merged commit da16144 into basecamp:main Nov 6, 2023
6 checks passed
@djmb
Copy link
Collaborator

djmb commented Nov 6, 2023

One other thing @leonvogt - could you raise a PR on https://github.com/basecamp/kamal-site to document the new setting? 🙏

@leonvogt
Copy link
Contributor Author

leonvogt commented Nov 6, 2023

One other thing @leonvogt - could you raise a PR on https://github.com/basecamp/kamal-site to document the new setting? 🙏

Sure thing, I'll do

@leonvogt
Copy link
Contributor Author

leonvogt commented Nov 6, 2023

If you had an insight into why the integrations test don't run for you, I'd be interested to know!

It fails at line 7 in the integration_test.
I'll get the following Docker output:

[...]
Step 10/20 : COPY *.sh .
When using COPY with more than one source file, the destination must be a directory and end with a /

The solution for me was to change COPY *.sh . to COPY *.sh ./ in the Dockerfile.
I'm not sure why I had to do this and other people don't. Therefore I didn't open a Issue or PR for that.

@leonvogt leonvogt deleted the ssh-port-option branch December 19, 2023 17:18
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.

3 participants