From 491777221f2b5949f99bcf005552d910c8ed9cce Mon Sep 17 00:00:00 2001 From: Samuel Sieg Date: Thu, 16 Mar 2023 16:15:31 +0100 Subject: [PATCH 1/5] Fix destination label filter --- lib/mrsk/commands/app.rb | 24 +++-- lib/mrsk/commands/base.rb | 1 + test/commands/app_test.rb | 180 ++++++++++++++++++++++++++++---------- 3 files changed, 146 insertions(+), 59 deletions(-) diff --git a/lib/mrsk/commands/app.rb b/lib/mrsk/commands/app.rb index 37d70476..e43276ed 100644 --- a/lib/mrsk/commands/app.rb +++ b/lib/mrsk/commands/app.rb @@ -27,7 +27,7 @@ class Mrsk::Commands::App < Mrsk::Commands::Base end def info - docker :ps, *service_filter_with_destination + docker :ps, *filter_args end @@ -76,13 +76,13 @@ class Mrsk::Commands::App < Mrsk::Commands::Base def current_container_id - docker :ps, "--quiet", *service_filter_with_destination + docker :ps, "--quiet", *filter_args end def current_running_version # FIXME: Find more graceful way to extract the version from "app-version" than using sed and tail! pipe \ - docker(:ps, *service_filter_with_destination, "--format", '"{{.Names}}"'), + docker(:ps, *filter_args, "--format", '"{{.Names}}"'), %(sed 's/-/\\n/g'), "tail -n 1" end @@ -101,7 +101,7 @@ class Mrsk::Commands::App < Mrsk::Commands::Base def list_containers - docker :container, :ls, "--all", *service_filter_with_destination + docker :container, :ls, "--all", *filter_args end def list_container_names @@ -115,7 +115,7 @@ class Mrsk::Commands::App < Mrsk::Commands::Base end def remove_containers - docker :container, :prune, "--force", *service_filter_with_destination + docker :container, :prune, "--force", *filter_args end def list_images @@ -123,7 +123,7 @@ class Mrsk::Commands::App < Mrsk::Commands::Base end def remove_images - docker :image, :prune, "--all", "--force", *service_filter + docker :image, :prune, "--all", "--force", *filter_args end @@ -136,15 +136,13 @@ class Mrsk::Commands::App < Mrsk::Commands::Base container_id_for(container_name: service_with_version_and_destination(version)) end - def service_filter - [ "--filter", "label=service=#{config.service}" ] + def filter_args + argumentize "--filter", filters end - def service_filter_with_destination - if config.destination - service_filter << "label=destination=#{config.destination}" - else - service_filter + def filters + ["label=service=#{config.service}"].tap do |filters| + filters << "label=destination=#{config.destination}" if config.destination end end end diff --git a/lib/mrsk/commands/base.rb b/lib/mrsk/commands/base.rb index e453dddc..38df62e9 100644 --- a/lib/mrsk/commands/base.rb +++ b/lib/mrsk/commands/base.rb @@ -1,6 +1,7 @@ module Mrsk::Commands class Base delegate :redact, to: Mrsk::Utils + delegate :argumentize, to: Mrsk::Utils MAX_LOG_SIZE = "10m" diff --git a/test/commands/app_test.rb b/test/commands/app_test.rb index c1a30f46..34596318 100644 --- a/test/commands/app_test.rb +++ b/test/commands/app_test.rb @@ -5,7 +5,6 @@ class CommandsAppTest < ActiveSupport::TestCase ENV["RAILS_MASTER_KEY"] = "456" @config = { service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" }, servers: [ "1.1.1.1" ], env: { "secret" => [ "RAILS_MASTER_KEY" ] } } - @app = Mrsk::Commands::App.new Mrsk::Configuration.new(@config).tap { |c| c.version = "999" } end teardown do @@ -15,7 +14,7 @@ class CommandsAppTest < ActiveSupport::TestCase test "run" do assert_equal \ "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e MRSK_CONTAINER_NAME=\"app-999\" -e RAILS_MASTER_KEY=\"456\" --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app-retry.retry.attempts=\"5\" --label traefik.http.middlewares.app-retry.retry.initialinterval=\"500ms\" --label traefik.http.routers.app.middlewares=\"app-retry@docker\" dhh/app:999", - @app.run.join(" ") + new_command.run.join(" ") end test "run with volumes" do @@ -23,7 +22,7 @@ class CommandsAppTest < ActiveSupport::TestCase assert_equal \ "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e MRSK_CONTAINER_NAME=\"app-999\" -e RAILS_MASTER_KEY=\"456\" --volume /local/path:/container/path --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/up\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app-retry.retry.attempts=\"5\" --label traefik.http.middlewares.app-retry.retry.initialinterval=\"500ms\" --label traefik.http.routers.app.middlewares=\"app-retry@docker\" dhh/app:999", - @app.run.join(" ") + new_command.run.join(" ") end test "run with custom healthcheck path" do @@ -31,148 +30,237 @@ class CommandsAppTest < ActiveSupport::TestCase assert_equal \ "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e MRSK_CONTAINER_NAME=\"app-999\" -e RAILS_MASTER_KEY=\"456\" --label service=\"app\" --label role=\"web\" --label traefik.http.routers.app.rule=\"PathPrefix(\\`/\\`)\" --label traefik.http.services.app.loadbalancer.healthcheck.path=\"/healthz\" --label traefik.http.services.app.loadbalancer.healthcheck.interval=\"1s\" --label traefik.http.middlewares.app-retry.retry.attempts=\"5\" --label traefik.http.middlewares.app-retry.retry.initialinterval=\"500ms\" --label traefik.http.routers.app.middlewares=\"app-retry@docker\" dhh/app:999", - @app.run.join(" ") + new_command.run.join(" ") end test "run with custom options" do @config[:servers] = { "web" => [ "1.1.1.1" ], "jobs" => { "hosts" => [ "1.1.1.2" ], "cmd" => "bin/jobs", "options" => { "mount" => "somewhere", "cap-add" => true } } } - @app = Mrsk::Commands::App.new Mrsk::Configuration.new(@config).tap { |c| c.version = "999" } assert_equal \ "docker run --detach --restart unless-stopped --log-opt max-size=10m --name app-999 -e MRSK_CONTAINER_NAME=\"app-999\" -e RAILS_MASTER_KEY=\"456\" --label service=\"app\" --label role=\"jobs\" --mount \"somewhere\" --cap-add dhh/app:999 bin/jobs", - @app.run(role: :jobs).join(" ") + new_command.run(role: :jobs).join(" ") end test "start" do assert_equal \ "docker start app-999", - @app.start.join(" ") + new_command.start.join(" ") + end + + test "start with destination" do + @destination = "staging" + assert_equal \ + "docker start app-staging-999", + new_command.start.join(" ") end test "stop" do assert_equal \ "docker ps --quiet --filter label=service=app | xargs docker stop", - @app.stop.join(" ") + new_command.stop.join(" ") + end + + test "stop with version" do + assert_equal \ + "docker container ls --all --filter name=app-123 --quiet | xargs docker stop", + new_command.stop(version: "123").join(" ") end test "info" do assert_equal \ "docker ps --filter label=service=app", - @app.info.join(" ") + new_command.info.join(" ") + end + + test "info with destination" do + @destination = "staging" + assert_equal \ + "docker ps --filter label=service=app --filter label=destination=staging", + new_command.info.join(" ") end test "logs" do assert_equal \ "docker ps --quiet --filter label=service=app | xargs docker logs 2>&1", - @app.logs.join(" ") + new_command.logs.join(" ") assert_equal \ "docker ps --quiet --filter label=service=app | xargs docker logs --since 5m 2>&1", - @app.logs(since: "5m").join(" ") + new_command.logs(since: "5m").join(" ") assert_equal \ "docker ps --quiet --filter label=service=app | xargs docker logs --tail 100 2>&1", - @app.logs(lines: "100").join(" ") + new_command.logs(lines: "100").join(" ") assert_equal \ "docker ps --quiet --filter label=service=app | xargs docker logs --since 5m --tail 100 2>&1", - @app.logs(since: "5m", lines: "100").join(" ") + new_command.logs(since: "5m", lines: "100").join(" ") assert_equal \ "docker ps --quiet --filter label=service=app | xargs docker logs 2>&1 | grep 'my-id'", - @app.logs(grep: "my-id").join(" ") + new_command.logs(grep: "my-id").join(" ") assert_equal \ "docker ps --quiet --filter label=service=app | xargs docker logs --since 5m 2>&1 | grep 'my-id'", - @app.logs(since: "5m", grep: "my-id").join(" ") + new_command.logs(since: "5m", grep: "my-id").join(" ") end test "follow logs" do - @app.stub(:run_over_ssh, ->(cmd, host:) { cmd.join(" ") }) do - assert_equal \ - "docker ps --quiet --filter label=service=app | xargs docker logs --timestamps --tail 10 --follow 2>&1", - @app.follow_logs(host: "app-1") + assert_match \ + "docker ps --quiet --filter label=service=app | xargs docker logs --timestamps --tail 10 --follow 2>&1", + new_command.follow_logs(host: "app-1") - assert_equal \ - "docker ps --quiet --filter label=service=app | xargs docker logs --timestamps --tail 10 --follow 2>&1 | grep \"Completed\"", - @app.follow_logs(host: "app-1", grep: "Completed") - end + assert_match \ + "docker ps --quiet --filter label=service=app | xargs docker logs --timestamps --tail 10 --follow 2>&1 | grep \"Completed\"", + new_command.follow_logs(host: "app-1", grep: "Completed") end test "execute in new container" do assert_equal \ "docker run --rm -e RAILS_MASTER_KEY=\"456\" dhh/app:999 bin/rails db:setup", - @app.execute_in_new_container("bin/rails", "db:setup").join(" ") + new_command.execute_in_new_container("bin/rails", "db:setup").join(" ") end test "execute in existing container" do assert_equal \ "docker exec app-999 bin/rails db:setup", - @app.execute_in_existing_container("bin/rails", "db:setup").join(" ") + new_command.execute_in_existing_container("bin/rails", "db:setup").join(" ") end test "execute in new container over ssh" do - @app.stub(:run_over_ssh, ->(cmd, host:) { cmd.join(" ") }) do - assert_match %r|docker run -it --rm -e RAILS_MASTER_KEY=\"456\" dhh/app:999 bin/rails c|, - @app.execute_in_new_container_over_ssh("bin/rails", "c", host: "app-1") - end + assert_match %r|docker run -it --rm -e RAILS_MASTER_KEY=\"456\" dhh/app:999 bin/rails c|, + new_command.execute_in_new_container_over_ssh("bin/rails", "c", host: "app-1") end test "execute in existing container over ssh" do - @app.stub(:run_over_ssh, ->(cmd, host:) { cmd.join(" ") }) do - assert_match %r|docker exec -it app-999 bin/rails c|, - @app.execute_in_existing_container_over_ssh("bin/rails", "c", host: "app-1") - end + assert_match %r|docker exec -it app-999 bin/rails c|, + new_command.execute_in_existing_container_over_ssh("bin/rails", "c", host: "app-1") end test "run over ssh" do - assert_equal "ssh -t root@1.1.1.1 'ls'", @app.run_over_ssh("ls", host: "1.1.1.1") + assert_equal "ssh -t root@1.1.1.1 'ls'", new_command.run_over_ssh("ls", host: "1.1.1.1") end test "run over ssh with custom user" do - @app = Mrsk::Commands::App.new Mrsk::Configuration.new(@config.tap { |c| c[:ssh] = { "user" => "app" } }) - assert_equal "ssh -t app@1.1.1.1 'ls'", @app.run_over_ssh("ls", host: "1.1.1.1") + @config[:ssh] = { "user" => "app" } + assert_equal "ssh -t app@1.1.1.1 'ls'", new_command.run_over_ssh("ls", host: "1.1.1.1") end test "run over ssh with proxy" do - @app = Mrsk::Commands::App.new Mrsk::Configuration.new(@config.tap { |c| c[:ssh] = { "proxy" => "2.2.2.2" } }) - assert_equal "ssh -J root@2.2.2.2 -t root@1.1.1.1 'ls'", @app.run_over_ssh("ls", host: "1.1.1.1") + @config[:ssh] = { "proxy" => "2.2.2.2" } + assert_equal "ssh -J root@2.2.2.2 -t root@1.1.1.1 'ls'", new_command.run_over_ssh("ls", host: "1.1.1.1") end test "run over ssh with proxy user" do - @app = Mrsk::Commands::App.new Mrsk::Configuration.new(@config.tap { |c| c[:ssh] = { "proxy" => "app@2.2.2.2" } }) - assert_equal "ssh -J app@2.2.2.2 -t root@1.1.1.1 'ls'", @app.run_over_ssh("ls", host: "1.1.1.1") + @config[:ssh] = { "proxy" => "app@2.2.2.2" } + assert_equal "ssh -J app@2.2.2.2 -t root@1.1.1.1 'ls'", new_command.run_over_ssh("ls", host: "1.1.1.1") end test "run over ssh with custom user with proxy" do - @app = Mrsk::Commands::App.new Mrsk::Configuration.new(@config.tap { |c| c[:ssh] = { "user" => "app", "proxy" => "2.2.2.2" } }) - assert_equal "ssh -J root@2.2.2.2 -t app@1.1.1.1 'ls'", @app.run_over_ssh("ls", host: "1.1.1.1") + @config[:ssh] = { "user" => "app", "proxy" => "2.2.2.2" } + assert_equal "ssh -J root@2.2.2.2 -t app@1.1.1.1 'ls'", new_command.run_over_ssh("ls", host: "1.1.1.1") end test "current_container_id" do assert_equal \ "docker ps --quiet --filter label=service=app", - @app.current_container_id.join(" ") + new_command.current_container_id.join(" ") + end + + test "current_container_id with destination" do + @destination = "staging" + assert_equal \ + "docker ps --quiet --filter label=service=app --filter label=destination=staging", + new_command.current_container_id.join(" ") end test "container_id_for" do assert_equal \ "docker container ls --all --filter name=app-999 --quiet", - @app.container_id_for(container_name: "app-999").join(" ") + new_command.container_id_for(container_name: "app-999").join(" ") end test "current_running_version" do assert_equal \ "docker ps --filter label=service=app --format \"{{.Names}}\" | sed 's/-/\\n/g' | tail -n 1", - @app.current_running_version.join(" ") + new_command.current_running_version.join(" ") end test "most_recent_version_from_available_images" do assert_equal \ "docker image ls --format \"{{.Tag}}\" dhh/app | head -n 1", - @app.most_recent_version_from_available_images.join(" ") + new_command.most_recent_version_from_available_images.join(" ") end + + test "list_containers" do + assert_equal \ + "docker container ls --all --filter label=service=app", + new_command.list_containers.join(" ") + end + + test "list_containers with destination" do + @destination = "staging" + assert_equal \ + "docker container ls --all --filter label=service=app --filter label=destination=staging", + new_command.list_containers.join(" ") + end + + test "list_container_names" do + assert_equal \ + "docker container ls --all --filter label=service=app --format '{{ .Names }}'", + new_command.list_container_names.join(" ") + end + + test "remove_container" do + assert_equal \ + "docker container ls --all --filter name=app-999 --quiet | xargs docker container rm", + new_command.remove_container(version: "999").join(" ") + end + + test "remove_container with destination" do + @destination = "staging" + assert_equal \ + "docker container ls --all --filter name=app-staging-999 --quiet | xargs docker container rm", + new_command.remove_container(version: "999").join(" ") + end + + test "remove_containers" do + assert_equal \ + "docker container prune --force --filter label=service=app", + new_command.remove_containers.join(" ") + end + + test "remove_containers with destination" do + @destination = "staging" + assert_equal \ + "docker container prune --force --filter label=service=app --filter label=destination=staging", + new_command.remove_containers.join(" ") + end + + test "list_images" do + assert_equal \ + "docker image ls dhh/app", + new_command.list_images.join(" ") + end + + test "remove_images" do + assert_equal \ + "docker image prune --all --force --filter label=service=app", + new_command.remove_images.join(" ") + end + + test "remove_images with destination" do + @destination = "staging" + assert_equal \ + "docker image prune --all --force --filter label=service=app --filter label=destination=staging", + new_command.remove_images.join(" ") + end + + private + def new_command + Mrsk::Commands::App.new(Mrsk::Configuration.new(@config, destination: @destination, version: "999")) + end end From 2de5250486d6338ed06abd9658ca53d03275887e Mon Sep 17 00:00:00 2001 From: Samuel Sieg Date: Fri, 17 Mar 2023 16:29:25 +0100 Subject: [PATCH 2/5] Add tests for `Mrsk::Utils` --- test/utils_test.rb | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 test/utils_test.rb diff --git a/test/utils_test.rb b/test/utils_test.rb new file mode 100644 index 00000000..0588ecb9 --- /dev/null +++ b/test/utils_test.rb @@ -0,0 +1,38 @@ +require "test_helper" + +class UtilsTest < ActiveSupport::TestCase + test "argumentize" do + assert_equal [ "--label", "foo=\"\\`bar\\`\"", "--label", "baz=\"qux\"", "--label", :quux ], \ + Mrsk::Utils.argumentize("--label", { foo: "`bar`", baz: "qux", quux: nil }) + end + + test "argumentize with redacted" do + assert_kind_of SSHKit::Redaction, \ + Mrsk::Utils.argumentize("--label", { foo: "bar" }, redacted: true).last + end + + test "argumentize_env_with_secrets" do + ENV.expects(:fetch).with("FOO").returns("secret") + assert_equal [ "-e", "FOO=\"secret\"", "-e", "BAZ=\"qux\"" ], \ + Mrsk::Utils.argumentize_env_with_secrets({ "secret" => [ "FOO" ], "clear" => { BAZ: "qux" } }) + end + + test "optionize" do + assert_equal [ "--foo", "\"bar\"", "--baz", "\"qux\"", "--quux" ], \ + Mrsk::Utils.optionize({ foo: "bar", baz: "qux", quux: true }) + end + + test "optionize with" do + assert_equal [ "--foo=\"bar\"", "--baz=\"qux\"", "--quux" ], \ + Mrsk::Utils.optionize({ foo: "bar", baz: "qux", quux: true }, with: "=") + end + + test "redact" do + assert_kind_of SSHKit::Redaction, Mrsk::Utils.redact("secret") + assert_equal "secret", Mrsk::Utils.redact("secret") + end + + test "escape_shell_value" do + assert_equal "\"\\`foo\\`\"", Mrsk::Utils.escape_shell_value("`foo`") + end +end From c3d0382935bbceb6f1d171d58c3de82ed6f7ce64 Mon Sep 17 00:00:00 2001 From: Samuel Sieg Date: Fri, 17 Mar 2023 16:31:10 +0100 Subject: [PATCH 3/5] Add another assertion for `escape_shell_value` --- test/utils_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/utils_test.rb b/test/utils_test.rb index 0588ecb9..f2af428b 100644 --- a/test/utils_test.rb +++ b/test/utils_test.rb @@ -33,6 +33,7 @@ class UtilsTest < ActiveSupport::TestCase end test "escape_shell_value" do + assert_equal "\"foo\"", Mrsk::Utils.escape_shell_value("foo") assert_equal "\"\\`foo\\`\"", Mrsk::Utils.escape_shell_value("`foo`") end end From 49d60a045a6d064a84eab61bc35d374b55580a34 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 21 Mar 2023 12:41:28 +0100 Subject: [PATCH 4/5] Style --- lib/mrsk/commands/base.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/mrsk/commands/base.rb b/lib/mrsk/commands/base.rb index 38df62e9..d84f4dc3 100644 --- a/lib/mrsk/commands/base.rb +++ b/lib/mrsk/commands/base.rb @@ -1,7 +1,6 @@ module Mrsk::Commands class Base - delegate :redact, to: Mrsk::Utils - delegate :argumentize, to: Mrsk::Utils + delegate :redact, :argumentize, to: Mrsk::Utils MAX_LOG_SIZE = "10m" From 790be0f5f3665ea246d61ac41a869eb6274e8144 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 21 Mar 2023 12:42:04 +0100 Subject: [PATCH 5/5] Style --- lib/mrsk/commands/app.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mrsk/commands/app.rb b/lib/mrsk/commands/app.rb index e43276ed..e650217a 100644 --- a/lib/mrsk/commands/app.rb +++ b/lib/mrsk/commands/app.rb @@ -141,7 +141,7 @@ class Mrsk::Commands::App < Mrsk::Commands::Base end def filters - ["label=service=#{config.service}"].tap do |filters| + [ "label=service=#{config.service}" ].tap do |filters| filters << "label=destination=#{config.destination}" if config.destination end end