Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions lib/kamal/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ def roles
servers.roles
end

def any_role_use_proxy_idle?
roles.any? { |role| role.proxy&.idle? }
end

def role(name)
roles.detect { |r| r.name == name.to_s }
end
Expand Down
12 changes: 12 additions & 0 deletions lib/kamal/configuration/docs/proxy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,17 @@ proxy:
- X-Request-ID
- X-Request-Start

# Idle
#
# Stop containers after a period of inactivity and wake them up on the next request.
#
# This requires the proxy to have access to the Docker socket.
#
# Defaults to disabled:
idle:
timeout: 300 # Stop containers after 5 minutes of no requests
wake_timeout: 30 # Maximum time to wait for containers to wake up

# Run configuration
#
# These options are used when booting the proxy container.
Expand All @@ -158,6 +169,7 @@ proxy:
metrics_port: 9090 # Port for Prometheus metrics
debug: true # Debug logging (default: false)
log_max_size: "30m" # Maximum log file size (default: "10m")
docker_socket: true # Mount the Docker socket (default: false, or true if idle is used)
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment for run.docker_socket says the default is "false, or true if idle is used", but the current implementation only defaults to true when the proxy is started via a proxy.run config (i.e., when Proxy::Run is used). If the proxy is started via the boot-file path (no proxy.run), proxy.idle currently won’t imply a Docker socket mount. Please either adjust the docs to reflect this limitation or update the boot-mode run options to match the documented default.

Suggested change
docker_socket: true # Mount the Docker socket (default: false, or true if idle is used)
docker_socket: true # Mount the Docker socket (default: false; may default to true with idle when using proxy.run)

Copilot uses AI. Check for mistakes.
publish: false # Publish ports to the host (default: true)
bind_ips: # List of IPs to bind to when publishing ports
- 0.0.0.0
Expand Down
6 changes: 6 additions & 0 deletions lib/kamal/configuration/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ def app_port
proxy_config.fetch("app_port", 80)
end

def idle?
proxy_config.dig("idle", "timeout").present?
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idle? only checks idle.timeout. If a user configures only idle.wake_timeout, idle? will be false (which also affects any_role_use_proxy_idle?) even though deploy_options may still emit an idle-related flag. Consider either treating wake_timeout as enabling idle as well, or validating that wake_timeout cannot be set without timeout.

Suggested change
proxy_config.dig("idle", "timeout").present?
idle_config = proxy_config["idle"]
idle_config.is_a?(Hash) && (idle_config["timeout"].present? || idle_config["wake_timeout"].present?)

Copilot uses AI. Check for mistakes.
end

def ssl?
proxy_config.fetch("ssl", false)
end
Expand Down Expand Up @@ -90,6 +94,8 @@ def deploy_options
"tls-redirect": proxy_config.dig("ssl_redirect"),
"log-request-header": proxy_config.dig("logging", "request_headers") || DEFAULT_LOG_REQUEST_HEADERS,
"log-response-header": proxy_config.dig("logging", "response_headers"),
"idle-timeout": seconds_duration(proxy_config.dig("idle", "timeout")),
"idle-wake-timeout": seconds_duration(proxy_config.dig("idle", "wake_timeout")),
"error-pages": error_pages
Comment on lines 96 to 99
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deploy_options will include idle-wake-timeout whenever idle.wake_timeout is set, even if idle.timeout is missing/disabled. That can produce a deploy command with a wake-timeout flag but no idle-timeout, and (depending on how the proxy container is started) without Docker socket access. Consider only emitting idle-wake-timeout when idle.timeout is configured, or enforce the dependency in validation.

Copilot uses AI. Check for mistakes.
}.compact
end
Expand Down
5 changes: 5 additions & 0 deletions lib/kamal/configuration/proxy/run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ def debug?
run_config.fetch("debug", nil)
end

