From 9fadae328c49ced071ea31c196c20d09bc26d44c Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Mon, 10 Feb 2025 18:30:42 -0700 Subject: [PATCH] Raise error when uncommitted changes are detected Changes the default behavior from printing a warning to printing an error message and exiting. New CLI introduced to suppress the error and proceed with previously-default behavior. --- lib/kamal/cli/build.rb | 9 ++++++- lib/kamal/cli/main.rb | 1 + test/cli/build_test.rb | 10 ++++++++ test/cli/main_test.rb | 53 ++++++++++++++++++++++++++++++++++++++---- test/test_helper.rb | 4 ++++ 5 files changed, 71 insertions(+), 6 deletions(-) diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 0187e6863..38c65ef93 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -4,6 +4,7 @@ class Kamal::Cli::Build < Kamal::Cli::Base class BuildError < StandardError; end desc "deliver", "Build app and push app image to registry then pull image on servers" + option :skip_uncommitted_changes_check, type: :boolean, default: false, desc: "Skip uncommitted git changes check" def deliver invoke :push invoke :pull @@ -11,6 +12,7 @@ def deliver desc "push", "Build and push app image to registry" option :output, type: :string, default: "registry", banner: "export_type", desc: "Exported type for the build result, and may be any exported type supported by 'buildx --output'." + option :skip_uncommitted_changes_check, type: :boolean, default: false, desc: "Skip uncommitted git changes check" def push cli = self @@ -21,7 +23,12 @@ def push if KAMAL.config.builder.git_clone? if uncommitted_changes.present? - say "Building from a local git clone, so ignoring these uncommitted changes:\n #{uncommitted_changes}", :yellow + if options[:skip_uncommitted_changes_check] + say "Building from a local git clone, so ignoring these uncommitted changes:\n #{uncommitted_changes}", :yellow + else + say "Uncommitted changes detected - commit your changes first. To ignore uncommitted changes and deploy from latest git commit, use --skip-uncommitted-changes-check. Uncommitted changes:\n#{uncommitted_changes}", :red + raise BuildError, "Uncommitted changes detected" + end end run_locally do diff --git a/lib/kamal/cli/main.rb b/lib/kamal/cli/main.rb index 2fae36e81..d696061d6 100644 --- a/lib/kamal/cli/main.rb +++ b/lib/kamal/cli/main.rb @@ -16,6 +16,7 @@ def setup desc "deploy", "Deploy app to servers" option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push" + option :skip_uncommitted_changes_check, type: :boolean, default: false, desc: "Skip uncommitted git changes check" def deploy(boot_accessories: false) runtime = print_runtime do invoke_options = deploy_options diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index 76db29244..f82b65067 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -20,6 +20,8 @@ class CliBuildTest < CliTestCase .with(:git, "-C", anything, :status, "--porcelain") .returns("") + stub_no_uncommitted_git_changes + run_command("push", "--verbose").tap do |output| assert_hook_ran "pre-build", output assert_match /Cloning repo into build directory/, output @@ -42,6 +44,8 @@ class CliBuildTest < CliTestCase .with(:git, "-C", anything, :status, "--porcelain") .returns("") + stub_no_uncommitted_git_changes + run_command("push", "--output=docker", "--verbose").tap do |output| assert_hook_ran "pre-build", output assert_match /Cloning repo into build directory/, output @@ -80,6 +84,8 @@ class CliBuildTest < CliTestCase .with(:git, "-C", anything, :status, "--porcelain") .returns("") + stub_no_uncommitted_git_changes + run_command("push", "--verbose").tap do |output| assert_match /Cloning repo into build directory/, output assert_match /Resetting local clone/, output @@ -124,6 +130,8 @@ class CliBuildTest < CliTestCase Dir.stubs(:chdir) + stub_no_uncommitted_git_changes + 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 @@ -162,6 +170,8 @@ class CliBuildTest < CliTestCase SSHKit::Backend::Abstract.any_instance.expects(:execute) .with(:docker, :buildx, :build, "--output=type=registry", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".") + stub_no_uncommitted_git_changes + run_command("push").tap do |output| assert_match /WARN Missing compatible builder, so creating a new one first/, output end diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index becace43f..d9f2b71bb 100644 --- a/test/cli/main_test.rb +++ b/test/cli/main_test.rb @@ -43,7 +43,7 @@ class CliMainTest < CliTestCase test "deploy" do with_test_secrets("secrets" => "DB_PASSWORD=secret") do - invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, "verbose" => true } + invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, "skip_uncommitted_changes_check" => false, "verbose" => true } Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false)) Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options) @@ -68,7 +68,7 @@ class CliMainTest < CliTestCase end test "deploy with skip_push" do - invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false } + invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, "skip_uncommitted_changes_check" => false } Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: true)) Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:pull", [], invoke_options) @@ -88,6 +88,45 @@ class CliMainTest < CliTestCase end end + test "deploy with uncommitted git changes" do + Kamal::Git.stubs(:uncommitted_changes).returns("M file\n") + + with_argv([ "deploy", "-c", "test/fixtures/deploy_simple.yml" ]) do + output = capture(:stdout) do + begin + Kamal::Cli::Main.start + rescue Kamal::Cli::Build::BuildError => e + @raised_error = e + end + end + assert_match /Uncommitted changes detected - commit your changes first. To ignore uncommitted changes and deploy from latest git commit, use --skip-uncommitted-changes-check. Uncommitted changes:\nM file/, output + end + assert_equal Kamal::Cli::Build::BuildError, @raised_error.class + assert_equal "Uncommitted changes detected", @raised_error.message + end + + test "deploy with uncommitted git changes and skip_uncommitted_changes_check" do + invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, "skip_uncommitted_changes_check" => true } + + Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false)) + Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options) + Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:proxy:boot", [], invoke_options) + Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true)) + Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:boot", [], invoke_options) + Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:prune:all", [], invoke_options) + + Kamal::Git.stubs(:uncommitted_changes).returns("M file\n") + + run_command("deploy", "--skip-uncommitted-changes-check").tap do |output| + assert_match /Acquiring the deploy lock/, output + assert_match /Log into image registry/, output + assert_match /Ensure kamal-proxy is running/, output + assert_match /Detect stale containers/, output + assert_match /Prune old containers and images/, output + assert_match /Releasing the deploy lock/, output + end + end + test "deploy when locked" do Thread.report_on_exception = false @@ -117,6 +156,8 @@ class CliMainTest < CliTestCase .returns("") .at_least_once + stub_no_uncommitted_git_changes + assert_raises(Kamal::Cli::LockError) do run_command("deploy") end @@ -148,13 +189,15 @@ class CliMainTest < CliTestCase .returns("") .at_least_once + stub_no_uncommitted_git_changes + assert_raises(SSHKit::Runner::ExecuteError) do run_command("deploy") end end test "deploy errors during outside section leave remove lock" do - invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, :skip_local => false } + invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, "skip_uncommitted_changes_check" => false, :skip_local => false } Kamal::Cli::Main.any_instance.expects(:invoke) .with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false)) @@ -168,7 +211,7 @@ class CliMainTest < CliTestCase end test "deploy with skipped hooks" do - invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => true } + invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => true, "skip_uncommitted_changes_check" => false } Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false)) Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options) @@ -183,7 +226,7 @@ class CliMainTest < CliTestCase end test "deploy with missing secrets" do - invoke_options = { "config_file" => "test/fixtures/deploy_with_secrets.yml", "version" => "999", "skip_hooks" => false } + invoke_options = { "config_file" => "test/fixtures/deploy_with_secrets.yml", "version" => "999", "skip_hooks" => false, "skip_uncommitted_changes_check" => false } Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false)) Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options) diff --git a/test/test_helper.rb b/test/test_helper.rb index e77a0a8a7..80636ccfa 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -75,6 +75,10 @@ def teardown_test_secrets Dir.chdir(@original_pwd) FileUtils.rm_rf(@secrets_tmpdir) end + + def stub_no_uncommitted_git_changes + Kamal::Git.stubs(:uncommitted_changes).returns("") + end end class SecretAdapterTestCase < ActiveSupport::TestCase