add unit tests for caching run and copy

pkg/commands/copy.go

@@ -17,6 +17,7 @@ limitations under the License.
 package commands
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 
@@ -156,8 +157,9 @@ func (c *CopyCommand) ShouldCacheOutput() bool {
 func (c *CopyCommand) CacheCommand(img v1.Image) DockerCommand {
 
 	return &CachingCopyCommand{
-		img: img,
-		cmd: c.cmd,
+		img:       img,
+		cmd:       c.cmd,
+		extractFn: util.ExtractFile,
 	}
 }
 
@@ -170,16 +172,23 @@ type CachingCopyCommand struct {
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.CopyCommand
+	extractFn      util.ExtractFunction
 }
 
 func (cr *CachingCopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	logrus.Infof("Found cached layer, extracting to filesystem")
 	var err error
-	cr.extractedFiles, err = util.GetFSFromImage(RootDir, cr.img)
+
+	if cr.img == nil {
+		return errors.New(fmt.Sprintf("cached command image is nil %v", cr.String()))
+	}
+	cr.extractedFiles, err = util.GetFSFromImage(RootDir, cr.img, cr.extractFn)
+
 	logrus.Infof("extractedFiles: %s", cr.extractedFiles)
 	if err != nil {
 		return errors.Wrap(err, "extracting fs from image")
 	}
+
 	return nil
 }
 
@@ -188,6 +197,9 @@ func (cr *CachingCopyCommand) FilesToSnapshot() []string {
 }
 
 func (cr *CachingCopyCommand) String() string {
+	if cr.cmd == nil {
+		return "nil command"
+	}
 	return cr.cmd.String()
 }
 

pkg/commands/copy_test.go

@@ -16,6 +16,7 @@ limitations under the License.
 package commands
 
 import (
+	"archive/tar"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -215,3 +216,143 @@ func Test_resolveIfSymlink(t *testing.T) {
 		})
 	}
 }
