Skip to content

Quote filter names in docker commands #1732

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.

Sign up for GitHub

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

Open
djmb wants to merge 1 commit into
base: main
Choose a base branch
from
+37 −37
Open

Quote filter names in docker commands #1732

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/kamal/commands/base.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def run_over_ssh(*command, host:)
end

def container_id_for(container_name:, only_running: false)
docker :container, :ls, *("--all" unless only_running), "--filter", "name=^#{container_name}$", "--quiet"
docker :container, :ls, *("--all" unless only_running), "--filter", "'name=^#{container_name}$'", "--quiet"
end

def make_directory_for(remote_file)
Expand Down
2 changes: 1 addition & 1 deletion lib/kamal/commands/proxy.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def start_or_run
end

def info
docker :ps, "--filter", "name=^#{container_name}$"
docker :ps, "--filter", "'name=^#{container_name}$'"
end

def version
Expand Down
38 changes: 19 additions & 19 deletions test/cli/app_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class CliAppTest < CliTestCase
run_command("boot").tap do |output|
assert_match "docker tag dhh/app:latest dhh/app:latest", output
assert_match /docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-[0-9a-f]{12} /, output
assert_match "docker container ls --all --filter name=^app-web-123$ --quiet | xargs docker stop", output
assert_match "docker container ls --all --filter 'name=^app-web-123$' --quiet | xargs docker stop", output
end
end

Expand All @@ -15,22 +15,22 @@ class CliAppTest < CliTestCase
run_command("details") # Preheat Kamal const

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet", raise_on_non_zero_exit: false)
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-latest$'", "--quiet", raise_on_non_zero_exit: false)
.returns("12345678") # running version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:sh, "-c", "'docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting --filter ancestor=$(docker image ls --filter reference=dhh/app:latest --format '\\''{{.ID}}'\\'') ; docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting'", "|", :head, "-1", "|", "while read line; do echo ${line#app-web-}; done", raise_on_non_zero_exit: false)
.returns("123") # old version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-latest$'", "--quiet")
.returns("12345678") # running version

run_command("boot").tap do |output|
assert_match /Renaming container .* to .* as already deployed on 1.1.1.1/, output # Rename
assert_match /docker rename app-web-latest app-web-latest_replaced_[0-9a-f]{16}/, output
assert_match /docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-[0-9a-f]{12} /, output
assert_match "docker container ls --all --filter name=^app-web-123$ --quiet | xargs docker stop", output
assert_match "docker container ls --all --filter 'name=^app-web-123$' --quiet | xargs docker stop", output
end
ensure
Thread.report_on_exception = true
Expand Down Expand Up @@ -84,23 +84,23 @@ class CliAppTest < CliTestCase
test "boot with assets" do
Object.any_instance.stubs(:sleep)
SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet", raise_on_non_zero_exit: false)
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-latest$'", "--quiet", raise_on_non_zero_exit: false)
.returns("12345678") # running version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:sh, "-c", "'docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting --filter ancestor=$(docker image ls --filter reference=dhh/app:latest --format '\\''{{.ID}}'\\'') ; docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting'", "|", :head, "-1", "|", "while read line; do echo ${line#app-web-}; done", raise_on_non_zero_exit: false)
.returns("123").twice # old version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-latest$'", "--quiet")
.returns("12345678") # running version

run_command("boot", config: :with_assets).tap do |output|
assert_match "docker tag dhh/app:latest dhh/app:latest", output
assert_match "/usr/bin/env mkdir -p .kamal/apps/app/assets/volumes/web-latest ; cp -rnT .kamal/apps/app/assets/extracted/web-latest .kamal/apps/app/assets/volumes/web-latest ; cp -rnT .kamal/apps/app/assets/extracted/web-latest .kamal/apps/app/assets/volumes/web-123 || true ; cp -rnT .kamal/apps/app/assets/extracted/web-123 .kamal/apps/app/assets/volumes/web-latest || true", output
assert_match "/usr/bin/env mkdir -p .kamal/apps/app/assets/extracted/web-latest && docker container rm app-web-assets 2> /dev/null || true && docker container create --name app-web-assets dhh/app:latest && docker container cp -L app-web-assets:/public/assets/. .kamal/apps/app/assets/extracted/web-latest && docker container rm app-web-assets", output
assert_match /docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-[0-9a-f]{12} /, output
assert_match "docker container ls --all --filter name=^app-web-123$ --quiet | xargs docker stop", output
assert_match "docker container ls --all --filter 'name=^app-web-123$' --quiet | xargs docker stop", output
assert_match "/usr/bin/env find .kamal/apps/app/assets/extracted -maxdepth 1 -name 'web-*' ! -name web-latest -exec rm -rf \"{}\" + ; find .kamal/apps/app/assets/volumes -maxdepth 1 -name 'web-*' ! -name web-latest -exec rm -rf \"{}\" +", output
end
end
Expand All @@ -109,11 +109,11 @@ class CliAppTest < CliTestCase
Object.any_instance.stubs(:sleep)

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet", raise_on_non_zero_exit: false)
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-latest$'", "--quiet", raise_on_non_zero_exit: false)
.returns("12345678") # running version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-latest$'", "--quiet")
.returns("12345678") # running version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
Expand All @@ -123,7 +123,7 @@ class CliAppTest < CliTestCase
run_command("boot", config: :with_env_tags).tap do |output|
assert_match "docker tag dhh/app:latest dhh/app:latest", output
assert_match %r{docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-[0-9a-f]{12} --env KAMAL_CONTAINER_NAME="app-web-latest" --env KAMAL_VERSION="latest" --env KAMAL_HOST="1.1.1.1" --env TEST="root" --env EXPERIMENT="disabled" --env SITE="site1"}, output
assert_match "docker container ls --all --filter name=^app-web-123$ --quiet | xargs docker stop", output
assert_match "docker container ls --all --filter 'name=^app-web-123$' --quiet | xargs docker stop", output
end
end

