Conversation
Templated server.xml/context.xml resolve DB credentials from container environment variables via Tomcat's EnvironmentPropertySource, and entrypoint.sh materializes the JWK signing key from an env var at startup. The image itself contains no secrets and is identical across every environment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Manual workflow_dispatch trigger; builds docker/Dockerfile and pushes to ghcr.io tagged <branch>-<sha> and <branch>-latest. No secrets required at build time. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
workflow_dispatch only appears in the Actions UI once this file exists on the default branch. Adding pull_request lets the workflow be validated on this PR first. Also fixes branch-name resolution for PR events (github.ref_name is '<pr>/merge' there, not the branch name). Must be reverted before merging to the default branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces <branch>-<sha>/<branch>-latest with:
build-<run_number> - immutable, sequential build identifier
sha-<short-sha> - immutable, traces to the exact commit
latest-<branch> - floating pointer to the most recent build from
that branch (latest-development, latest-qa,
latest-staging, latest-production once
triggered from those branches)
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Lists every environment variable the secret-free image needs or accepts (DB/JWK required-to-start vars, filesystem-path vars needing a volume mount, environment-dependent non-secret vars, and all 31 properties currently marked SECRET in environment.properties/ email.properties, grouped by feature area). Points at AudaciousInquiry/chpl-build for where real values live today without reproducing any of them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
docker/build-push-action v6 enables build provenance and SBOM attestations by default, each pushing an extra untagged manifest to GHCR per build (2 per run, with no architecture info, cluttering the package version list). Not needed here, so both are turned off. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This reverts commit 1cf2ee3.
Adds push triggers for development, qa, staging, and production so merging into any of them builds and pushes an image tagged latest-<branch>, without needing per-branch copies of the workflow. push triggers are evaluated per-ref and aren't gated by which branch is the repo's default, unlike workflow_dispatch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No code reads a keyLocation property or local JWK file anymore - JWT verification goes through Cognito's public JWKS URL. Removing the dead entrypoint.sh materialization step and its references in the Dockerfile, README, and server.xml.
.git (177M) and target/ build dirs (chpl-api/target alone is 1.7GB) were being sent to the Docker daemon as build context on every CI push despite never being referenced by docker/Dockerfile.
docker/entrypoint.sh was deleted (its only job, materializing JWK_KEY, was dead code) - the eol=lf rule for it is now orphaned.
The workflow publishes a floating latest-<branch> tag; without a concurrency group, two runs for the same branch could race and let an older commit's build finish last, overwriting latest-<branch> with a stale image.
Add a unit-tests job to docker-publish.yml that runs `mvn clean test` on JDK 21, mirroring the Bamboo chpl-build-dev "Run API Unit Tests" job. build-and-push now depends on it via needs, so a broken test suite blocks the image push instead of publishing a broken build.
Add a compile job that runs `mvn clean package -DskipTests`, the same command docker/Dockerfile's build stage runs, mirroring the "Build API"/CompileApiTask job in the chpl-build-dev Bamboo spec. build-and-push now needs both unit-tests and compile, so a broken build fails fast instead of partway through the slower Docker build.
Scratch commit for testing docker-publish.yml's unit-tests job - will be reverted.
This reverts commit cfb9ae6.
Scratch commit for testing docker-publish.yml's compile job (missing semicolon) - will be reverted.
This reverts commit 1d610f9.
Nothing enforces the ff4jUser role at the servlet-container level - no web.xml security-constraint references it. CHPLHttpSecurityConfig already secures /ff4j-console via Spring Security's own in-memory user sourced from ff4j.webconsole.username/password. The Tomcat-side file added no protection while baking a credential hash into what's otherwise a secret-free image. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
quartz.properties lists user-triggers.xml in org.quartz.plugin.jobInitializer.fileNames with failOnFileNotFound=true. Unlike its siblings jobs.xml/system-triggers.xml, which ship on the classpath via chpl-resources, this file exists nowhere in the repo or classpath, so Quartz startup would fail without it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verified every stock conf/ file (web.xml, catalina.properties, context.xml, server.xml, tomcat-users.xml/xsd, jaspic-providers.xml/xsd, logging.properties) is byte-for-byte identical between 11.0.23 and 11.0.24, and re-ran a full local smoke test (DB, Redis, Cognito, FF4J) against the rebuilt image with no behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cancel-in-progress killed a build's push step mid-flight whenever another push landed on the same branch a few minutes later, discarding completed work. Queuing instead guarantees every push gets built and that they finish in the order they were triggered. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Port 8181 was picked without a stated reason when the secret-free image was first added, and doesn't match anything else in the stack: the real dev box's server.xml listens on 8080, and chpl-build's start-tomcat-containers.sh hard-codes -p host:8080 for every environment's docker run. Verified locally (build, deploy, health check) with the corrected port. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CHPLConfig/CHPLServiceConfig's messageSource bean uses
ResourceBundleMessageSource with basename "errors-override" as the
primary bundle and "errors" as its parent fallback.
ResourceBundleMessageSource has no ignore-missing option, so every
single message lookup logged a WARN when the bundle couldn't be found
at all. The legacy Bamboo deployment worked around this by dropping an
empty errors-override.properties into Tomcat's conf dir at runtime
(made classpath-visible via shared.loader) - the new secret-free image
never created that file, since it never held any real per-environment
content to preserve. Checking in an empty file fixes it for every
environment with no runtime wiring needed.
Verified locally: ResourceBundle.getBundle("errors-override") resolves
successfully (0 keys) against the built classpath, and the app starts
and serves requests normally.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
log4j2.xml (the only config always on the classpath, regardless of the
-Denv Maven profile that never activates in docker/Dockerfile's build)
was using the "-local" appenders/loggers, which route every scheduled
job/report/cache logger to a file only - never to stdout - so none of
that output ever reached Datadog, only the general app loggers did.
Switched to the "deployed" loggers config, which routes every job
logger to both a file and a console JSON appender. Doing this
surfaced a real, previously-latent bug: Log4j2 does not merge multiple
sibling <Appenders> XIncludes within one Configuration - only the last
one processed survives, silently dropping the rest (confirmed this
also affects the never-yet-exercised resources-{dev,qa,staging,
production}/log4j2.xml, which has the identical pattern). Tried the
documented XPointer child-selection workaround first
(xpointer(/Appenders/node())) but this JVM's XInclude engine doesn't
support the xpointer() scheme at all - it fails to parse and Log4j2
silently falls back to its bare-minimum default configuration instead,
which is worse (no job routing at all, not even to file).
Fixed by merging log4j2-xinclude-file-appenders-console.xml's Console
appenders directly into log4j2-xinclude-file-appenders.xml, so
log4j2.xml only ever needs one physical <Appenders> block. Verified
locally: 0 "Unable to locate appender" errors (previously 66), and
confirmed real job-triggered JSON output on stdout for
cognitoUserCacheRefreshJob, directReviewCacheRefreshJob,
listingSearchCacheRefresh, sharedDataStore, and redisson.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Reversible marker to confirm the ready-for-integration flow pushes ONC-5395's Bamboo/Docker migration work to upstream/development and kicks off docker-publish.yml. Revert once the test build is confirmed.
Reverts the docker-publish.yml push-trigger marker now that it's confirmed working. Adds a standalone failing test to verify the unit-tests job correctly gates build-and-push from running. Both changes are temporary and will be reverted after this test build.
Confirmed docker-publish.yml's unit-tests job correctly gates build-and-push on failure. Removing the temporary test now that the negative-path test is complete.
quartz.properties' default profile (the one the Docker build actually packages, since it never passes -Denv=<name>) only references jobs.xml,startup-triggers.xml. user-triggers.xml was added under the mistaken belief failOnFileNotFound required it, but Quartz never looks for it in this image - confirmed by extracting the packaged quartz.properties and tracing CHPLServiceConfig's schedulerFactory bean. Same category as the earlier tomcat-users.xml revert.
docker/Dockerfile ran `mvn clean package -DskipTests` with no -Denv flag,
so chpl-resources' environment-specific Maven profile never activated and
resources-{dev,qa,staging,production}/log4j2.xml never got layered onto
the WAR. That file defines the catch-all gov.healthit.chpl logger and the
chplserviceJson console appender - without it, Datadog's chplservice
service tag never appears for images built this way. Confirmed via
Datadog: CHPL-DEV-API's chplservice logs stopped the moment its container
switched to the GHCR image, while job-specific loggers (unaffected,
defined in the shared/base resources) kept working.
Threaded a MAVEN_ENV build arg through docker-publish.yml, computed from
the branch being built (development/qa/staging/production -> dev/qa/
staging/production, matching chpl-build-common's
BuildEnvironment.getMavenEnvProperty()), instead of hardcoding one
environment into a Dockerfile shared by all of them.
Also finished a fix the prior "make deployed log4j2 config the default"
commit (8b4471e) explicitly flagged but deferred: since -Denv never
activated, resources-{dev,qa,staging,production}/log4j2.xml were
never-exercised and still had the same "two sibling <Appenders> XIncludes"
bug that commit fixed in the base config (only the last one survives,
silently dropping the other's appenders). Removed their now-redundant
log4j2-xinclude-file-appenders-console.xml include - its content was
already merged into log4j2-xinclude-file-appenders.xml by that commit -
and deleted the now-fully-orphaned file.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aging Regression from the -Denv activation fix (01a2dd2): resources-dev/qa/ staging's quartz.properties set org.quartz.plugin.jobInitializer.fileNames=jobs.xml,system-triggers.xml, user-triggers.xml with failOnFileNotFound=true, but that file was deleted from the image on 2026-08-10 (50468af) on the (then-correct) assumption that the Docker build never passed -Denv, so only the base quartz.properties (no user-triggers.xml) ever got packaged. Activating -Denv reintroduced the dependency. Confirmed via Datadog: chpl-api-dev-inst-1's Tomcat process started and bound its port, but the chpl-service webapp's Spring context failed entirely - UnsatisfiedDependencyException chain (CHPLHttpSecurityConfig -> apiKeyManager -> chplEmailFactory -> chplSchedulerReference -> schedulerFactory: "File named 'user-triggers.xml' does not exist") - container looked "up" but never became a working app server. Restored the file and its COPY line verbatim from before the revert. production's quartz.properties doesn't reference user-triggers.xml, so this is a no-op there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous fix (d502b79) restored the file but put it back at the same path an untested, cargo-culted placement had used: /usr/local/ tomcat/conf/user-triggers.xml. Verified against the real published image (ghcr.io/chpladmin/chpl-api@sha256:745cd984..., confirmed via Bamboo's pull log as the digest actually deployed to chpl-api-dev-inst-1) that this placement never worked and never could: - Decompiled quartz-2.5.2's XMLSchedulingDataProcessor: the jobInitializer plugin loads fileNames exclusively via ClassLoadHelper.getResourceAsStream() - pure classloader lookup, no java.io.File/working-directory fallback for the actual content. - catalina.properties' common.loader (checked into this image, unlike the host-mounted one QA/staging/prod's non-GHCR build relies on) only lists ${catalina.home}/lib - never conf/, shared.loader, or server.loader. - Confirmed empirically inside the real image using Tomcat's own org.apache.catalina.startup.ClassLoaderFactory (from its bootstrap.jar) with catalina.properties' actual repository list: a file placed in lib/ resolves via getResourceAsStream; conf/ never would. This is why chpl-api-dev-inst-1 kept failing with the identical "File named 'user-triggers.xml' does not exist" error even after Bamboo pulled the digest containing d502b79's fix - the file was present in the image, just never reachable from the classloader Quartz actually uses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a GitHub Actions-driven Docker build/publish path and checks in the Tomcat configuration needed to run the chpl-service.war with environment-driven configuration, reducing reliance on external (Bamboo-provided) artifacts/config.
Changes:
- Add a multi-stage Docker build that compiles the WAR with an environment-selected Maven profile and deploys it to a Tomcat 11 runtime image.
- Check in Tomcat configuration (server/context/catalina properties) and a Quartz
user-triggers.xmltemplate to support non-prod Quartz configs. - Consolidate Log4j2 XInclude appender definitions so console + file appenders work together across environment-specific configs; add a GitHub workflow to build/test/publish images to GHCR.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docker/tomcat-conf/user-triggers.xml | Adds an (empty) Quartz scheduling-data template to satisfy non-prod Quartz config expectations. |
| docker/tomcat-conf/server.xml | Introduces a Tomcat server config including a JNDI jdbc/openchpl resource populated from env vars. |
| docker/tomcat-conf/context.xml | Adds a ResourceLink to expose the global jdbc/openchpl resource to the webapp. |
| docker/tomcat-conf/catalina.properties | Enables ${ENV_VAR} substitution in Tomcat XML via EnvironmentPropertySource. |
| docker/README.md | Documents runtime environment variables and operational expectations for the Docker image. |
| docker/Dockerfile | Builds the WAR from source with a Maven env profile and packages it into a Tomcat image with checked-in config. |
| chpl/chpl-resources/src/main/resources/errors-override.properties | Adds an empty override bundle to prevent noisy missing-bundle warnings. |
| chpl/chpl-resources/src/main/resources-*/log4j2.xml | Removes the console-appenders XInclude to rely on the merged appenders include. |
| chpl/chpl-api/src/main/resources/log4j2.xml | Switches local config to use the shared XInclude files. |
| chpl/chpl-api/src/main/resources/log4j2-xinclude-file-appenders.xml | Merges console JSON appenders into the primary appender include. |
| chpl/chpl-api/src/main/resources/log4j2-xinclude-file-appenders-console.xml | Deletes the now-redundant console-appenders include file. |
| .github/workflows/docker-publish.yml | Adds CI to run unit tests, compile, then build/push Docker images tagged by branch/run/SHA. |
| .dockerignore | Adds Docker build-context excludes to keep builds smaller/faster and avoid copying outputs like target/. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Adds actions/setup-java's built-in ~/.m2 cache to the unit-tests and compile jobs, and a BuildKit cache mount + type=gha cache backend for the mvn build that runs inside the Docker image build, so repeated CI runs stop re-downloading the full dependency tree from Maven Central (which previously triggered 429 Too Many Requests failures).
The compile job already ran the identical mvn clean package -Denv=<env> command that docker/Dockerfile's build stage repeated from scratch, so every image build re-downloaded and recompiled the whole dependency tree instead of reusing the setup-java-cached artifact that already existed. Pass the WAR between jobs via upload/download-artifact instead, and drop Docker's now-redundant Maven build stage entirely. OCD-5395
kekey1
reviewed
Sep 8, 2026
Without a factory attribute, Tomcat 11 falls back to DBCP2, which ignores the legacy maxActive/maxWait names and would silently run the pool at its own default of maxTotal=8. Naming the Tomcat JDBC pool factory keeps the maxActive/maxIdle/maxWait values carried over from the Bamboo server.xml meaningful and makes the pool implementation deterministic. OCD-5395 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
These two lines were needed at 8b4471e, when the Docker build passed no -Denv and chpl-api's own log4j2.xml therefore won on the WEB-INF/classes classpath. 01a2dd2 made the build pass -Denv, and chpl-api/pom.xml's environment-specific profile copies chpl-resources/src/main/resources-${env} into WEB-INF/classes ahead of src/main/resources, where first-copied wins. resources-{env}/log4j2.xml now shadows this file in every deployment, so the change is a no-op everywhere except a plain `mvn package`. That one remaining case is local development, where it is a regression: the deployed fragments carry no STDOUT or chplServiceJsonLog appender refs and none of the org.hibernate / com.fasterxml.jackson / org.jose4j / org.springframework noise-suppression loggers the -local variants define. The substantive logging fix in this branch is unaffected - it lives in the resources-{env}/log4j2.xml files, which drop the second sibling <Appenders> xinclude that Log4j2 silently discards. OCD-5395 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…stub Replaces the placeholder errors-override.properties with a code change, per review. Both messageSource beans used ResourceBundleMessageSource, which has no ignore-missing option and logs a WARN per lookup when the bundle is absent, so the file had to exist purely to keep the log quiet - chpl-build's override-api-properties.sh writes an equally empty copy for the same reason. ReloadableResourceBundleMessageSource treats a missing bundle as absent, and takes both basenames in precedence order, so the parent-source wiring goes away too. CHPLServiceConfig already read this file with @propertysource(ignoreResourceNotFound = true), so optional is now consistent. Verified: ReloadableResourceBundleMessageSource and setBasenames(String...) confirmed present in spring-context 7.0.9 via javap; checkstyle clean on both files. Note `mvn clean compile` fails on this branch with ~413 pre-existing Lombok "cannot find symbol getX()" errors, identically with these changes stashed - unrelated to this commit. OCD-5395 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the cleanup started in e1a1da5 (2021-02-22), which deleted user-triggers.xml from all four resources-{env} directories but only removed it from production's jobInitializer.fileNames - dev/qa/staging kept listing a file that no longer existed, with failOnFileNotFound=true. Since then the file has been an empty template kept alive purely to stop the app from crashing on startup, shipped from two repos: docker/tomcat-conf here and chpl-build's CreateUserTriggersTask at deploy time. Nothing has used it since 2021. Before then it held per-person cron triggers (a Summary Statistics Email to a named address, and similar), which are now managed at runtime through SchedulerController and ChplRepeatableTrigger / ChplOneTimeTrigger against the clustered JDBC job store - DB-backed, and it survives deploys rather than needing a file edit plus a restart. dev/qa/staging now list jobs.xml,system-triggers.xml exactly as production does, and both of those ship on the classpath via chpl-resources. This also removes the requirement Copilot flagged, rather than satisfying it: no deployment of any profile, Docker or otherwise, needs the file on its classpath now. chpl-build's CreateUserTriggersTask / override-user-triggers.sh becomes vestigial - it writes a file nothing reads, harmless, to be retired there. OCD-5395 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
715ed68 named the tomcat-jdbc pool factory so the maxActive/maxIdle/maxWait values carried over from the Bamboo server.xml would stay meaningful. That had an unintended side effect: tomcat-jdbc opens initialSize connections eagerly, so Tomcat now builds this pool during StandardServer.startInternal - before any webapp exists, under the common classloader. postgresql-*.jar ships only in the webapp's WEB-INF/lib, so that classloader cannot see it and every container start logs a stack trace ending in: java.sql.SQLException: Unable to load class: org.postgresql.Driver from ClassLoader:java.net.URLClassLoader@... It was non-fatal - the webapp's own ResourceLink lookup runs on a webapp thread whose context classloader does see the driver, so the pool gets built there and connections work - but it produced a failing stack trace on every boot, which trains people to ignore startup errors and could mask a real DB misconfiguration. On DEV it fired ~20 log lines per container per start from 2026-09-09 onward. Dropping the factory attribute returns this to Tomcat's built-in DBCP2, which lives in tomcat/lib and creates its pool lazily (initialSize defaults to 0), so nothing connects until a webapp thread asks for it. That is the pool that ran from the image's introduction on 2026-07-21 until 2026-09-09 without this error. The attributes are renamed to DBCP2's names to preserve 715ed68's actual goal. This is required, not cosmetic: DBCP2 has no setMaxActive, and maxWait now binds to setMaxWait(Duration) where "10000" does not convert - leaving either name in place would silently run the pool at DBCP2's default of maxTotal=8. Verified against the bundled tomcat-dbcp-11.0.25, which exposes setMaxTotal(int), setMaxIdle(int) and setMaxWaitMillis(long). Trade-off: DBCP2's lazy init means a bad DB_URL surfaces on first use rather than at startup. Accepted - the eager check is what broke, and the application ran on lazy init for years. OCD-5395 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
surveillance.newlineCharacterFound throws NoSuchMessageException on chpl-dev,
but everything the code describes is verifiably correct:
- the key is present at errors.properties:723 in the exact deployed image
(build-69, config digest matched the running container's image_id)
- that file parses as UTF-8 java.util.Properties into 959 keys and returns
a value for the code
- no shadowing errors.properties in WEB-INF/classes, no XML or alternate
messageSource bean anywhere in the repo
- errors.properties is byte-identical between the last clean build (build-66)
and the first failing one (build-69)
- Spring 7.0.9 with these exact basenames resolves the code standalone, from
the deployed jar and from a directory
- the same standalone test run *inside the running container*, on the real
WEB-INF/lib classpath, also resolves it
That leaves the live Spring context as the only remaining difference, and it
cannot be reproduced outside a deployed WAR. So ask the running application
what it got instead of inferring it.
Probes four codes spread across the bundle rather than only the failing one:
if every probe comes back **UNRESOLVED** the whole bundle is unavailable in
this context and the code merely happens to be the only one these jobs ever
request; if only the last one fails it is genuinely key-specific. Also logs the
concrete MessageSource class, so a DelegatingMessageSource with no parent is
immediately distinguishable from a ReloadableResourceBundleMessageSource.
Uses the defaultMessage overload of getMessage, so a miss returns the marker
instead of throwing - this diagnostic cannot itself break startup.
Temporary. Remove once root-caused.
OCD-5395
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kekey1
requested changes
Sep 16, 2026
kekey1
requested changes
Sep 16, 2026
OCD-5395 Bumps all three pins together, per the "Tomcat version updates" procedure in chpl-documentation's docs/ghcr_container_images.md: docker/Dockerfile 11.0.25-jdk21 -> 11.0.26-jdk21 chpl-service/pom.xml dbcp 11.0.23 -> 11.0.26 chpl-service/pom.xml catalina 11.0.23 -> 11.0.26 tomcat-catalina is provided scope, so leaving the poms behind the image means compiling against a different API than we deploy on. The two pom pins were already two patch releases behind the image before this change; that drift is listed as an open item in the doc, and this closes it. Verified 11.0.26 is released and self-consistent: tomcat-dbcp, tomcat-jdbc and tomcat-catalina are all present as <latest> in Maven Central (published 2026-09-15), both jars this pom references return 200, and the tomcat:11.0.26-jdk21 tag exists on Docker Hub. Also re-checked the DBCP2 setters that server.xml depends on, since the no-factory fallback in 3057207 binds attributes by setter name. javap against tomcat-dbcp-11.0.26's BasicDataSource confirms setMaxTotal(int), setMaxIdle(int) and setMaxWaitMillis(long) are all still present, and setMaxActive is still absent, so maxTotal/maxIdle/maxWaitMillis keep their meaning and the rename remains required rather than cosmetic. Not done here, both of which need a running Docker daemon: - Steps 2-4, re-basing docker/tomcat-conf/{server,context}.xml and catalina.properties onto 11.0.26's stock config. These files are forks of stock, so upstream changes need carrying forward; for a patch bump this is usually a no-op but it has not been verified. - Step 6, the local docker build + run smoke test. Left alone deliberately: the root Dockerfile is still on 11.0.23, but the doc calls it unused with an open item to delete it, and chpl-service/bin/pom.xml carries its own Tomcat pins but is untracked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OCD-5395 Reverts 3642c4b. ErrorMessageUtil.java is now byte-identical to staging, so it drops out of this PR's diff entirely. The diagnostic was always marked temporary ("Remove once root-caused"), and it logged six lines on every context start in every environment. The DEV NoSuchMessageException it was added to chase is no longer reproducing. Worth being honest about why it is going, since the commit that added it set a condition this does not meet: the failure was never root-caused. It stopped on its own, and the probe logging is what would have identified the cause if it came back. Removing it is still right for a merge - it is startup noise for a problem that is not currently happening - but it is removal because the symptom went away, not because we understand it. ErrorMessageUtilResolutionTest, added in the next commit, is the durable replacement for the part of this that was actually worth keeping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OCD-5395 Every existing test that touches ErrorMessageUtil mocks MessageSource, so nothing in the suite exercises the actual bundle. That means a code present in Java but missing from errors.properties - or a bundle the configured basenames cannot load at all - is invisible until it throws at runtime, which is exactly the shape of the DEV failure that prompted the diagnostic reverted in the previous commit. Wires ReloadableResourceBundleMessageSource with the same basenames and encoding as CHPLServiceConfig.messageSource(), pins the locale to en_US (the locale the failing jobs ran under), and resolves three codes spread across the bundle: surveillance.newlineCharacterFound (the one that threw), surveillance.badCharacterFound and listing.newlineCharacterFound. Spreading them means a bundle that fails to load fails all three, distinguishing that from a single bad key. This is a build-time guard rather than a root cause. If the DEV failure was context-specific - a MessageSource other than the configured bean being injected into the running WAR - this test will not reproduce it, since it constructs the bean directly. It does cover the whole class of bundle-and-key-integrity regressions, which is the part that can be caught before deploy. Verified: mvn -pl chpl-service -am -Dtest=ErrorMessageUtilResolutionTest test passes, 3 tests, 0 failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
OCD-5395 Resolves the PR's conflict with staging. One file conflicted, chpl/chpl-service/pom.xml, in two hunks: the tomcat-dbcp and tomcat-catalina version lines. Both sides moved the same two lines off 11.0.23. Staging went to 11.0.25 in 84ad5a3 ("Update missed tomcat-catalina jar", OCD-5423); this branch went to 11.0.26 in 432e91c. Resolved in favour of 11.0.26, since docker/Dockerfile is now on tomcat:11.0.26-jdk21 and the pins exist to match the image - the provided-scope tomcat-catalina is what we compile against, so a pin behind the image means compiling against an API we do not deploy on. Staging was moving the same direction, one release behind. After resolution the pom differs from upstream/staging in those two version lines and nothing else. Staging's dependabot bumps are preserved (jackson 3.2.1, postgresql 42.7.12, hibernate-validator 9.1.3.Final, commons-validator 1.11.0). CHPLConfig.java auto-merged; verified by hand that both messageSource beans still carry the ReloadableResourceBundleMessageSource change from 6bf6cce, which ErrorMessageUtilResolutionTest mirrors. Staging touched none of this branch's other files - docker/Dockerfile, docker/tomcat-conf/*, the resources-{dev,qa,staging}/quartz.properties files, ErrorMessageUtil.java and docker-publish.yml all have zero commits on staging since the merge base - so the pom was the only real interaction. Verified: mvn -pl chpl-service -am test passes on the merge result. 3009 tests, 0 failures, 0 errors, 11 skipped, BUILD SUCCESS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kekey1
approved these changes
Sep 18, 2026
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.
No description provided.