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 parent directory permissions #619

Merged
merged 4 commits into from
Mar 19, 2019

Conversation

dtaniwaki
Copy link
Contributor

I found chown a directory after adding a file in it causes wrong directory owner and permission.

My Dockerfile was something like below.

FROM ubuntu:16.04

RUN adduser --disabled-password --gecos "" --uid 1000 user
RUN mkdir -p /home/user/foo
RUN chown -R user /home/user
ADD bar /home/user/foo/bar
RUN chown -R user /home/user

I tried the Docker image built in Kaniko and checked the owner and permissions by docker run -it IMAGE ls -l /home/user, then I got the following result.

total 4
drwxr-xr-x    1 root     root          4096 Mar 16 16:34 foo

I expect it to be:

total 4
drwx------    1 user     user          4096 Mar 16 16:34 foo

I compared the built image with the one built by docker build, then I found the image layers of docker build has parent directories if layers have newly added files.

I think some Docker volume driver cannot update a file system if intermediate layers lost info of parent directories of a file.

Here's a similar issue in Docker (originally an issue of aufs).
moby/moby#1295

@dtaniwaki dtaniwaki changed the title Add parent dirs Fix parent directory permissions Mar 16, 2019
@dtaniwaki dtaniwaki force-pushed the add-parent-dirs branch 2 times, most recently from bdef908 to 147f58a Compare March 16, 2019 17:16
@@ -147,7 +158,10 @@ func TestSnapshotFiles(t *testing.T) {
}
defer os.Remove(tarPath)

expectedFiles := []string{"/", "/tmp", filepath.Join(testDir, "foo")}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually don't know why / and /tmp was included...

@dtaniwaki
Copy link
Contributor Author

@dlorenc I updated the PR!

RUN mkdir -p /home/user/foo
RUN chown -R user /home/user
RUN chmod 700 /home/user/foo
ADD Makefile /home/user/foo/Makefile
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you remove this line? We don't have a Makefile in the context which is causing the integration test build to fail.

Copy link
Contributor Author

@dtaniwaki dtaniwaki Mar 18, 2019

Choose a reason for hiding this comment

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

This line cannot be deleted because having ADD between chowns causes this issue. Is there any file available in the environment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, maybe we can use some URL to download a file because it’s ADD. let me update the PR.

@dtaniwaki
Copy link
Contributor Author

@dlorenc I updated the Dockerfile, please check it.

@dlorenc dlorenc merged commit 1bf4421 into GoogleContainerTools:master Mar 19, 2019
@dtaniwaki dtaniwaki deleted the add-parent-dirs branch March 19, 2019 18:18
@dtaniwaki
Copy link
Contributor Author

Thank you for merging the PR! Do you have any plan to release new version?

@dlorenc
Copy link
Collaborator

dlorenc commented Mar 19, 2019

Id like to get #621 in then cut a release

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