Expand All @@ -133,7 +133,7 @@ class CliAppTest < CliTestCase
SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info).returns("123") # old version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-workers-latest$", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-workers-latest$'", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.returns("running").at_least_once # workers health check

run_command("boot", config: :with_roles, host: nil).tap do |output|
Expand All @@ -153,7 +153,7 @@ class CliAppTest < CliTestCase

SSHKit::Backend::Abstract.any_instance.stubs(:execute).returns("")
SSHKit::Backend::Abstract.any_instance.stubs(:execute)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-latest$", "--quiet", "|", :xargs, :docker, :stop, raise_on_non_zero_exit: false)
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-latest$'", "--quiet", "|", :xargs, :docker, :stop, raise_on_non_zero_exit: false)
SSHKit::Backend::Abstract.any_instance.expects(:execute)
.with(:docker, :exec, "kamal-proxy", "kamal-proxy", :deploy, "app-web", "--target=\"123:80\"", "--deploy-timeout=\"1s\"", "--drain-timeout=\"30s\"", "--buffer-requests", "--buffer-responses", "--log-request-header=\"Cache-Control\"", "--log-request-header=\"Last-Modified\"", "--log-request-header=\"User-Agent\"").raises(SSHKit::Command::Failed.new("Failed to deploy"))

Expand All @@ -177,7 +177,7 @@ class CliAppTest < CliTestCase
SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info).returns("123") # old version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-workers-latest$", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-workers-latest$'", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.returns("unhealthy").at_least_once # workers health check

run_command("boot", config: :with_roles, host: nil, allow_execute_error: true).tap do |output|
Expand All @@ -200,7 +200,7 @@ class CliAppTest < CliTestCase
SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info).returns("123") # old version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-workers-latest$", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-workers-latest$'", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.returns("running", "stopped").at_least_once # workers health check

run_command("boot", config: :with_roles, host: "1.1.1.3", allow_execute_error: true).tap do |output|
Expand All @@ -216,7 +216,7 @@ class CliAppTest < CliTestCase
SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info).returns("123") # old version

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-workers-latest$", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-workers-latest$'", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.returns("running").at_least_once # workers health check

run_command("boot", config: :with_only_workers, host: nil).tap do |output|
Expand All @@ -232,7 +232,7 @@ class CliAppTest < CliTestCase
assert_match /Uploading .*kamal-error-pages.*\/latest to \.kamal\/proxy\/apps-config\/app\/error_pages/, output
assert_match "docker tag dhh/app:latest dhh/app:latest", output
assert_match /docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-[0-9a-f]{12} /, output
assert_match "docker container ls --all --filter name=^app-web-123$ --quiet | xargs docker stop", output
assert_match "docker container ls --all --filter 'name=^app-web-123$' --quiet | xargs docker stop", output
assert_match "Running /usr/bin/env find .kamal/proxy/apps-config/app/error_pages -mindepth 1 -maxdepth 1 ! -name latest -exec rm -rf {} + on 1.1.1.1", output
end
end
Expand Down Expand Up @@ -293,7 +293,7 @@ class CliAppTest < CliTestCase

run_command("stale_containers", "--stop").tap do |output|
assert_match /Stopping stale container for role web with version 87654321/, output
assert_match /#{Regexp.escape("docker container ls --all --filter name=^app-web-87654321$ --quiet | xargs docker stop")}/, output
assert_match /#{Regexp.escape("docker container ls --all --filter 'name=^app-web-87654321$' --quiet | xargs docker stop")}/, output
end
end

Expand Down Expand Up @@ -337,7 +337,7 @@ class CliAppTest < CliTestCase

