Conversation
| # value is not decisive as it is used as a last resort host regardless. | ||
| # However, you must set it for any further virtual host explicitly. | ||
|
|
||
| ServerName chpl.ahrqdev.org |
There was a problem hiding this comment.
If we're committing this, for all time, we should probably use the names that are actually correct. AHRQ has been gone for a while, and while it's probably not hurt anything, this would be a good time to update it
There was a problem hiding this comment.
Perhaps make the server name (and/or ServerAdmin below) a parameter as well
There was a problem hiding this comment.
Good catch on both counts — went with the parameter route, in fc08fbc.
ServerName/ServerAdmin in the maintenance vhosts are now ${SERVER_NAME}/${SERVER_ADMIN}, rendered at container start by a new docker/docker-entrypoint-maint.sh (defaults: localhost and chpl@ainq.com), which mirrors how the live image already templates BACKEND_URLS/API_KEY. So the stale chpl.ahrqdev.org is gone and the same maintenance image can serve any environment's hostname without a rebuild.
Two details worth flagging:
001-error.confhad the identical stale pair, and since the image doesa2dissite 000-default.conf/a2ensite 001-error.conf, that is the vhost actually in effect — both are parameterized.- The entrypoint renders from pristine copies stashed at
/etc/apache2/conf-templates/rather than substituting in place, so a container restart re-renders instead of finding the placeholders already consumed.envsubstis given an explicit variable list so Apache's own${APACHE_LOG_DIR}survives untouched.
I verified the rendering logic locally (defaults, override, repeat runs, ${APACHE_LOG_DIR} preserved) but could not build the image — no Docker daemon on my machine. Also left chpl@ainq.com as the default admin address; say the word if that one is stale too.
There was a problem hiding this comment.
Pull request overview
This PR shifts environment-specific configuration (notably the API key and backend routing targets) from build-time/static artifacts toward runtime/container templating, supporting the goal of reducing reliance on Bamboo-provided build outputs and enabling consistent image builds/promotion across environment branches.
Changes:
- Adds a runtime-loaded
/env-config.jsfile (rendered at container start) and updates the frontend to readAPI_KEYfromwindow.__envwith a fallback. - Introduces Docker build/serve setup (including an entrypoint that templates Apache proxy/status config and runtime env config).
- Adds a GitHub Actions workflow to build and publish Docker images to GHCR (including an optional maintenance-mode image path).
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/unsupported-browser.html | Loads runtime /env-config.js before other scripts. |
| src/index.html | Loads runtime /env-config.js so the SPA can read runtime config. |
| src/error.html | Loads runtime /env-config.js on the error page. |
| src/app/services/services.module.js | Replaces hardcoded API key constant with getApiKey() lookup. |
| src/app/api/axios.jsx | Replaces hardcoded API-Key request header with getApiKey(). |
| src/app/api/api-key.js | Adds getApiKey() helper reading from window.__env with fallback. |
| docker/env-config.template.js | Adds template for runtime-generated env-config.js. |
| docker/Dockerfile.maint | Adds maintenance-mode Apache image build definition. |
| docker/Dockerfile | Adds multi-stage build + Apache runtime with templated config and entrypoint. |
| docker/docker-entrypoint.sh | Renders Apache proxy/status config and env-config.js from env vars at startup. |
| docker/apache2/sites-available/001-error.conf | Adds error vhost config for maintenance image. |
| docker/apache2/sites-available/000-default.conf | Adds default vhost with proxy/balancer configuration for maintenance image. |
| docker/apache2/ports.conf | Adds ports configuration for maintenance image’s Apache layout. |
| docker/apache2/mods-enabled/status.conf | Adds mod_status configuration for maintenance image. |
| docker/apache2/apache2.conf | Adds Apache main config for maintenance image. |
| docker/apache-config/status.conf.template | Adds runtime-templated mod_status config for the live image. |
| docker/apache-config/proxy.conf.template | Adds runtime-templated proxy/balancer config for the live image. |
| .gitignore | Ignores generated docker/www/ staging directory. |
| .github/workflows/docker-publish.yml | Adds GHCR publish workflow for live + optional maintenance images. |
| .dockerignore | Adds Docker build context exclusions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # file that was never created. | ||
| COPY package.json yarn.lock .yarnrc.yml ./ | ||
| COPY .yarn ./.yarn | ||
| RUN yarn install |
There was a problem hiding this comment.
Good catch � added in the latest commit.
One detail that makes this more relevant than it looks: .yarnrc.yml sets enableImmutableInstalls: false, so Yarn's usual CI auto-detection is disabled here and the bare yarn install was non-immutable even in CI. The explicit CLI flag is what actually closes the gap. (That setting predates this PR � it came in with 6df1af1cd, "ci: ensure UI AQA tests can run" � so I left it alone rather than flipping a global that something else may depend on.)
Verified rather than assumed: the lockfile at this branch's HEAD is byte-identical to origin/development's, and the workflow's build job � which already runs yarn install --immutable � has succeeded on the last 8 runs there, most recently 2026-09-11. And since build-and-push declares needs: [build], any lockfile divergence fails that gate before the image build starts, so this flag can't introduce a new CI failure mode; it only stops a local image build from quietly papering over one.
The scenario where it would bite, for the record: checksumBehavior: update is also set, so a registry re-publishing a tarball under a new checksum would make Yarn want to rewrite yarn.lock, and --immutable would then fail. That's already true of the build job today, so it isn't new exposure.
Build the CHPL website image in GitHub Actions and publish it to GHCR, reducing the build's dependency on Bamboo artifacts. The image serves the built bundle from Apache and is configured at runtime rather than at build time: docker-entrypoint.sh renders env-config.template.js and the Apache reverse-proxy template, so a single image can be pointed at different backends and API keys per environment. The frontend API key is supplied as window.__env.API_KEY, which the existing browserInfo slice already reads (with a hardcoded fallback), so no application code changes are needed here. Squashed from: c4ebee7, 63ee698, a6fe98c, 8dcb37d, 2cd536c
…point BACKEND_URLS accepts a comma-separated list of backend origins, which the entrypoint expands into an Apache balancer member set so the image can front multiple API instances. Also adds an optional mod_status /server-status endpoint for the Datadog agent to poll. Access is restricted to STATUS_ALLOWED_IPS rather than a hardcoded CIDR, so it stays closed by default per environment. Squashed from: 2eb8c8b, 36d6a9e
Dockerfile.maint builds a standalone Apache image that serves a static maintenance page, so the site can be put into maintenance without keeping the application image running. The publish workflow builds and pushes this image alongside the app image. The maint-image step is guarded at the step level rather than the job level so the rest of the pipeline still runs when it is skipped. Squashed from: 9dbae16, 6c62dc6, ce62b97
… legacy image - Duplicate the Apache access log to stdout so requests are visible to the Datadog agent via container logs. - Derive the per-instance /rest1, /rest2, ... routes from BACKEND_URLS instead of hardcoding them, so they stay in sync with the balancer members. - Raise Timeout to 600s to match the timeout the legacy Bamboo-built image ran with; the default 60s cut off long-running report downloads. Squashed from: fdc3a97, 31af707, 1d5db65
The maintenance image's vhosts hardcoded ServerName chpl.ahrqdev.org - an AHRQ-era hostname that has been wrong for a while. Replace it (and ServerAdmin) with placeholders rendered at container start from SERVER_NAME/SERVER_ADMIN, matching how the live image already templates its backend and API key, so the same maintenance image can serve any environment's hostname without a rebuild. Both 000-default.conf and 001-error.conf carried the stale pair; 001-error.conf is the vhost the image actually enables. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The template interpolated API_KEY between literal quotes, so a key containing a quote, backslash or newline would emit a syntactically broken env-config.js - breaking the app for every user - or, with a crafted value, inject script into every page. The entrypoint now builds a complete, pre-quoted JSON string literal and the template renders it unquoted. httpd:2.4-alpine has no jq or python, so the escaping is busybox sed plus awk. Verified by rendering a key containing a quote, a backslash, a tab, a newline and an HTML payload, then parsing the result with node: it parses and the value round-trips byte-for-byte. Unset and ordinary keys are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… context docker-publish.yml stages dist/ into docker/www/chpl and docker/www/error for the maintenance image, so the directory is roughly twice the build output. The live image builds with `context: .`, which sent both copies along whenever they existed locally. The maintenance build is unaffected: it uses `context: docker` and so reads docker/.dockerignore rather than this file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Access was pinned to a hardcoded 10.0.1.0/24. That constant is right for the current environment, but the image is built once and could run anywhere, so any environment where that subnet means something else got server-status exposed to it - and the range could only be changed by rebuilding. Both blocks now render from STATUS_ALLOWED_IPS, reusing the variable name and the 127.0.0.1 default the live image already uses, so an unconfigured build is localhost-only and an environment can pass one value to both containers. Covers the duplicate /server-status block in 000-default.conf as well; that vhost is disabled in this image (a2dissite), so it was latent rather than live, but it carried the same hardcoded CIDR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The image build ran a bare `yarn install` while the workflow's gated `build` job runs `yarn install --immutable`, so the two could resolve differently. .yarnrc.yml sets enableImmutableInstalls: false, so the explicit flag is what makes the difference - Yarn's usual CI auto-detection is disabled here. This cannot introduce a new CI failure: build-and-push has `needs: [build]`, and that job already runs the same immutable install on the same lockfile, so any divergence fails the gate before the image build starts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…0558
httpd logged AH00558 ("Could not reliably determine the server's fully
qualified domain name") on every start, guessing an FQDN by reverse DNS. The
live image assembles its httpd.conf from the base image's stock config, which
ships ServerName commented out, and nothing set it afterwards.
docker/apache2/apache2.conf does set ServerName 127.0.0.1, but that file only
reaches the maintenance image - which is why the reported address was a real
VPC IP rather than 127.0.0.1, and why the warning appeared for the live
development container.
Rendered from SERVER_NAME (default localhost) via a new servername.conf
template, following the existing proxy.conf/status.conf pattern and reusing the
variable name the maintenance image already takes, so one value serves both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No description provided.