feat(Dockerfile): Run as nonroot user

[jchauncey]

This pr moves the final image from scratch to distroless/static:nonroot

[k8s-ci-robot]

Welcome @jchauncey!

It looks like this is your first PR to kubernetes-sigs/dashboard-metrics-scraper 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/dashboard-metrics-scraper has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[jchauncey]

still need to test that this actually works

[jeefy]

Unfortunately this won't work with scratch because of the file permissions with tmp (just verified)

That said, if you change the base image from "scratch" to "distroless", this should work:

FROM gcr.io/distroless/base AS final

I think that change on top of your user change will shore things up. :)

[jchauncey]

yeah i tried starting it locally and noticed the tmp error. We could just use distroless:nonroot

[jchauncey]

Ok i updated to use nonroot distroless which seems it may have fixed the tmp issue.

[jchauncey]

fixes #16

[jeefy]

This is great, thanks for tackling this!

/approve /assign @maciaszczykm @floreks

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jchauncey, jeefy

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

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/dashboard-metrics-scraper/blob/master/OWNERS)~~ [jeefy] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

[floreks]

Is there any actual reason to use distroless instead of scratch? The container itself does not enforce privileges. We do restrict container privileges in our deployment files already.

https://github.com/kubernetes/dashboard/blob/5a289e51a60958bafacb5266dc602ca4bb565806/aio/deploy/recommended.yaml#L288-L292

[jchauncey]

Our customers (aks) are having this clusters scanned with the twistlock tool and its flagging this image (along with a few others) as being created with a nonroot user. Even though you set the user at runtime that isnt picked up by the scanning tool and I believe this is failing the scan as its marked a high severity.

[jeefy]

@floreks Distroless is widely used elsewhere across K8s and solves a couple problems that we can run into with scratch (certificates, nonroot, and tmp are the three primary ones)

It's a small change for a pretty big win IMO and strongly endorse the move.

On Tue, Jun 2, 2020 at 1:52 PM Jonathan Chauncey notifications@github.com wrote:

Our customers (aks) are having this clusters scanned with the twistlock tool and its flagging this image (along with a few others) as being created with a nonroot user. Even though you set the user at runtime that isnt picked up by the scanning tool and I believe this is failing the scan as its marked a high severity.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/dashboard-metrics-scraper/pull/32#issuecomment-637707723, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKUO6EONY5VKX22D6EGLWLRUU34LANCNFSM4NQ3C73Q .

[floreks]

Why I like scratch is because it does not add any additional layers to the final image. It is basically an empty image with just the binary. Distroless images have all this additional stuff that we do not need and will most probably never need. If some static check fails because it cannot see nonroot user inside dockerfile then we can simply add passwd file and change user without changing the base image. We've already had this discussion inside dashboard repository https://github.com/kubernetes/dashboard/pull/4301.

[sozercan]

distroless/static only have these components: https://github.com/GoogleContainerTools/distroless/blob/master/base/README.md

• ca-certificates and /tmp are already being used here
• as you mentioned, /etc/passwd for adding (nonroot) user so the container itself doesn't get flagged

[floreks]

We don't really need ca-certificates or tmp dir already inside the image. I'd only add passwd to the current Dockerfile, then change user and leave it as is.

[jchauncey]

I must be doing something wrong then because I cant seem to get that to work. It complains about writing to /tmp.

[floreks]

tmp dir is handled by our yaml deployment file.

[jchauncey]

Ok new commit that removes /tmp and ca-certificates as well as creates a user in the builder stage and copies it to the final image. went ahead and pushed this commit to see if people are ok with this approach. still need to verify it works

[jchauncey]

Ok i verified that the pod starts and runs with no errors in the logs with the latest commit

[floreks]

/lgtm