public_git
/
kaniko
mirror of https://github.com/GoogleContainerTools/kaniko
1
0
Fork 0
Code Issues Releases Wiki Activity

Merge pull request #334 from peter-evans/fix-volume-cmd 

Fix handling of the volume directive
This commit is contained in:
priyawadhwa 2018-10-01 14:49:33 -07:00 committed by GitHub
parent 442cf27c4b b1e28ddb4f
commit 8f0d257134
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 128 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
integration/dockerfiles/Dockerfile_test_volume
View File

@ -1,7 +1,7 @@
FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0
RUN mkdir /foo
RUN echo "hello" > /foo/hey
VOLUME /foo/bar /tmp
VOLUME /foo/bar /tmp /qux/quux
ENV VOL /baz/bat
VOLUME ["${VOL}"]
RUN echo "hello again" > /tmp/hey

13
integration/dockerfiles/Dockerfile_test_volume_2 Normal file
View File

@ -0,0 +1,13 @@
FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0
VOLUME /foo1
RUN echo "hello" > /foo1/hello
WORKDIR /foo1/bar
ADD context/foo /foo1/foo
COPY context/foo /foo1/foo2
RUN mkdir /bar1
VOLUME /foo2
VOLUME /foo3
RUN echo "bar2"
VOLUME /foo4
RUN mkdir /bar3
VOLUME /foo5

3
integration/integration_test.go
View File

