Conversation
|
SonarCloud complaints doesn't have a solution at the moment because the available docker hardening images don't provide rootless image unless we go for the enterprise version |
|
We may need an account for dhi.io if we don't have one yet |
There was a problem hiding this comment.
I'll go through and do a review of the individual files but a couple of notable things so far that I've found that I'll wait on before I carry on:
Build errors
I can't get the build to work 🙁 If I run docker build -f Dockerfile -t dd-api . it builds but spits out a warning:
1 warning found (use docker --debug to expand):
- InvalidBaseImagePlatform: Base image gcr.io/distroless/base-debian12:nonroot-arm64 was pulled with platform "linux/arm64", expected "linux/amd64" for current build (line 33)
You get a warning when attempting to run the image too.
I assume these are because the base debian image is dependant on the platform it's built on whereas the prod stage is specifically pinned to arm64 (the image tag has it in it) so without specifying the build platform you can get mismatches (e.g. on my machine which is amd64 i get this warning presumably because the debian image is amd64 whereas the prod stage is based on an arm64 image).
If I run docker build --platform linux/arm64 -f Dockerfile -t dd-api . it doesn't:
=> ERROR [build 6/7] RUN bash /usr/local/bin/build_distroless_runtime.sh 0.1s
------
> [build 6/7] RUN bash /usr/local/bin/build_distroless_runtime.sh:
0.086 exec /bin/sh: exec format error
------
Dockerfile:25
--------------------
23 | # Dockerfile stays readable and the build logic can be tested/iterated on.
24 | COPY docker/build_distroless_runtime.sh /usr/local/bin/build_distroless_runtime.sh
25 | >>> RUN bash /usr/local/bin/build_distroless_runtime.sh
26 |
27 | # Mounts the application code into the image
--------------------
ERROR: failed to build: failed to solve: process "/bin/sh -c bash /usr/local/bin/build_distroless_runtime.sh" did not complete successfully: exit code: 255
distroless & the shell
The point of a distroless image is that it only has your application runtime in it, from the docs for the image you're using:
"Distroless" images contain only your application and its runtime dependencies. They do not contain package managers, shells or any other programs you would expect to find in a standard Linux distribution.
To this end, I don't think we should be copying bash into the distroless image as this defeats the point. The problem with this is that we use bash* to run random tasks through a utility worker which invokes the uhd script directly, e.g. this bootstrap task.
So I think we have a choice - we either accept that a distroless image we've copied bash* into is better than a base python image with apt etc in it, or we don't bother doing this until we can remove the dependency on a shell script for management tasks.
I'm inclined to think we're unlikely to get rid of the shell scripts any time soon (e.g. by converting it to python) so we should probably just do as you have done and copy a shell into the distroless image. Appreciate that this comment doesn't actually ask you to change anything but it's useful as a historical context of why this is how it is.
*we don't actually use bash, we use zsh for these shell scripts. E.g. all the github actions install zsh before using uhd, so we should probably use that instead. You might even be able to get away with installing a statically linked version and therefore not having to copy all the libs, e.g. https://packages.debian.org/bookworm/zsh-static. Worth a try at least!
python libs
Do we need to copy all the python libs in the way you have? Referencing Sam's (one of our devops engineers who had a play with this) work he just copies a few specific dirs (https://github.com/ukhsa-collaboration/distroless-base/blob/main/python3/Dockerfile#L17). Does his solution copy more than necessary for example? The only reason I bring it up is because it's much simpler than your shell script and therefore easier to reason about and explain.
f684fac to
4197174
Compare
|
jrdh
left a comment
There was a problem hiding this comment.
I'm still getting the build error when I try locally. Pretty sure it's the same as I reported previously:
➜ data-dashboard-api git:(spike/CDD-3109-use-hardened-image) docker build --platform linux/arm64 -f Dockerfile -t dd-be .
[+] Building 0.8s (15/23) docker:default
=> [internal] load build definition from Dockerfile 0.0s
=> => transferring dockerfile: 2.21kB 0.0s
=> resolve image config for docker-image://docker.io/docker/dockerfile:1 0.3s
=> CACHED docker-image://docker.io/docker/dockerfile:1@sha256:b6afd42430b15f2d2a4c5a02b919e98a525b785b1aaff16747d2f623364e39b 0.0s
=> => resolve docker.io/docker/dockerfile:1@sha256:b6afd42430b15f2d2a4c5a02b919e98a525b785b1aaff16747d2f623364e39b6 0.0s
=> [internal] load metadata for docker.io/library/python:3.12.6-slim-bookworm 0.3s
=> [internal] load metadata for gcr.io/distroless/cc-debian12:nonroot 0.3s
=> [internal] load .dockerignore 0.0s
=> => transferring context: 553B 0.0s
=> [internal] load build context 0.0s
=> => transferring context: 287.90kB 0.0s
=> [production 1/9] FROM gcr.io/distroless/cc-debian12:nonroot@sha256:1460b2049b1d605cba0b45c73d5e3971dcce300cfd3b95acfe22b2f 0.1s
=> => resolve gcr.io/distroless/cc-debian12:nonroot@sha256:1460b2049b1d605cba0b45c73d5e3971dcce300cfd3b95acfe22b2f34e1f5d88 0.0s
=> => sha256:99889590e328930aa82b8d3b42e03b059b3206e81dea3e120e3dd0ac4a54bce0 0B / 872.46kB 0.1s
=> => sha256:d0b859bfa04f3ff05dc81f56b479654e0110db2390f12fab3c5c8c053589a897 0B / 146.50kB 0.1s
=> => sha256:2215264100f7734c92368c990dc19c827f47dea5d143111a61856777bc75dc4c 0B / 47.69kB 0.1s
=> => sha256:d6ffcb7a12076bf375c61d013dc40c30dc186e18fa59689fc19a3a369cd57513 0B / 44.99kB 0.1s
=> [build 1/7] FROM docker.io/library/python:3.12.6-slim-bookworm@sha256:ad48727987b259854d52241fac3bc633574364867b8e20aec305 0.0s
=> => resolve docker.io/library/python:3.12.6-slim-bookworm@sha256:ad48727987b259854d52241fac3bc633574364867b8e20aec305e6e7f4 0.0s
=> CACHED [build 2/7] WORKDIR /build 0.0s
=> CACHED [build 3/7] COPY requirements-prod.txt requirements-prod.txt 0.0s
=> CACHED [build 4/7] COPY requirements-prod-ingestion.txt requirements-prod-ingestion.txt 0.0s
=> CACHED [build 5/7] COPY docker/build_distroless_runtime.sh /usr/local/bin/build_distroless_runtime.sh 0.0s
=> ERROR [build 6/7] RUN bash /usr/local/bin/build_distroless_runtime.sh 0.1s
=> CANCELED [production 2/9] WORKDIR /code 0.0s
------
> [build 6/7] RUN bash /usr/local/bin/build_distroless_runtime.sh:
0.069 exec /bin/sh: exec format error
------
6 warnings found (use docker --debug to expand):
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 54)
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 55)
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 15)
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 16)
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 52)
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 53)
Dockerfile:24
--------------------
22 | # Build runtime bundle and collect shared library deps via dedicated script.
23 | COPY docker/build_distroless_runtime.sh /usr/local/bin/build_distroless_runtime.sh
24 | >>> RUN bash /usr/local/bin/build_distroless_runtime.sh
25 |
26 | # Mounts the application code into the image
--------------------
ERROR: failed to build: failed to solve: process "/bin/sh -c bash /usr/local/bin/build_distroless_runtime.sh" did not complete successfully: exit code: 255
jrdh
left a comment
There was a problem hiding this comment.
The deployment issue I've already shared with you needs to be fixed obviously, and then a couple of minor things in line and also when I build this I get a warning about the ENV lines as they are using the old format, might as well convert them to ENV key=value to remove those.
2c6e6ab to
dee516d
Compare
…irtualenv dependency because initial version has been removed from release
e81af6d to
de26216
Compare
…odels. De-duplicate the search_fields, putting them in the UKHSAPage class.
Bumps [python-dotenv](https://github.com/theskumar/python-dotenv) from 1.2.1 to 1.2.2. - [Release notes](https://github.com/theskumar/python-dotenv/releases) - [Changelog](https://github.com/theskumar/python-dotenv/blob/main/CHANGELOG.md) - [Commits](theskumar/python-dotenv@v1.2.1...v1.2.2) --- updated-dependencies: - dependency-name: python-dotenv dependency-version: 1.2.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [black](https://github.com/psf/black) from 26.3.0 to 26.3.1. - [Release notes](https://github.com/psf/black/releases) - [Changelog](https://github.com/psf/black/blob/main/CHANGES.md) - [Commits](psf/black@26.3.0...26.3.1) --- updated-dependencies: - dependency-name: black dependency-version: 26.3.1 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [identify](https://github.com/pre-commit/identify) from 2.6.16 to 2.6.18. - [Commits](pre-commit/identify@v2.6.16...v2.6.18) --- updated-dependencies: - dependency-name: identify dependency-version: 2.6.18 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [setuptools](https://github.com/pypa/setuptools) from 82.0.0 to 82.0.1. - [Release notes](https://github.com/pypa/setuptools/releases) - [Changelog](https://github.com/pypa/setuptools/blob/main/NEWS.rst) - [Commits](pypa/setuptools@v82.0.0...v82.0.1) --- updated-dependencies: - dependency-name: setuptools dependency-version: 82.0.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [platformdirs](https://github.com/tox-dev/platformdirs) from 4.9.2 to 4.9.4. - [Release notes](https://github.com/tox-dev/platformdirs/releases) - [Changelog](https://github.com/tox-dev/platformdirs/blob/main/docs/changelog.rst) - [Commits](tox-dev/platformdirs@4.9.2...4.9.4) --- updated-dependencies: - dependency-name: platformdirs dependency-version: 4.9.4 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [plotly](https://github.com/plotly/plotly.py) from 6.5.2 to 6.6.0. - [Release notes](https://github.com/plotly/plotly.py/releases) - [Changelog](https://github.com/plotly/plotly.py/blob/main/CHANGELOG.md) - [Commits](plotly/plotly.py@v6.5.2...v6.6.0) --- updated-dependencies: - dependency-name: plotly dependency-version: 6.6.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
* CMS: Make landing page related link body optional * CMS: Update test and migration --------- Co-authored-by: sahmed06 <58435820+sahmed06@users.noreply.github.com>
Bumps [filelock](https://github.com/tox-dev/py-filelock) from 3.25.0 to 3.25.2. - [Release notes](https://github.com/tox-dev/py-filelock/releases) - [Changelog](https://github.com/tox-dev/filelock/blob/main/docs/changelog.rst) - [Commits](tox-dev/filelock@3.25.0...3.25.2) --- updated-dependencies: - dependency-name: filelock dependency-version: 3.25.2 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
|


Description
This PR includes the following:
Fixes #CDD-3109
Type of change
Please select the options that are relevant.
Checklist: