From ee2c62196c2fa38e74c921290729e9dc9d170bb9 Mon Sep 17 00:00:00 2001
From: Johannes 'fish' Ziemke <github@freigeist.org>
Date: Mon, 1 Apr 2019 13:00:57 +0200
Subject: [PATCH 1/3] Revert "Change cache key calculation to be more
 reproducible. (#525)"

This reverts commit 1ffae47fdd91d38006c1d28e14e66869742cbdba.
---
 integration/dockerfiles/Dockerfile_test_cache | 14 ++-------
 pkg/executor/build.go                         |  7 +----
 pkg/util/fs_util.go                           | 20 +++----------
 pkg/util/tar_util.go                          |  1 -
 pkg/util/util.go                              | 29 -------------------
 5 files changed, 8 insertions(+), 63 deletions(-)

diff --git a/integration/dockerfiles/Dockerfile_test_cache b/integration/dockerfiles/Dockerfile_test_cache
index 71644d560a..e3ebe304dc 100644
--- a/integration/dockerfiles/Dockerfile_test_cache
+++ b/integration/dockerfiles/Dockerfile_test_cache
@@ -16,15 +16,7 @@
 # If the image is built twice, /date should be the same in both images
 # if the cache is implemented correctly
 
-FROM alpine as base_stage
-
-RUN mkdir foo && echo base_stage > foo/base
-
-FROM base_stage as cached_stage
-
-RUN echo cached_stage > foo/cache
-
-FROM cached_stage as bug_stage
-
-RUN echo bug_stage > foo/bug
+FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0
 RUN date > /date
+COPY context/foo /foo
+RUN echo hey
diff --git a/pkg/executor/build.go b/pkg/executor/build.go
index 2fbf592cef..c4bdb0f4de 100644
--- a/pkg/executor/build.go
+++ b/pkg/executor/build.go
@@ -168,7 +168,6 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro
 			if err != nil {
 				logrus.Debugf("Failed to retrieve layer: %s", err)
 				logrus.Infof("No cached layer found for cmd %s", command.String())
-				logrus.Debugf("Key missing was: %s", compositeKey.Key())
 				break
 			}
 
@@ -190,11 +189,7 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro
 
 func (s *stageBuilder) build() error {
 	// Set the initial cache key to be the base image digest, the build args and the SrcContext.
-	dgst, err := util.ReproducibleDigest(s.image)
-	if err != nil {
-		return err
-	}
-	compositeKey := NewCompositeCache(dgst)
+	compositeKey := NewCompositeCache(s.baseImageDigest)
 	compositeKey.AddKey(s.opts.BuildArgs...)
 
 	// Apply optimizations to the instructions.
diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go
index 5ef48845fc..5f72c7784e 100644
--- a/pkg/util/fs_util.go
+++ b/pkg/util/fs_util.go
@@ -79,10 +79,7 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) {
 	if err != nil {
 		return nil, err
 	}
-
-	// Store a map of files to their mtime. We need to set mtimes in a second pass because creating files
-	// can change the mtime of a directory.
-	extractedFiles := map[string]time.Time{}
+	extractedFiles := []string{}
 
 	for i, l := range layers {
 		logrus.Debugf("Extracting layer %d", i)
@@ -113,17 +110,10 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) {
 			if err := extractFile(root, hdr, tr); err != nil {
 				return nil, err
 			}
-			extractedFiles[filepath.Join(root, filepath.Clean(hdr.Name))] = hdr.ModTime
+			extractedFiles = append(extractedFiles, filepath.Join(root, filepath.Clean(hdr.Name)))
 		}
 	}
-
-	fileNames := []string{}
-	for f, t := range extractedFiles {
-		fileNames = append(fileNames, f)
-		os.Chtimes(f, time.Time{}, t)
-	}
-
-	return fileNames, nil
+	return extractedFiles, nil
 }
 
 // DeleteFilesystem deletes the extracted image file system
@@ -272,7 +262,6 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error {
 			return err
 		}
 	}
-
 	return nil
 }
 
@@ -377,8 +366,7 @@ func RelativeFiles(fp string, root string) ([]string, error) {
 }
 
 // ParentDirectories returns a list of paths to all parent directories