@ -214,9 +214,6 @@ func TestLayers(t *testing.T) {
offset := map[string]int{
"Dockerfile_test_add": 11,
"Dockerfile_test_scratch": 3,
// the Docker built image combined some of the dirs defined by separate VOLUME commands into one layer
// which is why this offset exists
"Dockerfile_test_volume": 1,
}
for dockerfile, built := range imageBuilder.FilesBuilt {
t.Run("test_layer_"+dockerfile, func(t *testing.T) {

4
pkg/commands/volume.go
View File

@ -48,7 +48,7 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.
for _, volume := range resolvedVolumes {
var x struct{}
existingVolumes[volume] = x
err := util.AddPathToVolumeWhitelist(volume)
err := util.AddVolumePathToWhitelist(volume)
if err != nil {
return err
}
@ -56,7 +56,7 @@ func (v *VolumeCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.
// Only create and snapshot the dir if it didn't exist already
if _, err := os.Stat(volume); os.IsNotExist(err) {
logrus.Infof("Creating directory %s", volume)
v.snapshotFiles = []string{volume}
v.snapshotFiles = append(v.snapshotFiles, volume)
if err := os.MkdirAll(volume, 0755); err != nil {
return fmt.Errorf("Could not create directory for volume %s: %s", volume, err)
}

3
pkg/constants/constants.go
View File

@ -62,6 +62,9 @@ const (
// Docker command names
Cmd = "cmd"
Entrypoint = "entrypoint"
// VolumeCmdName is the name of the volume command
VolumeCmdName = "volume"
)
// KanikoBuildFiles is the list of files required to build kaniko

11
pkg/executor/build.go
View File

@ -138,6 +138,7 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error {
if err := s.snapshotter.Init(); err != nil {
return err
}
var volumes []string
args := dockerfile.NewBuildArgs(opts.BuildArgs)
for index, cmd := range s.stage.Commands {
finalCmd := index == len(s.stage.Commands)-1
@ -167,6 +168,10 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error {
return err
}
files := command.FilesToSnapshot()
if cmd.Name() == constants.VolumeCmdName {
volumes = append(volumes, files...)
continue
}
var contents []byte
// If this is an intermediate stage, we only snapshot for the last command and we
@ -188,9 +193,14 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error {
// the files that were changed, we'll snapshot those explicitly, otherwise we'll
// check if anything in the filesystem changed.
if files != nil {
if len(files) > 0 {
files = append(files, volumes...)
volumes = []string{}
}
contents, err = s.snapshotter.TakeSnapshot(files)
} else {
contents, err = s.snapshotter.TakeSnapshotFS()
volumes = []string{}
}
}
}
@ -198,7 +208,6 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error {
return fmt.Errorf("Error taking snapshot of files for command %s: %s", command, err)
}
util.MoveVolumeWhitelistToWhitelist()
if contents == nil {
logrus.Info("No files were changed, appending empty layer to config. No layer added to image.")
continue

41
pkg/snapshot/snapshot.go
View File

@ -25,7 +25,6 @@ import (
"path/filepath"
"syscall"
"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/util"
"github.com/sirupsen/logrus"
)
@ -84,21 +83,6 @@ func (s *Snapshotter) TakeSnapshotFS() ([]byte, error) {
return contents, err
}
func shouldSnapshot(file string, snapshottedFiles map[string]bool) (bool, error) {
if val, ok := snapshottedFiles[file]; ok && val {
return false, nil
}
whitelisted, err := util.CheckWhitelist(file)
if err != nil {
return false, fmt.Errorf("Error checking for %s in whitelist: %s", file, err)
}
if whitelisted && !isBuildFile(file) {
logrus.Infof("Not adding %s to layer, as it's whitelisted", file)
return false, nil
}
return true, nil
}
// snapshotFiles creates a snapshot (tar) and adds the specified files.
// It will not add files which are whitelisted.
func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) {
@ -123,11 +107,7 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) {
}
for _, file := range parentDirs {
file = filepath.Clean(file)
shouldSnapshot, err := shouldSnapshot(file, snapshottedFiles)
if err != nil {
return false, fmt.Errorf("Error checking if parent dir %s can be snapshotted: %s", file, err)
}
if !shouldSnapshot {
if val, ok := snapshottedFiles[file]; ok && val {
continue
}
snapshottedFiles[file] = true
@ -148,19 +128,15 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) {
// Next add the files themselves to the tar
for _, file := range files {
file = filepath.Clean(file)
shouldSnapshot, err := shouldSnapshot(file, snapshottedFiles)
if err != nil {
return false, fmt.Errorf("Error checking if file %s can be snapshotted: %s", file, err)
}
if !shouldSnapshot {
if val, ok := snapshottedFiles[file]; ok && val {
continue
}
snapshottedFiles[file] = true
if err = s.l.Add(file); err != nil {
if err := s.l.Add(file); err != nil {
return false, fmt.Errorf("Unable to add file %s to layered map: %s", file, err)
}
if err = t.AddFileToTar(file); err != nil {
if err := t.AddFileToTar(file); err != nil {
return false, fmt.Errorf("Error adding file %s to tar: %s", file, err)
}
filesAdded = true
@ -168,15 +144,6 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) {
return filesAdded, nil
}
func isBuildFile(file string) bool {
for _, buildFile := range constants.KanikoBuildFiles {
if file == buildFile {
return true
}
}
return false
}
// shapShotFS creates a snapshot (tar) of all files in the system which are not
// whitelisted and which have changed.
func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) {

82
pkg/util/fs_util.go
View File

@ -34,17 +34,30 @@ import (
"github.com/sirupsen/logrus"
)
var whitelist = []string{
"/kaniko",
// /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.
"/var/run",
// similarly, we whitelist /etc/mtab, since there is no way to know if the file was mounted or came
// from the base image
"/etc/mtab",
type WhitelistEntry struct {
Path string
PrefixMatchOnly bool
}
var whitelist = []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
Path: "/etc/mtab",
PrefixMatchOnly: false,
},
}
var volumeWhitelist = []string{}
// GetFSFromImage extracts the layers of img to root
// It returns a list of all files extracted
@ -118,13 +131,13 @@ func DeleteFilesystem() error {
func ChildDirInWhitelist(path, directory string) bool {
for _, d := range constants.KanikoBuildFiles {
dirPath := filepath.Join(directory, d)
if HasFilepathPrefix(dirPath, path) {
if HasFilepathPrefix(dirPath, path, false) {
return true
}
}
for _, d := range whitelist {
dirPath := filepath.Join(directory, d)
if HasFilepathPrefix(dirPath, path) {
dirPath := filepath.Join(directory, d.Path)
if HasFilepathPrefix(dirPath, path, d.PrefixMatchOnly) {
return true
}
}
@ -265,7 +278,7 @@ func CheckWhitelist(path string) (bool, error) {
return false, err
}
for _, wl := range whitelist {
if HasFilepathPrefix(abs, wl) {
if HasFilepathPrefix(abs, wl.Path, wl.PrefixMatchOnly) {
return true, nil
}
}
@ -277,7 +290,7 @@ func checkWhitelistRoot(root string) bool {
return false
}
for _, wl := range whitelist {
if HasFilepathPrefix(root, wl) {
if HasFilepathPrefix(root, wl.Path, wl.PrefixMatchOnly) {
return true
}
}
@ -290,7 +303,7 @@ func checkWhitelistRoot(root string) bool {
// (1)(2)(3) (4) (5) (6) (7) (8) (9) (10) (11)
// Where (5) is the mount point relative to the process's root
// From: https://www.kernel.org/doc/Documentation/filesystems/proc.txt
func fileSystemWhitelist(path string) ([]string, error) {
func fileSystemWhitelist(path string) ([]WhitelistEntry, error) {
f, err := os.Open(path)
if err != nil {
return nil, err
@ -313,7 +326,10 @@ func fileSystemWhitelist(path string) ([]string, error) {
}
if lineArr[4] != constants.RootDir {
logrus.Debugf("Appending %s from line: %s", lineArr[4], line)
whitelist = append(whitelist, lineArr[4])
whitelist = append(whitelist, WhitelistEntry{
Path: lineArr[4],
PrefixMatchOnly: false,
})
}
if err == io.EOF {
logrus.Debugf("Reached end of file %s", path)
@ -336,7 +352,7 @@ func RelativeFiles(fp string, root string) ([]string, error) {
if err != nil {
return err
}
if whitelisted && !HasFilepathPrefix(path, root) {
if whitelisted && !HasFilepathPrefix(path, root, false) {
return nil
}
if err != nil {
@ -399,22 +415,15 @@ func CreateFile(path string, reader io.Reader, perm os.FileMode, uid uint32, gid
return dest.Chown(int(uid), int(gid))
}
// AddPathToVolumeWhitelist adds the given path to the volume whitelist
// It will get snapshotted when the VOLUME command is run then ignored
// for subsequent commands.
func AddPathToVolumeWhitelist(path string) error {
logrus.Infof("adding %s to volume whitelist", path)
volumeWhitelist = append(volumeWhitelist, path)
return nil
}
// MoveVolumeWhitelistToWhitelist copies over all directories that were volume mounted
// in this step to be whitelisted for all subsequent docker commands.
func MoveVolumeWhitelistToWhitelist() error {
if len(volumeWhitelist) > 0 {
whitelist = append(whitelist, volumeWhitelist...)
volumeWhitelist = []string{}
}
// AddVolumePathToWhitelist adds the given path to the whitelist with
// PrefixMatchOnly set to true. Snapshotting will ignore paths prefixed
// with the volume, but the volume itself will not be ignored.
func AddVolumePathToWhitelist(path string) error {
logrus.Infof("adding volume %s to whitelist", path)
whitelist = append(whitelist, WhitelistEntry{
Path: path,
PrefixMatchOnly: true,
})
return nil
}
@ -514,7 +523,7 @@ func CopyFile(src, dest string) error {
}
// HasFilepathPrefix checks if the given file path begins with prefix
func HasFilepathPrefix(path, prefix string) bool {
func HasFilepathPrefix(path, prefix string, prefixMatchOnly bool) bool {
path = filepath.Clean(path)
prefix = filepath.Clean(prefix)
pathArray := strings.Split(path, "/")
@ -523,6 +532,9 @@ func HasFilepathPrefix(path, prefix string) bool {
if len(pathArray) < len(prefixArray) {
return false
}
if prefixMatchOnly && len(pathArray) == len(prefixArray) {
return false
}
for index := range prefixArray {
if prefixArray[index] == pathArray[index] {
continue

69
pkg/util/fs_util_test.go
View File

@ -50,9 +50,21 @@ func Test_fileSystemWhitelist(t *testing.T) {
}
actualWhitelist, err := fileSystemWhitelist(path)
expectedWhitelist := []string{"/kaniko", "/proc", "/dev", "/dev/pts", "/sys", "/var/run", "/etc/mtab"}
sort.Strings(actualWhitelist)
sort.Strings(expectedWhitelist)
expectedWhitelist := []WhitelistEntry{
{"/kaniko", false},
{"/proc", false},
{"/dev", false},
{"/dev/pts", false},
{"/sys", false},
{"/var/run", false},
{"/etc/mtab", false},
}
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)
}
@ -167,7 +179,7 @@ func Test_ParentDirectories(t *testing.T) {
func Test_CheckWhitelist(t *testing.T) {
type args struct {
path string
whitelist []string
whitelist []WhitelistEntry
}
tests := []struct {
name string
@ -178,7 +190,7 @@ func Test_CheckWhitelist(t *testing.T) {
name: "file whitelisted",
args: args{
path: "/foo",
whitelist: []string{"/foo"},
whitelist: []WhitelistEntry{{"/foo", false}},
},
want: true,
},
@ -186,7 +198,7 @@ func Test_CheckWhitelist(t *testing.T) {
name: "directory whitelisted",
args: args{
path: "/foo/bar",
whitelist: []string{"/foo"},
whitelist: []WhitelistEntry{{"/foo", false}},
},
want: true,
},
@ -194,7 +206,7 @@ func Test_CheckWhitelist(t *testing.T) {
name: "grandparent whitelisted",
args: args{
path: "/foo/bar/baz",
whitelist: []string{"/foo"},
whitelist: []WhitelistEntry{{"/foo", false}},
},
want: true,
},
@ -202,7 +214,7 @@ func Test_CheckWhitelist(t *testing.T) {
name: "sibling whitelisted",
args: args{
path: "/foo/bar/baz",
whitelist: []string{"/foo/bat"},
whitelist: []WhitelistEntry{{"/foo/bat", false}},
},
want: false,
},
@ -227,8 +239,9 @@ func Test_CheckWhitelist(t *testing.T) {
func TestHasFilepathPrefix(t *testing.T) {
type args struct {
path string
prefix string
path string
prefix string
prefixMatchOnly bool
}
tests := []struct {
name string
@ -238,47 +251,61 @@ func TestHasFilepathPrefix(t *testing.T) {
{
name: "parent",
args: args{
path: "/foo/bar",
prefix: "/foo",
path: "/foo/bar",
prefix: "/foo",
prefixMatchOnly: false,
},
want: true,
},
{
name: "nested parent",
args: args{
path: "/foo/bar/baz",
prefix: "/foo/bar",
path: "/foo/bar/baz",
prefix: "/foo/bar",
prefixMatchOnly: false,
},
want: true,
},
{
name: "sibling",
args: args{
path: "/foo/bar",
prefix: "/bar",
path: "/foo/bar",
prefix: "/bar",
prefixMatchOnly: false,
},
want: false,
},
{
name: "nested sibling",
args: args{
path: "/foo/bar/baz",
prefix: "/foo/bar",
path: "/foo/bar/baz",
prefix: "/foo/bar",
prefixMatchOnly: false,
},
want: true,
},
{
name: "name prefix",
args: args{
path: "/foo2/bar",
prefix: "/foo",
path: "/foo2/bar",
prefix: "/foo",
prefixMatchOnly: false,
},
want: false,
},
{
name: "prefix match only (volume)",
args: args{
path: "/foo",
prefix: "/foo",
prefixMatchOnly: true,
},
want: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := HasFilepathPrefix(tt.args.path, tt.args.prefix); got != tt.want {
if got := HasFilepathPrefix(tt.args.path, tt.args.prefix, tt.args.prefixMatchOnly); got != tt.want {
t.Errorf("HasFilepathPrefix() = %v, want %v", got, tt.want)
}
})