Skip to content

Commit

Permalink
Fix handling of volume directive
Browse files Browse the repository at this point in the history
  • Loading branch information
peter-evans committed Sep 26, 2018
1 parent bb0df68 commit ab4b65e
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 63 deletions.
2 changes: 1 addition & 1 deletion integration/dockerfiles/Dockerfile_test_volume
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions integration/dockerfiles/Dockerfile_test_volume_2
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0
VOLUME /foo1
RUN mkdir /bar1
VOLUME /foo2
VOLUME /foo3
RUN echo "bar2"
VOLUME /foo4
RUN mkdir /bar3
VOLUME /foo5
3 changes: 0 additions & 3 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,6 @@ func TestLayers(t *testing.T) {
offset := map[string]int{
"Dockerfile_test_add": 10,
"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) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/commands/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ 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
}

// 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)
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ const (
// DefaultHOMEValue is the default value Docker sets for $HOME
HOME = "HOME"
DefaultHOMEValue = "/root"

// VolumeCmdName is the name of the volume command
VolumeCmdName = "volume"
)

// KanikoBuildFiles is the list of files required to build kaniko
Expand Down
11 changes: 10 additions & 1 deletion pkg/executor/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -188,17 +193,21 @@ 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{}
}
}
}
if err != nil {
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
Expand Down
82 changes: 47 additions & 35 deletions pkg/util/fs_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -136,13 +149,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
}
}
Expand Down Expand Up @@ -251,7 +264,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
}
}
Expand All @@ -263,7 +276,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
}
}
Expand All @@ -276,7 +289,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
Expand All @@ -299,7 +312,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)
Expand All @@ -322,7 +338,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 {
Expand Down Expand Up @@ -385,22 +401,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
}

Expand Down Expand Up @@ -500,7 +509,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, "/")
Expand All @@ -509,6 +518,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
Expand Down
Loading

0 comments on commit ab4b65e

Please sign in to comment.