Skip to content
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

Add tests for rootless containers #2546

Merged
merged 6 commits into from
Jan 30, 2024

Conversation

juliusvonkohout
Copy link
Member

@juliusvonkohout juliusvonkohout commented Oct 5, 2023

Which issue is resolved by this Pull Request:
We need to list all containers that do not have runasnonroot set or run as the root user. Based on this list i can create per workinggroup patches and at some point we can just enable the test in the e2e pipeline.

Description of your changes:
Adding tests to warn and later on error on root containers in the cicd.

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: juliusvonkohout
Once this PR has been reviewed and has the lgtm label, please assign kimwnasptd for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot added size/S and removed size/XS labels Oct 5, 2023
@google-oss-prow google-oss-prow bot added size/M and removed size/S labels Oct 5, 2023
@google-oss-prow google-oss-prow bot added size/L and removed size/M labels Oct 5, 2023
@juliusvonkohout juliusvonkohout changed the title WIP: Update README.md Add tests for rootless containers Oct 5, 2023
@juliusvonkohout juliusvonkohout marked this pull request as ready for review October 5, 2023 16:48
@google-oss-prow google-oss-prow bot requested a review from elikatsis October 5, 2023 16:48
@juliusvonkohout
Copy link
Member Author

@annajung @kimwnasptd please review.

@annajung
Copy link
Member

From quick look, the script checks to two specific fields in pod spec and container spec. In the description you mention needing to create a list of containers that do not have runasnonroot set, that doesn't seem to be part of the current script. Is that intended?

@juliusvonkohout
Copy link
Member Author

juliusvonkohout commented Oct 13, 2023

@annajung thanks for the review.

It is a bit more complex. In the end we want to determine whether containers run as root or have been run as root (initcontainers)

Some containers are already completed (initcontainers) some are missing a shell or the Id command to get the current user, some are missing a security context and or the runasnonroot within the securitycontext, some on the pod level and some on the container level.

In the end it is a diverse set of problems for which we need solutions. There are just many ways to check rootfulness. The script provides per container heuristics and negative hypothesis tests that let you easily gauge whether a container runs or ran as root.

For the moment it is useful for the development focus and creating securitycontext/runasnonroot PRs in each working group. In the long term we may fail the cicd if this script finds something potentially rootful.

@juliusvonkohout
Copy link
Member Author

@annajung can we move forward with a /lgtm or do you think it should be refined?

@rimolive
Copy link
Member

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Jan 29, 2024
@juliusvonkohout juliusvonkohout merged commit 337dfb1 into kubeflow:master Jan 30, 2024
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants