-
Notifications
You must be signed in to change notification settings - Fork 119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove rootless images from build process #558
Remove rootless images from build process #558
Conversation
Hey Tyler, thanks for picking this one up! I think the first thing we need to do is to set the user arg to non root: https://github.com/open-policy-agent/opa-envoy-plugin/blob/main/Dockerfile#L13C1-L13C11, similar to this: https://github.com/open-policy-agent/opa/blob/b463d30028d927a00bf0143d6fbf0533bf5d6783/Dockerfile#L15 Then we can get a release out and share that that the default user has been updated to non-root. Either in the same release, or in a follow on release, we can do as you have here, where we drop the rootless tags. We've made a big noise about the end of rootless images in the past for opa, but it might be good to do a final reminder in the next release for opa-envoy too. @ashutosh-narkar might be able to provide some metrics on the image pulls for the rootless tags. Also Ash, let me know if you think that plan above sounds ok, and if you think we can do the user change and the dropping of the tags in the same release. |
😞 |
Signed-off-by: Tyler Schade <tyler.schade@solo.io>
5424666
to
5575da5
Compare
@charlieegan3 thanks for the pointer about the default user, I missed that part! Would you like me to restore the |
Hey, I think that the best course of action is to drop rootless tag at the same time as making the default image use a non-root user. The rationale here being that we've already spent a lot of time letting users know that rootless images are discontinued and that they are not to be used. Making this change now in opa-envoy will be 'just' updating opa-envoy to be the same as OPA. Do you think that makes sense too? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work here @tjons 👏
Merge after open-policy-agent#558 Follows open-policy-agent/opa#6830 Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
When merging, we should rebase and merge #560 too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Merge after open-policy-agent#558 Follows open-policy-agent/opa#6830 Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
Merge after #558 Follows open-policy-agent/opa#6830 Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com> Co-authored-by: Charlie Egan <charlieegan3@users.noreply.github.com>
Resolves open-policy-agent/opa#6810 by removing rootless images from the
Makefile
, and thus, from the build/release process foropa-envoy-plugin
.