pack icon indicating copy to clipboard operation
pack copied to clipboard

bug: Update `getFileFilter` to include parent folders

Open imnitishng opened this issue 2 years ago • 2 comments

Summary

  • The include directive when specified with a file inside a folder testdir/testfile did not pass the parent folder to the builder image, which led to the folder being created by docker instead pack causing the permissions of the folder to be cnb.
  • This PR fixes the include logic to get parent folders and add them to the file filter, so they get created via pack.

Output

Before

After

Documentation

  • Should this change be documented?
    • [ ] Yes, see #___
    • [x] No

Related

Resolves #1419

imnitishng avatar May 11 '22 20:05 imnitishng

The PR focuses on include part of the issue for now. Will add more unit/acceptance tests here.

imnitishng avatar May 11 '22 21:05 imnitishng

Hey @jromero, I have tried to fix the include part of the issue based on https://github.com/buildpacks/pack/issues/1419#issuecomment-1121562242 The idea is to add parent folder entries to the fileFilter so the folders are included in the final build. The approach works fine for cases when we know the directory name (for ex: testdir/testfile.txt, testdir/abc/name.txt, etc) but it would not work when we do not know the folder name as in Regex (for ex: abc/*/name.txt, test/**/file.txt, this would cause abc/def/ to have root owner, since we did not add ["!abc/def/*", "abc/def/name.txt"] to the filter). I don't see a way where we could tweak the getFileFilter to include such cases, I'd like to know if there's a way I'm missing. Moreover if we DO want to cover these cases then I guess we will have to modify WriteDirToTar to achieve this, but I'm not sure if the added complexity would be worth. For now I have added a check that reverts back to old logic on encountering a regex containing either of *. The compatibility tests also fail due to the changes.

imnitishng avatar May 13 '22 17:05 imnitishng