Chore: [AEA-0000] - changes to get image from dockerfile#77
Chore: [AEA-0000] - changes to get image from dockerfile#77anthony-nhs merged 2 commits intomainfrom
Conversation
|
This PR is linked to a ticket in an NHS Digital JIRA Project. Here's a handy link to the ticket: AEA-0000 |
There was a problem hiding this comment.
Pull request overview
Updates repo-status tooling and CI configuration to derive the devcontainer image name/version from the .devcontainer/Dockerfile when it’s not present in .devcontainer/devcontainer.json, aligning local devcontainer config and automation around a single source of truth.
Changes:
- Added Dockerfile fallback parsing in
get_devcontainer_detailsand expanded unit tests to cover fallback behavior. - Simplified devcontainer configuration by removing
IMAGE_NAME/IMAGE_VERSIONargs fromdevcontainer.jsonand pinning the base image directly in.devcontainer/Dockerfile. - Added Dependabot updates for the devcontainer Dockerfile and updated reusable workflow refs for config extraction.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/get_repo_status/app/github_client.py |
Adds Dockerfile fallback logic when devcontainer build args don’t provide image details. |
packages/get_repo_status/tests/test_github_client.py |
Adds/updates tests validating Dockerfile fallback behavior and call sequencing. |
.github/workflows/release.yml |
Updates reusable workflow reference for config extraction. |
.github/workflows/pull_request.yml |
Updates reusable workflow reference for config extraction in PR workflow. |
.github/dependabot.yml |
Adds a Docker ecosystem entry to keep .devcontainer base image up to date. |
.devcontainer/Dockerfile |
Pins the base image directly rather than via build args. |
.devcontainer/devcontainer.json |
Removes IMAGE_* build args so image metadata is derived elsewhere. |
Comments suppressed due to low confidence (1)
.devcontainer/Dockerfile:5
- The Dockerfile uses
${DOCKER_GID}during build, anddevcontainer.jsonpassesDOCKER_GIDas a build arg, but the Dockerfile does not declareARG DOCKER_GID. Without anARG, the build arg won’t be available inRUN, so this block will never apply even when the arg is provided. DeclareARG DOCKER_GID(and any other expected build args) after theFROMline.
FROM ghcr.io/nhsdigital/eps-devcontainers/node_24_python_3_14:v1.1.7
USER root
# specify DOCKER_GID to force container docker group id to match host
RUN if [ -n "${DOCKER_GID}" ]; then \
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if line.startswith("FROM"): | ||
| parts = line.split(":") | ||
| if len(parts) == 2: | ||
| image_name = parts[0].replace("FROM ", "").strip() | ||
| image_name = image_name.replace("ghcr.io/nhsdigital/eps-devcontainers/", "").strip() | ||
| image_version = parts[1].strip() |
There was a problem hiding this comment.
Dockerfile parsing here is too brittle: line.split(":") will mis-parse common valid FROM forms (e.g. FROM --platform=linux/amd64 image:tag, FROM image:tag AS builder, images with a registry port like localhost:5000/image:tag, or digests). This can lead to incorrect IMAGE_NAME/IMAGE_VERSION outputs. Consider extracting the image reference token after FROM (handling optional --platform), then splitting tag on the last : (and handling @sha256), and stop after the first matching FROM (or make stage-selection explicit).
| if line.startswith("FROM"): | |
| parts = line.split(":") | |
| if len(parts) == 2: | |
| image_name = parts[0].replace("FROM ", "").strip() | |
| image_name = image_name.replace("ghcr.io/nhsdigital/eps-devcontainers/", "").strip() | |
| image_version = parts[1].strip() | |
| stripped = line.strip() | |
| # Ignore empty lines and comments | |
| if not stripped or stripped.startswith("#"): | |
| continue | |
| if not stripped.startswith("FROM "): | |
| continue | |
| tokens = stripped.split() | |
| if not tokens or tokens[0] != "FROM": | |
| continue | |
| # Handle optional platform specifier: FROM --platform=... image[:tag][@digest] [AS stage] | |
| image_idx = 1 | |
| if len(tokens) > 1 and tokens[1].startswith("--platform="): | |
| image_idx = 2 | |
| if len(tokens) <= image_idx: | |
| continue | |
| image_ref = tokens[image_idx] | |
| # Strip any digest (e.g. @sha256:...) | |
| image_ref_no_digest = image_ref.split("@", 1)[0] | |
| # Split on the last ":" to separate name and tag, if present | |
| name_part, sep, tag_part = image_ref_no_digest.rpartition(":") | |
| if sep: | |
| image_name = name_part.replace( | |
| "ghcr.io/nhsdigital/eps-devcontainers/", "" | |
| ).strip() | |
| image_version = tag_part.strip() | |
| # Use the first successfully parsed FROM line | |
| break |
| jobs: | ||
| get_config_values: | ||
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@5ac2707dd9cd60ad127275179495b9c890d74711 | ||
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@tag_from_dockerfile |
There was a problem hiding this comment.
This workflow reference was previously pinned to a commit SHA but is now using @tag_from_dockerfile. If this is a branch name or mutable tag, it weakens supply-chain guarantees and makes builds non-reproducible. Prefer pinning to an immutable tag or commit SHA (and update intentionally when you need new behavior).
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@tag_from_dockerfile | |
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@5ac2707dd9cd60ad127275179495b9c890d74711 |
|
|
||
| get_config_values: | ||
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@5ac2707dd9cd60ad127275179495b9c890d74711 | ||
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@tag_from_dockerfile |
There was a problem hiding this comment.
This workflow reference was previously pinned to a commit SHA but is now using @tag_from_dockerfile. If this is a branch name or mutable tag, it weakens supply-chain guarantees and makes builds non-reproducible. Prefer pinning to an immutable tag or commit SHA (and update intentionally when you need new behavior).
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@tag_from_dockerfile | |
| uses: NHSDigital/eps-common-workflows/.github/workflows/get-repo-config.yml@5ac2707dd9cd60ad127275179495b9c890d74711 |
.github/dependabot.yml
Outdated
|
|
||
| updates: | ||
| - package-ecosystem: "docker" | ||
| directory: ".devcontainer" |
There was a problem hiding this comment.
Dependabot directory values are consistently specified with a leading / elsewhere in this file (e.g. directory: "/"). Using directory: ".devcontainer" is likely to be treated as an invalid path and can prevent updates from running. Consider changing to directory: "/.devcontainer" for consistency and to match Dependabot’s expected format.
| directory: ".devcontainer" | |
| directory: "/.devcontainer" |
This reverts commit 92a41a6.
Summary
Details