+
+func Test_CachingCopyCommand_ExecuteCommand(t *testing.T) {
+	tarContent, err := prepareTarFixture([]string{"foo.txt"})
+	if err != nil {
+		t.Errorf("couldn't prepare tar fixture %v", err)
+	}
+
+	config := &v1.Config{}
+	buildArgs := &dockerfile.BuildArgs{}
+
+	type testCase struct {
+		desctiption    string
+		expectLayer    bool
+		expectErr      bool
+		count          *int
+		expectedCount  int
+		command        *CachingCopyCommand
+		extractedFiles []string
+		contextFiles   []string
+	}
+	testCases := []testCase{
+		func() testCase {
+			c := &CachingCopyCommand{
+				img: fakeImage{
+					ImageLayers: []v1.Layer{
+						fakeLayer{TarContent: tarContent},
+					},
+				},
+				cmd: &instructions.CopyCommand{
+					SourcesAndDest: []string{
+						"foo.txt", "foo.txt",
+					},
+				},
+			}
+			count := 0
+			tc := testCase{
+				desctiption:    "with valid image and valid layer",
+				count:          &count,
+				expectedCount:  1,
+				expectLayer:    true,
+				extractedFiles: []string{"/foo.txt"},
+				contextFiles:   []string{"foo.txt"},
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				*tc.count++
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			c := &CachingCopyCommand{}
+			tc := testCase{
+				desctiption: "with no image",
+				expectErr:   true,
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			c := &CachingCopyCommand{
+				img: fakeImage{},
+			}
+			tc := testCase{
+				desctiption: "with image containing no layers",
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			c := &CachingCopyCommand{
+				img: fakeImage{
+					ImageLayers: []v1.Layer{
+						fakeLayer{},
+					},
+				},
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc := testCase{
+				desctiption: "with image one layer which has no tar content",
+				expectErr:   false, // this one probably should fail but doesn't because of how ExecuteCommand and util.GetFSFromLayers are implemented - cvgw- 2019-11-25
+				expectLayer: true,
+			}
+			tc.command = c
+			return tc
+		}(),
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desctiption, func(t *testing.T) {
+			c := tc.command
+			err := c.ExecuteCommand(config, buildArgs)
+			if !tc.expectErr && err != nil {
+				t.Errorf("Expected err to be nil but was %v", err)
+			} else if tc.expectErr && err == nil {
+				t.Error("Expected err but was nil")
+			}
+
+			if tc.count != nil {
+				if *tc.count != tc.expectedCount {
+					t.Errorf("Expected extractFn to be called %v times but was called %v times", tc.expectedCount, *tc.count)
+				}
+				for _, file := range tc.extractedFiles {
+					match := false
+					cFiles := c.FilesToSnapshot()
+					for _, cFile := range cFiles {
+						if file == cFile {
+							match = true
+							break
+						}
+					}
+					if !match {
+						t.Errorf("Expected extracted files to include %v but did not %v", file, cFiles)
+					}
+				}
+				// CachingCopyCommand does not override BaseCommand
+				// FilesUseFromContext so this will always return an empty slice and no error
+				// This seems like it might be a bug as it results in CopyCommands and CachingCopyCommands generating different cache keys - cvgw - 2019-11-27
+				cmdFiles, err := c.FilesUsedFromContext(
+					config, buildArgs,
+				)
+				if err != nil {
+					t.Errorf("failed to get files used from context from command")
+				}
+				if len(cmdFiles) != 0 {
+					t.Errorf("expected files used from context to be empty but was not")
+				}
+			}
+
+		})
+	}
+}

pkg/commands/fake_commands.go

@@ -0,0 +1,88 @@
+/*
+Copyright 2018 Google LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// used for testing in the commands package
+package commands
+
+import (
+	"bytes"
+	"io"
+	"io/ioutil"
+
+	v1 "github.com/google/go-containerregistry/pkg/v1"
+	"github.com/google/go-containerregistry/pkg/v1/types"
+)
+
+type fakeLayer struct {
+	TarContent []byte
+}
+
+func (f fakeLayer) Digest() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeLayer) DiffID() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeLayer) Compressed() (io.ReadCloser, error) {
+	return nil, nil
+}
+func (f fakeLayer) Uncompressed() (io.ReadCloser, error) {
+	return ioutil.NopCloser(bytes.NewReader(f.TarContent)), nil
+}
+func (f fakeLayer) Size() (int64, error) {
+	return 0, nil
+}
+func (f fakeLayer) MediaType() (types.MediaType, error) {
+	return "", nil
+}
+
+type fakeImage struct {
+	ImageLayers []v1.Layer
+}
+
+func (f fakeImage) Layers() ([]v1.Layer, error) {
+	return f.ImageLayers, nil
+}
+func (f fakeImage) MediaType() (types.MediaType, error) {
+	return "", nil
+}
+func (f fakeImage) Size() (int64, error) {
+	return 0, nil
+}
+func (f fakeImage) ConfigName() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeImage) ConfigFile() (*v1.ConfigFile, error) {
+	return &v1.ConfigFile{}, nil
+}
+func (f fakeImage) RawConfigFile() ([]byte, error) {
+	return []byte{}, nil
+}
+func (f fakeImage) Digest() (v1.Hash, error) {
+	return v1.Hash{}, nil
+}
+func (f fakeImage) Manifest() (*v1.Manifest, error) {
+	return &v1.Manifest{}, nil
+}
+func (f fakeImage) RawManifest() ([]byte, error) {
+	return []byte{}, nil
+}
+func (f fakeImage) LayerByDigest(v1.Hash) (v1.Layer, error) {
+	return fakeLayer{}, nil
+}
+func (f fakeImage) LayerByDiffID(v1.Hash) (v1.Layer, error) {
+	return fakeLayer{}, nil
+}

pkg/commands/run.go

@@ -164,8 +164,9 @@ func (r *RunCommand) FilesToSnapshot() []string {
 func (r *RunCommand) CacheCommand(img v1.Image) DockerCommand {
 
 	return &CachingRunCommand{
-		img: img,
-		cmd: r.cmd,
+		img:       img,
+		cmd:       r.cmd,
+		extractFn: util.ExtractFile,
 	}
 }
 
@@ -186,15 +187,21 @@ type CachingRunCommand struct {
 	img            v1.Image
 	extractedFiles []string
 	cmd            *instructions.RunCommand
+	extractFn      util.ExtractFunction
 }
 
 func (cr *CachingRunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error {
 	logrus.Infof("Found cached layer, extracting to filesystem")
 	var err error
-	cr.extractedFiles, err = util.GetFSFromImage(constants.RootDir, cr.img)
+
+	if cr.img == nil {
+		return errors.New(fmt.Sprintf("command image is nil %v", cr.String()))
+	}
+	cr.extractedFiles, err = util.GetFSFromImage(constants.RootDir, cr.img, cr.extractFn)
 	if err != nil {
 		return errors.Wrap(err, "extracting fs from image")
 	}
+
 	return nil
 }
 
@@ -203,5 +210,8 @@ func (cr *CachingRunCommand) FilesToSnapshot() []string {
 }
 
 func (cr *CachingRunCommand) String() string {
+	if cr.cmd == nil {
+		return "nil command"
+	}
 	return cr.cmd.String()
 }

pkg/commands/run_test.go

@@ -16,10 +16,19 @@ limitations under the License.
 package commands
 
 import (
+	"archive/tar"
+	"bytes"
+	"io"
+	"io/ioutil"
+	"log"
+	"os"
 	"os/user"
+	"path/filepath"
 	"testing"
 
+	"github.com/GoogleContainerTools/kaniko/pkg/dockerfile"
 	"github.com/GoogleContainerTools/kaniko/testutil"
+	v1 "github.com/google/go-containerregistry/pkg/v1"
 )
 
 func Test_addDefaultHOME(t *testing.T) {
@@ -120,3 +129,187 @@ func Test_addDefaultHOME(t *testing.T) {
 		})
 	}
 }
+
+func prepareTarFixture(fileNames []string) ([]byte, error) {
+	dir, err := ioutil.TempDir("/tmp", "tar-fixture")
+	if err != nil {
+		return nil, err
+	}
+
+	content := `
+Meow meow meow meow
+meow meow meow meow
+`
+	for _, name := range fileNames {
+		if err := ioutil.WriteFile(filepath.Join(dir, name), []byte(content), 0777); err != nil {
+			return nil, err
+		}
+	}
+	writer := bytes.NewBuffer([]byte{})
+	tw := tar.NewWriter(writer)
+	defer tw.Close()
+	filepath.Walk(dir, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if info.IsDir() {
+			return nil
+		}
+
+		hdr, err := tar.FileInfoHeader(info, "")
+		if err != nil {
+			return err
+		}
+		if err := tw.WriteHeader(hdr); err != nil {
+			log.Fatal(err)
+		}
+		body, err := ioutil.ReadFile(path)
+		if err != nil {
+			return err
+		}
+		if _, err := tw.Write(body); err != nil {
+			log.Fatal(err)
+		}
+
+		return nil
+	})
+
+	return writer.Bytes(), nil
+}
+
+func Test_CachingRunCommand_ExecuteCommand(t *testing.T) {
+	tarContent, err := prepareTarFixture([]string{"foo.txt"})
+	if err != nil {
+		t.Errorf("couldn't prepare tar fixture %v", err)
+	}
+
+	config := &v1.Config{}
+	buildArgs := &dockerfile.BuildArgs{}
+
+	type testCase struct {
+		desctiption    string
+		expectLayer    bool
+		expectErr      bool
+		count          *int
+		expectedCount  int
+		command        *CachingRunCommand
+		extractedFiles []string
+		contextFiles   []string
+	}
+	testCases := []testCase{
+		func() testCase {
+			c := &CachingRunCommand{
+				img: fakeImage{
+					ImageLayers: []v1.Layer{
+						fakeLayer{TarContent: tarContent},
+					},
+				},
+			}
+			count := 0
+			tc := testCase{
+				desctiption:    "with valid image and valid layer",
+				count:          &count,
+				expectedCount:  1,
+				expectLayer:    true,
+				extractedFiles: []string{"/foo.txt"},
+				contextFiles:   []string{"foo.txt"},
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				*tc.count++
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			c := &CachingRunCommand{}
+			tc := testCase{
+				desctiption: "with no image",
+				expectErr:   true,
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			c := &CachingRunCommand{
+				img: fakeImage{},
+			}
+			tc := testCase{
+				desctiption: "with image containing no layers",
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc.command = c
+			return tc
+		}(),
+		func() testCase {
+			c := &CachingRunCommand{
+				img: fakeImage{
+					ImageLayers: []v1.Layer{
+						fakeLayer{},
+					},
+				},
+			}
+			c.extractFn = func(_ string, _ *tar.Header, _ io.Reader) error {
+				return nil
+			}
+			tc := testCase{
+				desctiption: "with image one layer which has no tar content",
+				expectErr:   false, // this one probably should fail but doesn't because of how ExecuteCommand and util.GetFSFromLayers are implemented - cvgw- 2019-11-25
+				expectLayer: true,
+			}
+			tc.command = c
+			return tc
+		}(),
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desctiption, func(t *testing.T) {
+			c := tc.command
+			err := c.ExecuteCommand(config, buildArgs)
+			if !tc.expectErr && err != nil {
+				t.Errorf("Expected err to be nil but was %v", err)
+			} else if tc.expectErr && err == nil {
+				t.Error("Expected err but was nil")
+			}
+
+			if tc.count != nil {
+				if *tc.count != tc.expectedCount {
+					t.Errorf("Expected extractFn to be called %v times but was called %v times", 1, *tc.count)
+				}
+				for _, file := range tc.extractedFiles {
+					match := false
+					cmdFiles := c.extractedFiles
+					for _, f := range cmdFiles {
+						if file == f {
+							match = true
+							break
+						}
+					}
+					if !match {
+						t.Errorf("Expected extracted files to include %v but did not %v", file, cmdFiles)
+					}
+				}
+
+				// CachingRunCommand does not override BaseCommand
+				// FilesUseFromContext so this will always return an empty slice and no error
+				// This seems like it might be a bug as it results in RunCommands and CachingRunCommands generating different cache keys - cvgw - 2019-11-27
+				cmdFiles, err := c.FilesUsedFromContext(
+					config, buildArgs,
+				)
+				if err != nil {
+					t.Errorf("failed to get files used from context from command")
+				}
+
+				if len(cmdFiles) != 0 {
+					t.Errorf("expected files used from context to be empty but was not")
+				}
+			}
+		})
+	}
+}

pkg/executor/build.go

@@ -211,10 +211,13 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro
 		if err != nil {
 			return errors.Wrap(err, "failed to hash composite key")
 		}
+
 		logrus.Debugf("optimize: cache key for command %v %v", command.String(), ck)
 		s.finalCacheKey = ck
+
 		if command.ShouldCacheOutput() && !stopCache {
 			img, err := s.layerCache.RetrieveLayer(ck)
+
 			if err != nil {
 				logrus.Debugf("Failed to retrieve layer: %s", err)
 				logrus.Infof("No cached layer found for cmd %s", command.String())
@@ -247,6 +250,7 @@ func (s *stageBuilder) build() error {
 	} else {
 		compositeKey = NewCompositeCache(s.baseImageDigest)
 	}
+
 	compositeKey.AddKey(s.opts.BuildArgs...)
 
 	// Apply optimizations to the instructions.
@@ -269,21 +273,26 @@ func (s *stageBuilder) build() error {
 
 	if shouldUnpack {
 		t := timing.Start("FS Unpacking")
-		if _, err := util.GetFSFromImage(constants.RootDir, s.image); err != nil {
+
+		if _, err := util.GetFSFromImage(constants.RootDir, s.image, util.ExtractFile); err != nil {
 			return errors.Wrap(err, "failed to get filesystem from image")
 		}
+
 		timing.DefaultRun.Stop(t)
 	} else {
 		logrus.Info("Skipping unpacking as no commands require it.")
 	}
+
 	if err := util.DetectFilesystemWhitelist(constants.WhitelistPath); err != nil {
 		return errors.Wrap(err, "failed to check filesystem whitelist")
 	}
+
 	// Take initial snapshot
 	t := timing.Start("Initial FS snapshot")
 	if err := s.snapshotter.Init(); err != nil {
 		return err
 	}
+
 	timing.DefaultRun.Stop(t)
 
 	cacheGroup := errgroup.Group{}
@@ -327,6 +336,9 @@ func (s *stageBuilder) build() error {
 		if err != nil {
 			return errors.Wrap(err, "failed to hash composite key")
 		}
+
+		logrus.Debugf("build: cache key for command %v %v", command.String(), ck)
+
 		// Push layer to cache (in parallel) now along with new config file
 		if s.opts.Cache && command.ShouldCacheOutput() {
 			cacheGroup.Go(func() error {
@@ -641,7 +653,7 @@ func extractImageToDependencyDir(name string, image v1.Image) error {
 		return err
 	}
 	logrus.Debugf("trying to extract to %s", dependencyDir)
-	_, err := util.GetFSFromImage(dependencyDir, image)
+	_, err := util.GetFSFromImage(dependencyDir, image, util.ExtractFile)
 	return err
 }
 

pkg/util/fs_util.go

@@ -69,9 +69,17 @@ var volumes = []string{}
 
 var excluded []string
 
+type ExtractFunction func(string, *tar.Header, io.Reader) error
+
 // GetFSFromImage extracts the layers of img to root
 // It returns a list of all files extracted
-func GetFSFromImage(root string, img v1.Image) ([]string, error) {
+func GetFSFromImage(root string, img v1.Image, extract ExtractFunction) ([]string, error) {
+	if extract == nil {
+		return nil, errors.New("must supply an extract function")
+	}
+	if img == nil {
+		return nil, errors.New("image cannot be nil")
+	}
 	if err := DetectFilesystemWhitelist(constants.WhitelistPath); err != nil {
 		return nil, err
 	}
@@ -114,7 +122,7 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) {
 				}
 				continue
 			}
-			if err := extractFile(root, hdr, tr); err != nil {
+			if err := extract(root, hdr, tr); err != nil {
 				return nil, err
 			}
 			extractedFiles = append(extractedFiles, filepath.Join(root, filepath.Clean(hdr.Name)))
@@ -179,7 +187,7 @@ func unTar(r io.Reader, dest string) ([]string, error) {
 		if err != nil {
 			return nil, err
 		}
-		if err := extractFile(dest, hdr, tr); err != nil {
+		if err := ExtractFile(dest, hdr, tr); err != nil {
 			return nil, err
 		}
 		extractedFiles = append(extractedFiles, dest)
@@ -187,7 +195,7 @@ func unTar(r io.Reader, dest string) ([]string, error) {
 	return extractedFiles, nil
 }
 
-func extractFile(dest string, hdr *tar.Header, tr io.Reader) error {
+func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error {
 	path := filepath.Join(dest, filepath.Clean(hdr.Name))
 	base := filepath.Base(path)
 	dir := filepath.Dir(path)

pkg/util/fs_util_test.go

@@ -659,7 +659,7 @@ func TestExtractFile(t *testing.T) {
 			defer os.RemoveAll(r)
 
 			for _, hdr := range tc.hdrs {
-				if err := extractFile(r, hdr, bytes.NewReader(tc.contents)); err != nil {
+				if err := ExtractFile(r, hdr, bytes.NewReader(tc.contents)); err != nil {
 					t.Fatal(err)
 				}
 			}