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 bug in extracting hardlinks in multistage builds #456

Merged
merged 4 commits into from
Nov 20, 2018

Conversation

priyawadhwa
Copy link
Collaborator

@priyawadhwa priyawadhwa commented Nov 17, 2018

When we execute multistage builds, we store the fs of each intermediate
stage at /kaniko/ if it's used later in the build. This
created a bug when extracting hardlinks, because we weren't appending
the new directory to the link path.

So, if /tmp/file1 and /tmp/file2 were hardlinked, kaniko was trying
to link /kaniko/0/tmp/file1 to /tmp/file2 instead of
/kaniko/0/tmp/file2. This change will append the correct directory to
the link, and

fixes #437
fixes #362
fixes #352
fixes #342.

When we execute multistage builds, we store the fs of each intermediate
stage at /kaniko/<stage number> if it's used later in the build. This
created a bug when extracting hardlinks, because we weren't appending
the new directory to the link path.

So, if `/tmp/file1` and `/tmp/file2` were hardlinked, kaniko was trying
to link `/kaniko/0/tmp/file1` to `/tmp/file2` instead of
`/kaniko/0/tmp/file2`. This change will append the correct directory to
the link, and fixes GoogleContainerTools#437 GoogleContainerTools#362 GoogleContainerTools#352 GoogleContainerTools#342.

if err := os.Link(filepath.Clean(filepath.Join("/", hdr.Linkname)), path); err != nil {
link := filepath.Join(dest, hdr.Linkname)
if err := os.Link(filepath.Clean(filepath.Join("/", link)), path); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious to know why "/". path is essentially filepath.Join(dest, hdr.Name), so it seems reasonable that the first arg to os.Link is filepath.Join(dest, hdr.Linkname), no?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks for pointing that out! i've removed the unnecessary filepath.Join

Priya Wadhwa added 3 commits November 19, 2018 15:03
Now that hardlink destinations take into account the directory that they are being extracted to, the unit test had to be updated to make sure that two hardlinks were extracted to /tmp/hardlink correctly.
@priyawadhwa priyawadhwa merged commit a7599d1 into GoogleContainerTools:master Nov 20, 2018
@priyawadhwa priyawadhwa deleted the composer branch November 20, 2018 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants