JCU/Docker update#1345
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Updates the project’s Docker images and GitHub Actions workflows to better support Dataquest’s publishing/release flow, adds new Postgres images with pgcrypto, and hardens the main runtime container to run as a non-root user.
Changes:
- Add new Postgres Docker images to enable
pgcrypto(including acurl/loadsql flavor) and wire them into the Docker CI workflow. - Adjust GitHub Actions Docker build/release workflows (tagging, registry login/push behavior, triggers) and add a small script to emit build/version metadata into the image build context.
- Update the main backend
Dockerfileto run DSpace as a dedicated numeric-UID user (Kubernetes-friendly).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/sourceversion.py |
New helper script to print build timestamp + git hash and a build-run link (used by Docker CI). |
dspace/src/main/docker/dspace-postgres-pgcrypto/install-pgcrypto.sh |
New init script to create extensions schema and enable pgcrypto. |
dspace/src/main/docker/dspace-postgres-pgcrypto/Dockerfile |
New Postgres image definition which installs the pgcrypto init script. |
dspace/src/main/docker/dspace-postgres-pgcrypto-curl/install-pgcrypto.sh |
Extends existing loadsql init script to also enable pgcrypto. |
dspace/src/main/docker/dspace-postgres-pgcrypto-curl/Dockerfile |
New Postgres+curl image definition for loading SQL dumps + enabling pgcrypto. |
dspace/src/main/docker/dspace-postgres-loadsql/Dockerfile |
Removes the previous postgres-loadsql image Dockerfile. |
Dockerfile |
Switch runtime to a non-root dspace user with a fixed UID and ownership updates. |
.github/workflows/tag-release.yml |
New workflow to retag sha-tagged images to a git tag and push to DockerHub. |
.github/workflows/reusable-docker-build.yml |
Updates reusable Docker build workflow (tagging, registry, action versions, optional version file generation). |
.github/workflows/docker.yml |
Updates Docker pipeline to Dataquest image names and adds pgcrypto image builds; changes triggers. |
.github/workflows/build.yml |
Updates CI triggers and action versions for Java build/test workflow. |
.dockerignore |
Updates ignored docker paths to match the new pgcrypto image directories. |
Comments suppressed due to low confidence (2)
dspace/src/main/docker/dspace-postgres-pgcrypto-curl/install-pgcrypto.sh:48
- This newly added pgcrypto setup block has the same issues as the standalone pgcrypto image: it hardcodes
dspaceas the database name and doesn't pass--dbname, so it won't work correctly ifPOSTGRES_DBis overridden. Also,$POSTGRES_USERis interpolated into SQL without identifier quoting.
.github/workflows/reusable-docker-build.yml:177 - The reusable Docker workflow currently skips the actual Docker build on pull_request events (
if: ${{ ! matrix.isPr }}), so PR runs won't validate that images still build. If the intent is to avoid pushing on PRs, you can still build withpush: falserather than skipping the build entirely.
- name: Build and push image to ${{ env.DOCKER_BUILD_REGISTRY }}
if: ${{ ! matrix.isPr }}
id: docker_build
uses: docker/build-push-action@v5
with:
build-contexts: |
${{ inputs.dockerfile_additional_contexts }}
context: ${{ inputs.dockerfile_context }}
file: ${{ inputs.dockerfile_path }}
# Tell DSpace's Docker files to use the build registry instead of DockerHub
build-args:
DOCKER_REGISTRY=${{ env.DOCKER_BUILD_REGISTRY }}
platforms: ${{ matrix.arch }}
push: true
# Use tags / labels provided by 'docker/metadata-action' above
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Caution Review failedAn error occurred during the review process. Please try again later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem description
Manual Testing (if applicable)
Copilot review