Skip to content

test(proxy): fail the build when kamal-proxy grows a flag the gem cannot emit - #83

Merged
mhenrixon merged 3 commits into
dashfrom
issue-82-proxy-flag-coverage
Jul 29, 2026
Merged

mhenrixon merged 3 commits into
dashfrom
issue-82-proxy-flag-coverage

Conversation

@mhenrixon

Copy link
Copy Markdown
Collaborator

Summary

kamal-proxy v1.0.0.0 accepts 80 deploy flags and 31 run flags. The gem emits 34 and 3.

Most of R3 and R5 — rate limiting, IP allow lists, mTLS, the response cache, compression, header rules, redirects, canary splits, scale-to-zero — plus every ACME/DNS-01 option, ships in the image and cannot be turned on from deploy.yml.

Nothing noticed it happening. 23 feat commits landed between v0.9.2.2 and v1.0.0.0 and the gem's config surface never moved, because no test relates the two. Fixing the backlog without fixing the detection just resets the clock.

This adds the detection.

Closes #82

How it works

Piece Role
bin/sync-proxy-flags regenerates the flag manifest from the proxy's own --help
test/fixtures/kamal_proxy_flags.yml the checked-in manifest, stamped with the version it came from
test/fixtures/deploy_with_every_proxy_option.yml every proxy key the gem can express
test/proxy_flag_coverage_test.rb the guard

Every flag must be emitted by the gem or waived with a reason. Waivers split in two, deliberately:

  • NEVER_EXPOSED — a decision (--force is a CLI concern; --http-port is published by docker)
  • R7_BACKLOG — a todo, keyed by its issue number

Collapsing those into one hash would let the backlog quietly become permanent, which is how this gap happened.

Two choices worth reviewing

Coverage is measured by generating the real commands, not by reading the keys of deploy_options. Costlier, but a config accessor that exists and is never wired does not pass. This is what makes each R7 issue self-verifying: implement it, delete its waiver, and the test proves the flag is actually emitted rather than merely named.

