From ab66f6bf857526f53174a284bc88cf859825be73 Mon Sep 17 00:00:00 2001 From: Nitish Gupta Date: Thu, 12 May 2022 02:22:40 +0530 Subject: [PATCH] bug: Update `getFileFilter` to include parent folders - 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. Signed-off-by: Nitish Gupta --- acceptance/acceptance_test.go | 37 ++++++ pkg/client/build.go | 29 ++++- pkg/client/build_test.go | 210 ++++++++++++++++++++++++++++++++++ 3 files changed, 274 insertions(+), 2 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index be56e18b2b..70eb73e2e7 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1969,6 +1969,43 @@ include = [ "*.jar", "media/mountain.jpg", "/media/person.png", ] assert.Contains(output, "mountain.jpg") assert.Contains(output, "person.png") }) + + it("should assign correct permissions to all directories and files", func() { + h.SkipIf(t, imageManager.HostOS() == "windows", "These tests are not yet compatible with Windows-based containers") + + projectToml := ` +[project] +name = "permission test" +[[project.licenses]] +type = "MIT" + +[build] +include = [ "nested/nested-cookie.jar", "media/person.png", ] + +[[build.buildpacks]] +id = "testing/workspace-info" + [build.buildpacks.script] + api = "0.7" + # prints the files and their owners + inline = "ls -alHR | awk '{print $3, $9}' && exit 1" +` + projectDescriptorPath := filepath.Join(tempAppDir, "project.toml") + err := ioutil.WriteFile(projectDescriptorPath, []byte(projectToml), 0755) + assert.Nil(err) + + output, err := pack.Run( + "build", + repoName, + "-p", tempAppDir, + "--descriptor", projectDescriptorPath, + ) + assert.Error(err) + assert.Contains(output, "pack media") + assert.Contains(output, "pack nested") + assert.Contains(output, "pack mountain.jpg") + assert.Contains(output, "pack person.png") + assert.Contains(output, "pack nested-cookie.jar") + }) }) }) diff --git a/pkg/client/build.go b/pkg/client/build.go index 2b5540b688..6f0acdade9 100644 --- a/pkg/client/build.go +++ b/pkg/client/build.go @@ -420,13 +420,38 @@ func getFileFilter(descriptor projectTypes.Descriptor) (func(string) bool, error }, nil } if len(descriptor.Build.Include) > 0 { - includes := ignore.CompileIgnoreLines(descriptor.Build.Include...) + includePatterns := []string{} + for _, entry := range descriptor.Build.Include { + if !strings.Contains(entry, "*") { + parentFolders, err := getParentDirs(entry) + for _, folder := range parentFolders { + includePatterns = append(includePatterns, "!"+folder+"/*") + includePatterns = append(includePatterns, folder) + } + if err != nil { + return nil, err + } + } + } + includePatterns = append(includePatterns, descriptor.Build.Include...) + includes := ignore.CompileIgnoreLines(includePatterns...) return includes.MatchesPath, nil } - return nil, nil } +func getParentDirs(file string) ([]string, error) { + parent := filepath.Dir(file) + if parent == filepath.VolumeName(file)+`\` || parent == "/" || parent == "." { + return []string{}, nil + } + parentDirs, err := getParentDirs(parent) + if err != nil { + return nil, err + } + return append(parentDirs, parent), nil +} + func lifecycleImageSupported(builderOS string, lifecycleVersion *builder.Version) bool { return lifecycleVersion.Equal(builder.VersionMustParse(prevLifecycleVersionSupportingImage)) || !lifecycleVersion.LessThan(semver.MustParse(minLifecycleVersionSupportingImage)) diff --git a/pkg/client/build_test.go b/pkg/client/build_test.go index f590984322..253e888ce1 100644 --- a/pkg/client/build_test.go +++ b/pkg/client/build_test.go @@ -2508,6 +2508,210 @@ func testBuild(t *testing.T, when spec.G, it spec.S) { }) }) }) + + when("#getFileFilter", func() { + it("project descriptor with include directive", func() { + toCheck := []fileFilterCompare{ + { + inputType: "unnested files", + directiveInput: []string{"file1.txt", "file2.txt"}, + patternOutputMap: map[string]bool{ + "parent": false, + "file1.txt": true, + "file2.txt": true, + "parent/file1.txt": true, + "parent/file2.txt": true, + }, + }, + { + inputType: "unnested directories", + directiveInput: []string{"dir1", "dir2"}, + patternOutputMap: map[string]bool{ + "dir1": true, + "dir2": true, + "file1.txt": false, + "dir1/dir2": true, + "dir1/dir2/folder3": true, + "folder/dir1": true, + "dir1/file1.txt": true, + "folder/folder2/dir2": true, + "dir2/folder/file2.txt": true, + }, + }, + { + inputType: "single level nested files", + directiveInput: []string{"parent/file1.txt", "parent/file2.txt"}, + patternOutputMap: map[string]bool{ + "parent": true, + "file1.txt": false, + "file2.txt": false, + "parent/file1.txt": true, + "parent/file2.txt": true, + }, + }, + { + inputType: "double level nested files", + directiveInput: []string{"parent1/file1.txt", "parent1/parent2/file2.txt"}, + patternOutputMap: map[string]bool{ + "parent1": true, + "parent2": false, + "file1.txt": false, + "file2.txt": false, + "parent1/parent2": true, + "parent1/file1.txt": true, + "parent2/file2.txt": false, + "parent1/parent2/file2.txt": true, + }, + }, + { + inputType: "multi level nested files with same dir names", + directiveInput: []string{"parent/samename/diffname/samename/file1.txt"}, + patternOutputMap: map[string]bool{ + "parent": true, + "samename": false, + "diffname": false, + "file1.txt": false, + "parent/samename": true, + "samename/diffname": false, + "diffname/samename": false, + "samename/file1.txt": false, + "parent/samename/diffname": true, + "samename/diffname/samename": false, + "diffname/samename/file1.txt": false, + "parent/samename/diffname/samename": true, + "parent/samename/diffname/samename/file1.txt": true, + }, + }, + { + inputType: "regex input 1", + directiveInput: []string{"parent1/*", "parent2/*", "abc/*/def", "!testdir/*", "!filetoexclude.txt"}, + patternOutputMap: map[string]bool{ + "parent1": false, + "parent2": false, + "absent": false, + "parent1/parent2/": true, + "testdir": false, + "testdir/file.txt": false, + "absent/parent2/": true, + "absent/parent2/file.txt": true, + "absent/testdir/file2.txt": false, + "filetoexclude.txt": false, + "absent/filetoexclude.txt": false, + "parent1/filetoexclude.txt": false, + "abc/ghi/def": true, + "abc/def": false, + "abc/def/def/file.txt": true, + }, + }, + } + + for _, checkEntry := range toCheck { + t.Logf("checking directive input type: %s", checkEntry.inputType) + fileFilter, err := getFileFilter(projectTypes.Descriptor{ + Build: projectTypes.Build{ + Include: checkEntry.directiveInput, + }, + }) + for pattern, expectedOutput := range checkEntry.patternOutputMap { + t.Logf("checking pattern %s", pattern) + h.AssertEq(t, fileFilter(pattern), expectedOutput) + } + h.AssertNil(t, err) + } + }) + + it("project descriptor with exclude directive", func() { + toCheck := []fileFilterCompare{ + { + inputType: "unnested files", + directiveInput: []string{"file1.txt", "file2.txt"}, + patternOutputMap: map[string]bool{ + "parent": true, + "file1.txt": false, + "file2.txt": false, + "parent/file1.txt": false, + "parent/file2.txt": false, + }, + }, + { + inputType: "unnested directories", + directiveInput: []string{"dir1", "dir2"}, + patternOutputMap: map[string]bool{ + "dir1": false, + "dir2": false, + "file1.txt": true, + "dir1/dir2": false, + "dir1/dir2/folder3": false, + "folder/dir1": false, + "dir1/file1.txt": false, + "folder/folder2/dir2": false, + "dir2/folder/file2.txt": false, + }, + }, + { + inputType: "single level nested files", + directiveInput: []string{"parent/file1.txt", "parent/file2.txt"}, + patternOutputMap: map[string]bool{ + "parent": true, + "file1.txt": true, + "file2.txt": true, + "parent/file1.txt": false, + "parent/file2.txt": false, + }, + }, + { + inputType: "double level nested files", + directiveInput: []string{"parent1/file1.txt", "parent1/parent2/file2.txt"}, + patternOutputMap: map[string]bool{ + "parent1": true, + "parent2": true, + "file1.txt": true, + "file2.txt": true, + "parent1/parent2": true, + "parent1/file1.txt": false, + "parent2/file2.txt": true, + "parent1/parent2/file2.txt": false, + }, + }, + } + + for _, checkEntry := range toCheck { + t.Logf("checking directive input type: %s", checkEntry.inputType) + fileFilter, err := getFileFilter(projectTypes.Descriptor{ + Build: projectTypes.Build{ + Exclude: checkEntry.directiveInput, + }, + }) + for pattern, expectedOutput := range checkEntry.patternOutputMap { + t.Logf("checking pattern: %s", pattern) + h.AssertEq(t, fileFilter(pattern), expectedOutput) + } + h.AssertNil(t, err) + } + }) + }) + + when("#getParentDirs", func() { + it("check parent directories", func() { + compare := map[string][]string{ + ".": {}, + "/": {}, + "parent": {}, + "parent/child1": {"parent"}, + "parent/child1/file.txt": {"parent", "parent/child1"}, + "parent/child1/child2/": {"parent", "parent/child1", "parent/child1/child2"}, + "parent/same/diff/same/file.txt": {"parent", "parent/same", "parent/same/diff", "parent/same/diff/same"}, + "parent/child/grandchild/*.ts": {"parent", "parent/child", "parent/child/grandchild"}, + } + + for input, expectedOutput := range compare { + t.Logf("checking parent directories for: %s", input) + parentDirs, err := getParentDirs(input) + h.AssertEq(t, parentDirs, expectedOutput) + h.AssertNil(t, err) + } + }) + }) } func diffIDForFile(t *testing.T, path string) string { @@ -2623,3 +2827,9 @@ func (f *executeFailsLifecycle) Execute(_ context.Context, opts build.LifecycleO f.Opts = opts return errors.New("") } + +type fileFilterCompare struct { + inputType string + directiveInput []string + patternOutputMap map[string]bool +}