Repository navigation
feat(proxy): expose the response cache in deploy.yml - #86
Merged
Merged
Conversation
The RFC 9111 response cache was fully built in the proxy and completely
unreachable from deploy.yml. Twelve flags had no key.
proxy:
cache:
enabled: true
max_ttl: 300
max_body: 1_048_576
max_variants: 8
vary_headers: [ Accept-Encoding ]
vary_cookies: [ locale ]
allow_set_cookie: false
run:
cache:
store: redis://cache.example.com:6379/0
store_timeout: 2
memory_size: 134_217_728
lease_ttl: 30
lease_wait: 1
The split follows which kamal-proxy subcommand owns each flag: the policy is per
service and rides the deploy, the store is proxy-wide and rides the run command,
so changing it moves config_digest and reboots the proxy once.
Twelve flags rather than the nine the issue's prose lists. The coverage canary's
waiver for this issue also named --cache-max-variants, --cache-lease-ttl and
--cache-lease-wait; exposing only the nine would have left the waiver
half-populated with no issue left to attribute the rest to.
Sizes are plain byte integers, not "1MB" strings. The issue asked for sizes to
follow whatever buffering already accepts, which is integers, and --cache-max-body
is a Cobra Int64Var that would reject "1MB" anyway.
The lease flags take negative values to switch cross-node coalescing off. That
survives the space-separated rendering the run command uses -- pflag's
parseLongArg reads the argument after --flag unconditionally, with no
leading-dash check -- so unlike the ACME booleans these need no `=` group.
Two silent failures are now config errors: a tuned cache block with no
`enabled: true`, which kamal-proxy ignores wholesale, and a store that is
neither memory nor a redis(s):// URL, which kills the proxy container in preRun
rather than failing a deploy.
seconds_duration moved from a private method on Proxy to Kamal::Utils, since
Proxy::Run needs it now too.
Closes #75
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
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
The RFC 9111 response cache, reachable from
deploy.yml. Policy per service, store proxy-wide.lib/kamal/configuration/proxy.rb— acache_optionsgroup merged intodeploy_options, alongsidetls_optionsandtls_domains_options.lib/kamal/configuration/proxy/run.rb— acache_optionsgroup merged intorun_command_options, so the store ridesconfig_digest.lib/kamal/configuration/validator/proxy.rb— two silent failures become config errors (below).lib/kamal/utils.rb—seconds_durationlifted out ofProxy's privates soProxy::Runcan use it too.test/proxy_flag_coverage_test.rb— both issue-75 waivers deleted; all twelve flags proven emitted from the maximal fixture.Closes #75
Two silent failures turned into config errors
cache:withmax_ttletc. but noenabled: trueproxy/cache: max_ttl has no effect without enabled: truerun.cache.store: memcached://…preRunrejects it, so the proxy container exits at bootproxy/run/cache: store must be 'memory' or a redis:// or rediss:// URLThe second matters more than it looks: an unsupported store does not fail a deploy, it takes the proxy down.
Test plan
bundle exec rubocop --parallel— clean, 210 filesbuilder_test/build_test, identical on pristinedashHEAD (host-arch dependent, pass in CI)cache.enabled: trueagainst an app sendingCache-Control: public, max-age=60; confirm a second request is served from cacheSet-Cookieresponse is refused withallow_set_cookie: falseand stored with it trueredis://store; confirm a fetch on one warms the otherrun.cache.storeand confirm exactly one proxy reboot per hostAutomated coverage
enabled alone emits only --cachedurations use the seconds convention and sizes are byte counts300→300s,1_048_576stays an integervary_headers and vary_cookies become repeated flags--cache-vary-header=pairs, checked againstinternal/cmd/deploy.go'sStringSliceVarnegative lease durations survive the space-separated rendering-1sreaches the proxy intact rather than reading as the next flagconfig_digest changes when the store changesno cache block leaves the deploy command unchanged/no run cache block leaves the run command unchangedpolicy keys without enabled are a config error/an unsupported store fails at config timeevery kamal-proxy deploy/run flag is exposed or waivedDeviations & judgment calls
Deviations from the issue
--cache-max-variants,--cache-lease-ttland--cache-lease-waitare also unexposed and attributed here. Exposing only the nine would leave the waiver half-populated with no issue left to attribute the rest to.max_body: 1_048_576, not1MB. The issue's example contradicts its own rule ("sizes follow whateverbuffer-memory/max_request_bodyalready accept" — those are plain integers).--cache-max-bodyis a CobraInt64Varthat would reject1MBoutright.seconds_durationmoved toKamal::Utilsrather than being copied into a second class.Judgment calls
Proxy::Cacheclass, unlikeProxy::Acme. Acme earned one — secrets resolution, an env file, a provider allowlist, credential names in the digest. The cache store is five scalars, so it follows thetls_optionsshape: a named private method merged into the options hash. Each file keeps its own local convention.buffer-requests,path-timeout, …). So a load-balanced deploy caches at the edge and again per host — a valid two-tier cache, but worth knowing before you tunemax_bodytwice.run.cache.storeis a plain value, so a Redis URL with a password in it lands on the docker command line — host process listings and kamal's audit log. TheCACHE_STOREenv fallback leaks identically. First-class proxy.run.acme block with secrets-backed DNS credentials #73 built exactly the right machinery for this (a 0600 env file passed with--env-file), but generalizing it from "ACME credentials" to "proxy secrets" is a refactor of just-merged code that this issue did not ask for. The docs stub warns about it; worth its own issue.run.cache.storedoes not reach it — the LB container runs the image's default CMD. Same pre-existing gap flagged in First-class proxy.run.acme block with secrets-backed DNS credentials #73 and Expose on-demand TLS, mTLS client CA and ACME cache path in deploy.yml #74.store_timeout/lease_ttl/lease_waitare not validated againststore. They only matter with a shared store, but the proxy ignores them harmlessly withmemory, so this is not the silent-failure class the other two checks catch.