Skip to content

Commit

Permalink
Refactor whitelist handling. (#559)
Browse files Browse the repository at this point in the history
Also speed up stage deletion.
  • Loading branch information
dlorenc authored Feb 13, 2019
1 parent 8179c47 commit 877abd3
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 48 deletions.
7 changes: 0 additions & 7 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,5 @@ const (
Dockerignore = ".dockerignore"
)

// KanikoBuildFiles is the list of files required to build kaniko
var KanikoBuildFiles = []string{"/kaniko/executor",
"/kaniko/ssl/certs/ca-certificates.crt",
"/kaniko/docker-credential-gcr",
"/kaniko/docker-credential-ecr-login",
"/kaniko/.docker/config.json"}

// ScratchEnvVars are the default environment variables needed for a scratch image.
var ScratchEnvVars = []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}
6 changes: 1 addition & 5 deletions pkg/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,7 @@ func (s *Snapshotter) TakeSnapshotFS() (string, error) {
timer = timing.Start("Writing tar file")
// Now create the tar.
for path := range memFs {
whitelisted, err := util.CheckWhitelist(path)
if err != nil {
return "", err
}
if whitelisted {
if util.CheckWhitelist(path) {
logrus.Debugf("Not adding %s to layer, as it's whitelisted", path)
continue
}
Expand Down
52 changes: 20 additions & 32 deletions pkg/util/fs_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,17 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) {
func DeleteFilesystem() error {
logrus.Info("Deleting filesystem...")
return filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, _ error) error {
whitelisted, err := CheckWhitelist(path)
if err != nil {
return err
}
if whitelisted || ChildDirInWhitelist(path, constants.RootDir) {
if CheckWhitelist(path) {
if info.IsDir() {
return filepath.SkipDir
}
logrus.Debugf("Not deleting %s, as it's whitelisted", path)
return nil
}
if childDirInWhitelist(path) {
logrus.Debugf("Not deleting %s, as it contains a whitelisted path", path)
return nil
}
if path == constants.RootDir {
return nil
}
Expand All @@ -146,16 +149,9 @@ func DeleteFilesystem() error {
}

// ChildDirInWhitelist returns true if there is a child file or directory of the path in the whitelist
func ChildDirInWhitelist(path, directory string) bool {
for _, d := range constants.KanikoBuildFiles {
dirPath := filepath.Join(directory, d)
if HasFilepathPrefix(dirPath, path, false) {
return true
}
}
func childDirInWhitelist(path string) bool {
for _, d := range whitelist {
dirPath := filepath.Join(directory, d.Path)
if HasFilepathPrefix(dirPath, path, d.PrefixMatchOnly) {
if HasFilepathPrefix(d.Path, path, d.PrefixMatchOnly) {
return true
}
}
Expand Down Expand Up @@ -190,11 +186,12 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error {
uid := hdr.Uid
gid := hdr.Gid

whitelisted, err := CheckWhitelist(path)
abs, err := filepath.Abs(path)
if err != nil {
return err
}
if whitelisted && !checkWhitelistRoot(dest) {

if CheckWhitelist(abs) && !checkWhitelistRoot(dest) {
logrus.Debugf("Not adding %s because it is whitelisted", path)
return nil
}
Expand Down Expand Up @@ -245,11 +242,11 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error {

case tar.TypeLink:
logrus.Debugf("link from %s to %s", hdr.Linkname, path)
whitelisted, err := CheckWhitelist(hdr.Linkname)
abs, err := filepath.Abs(hdr.Linkname)
if err != nil {
return err
}
if whitelisted {
if CheckWhitelist(abs) {
logrus.Debugf("skipping symlink from %s to %s because %s is whitelisted", hdr.Linkname, path, hdr.Linkname)
return nil
}
Expand Down Expand Up @@ -299,19 +296,14 @@ func IsInWhitelist(path string) bool {
return false
}

func CheckWhitelist(path string) (bool, error) {
abs, err := filepath.Abs(path)
if err != nil {
logrus.Infof("unable to get absolute path for %s", path)
return false, err
}
func CheckWhitelist(path string) bool {
for _, wl := range whitelist {
if HasFilepathPrefix(abs, wl.Path, wl.PrefixMatchOnly) {
return true, nil
if HasFilepathPrefix(path, wl.Path, wl.PrefixMatchOnly) {
return true
}
}

return false, nil
return false
}

func checkWhitelistRoot(root string) bool {
Expand Down Expand Up @@ -379,11 +371,7 @@ func RelativeFiles(fp string, root string) ([]string, error) {
if err != nil {
return err
}
whitelisted, err := CheckWhitelist(path)
if err != nil {
return err
}
if whitelisted && !HasFilepathPrefix(path, root, false) {
if CheckWhitelist(path) && !HasFilepathPrefix(path, root, false) {
return nil
}
if err != nil {
Expand Down
50 changes: 46 additions & 4 deletions pkg/util/fs_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,7 @@ func Test_CheckWhitelist(t *testing.T) {
whitelist = original
}()
whitelist = tt.args.whitelist
got, err := CheckWhitelist(tt.args.path)
if err != nil {
t.Fatalf("error checking whitelist: %v", err)
}
got := CheckWhitelist(tt.args.path)
if got != tt.want {
t.Errorf("CheckWhitelist() = %v, want %v", got, tt.want)
}
Expand Down Expand Up @@ -596,3 +593,48 @@ func TestCopySymlink(t *testing.T) {
})
}
}

func Test_childDirInWhitelist(t *testing.T) {
type args struct {
path string
whitelist []WhitelistEntry
}
tests := []struct {
name string
args args
want bool
}{
{
name: "not in whitelist",
args: args{
path: "/foo",
},
want: false,
},
{
name: "child in whitelist",
args: args{
path: "/foo",
whitelist: []WhitelistEntry{
{
Path: "/foo/bar",
},
},
},
want: true,
},
}
oldWhitelist := whitelist
defer func() {
whitelist = oldWhitelist
}()

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
whitelist = tt.args.whitelist
if got := childDirInWhitelist(tt.args.path); got != tt.want {
t.Errorf("childDirInWhitelist() = %v, want %v", got, tt.want)
}
})
}
}

0 comments on commit 877abd3

Please sign in to comment.