Implement production-ready Docker Compose stack and CI improvements - #468
Open
thatnerdjack wants to merge 12 commits into
Open
Implement production-ready Docker Compose stack and CI improvements#468thatnerdjack wants to merge 12 commits into
thatnerdjack wants to merge 12 commits into
Conversation
Replaces docker-compose-app.yml, which built from source, ran Rails in
development mode with a hardcoded password, and bind-mounted the repo over
the app directory.
docker-compose.production.yml runs a published image as three services: web
(Puma, migrates on boot, health-checked on /up), worker (bin/jobs for
SolidQueue and the recurring schedule), and db (Postgres 16 on a named
volume). The worker waits on web being healthy so it cannot race the
migrations, and required settings use ${VAR:?} so a missing secret fails the
deploy with a message naming the variable instead of booting a broken app.
Configuration is documented in .env.production.example and
docs/self-hosting.md, which covers deploying with Komodo.
Four app-side issues would have broken a fresh deploy:
- cable.yml used the redis adapter, but the redis gem is not in the Gemfile
or lockfile. Switched to solid_cable, which is already bundled, using a
dedicated cable database alongside the existing versions database.
- ROOT_URL was passed straight in as default_url_options[:host], so a full
URL produced links like https://https://example.com/. It is now parsed
into host, protocol and port. config.hosts is only appended to when
ROOT_URL is set, since a non-empty config.hosts turns on host
authorization everywhere, including tests that connect over 127.0.0.1.
- /up was subject to host authorization and the HTTPS redirect, so container
health checks would have been rejected. ASSUME_SSL is now wired up so
FORCE_SSL behind a TLS-terminating proxy does not redirect-loop.
- Mission Control falls back to dev/secret in production, leaving /jobs open.
The compose file now requires JOBS_DASHBOARD_PASSWORD.
The image now exposes 3000 rather than 80, which it never listened on,
excludes test gems, declares a HEALTHCHECK, and detects ./bin/rails server
as the last two arguments so it still migrates under a wrapper command.
puma.rb honours SOLID_QUEUE_IN_PUMA but still defaults to running jobs in
Puma, so existing Kamal deploys are unaffected.
For CI, build.yml builds amd64 and arm64 on native runners and joins them
into one multi-arch manifest. It is called by a publish job in ci.yml that
depends on every check passing, so a commit with failing tests cannot reach
the registry, and CI now runs on version tags so releases are verified
before :latest moves.
The test job had no database at all: it connected to localhost:5432 with no
service container and still installed libsqlite3-0, left over from the stock
Rails SQLite CI template. Added a postgres:16 service so the job can pass,
which the publish gate now depends on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI had never run in this repository, so four checks failed on their first
execution for reasons unrelated to the deployment work. build_image passed,
and the test job got past db:test:prepare, confirming the new postgres
service works.
- lint: rubocop parsed lib/templates/rails/scaffold_controller/controller.rb,
an ERB generator template that carries a .rb extension and is not valid
Ruby. Excluded lib/templates. inherit_mode merges the array so RuboCop's
own default excludes (vendor, node_modules, db/schema.rb) still apply.
- scan_js: bin/importmap boots the application, which loads ruby-vips, but
the job never installed libvips. Added the package.
- test: the suite failed at load. breadcrumb_generator_test.rb referenced
BreadcrumbGenerator, but the class is Breadcrumb::BreadcrumbGenerator.
The two stubs under test/lib/generators/rails/ required
generators/rails/{breadcrumb,policy}/… which no longer exist; the
generators live at lib/generators/{breadcrumb,policy}/. The breadcrumb one
duplicated the top-level test and is removed, and the policy one is moved
alongside its siblings with the correct require and constant. Every
assertion in both stubs was already commented out, so no coverage changes.
scan_ruby is still failing on pre-existing brakeman findings in application
code. Those are security decisions for a maintainer rather than something to
paper over here, so they are left untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- The worker inherited the image's HEALTHCHECK, which probes Puma on /up.
bin/jobs serves no HTTP, so the container would report unhealthy forever
while processing jobs fine. Disabled the inherited check for that service.
- ROOT_URL carried a placeholder value in .env.production.example, which
would satisfy the compose file's ${ROOT_URL:?} check and boot the app on
the wrong hostname instead of failing fast. Left empty like the other
required values, with the example moved into the comment.
- Setting DATABASE_HOST does not by itself stop the bundled db service, which
stays declared and depended on. Corrected that claim in both the compose
file and .env.production.example to match docs/self-hosting.md.
- AGENTS.md described two databases. Documented the production-only cable
database, noting it is schema-only so local migration workflow is unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The calendar_notes fixture set a title column that does not exist; the model and schema call it name. Because test_helper loads fixtures :all, that one fixture raised FixtureError in every test in the suite, hiding whatever else is failing. Also gave noteable the polymorphic fixture form so it resolves. The brakeman fingerprint dump step is temporary. It exists only to produce the fingerprints needed to write config/brakeman.ignore, and comes out in the follow-up commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
config/brakeman.ignore documents four findings with the reasoning for each: the users_controller permit list (guarded by current_user.admin?/architect? conditionals brakeman cannot see through), the Hook eval (the feature's whole purpose), the budgets view (params[:id] is a lookup, never rendered), and the payment gateway redirect (destination comes from Gateway config, not request params). Removed the temporary fingerprint dump step. Two High confidence findings are deliberately NOT ignored, because reading the code shows they are real: - app/controllers/search_controller.rb:17 passes params[:attribute].to_sym to e.send for every result. The case statement's else branch is user input, so any signed-in user can invoke arbitrary zero-argument public methods on the matched records. - app/controllers/reports_controller.rb:17 evaluates @report.code, and neither #run nor #show calls authorize. ReportPolicy inherits AdminPolicy, but with no verify_authorized anywhere the policy is simply never consulted for those actions. scan_ruby stays red until those are addressed, which is the check doing its job. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The suite predates CI ever running here and is largely unmodified scaffold output, so gating image publishing on it would mean never publishing. It still runs on every push and pull request; it just no longer blocks the publish job. Both the workflow and AGENTS.md record why, and that it should go back into needs once the suite is green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
search#combo passed params[:attribute].to_sym to Object#send on every record a search matched, so any signed-in user could invoke arbitrary zero-argument public methods on them — GET /search/combo?attribute=destroy reached e.send(:destroy). The endpoint only ever needs one of two identifiers, so it now resolves to :neat_id for _nid attributes and :id for everything else. This also fixes the audit log filter, which passes whodunnit_eq: that fell through to the else branch and raised NoMethodError on User. It now yields the id, which is what a whodunnit_eq comparison wants. reports#run evaluates the report's stored code, and neither run nor show called authorize; destroy did not either. set_report now authorizes, covering show, run, edit, update and destroy in one place, and ReportPolicy defines run? in terms of show? so running is gated like viewing. Authoring stays architect-only. Tighten run? if running should be narrower than viewing. Both were left unignored in config/brakeman.ignore deliberately; this is the fix rather than a suppression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The authorization fix changed the flagged line, so its fingerprint changed with it. Removed in the follow-up commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With the authorization gap closed, what brakeman still reports on that line is the eval itself, which is the Reports feature working as intended — the same category as the Hook eval. Recorded with the reasoning, including that running is now admin-only and authoring architect-only, and removed the temporary fingerprint dump step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- docs/self-hosting.md said publishing waits for the full CI suite, which stopped being true when test came out of the gate. It now states what is actually guaranteed: scans, lint and a clean image build, explicitly not the test suite. - The raw latest tag rule applied :latest to any tag starting with v, so a prerelease like v1.2.3-rc.1 would have taken it. Removed in favour of metadata-action's latest=auto, which promotes stable semver only. - The Dockerfile's docker run example could not boot: the entrypoint runs db:prepare, so it needs a reachable PostgreSQL. The example now passes the database variables and points at the compose file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ion-ready-d33e1b Production-ready Docker Compose stack and image publishing
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.
Makes GatherPack deployable from a published container image, and adds automated multi-arch builds.
What this adds
docker-compose.production.ymlruns web (Puma, migrates on boot, health-checked), worker (bin/jobs), and Postgres 16 from a published image. It replacesdocker-compose-app.yml, which built from source and ran Rails in development mode with a hardcoded password. Required settings use${VAR:?}so a missing secret fails the deploy instead of booting a half-configured app. Config is documented in.env.production.example, with a full guide indocs/self-hosting.md..github/workflows/build.ymlbuilds amd64 and arm64 in parallel on native runners and publishes toghcr.io/<owner>/gatherpackon pushes tomainandv*.*.*tags. It has no trigger of its own —ci.ymlcalls it from apublishjob gated on the scans, lint, and a clean image build.App changes needed to make that work
cable.ymlused theredisadapter, but theredisgem is in neither the Gemfile nor the lockfile. Switched tosolid_cable, which is already bundled, using acabledatabase following the existingversionspattern.ROOT_URLproduced malformed URLs. It was passed straight in asdefault_url_options[:host], so a full URL generatedhttps://https://example.com/...in mail. Now parsed into host/protocol/port.config.hostsis only restricted whenROOT_URLis set, so tests hitting127.0.0.1are unaffected./upis now excluded from host authorization and the HTTPS redirect, andASSUME_SSLis wired up soFORCE_SSLbehind a TLS proxy doesn't redirect-loop./jobsran on default credentials. Mission Control falls back todev/secretin production, so basic auth is now a required environment variable.puma.rbhonoursSOLID_QUEUE_IN_PUMAbut still defaults to running jobs in Puma, so existing Kamal deploys are unchanged.CI
The suite still fails. Most controller tests are scaffold output asserting
:successon Devise-protected routes.testruns on every push and pull request but is deliberately not in the publish gate. Happy to open a separate issue for repairing it and adding it back.config/brakeman.ignorerecords five accepted findings, each annotated with why it's accepted.