Skip to content

Chore: [AEA-0000] - get tag from dockerfile if not in devcontainer.json #121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
anthony-nhs wants to merge 1 commit into from
Closed

Chore: [AEA-0000] - get tag from dockerfile if not in devcontainer.json #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
ARG IMAGE_NAME=node_24_python_3_14
ARG IMAGE_VERSION=latest
FROM ghcr.io/nhsdigital/eps-devcontainers/${IMAGE_NAME}:${IMAGE_VERSION}
FROM ghcr.io/nhsdigital/eps-devcontainers/node_24_python_3_14:v1.2.0

USER root
# specify DOCKER_GID to force container docker group id to match host
Expand Down
2 changes: 0 additions & 2 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
"context": "..",
"args": {
"DOCKER_GID": "${env:DOCKER_GID:}",
"IMAGE_NAME": "node_24_python_3_14",
"IMAGE_VERSION": "v1.2.0",
"USER_UID": "${localEnv:USER_ID:}",
"USER_GID": "${localEnv:GROUP_ID:}"
},
Comment on lines 6 to 10
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing IMAGE_NAME/IMAGE_VERSION from devcontainer.json will break workflows/scripts that read these fields (e.g. .github/workflows/update-dev-container-version.yml reads .build.args.IMAGE_NAME and .build.args.IMAGE_VERSION). Either keep these args in devcontainer.json, or update the version-bump workflow(s) to derive and update the Dockerfile FROM tag instead.

Copilot uses AI. Check for mistakes.
Comment on lines 6 to 10
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs now appear out of date: README.md ("Update Dev Container Version" section) states devcontainer.json must include build.args.IMAGE_NAME and build.args.IMAGE_VERSION, but this PR removes them. Please update the documentation (and/or the update-dev-container-version workflow) to match the new source of truth (Dockerfile).

Copilot uses AI. Check for mistakes.
Expand Down
13 changes: 12 additions & 1 deletion .github/workflows/get-repo-config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ jobs:
- name: Checkout code
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd
with:
ref: ${{ env.BRANCH_NAME }}
fetch-depth: 0

- name: Load config value
Expand All @@ -60,6 +59,18 @@ jobs:
TAG_FORMAT=$(yq '.TAG_FORMAT' .github/config/settings.yml)
DEVCONTAINER_IMAGE=$(jq -r '.build.args.IMAGE_NAME' .devcontainer/devcontainer.json)
DEVCONTAINER_VERSION=$(jq -r '.build.args.IMAGE_VERSION' .devcontainer/devcontainer.json)
if [ "$DEVCONTAINER_IMAGE" == "null" ]; then
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback only checks DEVCONTAINER_IMAGE for "null". If IMAGE_NAME is present but IMAGE_VERSION is missing/"null" (or either value is empty), later steps will build an invalid RUNTIME_DOCKER_IMAGE. Consider checking both DEVCONTAINER_IMAGE and DEVCONTAINER_VERSION (and empty-string) before deciding whether to parse the Dockerfile.

Suggested change
if [ "$DEVCONTAINER_IMAGE" == "null" ]; then
if [ -z "$DEVCONTAINER_IMAGE" ] || [ "$DEVCONTAINER_IMAGE" = "null" ] || [ -z "$DEVCONTAINER_VERSION" ] || [ "$DEVCONTAINER_VERSION" = "null" ]; then

Copilot uses AI. Check for mistakes.
dockerfile=".devcontainer/Dockerfile"
Comment on lines 59 to +63
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This workflow can now source devcontainer_image/devcontainer_version from the Dockerfile rather than devcontainer.json; please update the workflow output descriptions/comments accordingly to avoid misleading callers.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback hard-codes .devcontainer/Dockerfile, but devcontainer.json already declares the dockerfile path (.build.dockerfile). For a reusable workflow this can break repos that use a different filename; consider reading .build.dockerfile from devcontainer.json and constructing the path from that value.

Suggested change
dockerfile=".devcontainer/Dockerfile"
dockerfile_name=$(jq -r '.build.dockerfile // "Dockerfile"' .devcontainer/devcontainer.json)
dockerfile=".devcontainer/${dockerfile_name}"

Copilot uses AI. Check for mistakes.

from_ref="$(
awk '/^FROM[[:space:]]+/ { print $2; exit }' "$dockerfile"
)"

Comment on lines 59 to +68
Copy link

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add set -euo pipefail at the start of this step (and fail if the Dockerfile/ FROM line can't be read) so jq/awk failures don't silently produce empty/incorrect outputs.

Suggested change
TAG_FORMAT=$(yq '.TAG_FORMAT' .github/config/settings.yml)
DEVCONTAINER_IMAGE=$(jq -r '.build.args.IMAGE_NAME' .devcontainer/devcontainer.json)
DEVCONTAINER_VERSION=$(jq -r '.build.args.IMAGE_VERSION' .devcontainer/devcontainer.json)
if [ "$DEVCONTAINER_IMAGE" == "null" ]; then
dockerfile=".devcontainer/Dockerfile"
from_ref="$(
awk '/^FROM[[:space:]]+/ { print $2; exit }' "$dockerfile"
)"
set -euo pipefail
TAG_FORMAT=$(yq '.TAG_FORMAT' .github/config/settings.yml)
DEVCONTAINER_IMAGE=$(jq -r '.build.args.IMAGE_NAME' .devcontainer/devcontainer.json)
DEVCONTAINER_VERSION=$(jq -r '.build.args.IMAGE_VERSION' .devcontainer/devcontainer.json)
if [ "$DEVCONTAINER_IMAGE" = "null" ]; then
dockerfile=".devcontainer/Dockerfile"
if [ ! -f "$dockerfile" ]; then
echo "Error: Dockerfile '$dockerfile' not found; cannot derive DEVCONTAINER_IMAGE/DEVCONTAINER_VERSION." >&2
exit 1
fi
from_ref="$(
awk '/^FROM[[:space:]]+/ { print $2; exit }' "$dockerfile"
)"
if [ -z "$from_ref" ]; then
echo "Error: No FROM line found in '$dockerfile'; cannot derive DEVCONTAINER_IMAGE/DEVCONTAINER_VERSION." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.
DEVCONTAINER_IMAGE="${from_ref##*/}"
DEVCONTAINER_IMAGE="${DEVCONTAINER_IMAGE%%:*}"

DEVCONTAINER_VERSION="${from_ref##*:}"
fi
RUNTIME_DOCKER_IMAGE="${DEVCONTAINER_IMAGE}:githubactions-${DEVCONTAINER_VERSION}"
{
echo "TAG_FORMAT=$TAG_FORMAT"
Expand Down
Loading