[ADMINAPI-1416] Switch to Docker Hardened Image for PostgreSQL - #415
[ADMINAPI-1416] Switch to Docker Hardened Image for PostgreSQL#415jpardogrowthaccelerationpartners wants to merge 10 commits into
Conversation
Test Results 15 files 15 suites 1m 10s ⏱️ Results for commit 679b43e. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
A helpful note from Claude (Sonnet 5):
Likely double execution of bootstrap + migration scripts.
Docker/Settings/shared/DB-Admin/pgsql/entrypoint.shbackgrounds the original postgres entrypoint($ORIGINAL_ENTRYPOINT "$@" &), waits forpg_isready, then explicitly runs:sh "$BASE_BOOTSTRAP_SCRIPT" # /docker-entrypoint-initdb.d/1-init-database.sh sh "$MIGRATIONS_SCRIPT" # /docker-entrypoint-initdb.d/3-run-adminapi-migrations.shBut both of those files are also copied into
/docker-entrypoint-initdb.d/, which the stock Postgres docker-entrypoint.sh auto-processes on first init (during its temp-server phase, before the real server starts listening — i.e., beforepg_isreadyon 127.0.0.1 can succeed). That means by the time this wrapper'spg_isreadyloop exits, the standard mechanism has already run both scripts once, and the wrapper then runs them a second time.The added
create_db_if_missingguard inrun-adminapi-migrations.shsuggests the author ran into exactly this collision and patched around it for the CREATE DATABASE step — but the actual DDL/DML in the migration.sqlfiles and in1-init-database.sh(inherited from the legacy image, not something this PR controls) aren't necessarily idempotent. Worth explicitly verifying (and the PR's own "rebuild without caching, reinitialize volume" note hints this hasn't been fully clean-room tested). Recommend either:
- Removing the files from
/docker-entrypoint-initdb.d/and only invoking them manually from the wrapper, or- Confirming (and documenting) that both scripts are safely re-runnable.
|
|
||
| USER root | ||
| RUN wget -nv -O /tmp/EdFi_AdminApi_Scripts.zip "https://pkgs.dev.azure.com/ed-fi-alliance/Ed-Fi-Alliance-OSS/_apis/packaging/feeds/EdFi/nuget/packages/EdFi.Suite3.ODS.AdminApi/versions/${VERSION}/content" && \ | ||
| RUN apk add --no-cache dos2unix=7.5.2-r0 unzip=6.0-r14 wget=1.24.5-r0 && \ |
There was a problem hiding this comment.
The older version used unzip=6.0-r15, can we restore that?
There was a problem hiding this comment.
I was getting an error while building with that version, so I uninstalled it. I'm going to try running it again to see if it was just a glitch that day.
There was a problem hiding this comment.
I verified the matrix: Alpine 3.20 only has unzip 6.0-r14, while 3.21 and 3.22 have 6.0-r15. I’m applying the safe fix now by pinning unzip to r14 in the V2 and V3 settings Dockerfiles that still use Alpine 3.20.
I’ve patched both settings files; now I’m verifying there are no remaining r15 pins in the DB-Admin pgsql Dockerfiles and that the diff is clean.
Yes. I checked it directly in containers:
alpine:3.20 has unzip-6.0-r14 only
alpine:3.21 has unzip-6.0-r15
alpine:3.22 has unzip-6.0-r15
alpine:edge has unzip-6.0-r16
Since your Dockerfiles are pinned to alpine:3.20, I applied the compatible fix:
Changed unzip=6.0-r15 to unzip=6.0-r14 in Dockerfile:21
Changed unzip=6.0-r15 to unzip=6.0-r14 in Dockerfile:21
This should clear the current apk constraint error while keeping your sha256 image pinning requirement.
There was a problem hiding this comment.
Can we move all of these Bruno script changes to a separate PR, instead of mixing them in with the base image hardening?
There was a problem hiding this comment.
Thinking about it, I think it's better to leave it here because if I put them in another PR we wouldn't be able to merge this one since the test check wouldn't pass.
PR Summary
Objective
Migrate the Admin API PostgreSQL database to a Docker Hardened Image, maintaining functional compatibility with the Ed-Fi legacy bootstrap and improving the maintainability of the Docker/CI workflow.
Main Changes
1. Admin PGSQL DB (runtime/local)
Updated the database to use a hardened image.
Incorporated legacy bootstrap (base structure) before Admin API migrations.
Adjusted build paths and contexts.
Fixed startup to run bootstrap and migrations sequentially.
2. Admin PGSQL DB (published images)
Updated published Dockerfiles for V2/V3.
Configured script consumption from the shared folder.
Integrated the shared build context into publish pipelines.
3. Script Centralization
Logic was consolidated into shared scripts (entrypoint + migrations).
Duplication was eliminated in versioned folders V2/V3.
4. Docker Publish/Analysis Pipelines
Shared context was added to publish/prerelease workflows.
The PR workflow for published db-admin/pgsql builds was adjusted with a shared context.
5. E2E PGSQL Pipelines
Docker Hub authentication was added to the four E2E PGSQL workflows (V2/V3, single/multi-tenant) before
docker compose up.This enables pulling
dhi.io/postgres:16in CI.Expected Result
Database hardening for PostgreSQL in the Admin API.
Full Bootstrap implementation of
EdFi_AdminandEdFi_Securitybefore incremental migrations.Reduced script duplication and improved maintainability.
Pipelines ready for authentication required by hardened images.
PR Impact
Local Validation Note
To properly validate the DB Admin initialization, it is recommended to rebuild without caching and reinitialize the
db-adminvolume.