Skip to content

Commit

Permalink
Raise error when uncommitted changes are detected
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
matthewbjones committed Feb 11, 2025
1 parent 1547089 commit 02916ae
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 6 deletions.
9 changes: 8 additions & 1 deletion lib/kamal/cli/build.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ 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
push
pull
end

desc "push", "Build and push app image to registry"
option :skip_uncommitted_changes_check, type: :boolean, default: false, desc: "Skip uncommitted git changes check"
def push
cli = self

Expand All @@ -20,7 +22,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
Expand Down
1 change: 1 addition & 0 deletions lib/kamal/cli/main.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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
runtime = print_runtime do
invoke_options = deploy_options
Expand Down
8 changes: 8 additions & 0 deletions test/cli/build_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,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, **hook_variables
assert_match /Cloning repo into build directory/, output
Expand Down Expand Up @@ -59,6 +61,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
Expand Down Expand Up @@ -104,6 +108,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
Expand Down Expand Up @@ -142,6 +148,8 @@ class CliBuildTest < CliTestCase
SSHKit::Backend::Abstract.any_instance.expects(:execute)
.with(:docker, :buildx, :build, "--push", "--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
Expand Down
53 changes: 48 additions & 5 deletions test/cli/main_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,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)
Expand All @@ -70,7 +70,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)
Expand All @@ -90,6 +90,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

Expand Down Expand Up @@ -119,6 +158,8 @@ class CliMainTest < CliTestCase
.returns("")
.at_least_once

stub_no_uncommitted_git_changes

assert_raises(Kamal::Cli::LockError) do
run_command("deploy")
end
Expand Down Expand Up @@ -150,13 +191,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))
Expand All @@ -170,7 +213,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)
Expand All @@ -185,7 +228,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)
Expand Down
4 changes: 4 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ def stub_ticks_with(command, succeed: true)
Kamal::Secrets::Adapters::Base.any_instance.stubs(:`)
end

def stub_no_uncommitted_git_changes
Kamal::Git.stubs(:uncommitted_changes).returns("")
end

def shellunescape(string)
"\"#{string}\"".undump.gsub(/\\([{}])/, "\\1")
end
Expand Down

0 comments on commit 02916ae

Please sign in to comment.