From c8b19894bb7cef363b43110de064520b531789fe Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 28 Jan 2020 08:52:29 -0800 Subject: [PATCH 1/2] Revert "additional-whitelist" This reverts commit ac8e12381535a4cae59921925de84bffcafb81e1. --- README.md | 4 -- cmd/executor/cmd/root.go | 8 ++- pkg/util/fs_util_test.go | 118 ++++++++++++++------------------------- 3 files changed, 50 insertions(+), 80 deletions(-) diff --git a/README.md b/README.md index 4033a57597..3dc6c7a38c 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,6 @@ _If you are interested in contributing to kaniko, see [DEVELOPMENT.md](DEVELOPME - [--single-snapshot](#--single-snapshot) - [--skip-tls-verify](#--skip-tls-verify) - [--skip-tls-verify-pull](#--skip-tls-verify-pull) - - [--additional-whitelist](#--additional-whitelist) - [--snapshotMode](#--snapshotmode) - [--target](#--target) - [--tarPath](#--tarpath) @@ -494,9 +493,6 @@ Set this flag to skip TLS certificate validation when pushing to a registry. It Set this flag to skip TLS certificate validation when pulling from a registry. It is supposed to be used for testing purposes only and should not be used in production! -#### --additional-whitelist -Set this flag with a list of filepaths and Kaniko will ignore these paths during the build. Useful for improving build performance on large filesystems. - #### --snapshotMode You can set the `--snapshotMode=` flag to set how kaniko will snapshot the filesystem. diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 892e6b016b..0374ba3417 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -75,6 +75,12 @@ var RootCmd = &cobra.Command{ return errors.New("You must provide --destination if setting ImageNameDigestFile") } + if additionalWhitelist == nil { + additionalWhitelist = []string{ + "/var/run", + } + } + for _, path := range additionalWhitelist { util.AddToWhitelist(path) } @@ -152,7 +158,7 @@ func addKanikoOptionsFlags() { // We use nil as the default value so we can differentiate between the flag passed // with an empty list and the flag not set - RootCmd.PersistentFlags().StringSliceVar(&additionalWhitelist, "additional-whitelist", []string{}, "Paths to whitelist. These will be ignored by kaniko to improve performance.") + RootCmd.PersistentFlags().StringSliceVar(&additionalWhitelist, "additional-whitelist", nil, "Paths to whitelist. These will be ignored be kaniko to improve performance.") } // addHiddenFlags marks certain flags as hidden from the executor help text diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 63c1e98f83..1ebf4e5b95 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -38,12 +38,26 @@ import ( ) func Test_DetectFilesystemWhitelist(t *testing.T) { - type testcase struct { - desc string - additionalWhitelist []string - expectedWhitelist []WhitelistEntry + testDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatalf("Error creating tempdir: %s", err) + } + fileContents := ` + 228 122 0:90 / / rw,relatime - aufs none rw,si=f8e2406af90782bc,dio,dirperm1 + 229 228 0:98 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw + 230 228 0:99 / /dev rw,nosuid - tmpfs tmpfs rw,size=65536k,mode=755 + 231 230 0:100 / /dev/pts rw,nosuid,noexec,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=666 + 232 228 0:101 / /sys ro,nosuid,nodev,noexec,relatime - sysfs sysfs ro` + + path := filepath.Join(testDir, "mountinfo") + if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil { + t.Fatalf("Error creating tempdir: %s", err) + } + if err := ioutil.WriteFile(path, []byte(fileContents), 0644); err != nil { + t.Fatalf("Error writing file contents to %s: %s", path, err) } + err = DetectFilesystemWhitelist(path) expectedWhitelist := []WhitelistEntry{ {"/kaniko", false}, {"/proc", false}, @@ -52,82 +66,36 @@ func Test_DetectFilesystemWhitelist(t *testing.T) { {"/sys", false}, {"/etc/mtab", false}, } + actualWhitelist := whitelist + sort.Slice(actualWhitelist, func(i, j int) bool { + return actualWhitelist[i].Path < actualWhitelist[j].Path + }) + sort.Slice(expectedWhitelist, func(i, j int) bool { + return expectedWhitelist[i].Path < expectedWhitelist[j].Path + }) + testutil.CheckErrorAndDeepEqual(t, false, err, expectedWhitelist, actualWhitelist) - testCases := []testcase{ - { - desc: "no additional whitelist", - expectedWhitelist: expectedWhitelist, - }, - { - desc: "one additional whitelist - /var/run", - additionalWhitelist: []string{"/var/run"}, - expectedWhitelist: append(expectedWhitelist, WhitelistEntry{"/var/run", false}), - }, - { - desc: "two additional whitelist - /var/run, /usr/bin", - additionalWhitelist: []string{"/var/run", "/usr/bin"}, - expectedWhitelist: append( - expectedWhitelist, - WhitelistEntry{"/var/run", false}, - WhitelistEntry{"/usr/bin", false}, - ), - }, - } - - for _, tc := range testCases { - t.Run(tc.desc, func(t *testing.T) { - expectedWhitelist := tc.expectedWhitelist - additionalWhitelist := tc.additionalWhitelist - - tmpWhitelist := make([]WhitelistEntry, len(initialWhitelist)) - copy(tmpWhitelist, initialWhitelist) - - testDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("Error creating tempdir: %s", err) - } - fileContents := ` - 228 122 0:90 / / rw,relatime - aufs none rw,si=f8e2406af90782bc,dio,dirperm1 - 229 228 0:98 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw - 230 228 0:99 / /dev rw,nosuid - tmpfs tmpfs rw,size=65536k,mode=755 - 231 230 0:100 / /dev/pts rw,nosuid,noexec,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=666 - 232 228 0:101 / /sys ro,nosuid,nodev,noexec,relatime - sysfs sysfs ro` - - path := filepath.Join(testDir, "mountinfo") - if err := os.MkdirAll(filepath.Dir(path), 0750); err != nil { - t.Fatalf("Error creating tempdir: %s", err) - } - if err := ioutil.WriteFile(path, []byte(fileContents), 0644); err != nil { - t.Fatalf("Error writing file contents to %s: %s", path, err) - } - - for _, wl := range additionalWhitelist { - AddToWhitelist(wl) - } - - err = DetectFilesystemWhitelist(path) - actualWhitelist := whitelist + tmpInitial := make([]WhitelistEntry, len(initialWhitelist)) - if len(actualWhitelist) != len(expectedWhitelist) { - t.Errorf( - "expected whitelist to have %d items but was %d", - len(expectedWhitelist), - len(actualWhitelist), - ) - } + copy(tmpInitial, initialWhitelist) + defer func() { + initialWhitelist = tmpInitial + }() - sort.Slice(actualWhitelist, func(i, j int) bool { - return actualWhitelist[i].Path < actualWhitelist[j].Path - }) - sort.Slice(expectedWhitelist, func(i, j int) bool { - return expectedWhitelist[i].Path < expectedWhitelist[j].Path - }) + AddToWhitelist("/var/run") - testutil.CheckErrorAndDeepEqual(t, false, err, expectedWhitelist, actualWhitelist) + err = DetectFilesystemWhitelist(path) + expectedWhitelist = append(expectedWhitelist, + WhitelistEntry{"/var/run", false}) - initialWhitelist = tmpWhitelist - }) - } + actualWhitelist = whitelist + sort.Slice(actualWhitelist, func(i, j int) bool { + return actualWhitelist[i].Path < actualWhitelist[j].Path + }) + sort.Slice(expectedWhitelist, func(i, j int) bool { + return expectedWhitelist[i].Path < expectedWhitelist[j].Path + }) + testutil.CheckErrorAndDeepEqual(t, false, err, expectedWhitelist, actualWhitelist) } var tests = []struct { From d49c198c907ffadd812bcaa20ca84fae49a4ad9a Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 28 Jan 2020 08:52:36 -0800 Subject: [PATCH 2/2] Revert "add option additonal-whitelist" This reverts commit 72bfed18506977054bb05589e87cb7b920b11823. --- cmd/executor/cmd/root.go | 21 +++------------------ pkg/util/fs_util.go | 11 +++++++---- pkg/util/fs_util_test.go | 23 +---------------------- 3 files changed, 11 insertions(+), 44 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 0374ba3417..e7a058abdc 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -38,10 +38,9 @@ import ( ) var ( - opts = &config.KanikoOptions{} - logLevel string - force bool - additionalWhitelist []string + opts = &config.KanikoOptions{} + logLevel string + force bool ) func init() { @@ -74,16 +73,6 @@ var RootCmd = &cobra.Command{ if len(opts.Destinations) == 0 && opts.ImageNameDigestFile != "" { return errors.New("You must provide --destination if setting ImageNameDigestFile") } - - if additionalWhitelist == nil { - additionalWhitelist = []string{ - "/var/run", - } - } - - for _, path := range additionalWhitelist { - util.AddToWhitelist(path) - } } return nil }, @@ -155,10 +144,6 @@ func addKanikoOptionsFlags() { RootCmd.PersistentFlags().DurationVarP(&opts.CacheTTL, "cache-ttl", "", time.Hour*336, "Cache timeout in hours. Defaults to two weeks.") RootCmd.PersistentFlags().VarP(&opts.InsecureRegistries, "insecure-registry", "", "Insecure registry using plain HTTP to push and pull. Set it repeatedly for multiple registries.") RootCmd.PersistentFlags().VarP(&opts.SkipTLSVerifyRegistries, "skip-tls-verify-registry", "", "Insecure registry ignoring TLS verify to push and pull. Set it repeatedly for multiple registries.") - - // We use nil as the default value so we can differentiate between the flag passed - // with an empty list and the flag not set - RootCmd.PersistentFlags().StringSliceVar(&additionalWhitelist, "additional-whitelist", nil, "Paths to whitelist. These will be ignored be kaniko to improve performance.") } // addHiddenFlags marks certain flags as hidden from the executor help text diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index cd9e91b22a..aada79fbe6 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -50,6 +50,13 @@ var initialWhitelist = []WhitelistEntry{ Path: "/kaniko", PrefixMatchOnly: false, }, + { + // /var/run is a special case. It's common to mount in /var/run/docker.sock or something similar + // which leads to a special mount on the /var/run/docker.sock file itself, but the directory to exist + // in the image with no way to tell if it came from the base image or not. + Path: "/var/run", + PrefixMatchOnly: false, + }, { // similarly, we whitelist /etc/mtab, since there is no way to know if the file was mounted or came // from the base image @@ -64,10 +71,6 @@ var volumes = []string{} var excluded []string -func AddToWhitelist(path string) { - initialWhitelist = append(initialWhitelist, WhitelistEntry{Path: path}) -} - type ExtractFunction func(string, *tar.Header, io.Reader) error type FSConfig struct { diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 1ebf4e5b95..c1c1c9ddd1 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -64,6 +64,7 @@ func Test_DetectFilesystemWhitelist(t *testing.T) { {"/dev", false}, {"/dev/pts", false}, {"/sys", false}, + {"/var/run", false}, {"/etc/mtab", false}, } actualWhitelist := whitelist @@ -74,28 +75,6 @@ func Test_DetectFilesystemWhitelist(t *testing.T) { return expectedWhitelist[i].Path < expectedWhitelist[j].Path }) testutil.CheckErrorAndDeepEqual(t, false, err, expectedWhitelist, actualWhitelist) - - tmpInitial := make([]WhitelistEntry, len(initialWhitelist)) - - copy(tmpInitial, initialWhitelist) - defer func() { - initialWhitelist = tmpInitial - }() - - AddToWhitelist("/var/run") - - err = DetectFilesystemWhitelist(path) - expectedWhitelist = append(expectedWhitelist, - WhitelistEntry{"/var/run", false}) - - actualWhitelist = whitelist - sort.Slice(actualWhitelist, func(i, j int) bool { - return actualWhitelist[i].Path < actualWhitelist[j].Path - }) - sort.Slice(expectedWhitelist, func(i, j int) bool { - return expectedWhitelist[i].Path < expectedWhitelist[j].Path - }) - testutil.CheckErrorAndDeepEqual(t, false, err, expectedWhitelist, actualWhitelist) } var tests = []struct {