Conversation
This repository releases by moving the pins in .env and writing a dated CHANGELOG.md entry, and since the Keep a Changelog adoption its own changes have somewhere to go - but nothing made anyone write them down. A pull request that changes what a deployment is built from now has to touch CHANGELOG.md or carry the no-changelog label. RELEASING.md says how a bump is done, what a release is here, and where this repository sits in the ecosystem's release order. CI pins actions/checkout to a commit SHA, drops the checkout's credentials from the runner, and runs with a read-only token. Dependabot watches the pinned actions and the Ubuntu base images of both Dockerfiles, so a base image bump arrives as a reviewable change rather than as whatever the tag resolved to on rebuild day.
…steps at real documentation CHANGELOG.md already says CI changes belong under [Unreleased], but the changelog check exempted anything under .github/. Add it to the pattern. RELEASING.md told the bumper to preserve deviations "the README documents"; the README documents none. Point at the rationale comments in config/ and the changelog instead, and give the exact command for the no-changelog label the check depends on, which the repository does not yet have. The dependabot comment promised an Ubuntu LTS bump; the docker updater offers any newer tag, so say base-image bump.
Images
- Build in one apt layer with the lists removed, without apt-get upgrade,
with shallow clones and a pip cache mount. Drop gcc-multilib: nothing needs
a 32-bit toolchain and the package does not exist on arm64, so the images
now build on Apple Silicon. Replace locales-all with locales plus
locale-gen for en_US.UTF-8. Add .dockerignore files.
- mcritweb no longer bakes FLASK_DEBUG=1 into the image and is served by
gunicorn (installed explicitly, requirements.txt does not list it) instead
of the Flask development server.
- The mcrit image is built once and shared by server and worker.
Runtime
- Entry scripts run under set -eu and exec their process; the mcrit services
get init: true so SIGTERM reaches Python, and docker compose stop returns
in well under a second instead of the ten-second timeout.
- Healthchecks on mongodb, mcrit-server and mcritweb, depends_on conditions
instead of sleep 1, restart: unless-stopped in production, ${VAR:?} guards
on every interpolated tag, config and nginx mounts read-only.
- MongoDB 5.0 (end of life) moves to 8.0; the README documents the stepped
upgrade an existing storage/mongodb needs. mongod logs to stdout.
- nginx pinned through NGINX_TAG instead of nginx:latest; the TLS config
serves TLS 1.2 and 1.3 with the Mozilla intermediate ciphers, drops DHE and
ssl_dhparam, and no longer requests HSTS preload.
- nginx/ssl/*.pem are untracked (placeholders kept as .example) so a filled-in
private key cannot be committed by accident.
Scripts and CI
- build.sh uses docker compose v2; reset.sh and clone_repositories.sh get
shebangs, set -euo pipefail and no cd juggling; test_build.sh drops
--no-cache and builds the compose image names.
- CI builds both images with Buildx and a GitHub Actions layer cache, reads
the tags from .env, and has a timeout.
Verified locally: both images build, the stack comes up healthy, the web UI
answers through nginx and gunicorn, the mcrit API reports 1.9.0, and the
test entrypoint passes the 191 upstream unit tests.
Both images carried their build toolchain into the runtime and ran everything as root: build-essential, python3-dev, git and the pip cache stayed in the final layer, and a compromise of a web request had uid 0 inside the container. Split each Dockerfile into a builder stage that installs into /opt/venv and a runtime stage that copies the venv and the source across and adds only what running needs. The MCRIT image goes from 1.19 GB to 714 MB and the MCRITweb image from 1.28 GB to 787 MB, and both end on uid 10001. Running unprivileged makes ./storage/mcritweb, a host bind mount, something the operator has to hand to that uid once; both MCRITweb entry scripts now check the directory and exit with the chown command in the message instead of failing later inside Flask. entry_test.sh no longer pip-installs pytest at startup, which an unprivileged user cannot do: the image carries pytest and the script runs it directly, so it needs no make either. The base images are pinned by digest rather than by the 24.04 tag, so a rebuild cannot silently pick up a different Ubuntu, and both images carry OCI source, version and licence labels plus a .git-revision file recording the upstream commit they were built from - a label cannot hold a value resolved during the build. On the compose side, every service gets no-new-privileges, a json-file log cap of 5 files of 50 MB through one x-logging anchor so a chatty container cannot fill the host's disk, and MCRIT_AUTH_TOKEN passed through to the server and the worker. CI gains a lint job running hadolint over both Dockerfiles, docker compose config over both compose files and shellcheck over every entry script; .hadolint.yaml records why apt and pip version pinning are not enforced here. The README gains a production checklist covering TLS, the API token pairing with MCRITweb, the instance directory ownership and backups.
The README had grown into a mix of a project pitch, a partial setup guide and three version-specific maintenance notes, while the things a first deployment actually needs - the uid the containers run as, where the version pins live, what to change for TLS - were scattered or absent. It also still used docker-compose and duplicated release information that CHANGELOG.md owns. Restructure it in the order an operator meets the deployment: what is run and where the pins are, requirements, first deployment, configuration, production, upgrading, development mode, and a pointer to CHANGELOG.md for the version history. The MongoDB 5.0 stepping procedure stays because it is a one-directional host-level operation; the MCRIT 1.7.0 migration note and the SMDA fingerprint note are dropped in favour of the changelog entry and the upstream migration guide, which are where a release states what it requires. Every command is a fenced block using docker compose, and every path, variable and script name is checked against the tree.
Both MCRIT containers mounted the tracked config/ read-only over the installed package's mcrit/config/, so the only way to set a deployment's own values - an AUTH_TOKEN, an external MongoDB - was to edit files this repository tracks. Every pull then either conflicted with those edits or quietly reverted them, and config/ stopped being a reviewable default. The containers now assemble their configuration instead of mounting it: config/ is mounted at /opt/mcrit/config.default and the untracked config.local/ at /opt/mcrit/config.local, and entry_common.sh, sourced by all three entry scripts, copies the defaults into the package's config directory and then copies the overlay on top. A file in config.local/ replaces the shipped module of the same name, so it is copied whole rather than merged; .gitignore keeps everything there out of commits except the README explaining it. The development compose file keeps the old read-only mount, because /opt/mcrit is a host checkout there and assembling would write into the developer's tree. Assembling makes a second problem visible: an MCRIT upgrade can define settings that config/ has not caught up with, and the assembled module then silently lacks them. The image keeps the stock configuration at /opt/config.stock, and the startup check compares the module-level and class-level upper-case names of each stock module against the assembled one, printing a warning per module that has fallen behind. It does not descend into function bodies, where an upper-case name is a local, and a module that fails to parse is reported and skipped so one broken override does not hide the rest. It warns rather than fails: a missing setting means running on something other than an upstream default, which is not a reason to refuse to start. Verified on a built stack: all services come up healthy, the assembled mcrit/config/ holds the eight shipped modules, /entry_test.sh passes 191 tests with 91 deselected, and no warning appears for the configuration as shipped. Dropping STORAGE_MONGODB_FLAGS from a config.local/StorageConfig.py produced the expected warning, and an unparsable second override was reported by name while the first warning still printed. The development compose file was run end to end: the API answers on 8000, MCRITweb on 5000, and no assembly happens. shellcheck and hadolint are clean. Closes danielplohmann#8
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.
This is stacked on #13 and will be rebased once that merges; 388e014 is the commit to read, everything before it belongs to that pull request. Both MCRIT containers mounted the tracked config/ directory read-only over the installed package's mcrit/config/, which meant that setting anything a deployment actually needs - an AUTH_TOKEN, an external MongoDB, a different cache budget - was an edit to files this repository tracks. Every subsequent pull then either conflicted with those edits or silently reverted them, and config/ stopped being a default anyone could review, because in practice no deployment was running it unmodified.
The containers now assemble their configuration rather than mounting it. config/ is mounted at /opt/mcrit/config.default and an untracked config.local/ at /opt/mcrit/config.local, and entry_common.sh, sourced by all three entry scripts, copies the defaults into the package's config directory and then copies the overlay on top. Whole modules are copied rather than merged, which is the honest granularity for Python configuration: a file in config.local/ is a copy of the shipped module with some values changed, so the mechanism has no way to be subtly wrong about what won. .gitignore keeps everything in that directory out of commits except a README explaining it. The development compose file keeps the old read-only mount, because /opt/mcrit is a host checkout there and assembling would write into the developer's own working tree. Assembling makes a second problem visible, and the same script reports it: the image keeps the stock configuration the installed MCRIT ships at /opt/config.stock, and at startup each container compares the module-level and class-level upper-case names of every stock module against the assembled one and prints a warning for each module that has fallen behind. It warns rather than fails, since a missing setting means running on something other than an upstream default, not a reason to refuse to start.
Verified on a stack built from this branch: all five services come up healthy, the assembled mcrit/config/ holds the eight shipped modules, the test entrypoint passes 191 unit tests with 91 mongo tests deselected, and no drift warning appears for the configuration as shipped, which is the case that matters. The warning was exercised by putting a copy of StorageConfig.py into config.local/ with STORAGE_MONGODB_FLAGS deleted, which produced exactly that name in the warning; adding a second, deliberately unparsable override showed it reported by name while the first module's warning still printed, so one broken file does not hide the rest. The development compose file was run end to end as well: the API answers on 8000, MCRITweb on 5000, and no assembly happens there. shellcheck and hadolint are clean.
The check reads names, not values, so a setting present with a stale value is not drift it can see, and it does not look for settings the deployment defines that MCRIT has dropped. It parses rather than imports, which is why it cannot be fooled by a module that has side effects but also why it only sees assignments and not anything computed. Names bound inside function bodies are ignored, since an upper-case name there is a local. The overlay itself has no merge and no templating; a deployment that wants one value changed still copies a whole module and carries the rest of it, which is a deliberate trade against a configuration format this repository would then own.
Closes #8