-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Docker image: add vim and nettools (netstat) #10815
Conversation
@lhotari is it true that now we are building the official docker image on CI ? IIRC you switched to some lightweight docker image the integration tests can you please confirm ? |
There's the java-test-image available added by #9625 . The docker file for that is https://github.com/apache/pulsar/blob/master/tests/docker-images/java-test-image/Dockerfile . java-test-image isn't used for integration tests in CI atm. There's support for that, but it's not used in CI. |
@lhotari thanks |
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.
this is a production image. so please be thoughtful when adding an additional package.
@sijie sure I know. Now that we are rootless it is very difficult to operate inside the containers. if you deploy Pulsar using the Open Source docker images and the helm chart you should be able to work |
@eolivelli thanks for your contribution. For this PR, do we need to update docs? |
@sijie @addisonj @michaeljmarshall do you have some suggestion about how to help users work on the pods now that we are rootless ? |
I think the largest difference is that you cannot download dependencies via Depending on how we move forward here, I can add docs for troubleshooting with the non-root docker images. (As the above links describe, one note is that you can still run the container as the root user. If you're starting the docker container yourself, you can specify This PR does raise one question for me: what are our container security goals for Pulsar? I agree that development is easier with these tools (and possibly others) in the docker image. However, I'd posit the main use case for these docker images is not development, but is rather running in a production environment where there shouldn't be frequent debugging. These tools technically introduce potential attack vectors and increase the size of these docker images. One option is to create "debug" docker images that include more tools. Given that we already include several pulsar clients in the basic Full disclosure, my perspective is influenced by Google's "distroless" work: https://github.com/GoogleContainerTools/distroless. Their perspective is strong (and if you go to their README, you'll see that they make use of "debug" images):
I'm not sure that Pulsar should (or can) become "distroless", but I do find the principles compelling. |
It is really a pain to upgrade to 2.8.0 docker images (a part from the fact that you have to fix the owner of the files on the bk and zk pods). I wonder if anyone will be happy to upgrade to 2.8.0 docker images. |
d3cfd4d
to
0f7c1ea
Compare
I think security conscious users/companies will be happy to see we're moving in this direction. Further, the Pulsar 2.8.0 image is the first pulsar image that is compliant with Open Shift's requirements to run a container as a random non root user. We can mitigate much of the complexity in the helm chart. In the official Helm chart, we are going to override the default user for the bookkeeper and zookeeper containers so they run as root until a user opts in to running them as non root.
I agree that we want to provide a docker image that is useful to the majority of our users. A "debug" docker image could eventually make sense too, but for now, we can just add in these packages. |
Give we have reverted the docker image from rootless to a root image, we should hold on to this change and push the discussion later when we switched back to rootless. |
Makes sense to me. I am keeping open this PR and add it to 2.9.0, so that we won't forget |
The pr had no activity for 30 days, mark with Stale label. |
@eolivelli:Thanks for your contribution. For this PR, do we need to update docs? |
Motivation
Now that we have rootless images the user (system administrator in this case) cannot install new tools easily, so it is better to add to the base image some common tools, like "vim" and "netstat"
e. What is the problem you're trying to solve.*
Modifications
Add "vim" and "netstat" and "unzip" to the base image.
The image will become a little bigger but it is worth
Verifying this change
This change is already covered by existing tests, the integration tests.