Fix prod compose ports override silently not applying - #476
Merged
Conversation
ports: [] doesn't reset a list inherited via extends; tag it !reset so postgres/ES actually stop publishing host ports in prod. Scopes check-yaml's --unsafe to just this file for the custom Compose tag.
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.
Closes #475
Problem
docker-compose.prod.yml'spostgres/elasticsearchservicesextendsthe basedocker-compose.yml's services rather than defining their own. A plainports: []override does NOT reset a list field inherited throughextends— Compose only replaces a list on a literal (non-extended) service override. Result: the base file's127.0.0.1:5432/127.0.0.1:9200host-port mapping was silently surviving into prod, confirmed live viass -tln, despitedocs/infrastructure.mdasserting the override worked.Severity (per the issue): moderated by the base mapping binding
127.0.0.1only — nothing internet-exposed — but the documented hardening was silently not in effect, and local-process DB/ES access with the compose-file credentials was live.Fix chosen: option (b) — Compose
!resetmerge tagConsidered option (a) (drop
ports:from the base file, add it explicitly wherever the dev workflow invokes compose) but the dev workflow (perdocker-compose.yml's own header comment — "spins up the database services required by the MPC Autofill backend... to enable local development through a Python virtual environment running natively on the dev machine") runs the base file directly, and nodocker-compose.dev.yml-style override already exists in this repo. Per the constraint on this build ("add a dev override file ONLY if one already exists"), creating one wasn't an option, so option (a) was not available — went with option (b) instead.Tagged both
postgresandelasticsearch'sports: []indocker-compose.prod.ymlwith the Compose merge tag!reset(ports: !reset [], Compose spec v2.24+), which forces an actual reset instead of a no-op override throughextends. Verified supported by this box's CLI (docker compose version→ v5.3.1).docs/infrastructure.md's Postgres/ES ports note updated in place to describe the real (previously wrong) mechanism and the fix.Verification
docker compose -f docker-compose.yml config, base file untouched): postgres/ES both still render their127.0.0.1:5432/127.0.0.1:9200ports:mapping — dev behavior unchanged.docker compose -f docker-compose.prod.yml config, with dummy env vars for interpolation): postgres/ES both renderports: null(i.e., no host-port mapping) whileexpose: ["5432"]/["9200"](container-to-container) stays populated. Confirms!resetactually works against this CLI version. Re-rendered after theexpose:typo fix below — confirmedports: nullunchanged for both services, only elasticsearch'sexpose:value moved from5432to9200; nothing else shifted.docs_lint.pyclean.check-yamlhook doesn't know the Compose-specific!resettag ("could not determine a constructor for the tag '!reset'") — scoped a secondcheck-yamlinstance with--unsafe(syntax-only, not "skip the file" — still catches real YAML syntax errors) to justdocker-compose.prod.ymlin.pre-commit-config.yaml, excluded that one file from the default strict instance. All hooks pass.Rider note (per issue #475)
The live listener does not disappear until the postgres/ES containers are next recreated — Docker doesn't retroactively apply a compose-file port change to an already-running container. This rides on #462's (
client_connection_check_interval) already-scheduled deliberate postgres recreate — if the two land together, one recreate covers both fixes. Until that recreate happens,127.0.0.1:5432/127.0.0.1:9200remain live on this box exactly as before.Follow-up commit (same PR)
elasticsearch'sexpose:indocker-compose.prod.ymlwas- 5432(should be9200) — pre-existing onmaster, not introduced by this PR's first commit, but fixed here in an additive commit to keep this PR's "prod compose port hygiene" story complete since it's the same file this PR already owns.expose:doesn't gate traffic the wayports:does (informational only, container-to-container), so this was cosmetic rather than a live-traffic bug, but it was pointing at the wrong port.Checklist
!resetverified against this box's Compose CLI (v5.3.1)docker compose -f docker-compose.prod.yml configshowsports: nullfor postgres and elasticsearchdocker compose -f docker-compose.yml config(dev) unchanged, still publishes both portsdocs/infrastructure.mdports note updated to match verified realitycheck-yaml --unsafescoping) documented aboveelasticsearchexpose:typo (5432 -> 9200) fixed in a follow-up commit; re-rendereddocker compose -f docker-compose.prod.yml configconfirmsports: nullunchanged for both services