Skip to content
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

Raise error (instead of warn) when uncommitted git changes are detected #1409

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Raise error (instead of warn) when uncommitted git changes are detected #1409

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
9 changes: 8 additions & 1 deletion lib/kamal/cli/build.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ 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
end

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

Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/kamal/cli/main.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions test/cli/build_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
53 changes: 48 additions & 5 deletions test/cli/main_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand All @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions test/test_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down