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

Fix#2122 #2125

Closed
wants to merge 4 commits into from
Closed

Fix#2122 #2125

wants to merge 4 commits into from

Conversation

BobbyNie
Copy link

@BobbyNie BobbyNie commented Jun 7, 2022

Fixes #<2122>

Description

BobbyNie@139.com

Do not call os.Chown when both DoNotChangeUID and DoNotChangeGID to support noroot container to run some basic Dockerfile

@google-cla
Copy link

google-cla bot commented Jun 7, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@imjasonh
Copy link
Collaborator

imjasonh commented Jun 7, 2022

cc @hown3d since you're also making changes to UID/GID handling in #2106

@@ -331,7 +331,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
return err
}

if err = setFilePermissions(path, mode, uid, gid); err != nil {
if err = setFilePermissions(path, mode, int64(uint32(uid)), int64(uint32(gid))); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if err = setFilePermissions(path, mode, int64(uint32(uid)), int64(uint32(gid))); err != nil {
if err = setFilePermissions(path, mode, int64(uid), int64(gid)); err != nil {

@hown3d
Copy link
Contributor

hown3d commented Jun 7, 2022

I don't think that this change will make running kaniko in rootless mode possible. Kaniko extracts the tared base image inside of the kaniko container, where folder destinations like /bin are used for creating symlinks, directories etc.

By using another user than root, you can't always 100% make sure, that every folder where the image will get extracted is writable by that user. In order to achieve that, kaniko would need to have some general changes on how the base images are extracted into the kaniko container filesystem and how the file permission settings are handled.

So going rootless requires a lot of steps.

The integration tests will succeed, since the container is always run without the --user flag and no other user is specified inside the Dockerfile, so the container will be still run as root.

@BobbyNie
Copy link
Author

BobbyNie commented Jun 9, 2022

I don't think that this change will make running kaniko in rootless mode possible. Kaniko extracts the tared base image inside of the kaniko container, where folder destinations like /bin are used for creating symlinks, directories etc.

By using another user than root, you can't always 100% make sure, that every folder where the image will get extracted is writable by that user. In order to achieve that, kaniko would need to have some general changes on how the base images are extracted into the kaniko container filesystem and how the file permission settings are handled.

So going rootless requires a lot of steps.

The integration tests will succeed, since the container is always run without the --user flag and no other user is specified inside the Dockerfile, so the container will be still run as root.

Thanks for review.

I'm runing kaniko in openshift cluster.
actually this attempt not work when the Dockerfile contained COPY or RUN cmd.

finally I created a service account and give it anyuid permission. then runAsUser 0 to sove it.

@BobbyNie BobbyNie closed this Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants