From c520218cec1956ac2ccbb3e7b51a999db70fcb63 Mon Sep 17 00:00:00 2001 From: Tom Prince Date: Mon, 13 Apr 2020 16:50:13 -0600 Subject: [PATCH] Don't generate cache key, if not caching builds. The cache key generation does environment subsitution in places that running the commands doesn't. This causes issues if a command uses complex shell substitutions. The cache key is generated even if caching isn't enabled. This disables the cache key generation if caching is not enabled. This doesn't fix the underlying issue, but limits it to when the cache is being used. --- .../Dockerfile_test_complex_substitution | 2 ++ pkg/executor/build.go | 32 +++++++++++-------- 2 files changed, 20 insertions(+), 14 deletions(-) create mode 100644 integration/dockerfiles/Dockerfile_test_complex_substitution diff --git a/integration/dockerfiles/Dockerfile_test_complex_substitution b/integration/dockerfiles/Dockerfile_test_complex_substitution new file mode 100644 index 000000000..9db50545f --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_complex_substitution @@ -0,0 +1,2 @@ +FROM docker.io/library/busybox:latest@sha256:afe605d272837ce1732f390966166c2afff5391208ddd57de10942748694049d +RUN echo ${s%s} diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 9ccd969e5..f05c34528 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -334,9 +334,11 @@ func (s *stageBuilder) build() error { return errors.Wrap(err, "failed to get files used from context") } - *compositeKey, err = s.populateCompositeKey(command, files, *compositeKey, s.args, s.cf.Config.Env) - if err != nil { - return err + if s.opts.Cache { + *compositeKey, err = s.populateCompositeKey(command, files, *compositeKey, s.args, s.cf.Config.Env) + if err != nil && s.opts.Cache { + return err + } } logrus.Info(command.String()) @@ -372,19 +374,21 @@ func (s *stageBuilder) build() error { return errors.Wrap(err, "failed to take snapshot") } - logrus.Debugf("build: composite key for command %v %v", command.String(), compositeKey) - ck, err := compositeKey.Hash() - if err != nil { - return errors.Wrap(err, "failed to hash composite key") - } + if s.opts.Cache { + logrus.Debugf("build: composite key for command %v %v", command.String(), compositeKey) + ck, err := compositeKey.Hash() + if err != nil { + return errors.Wrap(err, "failed to hash composite key") + } - logrus.Debugf("build: cache key for command %v %v", command.String(), ck) + 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 { - return s.pushLayerToCache(s.opts, ck, tarPath, command.String()) - }) + // Push layer to cache (in parallel) now along with new config file + if command.ShouldCacheOutput() { + cacheGroup.Go(func() error { + return s.pushLayerToCache(s.opts, ck, tarPath, command.String()) + }) + } } if err := s.saveSnapshotToImage(command.String(), tarPath); err != nil { return errors.Wrap(err, "failed to save snapshot to image")