Skip to content

Commit

Permalink
bug: Update getFileFilter to include parent folders
Browse files Browse the repository at this point in the history 
- 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 <[email protected]>
  • Loading branch information
@imnitishng
imnitishng committed May 13, 2022
1 parent bed572e commit 55e3f0e
Show file tree
Hide file tree
Showing 3 changed files with 296 additions and 2 deletions.
37 changes: 37 additions & 0 deletions acceptance/acceptance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
})
})

Expand Down
29 changes: 27 additions & 2 deletions pkg/client/build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
232 changes: 232 additions & 0 deletions pkg/client/build_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2508,6 +2508,232 @@ 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 non windows", func() {
h.SkipIf(t, runtime.GOOS == "windows", "These tests are disabled for windows environments")
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)
}
})

it("check parent directories windows", func() {
h.SkipIf(t, runtime.GOOS != "windows", "These tests are disabled for non windows environments")
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 {
Expand Down Expand Up @@ -2623,3 +2849,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
}

0 comments on commit 55e3f0e

Please sign in to comment.