-
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.
| image: | ||
| # -- Docker image for the proxy-init container | ||
| name: cr.l5d.io/linkerd/proxy-init | ||
| # -- Pull policy for the proxy-init container image | ||
| # @default -- imagePullPolicy | ||
| pullPolicy: "" | ||
| # -- Tag for the proxy-init container image | ||
| version: v2.4.3 |
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.
cross-referencing linkerd/linkerd2-proxy#4333.
…init binary Add symlink at `/usr/lib/linkerd/linkerd2-proxy-init` pointing to `/usr/bin/proxy-init` for compatibility with upstream expectations. Create /usr/lib/linkerd directory before creating symlink. Add check in tests. Bump epoch to 6. Helps with linkerd2 PR: linkerd/linkerd2#14577 Signed-off-by: Francesco Bartolini <[email protected]>
…init binary (#74692) Add symlink at `/usr/lib/linkerd/linkerd2-proxy-init` pointing to `/usr/bin/proxy-init` for compatibility with upstream expectations. Create /usr/lib/linkerd directory before creating symlink. Add check in tests. Bump epoch to 6. Helps with linkerd2 PR: linkerd/linkerd2#14577 Signed-off-by: Francesco Bartolini <[email protected]> <!--ci-cve-scan:fail-any--> Signed-off-by: Francesco Bartolini <[email protected]>
in linkerd/linkerd2#14577, we combined the proxy and proxy-init image. in https://github.com/linkerd/linkerd2#14348, we removed the policy-controller image. this branch aims to address these breaking changes, and restore CI builds in this repository. besides just combining images, \#14577 included some breaking changes to the helm charts, specifically in how we configure the control plane. this has affected us, because we do _not_ use the production-oriented `ghcr.io/linkerd/proxy` image when running the `linkerd-install` test, or running a local cluster via the commands provided in the `justfile`. we use a development-oriented image provided [here](https://github.com/linkerd/linkerd2-proxy/blob/main/Dockerfile) so that we can attach a shell to the proxy container during development, and run various networking utilities to e.g. diagnose bugs. this caused us some problems because \#14577 removed the helm configuration surface used to configure an image for the proxy-init container. thus, pods would run the development image from the proxy repo, rather than the `'ghcr.io/linkerd/proxy'` image that i've attempted to specify in the justfile. ``` Init Containers: linkerd-init: Container ID: containerd://bfe539d4686401676687f3526c43611b90bf794c300c0c9797582a57ce13196d Image: localhost/linkerd/proxy:kate.init-image-is-combined.c374b8d8c Image ID: sha256:e6bd21b9afa12fdac81f8b3df6c3ba035015a162165afbbba5cc26f729a49f9f Port: <none> Host Port: <none> SeccompProfile: RuntimeDefault Command: /usr/lib/linkerd/linkerd2-proxy-init ``` there are two ways to remediate this: (1) copy the same changes involving `setcap` commands over to that development dockerfile (_i also found that this required setting the user to `root` for things to work_), or (2) revert parts of \#14577 and reintroduce these settings to the control plane chart, instead defaulting to the `cr.l5d.io/linkerd/proxy` image. my understanding is that (2) would be unfortunate because we are actively trying to get away from having to maintain too much init-related configuration surface, as it introduces a significant maintenance burden in the control plane. this branch updates the no-longer-recent edge release used in our development image, and then updates the images used in our justfile. changes from linkerd/linkerd2#14577 are additionally introduced here, so that the proxy image also functions as a workable init container image. --- * chore(Dockerfile): update `LINKERD2_IMAGE` edge version Signed-off-by: katelyn martin <[email protected]> * fix: address upstream changes to proxy-init image Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>
see linkerd/linkerd-proxy#4333 for previous context. this commit makes changes to the Dockerfile provided in this repository, for use in the proxy's development process. rather than using `debian:bookworm-slim` as the base image, this commit helps deduplicate the tricky business of setting networking capabilities on executables needed when running as an init container. this has one negative consequence, which is that we can no longer attach to a `bash` shell in a running pod when using this image. this is unfortunate, but in my experience isn't often needed by proxy developers. i believe that, should we need to revisit the need for a shell in this image, we should do instead make use of the `Dockerfile-debug` image provided in the linkerd2 repo. if we ran a command like `just docker --build-arg LINKERD2_IMAGE='ghcr.io/linkerd/debug:edge-25-11.3'` we could specify the debug image as a base image instead, providing developers not only with a shell, but also other helpful utilities like `curl`, `tcpdump`, and so on. unfortunately, this does not work today, because the image appears to no longer be published, and has drifted from our latest edge release. i have not pulled on that string further at the time of writing. one explicit _benefit_ of the changes in this commit is that we bring proxy development closer to the real world, meaning that CI in this repository runs using the same image that the proxy will run inside of in the linkerd2 repository and in typical clusters. --- * linkerd/linkerd2#14348 * linkerd/linkerd2#14577 * linkerd/linkerd-proxy#4333 Signed-off-by: katelyn martin <[email protected]>
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.