From 2c2053558aa5e43425b411b4d5865970f35fc121 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 27 May 2024 11:11:51 +0100 Subject: [PATCH] Handle corrupt git clones When cloning the git repo: 1. Try to clone 2. If there's already a build directory reset it 3. Check the clone is valid If anything goes wrong during that process: 1. Delete the clone directory 2. Clone it again 3. Check the clone is valid Raise any errors after that --- lib/kamal/cli/build.rb | 23 +------ lib/kamal/cli/build/clone.rb | 61 ++++++++++++++++++ lib/kamal/commands/builder.rb | 20 +----- lib/kamal/commands/builder/clone.rb | 28 +++++++++ test/cli/build_test.rb | 95 ++++++++++++++++++++++------- test/cli/main_test.rb | 16 +++++ 6 files changed, 184 insertions(+), 59 deletions(-) create mode 100644 lib/kamal/cli/build/clone.rb create mode 100644 lib/kamal/commands/builder/clone.rb diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 0af76b8c..45b53eb7 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -23,7 +23,9 @@ class Kamal::Cli::Build < Kamal::Cli::Base say "Building from a local git clone, so ignoring these uncommitted changes:\n #{uncommitted_changes}", :yellow end - prepare_clone + run_locally do + Clone.new(self).prepare + end elsif uncommitted_changes.present? say "Building with uncommitted changes:\n #{uncommitted_changes}", :yellow end @@ -126,23 +128,4 @@ class Kamal::Cli::Build < Kamal::Cli::Base end end end - - def prepare_clone - run_locally do - begin - info "Cloning repo into build directory `#{KAMAL.config.builder.build_directory}`..." - - execute *KAMAL.builder.create_clone_directory - execute *KAMAL.builder.clone - rescue SSHKit::Command::Failed => e - if e.message =~ /already exists and is not an empty directory/ - info "Resetting local clone as `#{KAMAL.config.builder.build_directory}` already exists..." - - KAMAL.builder.clone_reset_steps.each { |step| execute *step } - else - raise - end - end - end - end end diff --git a/lib/kamal/cli/build/clone.rb b/lib/kamal/cli/build/clone.rb new file mode 100644 index 00000000..98a1751a --- /dev/null +++ b/lib/kamal/cli/build/clone.rb @@ -0,0 +1,61 @@ +require "uri" + +class Kamal::Cli::Build::Clone + attr_reader :sshkit + delegate :info, :error, :execute, :capture_with_info, to: :sshkit + + def initialize(sshkit) + @sshkit = sshkit + end + + def prepare + begin + clone_repo + rescue SSHKit::Command::Failed => e + if e.message =~ /already exists and is not an empty directory/ + reset + else + raise Kamal::Cli::Build::BuildError, "Failed to clone repo: #{e.message}" + end + end + + validate! + rescue Kamal::Cli::Build::BuildError => e + error "Error preparing clone: #{e.message}, deleting and retrying..." + + FileUtils.rm_rf KAMAL.config.builder.clone_directory + clone_repo + validate! + end + + private + def clone_repo + info "Cloning repo into build directory `#{KAMAL.config.builder.build_directory}`..." + + FileUtils.mkdir_p KAMAL.config.builder.clone_directory + execute *KAMAL.builder.clone + end + + def reset + info "Resetting local clone as `#{KAMAL.config.builder.build_directory}` already exists..." + + KAMAL.builder.clone_reset_steps.each { |step| execute *step } + rescue SSHKit::Command::Failed => e + raise Kamal::Cli::Build::BuildError, "Failed to clone repo: #{e.message}" + end + + def validate! + status = capture_with_info(*KAMAL.builder.clone_status).strip + + unless status.empty? + raise Kamal::Cli::Build::BuildError, "Clone in #{KAMAL.config.builder.build_directory} is dirty, #{status}" + end + + revision = capture_with_info(*KAMAL.builder.clone_revision).strip + if revision != Kamal::Git.revision + raise Kamal::Cli::Build::BuildError, "Clone in #{KAMAL.config.builder.build_directory} is not on the correct revision, expected `#{Kamal::Git.revision}` but got `#{revision}`" + end + rescue SSHKit::Command::Failed => e + raise Kamal::Cli::Build::BuildError, "Failed to validate clone: #{e.message}" + end +end diff --git a/lib/kamal/commands/builder.rb b/lib/kamal/commands/builder.rb index d8fa9fcf..2e5862a8 100644 --- a/lib/kamal/commands/builder.rb +++ b/lib/kamal/commands/builder.rb @@ -2,7 +2,8 @@ require "active_support/core_ext/string/filters" class Kamal::Commands::Builder < Kamal::Commands::Base delegate :create, :remove, :push, :clean, :pull, :info, :validate_image, to: :target - delegate :clone_directory, :build_directory, to: :"config.builder" + + include Clone def name target.class.to_s.remove("Kamal::Commands::Builder::").underscore.inquiry @@ -54,23 +55,6 @@ class Kamal::Commands::Builder < Kamal::Commands::Base end end - def create_clone_directory - make_directory clone_directory - end - - def clone - git :clone, Kamal::Git.root, path: clone_directory - end - - def clone_reset_steps - [ - git(:remote, "set-url", :origin, Kamal::Git.root, path: build_directory), - git(:fetch, :origin, path: build_directory), - git(:reset, "--hard", Kamal::Git.revision, path: build_directory), - git(:clean, "-fdx", path: build_directory) - ] - end - private def ensure_local_docker_installed docker "--version" diff --git a/lib/kamal/commands/builder/clone.rb b/lib/kamal/commands/builder/clone.rb new file mode 100644 index 00000000..b40e9e4e --- /dev/null +++ b/lib/kamal/commands/builder/clone.rb @@ -0,0 +1,28 @@ +module Kamal::Commands::Builder::Clone + extend ActiveSupport::Concern + + included do + delegate :clone_directory, :build_directory, to: :"config.builder" + end + + def clone + git :clone, Kamal::Git.root, path: clone_directory + end + + def clone_reset_steps + [ + git(:remote, "set-url", :origin, Kamal::Git.root, path: build_directory), + git(:fetch, :origin, path: build_directory), + git(:reset, "--hard", Kamal::Git.revision, path: build_directory), + git(:clean, "-fdx", path: build_directory) + ] + end + + def clone_status + git :status, "--porcelain", path: build_directory + end + + def clone_revision + git :"rev-parse", :HEAD, path: build_directory + end +end diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index 606aaf97..b0bbd83b 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -9,10 +9,18 @@ class CliBuildTest < CliTestCase end test "push" do - with_build_directory do + with_build_directory do |build_directory| Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) hook_variables = { version: 999, service_version: "app@999", hosts: "1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", command: "build", subcommand: "push" } + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :"rev-parse", :HEAD) + .returns(Kamal::Git.revision) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :status, "--porcelain") + .returns("") + run_command("push", "--verbose").tap do |output| assert_hook_ran "pre-build", output, **hook_variables assert_match /Cloning repo into build directory/, output @@ -23,28 +31,33 @@ class CliBuildTest < CliTestCase end end - test "push reseting clone" do - with_build_directory do + test "push resetting clone" do + with_build_directory do |build_directory| stub_setup - build_dir = "#{Dir.tmpdir}/kamal-clones/app-#{pwd_sha}/kamal/" - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:docker, "--version", "&&", :docker, :buildx, "version") - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:docker, :buildx, :create, "--use", "--name", "kamal-app-multiarch") + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "--version", "&&", :docker, :buildx, "version") - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:mkdir, "-p", "#{Dir.tmpdir}/kamal-clones/app-#{pwd_sha}") - SSHKit::Backend::Abstract.any_instance.stubs(:execute) + SSHKit::Backend::Abstract.any_instance.expects(:execute) .with(:git, "-C", "#{Dir.tmpdir}/kamal-clones/app-#{pwd_sha}", :clone, Dir.pwd) .raises(SSHKit::Command::Failed.new("fatal: destination path 'kamal' already exists and is not an empty directory")) .then .returns(true) - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:git, "-C", build_dir, :remote, "set-url", :origin, Dir.pwd) - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:git, "-C", build_dir, :fetch, :origin) - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:git, "-C", build_dir, :reset, "--hard", Kamal::Git.revision) - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:git, "-C", build_dir, :clean, "-fdx") + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:git, "-C", build_directory, :remote, "set-url", :origin, Dir.pwd) + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:git, "-C", build_directory, :fetch, :origin) + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:git, "-C", build_directory, :reset, "--hard", Kamal::Git.revision) + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:git, "-C", build_directory, :clean, "-fdx") - SSHKit::Backend::Abstract.any_instance.stubs(:execute) + SSHKit::Backend::Abstract.any_instance.expects(:execute) .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64,linux/arm64", "--builder", "kamal-app-multiarch", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".") + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :"rev-parse", :HEAD) + .returns(Kamal::Git.revision) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :status, "--porcelain") + .returns("") + run_command("push", "--verbose").tap do |output| assert_match /Cloning repo into build directory/, output assert_match /Resetting local clone/, output @@ -64,25 +77,65 @@ class CliBuildTest < CliTestCase end end - test "push without builder" do - with_build_directory do + test "push with corrupt clone" do + with_build_directory do |build_directory| stub_setup - SSHKit::Backend::Abstract.any_instance.stubs(:execute) + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "--version", "&&", :docker, :buildx, "version") + + SSHKit::Backend::Abstract.any_instance.expects(:execute) + .with(:git, "-C", "#{Dir.tmpdir}/kamal-clones/app-#{pwd_sha}", :clone, Dir.pwd) + .raises(SSHKit::Command::Failed.new("fatal: destination path 'kamal' already exists and is not an empty directory")) + .then + .returns(true) + .twice + + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:git, "-C", build_directory, :remote, "set-url", :origin, Dir.pwd) + .raises(SSHKit::Command::Failed.new("fatal: not a git repository")) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :"rev-parse", :HEAD) + .returns(Kamal::Git.revision) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :status, "--porcelain") + .returns("") + + Dir.stubs(:chdir) + + run_command("push", "--verbose") do |output| + assert_match /Cloning repo into build directory `#{build_directory}`\.\.\..*Cloning repo into build directory `#{build_directory}`\.\.\./, output + assert_match "Resetting local clone as `#{build_directory}` already exists...", output + assert_match "Error preparing clone: Failed to clone repo: fatal: not a git repository, deleting and retrying...", output + end + end + end + + test "push without builder" do + with_build_directory do |build_directory| + stub_setup + + SSHKit::Backend::Abstract.any_instance.expects(:execute) .with(:docker, "--version", "&&", :docker, :buildx, "version") - SSHKit::Backend::Abstract.any_instance.stubs(:execute) + SSHKit::Backend::Abstract.any_instance.expects(:execute) .with(:docker, :buildx, :create, "--use", "--name", "kamal-app-multiarch") - SSHKit::Backend::Abstract.any_instance.stubs(:execute) + SSHKit::Backend::Abstract.any_instance.expects(:execute) .with { |*args| args[0..1] == [ :docker, :buildx ] } .raises(SSHKit::Command::Failed.new("no builder")) .then .returns(true) - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with { |*args| args.first.start_with?("git") } + SSHKit::Backend::Abstract.any_instance.expects(:execute).with { |*args| args.first.start_with?("git") } - SSHKit::Backend::Abstract.any_instance.stubs(:execute).with(:mkdir, "-p", "#{Dir.tmpdir}/kamal-clones/app-#{pwd_sha}") + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :"rev-parse", :HEAD) + .returns(Kamal::Git.revision) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :status, "--porcelain") + .returns("") run_command("push").tap do |output| assert_match /WARN Missing compatible builder, so creating a new one first/, output @@ -183,7 +236,7 @@ class CliBuildTest < CliTestCase build_directory = File.join Dir.tmpdir, "kamal-clones", "app-#{pwd_sha}", "kamal" FileUtils.mkdir_p build_directory FileUtils.touch File.join build_directory, "Dockerfile" - yield + yield build_directory + "/" ensure FileUtils.rm_rf build_directory end diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index f7e479d7..595556fe 100644 --- a/test/cli/main_test.rb +++ b/test/cli/main_test.rb @@ -108,6 +108,14 @@ class CliMainTest < CliTestCase SSHKit::Backend::Abstract.any_instance.expects(:capture_with_debug) .with(:stat, ".kamal/locks/app", ">", "/dev/null", "&&", :cat, ".kamal/locks/app/details", "|", :base64, "-d") + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :"rev-parse", :HEAD) + .returns(Kamal::Git.revision) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :status, "--porcelain") + .returns("") + assert_raises(Kamal::Cli::LockError) do run_command("deploy") end @@ -129,6 +137,14 @@ class CliMainTest < CliTestCase .with { |*arg| arg[0..1] == [ :mkdir, ".kamal/locks/app" ] } .raises(SocketError, "getaddrinfo: nodename nor servname provided, or not known") + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :"rev-parse", :HEAD) + .returns(Kamal::Git.revision) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :status, "--porcelain") + .returns("") + assert_raises(SSHKit::Runner::ExecuteError) do run_command("deploy") end