-
Notifications
You must be signed in to change notification settings - Fork 39
Conversation
Welcome @jchauncey! |
still need to test that this actually works |
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:
I think that change on top of your user change will shore things up. :) |
yeah i tried starting it locally and noticed the tmp error. We could just use distroless:nonroot |
Ok i updated to use nonroot distroless which seems it may have fixed the tmp issue. |
fixes #16 |
This is great, thanks for tackling this! /approve |
[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:
Approvers can indicate their approval by writing |
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. |
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. |
@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 ***@***.***> 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
<#32 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKUO6EONY5VKX22D6EGLWLRUU34LANCNFSM4NQ3C73Q>
.
|
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 |
distroless/static only have these components: https://github.com/GoogleContainerTools/distroless/blob/master/base/README.md
|
We don't really need |
I must be doing something wrong then because I cant seem to get that to work. It complains about writing to /tmp. |
tmp dir is handled by our yaml deployment file. |
* also removes /tmp and ca-certificates from the final image
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 |
Ok i verified that the pod starts and runs with no errors in the logs with the latest commit |
/lgtm |
This pr moves the final image from scratch to distroless/static:nonroot