Repository navigation
build: upgrade from JDK 8 to JDK 17 (#802) - #847
Merged
Merged
Conversation
Target JDK 17 (LTS) across the build, runtime, CI, and docs. JDK 8 public updates ended in 2019 and Tomcat 8.5 is EOL; JDK 17 brings security patches and modern APIs while staying on javax.* / Tomcat 9 (no jakarta namespace migration). - pom.xml: maven-compiler-plugin source/target 1.8 -> release 17 - .env: TOMCAT_DOCKER_VERSION 8-jre8-openjdk -> 9.0-jre17-temurin - Dockerfile: libargon2-0 -> libargon2-1 (newer Ubuntu base) - CI (test.yml, release.yml): java-version 8 -> 17; release job aligned to temurin - README.md, provisioning/install.sh: openjdk-8-jdk -> openjdk-17-jdk - shepherdTotalInstall.sh: oracle-java8-installer -> openjdk-17-jdk, drop dead webupd8team/java PPA, JAVA_HOME -> java-17-openjdk-amd64 Verified: mvn clean test passes under Temurin 17 (153 tests, 0 failures). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Tomcat 9 base image (9.0-jre17-temurin) ships a server.xml that adds maxParameterCount="1000" to the connectors, shifting the context lines the old patch (written against Tomcat 8) relied on. Hunk #1 failed to apply, breaking the Docker Compose Build CI step. Regenerated the patch against tomcat:9.0-jre17-temurin's server.xml, preserving the same intent: comment out the plain 8080 connector, enable the thread-pool connector with redirectPort=HTTPS_PORT, and append the custom 8443 TLS connector. Verified it applies cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
5 tasks
Member
Author
|
The The patch-based config approach is fragile by design — filed #848 to replace it with something more robust as a separate piece of work. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR upgrades the project build, runtime, CI, and installation references from Java 8/Tomcat 8 toward JDK 17 with Tomcat 9, while keeping the existing javax.* servlet stack.
Changes:
- Updates Maven compilation to
release 17and CI/release jobs to JDK 17. - Updates Docker/Tomcat runtime image and native Argon2 package.
- Updates installation/docs scripts to install OpenJDK 17 and adjusts Tomcat patching for the newer base image.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
pom.xml |
Sets Maven compiler release to Java 17. |
.env |
Updates Tomcat Docker image tag to JRE 17/Tomcat 9. |
Dockerfile |
Updates native Argon2 package for the newer base image. |
docker/tomcat/serverxml.patch |
Refreshes Tomcat server XML patch for Tomcat 9 layout. |
.github/workflows/test.yml |
Runs build, unit, and integration jobs on JDK 17. |
.github/workflows/release.yml |
Builds releases using Temurin JDK 17. |
README.md |
Updates documented prerequisite JDK package. |
provisioning/install.sh |
Updates Vagrant provisioning to install OpenJDK 17. |
src/setupFiles/shepherdTotalInstall.sh |
Updates manual install script Java package and JAVA_HOME. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sudo apt-get update -y | ||
| sudo apt-get install -y maven | ||
| sudo apt-get install -y openjdk-8-jdk | ||
| sudo apt-get install -y openjdk-17-jdk |
The provisioning script (provisioning/install.sh) now installs openjdk-17-jdk, but the Vagrant box was ubuntu/xenial64 (16.04), whose default apt repos top out at JDK 9 — so `vagrant up` would fail at the install step. Move to ubuntu/jammy64 (22.04 LTS), which ships openjdk-17-jdk in its default repositories. The Docker repo setup already adapts via `lsb_release -cs`, so no other changes needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ismisepaul
added a commit
that referenced
this pull request
Oct 23, 2026
…(PR 8 of jakarta migration #861) (#942) * docs: document Java 17 / Jakarta EE 11 / Tomcat 11 and pac4j SAML (PR 8 of jakarta migration #861) - AGENTS.md said the project targets Java 8; it has targeted Java 17 since the JDK 17 upgrade (#847). Documents the Jakarta EE 11 / Tomcat 11 platform, which javax packages remain fine, the jakarta.tags.* taglibs, Spring being test-only, and the pac4j-based SAML SSO and its configuration files. - docs/testing.md: JDK 8 -> JDK 17 prerequisite. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: upgrade note for the Tomcat conf volume, Mongo IT image workaround - README / AGENTS: an existing Docker environment keeps its old Tomcat 9/10.1 conf/ in the named conf volume, so Tomcat 11 would start with stale server.xml / web.xml. Remove the volume when upgrading; data volumes are untouched. - AGENTS: the it-test profile tags its Mongo build as mongo:${MONGODB_VERSION}, overwriting the official image locally so the next IT run fails on chown. Documents the re-pull workaround. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: back up the Tomcat conf volume before recreating it Addresses review on #942: the conf volume also holds the TLS keystore and database.properties / mongo.properties, not just Tomcat's XML. The upgrade steps now back the volume up first and explain restoring only a customised keystore or connection settings, never the old server.xml / web.xml. Backup and selective restore commands were checked against a scratch volume. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: re-apply custom Tomcat XML edits rather than restoring old files Addresses review on #942: covers operators who hand-edited server.xml / web.xml, alongside the existing backup and selective keystore / properties restore. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: create the conf backup archive mode 600 Addresses review on #942: the archive holds the TLS private key and the database password, but under the default 022 umask it was created 0644. The backup now runs with umask 077 and chmods the archive to 600 before writing (which also restricts an existing archive being overwritten). The docs say to delete it after the upgrade, and that it is root-owned on Linux. Checked against a scratch volume, overwriting an existing 0644 file. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: Spring is a runtime dependency for SAML SSO, not test-only Addresses review on #942: AGENTS.md said Spring 7 was used only in tests, but pac4j-saml needs it at runtime and SamlSso uses spring-core's ByteArrayResource. Separates the runtime and test uses, and warns against test-scoping spring-core. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: start the upgraded stack detached before restoring conf files Addresses review on #942: an attached docker-compose up has to be interrupted to run the restore commands, which stops the databases too, and the later restart of web doesn't bring them back. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: keep the conf backup outside the Docker build context Addresses review on #942: the backup was written to the repository root, which is the Tomcat image's build context. On Linux the archive is root-owned and mode 600, which can break the next build, and the secrets were sent into the build context. The backup now goes to a mode-700 ~/shepherd-conf-backup directory outside the repository, which the user can delete even though the archive is root-owned. Backup and restore commands checked end to end on a scratch volume. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: document the OpenSAML repository controls and PGP verification Explains why OpenSAML comes from Shibboleth's repository, the repository filter, the pinned-key PGP verification, the no-wildcard rule for .mvn/pgp-keys.map, how to add a dependency, and the CI test. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: Maven 3.9.2+ requirement for the Shibboleth repository filter Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: note the hamcrest-core 1.1 noSig exception in AGENTS.md Addresses review on #942: the signature-checking guidance said every dependency is verified, but .mvn/pgp-keys.map allows the unsigned org.hamcrest:hamcrest-core:1.1 (via json-simple, #944) by exact version. Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: AGENTS.md notes SSO gating, SP-initiated-only login and blank-ID rejection Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: binding cookie is scoped to the SAML directory Refs #861 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: the bearer confirmation must name the same SAML request Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: how to run the full CI pipeline locally before pushing Running only mvn test let compile, Docker and integration-test failures reach CI. Document every test.yml step and the things CI does that a local run has to repeat: the challenge DB user host rewrite, restoring the official Mongo image, clearing leftover Mongo containers, and running in a throwaway worktree. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: each SAML login has its own binding cookie Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: fix only what a change introduces, file issues for the rest Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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 #802.
Summary
Targets JDK 17 (LTS) across the build, runtime, CI, and docs. JDK 8 public updates ended in 2019 and Tomcat 8.5 is EOL; JDK 17 brings security patches and modern APIs while staying on
javax.*/ Tomcat 9 — nojakartanamespace migration (that would require Tomcat 10+ / JDK 21).Changes
pom.xmlmaven-compiler-plugin<source>1.8</source>/<target>1.8</target>→<release>17</release>.envTOMCAT_DOCKER_VERSION8-jre8-openjdk→9.0-jre17-temurinDockerfilelibargon2-0→libargon2-1(newer Ubuntu base in the temurin image)docker/tomcat/serverxml.patchserver.xml(it addedmaxParameterCount="1000"to the connectors, shifting context lines and breaking the old patch). Same intent preserved; see #848 for replacing the brittle patch mechanism..github/workflows/test.ymljava-version: '8'→'17'for build / unit-tests / integration-tests (lint-java's'24'unchanged).github/workflows/release.ymljava-version: '8'→'17'; distributionzulu→temurin; step label fixedREADME.md,provisioning/install.shopenjdk-8-jdk→openjdk-17-jdkVagrantfileubuntu/xenial64→ubuntu/jammy64(xenial's repos max out at JDK 9, sovagrant upwould fail installingopenjdk-17-jdk)src/setupFiles/shepherdTotalInstall.shoracle-java8-installer→openjdk-17-jdk; drop deadwebupd8team/javaPPA;JAVA_HOME→java-17-openjdk-amd64What this does NOT change
javax.*→jakarta.*migrationVerification
mvn clean testunder Temurin 17 → BUILD SUCCESS, 153 tests, 0 failures<release>17</release>serverxml.patchverified to apply cleanly againsttomcat:9.0-jre17-temurin'sserver.xml;webxml.patchconfirmed still applies*IT) require DB containers — exercised by the CIintegration-testsjob on Java 17Notes
dependency-check.yml, but no such workflow exists in this repo (onlytest.ymlandrelease.yml).🤖 Generated with Claude Code