The flag list is generated from Cobra, never parsed out of the Go source. Not caution for its own sake — the flag counts I put in the R7 epic came from a regex over internal/cmd/*.go that silently missed every flag registered via Int64Var/Uint16Var, because the method name contains digits. Cobra found three the regex did not (cache-max-variants, cache-lease-ttl, cache-lease-wait). Building a drift guard on the technique that caused the drift would have been a poor foundation.

Corrected figures, epic #13's summary table is understated: 80/31 flags (not 79/29), 45 deploy + 25 run unexposed (not 44/26). No flag moves between R7 issues; I'll fix the table.

Test plan

  • RED with an empty waiver list — reports all 45 + 25 unmapped flags with per-flag guidance
  • Upstream adds a flag — injected brand-new-thing into the manifest; fails naming it, with the expose-or-waive instructions
  • Upstream removes a waived flag — deleted canonical-host; fails naming the now-dead waiver
  • MINIMUM_VERSION bumped without refreshing — set it to v1.0.1.1; fails telling you to run bin/sync-proxy-flags
  • Fixture is load-bearing — deleted ssl_redirect from the fixture; --tls-redirect drops into the uncovered list
  • bundle exec rubocop --parallel clean (205 files)
  • Unit suite: 1188 runs, 2 failures — both the documented Apple-Silicon builder tests

bin/test not run: integration needs ghcr.io/mhenrixon/kamal-proxy:v1.0.0.0, which was still building when this was pushed. Must be green before merge — see below.

Deviations & judgment calls

Deviations

  • Bundled the MINIMUM_VERSION bump v0.9.2.2 → v1.0.0.0. Not asked for, but the manifest must be generated against some version and the backlog is only accurate against this one — cache-max-variants, cache-lease-ttl and cache-lease-wait exist only in v1.0.0.0. Generating against v0.9.2.2 would have encoded a stale backlog on day one. This is the merge risk: integration tests now pull the v1.0.0.0 image. If you'd rather decouple, say so and I'll regenerate against v0.9.2.2 and land the bump separately.
  • Also bumped the pinned version in docs/proxy.yml:341 — not optional, test_docs_example_run_version_matches_the_pinned_minimum_version enforces it.

Judgment calls

  • Source of truth is a hybrid of the issue's options 1 and 2. The issue preferred a kamal-proxy flags --json subcommand, but that needs a proxy-repo change and another release before the gem can use it. bin/sync-proxy-flags runs the proxy's own --help instead — same authoritativeness, no cross-repo dependency. If flags --json lands later, only the script changes; the manifest format and the test do not.
  • Dropped three waivers the issue's sketch assumed. host, tls and target are emitted by deploy_command_args. Waiving them would have been a lie that masked real coverage.
  • Scoped to deploy and run. The other subcommands (list, stop, pause, drain, rollout, cache, domains) carry no operator-tunable options, so they are not a deploy.yml surface. Documented in the script.
  • --recheck-targets-on-restore needed no waiver — it is already emitted unconditionally.

…not emit

kamal-proxy accepts 80 deploy flags and 31 run flags. The gem emits 34 and 3.
Most of R3 and R5 — rate limiting, mTLS, the response cache, compression,
header rules, redirects, scale-to-zero — plus every ACME/DNS-01 option ships
in the image and cannot be turned on from deploy.yml.

Nothing noticed it happening. 23 feat commits landed between v0.9.2.2 and
v1.0.0.0 and the gem's config surface never moved, because no test relates the
two. Fixing the backlog without fixing the detection just resets the clock.

So: every flag kamal-proxy accepts must be emitted by the gem or waived with a
reason. Waivers split into NEVER_EXPOSED (a decision) and R7_BACKLOG (a todo
carrying its issue number) so the second cannot quietly become the first.

Coverage is measured by generating the real commands from a maximal fixture,
not by reading the keys of deploy_options — a config accessor that exists but
is never wired does not pass. Deleting ssl_redirect from the fixture drops
--tls-redirect into the uncovered list, which is the property that makes each
R7 issue self-verifying: delete the waiver, and the test proves the wiring.

The flag list is generated by bin/sync-proxy-flags from Cobra's own --help,
never by parsing the Go source. That is not caution for its own sake: the
counts in the R7 epic came from a regex that silently missed every flag
registered via Int64Var, because the method name contains digits. Cobra found
three the regex did not.

Bumps MINIMUM_VERSION to v1.0.0.0 — the manifest has to be generated against a
version, and the backlog is only accurate against this one. The docs example
version moves with it, as its own guard test requires.

Refs #82
@mhenrixon mhenrixon self-assigned this Jul 29, 2026
@mhenrixon mhenrixon added enhancement New feature or request proxy dash-proxy integration — proxy config, flags, layering and the load balancer labels Jul 29, 2026
setup.sh seeds the private registry with the proxy image so app_with_roles
keeps exercising proxy.run.registry. The tag was hardcoded to v0.9.2.2 next to
a comment saying it must match MINIMUM_VERSION — so bumping the constant to
v1.0.0.0 left the two disagreeing, and every integration deploy died with
"manifest for registry:4443/...:v1.0.0.0 not found: manifest unknown".

The failure surfaces inside an unrelated deploy, several layers from the line
that caused it, which is the expensive part. Read the constant from the source
instead, and fail loudly at setup time if it cannot be read.

Refs #82
`kamal proxy boot` failed intermittently on a random host with:

    ERROR (RuntimeError): Exception while executing on host vm1:
    Digest::Base cannot be directly inherited in Ruby
      lib/kamal/configuration/proxy/run.rb:21:in `digest'

Kamal::Cli::Proxy#boot calls drift.expected_digest inside
`on(KAMAL.proxy_hosts)`, so Proxy::Run.digest first touches Digest::SHA256
from one SSHKit thread per host, simultaneously. Nothing in lib/ required
digest, so that first reference went through Digest.const_missing, which runs
`require "digest/sha2"` outside any mutex. Two threads entering it together
observe the extension's half-built class hierarchy and one raises.

Requiring "digest" would not have fixed it — that defines the module but still
leaves SHA256 to const_missing. It has to be the implementation file.

Pre-existing, latent since config_digest was introduced; surfaced here because
one of the two Ruby 3.3 CI legs happened to lose the race. Both legs run
identical code inside the deployer container, which is what identifies this as
non-deterministic rather than a gemfile difference.

The test runs in a subprocess on purpose: the suite already pulls in digest via
test/sshkit_patch_drift_test.rb, so an in-process check would pass whether or
not lib/kamal.rb requires it.

Refs #82
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request proxy dash-proxy integration — proxy config, flags, layering and the load balancer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Coverage guard: every kamal-proxy flag is mapped or explicitly waived

1 participant