test "remove_container" do
run_command("remove_container", "1234567").tap do |output|
assert_match "docker container ls --all --filter name=^app-web-1234567$ --quiet | xargs docker container rm", output
assert_match "docker container ls --all --filter 'name=^app-web-1234567$' --quiet | xargs docker container rm", output
end
end

Expand Down Expand Up @@ -548,7 +548,7 @@ class CliAppTest < CliTestCase
assert_match /docker rename app-web-latest app-web-latest_replaced_[0-9a-f]{16}/, output
assert_match /docker run --detach --restart unless-stopped --name app-web-latest --network kamal --hostname 1.1.1.1-[0-9a-f]{12} --env KAMAL_CONTAINER_NAME="app-web-latest" --env KAMAL_VERSION="latest" --env KAMAL_HOST="1.1.1.1" --env-file .kamal\/apps\/app\/env\/roles\/web.env --log-opt max-size="10m" --label service="app" --label role="web" --label destination dhh\/app:latest/, output
assert_match /docker exec kamal-proxy kamal-proxy deploy app-web --target="123:80"/, output
assert_match "docker container ls --all --filter name=^app-web-123$ --quiet | xargs docker stop", output
assert_match "docker container ls --all --filter 'name=^app-web-123$' --quiet | xargs docker stop", output
end
end

Expand Down
14 changes: 7 additions & 7 deletions test/cli/main_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ class CliMainTest < CliTestCase
run_command("details") # Preheat Kamal const

run_command("rollback", "nonsense").tap do |output|
assert_match /docker container ls --all --filter name=\^app-web-nonsense\$ --quiet/, output
assert_match /docker container ls --all --filter 'name=\^app-web-nonsense\$' --quiet/, output
assert_match /The app version 'nonsense' is not available as a container/, output
end
end
Expand All @@ -333,18 +333,18 @@ class CliMainTest < CliTestCase
Object.any_instance.stubs(:sleep)
[ "web", "workers" ].each do |role|
SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-#{role}-123$", "--quiet", raise_on_non_zero_exit: false)
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-#{role}-123$'", "--quiet", raise_on_non_zero_exit: false)
.returns("").at_least_once
SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-#{role}-123$", "--quiet")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-#{role}-123$'", "--quiet")
.returns("version-to-rollback\n").at_least_once
SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:sh, "-c", "'docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=#{role} --filter status=running --filter status=restarting --filter ancestor=$(docker image ls --filter reference=dhh/app:latest --format '\\''{{.ID}}'\\'') ; docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=#{role} --filter status=running --filter status=restarting'", "|", :head, "-1", "|", "while read line; do echo ${line#app-#{role}-}; done", raise_on_non_zero_exit: false)
.returns("version-to-rollback\n").at_least_once
end

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-workers-123$", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-workers-123$'", "--quiet", "|", :xargs, :docker, :inspect, "--format", "'{{if .State.Health}}{{.State.Health.Status}}{{else}}{{.State.Status}}{{end}}'")
.returns("running").at_least_once # health check

Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true)
Expand All @@ -353,7 +353,7 @@ class CliMainTest < CliTestCase
assert_hook_ran "pre-deploy", output
assert_match "docker tag dhh/app:123 dhh/app:latest", output
assert_match "docker run --detach --restart unless-stopped --name app-web-123", output
assert_match "docker container ls --all --filter name=^app-web-version-to-rollback$ --quiet | xargs docker stop", output, "Should stop the container that was previously running"
assert_match "docker container ls --all --filter 'name=^app-web-version-to-rollback$' --quiet | xargs docker stop", output, "Should stop the container that was previously running"
assert_hook_ran "post-deploy", output
end
end
Expand All @@ -362,10 +362,10 @@ class CliMainTest < CliTestCase
Kamal::Cli::Main.any_instance.stubs(:container_available?).returns(true)

SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-123$", "--quiet", raise_on_non_zero_exit: false)
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-123$'", "--quiet", raise_on_non_zero_exit: false)
.returns("").at_least_once
SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:docker, :container, :ls, "--all", "--filter", "name=^app-web-123$", "--quiet")
.with(:docker, :container, :ls, "--all", "--filter", "'name=^app-web-123$'", "--quiet")
.returns("123").at_least_once
SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info)
.with(:sh, "-c", "'docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting --filter ancestor=$(docker image ls --filter reference=dhh/app:latest --format '\\''{{.ID}}'\\'') ; docker ps --latest --format '\\''{{.Names}}'\\'' --filter label=service=app --filter label=destination= --filter label=role=web --filter status=running --filter status=restarting'", "|", :head, "-1", "|", "while read line; do echo ${line#app-web-}; done", raise_on_non_zero_exit: false)
Expand Down
Loading