Repository navigation
fix(loadbalancer): propagate full proxy deploy options to the LB - #14
Merged
Merged
Conversation
## Summary The loadbalancer deploy re-implemented flag construction from scratch and silently dropped every rich per-app proxy option (deploy/drain/health-check timeouts, response timeout, buffering, path prefixes, error pages, logged headers), hardcoded the target port to `:80` ignoring `app_port`, and hardcoded `--publish 80:80 --publish 443:443` in `run` ignoring `proxy.run.publish`/ports/options. Now `Commands::Loadbalancer#deploy` reuses the LB config's inherited `deploy_command_args`, so the LB deploys with the same flags as the per-app proxy (it is the same kamal-proxy image). `#run` honours `proxy.run` when present, falling back to the default 80/443 publish otherwise. Also fixes a latent bug in `Configuration::Loadbalancer#deploy_options`: it re-added `tls: proxy_config["ssl"].presence` after the parent compacted, so non-SSL load balancers emitted a dangling `--tls=`. Now `tls: true if ssl?`, matching the parent semantics. ## Test Coverage - deploy propagates rich options (timeouts, healthcheck, path prefix) - deploy uses app_port for target ports (not hardcoded 80) - deploy with multiple hosts emits repeated --host flags - run honors proxy.run.publish false + custom options - run honors custom http/https ports - updated cli/proxy loadbalancer deploy assertion (fixture deploy_timeout: 6 now flows through) ## Verification - [x] bundle exec rubocop --parallel passes (4 files, no offenses) - [x] unit tests pass (only the 2 known Apple-Silicon builder failures remain, confirmed pre-existing on the base branch) Closes #1
mhenrixon
added
gem
…deploy-options # Conflicts: # test/commands/loadbalancer_test.rb
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the loadbalancer deploy dropping every rich per-app proxy option (R1,
size:S,bugfix). Discovered via the integration failures on 2026-07-03.The load balancer runs the same
kamal-proxyimage as the per-app proxy, butKamal::Commands::Loadbalancer#deployhand-built only--target/--host/--tls, so timeouts, health checks, buffering, path prefixes, error pages, and logged headers configured indeploy.ymlnever reached it. It also hardcoded target port:80(ignoringapp_port) and--publish 80:80 --publish 443:443inrun(ignoringproxy.run.publish/ports/options).Changes
Commands::Loadbalancer#deployreuses the config's inheriteddeploy_command_args, propagating the full flag set (lib/kamal/commands/loadbalancer.rb).Configuration::Loadbalancer#deploy_command_args(targets:)— the multi-target analogue of the parent's single-target method, honouringapp_port.#runhonoursproxy.run(publish/ports/options) when present, default 80/443 otherwise.--tls=dangling-flag bug inConfiguration::Loadbalancer#deploy_options(tls: nilsurvived the parent's compact).Test plan
bundle exec rubocop --parallel— cleandeploy_timeout/healthcheck/proxy.runand they take effect (bin/test, needs Docker + published proxy image)Closes #1