feat: Add OIDC plugin - #34
RobertGemmaJr wants to merge 47 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The Docker Compose healthcheck targets the wrong port and the k8s Kustomize env file path conflicts with the documented folder structure, which can break local deployments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Adds OpenID Connect (OIDC) authentication support to the XNAT image and restructures the repository’s local deployment tooling so Docker Compose and Kubernetes setups live under dedicated docker/ and k8s/ folders with their own docs/config.
Changes:
- Installs the OIDC (openid-auth) plugin in the Docker image and introduces a single
XNAT_VERSIONbuild arg. - Replaces root-level local deployment manifests with folder-scoped Docker Compose and Kustomize configurations plus new deployment docs.
- Updates README to focus on XNAT, supported plugins, release/versioning, and links to deployment subfolders.
File summaries
| File | Description |
|---|---|
| README.md | Rewrites repo overview, plugin listing, releases/versioning, and links to local deployment docs. |
| kustomization.yaml | Removes legacy root-level Kustomize entrypoint. |
| k8s/namespace.yaml | Adds a dedicated xnat namespace manifest for local k8s deployments. |
| k8s/kustomization.yaml | Adds new k8s Kustomize entrypoint and secret generation for env/auth provider configs. |
| k8s/K8S.md | Adds Kubernetes local deployment documentation and setup steps. |
| k8s/db.yaml | Updates Postgres to v16, adds PVC, probes, and switches env wiring to Secrets. |
| k8s/config/oidc-provider.properties.example | Adds example OIDC provider properties for k8s deployment. |
| k8s/config/ldap-provider.properties.example | Adds example LDAP provider properties for k8s deployment. |
| k8s/app.yaml | Adds new k8s XNAT deployment/service with probes and auth-config secret mounts. |
| Dockerfile | Adds OIDC plugin download and consolidates XNAT version configuration via build arg. |
| docker/DOCKER.md | Adds Docker Compose local deployment documentation and setup steps. |
| docker/docker-compose.yaml | Adds new Docker Compose stack using shared env file and auth config mounts. |
| docker/config/oidc-provider.properties.example | Adds example OIDC provider properties for Docker Compose deployment. |
| docker/config/ldap-provider.properties.example | Updates LDAP provider example properties for Docker Compose deployment. |
| docker-compose.yaml | Removes legacy root-level docker-compose file. |
| app.yaml | Removes legacy root-level Kubernetes manifests. |
| .gitignore | Adds .vscode to ignore list. |
| .github/workflows/docker-build-push.yaml | Updates workflow name/quoting and adds a TODO note. |
| k8s/env/.env.example | Example env file moved under k8s env folder (content excluded from review). |
| docker/env/.env.example | Example env file moved under docker env folder (content excluded from review). |
Review details
Files excluded by content exclusion policy (2)
- docker/env/.env.example
- k8s/env/.env.example
- Files reviewed: 19/20 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Moderate documentation and PostgreSQL startup issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Review details
Files excluded by content exclusion policy (2)
- docker/.env
- k8s/.env
Suppressed comments (5)
docker/DOCKER.md:68
- The GitHub alert marker is case-sensitive;
[!Caution]will be rendered as a normal blockquote instead of a CAUTION alert, so this warning loses its intended emphasis.
>[!Caution]
k8s/K8S.md:144
- The manifests are applied into the
xnatnamespace, but this command uses kubectl's current namespace (normallydefault). A user following these instructions will typically getservices "xnat" not found; include the namespace in the port-forward command.
kubectl port-forward svc/xnat 8080:80
k8s/K8S.md:161
- The documented cleanup deletes every resource in this kustomization, including
namespace.yamland thedb-dataPVC, so it can remove the local database and any other resources placed in thexnatnamespace rather than merely stopping XNAT. Warn about this destructive behavior or provide a stop command that preserves the namespace and PVC.
kubectl delete -k .
k8s/K8S.md:61
- The deployment advertises minikube as a valid alternative, but
imagePullPolicy: Neveronly works whenxnat:localis present inside the minikube node. The documented host-sidedocker builddoes not load the image there, so the pod will remain Pending/ImagePullBackOff unless the image is explicitly loaded.
> You may prefer [minikube](https://minikube.sigs.k8s.io/docs/start/) for running Kubernetes locally instead of Docker Desktop. Both are valid options.
k8s/db.yaml:32
- PostgreSQL's liveness probe starts immediately and uses the default three failures at 10-second intervals. On a fresh PVC, initialization can exceed that window, causing kubelet to restart PostgreSQL before it becomes ready and potentially creating a restart loop. Add a startup probe (or an equivalent startup grace period) before enabling liveness checks.
- Files reviewed: 18/19 changed files
- Comments generated: 4
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved Compose connectivity, persistence, and PostgreSQL startup issues block approval.
Get a fresh assessment by requesting another Copilot review.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Review details
Files excluded by content exclusion policy (2)
- docker/.env
- k8s/.env
Suppressed comments (5)
Previously missed (2) — in code that hasn't changed since the last review.
k8s/db.yaml:32
- The database liveness check starts immediately and has Kubernetes' default failure window (three failures at 10-second intervals), while a new PVC still needs
initdband startup scripts to finish. If initialization takes longer than that on a local cluster, the kubelet will restart Postgres before it can become ready, potentially leaving the deployment in a crash loop. Add astartupProbe(or an appropriate initial delay) so liveness is deferred until initialization completes.
k8s/K8S.md:163 - This service is created in the
xnatnamespace by the Kustomization, butkubectl port-forwarddefaults to the current namespace (normallydefault). Running this documented command therefore cannot findsvc/xnatunless the user has separately changed their namespace context; include-n xnatso the documented deployment works as-is.
Dockerfile:56
- Removing the image-level
VOLUMEdeclaration makes/data/xnat/home/configpart of the container's writable layer. The entrypoint writes XNAT's generated configuration there, so consumers that recreate the container now lose those settings, and the new Compose file does not define a replacement volume; preserve this volume contract or provide an explicit persistent volume in the deployment.
# Copy built files
COPY --from=build "/root/xnat-web/build/libs/xnat-web-${XNAT_VERSION}.war" \
"${CATALINA_HOME}/webapps/ROOT.war"
docker/docker-compose.yaml:28
- The Compose
postgresservice has no explicit persistent volume, so its database storage is an anonymous image volume rather than the durable storage described for this deployment; afterdocker compose downand a subsequent recreate, local XNAT data can be lost. Add a named volume for/var/lib/postgresql/data(and keep the normaldowncommand without-vwhen data should be retained).
image: postgres:16
ports:
- "5432:5432"
env_file: .env
healthcheck:
k8s/K8S.md:28
auth-configis commented out ink8s/kustomization.yaml, and the corresponding mounts are commented out inapp.yaml, so the defaultkubectl apply -k ./k8sdoes not generate or mount this Secret. Mark this entry as optional so the overview does not imply that LDAP/OIDC is active before the provider setup steps are followed.
- *`auth-config` — from the LDAP and OIDC `.properties` files, mounted into the XNAT container*
- Files reviewed: 18/19 changed files
- Comments generated: 2
- Review effort level: Lite
| - "8000:8000" | ||
| - "8080:8080" | ||
| - "8104:8104" | ||
| env_file: .env |
| # From this directory | ||
| docker build -t xnat:local .. | ||
| # From the root directory | ||
| docker build -t xnat:local . | ||
| ``` |
Dockerfile
XNAT_VERSIONso it only needs to be set onceLocal Deployments
docker-composesetup into a nested folder (docker/)k8s/)<deployment>/config/Docker Compose
xnat-webservice waits for the postgres to be ready before startingK8s
.propertiesfiles are now mounted as a single shared secretnamespace.yamlfile for the xnat namespacestartupProbe,readinessProbe, andlivenessProbeto the deploymentsDocumentation
DOCKER.mdfor instructions specific to the Docker Compose local deploymentK8s.mdfor instructions specific to the Kubernetes local deployment