def docker_socket?
run_config.fetch("docker_socket", config.any_role_use_proxy_idle?)
end
Comment on lines +20 to +22
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Docker socket auto-mount is implemented via Proxy::Run#docker_socket?, but Proxy::Run is only instantiated/used when a per-host proxy.run config exists. When proxy.run is not configured (the default path in Kamal::Commands::Proxy#run uses the boot-file pipeline), idle can be enabled via proxy.idle but the proxy container still won't get the Docker socket mount. If the intent is that setting proxy.idle alone enables scale-to-zero, this needs to also affect the boot-mode run options (or the docs should explicitly require proxy.run).

Copilot uses AI. Check for mistakes.

def publish?
run_config.fetch("publish", true)
end
Expand Down Expand Up @@ -94,6 +98,7 @@ def run_command_options
def docker_options_args
[
*apps_volume_args,
*("--volume=/var/run/docker.sock:/var/run/docker.sock" if docker_socket?),
*publish_args,
*logging_args,
*("--expose=#{metrics_port}" if metrics_port.present?),
Expand Down
10 changes: 10 additions & 0 deletions lib/kamal/configuration/validator/proxy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ def validate!
end
end

if config["idle"].present?
if config["idle"]["timeout"].present? && !config["idle"]["timeout"].is_a?(Integer)
error "Idle timeout must be an integer (seconds)"
end

if config["idle"]["wake_timeout"].present? && !config["idle"]["wake_timeout"].is_a?(Integer)
error "Idle wake timeout must be an integer (seconds)"
Comment on lines +25 to +30
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new idle validation only checks that the values are Integers, but it does not enforce the documented/PR-stated requirement that idle.timeout and idle.wake_timeout be positive (e.g., 0 or negative values would pass and produce "0s"/"-5s" durations later). Consider validating > 0 (and possibly adding a targeted error message) for any configured idle timeout values.

Suggested change
if config["idle"]["timeout"].present? && !config["idle"]["timeout"].is_a?(Integer)
error "Idle timeout must be an integer (seconds)"
end
if config["idle"]["wake_timeout"].present? && !config["idle"]["wake_timeout"].is_a?(Integer)
error "Idle wake timeout must be an integer (seconds)"
if config["idle"]["timeout"].present?
if !config["idle"]["timeout"].is_a?(Integer)
error "Idle timeout must be an integer (seconds)"
elsif config["idle"]["timeout"] <= 0
error "Idle timeout must be a positive integer (seconds)"
end
end
if config["idle"]["wake_timeout"].present?
if !config["idle"]["wake_timeout"].is_a?(Integer)
error "Idle wake timeout must be an integer (seconds)"
elsif config["idle"]["wake_timeout"] <= 0
error "Idle wake timeout must be a positive integer (seconds)"
end

Copilot uses AI. Check for mistakes.
end
end

if run_config = config["run"]
if run_config["bind_ips"].present?
ensure_valid_bind_ips(config["bind_ips"])
Expand Down
8 changes: 8 additions & 0 deletions test/commands/app_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,14 @@ class CommandsAppTest < ActiveSupport::TestCase
new_command.deploy(target: "172.1.0.2").join(" ")
end

test "deploy with idle" do
@config[:proxy] = { "idle" => { "timeout" => 300, "wake_timeout" => 30 } }

assert_equal \
"docker exec kamal-proxy kamal-proxy deploy app-web --target=\"172.1.0.2:80\" --deploy-timeout=\"30s\" --drain-timeout=\"30s\" --buffer-requests --buffer-responses --log-request-header=\"Cache-Control\" --log-request-header=\"Last-Modified\" --log-request-header=\"User-Agent\" --idle-timeout=\"300s\" --idle-wake-timeout=\"30s\"",
new_command.deploy(target: "172.1.0.2").join(" ")
end

test "remove" do
assert_equal \
"docker exec kamal-proxy kamal-proxy remove app-web",
Expand Down
14 changes: 14 additions & 0 deletions test/commands/proxy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,20 @@ class CommandsProxyTest < ActiveSupport::TestCase
new_command.run.join(" ")
end

test "docker socket mount when idle configured" do
@config[:proxy] = { "idle" => { "timeout" => 300 }, "run" => { "version" => "v0.9.2" } }
assert_equal \
"docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy --volume $PWD/.kamal/proxy/apps-config:/home/kamal-proxy/.apps-config --volume=/var/run/docker.sock:/var/run/docker.sock --publish 80:80 --publish 443:443 --log-opt max-size=10m basecamp/kamal-proxy:v0.9.2 kamal-proxy run",
new_command.run.join(" ")
end

Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test covers the Docker socket mount when proxy.idle is set and proxy.run is configured (so the code path uses proxy_run_config). Given the intended behavior in the PR description/docs, it would also be useful to add a regression test for proxy.idle without any proxy.run config to ensure the boot-mode run command still mounts /var/run/docker.sock (or to lock in the requirement that proxy.run must be configured).

Suggested change
test "docker socket mount when idle configured without run config" do
@config[:proxy] = { "idle" => { "timeout" => 300 } }
assert_equal \
"docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy --volume $PWD/.kamal/proxy/apps-config:/home/kamal-proxy/.apps-config --volume=/var/run/docker.sock:/var/run/docker.sock --publish 80:80 --publish 443:443 --log-opt max-size=10m basecamp/kamal-proxy:v0.9.2 kamal-proxy run",
new_command.run.join(" ")
end

Copilot uses AI. Check for mistakes.
test "explicit docker socket mount config" do
@config[:proxy] = { "run" => { "docker_socket" => true } }
assert_equal \
"docker run --name kamal-proxy --network kamal --detach --restart unless-stopped --volume kamal-proxy-config:/home/kamal-proxy/.config/kamal-proxy --volume $PWD/.kamal/proxy/apps-config:/home/kamal-proxy/.apps-config --volume=/var/run/docker.sock:/var/run/docker.sock --publish 80:80 --publish 443:443 --log-opt max-size=10m basecamp/kamal-proxy:v0.9.2 kamal-proxy run",
new_command.run.join(" ")
end

private
def new_command
Kamal::Commands::Proxy.new(Kamal::Configuration.new(@config, version: "123"), host: "1.1.1.1")
Expand Down
26 changes: 26 additions & 0 deletions test/configuration/proxy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,32 @@ class ConfigurationProxyTest < ActiveSupport::TestCase
end
end

test "idle configuration" do
@deploy[:proxy] = {
"host" => "example.com",
"idle" => {
"timeout" => 300,
"wake_timeout" => 30
}
}

proxy = config.proxy
assert_equal true, proxy.idle?
assert_equal "300s", proxy.deploy_options[:"idle-timeout"]
assert_equal "30s", proxy.deploy_options[:"idle-wake-timeout"]
end

test "idle validation" do
@deploy[:proxy] = {
"host" => "example.com",
"idle" => {
"timeout" => "300"
}
}

assert_raises(Kamal::ConfigurationError) { config.proxy }
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added validation test covers type-checking (string vs integer), but it doesn't cover the documented requirement that idle timeouts be positive. If the validator is updated to reject 0/negative values, consider adding assertions here for those cases to prevent regressions.

Suggested change
assert_raises(Kamal::ConfigurationError) { config.proxy }
assert_raises(Kamal::ConfigurationError) { config.proxy }
@deploy[:proxy] = {
"host" => "example.com",
"idle" => {
"timeout" => 0
}
}
assert_raises(Kamal::ConfigurationError) { config.proxy }
@deploy[:proxy] = {
"host" => "example.com",
"idle" => {
"timeout" => -1
}
}
assert_raises(Kamal::ConfigurationError) { config.proxy }

Copilot uses AI. Check for mistakes.
end

private
def config
Kamal::Configuration.new(@deploy)
Expand Down
Loading