-// Ex. /some/temp/dir -> [/some, /some/temp, /some/temp/dir]
-// This purposefully excludes the /.
+// Ex. /some/temp/dir -> [/, /some, /some/temp, /some/temp/dir]
 func ParentDirectories(path string) []string {
 	path = filepath.Clean(path)
 	dirs := strings.Split(path, "/")
diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go
index b727857075..bc1cc67a0c 100644
--- a/pkg/util/tar_util.go
+++ b/pkg/util/tar_util.go
@@ -54,7 +54,6 @@ func (t *Tar) Close() {
 
 // AddFileToTar adds the file at path p to the tar
 func (t *Tar) AddFileToTar(p string) error {
-	logrus.Debugf("Adding file %s to tar", p)
 	i, err := os.Lstat(p)
 	if err != nil {
 		return fmt.Errorf("Failed to get file info for %s: %s", p, err)
diff --git a/pkg/util/util.go b/pkg/util/util.go
index e7344c88fc..873cbae20e 100644
--- a/pkg/util/util.go
+++ b/pkg/util/util.go
@@ -20,14 +20,11 @@ import (
 	"crypto/md5"
 	"crypto/sha256"
 	"encoding/hex"
-	"encoding/json"
 	"io"
 	"os"
 	"strconv"
 	"syscall"
 
-	"github.com/google/go-containerregistry/pkg/v1"
-	"github.com/google/go-containerregistry/pkg/v1/partial"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -130,29 +127,3 @@ func SHA256(r io.Reader) (string, error) {
 	}
 	return hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), nil
 }
-
-type ReproducibleManifest struct {
-	Layers []v1.Descriptor
-	Config v1.Config
-}
-
-func ReproducibleDigest(img partial.WithManifestAndConfigFile) (string, error) {
-	mfst, err := img.Manifest()
-	if err != nil {
-		return "", err
-	}
-	cfg, err := img.ConfigFile()
-	if err != nil {
-		return "", err
-	}
-	rm := ReproducibleManifest{
-		Layers: mfst.Layers,
-		Config: cfg.Config,
-	}
-
-	b, err := json.Marshal(rm)
-	if err != nil {
-		return "", err
-	}
-	return string(b), nil
-}

From 5fd83500b2bfaa2077dd401ccbc99fd6361bccf7 Mon Sep 17 00:00:00 2001
From: Johannes 'fish' Ziemke <github@freigeist.org>
Date: Mon, 1 Apr 2019 19:52:13 +0200
Subject: [PATCH 2/3] Add logging of composition key back

---
 pkg/executor/build.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pkg/executor/build.go b/pkg/executor/build.go
index c4bdb0f4de..7c84b6ce45 100644
--- a/pkg/executor/build.go
+++ b/pkg/executor/build.go
@@ -168,6 +168,7 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro
 			if err != nil {
 				logrus.Debugf("Failed to retrieve layer: %s", err)
 				logrus.Infof("No cached layer found for cmd %s", command.String())
+				logrus.Debugf("Key missing was: %s", compositeKey.Key())
 				break
 			}
 

From 48a0dcf021068228f2fb796d60172c4044c7a539 Mon Sep 17 00:00:00 2001
From: Johannes 'fish' Ziemke <github@freigeist.org>
Date: Tue, 2 Apr 2019 10:40:00 +0200
Subject: [PATCH 3/3] Do not include build args in cache key

This should be save, given that the commands will have the args included
when the cache key gets built.
---
 pkg/executor/build.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pkg/executor/build.go b/pkg/executor/build.go
index 7c84b6ce45..c29de8ca1b 100644
--- a/pkg/executor/build.go
+++ b/pkg/executor/build.go
@@ -191,7 +191,6 @@ func (s *stageBuilder) optimize(compositeKey CompositeCache, cfg v1.Config) erro
 func (s *stageBuilder) build() error {
 	// Set the initial cache key to be the base image digest, the build args and the SrcContext.
 	compositeKey := NewCompositeCache(s.baseImageDigest)
-	compositeKey.AddKey(s.opts.BuildArgs...)
 
 	// Apply optimizations to the instructions.
 	if err := s.optimize(*compositeKey, s.cf.Config); err != nil {