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

Add proxy boot_config --publish-ip argument #1265

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

Conversation

phoozle
Copy link

@phoozle phoozle commented Nov 30, 2024

Implements basecamp/kamal-proxy#74

Allows for kamal proxy boot_config set --publish-ip=x.x.x.x

Context:
By default Docker will bind to all available interfaces.

@julianrubisch
Copy link

This looks great 👏🏻 will try it out asap, thanks!

lib/kamal/configuration.rb Outdated Show resolved Hide resolved
lib/kamal/cli/proxy.rb Outdated Show resolved Hide resolved
@phoozle
Copy link
Author

phoozle commented Dec 2, 2024

@djmb how would you feel about me allowing support for multiple IP addresses to be specified? Would use Thor's :repeatable option

I was just thinking about the folks that may want to bind to a specific interface that supports both IPv4 and IPv6.

Example:

$ kamal proxy boot_config set --publish-host-ip 127.0.0.1 --publish-host-ip '[::1]'
Host my_host_ip: --publish 127.0.0.1:80:80 --publish 127.0.0.1:443:443 --publish [::1]:80:80 --publish [::1]:443:443 --log-opt max-size=10m

@phoozle
Copy link
Author

phoozle commented Dec 4, 2024

@djmb hey mate, I have just added support for multiple IP addresses. There was a bit of a gotcha with using option default: [ nil ] when also using repeatable: true. Having the default array with nil is needed for the map iteration to run for wildcard interface binding (no IPs specified by user).

option :publish_host_ip, type: :string, repeatable: true, default: [ nil ]
Would produce the following when --publish-host-ip was provided:
(bind_ips => [ nil, "127.0.0.1" ]

So instead, I went ahead and implemented the logic inside #proxy_publish_args with (bind_ips || [ nil ]) and changed publish_host_ip to default to nil instead.

I'm of course open to feedback / alternative solutions on this.

In addition I added a simple IPv4/v6 validator for the argument value (not sure if we want to support this or not).

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