From 2bffc3bc74417bf80b162a21c68f0827f5b36680 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dorian=20Mari=C3=A9?= Date: Fri, 1 Mar 2024 09:54:06 +0100 Subject: [PATCH 1/5] Replace \`service\` by 'service' so it doesn't get executed by bash Fixes #694 --- lib/kamal/commands/builder/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kamal/commands/builder/base.rb b/lib/kamal/commands/builder/base.rb index 8723fa45..787ea6f3 100644 --- a/lib/kamal/commands/builder/base.rb +++ b/lib/kamal/commands/builder/base.rb @@ -24,7 +24,7 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base def validate_image pipe \ docker(:inspect, "-f", "'{{ .Config.Labels.service }}'", config.absolute_image), - [:grep, "-x", config.service, "||", "(echo \"Image #{config.absolute_image} is missing the `service` label\" && exit 1)"] + [:grep, "-x", config.service, "||", "(echo \"Image #{config.absolute_image} is missing the 'service' label\" && exit 1)"] end From 2be397b679a6b5a259aa988c24b77302ca1af4b8 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 6 Mar 2024 11:02:46 +0000 Subject: [PATCH 2/5] Escape the docker registry username and password Fixes: https://github.com/basecamp/kamal/issues/278 --- lib/kamal/commands/registry.rb | 5 ++++- test/commands/registry_test.rb | 17 ++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/kamal/commands/registry.rb b/lib/kamal/commands/registry.rb index eb02acad..e13c90ac 100644 --- a/lib/kamal/commands/registry.rb +++ b/lib/kamal/commands/registry.rb @@ -2,7 +2,10 @@ class Kamal::Commands::Registry < Kamal::Commands::Base delegate :registry, to: :config def login - docker :login, registry["server"], "-u", sensitive(lookup("username")), "-p", sensitive(lookup("password")) + docker :login, + registry["server"], + "-u", sensitive(Kamal::Utils.escape_shell_value(lookup("username"))), + "-p", sensitive(Kamal::Utils.escape_shell_value(lookup("password"))) end def logout diff --git a/test/commands/registry_test.rb b/test/commands/registry_test.rb index 83b73a7b..c25d7585 100755 --- a/test/commands/registry_test.rb +++ b/test/commands/registry_test.rb @@ -15,7 +15,7 @@ class CommandsRegistryTest < ActiveSupport::TestCase test "registry login" do assert_equal \ - "docker login hub.docker.com -u dhh -p secret", + "docker login hub.docker.com -u \"dhh\" -p \"secret\"", @registry.login.join(" ") end @@ -24,7 +24,18 @@ class CommandsRegistryTest < ActiveSupport::TestCase @config[:registry]["password"] = [ "KAMAL_REGISTRY_PASSWORD" ] assert_equal \ - "docker login hub.docker.com -u dhh -p more-secret", + "docker login hub.docker.com -u \"dhh\" -p \"more-secret\"", + @registry.login.join(" ") + ensure + ENV.delete("KAMAL_REGISTRY_PASSWORD") + end + + test "registry login escape password" do + ENV["KAMAL_REGISTRY_PASSWORD"] = "more-secret'\"" + @config[:registry]["password"] = [ "KAMAL_REGISTRY_PASSWORD" ] + + assert_equal \ + "docker login hub.docker.com -u \"dhh\" -p \"more-secret'\\\"\"", @registry.login.join(" ") ensure ENV.delete("KAMAL_REGISTRY_PASSWORD") @@ -35,7 +46,7 @@ class CommandsRegistryTest < ActiveSupport::TestCase @config[:registry]["username"] = [ "KAMAL_REGISTRY_USERNAME" ] assert_equal \ - "docker login hub.docker.com -u also-secret -p secret", + "docker login hub.docker.com -u \"also-secret\" -p \"secret\"", @registry.login.join(" ") ensure ENV.delete("KAMAL_REGISTRY_USERNAME") From 8e2184d65e2bcbbf092699aae024083eeabfc850 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 6 Mar 2024 14:59:26 +0000 Subject: [PATCH 3/5] Ensure `kamal remove` completes without setup If `kamal setup` has not run or errored out part way through, `kamal remove` should still complete. Fixes: https://github.com/basecamp/kamal/issues/629 --- lib/kamal/cli/traefik.rb | 4 ++-- test/integration/main_test.rb | 31 +++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lib/kamal/cli/traefik.rb b/lib/kamal/cli/traefik.rb index e835b1d8..c67dc69e 100644 --- a/lib/kamal/cli/traefik.rb +++ b/lib/kamal/cli/traefik.rb @@ -20,7 +20,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base on(hosts) do execute *KAMAL.auditor.record("Rebooted traefik"), verbosity: :debug execute *KAMAL.registry.login - execute *KAMAL.traefik.stop + execute *KAMAL.traefik.stop, raise_on_non_zero_exit: false execute *KAMAL.traefik.remove_container execute *KAMAL.traefik.run end @@ -44,7 +44,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base mutating do on(KAMAL.traefik_hosts) do execute *KAMAL.auditor.record("Stopped traefik"), verbosity: :debug - execute *KAMAL.traefik.stop + execute *KAMAL.traefik.stop, raise_on_non_zero_exit: false end end end diff --git a/test/integration/main_test.rb b/test/integration/main_test.rb index 856781cb..22f60860 100644 --- a/test/integration/main_test.rb +++ b/test/integration/main_test.rb @@ -60,6 +60,19 @@ class MainTest < IntegrationTest assert_equal({ "path" => "/up", "port" => 3000, "max_attempts" => 7, "exposed_port" => 3999, "cord"=>"/tmp/kamal-cord", "log_lines" => 50, "cmd"=>"wget -qO- http://localhost > /dev/null || exit 1" }, config[:healthcheck]) end + test "setup and remove" do + # Check remove completes when nothing has been setup yet + kamal :remove, "-y" + assert_no_images_or_containers + + kamal :envify + kamal :setup + assert_images_and_containers + + kamal :remove, "-y" + assert_no_images_or_containers + end + private def assert_local_env_file(contents) assert_equal contents, deployer_exec("cat .env", capture: true) @@ -84,4 +97,22 @@ class MainTest < IntegrationTest assert_equal "200", Net::HTTP.get_response(URI.parse("http://localhost:12345/versions/.hidden")).code end + + def vm1_image_ids + docker_compose("exec vm1 docker image ls -q", capture: true).strip.split("\n") + end + + def vm1_container_ids + docker_compose("exec vm1 docker ps -a -q", capture: true).strip.split("\n") + end + + def assert_no_images_or_containers + assert vm1_image_ids.empty? + assert vm1_container_ids.empty? + end + + def assert_images_and_containers + assert vm1_image_ids.any? + assert vm1_container_ids.any? + end end From c7cfc074b6e483f332d37b574e19fcb9ae74649e Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 6 Mar 2024 15:40:21 +0000 Subject: [PATCH 4/5] Ensure a minimum limit of 1 for % boot strategy Fixes: https://github.com/basecamp/kamal/issues/681 --- lib/kamal/configuration/boot.rb | 2 +- test/commander_test.rb | 6 ++++++ ...deploy_with_low_percentage_boot_strategy.yml | 17 +++++++++++++++++ .../deploy_with_percentage_boot_strategy.yml | 2 +- 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 test/fixtures/deploy_with_low_percentage_boot_strategy.yml diff --git a/lib/kamal/configuration/boot.rb b/lib/kamal/configuration/boot.rb index f6daa1fa..e5cf5a6e 100644 --- a/lib/kamal/configuration/boot.rb +++ b/lib/kamal/configuration/boot.rb @@ -8,7 +8,7 @@ class Kamal::Configuration::Boot limit = @options["limit"] if limit.to_s.end_with?("%") - @host_count * limit.to_i / 100 + [@host_count * limit.to_i / 100, 1].max else limit end diff --git a/test/commander_test.rb b/test/commander_test.rb index 4e848a4b..598fc56b 100644 --- a/test/commander_test.rb +++ b/test/commander_test.rb @@ -109,6 +109,12 @@ class CommanderTest < ActiveSupport::TestCase assert_equal({ in: :groups, limit: 1, wait: 2 }, @kamal.boot_strategy) end + test "percentage-based group strategy limit is at least 1" do + configure_with(:deploy_with_low_percentage_boot_strategy) + + assert_equal({ in: :groups, limit: 1, wait: 2 }, @kamal.boot_strategy) + end + test "try to match the primary role from a list of specific roles" do configure_with(:deploy_primary_web_role_override) diff --git a/test/fixtures/deploy_with_low_percentage_boot_strategy.yml b/test/fixtures/deploy_with_low_percentage_boot_strategy.yml new file mode 100644 index 00000000..9b6a3c64 --- /dev/null +++ b/test/fixtures/deploy_with_low_percentage_boot_strategy.yml @@ -0,0 +1,17 @@ +service: app +image: dhh/app +servers: + web: + - "1.1.1.1" + - "1.1.1.2" + workers: + - "1.1.1.3" + - "1.1.1.4" + +registry: + username: user + password: pw + +boot: + limit: 1% + wait: 2 diff --git a/test/fixtures/deploy_with_percentage_boot_strategy.yml b/test/fixtures/deploy_with_percentage_boot_strategy.yml index eb68a52f..9b6a3c64 100644 --- a/test/fixtures/deploy_with_percentage_boot_strategy.yml +++ b/test/fixtures/deploy_with_percentage_boot_strategy.yml @@ -13,5 +13,5 @@ registry: password: pw boot: - limit: 25% + limit: 1% wait: 2 From dc5af035937f4e48dc0d5a6c9bda01155f972e3f Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 6 Mar 2024 16:03:08 +0000 Subject: [PATCH 5/5] Update tests to match single quotes --- test/cli/build_test.rb | 2 +- test/commands/builder_test.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index cd71e279..082881c5 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -57,7 +57,7 @@ class CliBuildTest < CliTestCase run_command("pull").tap do |output| assert_match /docker image rm --force dhh\/app:999/, output assert_match /docker pull dhh\/app:999/, output - assert_match "docker inspect -f '{{ .Config.Labels.service }}' dhh/app:999 | grep -x app || (echo \"Image dhh/app:999 is missing the `service` label\" && exit 1)", output + assert_match "docker inspect -f '{{ .Config.Labels.service }}' dhh/app:999 | grep -x app || (echo \"Image dhh/app:999 is missing the 'service' label\" && exit 1)", output end end diff --git a/test/commands/builder_test.rb b/test/commands/builder_test.rb index 51bb837c..fe1b879c 100644 --- a/test/commands/builder_test.rb +++ b/test/commands/builder_test.rb @@ -120,7 +120,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase end test "validate image" do - assert_equal "docker inspect -f '{{ .Config.Labels.service }}' dhh/app:123 | grep -x app || (echo \"Image dhh/app:123 is missing the `service` label\" && exit 1)", new_builder_command.validate_image.join(" ") + assert_equal "docker inspect -f '{{ .Config.Labels.service }}' dhh/app:123 | grep -x app || (echo \"Image dhh/app:123 is missing the 'service' label\" && exit 1)", new_builder_command.validate_image.join(" ") end private