-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(proxy, proxy-init): Combine proxy and proxy-init and use minimal base image #14577
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
Conversation
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Alex Leong <[email protected]>
alpeb
left a comment
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.
Great change 👍
I think you missed removing the proxyInit.image section from the values.yaml file 😉
cratelyn
left a comment
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.
✔️
Signed-off-by: Alex Leong <[email protected]>
zaharidichev
left a comment
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.
Awesome work. All looks great, I just left one comment with questions I feel we need some clarity on before merging!
| COPY --from=proxy-init /out/linkerd2-proxy-init /usr/lib/linkerd/linkerd2-proxy-init | ||
| # Set sys caps for iptables utilities and proxy-init | ||
| USER root | ||
| RUN ["/usr/sbin/setcap", "cap_net_raw,cap_net_admin+eip", "/usr/sbin/xtables-legacy-multi"] | ||
| RUN ["/usr/sbin/setcap", "cap_net_raw,cap_net_admin+eip", "/usr/sbin/xtables-nft-multi"] | ||
| RUN ["/usr/sbin/setcap", "cap_net_raw,cap_net_admin+eip", "/usr/lib/linkerd/linkerd2-proxy-init"] | ||
| USER 65534 |
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.
Assume that there are users that run the CNI plugin because they do not want containers with any CAP_NET_ADMIN.
In this situation they would now still pull that image. Is that a problem? Or that is ultimately limited by what caps are being set in the workload definition? Which brings me to the next question. Do we have any tests that exercise this image in CNI mode?
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.
My understanding is that this is okay because the proxy containers (which do not have the CAP_NET_ADMIN capability) never attempt to execute any of these binaries which have additional caps set on them. If it did, they would fail with a permission denied error. On the other hand, the proxy-init container does have the CAP_NET_ADMIN capability and can execute these binaries just fine. Same image, but different runtime capabilities and different binaries executed.
Security policies will be looking to make sure that the proxy container itself doesn't have CAP_NET_ADMIN, which it doesn't.
The cni-calico-deep integration test exercises CNI mode with this proxy.
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.
Ok this is in line with what I was also imagining the case to be. Thanks for verifying that there is a test that exercises this logic.
To simplify the docker images we ship, we combine the proxy and proxy-init images into a unified image (named proxy) which includes both the proxy and proxy-init binaries.
To reduce the security surface area of this unified image, we build it on a minimal Wolfi-based runtime image via apko instead of building on
gcr.io/distroless/cc-debian12. This allows us to avoid including unused packages such aslibsslwhich can cause spurious security scan alerts in our images.In order to build this minimal runtime base image in CI, we start a local temporary registry so that we can push the apko created runtime image and use it as a base image for the proxy image.
We update the Linkerd templates to use this new unified proxy image in the linkerd-init container.