Use xargs to handle spaces in proxy options

We cat the options file, append the proxy image and then pass it
to xargs to ensure it handles spaces correctly.

Works better than using eval which can handle spaces but tries
to evaluate things like backticks.

Fixes: https://github.com/basecamp/kamal/issues/1448

lib/kamal/commands/proxy.rb

@@ -2,14 +2,14 @@ class Kamal::Commands::Proxy < Kamal::Commands::Base
   delegate :argumentize, :optionize, to: Kamal::Utils
 
   def run
-    docker :run,
+    pipe \
-      "--name", container_name,
+      [ :echo, "\$\(#{get_boot_options.join(" ")}\) #{config.proxy_image}" ],
-      "--network", "kamal",
+      xargs(docker(:run,
-      "--detach",
+        "--name", container_name,
-      "--restart", "unless-stopped",
+        "--network", "kamal",
-      "--volume", "kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy",
+        "--detach",
-      "\$\(#{get_boot_options.join(" ")}\)",
+        "--restart", "unless-stopped",
-      config.proxy_image
+        "--volume", "kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy"))
   end
 
   def start
@@ -73,7 +73,7 @@ class Kamal::Commands::Proxy < Kamal::Commands::Base
   end
 
   def get_boot_options
-    combine [ :cat, config.proxy_options_file ], [ :echo, "\"#{config.proxy_options_default.join(" ")}\"" ], by: "||"
+    combine [ :cat, config.proxy_options_file, "2>", "/dev/null" ], [ :echo, "\"#{config.proxy_options_default.join(" ")}\"" ], by: "||"
   end
 
   def reset_boot_options

test/cli/proxy_test.rb

@@ -4,7 +4,7 @@ class CliProxyTest < CliTestCase
   test "boot" do
     run_command("boot").tap do |output|
       assert_match "docker login", output
-      assert_match "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") #{KAMAL.config.proxy_image}", output
+      assert_match "echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy", output
     end
   end
 
@@ -18,7 +18,7 @@ class CliProxyTest < CliTestCase
     exception = assert_raises do
       run_command("boot").tap do |output|
         assert_match "docker login", output
-        assert_match "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") #{KAMAL.config.proxy_image}", output
+        assert_match "echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy", output
       end
     end
 
@@ -36,7 +36,7 @@ class CliProxyTest < CliTestCase
 
     run_command("boot").tap do |output|
       assert_match "docker login", output
-      assert_match "docker container start kamal-proxy || docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") #{KAMAL.config.proxy_image}", output
+      assert_match "docker container start kamal-proxy || echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy", output
     end
   ensure
     Thread.report_on_exception = false
@@ -56,12 +56,12 @@ class CliProxyTest < CliTestCase
     run_command("reboot", "-y").tap do |output|
       assert_match "docker container stop kamal-proxy on 1.1.1.1", output
       assert_match "docker container prune --force --filter label=org.opencontainers.image.title=kamal-proxy on 1.1.1.1", output
-      assert_match "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") #{KAMAL.config.proxy_image} on 1.1.1.1", output
+      assert_match "echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy on 1.1.1.1", output
       assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"abcdefabcdef:80\" --deploy-timeout=\"6s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\" on 1.1.1.1", output
 
       assert_match "docker container stop kamal-proxy on 1.1.1.2", output
       assert_match "docker container prune --force --filter label=org.opencontainers.image.title=kamal-proxy on 1.1.1.2", output
-      assert_match "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") #{KAMAL.config.proxy_image} on 1.1.1.2", output
+      assert_match "echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy on 1.1.1.2", output
       assert_match "docker exec kamal-proxy kamal-proxy deploy app-web --target=\"abcdefabcdef:80\" --deploy-timeout=\"6s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\" on 1.1.1.2", output
     end
   end
@@ -196,7 +196,7 @@ class CliProxyTest < CliTestCase
       assert_match "/usr/bin/env mkdir -p .kamal", output
       assert_match "docker network create kamal", output
       assert_match "docker login -u [REDACTED] -p [REDACTED]", output
-      assert_match "docker container start kamal-proxy || docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION}", output
+      assert_match "docker container start kamal-proxy || echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy", output
       assert_match "/usr/bin/env mkdir -p .kamal", output
       assert_match %r{docker rename app-web-latest app-web-latest_replaced_.*}, output
       assert_match "/usr/bin/env mkdir -p .kamal/apps/app/env/roles", output
@@ -316,7 +316,7 @@ class CliProxyTest < CliTestCase
 
   test "boot_config get" do
     SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
-      .with(:cat, ".kamal/proxy/options", "||", :echo, "\"--publish 80:80 --publish 443:443 --log-opt max-size=10m\"")
+      .with(:cat, ".kamal/proxy/options", "2>", "/dev/null", "||", :echo, "\"--publish 80:80 --publish 443:443 --log-opt max-size=10m\"")
       .returns("--publish 80:80 --publish 8443:443 --label=foo=bar")
       .twice
 

test/commands/proxy_test.rb

@@ -15,7 +15,7 @@ class CommandsProxyTest < ActiveSupport::TestCase
 
   test "run" do
     assert_equal \
-      "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION}",
+      "echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy",
       new_command.run.join(" ")
   end
 
@@ -23,7 +23,7 @@ class CommandsProxyTest < ActiveSupport::TestCase
     @config.delete(:proxy)
 
     assert_equal \
-      "docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy $(cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION}",
+      "echo $(cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\") basecamp/kamal-proxy:#{Kamal::Configuration::PROXY_MINIMUM_VERSION} | xargs docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy",
       new_command.run.join(" ")
   end
 
@@ -113,7 +113,7 @@ class CommandsProxyTest < ActiveSupport::TestCase
 
   test "get_boot_options" do
     assert_equal \
-      "cat .kamal/proxy/options || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\"",
+      "cat .kamal/proxy/options 2> /dev/null || echo \"--publish 80:80 --publish 443:443 --log-opt max-size=10m\"",
       new_command.get_boot_options.join(" ")
   end
 

test/integration/docker/deployer/app_with_traefik/.kamal/hooks/pre-deploy

@@ -1,3 +1,4 @@
 kamal proxy boot_config set --publish false \
                             --docker_options label=traefik.http.services.kamal_proxy.loadbalancer.server.scheme=http \
-                                             label=traefik.http.routers.kamal_proxy.rule=PathPrefix\(\`/\`\)
+                                             label=traefik.http.routers.kamal_proxy.rule=PathPrefix\(\`/\`\) \
+                                             sysctl=net.ipv4.ip_local_port_range=\"10000\ 60999\"

test/integration/proxy_test.rb

@@ -46,7 +46,27 @@ class ProxyTest < IntegrationTest
     logs = kamal :proxy, :logs, capture: true
     assert_match /No previous state to restore/, logs
 
+    kamal :proxy, :boot_config, :set, "--docker-options='sysctl net.ipv4.ip_local_port_range=\"10000 60999\"'"
+    assert_docker_options_in_file
+
+    kamal :proxy, :reboot, "-y"
+    assert_docker_options_in_container
+
+    kamal :proxy, :boot_config, :reset
+
     kamal :proxy, :remove
     assert_proxy_not_running
   end
+
+  private
+    def assert_docker_options_in_file
+      boot_config = kamal :proxy, :boot_config, :get, capture: true
+      assert_match "Host vm1: --publish 80:80 --publish 443:443 --log-opt max-size=10m --sysctl net.ipv4.ip_local_port_range=\"10000 60999\"", boot_config
+    end
+
+    def assert_docker_options_in_container
+      assert_equal \
+        "{\"net.ipv4.ip_local_port_range\":\"10000 60999\"}",
+        docker_compose("exec vm1 docker inspect --format '{{ json .HostConfig.Sysctls }}' kamal-proxy", capture: true).strip
+    end
 end