Run all knative-sample images as non root

[karthikmurali60]

Fixes knative/serving#14566

Proposed Changes

• Run all knative-sample images as non root

[knative-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: karthikmurali60
Once this PR has been reviewed and has the lgtm label, please assign snneji for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[netlify]

✅ Deploy Preview for knative ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	789281c
🔍 Latest deploy log	https://app.netlify.com/sites/knative/deploys/655195e43e27a80008c2bef1
😎 Deploy Preview	https://deploy-preview-5758--knative.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[knative-prow]

Welcome @karthikmurali60! It looks like this is your first PR to knative/docs 🎉

[kauana]

Hello @karthikmurali60, I'm thinking we should remove the -S from the adduser command since not all images run on alpine.

Thoughts @dprotaso?

[ReToCode]

+1 on the -S comment, other than that, good to get this in.

[ReToCode]

See comments above

[prushh]

Sorry for the intrusion; I was working on this issue before that @karthikmurali60 answered me (see comment).

I have a draft PR (prushh#1) on my repository if you are interested.

I would like to expose some points:

• the README.md file of each project should be updated when the Dockerfile is reported;
• the containers should be tested to be sure that there are no errors at build or run time;
• why define the user twice when using multi-stage build instead of just the last stage?
• if I am not wrong, non-alpine images should use useradd /groupadd commands instead of adduser/addgroup.

Thanks for your time.

[ReToCode]

Ah, I did not get that.

These are all very good points 👍 is it possible that you coordinate to combine your work?

[kauana]

• if I am not wrong, non-alpine images should use useradd /groupadd commands instead of adduser/addgroup.

It seems both useradd and adduser offer the same features. I would prefer keeping all images consistent, unless this is not possible for some obscure reason.

[prushh]

Ah, I did not get that.

My bad, I started working on it since he hadn't responded for a while.

is it possible that you coordinate to combine your work?

No problem here!

---

I would like to know if you have any preferences on the Dockerfile structure in order to standardise them a bit:

1. Is it better to have some ARG instructions containing variables (e.g. username, pid, etc.) or to specify them hardcoded in the RUN instructions?
2. Is it okay to define the user only in the last stage when possible?
3. As @kauana suggests, we will use adduser/ addgroup commands to keep all images consistent (I thought they weren't available in some distros)
4. Is it okay to use the user directory (ex. "/home/${USER}/app") for each application?
5. What type of OS are gcr.io/distroless/static and gcr.io/distroless/base images?

[ReToCode]

Is it better to have some ARG instructions containing variables (e.g. username, pid, etc.) or to specify them hardcoded in the RUN instructions?

I don't have a strong preference here, but as these are "just" code-samples, I'd vote to keep it as simple as possible.

Is it okay to define the user only in the last stage when possible?

Sure.

As @kauana suggests, we will use adduser/ addgroup commands to keep all images consistent (I thought they weren't available in some distros)

👍

Is it okay to use the user directory (ex. "/home/${USER}/app") for each application?

Yeah, but keep in mind that those images are also used in more restrictive environments than vanilla K8s like OpenShift, which will overwrite the uid to a random high number uid. So there is a slight risk that this could cause permission issues there. But if you have a draft-build pushed to a public registry I'm happy to try to run them before merging.

What type of OS are gcr.io/distroless/static and gcr.io/distroless/base images?

Debian, see https://github.com/GoogleContainerTools/distroless

[prushh]

But if you have a draft-build pushed to a public registry I'm happy to try to run them before merging.

I pushed the cloudevents-go sample using the following image definition.

Show Dockerfile

# Use the official Golang image to create a build artifact.
# This is based on Debian and sets the GOPATH to /go.
# https://hub.docker.com/_/golang
FROM golang:1.13 as builder

ARG TARGETOS
ARG TARGETARCH

# Create and change to the app directory.
WORKDIR /app

# Copy local code to the container image.
COPY . ./

# Ensure Go modules are on and create the go.mod and go.sum files.
# Also builds the binary
ENV GO111MODULE=on
RUN go mod init cloudevents.go \
    && CGO_ENABLED=0 GOOS=linux GOOS=${TARGETOS} GOARCH=${TARGETARCH} go build -v -o server

# Allows the container builds to reuse downloaded dependencies.
RUN go mod download

# Use the official Alpine image for a lean production container.
# https://hub.docker.com/_/alpine
# https://docs.docker.com/develop/develop-images/multistage-build/#use-multi-stage-builds
FROM alpine:3

ARG USER=appuser
ARG USER_UID=1000
ARG USER_GID=$USER_UID

# Add a user so the server will run as a non-root user.
RUN addgroup -g $USER_GID $USER && \
    adduser -u ${USER_UID} -G $USER -D $USER && \
    apk add --no-cache ca-certificates

# Create and change to the app directory.
WORKDIR "/home/${USER}/app"

# Copy the binary to the production image from the builder stage.
COPY --from=builder /app/server ./server

# Set the non-root user as current.
USER $USER

# Run the web service on container startup.
CMD ["./server"]

You can find it at https://hub.docker.com/r/prushh/cloudevents-go

[ReToCode]

@prushh tested on OpenShift, works fine with a random runtime user:

/home/appuser/app $ id
uid=1000730000(1000730000) gid=0(root) groups=0(root),1000730000

/home/appuser/app $ ls -ltra
total 12160
drwxr-sr-x    1 appuser  appuser         17 Nov 17 15:55 ..
-rwxr-xr-x    1 root     root      12450916 Nov 17 15:55 server
drwxr-sr-x    1 root     appuser         20 Nov 17 15:55 .

/home/appuser/app $ ps
PID   USER     TIME  COMMAND
    1 10007300  0:00 ./server
   13 10007300  0:00 sh
   29 10007300  0:00 ps

[prushh]

@karthikmurali60 Could you please give me access to your sample-images-non-root branch?

Otherwise, I could create a new PR with both our commits.

[prushh]

@ReToCode How can I proceed? He hasn't answered in a while...

[ReToCode]

@ReToCode How can I proceed? He hasn't answered in a while...

Feel free to open a new PR on your fork, referencing this PR here. I'll close this one.