From 77311e6c0f093dec94618abfe3e21542f1f1e17c Mon Sep 17 00:00:00 2001
From: Neil Wilson <neil@aldur.co.uk>
Date: Thu, 10 Oct 2024 16:24:19 +0100
Subject: [PATCH] Add user to docker group if not superuser

This allows docker commands to function with a non-root
ssh user

Fixes: #980
---
 lib/kamal/cli/server.rb      |  8 ++++++++
 lib/kamal/commands/docker.rb |  6 ++++++
 test/cli/server_test.rb      | 21 +++++++++++++++++++++
 test/commands/docker_test.rb |  4 ++++
 4 files changed, 39 insertions(+)

diff --git a/lib/kamal/cli/server.rb b/lib/kamal/cli/server.rb
index 452ab089e..3adabb6a0 100644
--- a/lib/kamal/cli/server.rb
+++ b/lib/kamal/cli/server.rb
@@ -32,6 +32,14 @@ def bootstrap
           if execute(*KAMAL.docker.superuser?, raise_on_non_zero_exit: false)
             info "Missing Docker on #{host}. Installing…"
             execute *KAMAL.docker.install
+            begin
+              execute *KAMAL.docker.add_group
+
+              # If the groups change, the session is terminated to force a re-login.
+              # Catch the resulting IOError and inform the user
+            rescue IOError
+              info "Session refreshed due to group change."
+            end
           else
             missing << host
           end
diff --git a/lib/kamal/commands/docker.rb b/lib/kamal/commands/docker.rb
index bc9345dfc..53dcdc21d 100644
--- a/lib/kamal/commands/docker.rb
+++ b/lib/kamal/commands/docker.rb
@@ -19,6 +19,12 @@ def superuser?
     [ '[ "${EUID:-$(id -u)}" -eq 0 ] || command -v sudo >/dev/null || command -v su >/dev/null' ]
   end
 
+  # If we're not root and not already in the docker group
+  # add us to the docker group and terminate all our current sessions
+  def add_group
+    [ '[ "${EUID:-$(id -u)}" -eq 0 ] || id -nG "${USER:-$(id -un)}" | grep -qw docker || { sudo usermod -aG docker "${USER:-$(id -un)}" && kill -HUP ${PPID:-ps -o ppid= -p $$}; }' ]
+  end
+
   def create_network
     docker :network, :create, :kamal
   end
diff --git a/test/cli/server_test.rb b/test/cli/server_test.rb
index 110e217dd..2cbe29b57 100644
--- a/test/cli/server_test.rb
+++ b/test/cli/server_test.rb
@@ -52,6 +52,7 @@ class CliServerTest < CliTestCase
     stub_setup
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "-v", raise_on_non_zero_exit: false).returns(false).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with('[ "${EUID:-$(id -u)}" -eq 0 ] || command -v sudo >/dev/null || command -v su >/dev/null', raise_on_non_zero_exit: false).returns(true).at_least_once
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with('[ "${EUID:-$(id -u)}" -eq 0 ] || id -nG "${USER:-$(id -un)}" | grep -qw docker || { sudo usermod -aG docker "${USER:-$(id -un)}" && kill -HUP ${PPID:-ps -o ppid= -p $$}; }').at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:sh, "-c", "'curl -fsSL https://get.docker.com || wget -O - https://get.docker.com || echo \"exit 1\"'", "|", :sh).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:mkdir, "-p", ".kamal").returns("").at_least_once
     Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true)
@@ -66,6 +67,26 @@ class CliServerTest < CliTestCase
     end
   end
 
+  test "bootstrap install as sudo non-root user" do
+    stub_setup
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "-v", raise_on_non_zero_exit: false).returns(false).at_least_once
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with('[ "${EUID:-$(id -u)}" -eq 0 ] || command -v sudo >/dev/null || command -v su >/dev/null', raise_on_non_zero_exit: false).returns(true).at_least_once
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with('[ "${EUID:-$(id -u)}" -eq 0 ] || id -nG "${USER:-$(id -un)}" | grep -qw docker || { sudo usermod -aG docker "${USER:-$(id -un)}" && kill -HUP ${PPID:-ps -o ppid= -p $$}; }').at_least_once.raises(IOError, "closed stream")
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:sh, "-c", "'curl -fsSL https://get.docker.com || wget -O - https://get.docker.com || echo \"exit 1\"'", "|", :sh).at_least_once
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:mkdir, "-p", ".kamal").returns("").at_least_once
+    Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true)
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with(".kamal/hooks/pre-connect", anything).at_least_once
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with(".kamal/hooks/docker-setup", anything).at_least_once
+
+    run_command("bootstrap").tap do |output|
+      ("1.1.1.1".."1.1.1.4").map do |host|
+        assert_match "Missing Docker on #{host}. Installing…", output
+        assert_match "Session refreshed due to group change.", output
+        assert_match "Running the docker-setup hook", output
+      end
+    end
+  end
+
   private
     def run_command(*command)
       stdouted { Kamal::Cli::Server.start([ *command, "-c", "test/fixtures/deploy_with_accessories.yml" ]) }
diff --git a/test/commands/docker_test.rb b/test/commands/docker_test.rb
index db2ed45c5..c2f661868 100644
--- a/test/commands/docker_test.rb
+++ b/test/commands/docker_test.rb
@@ -23,4 +23,8 @@ class CommandsDockerTest < ActiveSupport::TestCase
   test "superuser?" do
     assert_equal '[ "${EUID:-$(id -u)}" -eq 0 ] || command -v sudo >/dev/null || command -v su >/dev/null', @docker.superuser?.join(" ")
   end
+
+  test "add_group" do
+    assert_equal '[ "${EUID:-$(id -u)}" -eq 0 ] || id -nG "${USER:-$(id -un)}" | grep -qw docker || { sudo usermod -aG docker "${USER:-$(id -un)}" && kill -HUP ${PPID:-ps -o ppid= -p $$}; }', @docker.add_group.join(" ")
+  end
 end