Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.43%. Comparing base (93c57b6) to head (b4b0bab).
Report is 1285 commits behind head on master.

@@            Coverage Diff             @@
##           master    #6416      +/-   ##
==========================================
+ Coverage   55.70%   58.43%   +2.73%     
==========================================
  Files         224      269      +45     
  Lines       10016    13990    +3974     
==========================================
+ Hits         5579     8175    +2596     
- Misses       3978     5199    +1221     
- Partials      459      616     +157     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

If the Dockerfile didn't override the USER line at all, it would pick up the uid from the distroless image
I've kept it as is, as it's clearer to someone reading the Dockerfile

👍

My only concern here is that there is an ARG to allow changing the base if building the image via the Dockerfile, while this enforces a specific USER unless commented out (since it's implicit already). By only having it as a comment for documentation, one could build the image with the distroless root image variant too.

Given coredns includes its own ca-certificates the difference is very minor.

This may change in future as it's been suggested to simplify the Dockerfile, where ca-certificates can rely upon the one provided by the base image instead of copying from the build stage which is not necessary: #6320

If those changes are approved, the feature for a custom base image ARG is less useful due to how minimal Dockerfile becomes, but may still warrant not enforcing a specific USER?

Dockerfile has run coredns as USER nonroot:nonroot, but it's not possible to enforce that with Kubernetes as it can only check UIDs.

As you have k8s experience, if you're able to provide any input on why my PR triggered a test failure for it, that'd be appreciated:
#6320 (comment)

Given the description here for the incompatibility with k8s, the test suite must not be running as non-root in the same manner, but not as root either if it's actually dependent upon setcap? (not required on Dockers network stack which relaxes the restriction a bit by default regardless of USER)

By only having it as a comment for documentation, one could build the image with the distroless root image variant too.

It would fail previously, as the user wouldn't exist, but UIDs always work, so now it would work, although run as nonroot without the user existing (which may or not break things, although most Go programs are okay with such things).

Given the description here for the incompatibility with k8s, the test suite must not be running as non-root in the same manner

The incompatibility is only in enforcing it's running as non-root (a security check), "runAsNonRoot" checks that USER is numeric but not 0 (as using a name needs some complex and potentially fragile logic that reads /etc/passwd within the container, i.e. what the image has or even a volume mount... you see why it doesn't implement that).

It would fail previously, as the user wouldn't exist, but UIDs always work, so now it would work

Isn't the UID already the default user though? The USER instruction isn't actually required in the Dockerfile here to be non-root by default right? That's already the case with the base image, is it different for k8s?

you see why it doesn't implement that

Yes, I am familiar with this issue in Docker land too. I encountered it when using COPY --link --chown to keep the UID + GID of data from another image, it could not use the friendly name, even if it was also present in the target image for the COPY.

i just tried to build this and facing the following error, any thoughts?

[+] Building 2.7s (9/12)
 => [internal] load build definition from Dockerfile                                                                           0.0s
 => => transferring dockerfile: 912B                                                                                           0.0s
 => [internal] load .dockerignore                                                                                              0.0s
 => => transferring context: 34B                                                                                               0.0s
 => [internal] load metadata for gcr.io/distroless/static-debian12:nonroot                                                     2.5s
 => [internal] load metadata for docker.io/library/debian:stable-slim                                                          2.0s
 => CANCELED [build 1/4] FROM docker.io/library/debian:stable-slim@sha256:382967fd7c35a0899ca3146b0b73d0791478fba2f71020c7aa8  0.0s
 => => resolve docker.io/library/debian:stable-slim@sha256:382967fd7c35a0899ca3146b0b73d0791478fba2f71020c7aa8c27e3a4f26672    0.0s
 => => sha256:382967fd7c35a0899ca3146b0b73d0791478fba2f71020c7aa8c27e3a4f26672 1.85kB / 1.85kB                                 0.0s
 => => sha256:90128f59a7c6f6fdcb6493f587ea352d5c7507f52a6ddfba66fc56cd3d99dc2b 529B / 529B                                     0.0s
 => => sha256:6d8a06cfe531142e7f7072b3395a20912c2a11673baad88f082b8b5aa17560f9 1.46kB / 1.46kB                                 0.0s
 => CANCELED [stage-1 1/3] FROM gcr.io/distroless/static-debian12:nonroot@sha256:8dd8d3ca2cf283383304fd45a5c9c74d5f2cd9da8d3b  0.0s
 => => resolve gcr.io/distroless/static-debian12:nonroot@sha256:8dd8d3ca2cf283383304fd45a5c9c74d5f2cd9da8d3b077d720e264880077  0.0s
 => => sha256:8dd8d3ca2cf283383304fd45a5c9c74d5f2cd9da8d3b077d720e264880077c65 1.51kB / 1.51kB                                 0.0s
 => => sha256:8ea8b3f8c8882795152cd742e5f55dc56b5c95d60a284d5c33ab8fb22738a923 1.50kB / 1.50kB                                 0.0s
 => => sha256:692140bfa125e9ada50d3b0237c67534a349869f636713833344c4d48623039e 1.95kB / 1.95kB                                 0.0s
 => [internal] load build context                                                                                              0.0s
 => => transferring context: 2B                                                                                                0.0s
 => CACHED [build 2/4] RUN export DEBCONF_NONINTERACTIVE_SEEN=true            DEBIAN_FRONTEND=noninteractive            DEBIA  0.0s
 => ERROR [build 3/4] COPY coredns /coredns                                                                                    0.0s
------
 > [build 3/4] COPY coredns /coredns:
------
failed to compute cache key: "/coredns" not found: not found

Fixes #6903

@SuperQ updated.

@SuperQ Thanks for reviewing this PR, would you mind merging it as well?

This should probably be cut with a minor release, which would be v1.12.0.

@chrisohaver Do we have any release plans coming up?

This should probably be cut with a minor release, which would be v1.12.0.

@chrisohaver Do we have any release plans coming up?

Yes. #6900