Skip to content

[GPCAPIM-289] - Add dependacies to dev container image #117

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

Merged
neil-sproston merged 8 commits into from
Mar 20, 2026
+53 −24
Merged

[GPCAPIM-289] - Add dependacies to dev container image #117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b066ec2
[GPCAPIM-258]: Add environment variables for SDS access module
DWolfsNHS Mar 19, 2026
05bebcb
correct formatting and move to more relevant part of the list
DWolfsNHS Mar 19, 2026
214ace4
formatting again
DWolfsNHS Mar 19, 2026
0abf492
[GPCAPIM-289]: Enhance Docker build process for dev certificates
DWolfsNHS Mar 19, 2026
21c9b90
Merge branch 'main' into bugfix/GPCAPIM-289
DWolfsNHS Mar 19, 2026
ec9779d
[GPCAPIM-289]: Update Docker build process to include dev certificates
DWolfsNHS Mar 20, 2026
4e54208
[GPCAPIM-289]: Update comments for clarity on dev certificate install…
DWolfsNHS Mar 20, 2026
5fdef33
[GPCAPIM-289]: Disable Trivy security scans
DWolfsNHS Mar 20, 2026
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
43 changes: 22 additions & 21 deletions .github/workflows/preview-env.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,24 +436,25 @@ jobs:
body: lines.join('\n'),
});

# ---------- Security scanning ----------
- name: Trivy IaC scan
if: github.event.action != 'closed'
uses: nhs-england-tools/trivy-action/iac-scan@289984b2f03034233a347d6dbadecd5ca9ea9634
with:
scan-ref: infrastructure/environments/preview
artifact-name: trivy-iac-scan-${{ steps.meta.outputs.branch_name }}

- name: Trivy image scan
if: github.event.action != 'closed'
uses: nhs-england-tools/trivy-action/image-scan@289984b2f03034233a347d6dbadecd5ca9ea9634
with:
image-ref: ${{steps.meta.outputs.ecr_url}}:${{steps.meta.outputs.branch_name}}
artifact-name: trivy-image-scan-${{ steps.meta.outputs.branch_name }}

- name: Generate SBOM
if: github.event.action != 'closed'
uses: nhs-england-tools/trivy-action/image-scan@289984b2f03034233a347d6dbadecd5ca9ea9634
with:
image-ref: ${{steps.meta.outputs.ecr_url}}:${{steps.meta.outputs.branch_name}}
artifact-name: trivy-sbom-${{ steps.meta.outputs.branch_name }}
# desable trivy in light of attack https://socket.dev/blog/trivy-under-attack-again-github-actions-compromise
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Spelling: "desable" should be "disable" in this workflow comment to keep the rationale clear/searchable (especially since this is documenting a security-related change).

Suggested change
# desable trivy in light of attack https://socket.dev/blog/trivy-under-attack-again-github-actions-compromise
# disable trivy in light of attack https://socket.dev/blog/trivy-under-attack-again-github-actions-compromise

Copilot uses AI. Check for mistakes.
# # ---------- Security scanning ----------
# - name: Trivy IaC scan
# if: github.event.action != 'closed'
# uses: nhs-england-tools/trivy-action/iac-scan@289984b2f03034233a347d6dbadecd5ca9ea9634
# with:
# scan-ref: infrastructure/environments/preview
# artifact-name: trivy-iac-scan-${{ steps.meta.outputs.branch_name }}

# - name: Trivy image scan
# if: github.event.action != 'closed'
# uses: nhs-england-tools/trivy-action/image-scan@289984b2f03034233a347d6dbadecd5ca9ea9634
# with:
# image-ref: ${{steps.meta.outputs.ecr_url}}:${{steps.meta.outputs.branch_name}}
# artifact-name: trivy-image-scan-${{ steps.meta.outputs.branch_name }}

# - name: Generate SBOM
# if: github.event.action != 'closed'
# uses: nhs-england-tools/trivy-action/image-scan@289984b2f03034233a347d6dbadecd5ca9ea9634
# with:
# image-ref: ${{steps.meta.outputs.ecr_url}}:${{steps.meta.outputs.branch_name}}
# artifact-name: trivy-sbom-${{ steps.meta.outputs.branch_name }}
Comment on lines +439 to +460
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

This PR is described as adding dev-container dependencies, but this change disables all Trivy scanning/SBOM generation for preview environments. That’s a significant security/operational change; consider moving it to a dedicated PR (or gate it behind a temporary flag) and add a clear tracking link (issue/incident) plus an explicit plan/condition for re-enabling scanning.

Copilot uses AI. Check for mistakes.
15 changes: 14 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ endif
IMAGE_NAME := ${IMAGE_REPOSITORY}:${IMAGE_TAG}
COMMIT_VERSION := $(shell git rev-parse --short HEAD)
BUILD_DATE := $(shell date -u +"%Y%m%d")
INCLUDE_DEV_CERTS ?= ${DEV_CERTS_INCLUDED}
# ==============================================================================

# Example CI/CD targets are: dependencies, build, publish, deploy, clean, etc.
Expand All @@ -41,13 +42,25 @@ build-gateway-api: dependencies
@rm -rf ../infrastructure/images/gateway-api/resources/build/
@mkdir ../infrastructure/images/gateway-api/resources/build/
@cp -r ./target/gateway-api ../infrastructure/images/gateway-api/resources/build/
# If dev certificates are present inside the dev container, copy them into
# the gateway-api image build context so they can be installed there too.
@if [ -d "/resources/dev-certificates" ]; then \
rm -rf ../infrastructure/images/gateway-api/resources/dev-certificates; \
mkdir -p ../infrastructure/images/gateway-api/resources/dev-certificates; \
cp -r /resources/dev-certificates/* ../infrastructure/images/gateway-api/resources/dev-certificates/; \
fi
# Remove temporary build artefacts once build has completed
@rm -rf target && rm -rf dist

.PHONY: build
build: build-gateway-api # Build the project artefact @Pipeline
@echo "Building Docker x86 image using Docker. Utilising python version: ${PYTHON_VERSION} ..."
@$(docker) buildx build --platform linux/amd64 --load --provenance=false --build-arg PYTHON_VERSION=${PYTHON_VERSION} --build-arg COMMIT_VERSION=${COMMIT_VERSION} --build-arg BUILD_DATE=${BUILD_DATE} -t ${IMAGE_NAME} infrastructure/images/gateway-api
@if [[ -n "$${IN_BUILD_CONTAINER}" ]]; then \
echo "building with dev certs ..." ; \
$(docker) buildx build --platform linux/amd64 --load --provenance=false --build-arg PYTHON_VERSION=${PYTHON_VERSION} --build-arg COMMIT_VERSION=${COMMIT_VERSION} --build-arg BUILD_DATE=${BUILD_DATE} --build-arg INCLUDE_DEV_CERTS=${INCLUDE_DEV_CERTS} -t ${IMAGE_NAME} infrastructure/images/gateway-api
else \
$(docker) buildx build --platform linux/amd64 --load --provenance=false --build-arg PYTHON_VERSION=${PYTHON_VERSION} --build-arg COMMIT_VERSION=${COMMIT_VERSION} --build-arg BUILD_DATE=${BUILD_DATE} -t ${IMAGE_NAME} infrastructure/images/gateway-api
fi
Comment on lines +58 to +63
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The build target checks IN_BUILD_CONTAINER with [[ -n ... ]], which treats any non-empty value (including "false") as true. This can route builds down the build-container branch unexpectedly and diverges from the earlier ifeq (${IN_BUILD_CONTAINER}, true) logic. Use an explicit equality check against "true" (or reuse the make-level condition) so the behavior is consistent and predictable.

Copilot uses AI. Check for mistakes.
@echo "Docker image '${IMAGE_NAME}' built successfully!"

publish: # Publish the project artefact @Pipeline
Expand Down
2 changes: 2 additions & 0 deletions infrastructure/images/build-container/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ RUN apk update && \
# Required to support the building of other docker images.
docker-cli \
docker-cli-buildx \
libxml2-dev \
libxslt-dev \
# pyenv suggested requirements taken from https://github.com/pyenv/pyenv/wiki#suggested-build-environment
build-base \
libffi-dev \
Expand Down
16 changes: 14 additions & 2 deletions infrastructure/images/gateway-api/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,25 @@
ARG PYTHON_VERSION=invalid
FROM python:${PYTHON_VERSION}-alpine3.23 AS gateway-api

# Controls whether dev certificates (if present) are installed into this image.
ARG INCLUDE_DEV_CERTS=false

COPY resources/ /resources

# Install required certificates for dev machines.
RUN if [ "$INCLUDE_DEV_CERTS" = "true" ] && [ -d /resources/dev-certificates ]; then \

Check warning on line 11 in infrastructure/images/gateway-api/Dockerfile

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Merge this RUN instruction with the consecutive ones. 


See more on https://sonarcloud.io/project/issues?id=NHSDigital_clinical-data-gateway-api&issues=AZ0K-A423n2bzDKT4-Qe&open=AZ0K-A423n2bzDKT4-Qe&pullRequest=117
cp -r /resources/dev-certificates/* /usr/local/share/ca-certificates/; \
update-ca-certificates; \
cp -r /resources/dev-certificates/* /etc/ssl/certs/; \
else \
rm -rf /resources/dev-certificates || true; \
fi

RUN apk upgrade --no-cache && \
pip install --no-cache-dir --upgrade pip && \
addgroup -S nonroot && \
adduser -S gateway_api_user -G nonroot
Comment on lines +8 to 22
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Moving COPY resources/ /resources before the apk upgrade layer will invalidate the package-install/user-creation layer cache whenever application build artifacts change (which is likely on most builds), making rebuilds slower. Consider copying only the cert directory first (for the conditional cert install), keep OS setup (apk/pip/adduser) in earlier stable layers, and copy the full /resources/build/... later.

Copilot uses AI. Check for mistakes.

COPY resources/ /resources

WORKDIR /resources/build/gateway-api

ENV PYTHONPATH=/resources/build/gateway-api
Expand Down
1 change: 1 addition & 0 deletions infrastructure/images/gateway-api/resources/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
/build
/dev-certificates
Loading