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: revert setting default UserID and GroupID to 0:0 #202

Merged
merged 5 commits into from
May 20, 2024

Conversation

johnstcn
Copy link
Member

@johnstcn johnstcn commented May 20, 2024

After looking into the failing unit tests some more, it looks like coder/kaniko@9f83bc8 actually does cause divergence from Docker behaviour.

I've included an integration test adapted from GoogleContainerTools/kaniko that reproduces the issue. It affects file permissions when changing permissions in a multi-stage build but does not appear to affect single-stage builds.

This test fails with the commit, and succeeds without. The fix has not been merged upstream yet as it would break legacy users. For now, we will revert to the upstream behaviour.

This will unfortunately re-break #70 but has a simple workaround of using COPY --chown 0:0.

@johnstcn johnstcn self-assigned this May 20, 2024
@mafredri
Copy link
Member

After looking into the failing unit tests some more, it looks like coder/kaniko@9f83bc8 actually does cause divergence from the Docker spec.

What's the divergence? As far as Docker spec is concerned, root is the default:

All files and directories copied from the build context are created with a UID and GID of 0.unless the optional --chown flag specifies a given username, groupname, or UID/GID combination to request specific ownership of the copied content.
From: https://docs.docker.com/reference/dockerfile/#copy

@johnstcn
Copy link
Member Author

johnstcn commented May 20, 2024

What's the divergence? As far as Docker spec is concerned, root is the default:

In a multi-stage build, the UIDs/GIDs of the source appear to be retained. This was the behaviour that the failing integration test was checking.

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

I think breaking multi stage builds is a good enough reason to revert this for the time being. Let's revisit later if we can bring this in line with Docker behavior.

@johnstcn johnstcn merged commit 43b5b37 into main May 20, 2024
3 checks passed
@johnstcn johnstcn deleted the cj/revert-kaniko-breaking-change branch May 20, 2024 16:15
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.

2 participants