Skip to content

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

Closed
anthony-nhs wants to merge 1 commit intomainfrom
tag_from_dockerfile
Closed

Chore: [AEA-0000] - get tag from dockerfile if not in devcontainer.json#121
anthony-nhs wants to merge 1 commit intomainfrom
tag_from_dockerfile

Conversation

@anthony-nhs
Copy link
Copy Markdown
Contributor

Summary

  • Routine Change

Details

  • get tag from dockerfile if not in devcontainer.json

Copilot AI review requested due to automatic review settings March 31, 2026 07:05
@github-actions
Copy link
Copy Markdown
Contributor

This PR is linked to a ticket in an NHS Digital JIRA Project. Here's a handy link to the ticket:

AEA-0000

@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the repo-config GitHub workflow to derive the devcontainer image tag from the .devcontainer/Dockerfile when it isn’t present in .devcontainer/devcontainer.json, and aligns the devcontainer files toward using the Dockerfile as the source of truth.

Changes:

  • Remove IMAGE_NAME / IMAGE_VERSION build args from .devcontainer/devcontainer.json.
  • Pin the devcontainer base image reference directly in .devcontainer/Dockerfile.
  • Add a fallback in .github/workflows/get-repo-config.yml to parse the FROM ... reference in the Dockerfile when the JSON args are missing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
.github/workflows/get-repo-config.yml Adds Dockerfile parsing fallback for devcontainer image/version discovery.
.devcontainer/Dockerfile Pins the base image reference (name + tag) directly in FROM.
.devcontainer/devcontainer.json Removes image/version args that were previously used as the workflow’s source of truth.

Comment on lines 59 to +63
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"
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.
Comment on lines 6 to 10
"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:}"
},
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 59 to +68
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"
)"

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.
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.
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"
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.
Comment on lines 6 to 10
"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:}"
},
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.
@anthony-nhs anthony-nhs closed this Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants