From 64a0b1d75f548f4631de553cc1f075f3651d0206 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 23 Aug 2018 16:23:59 -0700 Subject: [PATCH 01/35] Added a KanikoStage type for each stage of a Dockerfile I added a KanikoStage to hold each stage of the Dockerfile along with information about each stage that would be useful later on. The new KanikoStage type holds the stage itself, along with some additional information: 1. FinalStage -- whether the current stage is the final stage 2. BaseImageStoredLocally/BaseImageIndex -- whether the base image for this stage is stored locally, and if so what the index of the base image is 3. SaveStage -- whether this stage needs to be saved for use in a future stage This is the first part of a larger refactor for building stages, which will later make it easier to add layer caching. --- cmd/executor/cmd/root.go | 4 +- pkg/{options => config}/args.go | 2 +- pkg/{options => config}/options.go | 2 +- pkg/config/stage.go | 28 +++++++ pkg/dockerfile/dockerfile.go | 70 ++++++++++++---- pkg/dockerfile/dockerfile_test.go | 129 ++++++++++++++++------------- pkg/executor/build.go | 28 ++----- pkg/executor/push.go | 4 +- pkg/util/image_util.go | 17 ++-- pkg/util/image_util_test.go | 15 +++- testutil/constants.go | 47 +++++++++++ 11 files changed, 231 insertions(+), 115 deletions(-) rename pkg/{options => config}/args.go (98%) rename pkg/{options => config}/options.go (98%) create mode 100644 pkg/config/stage.go create mode 100644 testutil/constants.go diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index c05c21b67..1baf8ff16 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -22,9 +22,9 @@ import ( "strings" "github.com/GoogleContainerTools/kaniko/pkg/buildcontext" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/GoogleContainerTools/kaniko/pkg/executor" - "github.com/GoogleContainerTools/kaniko/pkg/options" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/genuinetools/amicontained/container" "github.com/pkg/errors" @@ -33,7 +33,7 @@ import ( ) var ( - opts = &options.KanikoOptions{} + opts = &config.KanikoOptions{} logLevel string force bool ) diff --git a/pkg/options/args.go b/pkg/config/args.go similarity index 98% rename from pkg/options/args.go rename to pkg/config/args.go index 5f88157c9..ae45b266c 100644 --- a/pkg/options/args.go +++ b/pkg/config/args.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package options +package config import ( "strings" diff --git a/pkg/options/options.go b/pkg/config/options.go similarity index 98% rename from pkg/options/options.go rename to pkg/config/options.go index 9f9d59354..d438d2479 100644 --- a/pkg/options/options.go +++ b/pkg/config/options.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package options +package config // KanikoOptions are options that are set by command line arguments type KanikoOptions struct { diff --git a/pkg/config/stage.go b/pkg/config/stage.go new file mode 100644 index 000000000..3acfdc409 --- /dev/null +++ b/pkg/config/stage.go @@ -0,0 +1,28 @@ +/* +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. +*/ + +package config + +import "github.com/moby/buildkit/frontend/dockerfile/instructions" + +// KanikoStage wraps a stage of the Dockerfile and provides extra information +type KanikoStage struct { + instructions.Stage + FinalStage bool + BaseImageStoredLocally bool + BaseImageIndex int + SaveStage bool +} diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index cf8ec9960..5f37a16cf 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -23,26 +23,61 @@ import ( "strconv" "strings" + "github.com/GoogleContainerTools/kaniko/pkg/config" + "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/moby/buildkit/frontend/dockerfile/parser" + "github.com/pkg/errors" ) -// Stages reads the Dockerfile, validates it's contents, and returns stages -func Stages(dockerfilePath, target string) ([]instructions.Stage, error) { - d, err := ioutil.ReadFile(dockerfilePath) +// Stages parses a Dockerfile and returns an array of KanikoStage +func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) { + d, err := ioutil.ReadFile(opts.DockerfilePath) if err != nil { - return nil, err + return nil, errors.Wrap(err, fmt.Sprintf("reading dockerfile at path %s", opts.DockerfilePath)) } - stages, err := Parse(d) + if err != nil { + return nil, errors.Wrap(err, "parsing dockerfile") + } + targetStage, err := targetStage(stages, opts.Target) if err != nil { return nil, err } - if err := ValidateTarget(stages, target); err != nil { - return nil, err + resolveStages(stages) + var kanikoStages []config.KanikoStage + for index, stage := range stages { + resolvedBaseName, err := util.ResolveEnvironmentReplacement(stage.BaseName, opts.BuildArgs, false) + if err != nil { + return nil, errors.Wrap(err, "resolving base name") + } + stage.Name = resolvedBaseName + kanikoStages = append(kanikoStages, config.KanikoStage{ + Stage: stage, + BaseImageIndex: baseImageIndex(opts, index, stages), + BaseImageStoredLocally: (baseImageIndex(opts, index, stages) != -1), + SaveStage: saveStage(index, stages), + FinalStage: index == targetStage, + }) + if index == targetStage { + break + } } - ResolveStages(stages) - return stages, nil + return kanikoStages, nil +} + +// baseImageIndex returns the index of the stage the current stage is built off +// returns -1 if the current stage isn't built off a previous stage +func baseImageIndex(opts *config.KanikoOptions, currentStage int, stages []instructions.Stage) int { + for i, stage := range stages { + if i > currentStage { + break + } + if stage.Name == stages[currentStage].BaseName { + return i + } + } + return -1 } // Parse parses the contents of a Dockerfile and returns a list of commands @@ -58,21 +93,22 @@ func Parse(b []byte) ([]instructions.Stage, error) { return stages, err } -func ValidateTarget(stages []instructions.Stage, target string) error { +// targetStage returns the index of the target stage kaniko is trying to build +func targetStage(stages []instructions.Stage, target string) (int, error) { if target == "" { - return nil + return len(stages) - 1, nil } - for _, stage := range stages { + for i, stage := range stages { if stage.Name == target { - return nil + return i, nil } } - return fmt.Errorf("%s is not a valid target build stage", target) + return -1, fmt.Errorf("%s is not a valid target build stage", target) } -// ResolveStages resolves any calls to previous stages with names to indices +// resolveStages resolves any calls to previous stages with names to indices // Ex. --from=second_stage should be --from=1 for easier processing later on -func ResolveStages(stages []instructions.Stage) { +func resolveStages(stages []instructions.Stage) { nameToIndex := make(map[string]string) for i, stage := range stages { index := strconv.Itoa(i) @@ -111,7 +147,7 @@ func ParseCommands(cmdArray []string) ([]instructions.Command, error) { } // SaveStage returns true if the current stage will be needed later in the Dockerfile -func SaveStage(index int, stages []instructions.Stage) bool { +func saveStage(index int, stages []instructions.Stage) bool { for stageIndex, stage := range stages { if stageIndex <= index { continue diff --git a/pkg/dockerfile/dockerfile_test.go b/pkg/dockerfile/dockerfile_test.go index d285f25a7..bd09c26ba 100644 --- a/pkg/dockerfile/dockerfile_test.go +++ b/pkg/dockerfile/dockerfile_test.go @@ -17,17 +17,15 @@ limitations under the License. package dockerfile import ( - "io/ioutil" - "os" - "path/filepath" "strconv" "testing" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/testutil" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) -func Test_ResolveStages(t *testing.T) { +func Test_resolveStages(t *testing.T) { dockerfile := ` FROM scratch RUN echo hi > /hi @@ -42,7 +40,7 @@ func Test_ResolveStages(t *testing.T) { if err != nil { t.Fatal(err) } - ResolveStages(stages) + resolveStages(stages) for index, stage := range stages { if index == 0 { continue @@ -55,7 +53,7 @@ func Test_ResolveStages(t *testing.T) { } } -func Test_ValidateTarget(t *testing.T) { +func Test_targetStage(t *testing.T) { dockerfile := ` FROM scratch RUN echo hi > /hi @@ -71,70 +69,44 @@ func Test_ValidateTarget(t *testing.T) { t.Fatal(err) } tests := []struct { - name string - target string - shouldErr bool + name string + target string + targetIndex int + shouldErr bool }{ { - name: "test valid target", - target: "second", - shouldErr: false, + name: "test valid target", + target: "second", + targetIndex: 1, + shouldErr: false, }, { - name: "test invalid target", - target: "invalid", - shouldErr: true, + name: "test no target", + target: "", + targetIndex: 2, + shouldErr: false, + }, + { + name: "test invalid target", + target: "invalid", + targetIndex: -1, + shouldErr: true, }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - actualErr := ValidateTarget(stages, test.target) - testutil.CheckError(t, test.shouldErr, actualErr) + target, err := targetStage(stages, test.target) + testutil.CheckError(t, test.shouldErr, err) + if !test.shouldErr { + if target != test.targetIndex { + t.Errorf("got incorrect target, expected %d got %d", test.targetIndex, target) + } + } }) } } func Test_SaveStage(t *testing.T) { - tempDir, err := ioutil.TempDir("", "") - if err != nil { - t.Fatalf("couldn't create temp dir: %v", err) - } - defer os.RemoveAll(tempDir) - files := map[string]string{ - "Dockerfile": ` - FROM scratch - RUN echo hi > /hi - - FROM scratch AS second - COPY --from=0 /hi /hi2 - - FROM second - RUN xxx - - FROM scratch - COPY --from=second /hi2 /hi3 - - FROM ubuntu:16.04 AS base - ENV DEBIAN_FRONTEND noninteractive - ENV LC_ALL C.UTF-8 - - FROM base AS development - ENV PS1 " ๐Ÿณ \[\033[1;36m\]\W\[\033[0;35m\] # \[\033[0m\]" - - FROM development AS test - ENV ORG_ENV UnitTest - - FROM base AS production - COPY . /code - `, - } - if err := testutil.SetupFiles(tempDir, files); err != nil { - t.Fatalf("couldn't create dockerfile: %v", err) - } - stages, err := Stages(filepath.Join(tempDir, "Dockerfile"), "") - if err != nil { - t.Fatalf("couldn't retrieve stages from Dockerfile: %v", err) - } tests := []struct { name string index int @@ -171,10 +143,51 @@ func Test_SaveStage(t *testing.T) { expected: false, }, } + stages, err := Parse([]byte(testutil.Dockerfile)) + if err != nil { + t.Fatalf("couldn't retrieve stages from Dockerfile: %v", err) + } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - actual := SaveStage(test.index, stages) + actual := saveStage(test.index, stages) testutil.CheckErrorAndDeepEqual(t, false, nil, test.expected, actual) }) } } + +func Test_baseImageIndex(t *testing.T) { + tests := []struct { + name string + currentStage int + expected int + }{ + { + name: "stage that is built off of a previous stage", + currentStage: 2, + expected: 1, + }, + { + name: "another stage that is built off of a previous stage", + currentStage: 5, + expected: 4, + }, + { + name: "stage that isn't built off of a previous stage", + currentStage: 4, + expected: -1, + }, + } + + stages, err := Parse([]byte(testutil.Dockerfile)) + if err != nil { + t.Fatalf("couldn't retrieve stages from Dockerfile: %v", err) + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual := baseImageIndex(&config.KanikoOptions{}, test.currentStage, stages) + if actual != test.expected { + t.Fatalf("unexpected result, expected %d got %d", test.expected, actual) + } + }) + } +} diff --git a/pkg/executor/build.go b/pkg/executor/build.go index a3f958e62..21544da6d 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -29,20 +29,19 @@ import ( "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/tarball" - "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/kaniko/pkg/commands" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" - "github.com/GoogleContainerTools/kaniko/pkg/options" "github.com/GoogleContainerTools/kaniko/pkg/snapshot" "github.com/GoogleContainerTools/kaniko/pkg/util" ) -func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { +func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { // Parse dockerfile and unpack base image to root - stages, err := dockerfile.Stages(opts.DockerfilePath, opts.Target) + stages, err := dockerfile.Stages(opts) if err != nil { return nil, err } @@ -52,9 +51,8 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { return nil, err } for index, stage := range stages { - finalStage := finalStage(index, opts.Target, stages) // Unpack file system to root - sourceImage, err := util.RetrieveSourceImage(index, opts.BuildArgs, stages) + sourceImage, err := util.RetrieveSourceImage(stage, opts.BuildArgs) if err != nil { return nil, err } @@ -89,7 +87,7 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { } // Don't snapshot if it's not the final stage and not the final command // Also don't snapshot if it's the final stage, not the final command, and single snapshot is set - if (!finalStage && !finalCmd) || (finalStage && !finalCmd && opts.SingleSnapshot) { + if (!stage.FinalStage && !finalCmd) || (stage.FinalStage && !finalCmd && opts.SingleSnapshot) { continue } // Now, we get the files to snapshot from this command and take the snapshot @@ -131,7 +129,7 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { if err != nil { return nil, err } - if finalStage { + if stage.FinalStage { if opts.Reproducible { sourceImage, err = mutate.Canonical(sourceImage) if err != nil { @@ -140,7 +138,7 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { } return sourceImage, nil } - if dockerfile.SaveStage(index, stages) { + if stage.SaveStage { if err := saveStageAsTarball(index, sourceImage); err != nil { return nil, err } @@ -156,16 +154,6 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { return nil, err } -func finalStage(index int, target string, stages []instructions.Stage) bool { - if index == len(stages)-1 { - return true - } - if target == "" { - return false - } - return target == stages[index].Name -} - func extractImageToDependecyDir(index int, image v1.Image) error { dependencyDir := filepath.Join(constants.KanikoDir, strconv.Itoa(index)) if err := os.MkdirAll(dependencyDir, 0755); err != nil { @@ -199,7 +187,7 @@ func getHasher(snapshotMode string) (func(string) (string, error), error) { return nil, fmt.Errorf("%s is not a valid snapshot mode", snapshotMode) } -func resolveOnBuild(stage *instructions.Stage, config *v1.Config) error { +func resolveOnBuild(stage *config.KanikoStage, config *v1.Config) error { if config.OnBuild == nil { return nil } diff --git a/pkg/executor/push.go b/pkg/executor/push.go index 74bf33b39..e7f9c0610 100644 --- a/pkg/executor/push.go +++ b/pkg/executor/push.go @@ -21,7 +21,7 @@ import ( "fmt" "net/http" - "github.com/GoogleContainerTools/kaniko/pkg/options" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/version" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/authn/k8schain" @@ -43,7 +43,7 @@ func (w *withUserAgent) RoundTrip(r *http.Request) (*http.Response, error) { } // DoPush is responsible for pushing image to the destinations specified in opts -func DoPush(image v1.Image, opts *options.KanikoOptions) error { +func DoPush(image v1.Image, opts *config.KanikoOptions) error { if opts.NoPush { logrus.Info("Skipping push to container registry due to --no-push flag") return nil diff --git a/pkg/util/image_util.go b/pkg/util/image_util.go index 0dd066d55..61a19b5d3 100644 --- a/pkg/util/image_util.go +++ b/pkg/util/image_util.go @@ -27,9 +27,9 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/tarball" - "github.com/moby/buildkit/frontend/dockerfile/instructions" "github.com/sirupsen/logrus" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/constants" ) @@ -40,9 +40,8 @@ var ( ) // RetrieveSourceImage returns the base image of the stage at index -func RetrieveSourceImage(index int, buildArgs []string, stages []instructions.Stage) (v1.Image, error) { - currentStage := stages[index] - currentBaseName, err := ResolveEnvironmentReplacement(currentStage.BaseName, buildArgs, false) +func RetrieveSourceImage(stage config.KanikoStage, buildArgs []string) (v1.Image, error) { + currentBaseName, err := ResolveEnvironmentReplacement(stage.BaseName, buildArgs, false) if err != nil { return nil, err } @@ -53,14 +52,10 @@ func RetrieveSourceImage(index int, buildArgs []string, stages []instructions.St } // Next, check if the base image of the current stage is built from a previous stage // If so, retrieve the image from the stored tarball - for i, stage := range stages { - if i > index { - continue - } - if stage.Name == currentBaseName { - return retrieveTarImage(i) - } + if stage.BaseImageStoredLocally { + return retrieveTarImage(stage.BaseImageIndex) } + // Otherwise, initialize image as usual return retrieveRemoteImage(currentBaseName) } diff --git a/pkg/util/image_util_test.go b/pkg/util/image_util_test.go index 419576d92..dbd7b8ee1 100644 --- a/pkg/util/image_util_test.go +++ b/pkg/util/image_util_test.go @@ -20,6 +20,7 @@ import ( "bytes" "testing" + "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/testutil" "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/empty" @@ -54,7 +55,9 @@ func Test_StandardImage(t *testing.T) { return nil, nil } retrieveRemoteImage = mock - actual, err := RetrieveSourceImage(0, nil, stages) + actual, err := RetrieveSourceImage(config.KanikoStage{ + Stage: stages[0], + }, nil) testutil.CheckErrorAndDeepEqual(t, false, err, nil, actual) } func Test_ScratchImage(t *testing.T) { @@ -62,7 +65,9 @@ func Test_ScratchImage(t *testing.T) { if err != nil { t.Error(err) } - actual, err := RetrieveSourceImage(1, nil, stages) + actual, err := RetrieveSourceImage(config.KanikoStage{ + Stage: stages[1], + }, nil) expected := empty.Image testutil.CheckErrorAndDeepEqual(t, false, err, expected, actual) } @@ -80,7 +85,11 @@ func Test_TarImage(t *testing.T) { return nil, nil } retrieveTarImage = mock - actual, err := RetrieveSourceImage(2, nil, stages) + actual, err := RetrieveSourceImage(config.KanikoStage{ + BaseImageStoredLocally: true, + BaseImageIndex: 0, + Stage: stages[2], + }, nil) testutil.CheckErrorAndDeepEqual(t, false, err, nil, actual) } diff --git a/testutil/constants.go b/testutil/constants.go new file mode 100644 index 000000000..c768c7aed --- /dev/null +++ b/testutil/constants.go @@ -0,0 +1,47 @@ +/* +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. +*/ + +package testutil + +const ( + // Dockerfile is used for unit testing + Dockerfile = ` + FROM scratch + RUN echo hi > /hi + + FROM scratch AS second + COPY --from=0 /hi /hi2 + + FROM second + RUN xxx + + FROM scratch + COPY --from=second /hi2 /hi3 + + FROM ubuntu:16.04 AS base + ENV DEBIAN_FRONTEND noninteractive + ENV LC_ALL C.UTF-8 + + FROM base AS development + ENV PS1 " ๐Ÿณ \[\033[1;36m\]\W\[\033[0;35m\] # \[\033[0m\]" + + FROM development AS test + ENV ORG_ENV UnitTest + + FROM base AS production + COPY . /code + ` +) From 4f3ab61b961e5a92ca3b45671c5773cd4b024e80 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 4 Sep 2018 13:16:05 -0700 Subject: [PATCH 02/35] Add CacheCommand to DockerCommand interface CacheCommand returns true if the command should be cached. Currently, it's only true for RUN but can be added to ADD/COPY later on (these are different since the contents of files for ADD/COPY need to be included in the cache key generation). I also changed CreatedBy to String so that we can log each command before cache extraction or regular execution takes place. --- pkg/commands/add.go | 15 ++++++++------- pkg/commands/arg.go | 15 ++++++++------- pkg/commands/cmd.go | 20 ++++++++------------ pkg/commands/commands.go | 7 +++++-- pkg/commands/copy.go | 16 ++++++++-------- pkg/commands/entrypoint.go | 20 ++++++++------------ pkg/commands/env.go | 19 ++++++++----------- pkg/commands/expose.go | 10 +++++++--- pkg/commands/healthcheck.go | 16 +++++++--------- pkg/commands/label.go | 18 ++++++++---------- pkg/commands/onbuild.go | 11 ++++++++--- pkg/commands/run.go | 17 ++++++++--------- pkg/commands/shell.go | 14 +++++++------- pkg/commands/stopsignal.go | 13 +++++++------ pkg/commands/user.go | 10 +++++++--- pkg/commands/volume.go | 10 +++++++--- pkg/commands/workdir.go | 11 ++++++++--- pkg/executor/build.go | 3 ++- 18 files changed, 129 insertions(+), 116 deletions(-) diff --git a/pkg/commands/add.go b/pkg/commands/add.go index e0e317967..52c5842a7 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -18,7 +18,6 @@ package commands import ( "path/filepath" - "strings" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" @@ -47,9 +46,6 @@ func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui srcs := a.cmd.SourcesAndDest[:len(a.cmd.SourcesAndDest)-1] dest := a.cmd.SourcesAndDest[len(a.cmd.SourcesAndDest)-1] - logrus.Infof("cmd: Add %s", srcs) - logrus.Infof("dest: %s", dest) - // First, resolve any environment replacement replacementEnvs := buildArgs.ReplacementEnvs(config.Env) resolvedEnvs, err := util.ResolveEnvironmentReplacementList(a.cmd.SourcesAndDest, replacementEnvs, true) @@ -112,7 +108,12 @@ func (a *AddCommand) FilesToSnapshot() []string { return a.snapshotFiles } -// CreatedBy returns some information about the command for the image config -func (a *AddCommand) CreatedBy() string { - return strings.Join(a.cmd.SourcesAndDest, " ") +// String returns some information about the command for the image config +func (a *AddCommand) String() string { + return a.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (a *AddCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/arg.go b/pkg/commands/arg.go index 202e52961..6174826b4 100644 --- a/pkg/commands/arg.go +++ b/pkg/commands/arg.go @@ -17,13 +17,10 @@ limitations under the License. package commands import ( - "strings" - "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" - "github.com/sirupsen/logrus" ) type ArgCommand struct { @@ -32,7 +29,6 @@ type ArgCommand struct { // ExecuteCommand only needs to add this ARG key/value as seen func (r *ArgCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - logrus.Info("ARG") replacementEnvs := buildArgs.ReplacementEnvs(config.Env) resolvedKey, err := util.ResolveEnvironmentReplacement(r.cmd.Key, replacementEnvs, false) if err != nil { @@ -55,7 +51,12 @@ func (r *ArgCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (r *ArgCommand) CreatedBy() string { - return strings.Join([]string{r.cmd.Name(), r.cmd.Key}, " ") +// String returns some information about the command for the image config history +func (r *ArgCommand) String() string { + return r.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (r *ArgCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/cmd.go b/pkg/commands/cmd.go index 36ce1805f..c6965c41e 100644 --- a/pkg/commands/cmd.go +++ b/pkg/commands/cmd.go @@ -33,7 +33,6 @@ type CmdCommand struct { // ExecuteCommand executes the CMD command // Argument handling is the same as RUN. func (c *CmdCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - logrus.Info("cmd: CMD") var newCommand []string if c.cmd.PrependShell { // This is the default shell on Linux @@ -60,15 +59,12 @@ func (c *CmdCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (c *CmdCommand) CreatedBy() string { - cmd := []string{"CMD"} - cmdLine := strings.Join(c.cmd.CmdLine, " ") - if c.cmd.PrependShell { - // TODO: Support shell command here - shell := []string{"/bin/sh", "-c"} - appendedShell := append(cmd, shell...) - return strings.Join(append(appendedShell, cmdLine), " ") - } - return strings.Join(append(cmd, cmdLine), " ") +// String returns some information about the command for the image config history +func (c *CmdCommand) String() string { + return c.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (c *CmdCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/commands.go b/pkg/commands/commands.go index 416ac870c..63cb388a2 100644 --- a/pkg/commands/commands.go +++ b/pkg/commands/commands.go @@ -30,10 +30,13 @@ type DockerCommand interface { // 2. Updating metadata fields in the config // It should not change the config history. ExecuteCommand(*v1.Config, *dockerfile.BuildArgs) error - // The config history has a "created by" field, should return information about the command - CreatedBy() string + // Returns a string representation of the command + String() string // A list of files to snapshot, empty for metadata commands or nil if we don't know FilesToSnapshot() []string + // Return true if this command should be true + // Currently only true for RUN + CacheCommand() bool } func GetCommand(cmd instructions.Command, buildcontext string) (DockerCommand, error) { diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index 020a61bb4..cdb05fba5 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -19,7 +19,6 @@ package commands import ( "os" "path/filepath" - "strings" "github.com/GoogleContainerTools/kaniko/pkg/constants" @@ -27,7 +26,6 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" - "github.com/sirupsen/logrus" ) type CopyCommand struct { @@ -40,9 +38,6 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu srcs := c.cmd.SourcesAndDest[:len(c.cmd.SourcesAndDest)-1] dest := c.cmd.SourcesAndDest[len(c.cmd.SourcesAndDest)-1] - logrus.Infof("cmd: copy %s", srcs) - logrus.Infof("dest: %s", dest) - // Resolve from if c.cmd.From != "" { c.buildcontext = filepath.Join(constants.KanikoDir, c.cmd.From) @@ -106,7 +101,12 @@ func (c *CopyCommand) FilesToSnapshot() []string { return c.snapshotFiles } -// CreatedBy returns some information about the command for the image config -func (c *CopyCommand) CreatedBy() string { - return strings.Join(c.cmd.SourcesAndDest, " ") +// String returns some information about the command for the image config +func (c *CopyCommand) String() string { + return c.cmd.String() +} + +// CacheCommand returns true since this command should be cached +func (c *CopyCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/entrypoint.go b/pkg/commands/entrypoint.go index 2a5b91c8c..ba8aa0c82 100644 --- a/pkg/commands/entrypoint.go +++ b/pkg/commands/entrypoint.go @@ -32,7 +32,6 @@ type EntrypointCommand struct { // ExecuteCommand handles command processing similar to CMD and RUN, func (e *EntrypointCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - logrus.Info("cmd: ENTRYPOINT") var newCommand []string if e.cmd.PrependShell { // This is the default shell on Linux @@ -58,15 +57,12 @@ func (e *EntrypointCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (e *EntrypointCommand) CreatedBy() string { - entrypoint := []string{"ENTRYPOINT"} - cmdLine := strings.Join(e.cmd.CmdLine, " ") - if e.cmd.PrependShell { - // TODO: Support shell command here - shell := []string{"/bin/sh", "-c"} - appendedShell := append(entrypoint, shell...) - return strings.Join(append(appendedShell, cmdLine), " ") - } - return strings.Join(append(entrypoint, cmdLine), " ") +// String returns some information about the command for the image config history +func (e *EntrypointCommand) String() string { + return e.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (e *EntrypointCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/env.go b/pkg/commands/env.go index b5948fb5d..249d438ed 100644 --- a/pkg/commands/env.go +++ b/pkg/commands/env.go @@ -17,14 +17,11 @@ limitations under the License. package commands import ( - "strings" - "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" - "github.com/sirupsen/logrus" ) type EnvCommand struct { @@ -32,7 +29,6 @@ type EnvCommand struct { } func (e *EnvCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - logrus.Info("cmd: ENV") newEnvs := e.cmd.Env replacementEnvs := buildArgs.ReplacementEnvs(config.Env) return util.UpdateConfigEnv(newEnvs, config, replacementEnvs) @@ -43,11 +39,12 @@ func (e *EnvCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (e *EnvCommand) CreatedBy() string { - envArray := []string{e.cmd.Name()} - for _, pair := range e.cmd.Env { - envArray = append(envArray, pair.Key+"="+pair.Value) - } - return strings.Join(envArray, " ") +// String returns some information about the command for the image config history +func (e *EnvCommand) String() string { + return e.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (e *EnvCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/expose.go b/pkg/commands/expose.go index cbb8d2236..1797b5010 100644 --- a/pkg/commands/expose.go +++ b/pkg/commands/expose.go @@ -76,7 +76,11 @@ func (r *ExposeCommand) FilesToSnapshot() []string { return []string{} } -func (r *ExposeCommand) CreatedBy() string { - s := []string{r.cmd.Name()} - return strings.Join(append(s, r.cmd.Ports...), " ") +func (r *ExposeCommand) String() string { + return r.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (r *ExposeCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/healthcheck.go b/pkg/commands/healthcheck.go index 52427b619..380953e73 100644 --- a/pkg/commands/healthcheck.go +++ b/pkg/commands/healthcheck.go @@ -17,12 +17,9 @@ limitations under the License. package commands import ( - "strings" - "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" - "github.com/sirupsen/logrus" ) type HealthCheckCommand struct { @@ -31,8 +28,6 @@ type HealthCheckCommand struct { // ExecuteCommand handles command processing similar to CMD and RUN, func (h *HealthCheckCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - logrus.Info("cmd: HEALTHCHECK") - check := v1.HealthConfig(*h.cmd.Health) config.Healthcheck = &check @@ -44,9 +39,12 @@ func (h *HealthCheckCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (h *HealthCheckCommand) CreatedBy() string { - entrypoint := []string{"HEALTHCHECK"} +// String returns some information about the command for the image config history +func (h *HealthCheckCommand) String() string { + return h.cmd.String() +} - return strings.Join(append(entrypoint, strings.Join(h.cmd.Health.Test, " ")), " ") +// CacheCommand returns false since this command shouldn't be cached +func (h *HealthCheckCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/label.go b/pkg/commands/label.go index 177d991aa..dae43546b 100644 --- a/pkg/commands/label.go +++ b/pkg/commands/label.go @@ -17,8 +17,6 @@ limitations under the License. package commands import ( - "strings" - "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" @@ -32,7 +30,6 @@ type LabelCommand struct { } func (r *LabelCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - logrus.Info("cmd: LABEL") return updateLabels(r.cmd.Labels, config, buildArgs) } @@ -72,11 +69,12 @@ func (r *LabelCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (r *LabelCommand) CreatedBy() string { - l := []string{r.cmd.Name()} - for _, kvp := range r.cmd.Labels { - l = append(l, kvp.String()) - } - return strings.Join(l, " ") +// String returns some information about the command for the image config history +func (r *LabelCommand) String() string { + return r.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (r *LabelCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/onbuild.go b/pkg/commands/onbuild.go index d7be8db68..b7b8728d3 100644 --- a/pkg/commands/onbuild.go +++ b/pkg/commands/onbuild.go @@ -44,7 +44,12 @@ func (o *OnBuildCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (o *OnBuildCommand) CreatedBy() string { - return o.cmd.Expression +// String returns some information about the command for the image config history +func (o *OnBuildCommand) String() string { + return o.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (o *OnBuildCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/run.go b/pkg/commands/run.go index b0de6cbc9..3b59f84c9 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -137,13 +137,12 @@ func (r *RunCommand) FilesToSnapshot() []string { return nil } -// CreatedBy returns some information about the command for the image config -func (r *RunCommand) CreatedBy() string { - cmdLine := strings.Join(r.cmd.CmdLine, " ") - if r.cmd.PrependShell { - // TODO: Support shell command here - shell := []string{"/bin/sh", "-c"} - return strings.Join(append(shell, cmdLine), " ") - } - return cmdLine +// String returns some information about the command for the image config +func (r *RunCommand) String() string { + return r.cmd.String() +} + +// CacheCommand returns true since this command should be cached +func (r *RunCommand) CacheCommand() bool { + return true } diff --git a/pkg/commands/shell.go b/pkg/commands/shell.go index f0bbe2666..bb763828e 100644 --- a/pkg/commands/shell.go +++ b/pkg/commands/shell.go @@ -17,8 +17,6 @@ limitations under the License. package commands import ( - "strings" - "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" @@ -46,10 +44,12 @@ func (s *ShellCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (s *ShellCommand) CreatedBy() string { - entrypoint := []string{"SHELL"} - cmdLine := strings.Join(s.cmd.Shell, " ") +// String returns some information about the command for the image config history +func (s *ShellCommand) String() string { + return s.cmd.String() +} - return strings.Join(append(entrypoint, cmdLine), " ") +// CacheCommand returns false since this command shouldn't be cached +func (s *ShellCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/stopsignal.go b/pkg/commands/stopsignal.go index c00f0fefb..87341f57f 100644 --- a/pkg/commands/stopsignal.go +++ b/pkg/commands/stopsignal.go @@ -17,8 +17,6 @@ limitations under the License. package commands import ( - "strings" - "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/docker/docker/pkg/signal" @@ -59,9 +57,12 @@ func (s *StopSignalCommand) FilesToSnapshot() []string { return []string{} } -// CreatedBy returns some information about the command for the image config history -func (s *StopSignalCommand) CreatedBy() string { - entrypoint := []string{"STOPSIGNAL"} +// String returns some information about the command for the image config history +func (s *StopSignalCommand) String() string { + return s.cmd.String() +} - return strings.Join(append(entrypoint, s.cmd.Signal), " ") +// CacheCommand returns false since this command shouldn't be cached +func (s *StopSignalCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/user.go b/pkg/commands/user.go index 5261339d3..d957ddfa3 100644 --- a/pkg/commands/user.go +++ b/pkg/commands/user.go @@ -63,7 +63,11 @@ func (r *UserCommand) FilesToSnapshot() []string { return []string{} } -func (r *UserCommand) CreatedBy() string { - s := []string{r.cmd.Name(), r.cmd.User} - return strings.Join(s, " ") +func (r *UserCommand) String() string { + return r.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (r *UserCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/volume.go b/pkg/commands/volume.go index 0fdad0c0e..5c9ecd99c 100644 --- a/pkg/commands/volume.go +++ b/pkg/commands/volume.go @@ -19,7 +19,6 @@ package commands import ( "fmt" "os" - "strings" "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" @@ -72,6 +71,11 @@ func (v *VolumeCommand) FilesToSnapshot() []string { return v.snapshotFiles } -func (v *VolumeCommand) CreatedBy() string { - return strings.Join(append([]string{v.cmd.Name()}, v.cmd.Volumes...), " ") +func (v *VolumeCommand) String() string { + return v.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (v *VolumeCommand) CacheCommand() bool { + return false } diff --git a/pkg/commands/workdir.go b/pkg/commands/workdir.go index 67186f73f..1eaeb072e 100644 --- a/pkg/commands/workdir.go +++ b/pkg/commands/workdir.go @@ -62,7 +62,12 @@ func (w *WorkdirCommand) FilesToSnapshot() []string { return w.snapshotFiles } -// CreatedBy returns some information about the command for the image config history -func (w *WorkdirCommand) CreatedBy() string { - return w.cmd.Name() + " " + w.cmd.Path +// String returns some information about the command for the image config history +func (w *WorkdirCommand) String() string { + return w.cmd.String() +} + +// CacheCommand returns false since this command shouldn't be cached +func (w *WorkdirCommand) CacheCommand() bool { + return false } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 22807d6a7..e7f5fec24 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -85,6 +85,7 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { if dockerCommand == nil { continue } + logrus.Info(dockerCommand.String()) if err := dockerCommand.ExecuteCommand(&imageConfig.Config, buildArgs); err != nil { return nil, err } @@ -138,7 +139,7 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { Layer: layer, History: v1.History{ Author: constants.Author, - CreatedBy: dockerCommand.CreatedBy(), + CreatedBy: dockerCommand.String(), }, }, ) From 13accbaf3243a5cf477c9e3d5116dc09dd80ae37 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 4 Sep 2018 13:37:15 -0700 Subject: [PATCH 03/35] Add Key() to LayeredMap and Snapshotter This will return a string representaiton of the current filesystem to be used with caching. Whenever a file is explictly added (via ADD or COPY), it will be stored in "added" in the LayeredMap. The file will map to a hash created by CacheHasher (which doesn't take into account mtime, since that will be different with every build, making the cache useless) Key() will returns a sha of the added files which will be used in determining the overall cache key for a command. --- pkg/executor/build.go | 2 +- pkg/snapshot/layered_map.go | 36 ++++++++++++--- pkg/snapshot/layered_map_test.go | 78 ++++++++++++++++++++++++++++++++ pkg/snapshot/snapshot.go | 8 +++- pkg/snapshot/snapshot_test.go | 2 +- pkg/util/util.go | 38 ++++++++++++++++ 6 files changed, 155 insertions(+), 9 deletions(-) create mode 100644 pkg/snapshot/layered_map_test.go diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 22807d6a7..f6ac89f46 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -62,7 +62,7 @@ func DoBuild(opts *options.KanikoOptions) (v1.Image, error) { if err := util.GetFSFromImage(constants.RootDir, sourceImage); err != nil { return nil, err } - l := snapshot.NewLayeredMap(hasher) + l := snapshot.NewLayeredMap(hasher, util.CacheHasher()) snapshotter := snapshot.NewSnapshotter(l, constants.RootDir) // Take initial snapshot if err := snapshotter.Init(); err != nil { diff --git a/pkg/snapshot/layered_map.go b/pkg/snapshot/layered_map.go index 0d382d766..c922bbafc 100644 --- a/pkg/snapshot/layered_map.go +++ b/pkg/snapshot/layered_map.go @@ -17,20 +17,27 @@ limitations under the License. package snapshot import ( + "bytes" + "encoding/json" "fmt" "path/filepath" "strings" + + "github.com/GoogleContainerTools/kaniko/pkg/util" ) type LayeredMap struct { - layers []map[string]string - whiteouts []map[string]string - hasher func(string) (string, error) + layers []map[string]string + whiteouts []map[string]string + added []map[string]string + hasher func(string) (string, error) + cacheHasher func(string) (string, error) } -func NewLayeredMap(h func(string) (string, error)) *LayeredMap { +func NewLayeredMap(h func(string) (string, error), c func(string) (string, error)) *LayeredMap { l := LayeredMap{ - hasher: h, + hasher: h, + cacheHasher: c, } l.layers = []map[string]string{} return &l @@ -39,8 +46,18 @@ func NewLayeredMap(h func(string) (string, error)) *LayeredMap { func (l *LayeredMap) Snapshot() { l.whiteouts = append(l.whiteouts, map[string]string{}) l.layers = append(l.layers, map[string]string{}) + l.added = append(l.added, map[string]string{}) } +// Key returns a hash for added files +func (l *LayeredMap) Key() (string, error) { + c := bytes.NewBuffer([]byte{}) + enc := json.NewEncoder(c) + enc.Encode(l.added) + return util.SHA256(c) +} + +// GetFlattenedPathsForWhiteOut returns all paths in the current FS func (l *LayeredMap) GetFlattenedPathsForWhiteOut() map[string]struct{} { paths := map[string]struct{}{} for _, l := range l.layers { @@ -85,11 +102,18 @@ func (l *LayeredMap) MaybeAddWhiteout(s string) (bool, error) { // Add will add the specified file s to the layered map. func (l *LayeredMap) Add(s string) error { + // Use hash function and add to layers newV, err := l.hasher(s) if err != nil { - return fmt.Errorf("Error creating hash for %s: %s", s, err) + return fmt.Errorf("Error creating hash for %s: %v", s, err) } l.layers[len(l.layers)-1][s] = newV + // Use cache hash function and add to added + cacheV, err := l.cacheHasher(s) + if err != nil { + return fmt.Errorf("Error creating cache hash for %s: %v", s, err) + } + l.added[len(l.added)-1][s] = cacheV return nil } diff --git a/pkg/snapshot/layered_map_test.go b/pkg/snapshot/layered_map_test.go new file mode 100644 index 000000000..e5ea64f02 --- /dev/null +++ b/pkg/snapshot/layered_map_test.go @@ -0,0 +1,78 @@ +/* +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. +*/ +package snapshot + +import ( + "testing" +) + +func Test_CacheKey(t *testing.T) { + tests := []struct { + name string + map1 map[string]string + map2 map[string]string + equal bool + }{ + { + name: "maps are the same", + map1: map[string]string{ + "a": "apple", + "b": "bat", + "c": "cat", + }, + map2: map[string]string{ + "c": "cat", + "b": "bat", + "a": "apple", + }, + equal: true, + }, + { + name: "maps are different", + map1: map[string]string{ + "a": "apple", + "b": "bat", + "c": "cat", + }, + map2: map[string]string{ + "c": "", + "b": "bat", + "a": "apple", + }, + equal: false, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + lm1 := LayeredMap{added: []map[string]string{test.map1}} + lm2 := LayeredMap{added: []map[string]string{test.map2}} + k1, err := lm1.Key() + if err != nil { + t.Fatalf("error getting key for map 1: %v", err) + } + k2, err := lm2.Key() + if err != nil { + t.Fatalf("error getting key for map 2: %v", err) + } + if test.equal && k1 != k2 { + t.Fatalf("keys differ.\nExpected\n%+v\nActual\n%+v", k1, k2) + } + if !test.equal && k1 == k2 { + t.Fatal("keys are the same, expected different keys") + } + }) + } +} diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 4f01441dc..55da45d65 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -49,6 +49,11 @@ func (s *Snapshotter) Init() error { return nil } +// Key returns a string based on the current state of the file system +func (s *Snapshotter) Key() (string, error) { + return s.l.Key() +} + // TakeSnapshot takes a snapshot of the specified files, avoiding directories in the whitelist, and creates // a tarball of the changed files. Return contents of the tarball, and whether or not any files were changed func (s *Snapshotter) TakeSnapshot(files []string) ([]byte, error) { @@ -102,7 +107,8 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) { logrus.Info("No files changed in this command, skipping snapshotting.") return false, nil } - logrus.Infof("Taking snapshot of files %v...", files) + logrus.Info("Taking snapshot of files...") + logrus.Debugf("Taking snapshot of files %v", files) snapshottedFiles := make(map[string]bool) filesAdded := false diff --git a/pkg/snapshot/snapshot_test.go b/pkg/snapshot/snapshot_test.go index 72b6a750a..95daa88f6 100644 --- a/pkg/snapshot/snapshot_test.go +++ b/pkg/snapshot/snapshot_test.go @@ -198,7 +198,7 @@ func setUpTestDir() (string, *Snapshotter, error) { } // Take the initial snapshot - l := NewLayeredMap(util.Hasher()) + l := NewLayeredMap(util.Hasher(), util.CacheHasher()) snapshotter := NewSnapshotter(l, testDir) if err := snapshotter.Init(); err != nil { return testDir, nil, errors.Wrap(err, "initializing snapshotter") diff --git a/pkg/util/util.go b/pkg/util/util.go index 617298a7b..bc09a7c27 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -18,6 +18,7 @@ package util import ( "crypto/md5" + "crypto/sha256" "encoding/hex" "io" "os" @@ -72,6 +73,36 @@ func Hasher() func(string) (string, error) { return hasher } +// CacheHasher takes into account everything the regular hasher does except for mtime +func CacheHasher() func(string) (string, error) { + hasher := func(p string) (string, error) { + h := md5.New() + fi, err := os.Lstat(p) + if err != nil { + return "", err + } + h.Write([]byte(fi.Mode().String())) + + h.Write([]byte(strconv.FormatUint(uint64(fi.Sys().(*syscall.Stat_t).Uid), 36))) + h.Write([]byte(",")) + h.Write([]byte(strconv.FormatUint(uint64(fi.Sys().(*syscall.Stat_t).Gid), 36))) + + if fi.Mode().IsRegular() { + f, err := os.Open(p) + if err != nil { + return "", err + } + defer f.Close() + if _, err := io.Copy(h, f); err != nil { + return "", err + } + } + + return hex.EncodeToString(h.Sum(nil)), nil + } + return hasher +} + // MtimeHasher returns a hash function, which only looks at mtime to determine if a file has changed. // Note that the mtime can lag, so it's possible that a file will have changed but the mtime may look the same. func MtimeHasher() func(string) (string, error) { @@ -86,3 +117,10 @@ func MtimeHasher() func(string) (string, error) { } return hasher } + +// SHA256 returns the shasum of the contents of r +func SHA256(r io.Reader) (string, error) { + hasher := sha256.New() + _, err := io.Copy(hasher, r) + return hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), err +} From e300101579299f96d28addb1cf1657bedcc88b70 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 4 Sep 2018 13:50:55 -0700 Subject: [PATCH 04/35] Fix linting error --- pkg/commands/add.go | 7 ++----- pkg/commands/copy.go | 7 ++----- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/pkg/commands/add.go b/pkg/commands/add.go index 52c5842a7..dfc7f63f8 100644 --- a/pkg/commands/add.go +++ b/pkg/commands/add.go @@ -43,18 +43,15 @@ type AddCommand struct { // 2. If is a local tar archive: // -If is a local tar archive, it is unpacked at the dest, as 'tar -x' would func (a *AddCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - srcs := a.cmd.SourcesAndDest[:len(a.cmd.SourcesAndDest)-1] - dest := a.cmd.SourcesAndDest[len(a.cmd.SourcesAndDest)-1] - // First, resolve any environment replacement replacementEnvs := buildArgs.ReplacementEnvs(config.Env) resolvedEnvs, err := util.ResolveEnvironmentReplacementList(a.cmd.SourcesAndDest, replacementEnvs, true) if err != nil { return err } - dest = resolvedEnvs[len(resolvedEnvs)-1] + dest := resolvedEnvs[len(resolvedEnvs)-1] // Resolve wildcards and get a list of resolved sources - srcs, err = util.ResolveSources(resolvedEnvs, a.buildcontext) + srcs, err := util.ResolveSources(resolvedEnvs, a.buildcontext) if err != nil { return err } diff --git a/pkg/commands/copy.go b/pkg/commands/copy.go index cdb05fba5..8b34d1b6b 100644 --- a/pkg/commands/copy.go +++ b/pkg/commands/copy.go @@ -35,9 +35,6 @@ type CopyCommand struct { } func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - srcs := c.cmd.SourcesAndDest[:len(c.cmd.SourcesAndDest)-1] - dest := c.cmd.SourcesAndDest[len(c.cmd.SourcesAndDest)-1] - // Resolve from if c.cmd.From != "" { c.buildcontext = filepath.Join(constants.KanikoDir, c.cmd.From) @@ -48,9 +45,9 @@ func (c *CopyCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bu if err != nil { return err } - dest = resolvedEnvs[len(resolvedEnvs)-1] + dest := resolvedEnvs[len(resolvedEnvs)-1] // Resolve wildcards and get a list of resolved sources - srcs, err = util.ResolveSources(resolvedEnvs, c.buildcontext) + srcs, err := util.ResolveSources(resolvedEnvs, c.buildcontext) if err != nil { return err } From 31647f86870537c21c362d1154d739d2eea7fa76 Mon Sep 17 00:00:00 2001 From: priyawadhwa Date: Tue, 4 Sep 2018 14:59:05 -0700 Subject: [PATCH 05/35] Update issue templates Add issue template for kaniko bugs. Should fix #338 --- .github/ISSUE_TEMPLATE/bug_report.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 000000000..f25773d76 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,23 @@ +--- +name: Bug report +about: Report a bug in kaniko + +--- + +**Actual behavior** +A clear and concise description of what the bug is. + +**Expected behavior** +A clear and concise description of what you expected to happen. + +**To Reproduce** +Steps to reproduce the behavior: +1. ... +2. ... + +**Additional Information** + - Dockerfile + Please provide either the Dockerfile you're trying to build or one that can reproduce this error. + - Build Context + Please provide or clearly describe any files needed to build the Dockerfile (ADD/COPY commands) + - Kaniko Image (fully qualified with digest) From 80a449f5419cf1d83e2d1d5cc1a17f78374af53d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 7 Sep 2018 16:03:56 -0700 Subject: [PATCH 06/35] code review comments --- pkg/snapshot/layered_map.go | 9 +++++---- pkg/snapshot/layered_map_test.go | 11 ++++++----- pkg/util/util.go | 5 ++++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/pkg/snapshot/layered_map.go b/pkg/snapshot/layered_map.go index c922bbafc..9a356b0e2 100644 --- a/pkg/snapshot/layered_map.go +++ b/pkg/snapshot/layered_map.go @@ -27,10 +27,11 @@ import ( ) type LayeredMap struct { - layers []map[string]string - whiteouts []map[string]string - added []map[string]string - hasher func(string) (string, error) + layers []map[string]string + whiteouts []map[string]string + added []map[string]string + hasher func(string) (string, error) + // cacheHasher doesn't include mtime in it's hash so that filesystem cache keys are stable cacheHasher func(string) (string, error) } diff --git a/pkg/snapshot/layered_map_test.go b/pkg/snapshot/layered_map_test.go index e5ea64f02..6bfff81b3 100644 --- a/pkg/snapshot/layered_map_test.go +++ b/pkg/snapshot/layered_map_test.go @@ -32,11 +32,15 @@ func Test_CacheKey(t *testing.T) { "a": "apple", "b": "bat", "c": "cat", + "d": "dog", + "e": "egg", }, map2: map[string]string{ "c": "cat", + "d": "dog", "b": "bat", "a": "apple", + "e": "egg", }, equal: true, }, @@ -67,11 +71,8 @@ func Test_CacheKey(t *testing.T) { if err != nil { t.Fatalf("error getting key for map 2: %v", err) } - if test.equal && k1 != k2 { - t.Fatalf("keys differ.\nExpected\n%+v\nActual\n%+v", k1, k2) - } - if !test.equal && k1 == k2 { - t.Fatal("keys are the same, expected different keys") + if test.equal != (k1 == k2) { + t.Fatalf("unexpected result: \nExpected\n%s\nActual\n%s\n", k1, k2) } }) } diff --git a/pkg/util/util.go b/pkg/util/util.go index bc09a7c27..873cbae20 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -122,5 +122,8 @@ func MtimeHasher() func(string) (string, error) { func SHA256(r io.Reader) (string, error) { hasher := sha256.New() _, err := io.Copy(hasher, r) - return hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), err + if err != nil { + return "", err + } + return hex.EncodeToString(hasher.Sum(make([]byte, 0, hasher.Size()))), nil } From d9022dd7de93f3fb254f2332a6475ca7ff4a5adb Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 7 Sep 2018 17:04:04 -0700 Subject: [PATCH 07/35] Refactor build into stageBuilder type Refactoring builds by stage will make it easier to generate cache keys for layers, since the stageBuilder type will contain everything required to generate the key: 1. Base image with digest 2. Config file 3. Snapshotter (which will provide a key for the filesystem) 4. The current command (which will be passed in) --- pkg/executor/build.go | 241 +++++++++++++++++++++++++----------------- 1 file changed, 145 insertions(+), 96 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 27f58d70f..7902c2e3a 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -30,6 +30,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/tarball" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/kaniko/pkg/commands" @@ -40,112 +41,160 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/util" ) +// stageBuilder contains all fields necessary to build one stage of a Dockerfile +type stageBuilder struct { + stage config.KanikoStage + v1.Image + *v1.ConfigFile + *snapshot.Snapshotter + baseImageDigest string +} + +// newStageBuilder returns a new type stageBuilder which contains all the information required to build the stage +func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*stageBuilder, error) { + sourceImage, err := util.RetrieveSourceImage(stage, opts.BuildArgs) + if err != nil { + return nil, err + } + imageConfig, err := util.RetrieveConfigFile(sourceImage) + if err != nil { + return nil, err + } + if err := resolveOnBuild(&stage, &imageConfig.Config); err != nil { + return nil, err + } + hasher, err := getHasher(opts.SnapshotMode) + if err != nil { + return nil, err + } + l := snapshot.NewLayeredMap(hasher) + snapshotter := snapshot.NewSnapshotter(l, constants.RootDir) + + digest, err := sourceImage.Digest() + if err != nil { + return nil, err + } + return &stageBuilder{ + stage: stage, + Image: sourceImage, + ConfigFile: imageConfig, + Snapshotter: snapshotter, + baseImageDigest: digest.String(), + }, nil +} + +// key will return a string representation of the build at the cmd +// TODO: priyawadhwa@ to fill this out when implementing caching +func (s *stageBuilder) key(cmd string) (string, error) { + return "", nil +} + +// extractCachedLayer will extract the cached layer and append it to the config file +// TODO: priyawadhwa@ to fill this out when implementing caching +func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { + return nil +} + +func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { + // Unpack file system to root + if err := util.GetFSFromImage(constants.RootDir, s.Image); err != nil { + return err + } + // Take initial snapshot + if err := s.Snapshotter.Init(); err != nil { + return err + } + buildArgs := dockerfile.NewBuildArgs(opts.BuildArgs) + for index, cmd := range s.stage.Commands { + finalCmd := index == len(s.stage.Commands)-1 + dockerCommand, err := commands.GetCommand(cmd, opts.SrcContext) + if err != nil { + return err + } + if dockerCommand == nil { + continue + } + logrus.Info(dockerCommand.String()) + if err := dockerCommand.ExecuteCommand(&s.ConfigFile.Config, buildArgs); err != nil { + return err + } + snapshotFiles := dockerCommand.FilesToSnapshot() + var contents []byte + + // If this is an intermediate stage, we only snapshot for the last command and we + // want to snapshot the entire filesystem since we aren't tracking what was changed + // by previous commands. + if !s.stage.FinalStage { + if finalCmd { + contents, err = s.Snapshotter.TakeSnapshotFS() + } + } else { + // If we are in single snapshot mode, we only take a snapshot once, after all + // commands have completed. + if opts.SingleSnapshot { + if finalCmd { + contents, err = s.Snapshotter.TakeSnapshotFS() + } + } else { + // Otherwise, in the final stage we take a snapshot at each command. If we know + // the files that were changed, we'll snapshot those explicitly, otherwise we'll + // check if anything in the filesystem changed. + if snapshotFiles != nil { + contents, err = s.Snapshotter.TakeSnapshot(snapshotFiles) + } else { + contents, err = s.Snapshotter.TakeSnapshotFS() + } + } + } + if err != nil { + return fmt.Errorf("Error taking snapshot of files for command %s: %s", dockerCommand, err) + } + + util.MoveVolumeWhitelistToWhitelist() + if contents == nil { + logrus.Info("No files were changed, appending empty layer to config. No layer added to image.") + continue + } + // Append the layer to the image + opener := func() (io.ReadCloser, error) { + return ioutil.NopCloser(bytes.NewReader(contents)), nil + } + layer, err := tarball.LayerFromOpener(opener) + if err != nil { + return err + } + s.Image, err = mutate.Append(s.Image, + mutate.Addendum{ + Layer: layer, + History: v1.History{ + Author: constants.Author, + CreatedBy: dockerCommand.String(), + }, + }, + ) + if err != nil { + return err + } + } + return nil +} + +// DoBuild executes building the Dockerfile func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { // Parse dockerfile and unpack base image to root stages, err := dockerfile.Stages(opts) if err != nil { return nil, err } - - hasher, err := getHasher(opts.SnapshotMode) - if err != nil { - return nil, err - } for index, stage := range stages { - // Unpack file system to root - sourceImage, err := util.RetrieveSourceImage(stage, opts.BuildArgs) + stageBuilder, err := newStageBuilder(opts, stage) if err != nil { - return nil, err + return nil, errors.Wrap(err, fmt.Sprintf("getting stage builder for stage %d", index)) } - if err := util.GetFSFromImage(constants.RootDir, sourceImage); err != nil { - return nil, err + if err := stageBuilder.buildStage(opts); err != nil { + return nil, errors.Wrap(err, "error building stage") } - l := snapshot.NewLayeredMap(hasher) - snapshotter := snapshot.NewSnapshotter(l, constants.RootDir) - // Take initial snapshot - if err := snapshotter.Init(); err != nil { - return nil, err - } - imageConfig, err := util.RetrieveConfigFile(sourceImage) - if err != nil { - return nil, err - } - if err := resolveOnBuild(&stage, &imageConfig.Config); err != nil { - return nil, err - } - buildArgs := dockerfile.NewBuildArgs(opts.BuildArgs) - for index, cmd := range stage.Commands { - finalCmd := index == len(stage.Commands)-1 - dockerCommand, err := commands.GetCommand(cmd, opts.SrcContext) - if err != nil { - return nil, err - } - if dockerCommand == nil { - continue - } - logrus.Info(dockerCommand.String()) - if err := dockerCommand.ExecuteCommand(&imageConfig.Config, buildArgs); err != nil { - return nil, err - } - snapshotFiles := dockerCommand.FilesToSnapshot() - var contents []byte - - // If this is an intermediate stage, we only snapshot for the last command and we - // want to snapshot the entire filesystem since we aren't tracking what was changed - // by previous commands. - if !stage.FinalStage { - if finalCmd { - contents, err = snapshotter.TakeSnapshotFS() - } - } else { - // If we are in single snapshot mode, we only take a snapshot once, after all - // commands have completed. - if opts.SingleSnapshot { - if finalCmd { - contents, err = snapshotter.TakeSnapshotFS() - } - } else { - // Otherwise, in the final stage we take a snapshot at each command. If we know - // the files that were changed, we'll snapshot those explicitly, otherwise we'll - // check if anything in the filesystem changed. - if snapshotFiles != nil { - contents, err = snapshotter.TakeSnapshot(snapshotFiles) - } else { - contents, err = snapshotter.TakeSnapshotFS() - } - } - } - if err != nil { - return nil, fmt.Errorf("Error taking snapshot of files for command %s: %s", dockerCommand, err) - } - - util.MoveVolumeWhitelistToWhitelist() - if contents == nil { - logrus.Info("No files were changed, appending empty layer to config. No layer added to image.") - continue - } - // Append the layer to the image - opener := func() (io.ReadCloser, error) { - return ioutil.NopCloser(bytes.NewReader(contents)), nil - } - layer, err := tarball.LayerFromOpener(opener) - if err != nil { - return nil, err - } - sourceImage, err = mutate.Append(sourceImage, - mutate.Addendum{ - Layer: layer, - History: v1.History{ - Author: constants.Author, - CreatedBy: dockerCommand.String(), - }, - }, - ) - if err != nil { - return nil, err - } - } - sourceImage, err = mutate.Config(sourceImage, imageConfig.Config) + sourceImage, err := mutate.Config(stageBuilder.Image, stageBuilder.ConfigFile.Config) if err != nil { return nil, err } From 63cecbff74d2446d8d76e1e798059362d5169cb2 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 10 Sep 2018 17:06:09 -0700 Subject: [PATCH 08/35] Whitelist /etc/mtab While looking into #345, we were seeing the error: Error: error building image: chmod /etc/mtab: operation not permitted during extraction of `amazonlinux:1`. I looked into why kaniko couldn't extract this file properly, and found that it already existed as a symlink pointing to /proc/mounts, which returned an error when we tried to run chmod on it. Confusingly, in the image the /etc/mtab is a regular file, not a symlink. I can think of two ways to solve this problem: 1. Whitelist /etc/mtab so that whatever already exists in the system is used 2. Check if a regular file already exists, and hasn't been extracted yet, before extracting I went with option 1 because for option 2 we'd have to keep a list of all files that had been extracted in memory. --- pkg/util/fs_util.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 4c6dd9fff..af5b6df42 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -40,6 +40,9 @@ var whitelist = []string{ // 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", } var volumeWhitelist = []string{} @@ -195,7 +198,6 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { return err } currFile.Close() - case tar.TypeDir: logrus.Debugf("creating dir %s", path) if err := os.MkdirAll(path, mode); err != nil { From 5d2d2829d05dea9b2bf2457d9449fcb37ddbcc92 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 10 Sep 2018 18:08:43 -0700 Subject: [PATCH 09/35] Review config for cmd/entrypoint after building a stage As mentioned in #346, if only ENTRYPOINT is set in a stage then any CMD inherited from a parent should be cleared. If both entrypoint and cmd are set then nothing should change. I added a function and unit test to review the config file after building a stage which clears out config.Cmd if ENTRYPOINT was declared but CMD wasn't. I also added an integration test to make sure this works, which should be tested by the preexisting container-diff --metadata test. --- integration/dockerfiles/Dockerfile_test_cmd | 5 ++ pkg/executor/build.go | 24 +++++++ pkg/executor/build_test.go | 76 +++++++++++++++++++++ 3 files changed, 105 insertions(+) create mode 100644 integration/dockerfiles/Dockerfile_test_cmd create mode 100644 pkg/executor/build_test.go diff --git a/integration/dockerfiles/Dockerfile_test_cmd b/integration/dockerfiles/Dockerfile_test_cmd new file mode 100644 index 000000000..631b93fd8 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_cmd @@ -0,0 +1,5 @@ +FROM scratch AS first +CMD ["mycmd"] + +FROM first +ENTRYPOINT ["myentrypoint"] # This should clear out CMD in the config metadata diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 27f58d70f..10cbeca9e 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -145,6 +145,9 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return nil, err } } + if err := reviewConfig(stage, &imageConfig.Config); err != nil { + return nil, err + } sourceImage, err = mutate.Config(sourceImage, imageConfig.Config) if err != nil { return nil, err @@ -228,3 +231,24 @@ func resolveOnBuild(stage *config.KanikoStage, config *v1.Config) error { config.OnBuild = nil return nil } + +// reviewConfig makes sure the value of CMD is correct after building the stage +// If ENTRYPOINT was set in this stage but CMD wasn't, then CMD should be cleared out +// See Issue #346 for more info +func reviewConfig(stage config.KanikoStage, config *v1.Config) error { + entrypoint := false + cmd := false + + for _, c := range stage.Commands { + if c.Name() == "cmd" { + cmd = true + } + if c.Name() == "entrypoint" { + entrypoint = true + } + } + if entrypoint && !cmd { + config.Cmd = []string{} + } + return nil +} diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go new file mode 100644 index 000000000..e1f120433 --- /dev/null +++ b/pkg/executor/build_test.go @@ -0,0 +1,76 @@ +/* +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. +*/ + +package executor + +import ( + "testing" + + "github.com/GoogleContainerTools/kaniko/pkg/config" + "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" + "github.com/GoogleContainerTools/kaniko/testutil" + "github.com/google/go-containerregistry/pkg/v1" +) + +func Test_reviewConfig(t *testing.T) { + tests := []struct { + name string + dockerfile string + originalCmd []string + originalEntrypoint []string + expectedCmd []string + }{ + { + name: "entrypoint and cmd declared", + dockerfile: ` + FROM scratch + CMD ["mycmd"] + ENTRYPOINT ["myentrypoint"]`, + originalEntrypoint: []string{"myentrypoint"}, + originalCmd: []string{"mycmd"}, + expectedCmd: []string{"mycmd"}, + }, + { + name: "only entrypoint declared", + dockerfile: ` + FROM scratch + ENTRYPOINT ["myentrypoint"]`, + originalEntrypoint: []string{"myentrypoint"}, + originalCmd: []string{"mycmd"}, + expectedCmd: []string{}, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + config := &v1.Config{ + Cmd: test.originalCmd, + Entrypoint: test.originalEntrypoint, + } + err := reviewConfig(stage(t, test.dockerfile), config) + testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedCmd, config.Cmd) + }) + } +} + +func stage(t *testing.T, d string) config.KanikoStage { + stages, err := dockerfile.Parse([]byte(d)) + if err != nil { + t.Fatalf("error parsing dockerfile: %v", err) + } + return config.KanikoStage{ + Stage: stages[0], + } +} From c13f6e84eda3da92d5ffca67f44412d68614ed10 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 10 Sep 2018 18:20:00 -0700 Subject: [PATCH 10/35] Fixed unit test --- pkg/util/fs_util_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 5bfb1a403..32df3c406 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -50,7 +50,7 @@ func Test_fileSystemWhitelist(t *testing.T) { } actualWhitelist, err := fileSystemWhitelist(path) - expectedWhitelist := []string{"/kaniko", "/proc", "/dev", "/dev/pts", "/sys", "/var/run"} + expectedWhitelist := []string{"/kaniko", "/proc", "/dev", "/dev/pts", "/sys", "/var/run", "/etc/mtab"} sort.Strings(actualWhitelist) sort.Strings(expectedWhitelist) testutil.CheckErrorAndDeepEqual(t, false, err, expectedWhitelist, actualWhitelist) From d923d5ef02631aafbe1920830cb740f717b4a4a1 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Sep 2018 10:05:15 -0700 Subject: [PATCH 11/35] Fix integration test --- integration/dockerfiles/Dockerfile_test_cmd | 2 +- pkg/commands/cmd.go | 2 -- pkg/commands/entrypoint.go | 2 -- pkg/executor/build.go | 2 +- pkg/executor/build_test.go | 2 +- 5 files changed, 3 insertions(+), 7 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_cmd b/integration/dockerfiles/Dockerfile_test_cmd index 631b93fd8..a2f0162b0 100644 --- a/integration/dockerfiles/Dockerfile_test_cmd +++ b/integration/dockerfiles/Dockerfile_test_cmd @@ -1,4 +1,4 @@ -FROM scratch AS first +FROM gcr.io/distroless/base@sha256:628939ac8bf3f49571d05c6c76b8688cb4a851af6c7088e599388259875bde20 AS first CMD ["mycmd"] FROM first diff --git a/pkg/commands/cmd.go b/pkg/commands/cmd.go index c6965c41e..308755200 100644 --- a/pkg/commands/cmd.go +++ b/pkg/commands/cmd.go @@ -23,7 +23,6 @@ import ( "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" - "github.com/sirupsen/logrus" ) type CmdCommand struct { @@ -48,7 +47,6 @@ func (c *CmdCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui newCommand = c.cmd.CmdLine } - logrus.Infof("Replacing CMD in config with %v", newCommand) config.Cmd = newCommand config.ArgsEscaped = true return nil diff --git a/pkg/commands/entrypoint.go b/pkg/commands/entrypoint.go index ba8aa0c82..5e403e23d 100644 --- a/pkg/commands/entrypoint.go +++ b/pkg/commands/entrypoint.go @@ -23,7 +23,6 @@ import ( "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" - "github.com/sirupsen/logrus" ) type EntrypointCommand struct { @@ -47,7 +46,6 @@ func (e *EntrypointCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerf newCommand = e.cmd.CmdLine } - logrus.Infof("Replacing Entrypoint in config with %v", newCommand) config.Entrypoint = newCommand return nil } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 10cbeca9e..c9d08b761 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -248,7 +248,7 @@ func reviewConfig(stage config.KanikoStage, config *v1.Config) error { } } if entrypoint && !cmd { - config.Cmd = []string{} + config.Cmd = nil } return nil } diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index e1f120433..51eca6f99 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -50,7 +50,7 @@ func Test_reviewConfig(t *testing.T) { ENTRYPOINT ["myentrypoint"]`, originalEntrypoint: []string{"myentrypoint"}, originalCmd: []string{"mycmd"}, - expectedCmd: []string{}, + expectedCmd: nil, }, } for _, test := range tests { From 99ab68e7f4dd49b0ea5d89ea09c93f2ade370ef7 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Sep 2018 10:31:20 -0700 Subject: [PATCH 12/35] Replace gometalinter with GolangCI-Lint gometalinter is broken @ HEAD, and I looked into why that was. During that process, I remembered that we took the linting scripts from skaffold, and found that in skaffold gometalinter was replaced with GolangCI-Lint: https://github.com/GoogleContainerTools/skaffold/pull/619 The change made linting in skaffold faster, so I figured instead of fixing gometalinter it made more sense to remove it and replace it with GolangCI-Lint for kaniko as well. --- cmd/executor/cmd/root.go | 4 +- hack/boilerplate/boilerplate.py | 29 +-- hack/gometalinter.json | 17 -- hack/install_golint.sh | 388 ++++++++++++++++++++++++++++ hack/{gometalinter.sh => linter.sh} | 30 ++- integration/integration_test.go | 15 +- pkg/buildcontext/buildcontext.go | 3 +- pkg/buildcontext/s3.go | 5 +- pkg/commands/user_test.go | 24 +- pkg/config/options.go | 10 +- pkg/config/stage.go | 2 +- pkg/dockerfile/buildargs.go | 7 +- pkg/executor/push.go | 2 +- pkg/util/bucket_util.go | 3 +- pkg/util/command_util_test.go | 14 +- pkg/util/fs_util_test.go | 4 +- pkg/util/image_util.go | 3 +- pkg/util/tar_util.go | 4 +- test.sh | 2 +- 19 files changed, 466 insertions(+), 100 deletions(-) delete mode 100644 hack/gometalinter.json create mode 100755 hack/install_golint.sh rename hack/{gometalinter.sh => linter.sh} (61%) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 841fce661..97511e092 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -86,7 +86,7 @@ func addKanikoOptionsFlags(cmd *cobra.Command) { RootCmd.PersistentFlags().StringVarP(&opts.SnapshotMode, "snapshotMode", "", "full", "Change the file attributes inspected during snapshotting") RootCmd.PersistentFlags().VarP(&opts.BuildArgs, "build-arg", "", "This flag allows you to pass in ARG values at build time. Set it repeatedly for multiple values.") RootCmd.PersistentFlags().BoolVarP(&opts.InsecurePush, "insecure", "", false, "Push to insecure registry using plain HTTP") - RootCmd.PersistentFlags().BoolVarP(&opts.SkipTlsVerify, "skip-tls-verify", "", false, "Push to insecure registry ignoring TLS verify") + RootCmd.PersistentFlags().BoolVarP(&opts.SkipTLSVerify, "skip-tls-verify", "", false, "Push to insecure registry ignoring TLS verify") RootCmd.PersistentFlags().StringVarP(&opts.TarPath, "tarPath", "", "", "Path to save the image in as a tarball instead of pushing") RootCmd.PersistentFlags().BoolVarP(&opts.SingleSnapshot, "single-snapshot", "", false, "Take a single snapshot at the end of the build.") RootCmd.PersistentFlags().BoolVarP(&opts.Reproducible, "reproducible", "", false, "Strip timestamps out of the image to make it reproducible") @@ -145,7 +145,7 @@ func resolveSourceContext() error { opts.SrcContext = opts.Bucket } } - // if no prefix use Google Cloud Storage as default for backwards compability + // if no prefix use Google Cloud Storage as default for backwards compatibility contextExecutor, err := buildcontext.GetBuildContext(opts.SrcContext) if err != nil { return err diff --git a/hack/boilerplate/boilerplate.py b/hack/boilerplate/boilerplate.py index bcc4b1c8f..83e6b1b3e 100644 --- a/hack/boilerplate/boilerplate.py +++ b/hack/boilerplate/boilerplate.py @@ -18,12 +18,14 @@ from __future__ import print_function import argparse import glob -import json -import mmap import os import re import sys + +SKIPPED_DIRS = ["Godeps", "third_party", ".git", "vendor", "examples", "testdata"] +SKIPPED_FILES = ["install_golint.sh"] + parser = argparse.ArgumentParser() parser.add_argument("filenames", help="list of files to check, all files if unspecified", nargs='*') @@ -71,7 +73,7 @@ def file_passes(filename, refs, regexs): (data, found) = p.subn("", data, 1) # remove shebang from the top of shell files - if extension == "sh": + elif extension == "sh": p = regexs["shebang"] (data, found) = p.subn("", data, 1) @@ -105,17 +107,11 @@ def file_passes(filename, refs, regexs): def file_extension(filename): return os.path.splitext(filename)[1].split(".")[-1].lower() -skipped_dirs = ['Godeps', 'third_party', '.git', "vendor", "differs/testDirs/pipTests"] - def normalize_files(files): newfiles = [] - for pathname in files: - if any(x in pathname for x in skipped_dirs): - continue - newfiles.append(pathname) - for i, pathname in enumerate(newfiles): + for i, pathname in enumerate(files): if not os.path.isabs(pathname): - newfiles[i] = os.path.join(args.rootdir, pathname) + newfiles.append(os.path.join(args.rootdir, pathname)) return newfiles def get_files(extensions): @@ -124,17 +120,14 @@ def get_files(extensions): files = args.filenames else: for root, dirs, walkfiles in os.walk(args.rootdir): - # don't visit certain dirs. This is just a performance improvement - # as we would prune these later in normalize_files(). But doing it - # cuts down the amount of filesystem walking we do and cuts down - # the size of the file list - for d in skipped_dirs: + for d in SKIPPED_DIRS: if d in dirs: dirs.remove(d) for name in walkfiles: - pathname = os.path.join(root, name) - files.append(pathname) + if name not in SKIPPED_FILES: + pathname = os.path.join(root, name) + files.append(pathname) files = normalize_files(files) outfiles = [] diff --git a/hack/gometalinter.json b/hack/gometalinter.json deleted file mode 100644 index 857b558e1..000000000 --- a/hack/gometalinter.json +++ /dev/null @@ -1,17 +0,0 @@ - -{ - "Vendor": true, - "EnableGC": true, - "Debug": false, - "Sort": ["linter", "severity", "path"], - "Enable": [ - "deadcode", - "gofmt", - "golint", - "gosimple", - "ineffassign", - "vet" - ], - - "LineLength": 200 -} diff --git a/hack/install_golint.sh b/hack/install_golint.sh new file mode 100755 index 000000000..6010f3b27 --- /dev/null +++ b/hack/install_golint.sh @@ -0,0 +1,388 @@ +#!/bin/sh +set -e +# Code generated by godownloader on 2018-06-05T12:04:55Z. DO NOT EDIT. +# + +usage() { + this=$1 + cat </dev/null +} +echoerr() { + echo "$@" 1>&2 +} +log_prefix() { + echo "$0" +} +_logp=6 +log_set_priority() { + _logp="$1" +} +log_priority() { + if test -z "$1"; then + echo "$_logp" + return + fi + [ "$1" -le "$_logp" ] +} +log_tag() { + case $1 in + 0) echo "emerg" ;; + 1) echo "alert" ;; + 2) echo "crit" ;; + 3) echo "err" ;; + 4) echo "warning" ;; + 5) echo "notice" ;; + 6) echo "info" ;; + 7) echo "debug" ;; + *) echo "$1" ;; + esac +} +log_debug() { + log_priority 7 || return 0 + echoerr "$(log_prefix)" "$(log_tag 7)" "$@" +} +log_info() { + log_priority 6 || return 0 + echoerr "$(log_prefix)" "$(log_tag 6)" "$@" +} +log_err() { + log_priority 3 || return 0 + echoerr "$(log_prefix)" "$(log_tag 3)" "$@" +} +log_crit() { + log_priority 2 || return 0 + echoerr "$(log_prefix)" "$(log_tag 2)" "$@" +} +uname_os() { + os=$(uname -s | tr '[:upper:]' '[:lower:]') + case "$os" in + msys_nt) os="windows" ;; + esac + echo "$os" +} +uname_arch() { + arch=$(uname -m) + case $arch in + x86_64) arch="amd64" ;; + x86) arch="386" ;; + i686) arch="386" ;; + i386) arch="386" ;; + aarch64) arch="arm64" ;; + armv5*) arch="armv5" ;; + armv6*) arch="armv6" ;; + armv7*) arch="armv7" ;; + esac + echo ${arch} +} +uname_os_check() { + os=$(uname_os) + case "$os" in + darwin) return 0 ;; + dragonfly) return 0 ;; + freebsd) return 0 ;; + linux) return 0 ;; + android) return 0 ;; + nacl) return 0 ;; + netbsd) return 0 ;; + openbsd) return 0 ;; + plan9) return 0 ;; + solaris) return 0 ;; + windows) return 0 ;; + esac + log_crit "uname_os_check '$(uname -s)' got converted to '$os' which is not a GOOS value. Please file bug at https://github.com/client9/shlib" + return 1 +} +uname_arch_check() { + arch=$(uname_arch) + case "$arch" in + 386) return 0 ;; + amd64) return 0 ;; + arm64) return 0 ;; + armv5) return 0 ;; + armv6) return 0 ;; + armv7) return 0 ;; + ppc64) return 0 ;; + ppc64le) return 0 ;; + mips) return 0 ;; + mipsle) return 0 ;; + mips64) return 0 ;; + mips64le) return 0 ;; + s390x) return 0 ;; + amd64p32) return 0 ;; + esac + log_crit "uname_arch_check '$(uname -m)' got converted to '$arch' which is not a GOARCH value. Please file bug report at https://github.com/client9/shlib" + return 1 +} +untar() { + tarball=$1 + case "${tarball}" in + *.tar.gz | *.tgz) tar -xzf "${tarball}" ;; + *.tar) tar -xf "${tarball}" ;; + *.zip) unzip "${tarball}" ;; + *) + log_err "untar unknown archive format for ${tarball}" + return 1 + ;; + esac +} +mktmpdir() { + test -z "$TMPDIR" && TMPDIR="$(mktemp -d)" + mkdir -p "${TMPDIR}" + echo "${TMPDIR}" +} +http_download_curl() { + local_file=$1 + source_url=$2 + header=$3 + if [ -z "$header" ]; then + code=$(curl -w '%{http_code}' -sL -o "$local_file" "$source_url") + else + code=$(curl -w '%{http_code}' -sL -H "$header" -o "$local_file" "$source_url") + fi + if [ "$code" != "200" ]; then + log_debug "http_download_curl received HTTP status $code" + return 1 + fi + return 0 +} +http_download_wget() { + local_file=$1 + source_url=$2 + header=$3 + if [ -z "$header" ]; then + wget -q -O "$local_file" "$source_url" + else + wget -q --header "$header" -O "$local_file" "$source_url" + fi +} +http_download() { + log_debug "http_download $2" + if is_command curl; then + http_download_curl "$@" + return + elif is_command wget; then + http_download_wget "$@" + return + fi + log_crit "http_download unable to find wget or curl" + return 1 +} +http_copy() { + tmp=$(mktemp) + http_download "${tmp}" "$1" "$2" || return 1 + body=$(cat "$tmp") + rm -f "${tmp}" + echo "$body" +} +github_release() { + owner_repo=$1 + version=$2 + test -z "$version" && version="latest" + giturl="https://github.com/${owner_repo}/releases/${version}" + json=$(http_copy "$giturl" "Accept:application/json") + test -z "$json" && return 1 + version=$(echo "$json" | tr -s '\n' ' ' | sed 's/.*"tag_name":"//' | sed 's/".*//') + test -z "$version" && return 1 + echo "$version" +} +hash_sha256() { + TARGET=${1:-/dev/stdin} + if is_command gsha256sum; then + hash=$(gsha256sum "$TARGET") || return 1 + echo "$hash" | cut -d ' ' -f 1 + elif is_command sha256sum; then + hash=$(sha256sum "$TARGET") || return 1 + echo "$hash" | cut -d ' ' -f 1 + elif is_command shasum; then + hash=$(shasum -a 256 "$TARGET" 2>/dev/null) || return 1 + echo "$hash" | cut -d ' ' -f 1 + elif is_command openssl; then + hash=$(openssl -dst openssl dgst -sha256 "$TARGET") || return 1 + echo "$hash" | cut -d ' ' -f a + else + log_crit "hash_sha256 unable to find command to compute sha-256 hash" + return 1 + fi +} +hash_sha256_verify() { + TARGET=$1 + checksums=$2 + if [ -z "$checksums" ]; then + log_err "hash_sha256_verify checksum file not specified in arg2" + return 1 + fi + BASENAME=${TARGET##*/} + want=$(grep "${BASENAME}" "${checksums}" 2>/dev/null | tr '\t' ' ' | cut -d ' ' -f 1) + if [ -z "$want" ]; then + log_err "hash_sha256_verify unable to find checksum for '${TARGET}' in '${checksums}'" + return 1 + fi + got=$(hash_sha256 "$TARGET") + if [ "$want" != "$got" ]; then + log_err "hash_sha256_verify checksum for '$TARGET' did not verify ${want} vs $got" + return 1 + fi +} +cat /dev/null < 1 { @@ -135,7 +135,7 @@ func (t *Tar) checkHardlink(p string, i os.FileInfo) (bool, string) { return hardlink, linkDst } -func getSyscallStat_t(i os.FileInfo) *syscall.Stat_t { +func getSyscallStatT(i os.FileInfo) *syscall.Stat_t { if sys := i.Sys(); sys != nil { if stat, ok := sys.(*syscall.Stat_t); ok { return stat diff --git a/test.sh b/test.sh index c332c8f29..c86b974cd 100755 --- a/test.sh +++ b/test.sh @@ -31,7 +31,7 @@ echo "Running validation scripts..." scripts=( "hack/boilerplate.sh" "hack/gofmt.sh" - "hack/gometalinter.sh" + "hack/linter.sh" "hack/dep.sh" ) fail=0 From ccb6259b0687226f8c5c83c091c4769035f9a06c Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Sep 2018 13:56:44 -0700 Subject: [PATCH 13/35] More linting errors --- pkg/commands/shell.go | 9 +-------- pkg/dockerfile/dockerfile.go | 6 +++--- pkg/dockerfile/dockerfile_test.go | 3 +-- pkg/util/command_util.go | 5 +---- pkg/util/fs_util.go | 6 ++++-- pkg/util/tar_util.go | 11 +++-------- 6 files changed, 13 insertions(+), 27 deletions(-) diff --git a/pkg/commands/shell.go b/pkg/commands/shell.go index bb763828e..f1bc1ab0b 100644 --- a/pkg/commands/shell.go +++ b/pkg/commands/shell.go @@ -20,7 +20,6 @@ import ( "github.com/GoogleContainerTools/kaniko/pkg/dockerfile" "github.com/google/go-containerregistry/pkg/v1" "github.com/moby/buildkit/frontend/dockerfile/instructions" - "github.com/sirupsen/logrus" ) type ShellCommand struct { @@ -29,13 +28,7 @@ type ShellCommand struct { // ExecuteCommand handles command processing similar to CMD and RUN, func (s *ShellCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.BuildArgs) error { - logrus.Info("cmd: SHELL") - var newShell []string - - newShell = s.cmd.Shell - - logrus.Infof("Replacing Shell in config with %v", newShell) - config.Shell = newShell + config.Shell = s.cmd.Shell return nil } diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 5f37a16cf..00fc71cb7 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -54,8 +54,8 @@ func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) { stage.Name = resolvedBaseName kanikoStages = append(kanikoStages, config.KanikoStage{ Stage: stage, - BaseImageIndex: baseImageIndex(opts, index, stages), - BaseImageStoredLocally: (baseImageIndex(opts, index, stages) != -1), + BaseImageIndex: baseImageIndex(index, stages), + BaseImageStoredLocally: (baseImageIndex(index, stages) != -1), SaveStage: saveStage(index, stages), FinalStage: index == targetStage, }) @@ -68,7 +68,7 @@ func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) { // baseImageIndex returns the index of the stage the current stage is built off // returns -1 if the current stage isn't built off a previous stage -func baseImageIndex(opts *config.KanikoOptions, currentStage int, stages []instructions.Stage) int { +func baseImageIndex(currentStage int, stages []instructions.Stage) int { for i, stage := range stages { if i > currentStage { break diff --git a/pkg/dockerfile/dockerfile_test.go b/pkg/dockerfile/dockerfile_test.go index bd09c26ba..cd83c79ff 100644 --- a/pkg/dockerfile/dockerfile_test.go +++ b/pkg/dockerfile/dockerfile_test.go @@ -20,7 +20,6 @@ import ( "strconv" "testing" - "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/testutil" "github.com/moby/buildkit/frontend/dockerfile/instructions" ) @@ -184,7 +183,7 @@ func Test_baseImageIndex(t *testing.T) { } for _, test := range tests { t.Run(test.name, func(t *testing.T) { - actual := baseImageIndex(&config.KanikoOptions{}, test.currentStage, stages) + actual := baseImageIndex(test.currentStage, stages) if actual != test.expected { t.Fatalf("unexpected result, expected %d got %d", test.expected, actual) } diff --git a/pkg/util/command_util.go b/pkg/util/command_util.go index c8bb50709..b6574faa8 100644 --- a/pkg/util/command_util.go +++ b/pkg/util/command_util.go @@ -228,10 +228,7 @@ func IsSrcRemoteFileURL(rawurl string) bool { return false } _, err = http.Get(rawurl) - if err != nil { - return false - } - return true + return err == nil } func UpdateConfigEnv(newEnvs []instructions.KeyValuePair, config *v1.Config, replacementEnvs []string) error { diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 4c6dd9fff..fa84107be 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -109,7 +109,7 @@ func GetFSFromImage(root string, img v1.Image) error { // DeleteFilesystem deletes the extracted image file system func DeleteFilesystem() error { logrus.Info("Deleting filesystem...") - err := filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, err error) error { + return filepath.Walk(constants.RootDir, func(path string, info os.FileInfo, _ error) error { whitelisted, err := CheckWhitelist(path) if err != nil { return err @@ -123,7 +123,6 @@ func DeleteFilesystem() error { } return os.RemoveAll(path) }) - return err } // ChildDirInWhitelist returns true if there is a child file or directory of the path in the whitelist @@ -310,6 +309,9 @@ func RelativeFiles(fp string, root string) ([]string, error) { fullPath := filepath.Join(root, fp) logrus.Debugf("Getting files and contents at root %s", fullPath) err := filepath.Walk(fullPath, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } whitelisted, err := CheckWhitelist(path) if err != nil { return err diff --git a/pkg/util/tar_util.go b/pkg/util/tar_util.go index a109d4111..bc1cc67a0 100644 --- a/pkg/util/tar_util.go +++ b/pkg/util/tar_util.go @@ -195,10 +195,10 @@ func fileIsCompressedTar(src string) (bool, archive.Compression) { func fileIsUncompressedTar(src string) bool { r, err := os.Open(src) - defer r.Close() if err != nil { return false } + defer r.Close() fi, err := os.Lstat(src) if err != nil { return false @@ -210,13 +210,8 @@ func fileIsUncompressedTar(src string) bool { if tr == nil { return false } - for { - _, err := tr.Next() - if err != nil { - return false - } - return true - } + _, err = tr.Next() + return err == nil } // UnpackCompressedTar unpacks the compressed tar at path to dir From 7635421ae99971346ac61d3c594973c974fde114 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 11 Sep 2018 16:24:21 -0700 Subject: [PATCH 14/35] Add t.Helper() call to checkLayers for better error logging --- integration/integration_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integration/integration_test.go b/integration/integration_test.go index e9601da96..20fd22480 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -245,6 +245,7 @@ func TestLayers(t *testing.T) { } func checkLayers(t *testing.T, image1, image2 string, offset int) { + t.Helper() img1, err := getImageDetails(image1) if err != nil { t.Fatalf("Couldn't get details from image reference for (%s): %s", image1, err) From bf72328611cb9da6f350c64ae5b542b9dfa10f70 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 12 Sep 2018 16:36:53 -0700 Subject: [PATCH 15/35] Addressed code review comment, removed stuttering variable names --- pkg/config/stage.go | 2 +- pkg/dockerfile/dockerfile.go | 2 +- pkg/executor/build.go | 44 ++++++++++++++++++------------------ 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/pkg/config/stage.go b/pkg/config/stage.go index 3acfdc409..7d4e90877 100644 --- a/pkg/config/stage.go +++ b/pkg/config/stage.go @@ -21,7 +21,7 @@ import "github.com/moby/buildkit/frontend/dockerfile/instructions" // KanikoStage wraps a stage of the Dockerfile and provides extra information type KanikoStage struct { instructions.Stage - FinalStage bool + Final bool BaseImageStoredLocally bool BaseImageIndex int SaveStage bool diff --git a/pkg/dockerfile/dockerfile.go b/pkg/dockerfile/dockerfile.go index 5f37a16cf..e1ae51649 100644 --- a/pkg/dockerfile/dockerfile.go +++ b/pkg/dockerfile/dockerfile.go @@ -57,7 +57,7 @@ func Stages(opts *config.KanikoOptions) ([]config.KanikoStage, error) { BaseImageIndex: baseImageIndex(opts, index, stages), BaseImageStoredLocally: (baseImageIndex(opts, index, stages) != -1), SaveStage: saveStage(index, stages), - FinalStage: index == targetStage, + Final: index == targetStage, }) if index == targetStage { break diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 7902c2e3a..06a9e7f23 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -44,8 +44,8 @@ import ( // stageBuilder contains all fields necessary to build one stage of a Dockerfile type stageBuilder struct { stage config.KanikoStage - v1.Image - *v1.ConfigFile + image v1.Image + cf *v1.ConfigFile *snapshot.Snapshotter baseImageDigest string } @@ -76,8 +76,8 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta } return &stageBuilder{ stage: stage, - Image: sourceImage, - ConfigFile: imageConfig, + image: sourceImage, + cf: imageConfig, Snapshotter: snapshotter, baseImageDigest: digest.String(), }, nil @@ -95,36 +95,36 @@ func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) erro return nil } -func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { +func (s *stageBuilder) build(opts *config.KanikoOptions) error { // Unpack file system to root - if err := util.GetFSFromImage(constants.RootDir, s.Image); err != nil { + if err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { return err } // Take initial snapshot if err := s.Snapshotter.Init(); err != nil { return err } - buildArgs := dockerfile.NewBuildArgs(opts.BuildArgs) + args := dockerfile.NewBuildArgs(opts.BuildArgs) for index, cmd := range s.stage.Commands { finalCmd := index == len(s.stage.Commands)-1 - dockerCommand, err := commands.GetCommand(cmd, opts.SrcContext) + command, err := commands.GetCommand(cmd, opts.SrcContext) if err != nil { return err } - if dockerCommand == nil { + if command == nil { continue } - logrus.Info(dockerCommand.String()) - if err := dockerCommand.ExecuteCommand(&s.ConfigFile.Config, buildArgs); err != nil { + logrus.Info(command.String()) + if err := command.ExecuteCommand(&s.cf.Config, args); err != nil { return err } - snapshotFiles := dockerCommand.FilesToSnapshot() + files := command.FilesToSnapshot() var contents []byte // If this is an intermediate stage, we only snapshot for the last command and we // want to snapshot the entire filesystem since we aren't tracking what was changed // by previous commands. - if !s.stage.FinalStage { + if !s.stage.Final { if finalCmd { contents, err = s.Snapshotter.TakeSnapshotFS() } @@ -139,15 +139,15 @@ func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { // Otherwise, in the final stage we take a snapshot at each command. If we know // the files that were changed, we'll snapshot those explicitly, otherwise we'll // check if anything in the filesystem changed. - if snapshotFiles != nil { - contents, err = s.Snapshotter.TakeSnapshot(snapshotFiles) + if files != nil { + contents, err = s.Snapshotter.TakeSnapshot(files) } else { contents, err = s.Snapshotter.TakeSnapshotFS() } } } if err != nil { - return fmt.Errorf("Error taking snapshot of files for command %s: %s", dockerCommand, err) + return fmt.Errorf("Error taking snapshot of files for command %s: %s", command, err) } util.MoveVolumeWhitelistToWhitelist() @@ -163,12 +163,12 @@ func (s *stageBuilder) buildStage(opts *config.KanikoOptions) error { if err != nil { return err } - s.Image, err = mutate.Append(s.Image, + s.image, err = mutate.Append(s.image, mutate.Addendum{ Layer: layer, History: v1.History{ Author: constants.Author, - CreatedBy: dockerCommand.String(), + CreatedBy: command.String(), }, }, ) @@ -187,18 +187,18 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return nil, err } for index, stage := range stages { - stageBuilder, err := newStageBuilder(opts, stage) + sb, err := newStageBuilder(opts, stage) if err != nil { return nil, errors.Wrap(err, fmt.Sprintf("getting stage builder for stage %d", index)) } - if err := stageBuilder.buildStage(opts); err != nil { + if err := sb.build(opts); err != nil { return nil, errors.Wrap(err, "error building stage") } - sourceImage, err := mutate.Config(stageBuilder.Image, stageBuilder.ConfigFile.Config) + sourceImage, err := mutate.Config(sb.image, sb.cf.Config) if err != nil { return nil, err } - if stage.FinalStage { + if stage.Final { sourceImage, err = mutate.CreatedAt(sourceImage, v1.Time{Time: time.Now()}) if err != nil { return nil, err From 7a6dfb6d8b8e1995eec115e539c27166b21c5323 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 12 Sep 2018 17:10:03 -0700 Subject: [PATCH 16/35] Removed incorrect FS extraction from earlier merge with master, and fixed linting errors --- pkg/executor/build.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 9bcbfda1e..b908a0beb 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -85,15 +85,15 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta // key will return a string representation of the build at the cmd // TODO: priyawadhwa@ to fill this out when implementing caching -func (s *stageBuilder) key(cmd string) (string, error) { - return "", nil -} +// func (s *stageBuilder) key(cmd string) (string, error) { +// return "", nil +// } // extractCachedLayer will extract the cached layer and append it to the config file // TODO: priyawadhwa@ to fill this out when implementing caching -func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { - return nil -} +// func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { +// return nil +// } func (s *stageBuilder) build(opts *config.KanikoOptions) error { // Unpack file system to root @@ -114,9 +114,6 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { if command == nil { continue } - if err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { - return err - } logrus.Info(command.String()) if err := command.ExecuteCommand(&s.cf.Config, args); err != nil { return err From c216fbf91b8d6771c416104e7537d37a3920ad6c Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 13 Sep 2018 18:01:43 -0700 Subject: [PATCH 17/35] Add layer caching to kaniko To add layer caching to kaniko, I added two flags: --cache and --use-cache. If --use-cache is set, then the cache will be used, and if --cache is specified then that repo will be used to store cached layers. If --cache isn't set, a cache will be inferred from the destination provided. Currently, caching only works for RUN commands. Before executing the command, kaniko checks if the cached layer exists. If it does, it pulls it and extracts it. It then adds those files to the snapshotter and append a layer to the config history. If the cached layer does not exist, kaniko executes the command and pushes the newly created layer to the cache. All cached layers are tagged with a stable key, which is built based off of: 1. The base image digest 2. The current state of the filesystem 3. The current command being run 4. The current config file (to account for metadata changes) I also added two integration tests to make sure caching works 1. Dockerfile_test_cache runs 'date', which should be exactly the same the second time the image is built 2. Dockerfile_test_cache_install makes sure apt-get install can be reproduced --- cmd/executor/cmd/root.go | 2 + integration/dockerfiles/Dockerfile_test_cache | 22 +++++ .../dockerfiles/Dockerfile_test_cache_install | 20 +++++ integration/images.go | 53 ++++++++++-- integration/integration_test.go | 84 ++++++++++++++----- pkg/cache/cache.go | 70 ++++++++++++++++ pkg/config/options.go | 2 + pkg/executor/build.go | 75 ++++++++++++++--- pkg/executor/push.go | 30 +++++++ pkg/util/fs_util.go | 26 +++--- 10 files changed, 338 insertions(+), 46 deletions(-) create mode 100644 integration/dockerfiles/Dockerfile_test_cache create mode 100644 integration/dockerfiles/Dockerfile_test_cache_install create mode 100644 pkg/cache/cache.go diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 97511e092..32c99af91 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -92,6 +92,8 @@ func addKanikoOptionsFlags(cmd *cobra.Command) { RootCmd.PersistentFlags().BoolVarP(&opts.Reproducible, "reproducible", "", false, "Strip timestamps out of the image to make it reproducible") RootCmd.PersistentFlags().StringVarP(&opts.Target, "target", "", "", "Set the target build stage to build") RootCmd.PersistentFlags().BoolVarP(&opts.NoPush, "no-push", "", false, "Do not push the image to the registry") + RootCmd.PersistentFlags().StringVarP(&opts.Cache, "cache", "", "", "Specify a registry to use as a chace, otherwise one will be inferred from the destination provided") + RootCmd.PersistentFlags().BoolVarP(&opts.UseCache, "use-cache", "", true, "Use cache when building image") } // addHiddenFlags marks certain flags as hidden from the executor help text diff --git a/integration/dockerfiles/Dockerfile_test_cache b/integration/dockerfiles/Dockerfile_test_cache new file mode 100644 index 000000000..215da54e7 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_cache @@ -0,0 +1,22 @@ +# Copyright 2018 Google, Inc. All rights reserved. +# +# 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. + +# Test to make sure the cache works properly +# /date should be the same regardless of when this image is built +# if the cache is implemented correctly + +FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0 +RUN date > /date +COPY context/foo /foo +RUN echo hey diff --git a/integration/dockerfiles/Dockerfile_test_cache_install b/integration/dockerfiles/Dockerfile_test_cache_install new file mode 100644 index 000000000..8a6a9a58e --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_cache_install @@ -0,0 +1,20 @@ +# Copyright 2018 Google, Inc. All rights reserved. +# +# 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. + +# Test to make sure the cache works properly +# /date should be the same regardless of when this image is built +# if the cache is implemented correctly + +FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0 +RUN apt-get update && apt-get install -y make diff --git a/integration/images.go b/integration/images.go index af48d2553..6cc8930db 100644 --- a/integration/images.go +++ b/integration/images.go @@ -23,6 +23,7 @@ import ( "path" "path/filepath" "runtime" + "strconv" "strings" ) @@ -77,12 +78,16 @@ func GetKanikoImage(imageRepo, dockerfile string) string { return strings.ToLower(imageRepo + kanikoPrefix + dockerfile) } +// GetVersionedKanikoImage versions constructs the name of the kaniko image that would be built +// with the dockerfile and versions it for cache testing +func GetVersionedKanikoImage(imageRepo, dockerfile string, version int) string { + return strings.ToLower(imageRepo + kanikoPrefix + dockerfile + strconv.Itoa(version)) +} + // FindDockerFiles will look for test docker files in the directory dockerfilesPath. // These files must start with `Dockerfile_test`. If the file is one we are intentionally // skipping, it will not be included in the returned list. func FindDockerFiles(dockerfilesPath string) ([]string, error) { - // TODO: remove test_user_run from this when https://github.com/GoogleContainerTools/container-diff/issues/237 is fixed - testsToIgnore := map[string]bool{"Dockerfile_test_user_run": true} allDockerfiles, err := filepath.Glob(path.Join(dockerfilesPath, "Dockerfile_test*")) if err != nil { return []string{}, fmt.Errorf("Failed to find docker files at %s: %s", dockerfilesPath, err) @@ -92,9 +97,8 @@ func FindDockerFiles(dockerfilesPath string) ([]string, error) { for _, dockerfile := range allDockerfiles { // Remove the leading directory from the path dockerfile = dockerfile[len("dockerfiles/"):] - if !testsToIgnore[dockerfile] { - dockerfiles = append(dockerfiles, dockerfile) - } + dockerfiles = append(dockerfiles, dockerfile) + } return dockerfiles, err } @@ -103,7 +107,9 @@ func FindDockerFiles(dockerfilesPath string) ([]string, error) { // keeps track of which files have been built. type DockerFileBuilder struct { // Holds all available docker files and whether or not they've been built - FilesBuilt map[string]bool + FilesBuilt map[string]bool + DockerfilesToIgnore map[string]struct{} + TestCacheDockerfiles map[string]struct{} } // NewDockerFileBuilder will create a DockerFileBuilder initialized with dockerfiles, which @@ -113,6 +119,14 @@ func NewDockerFileBuilder(dockerfiles []string) *DockerFileBuilder { for _, f := range dockerfiles { d.FilesBuilt[f] = false } + d.DockerfilesToIgnore = map[string]struct{}{ + // TODO: remove test_user_run from this when https://github.com/GoogleContainerTools/container-diff/issues/237 is fixed + "Dockerfile_test_user_run": {}, + } + d.TestCacheDockerfiles = map[string]struct{}{ + "Dockerfile_test_cache": {}, + "Dockerfile_test_cache_install": {}, + } return &d } @@ -164,6 +178,7 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do } } + cacheFlag := "--use-cache=false" // build kaniko image additionalFlags = append(buildArgs, additionalKanikoFlagsMap[dockerfile]...) kanikoImage := GetKanikoImage(imageRepo, dockerfile) @@ -174,6 +189,7 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do ExecutorImage, "-f", path.Join(buildContextPath, dockerfilesPath, dockerfile), "-d", kanikoImage, reproducibleFlag, + cacheFlag, contextFlag, contextPath}, additionalFlags...)..., ) @@ -186,3 +202,28 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do d.FilesBuilt[dockerfile] = true return nil } + +// buildCachedImages builds the images for testing caching via kaniko where version is the nth time this image has been built +func (d *DockerFileBuilder) buildCachedImages(imageRepo, cache, dockerfilesPath, dockerfile string, version int) error { + _, ex, _, _ := runtime.Caller(0) + cwd := filepath.Dir(ex) + + for dockerfile := range d.TestCacheDockerfiles { + kanikoImage := GetVersionedKanikoImage(imageRepo, dockerfile, version) + kanikoCmd := exec.Command("docker", + append([]string{"run", + "-v", os.Getenv("HOME") + "/.config/gcloud:/root/.config/gcloud", + "-v", cwd + ":/workspace", + ExecutorImage, + "-f", path.Join(buildContextPath, dockerfilesPath, dockerfile), + "-d", kanikoImage, + "-c", buildContextPath, + "--cache", cache})..., + ) + + if _, err := RunCommandWithoutTest(kanikoCmd); err != nil { + return fmt.Errorf("Failed to build cached image %s with kaniko command \"%s\": %s", kanikoImage, kanikoCmd.Args, err) + } + } + return nil +} diff --git a/integration/integration_test.go b/integration/integration_test.go index 20fd22480..328fd39f8 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -24,8 +24,10 @@ import ( "math" "os" "os/exec" + "path/filepath" "strings" "testing" + "time" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/daemon" @@ -148,6 +150,7 @@ func TestMain(m *testing.M) { fmt.Printf("error building onbuild base: %v", err) os.Exit(1) } + pushOnbuildBase := exec.Command("docker", "push", config.onbuildBaseImage) if err := pushOnbuildBase.Run(); err != nil { fmt.Printf("error pushing onbuild base %s: %v", config.onbuildBaseImage, err) @@ -165,7 +168,6 @@ func TestMain(m *testing.M) { fmt.Printf("error pushing hardlink base %s: %v", config.hardlinkBaseImage, err) os.Exit(1) } - dockerfiles, err := FindDockerFiles(dockerfilesPath) if err != nil { fmt.Printf("Coudn't create map of dockerfiles: %s", err) @@ -177,6 +179,12 @@ func TestMain(m *testing.M) { func TestRun(t *testing.T) { for dockerfile, built := range imageBuilder.FilesBuilt { t.Run("test_"+dockerfile, func(t *testing.T) { + if _, ok := imageBuilder.DockerfilesToIgnore[dockerfile]; ok { + t.SkipNow() + } + if _, ok := imageBuilder.TestCacheDockerfiles[dockerfile]; ok { + t.SkipNow() + } if !built { err := imageBuilder.BuildImage(config.imageRepo, config.gcsBucket, dockerfilesPath, dockerfile) if err != nil { @@ -195,25 +203,8 @@ func TestRun(t *testing.T) { t.Logf("diff = %s", string(diff)) expected := fmt.Sprintf(emptyContainerDiff, dockerImage, kanikoImage, dockerImage, kanikoImage) + checkContainerDiffOutput(t, diff, expected) - // Let's compare the json objects themselves instead of strings to avoid - // issues with spaces and indents - var diffInt interface{} - var expectedInt interface{} - - err := json.Unmarshal(diff, &diffInt) - if err != nil { - t.Error(err) - t.Fail() - } - - err = json.Unmarshal([]byte(expected), &expectedInt) - if err != nil { - t.Error(err) - t.Fail() - } - - testutil.CheckErrorAndDeepEqual(t, false, nil, expectedInt, diffInt) }) } } @@ -228,6 +219,9 @@ func TestLayers(t *testing.T) { } for dockerfile, built := range imageBuilder.FilesBuilt { t.Run("test_layer_"+dockerfile, func(t *testing.T) { + if _, ok := imageBuilder.DockerfilesToIgnore[dockerfile]; ok { + t.SkipNow() + } if !built { err := imageBuilder.BuildImage(config.imageRepo, config.gcsBucket, dockerfilesPath, dockerfile) if err != nil { @@ -244,6 +238,58 @@ func TestLayers(t *testing.T) { } } +// Build each image with kaniko twice, and then make sure they're exactly the same +func TestCache(t *testing.T) { + for dockerfile := range imageBuilder.TestCacheDockerfiles { + t.Run("test_cache_"+dockerfile, func(t *testing.T) { + cache := filepath.Join(config.imageRepo, "cache", fmt.Sprintf("%v", time.Now().UnixNano())) + // Build the initial image which will cache layers + if err := imageBuilder.buildCachedImages(config.imageRepo, cache, dockerfilesPath, dockerfile, 0); err != nil { + t.Fatalf("error building cached image for the first time: %v", err) + } + // Build the second image which should pull from the cache + if err := imageBuilder.buildCachedImages(config.imageRepo, cache, dockerfilesPath, dockerfile, 1); err != nil { + t.Fatalf("error building cached image for the first time: %v", err) + } + // Make sure both images are the same + kanikoVersion0 := GetVersionedKanikoImage(config.imageRepo, dockerfile, 0) + kanikoVersion1 := GetVersionedKanikoImage(config.imageRepo, dockerfile, 1) + + // container-diff + containerdiffCmd := exec.Command("container-diff", "diff", + kanikoVersion0, kanikoVersion1, + "-q", "--type=file", "--type=metadata", "--json") + + diff := RunCommand(containerdiffCmd, t) + t.Logf("diff = %s", diff) + + expected := fmt.Sprintf(emptyContainerDiff, kanikoVersion0, kanikoVersion1, kanikoVersion0, kanikoVersion1) + checkContainerDiffOutput(t, diff, expected) + }) + } +} + +func checkContainerDiffOutput(t *testing.T, diff []byte, expected string) { + // Let's compare the json objects themselves instead of strings to avoid + // issues with spaces and indents + t.Helper() + + var diffInt interface{} + var expectedInt interface{} + + err := json.Unmarshal(diff, &diffInt) + if err != nil { + t.Error(err) + } + + err = json.Unmarshal([]byte(expected), &expectedInt) + if err != nil { + t.Error(err) + } + + testutil.CheckErrorAndDeepEqual(t, false, nil, expectedInt, diffInt) +} + func checkLayers(t *testing.T, image1, image2 string, offset int) { t.Helper() img1, err := getImageDetails(image1) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go new file mode 100644 index 000000000..08909ce94 --- /dev/null +++ b/pkg/cache/cache.go @@ -0,0 +1,70 @@ +/* +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. +*/ + +package cache + +import ( + "fmt" + + "github.com/GoogleContainerTools/kaniko/pkg/config" + "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/authn/k8schain" + "github.com/google/go-containerregistry/pkg/name" + "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" +) + +// RetrieveLayer checks the specified cache for a layer with the tag :cacheKey +func RetrieveLayer(opts *config.KanikoOptions, cacheKey string) (v1.Image, error) { + cache, err := Destination(opts, cacheKey) + if err != nil { + return nil, errors.Wrap(err, "getting cache destination") + } + logrus.Infof("Checking for cached layer %s...", cache) + + cacheRef, err := name.NewTag(cache, name.WeakValidation) + if err != nil { + return nil, errors.Wrap(err, fmt.Sprintf("getting reference for %s", cache)) + } + k8sc, err := k8schain.NewNoClient() + if err != nil { + return nil, err + } + kc := authn.NewMultiKeychain(authn.DefaultKeychain, k8sc) + img, err := remote.Image(cacheRef, remote.WithAuthFromKeychain(kc)) + if err != nil { + return nil, err + } + _, err = img.Layers() + return img, err +} + +// Destination returns the repo where the layer should be stored +// If no cache is specified, one is inferred from the destination provided +func Destination(opts *config.KanikoOptions, cacheKey string) (string, error) { + cache := opts.Cache + if cache == "" { + destination := opts.Destinations[0] + destRef, err := name.NewTag(destination, name.WeakValidation) + if err != nil { + return "", errors.Wrap(err, "getting tag for destination") + } + return fmt.Sprintf("%s/cache", destRef.Context()), nil + } + return fmt.Sprintf("%s:%s", cache, cacheKey), nil +} diff --git a/pkg/config/options.go b/pkg/config/options.go index 18e1d2028..3c84fa7bb 100644 --- a/pkg/config/options.go +++ b/pkg/config/options.go @@ -24,6 +24,7 @@ type KanikoOptions struct { Bucket string TarPath string Target string + Cache string Destinations multiArg BuildArgs multiArg InsecurePush bool @@ -31,4 +32,5 @@ type KanikoOptions struct { SingleSnapshot bool Reproducible bool NoPush bool + UseCache bool } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index b908a0beb..b4f794d0b 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -18,6 +18,7 @@ package executor import ( "bytes" + "encoding/json" "fmt" "io" "io/ioutil" @@ -33,6 +34,7 @@ import ( "github.com/pkg/errors" "github.com/sirupsen/logrus" + "github.com/GoogleContainerTools/kaniko/pkg/cache" "github.com/GoogleContainerTools/kaniko/pkg/commands" "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/constants" @@ -84,20 +86,52 @@ func newStageBuilder(opts *config.KanikoOptions, stage config.KanikoStage) (*sta } // key will return a string representation of the build at the cmd -// TODO: priyawadhwa@ to fill this out when implementing caching -// func (s *stageBuilder) key(cmd string) (string, error) { -// return "", nil -// } +func (s *stageBuilder) key(cmd string) (string, error) { + fsKey, err := s.snapshotter.Key() + if err != nil { + return "", err + } + c := bytes.NewBuffer([]byte{}) + enc := json.NewEncoder(c) + enc.Encode(s.cf) + cf, err := util.SHA256(c) + if err != nil { + return "", err + } + logrus.Debugf("%s\n%s\n%s\n%s\n", s.baseImageDigest, fsKey, cf, cmd) + return util.SHA256(bytes.NewReader([]byte(s.baseImageDigest + fsKey + cf + cmd))) +} // extractCachedLayer will extract the cached layer and append it to the config file -// TODO: priyawadhwa@ to fill this out when implementing caching -// func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { -// return nil -// } +func (s *stageBuilder) extractCachedLayer(layer v1.Image, createdBy string) error { + logrus.Infof("Found cached layer, extracting to filesystem") + extractedFiles, err := util.GetFSFromImage(constants.RootDir, layer) + if err != nil { + return errors.Wrap(err, "extracting fs from image") + } + if _, err := s.snapshotter.TakeSnapshot(extractedFiles); err != nil { + return err + } + logrus.Infof("Appending cached layer to base image") + l, err := layer.Layers() + if err != nil { + return errors.Wrap(err, "getting cached layer from image") + } + s.image, err = mutate.Append(s.image, + mutate.Addendum{ + Layer: l[0], + History: v1.History{ + Author: constants.Author, + CreatedBy: createdBy, + }, + }, + ) + return err +} func (s *stageBuilder) build(opts *config.KanikoOptions) error { // Unpack file system to root - if err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { + if _, err := util.GetFSFromImage(constants.RootDir, s.image); err != nil { return err } // Take initial snapshot @@ -115,6 +149,20 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { continue } logrus.Info(command.String()) + cacheKey, err := s.key(command.String()) + if err != nil { + return errors.Wrap(err, "getting key") + } + if command.CacheCommand() && opts.UseCache { + image, err := cache.RetrieveLayer(opts, cacheKey) + if err == nil { + if err := s.extractCachedLayer(image, command.String()); err != nil { + return errors.Wrap(err, "extracting cached layer") + } + continue + } + logrus.Info("No cached layer found, executing command...") + } if err := command.ExecuteCommand(&s.cf.Config, args); err != nil { return err } @@ -163,6 +211,12 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { if err != nil { return err } + // Push layer to cache now along with new config file + if command.CacheCommand() && opts.UseCache { + if err := pushLayerToCache(opts, cacheKey, layer, command.String()); err != nil { + return err + } + } s.image, err = mutate.Append(s.image, mutate.Addendum{ Layer: layer, @@ -233,7 +287,8 @@ func extractImageToDependecyDir(index int, image v1.Image) error { return err } logrus.Infof("trying to extract to %s", dependencyDir) - return util.GetFSFromImage(dependencyDir, image) + _, err := util.GetFSFromImage(dependencyDir, image) + return err } func saveStageAsTarball(stageIndex int, image v1.Image) error { diff --git a/pkg/executor/push.go b/pkg/executor/push.go index 53799b71a..aa68c723c 100644 --- a/pkg/executor/push.go +++ b/pkg/executor/push.go @@ -21,12 +21,16 @@ import ( "fmt" "net/http" + "github.com/GoogleContainerTools/kaniko/pkg/cache" "github.com/GoogleContainerTools/kaniko/pkg/config" + "github.com/GoogleContainerTools/kaniko/pkg/constants" "github.com/GoogleContainerTools/kaniko/pkg/version" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/authn/k8schain" "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1" + "github.com/google/go-containerregistry/pkg/v1/empty" + "github.com/google/go-containerregistry/pkg/v1/mutate" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/tarball" "github.com/pkg/errors" @@ -100,3 +104,29 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error { } return nil } + +// pushLayerToCache pushes layer (tagged with cacheKey) to opts.Cache +// if opts.Cache doesn't exist, infer the cache from the given destination +func pushLayerToCache(opts *config.KanikoOptions, cacheKey string, layer v1.Layer, createdBy string) error { + cache, err := cache.Destination(opts, cacheKey) + if err != nil { + return errors.Wrap(err, "getting cache destination") + } + logrus.Infof("Pushing layer %s to cache now", cache) + empty := empty.Image + empty, err = mutate.Append(empty, + mutate.Addendum{ + Layer: layer, + History: v1.History{ + Author: constants.Author, + CreatedBy: createdBy, + }, + }, + ) + if err != nil { + return errors.Wrap(err, "appending layer onto empty image") + } + return DoPush(empty, &config.KanikoOptions{ + Destinations: []string{cache}, + }) +} diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index d1136d7bc..3e45c69c1 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -46,22 +46,25 @@ var whitelist = []string{ } var volumeWhitelist = []string{} -func GetFSFromImage(root string, img v1.Image) 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) { whitelist, err := fileSystemWhitelist(constants.WhitelistPath) if err != nil { - return err + return nil, err } - logrus.Infof("Mounted directories: %v", whitelist) + logrus.Debugf("Mounted directories: %v", whitelist) layers, err := img.Layers() if err != nil { - return err + return nil, err } + extractedFiles := []string{} for i, l := range layers { logrus.Infof("Extracting layer %d", i) r, err := l.Uncompressed() if err != nil { - return err + return nil, err } tr := tar.NewReader(r) for { @@ -70,7 +73,7 @@ func GetFSFromImage(root string, img v1.Image) error { break } if err != nil { - return err + return nil, err } path := filepath.Join(root, filepath.Clean(hdr.Name)) base := filepath.Base(path) @@ -79,13 +82,13 @@ func GetFSFromImage(root string, img v1.Image) error { logrus.Debugf("Whiting out %s", path) name := strings.TrimPrefix(base, ".wh.") if err := os.RemoveAll(filepath.Join(dir, name)); err != nil { - return errors.Wrapf(err, "removing whiteout %s", hdr.Name) + return nil, errors.Wrapf(err, "removing whiteout %s", hdr.Name) } continue } whitelisted, err := CheckWhitelist(path) if err != nil { - return err + return nil, err } if whitelisted && !checkWhitelistRoot(root) { logrus.Debugf("Not adding %s because it is whitelisted", path) @@ -94,7 +97,7 @@ func GetFSFromImage(root string, img v1.Image) error { if hdr.Typeflag == tar.TypeSymlink { whitelisted, err := CheckWhitelist(hdr.Linkname) if err != nil { - return err + return nil, err } if whitelisted { logrus.Debugf("skipping symlink from %s to %s because %s is whitelisted", hdr.Linkname, path, hdr.Linkname) @@ -102,11 +105,12 @@ func GetFSFromImage(root string, img v1.Image) error { } } if err := extractFile(root, hdr, tr); err != nil { - return err + return nil, err } + extractedFiles = append(extractedFiles, filepath.Join(root, filepath.Clean(hdr.Name))) } } - return nil + return extractedFiles, nil } // DeleteFilesystem deletes the extracted image file system From eb7194a16538e44f0591454f73c6c55b2e45fb96 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 13 Sep 2018 22:06:38 -0700 Subject: [PATCH 18/35] Fix linting errors --- integration/images.go | 2 +- integration/integration_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/images.go b/integration/images.go index 6cc8930db..61fc6a7ac 100644 --- a/integration/images.go +++ b/integration/images.go @@ -204,7 +204,7 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do } // buildCachedImages builds the images for testing caching via kaniko where version is the nth time this image has been built -func (d *DockerFileBuilder) buildCachedImages(imageRepo, cache, dockerfilesPath, dockerfile string, version int) error { +func (d *DockerFileBuilder) buildCachedImages(imageRepo, cache, dockerfilesPath string, version int) error { _, ex, _, _ := runtime.Caller(0) cwd := filepath.Dir(ex) diff --git a/integration/integration_test.go b/integration/integration_test.go index 328fd39f8..de2409ed7 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -244,11 +244,11 @@ func TestCache(t *testing.T) { t.Run("test_cache_"+dockerfile, func(t *testing.T) { cache := filepath.Join(config.imageRepo, "cache", fmt.Sprintf("%v", time.Now().UnixNano())) // Build the initial image which will cache layers - if err := imageBuilder.buildCachedImages(config.imageRepo, cache, dockerfilesPath, dockerfile, 0); err != nil { + if err := imageBuilder.buildCachedImages(config.imageRepo, cache, dockerfilesPath, 0); err != nil { t.Fatalf("error building cached image for the first time: %v", err) } // Build the second image which should pull from the cache - if err := imageBuilder.buildCachedImages(config.imageRepo, cache, dockerfilesPath, dockerfile, 1); err != nil { + if err := imageBuilder.buildCachedImages(config.imageRepo, cache, dockerfilesPath, 1); err != nil { t.Fatalf("error building cached image for the first time: %v", err) } // Make sure both images are the same From f7ba67ea2573ad504f1e1e7222ce1d9eb12a16ed Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 14 Sep 2018 09:53:03 -0700 Subject: [PATCH 19/35] Specify cache key to differentiate cache layers --- pkg/cache/cache.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 08909ce94..198ec6651 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -64,7 +64,7 @@ func Destination(opts *config.KanikoOptions, cacheKey string) (string, error) { if err != nil { return "", errors.Wrap(err, "getting tag for destination") } - return fmt.Sprintf("%s/cache", destRef.Context()), nil + return fmt.Sprintf("%s/cache:%s", destRef.Context(), cacheKey), nil } return fmt.Sprintf("%s:%s", cache, cacheKey), nil } From 49d7c7c0ee799a08b808bd9e5ca27681ce2ef96f Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 14 Sep 2018 11:51:01 -0700 Subject: [PATCH 20/35] Suppress usage upon Run error I changed RunE to Run so that usage wouldn't show upon error. Usage will still show if PersistentPreRunE fails, which makes sense since those functions check to make sure arguments passed in are valid. Also changed logging of multi arg flags to Debugf so that output would be cleaner. --- cmd/executor/cmd/root.go | 18 +++++++++++++----- pkg/config/args.go | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 97511e092..e5e0f8647 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -17,6 +17,7 @@ limitations under the License. package cmd import ( + "fmt" "os" "path/filepath" "strings" @@ -59,21 +60,23 @@ var RootCmd = &cobra.Command{ } return resolveDockerfilePath() }, - RunE: func(cmd *cobra.Command, args []string) error { + Run: func(cmd *cobra.Command, args []string) { if !checkContained() { if !force { - return errors.New("kaniko should only be run inside of a container, run with the --force flag if you are sure you want to continue") + exit(errors.New("kaniko should only be run inside of a container, run with the --force flag if you are sure you want to continue")) } logrus.Warn("kaniko is being run outside of a container. This can have dangerous effects on your system") } if err := os.Chdir("/"); err != nil { - return errors.Wrap(err, "error changing to root dir") + exit(errors.Wrap(err, "error changing to root dir")) } image, err := executor.DoBuild(opts) if err != nil { - return errors.Wrap(err, "error building image") + exit(errors.Wrap(err, "error building image")) + } + if err := executor.DoPush(image, opts); err != nil { + exit(errors.Wrap(err, "error pushing image")) } - return executor.DoPush(image, opts) }, } @@ -158,3 +161,8 @@ func resolveSourceContext() error { logrus.Debugf("Build context located at %s", opts.SrcContext) return nil } + +func exit(err error) { + fmt.Println(err) + os.Exit(1) +} diff --git a/pkg/config/args.go b/pkg/config/args.go index ae45b266c..44ac074e3 100644 --- a/pkg/config/args.go +++ b/pkg/config/args.go @@ -34,7 +34,7 @@ func (b *multiArg) String() string { // The second method is Set(value string) error func (b *multiArg) Set(value string) error { - logrus.Infof("appending to multi args %s", value) + logrus.Debugf("appending to multi args %s", value) *b = append(*b, value) return nil } From 177bd4f40e433fac13cbe96db1077034cc3d7144 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 17 Sep 2018 11:05:57 +0100 Subject: [PATCH 21/35] Fix typo and update comments --- cmd/executor/cmd/root.go | 2 +- integration/dockerfiles/Dockerfile_test_cache | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 32c99af91..351595966 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -92,7 +92,7 @@ func addKanikoOptionsFlags(cmd *cobra.Command) { RootCmd.PersistentFlags().BoolVarP(&opts.Reproducible, "reproducible", "", false, "Strip timestamps out of the image to make it reproducible") RootCmd.PersistentFlags().StringVarP(&opts.Target, "target", "", "", "Set the target build stage to build") RootCmd.PersistentFlags().BoolVarP(&opts.NoPush, "no-push", "", false, "Do not push the image to the registry") - RootCmd.PersistentFlags().StringVarP(&opts.Cache, "cache", "", "", "Specify a registry to use as a chace, otherwise one will be inferred from the destination provided") + RootCmd.PersistentFlags().StringVarP(&opts.Cache, "cache", "", "", "Specify a registry to use as a cache, otherwise one will be inferred from the destination provided") RootCmd.PersistentFlags().BoolVarP(&opts.UseCache, "use-cache", "", true, "Use cache when building image") } diff --git a/integration/dockerfiles/Dockerfile_test_cache b/integration/dockerfiles/Dockerfile_test_cache index 215da54e7..e3ebe304d 100644 --- a/integration/dockerfiles/Dockerfile_test_cache +++ b/integration/dockerfiles/Dockerfile_test_cache @@ -13,7 +13,7 @@ # limitations under the License. # Test to make sure the cache works properly -# /date should be the same regardless of when this image is built +# If the image is built twice, /date should be the same in both images # if the cache is implemented correctly FROM gcr.io/google-appengine/debian9@sha256:1d6a9a6d106bd795098f60f4abb7083626354fa6735e81743c7f8cfca11259f0 From cd1b957e4327d4b58fcf527d906cf1fbbbb0ae42 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 17 Sep 2018 11:11:51 +0100 Subject: [PATCH 22/35] Address code review comments; review unnecessary error check --- pkg/constants/constants.go | 6 +++++- pkg/executor/build.go | 11 ++++------- pkg/executor/build_test.go | 4 ++-- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index bcc658fca..d8fcc722e 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -55,9 +55,13 @@ const ( S3BuildContextPrefix = "s3://" LocalDirBuildContextPrefix = "dir://" + HOME = "HOME" // DefaultHOMEValue is the default value Docker sets for $HOME - HOME = "HOME" DefaultHOMEValue = "/root" + + // Docker command names + Cmd = "cmd" + Entrypoint = "entrypoint" ) // KanikoBuildFiles is the list of files required to build kaniko diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 8b9659a76..923741126 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -145,9 +145,7 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return nil, err } } - if err := reviewConfig(stage, &imageConfig.Config); err != nil { - return nil, err - } + reviewConfig(stage, &imageConfig.Config) sourceImage, err = mutate.Config(sourceImage, imageConfig.Config) if err != nil { return nil, err @@ -235,20 +233,19 @@ func resolveOnBuild(stage *config.KanikoStage, config *v1.Config) error { // reviewConfig makes sure the value of CMD is correct after building the stage // If ENTRYPOINT was set in this stage but CMD wasn't, then CMD should be cleared out // See Issue #346 for more info -func reviewConfig(stage config.KanikoStage, config *v1.Config) error { +func reviewConfig(stage config.KanikoStage, config *v1.Config) { entrypoint := false cmd := false for _, c := range stage.Commands { - if c.Name() == "cmd" { + if c.Name() == constants.Cmd { cmd = true } - if c.Name() == "entrypoint" { + if c.Name() == constants.Entrypoint { entrypoint = true } } if entrypoint && !cmd { config.Cmd = nil } - return nil } diff --git a/pkg/executor/build_test.go b/pkg/executor/build_test.go index 51eca6f99..edf162261 100644 --- a/pkg/executor/build_test.go +++ b/pkg/executor/build_test.go @@ -59,8 +59,8 @@ func Test_reviewConfig(t *testing.T) { Cmd: test.originalCmd, Entrypoint: test.originalEntrypoint, } - err := reviewConfig(stage(t, test.dockerfile), config) - testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedCmd, config.Cmd) + reviewConfig(stage(t, test.dockerfile), config) + testutil.CheckErrorAndDeepEqual(t, false, nil, test.expectedCmd, config.Cmd) }) } } From e2ca1152f4b538d3623a5776e38294f1ecc6474b Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 24 Sep 2018 13:18:42 -0700 Subject: [PATCH 23/35] Rename flags and default caching to false Rename --use-cache to --cache, and --cache to --cache-repo to clarify what the flags are used for. Default caching to false. --- cmd/executor/cmd/root.go | 4 ++-- integration/images.go | 9 +++++---- pkg/cache/cache.go | 2 +- pkg/config/options.go | 4 ++-- pkg/executor/build.go | 4 ++-- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index 351595966..1b2cf06ff 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -92,8 +92,8 @@ func addKanikoOptionsFlags(cmd *cobra.Command) { RootCmd.PersistentFlags().BoolVarP(&opts.Reproducible, "reproducible", "", false, "Strip timestamps out of the image to make it reproducible") RootCmd.PersistentFlags().StringVarP(&opts.Target, "target", "", "", "Set the target build stage to build") RootCmd.PersistentFlags().BoolVarP(&opts.NoPush, "no-push", "", false, "Do not push the image to the registry") - RootCmd.PersistentFlags().StringVarP(&opts.Cache, "cache", "", "", "Specify a registry to use as a cache, otherwise one will be inferred from the destination provided") - RootCmd.PersistentFlags().BoolVarP(&opts.UseCache, "use-cache", "", true, "Use cache when building image") + RootCmd.PersistentFlags().StringVarP(&opts.CacheRepo, "cache-repo", "", "", "Specify a repository to use as a cache, otherwise one will be inferred from the destination provided") + RootCmd.PersistentFlags().BoolVarP(&opts.Cache, "cache", "", false, "Use cache when building image") } // addHiddenFlags marks certain flags as hidden from the executor help text diff --git a/integration/images.go b/integration/images.go index 61fc6a7ac..cf1f90105 100644 --- a/integration/images.go +++ b/integration/images.go @@ -178,7 +178,6 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do } } - cacheFlag := "--use-cache=false" // build kaniko image additionalFlags = append(buildArgs, additionalKanikoFlagsMap[dockerfile]...) kanikoImage := GetKanikoImage(imageRepo, dockerfile) @@ -189,7 +188,6 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do ExecutorImage, "-f", path.Join(buildContextPath, dockerfilesPath, dockerfile), "-d", kanikoImage, reproducibleFlag, - cacheFlag, contextFlag, contextPath}, additionalFlags...)..., ) @@ -204,10 +202,12 @@ func (d *DockerFileBuilder) BuildImage(imageRepo, gcsBucket, dockerfilesPath, do } // buildCachedImages builds the images for testing caching via kaniko where version is the nth time this image has been built -func (d *DockerFileBuilder) buildCachedImages(imageRepo, cache, dockerfilesPath string, version int) error { +func (d *DockerFileBuilder) buildCachedImages(imageRepo, cacheRepo, dockerfilesPath string, version int) error { _, ex, _, _ := runtime.Caller(0) cwd := filepath.Dir(ex) + cacheFlag := "--cache=true" + for dockerfile := range d.TestCacheDockerfiles { kanikoImage := GetVersionedKanikoImage(imageRepo, dockerfile, version) kanikoCmd := exec.Command("docker", @@ -218,7 +218,8 @@ func (d *DockerFileBuilder) buildCachedImages(imageRepo, cache, dockerfilesPath "-f", path.Join(buildContextPath, dockerfilesPath, dockerfile), "-d", kanikoImage, "-c", buildContextPath, - "--cache", cache})..., + cacheFlag, + "--cache-repo", cacheRepo})..., ) if _, err := RunCommandWithoutTest(kanikoCmd); err != nil { diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go index 198ec6651..69682ba48 100644 --- a/pkg/cache/cache.go +++ b/pkg/cache/cache.go @@ -57,7 +57,7 @@ func RetrieveLayer(opts *config.KanikoOptions, cacheKey string) (v1.Image, error // Destination returns the repo where the layer should be stored // If no cache is specified, one is inferred from the destination provided func Destination(opts *config.KanikoOptions, cacheKey string) (string, error) { - cache := opts.Cache + cache := opts.CacheRepo if cache == "" { destination := opts.Destinations[0] destRef, err := name.NewTag(destination, name.WeakValidation) diff --git a/pkg/config/options.go b/pkg/config/options.go index 3c84fa7bb..c9bad39e6 100644 --- a/pkg/config/options.go +++ b/pkg/config/options.go @@ -24,7 +24,7 @@ type KanikoOptions struct { Bucket string TarPath string Target string - Cache string + CacheRepo string Destinations multiArg BuildArgs multiArg InsecurePush bool @@ -32,5 +32,5 @@ type KanikoOptions struct { SingleSnapshot bool Reproducible bool NoPush bool - UseCache bool + Cache bool } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index b4f794d0b..49313900b 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -153,7 +153,7 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { if err != nil { return errors.Wrap(err, "getting key") } - if command.CacheCommand() && opts.UseCache { + if command.CacheCommand() && opts.Cache { image, err := cache.RetrieveLayer(opts, cacheKey) if err == nil { if err := s.extractCachedLayer(image, command.String()); err != nil { @@ -212,7 +212,7 @@ func (s *stageBuilder) build(opts *config.KanikoOptions) error { return err } // Push layer to cache now along with new config file - if command.CacheCommand() && opts.UseCache { + if command.CacheCommand() && opts.Cache { if err := pushLayerToCache(opts, cacheKey, layer, command.String()); err != nil { return err } From 6c39f29081d9a9cbdbe4132efa047866ee6e3a7d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 25 Sep 2018 10:05:11 -0700 Subject: [PATCH 24/35] Only return stdout when running commands for integration tests When running container-diff in integration tests, both stdout and stderr were being returned and unmarshalled into the container-diff json object. If container-diff was giving any error messages (such as, if it didn't have permissions to extract a file), this would fail even if ultimately no differences between the filesystems existed. I updated the RunCommands to only return stdout and print stderr if the command fails. --- integration/cmd.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/integration/cmd.go b/integration/cmd.go index 51be33733..a254197bf 100644 --- a/integration/cmd.go +++ b/integration/cmd.go @@ -17,6 +17,7 @@ limitations under the License. package integration import ( + "bytes" "fmt" "os/exec" "testing" @@ -25,9 +26,12 @@ import ( // RunCommandWithoutTest will run cmd and if it fails will output relevant info // for debugging before returning an error. It can be run outside the context of a test. func RunCommandWithoutTest(cmd *exec.Cmd) ([]byte, error) { - output, err := cmd.CombinedOutput() + var stderr bytes.Buffer + cmd.Stderr = &stderr + output, err := cmd.Output() if err != nil { fmt.Println(cmd.Args) + fmt.Println(stderr.String()) fmt.Println(string(output)) } return output, err @@ -37,9 +41,12 @@ func RunCommandWithoutTest(cmd *exec.Cmd) ([]byte, error) { // before it fails. It must be run within the context of a test t and if the command // fails, it will the test. Returns the output from the command. func RunCommand(cmd *exec.Cmd, t *testing.T) []byte { - output, err := cmd.CombinedOutput() + var stderr bytes.Buffer + cmd.Stderr = &stderr + output, err := cmd.Output() if err != nil { t.Log(cmd.Args) + t.Log(stderr.String()) t.Log(string(output)) t.Error(err) t.FailNow() From cd2fedf9d2eca713b8f2a7c6e6160583a46b6ab9 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 25 Sep 2018 10:24:06 -0700 Subject: [PATCH 25/35] Update README to add information about layer caching --- README.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/README.md b/README.md index 15775d916..f55cdd59d 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ We do **not** recommend running the kaniko executor binary in another image, as - [Running kaniko in gVisor](#running-kaniko-in-gvisor) - [Running kaniko in Google Container Builder](#running-kaniko-in-google-container-builder) - [Running kaniko locally](#running-kaniko-locally) + - [Caching](#caching) - [Pushing to Different Registries](#pushing-to-different-registries) - [Additional Flags](#additional-flags) - [Debug Image](#debug-image) @@ -188,6 +189,16 @@ We can run the kaniko executor image locally in a Docker daemon to build and pus ./run_in_docker.sh ``` +### Caching +kaniko currently can cache layers created by `RUN` commands in a remote repository. +Before executing a command, kaniko checks the cache for the layer. +If it exists, kaniko will pull and extract the cached layer instead of executing the command. +If not, kaniko will execute the command and then push the newly created layer to the cache. + +Users can opt in to caching by setting the `--cache=true` flag. +A remote repository for storing cached layers can be provided via the `--cache-repo` flag. +If this flag isn't provided, a cached repo will be inferred from the `--destination` provided. + ### Pushing to Different Registries kaniko uses Docker credential helpers to push images to a registry. @@ -293,6 +304,19 @@ Set this flag if you want to connect to a plain HTTP registry. It is supposed to Set this flag to skip TLS certificate validation when connecting to a registry. It is supposed to be used for testing purposes only and should not be used in production! +#### --cache + +Set this flag as `--cache=true` to opt in to caching with kaniko. + +#### --cache-repo + +Set this flag to specify a remote repository which will be used to store cached layers. + +If this flag is not provided, a cache repo will be inferred from the `--destination` flag. +If `--destination=gcr.io/kaniko-project/test`, then cached layers will be stored in `gcr.io/kaniko-project/test/cache`. + +_This flag must be used in conjunction with the `--cache=true` flag._ + ### Debug Image The kaniko executor image is based off of scratch and doesn't contain a shell. From 59cb0ebec969fde3f8c8c7c9468d50580eb1e1ac Mon Sep 17 00:00:00 2001 From: xanonid Date: Wed, 26 Sep 2018 16:14:35 +0200 Subject: [PATCH 26/35] Enable overwriting of links (solves #351) (#360) * Enable overwriting of links (solves #351) * add integration test to check extraction of images with replaced hardlinks * Prevent following symlinks during extracting normal files This fixes #359, #361, #362. --- .../Dockerfile_test_replaced_hardlinks | 1 + .../dockerfiles/Dockerfile_test_replaced_symlinks | 2 ++ pkg/util/fs_util.go | 15 +++++++++++++++ 3 files changed, 18 insertions(+) create mode 100644 integration/dockerfiles/Dockerfile_test_replaced_hardlinks create mode 100644 integration/dockerfiles/Dockerfile_test_replaced_symlinks diff --git a/integration/dockerfiles/Dockerfile_test_replaced_hardlinks b/integration/dockerfiles/Dockerfile_test_replaced_hardlinks new file mode 100644 index 000000000..78ca22b11 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_replaced_hardlinks @@ -0,0 +1 @@ +FROM jboss/base-jdk@sha256:70d956f632c26d1f1df57cbb99870a6141cfe470de0eb2d51bccd44929df9367 diff --git a/integration/dockerfiles/Dockerfile_test_replaced_symlinks b/integration/dockerfiles/Dockerfile_test_replaced_symlinks new file mode 100644 index 000000000..70e3d1f2b --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_replaced_symlinks @@ -0,0 +1,2 @@ +FROM tenstartups/alpine@sha256:31dc8b12e0f73a1de899146c3663644b7668f8fd198cfe9b266886c9abfa944b +RUN pwd diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index 3e45c69c1..b7c2baddf 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -186,6 +186,13 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { return err } } + // Check if something already exists at path (symlinks etc.) + // If so, delete it + if FilepathExists(path) { + if err := os.Remove(path); err != nil { + return errors.Wrapf(err, "error removing %s to make way for new file.", path) + } + } currFile, err := os.Create(path) if err != nil { return err @@ -220,6 +227,14 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { if err := os.MkdirAll(dir, 0755); err != nil { return err } + // Check if something already exists at path + // If so, delete it + if FilepathExists(path) { + if err := os.Remove(path); err != nil { + return errors.Wrapf(err, "error removing %s to make way for new link", hdr.Name) + } + } + if err := os.Link(filepath.Clean(filepath.Join("/", hdr.Linkname)), path); err != nil { return err } From 49184c2114f71730583b4625308d4bbdce55f266 Mon Sep 17 00:00:00 2001 From: Sharif Elgamal Date: Thu, 27 Sep 2018 07:31:51 -0700 Subject: [PATCH 27/35] set default HOME env properly (#341) * set default HOME env properly * set HOME to / if user is set by uid * fix test * continue to skip user_run test * fix unit test to match new functionality --- .../dockerfiles/Dockerfile_test_user_run | 4 ++++ pkg/commands/run.go | 19 +++++++++++++++---- pkg/commands/run_test.go | 2 +- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/integration/dockerfiles/Dockerfile_test_user_run b/integration/dockerfiles/Dockerfile_test_user_run index d7f0ae3b8..bdad4f4b9 100644 --- a/integration/dockerfiles/Dockerfile_test_user_run +++ b/integration/dockerfiles/Dockerfile_test_user_run @@ -20,8 +20,12 @@ RUN echo "hey" > /tmp/foo USER testuser:1001 RUN echo "hey2" >> /tmp/foo +USER root + RUN useradd -ms /bin/bash newuser USER newuser RUN echo "hi" > $HOME/file COPY context/foo $HOME/foo +USER 1001 +RUN echo $HOME diff --git a/pkg/commands/run.go b/pkg/commands/run.go index 3b59f84c9..7cfe7b486 100644 --- a/pkg/commands/run.go +++ b/pkg/commands/run.go @@ -20,6 +20,7 @@ import ( "fmt" "os" "os/exec" + "os/user" "strconv" "strings" "syscall" @@ -116,19 +117,29 @@ func (r *RunCommand) ExecuteCommand(config *v1.Config, buildArgs *dockerfile.Bui } // addDefaultHOME adds the default value for HOME if it isn't already set -func addDefaultHOME(user string, envs []string) []string { +func addDefaultHOME(u string, envs []string) []string { for _, env := range envs { split := strings.SplitN(env, "=", 2) if split[0] == constants.HOME { return envs } } + // If user isn't set, set default value of HOME - if user == "" { + if u == "" { return append(envs, fmt.Sprintf("%s=%s", constants.HOME, constants.DefaultHOMEValue)) } - // If user is set, set value of HOME to /home/${user} - return append(envs, fmt.Sprintf("%s=/home/%s", constants.HOME, user)) + + // If user is set to username, set value of HOME to /home/${user} + // Otherwise the user is set to uid and HOME is / + home := fmt.Sprintf("%s=/", constants.HOME) + userObj, err := user.Lookup(u) + if err == nil { + u = userObj.Username + home = fmt.Sprintf("%s=/home/%s", constants.HOME, u) + } + + return append(envs, home) } // FilesToSnapshot returns nil for this command because we don't know which files diff --git a/pkg/commands/run_test.go b/pkg/commands/run_test.go index 5a3971c68..fd3afb591 100644 --- a/pkg/commands/run_test.go +++ b/pkg/commands/run_test.go @@ -59,7 +59,7 @@ func Test_addDefaultHOME(t *testing.T) { }, expected: []string{ "PATH=/something/else", - "HOME=/home/newuser", + "HOME=/", }, }, } From b1e28ddb4f56f67a8ab7523169eb407644a878c4 Mon Sep 17 00:00:00 2001 From: peter-evans Date: Wed, 26 Sep 2018 13:59:11 +0900 Subject: [PATCH 28/35] Fix handling of volume directive --- .../dockerfiles/Dockerfile_test_volume | 2 +- .../dockerfiles/Dockerfile_test_volume_2 | 13 +++ integration/integration_test.go | 3 - pkg/commands/volume.go | 4 +- pkg/constants/constants.go | 3 + pkg/executor/build.go | 11 ++- pkg/snapshot/snapshot.go | 41 +--------- pkg/util/fs_util.go | 82 +++++++++++-------- pkg/util/fs_util_test.go | 69 +++++++++++----- 9 files changed, 128 insertions(+), 100 deletions(-) create mode 100644 integration/dockerfiles/Dockerfile_test_volume_2 diff --git a/integration/dockerfiles/Dockerfile_test_volume b/integration/dockerfiles/Dockerfile_test_volume index 1ffb7a514..a4db420ac 100644 --- a/integration/dockerfiles/Dockerfile_test_volume +++ b/integration/dockerfiles/Dockerfile_test_volume @@ -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 diff --git a/integration/dockerfiles/Dockerfile_test_volume_2 b/integration/dockerfiles/Dockerfile_test_volume_2 new file mode 100644 index 000000000..77a116a08 --- /dev/null +++ b/integration/dockerfiles/Dockerfile_test_volume_2 @@ -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 diff --git a/integration/integration_test.go b/integration/integration_test.go index de2409ed7..c86c62ffd 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -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) { diff --git a/pkg/commands/volume.go b/pkg/commands/volume.go index 5c9ecd99c..01c9fdab2 100644 --- a/pkg/commands/volume.go +++ b/pkg/commands/volume.go @@ -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) } diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index d8fcc722e..1c8d774b8 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -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 diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 29f52d6ff..7cd311b34 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -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 diff --git a/pkg/snapshot/snapshot.go b/pkg/snapshot/snapshot.go index 55da45d65..4175d4761 100644 --- a/pkg/snapshot/snapshot.go +++ b/pkg/snapshot/snapshot.go @@ -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) { diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index b7c2baddf..9ce2cb45a 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -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 @@ -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 } } @@ -266,7 +279,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 } } @@ -278,7 +291,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 } } @@ -291,7 +304,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 @@ -314,7 +327,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) @@ -337,7 +353,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 { @@ -400,22 +416,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 } @@ -515,7 +524,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, "/") @@ -524,6 +533,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 diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index 99202e1bb..968cacf99 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -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) } }) From 49ab8e497946da68223c7a88433f64da96bd6dcf Mon Sep 17 00:00:00 2001 From: Vincent Behar Date: Thu, 27 Sep 2018 14:32:29 +0200 Subject: [PATCH 29/35] Add a new flag to cleanup the filesystem at the end Currently, kaniko can only build a single image per container run, because the filesystem is full of the content of the first image. When running kaniko in Jenkins, where we need to start the container "doing nothing" first (using the debug kaniko container), and then exec /kaniko/executor, this is a limitation because it means that if we want to build multiple images, we need to start multiple containers - see https://groups.google.com/forum/#!topic/kaniko-users/_7LivHdMdy0 for more details A solution to fix this issue is to add a new flag to cleanup the filesystem at the end - the same way it is done between stages when building a multi-stages image. This way, the same (debug) container can be used to build multiple images. --- README.md | 4 ++++ cmd/executor/cmd/root.go | 1 + pkg/config/options.go | 1 + pkg/executor/build.go | 5 +++++ 4 files changed, 11 insertions(+) diff --git a/README.md b/README.md index f55cdd59d..39c24e310 100644 --- a/README.md +++ b/README.md @@ -317,6 +317,10 @@ If `--destination=gcr.io/kaniko-project/test`, then cached layers will be stored _This flag must be used in conjunction with the `--cache=true` flag._ +#### --cleanup + +Set this flag to cleanup the filesystem at the end, leaving a clean kaniko container (if you want to build multiple images in the same container, using the debug kaniko image) + ### Debug Image The kaniko executor image is based off of scratch and doesn't contain a shell. diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index b2e623f1c..7f8ba1415 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -97,6 +97,7 @@ func addKanikoOptionsFlags(cmd *cobra.Command) { RootCmd.PersistentFlags().BoolVarP(&opts.NoPush, "no-push", "", false, "Do not push the image to the registry") RootCmd.PersistentFlags().StringVarP(&opts.CacheRepo, "cache-repo", "", "", "Specify a repository to use as a cache, otherwise one will be inferred from the destination provided") RootCmd.PersistentFlags().BoolVarP(&opts.Cache, "cache", "", false, "Use cache when building image") + RootCmd.PersistentFlags().BoolVarP(&opts.Cleanup, "cleanup", "", false, "Clean the filesystem at the end") } // addHiddenFlags marks certain flags as hidden from the executor help text diff --git a/pkg/config/options.go b/pkg/config/options.go index c9bad39e6..26fecec29 100644 --- a/pkg/config/options.go +++ b/pkg/config/options.go @@ -33,4 +33,5 @@ type KanikoOptions struct { Reproducible bool NoPush bool Cache bool + Cleanup bool } diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 29f52d6ff..5e10d16b5 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -264,6 +264,11 @@ func DoBuild(opts *config.KanikoOptions) (v1.Image, error) { return nil, err } } + if opts.Cleanup { + if err = util.DeleteFilesystem(); err != nil { + return nil, err + } + } return sourceImage, nil } if stage.SaveStage { From d10e3f5b74929fb2f1cf6e42d0d19498aef9ee65 Mon Sep 17 00:00:00 2001 From: Vincent Behar Date: Thu, 27 Sep 2018 11:53:45 +0200 Subject: [PATCH 30/35] Whitelist /busybox in the debug image In the debug image, declare /busybox as a volume so that it is automatically whitelisted, because we don't want to delete it when building multi-stages images. FYI this is required when using Jenkins, because we need to use the debug kaniko image to be able to start the container "doing nothing" (with /busybox/cat) before building (by executing /kaniko/executor directly inside the container) See https://issues.jenkins-ci.org/browse/JENKINS-52576 --- deploy/Dockerfile_debug | 2 ++ 1 file changed, 2 insertions(+) diff --git a/deploy/Dockerfile_debug b/deploy/Dockerfile_debug index b01896f00..c8233ad37 100644 --- a/deploy/Dockerfile_debug +++ b/deploy/Dockerfile_debug @@ -38,6 +38,8 @@ COPY --from=0 /go/src/github.com/GoogleContainerTools/kaniko/out/executor /kanik COPY --from=0 /usr/local/bin/docker-credential-gcr /kaniko/docker-credential-gcr COPY --from=0 /go/src/github.com/awslabs/amazon-ecr-credential-helper/bin/linux-amd64/docker-credential-ecr-login /kaniko/docker-credential-ecr-login COPY --from=1 /distroless/bazel-genfiles/experimental/busybox/busybox/ /busybox/ +# Declare /busybox as a volume to get it automatically whitelisted +VOLUME /busybox COPY files/ca-certificates.crt /kaniko/ssl/certs/ COPY files/config.json /kaniko/.docker/ ENV HOME /root From d904a4c872dba2e8f549d9bf31b32e6d1f964ae1 Mon Sep 17 00:00:00 2001 From: dlorenc Date: Fri, 28 Sep 2018 09:13:17 -0700 Subject: [PATCH 31/35] Add a benchmark package to store and monitor timings. (#367) --- pkg/timing/timing.go | 90 ++++++++++++++++++++++++ pkg/timing/timing_test.go | 139 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 229 insertions(+) create mode 100644 pkg/timing/timing.go create mode 100644 pkg/timing/timing_test.go diff --git a/pkg/timing/timing.go b/pkg/timing/timing.go new file mode 100644 index 000000000..fcd212768 --- /dev/null +++ b/pkg/timing/timing.go @@ -0,0 +1,90 @@ +/* +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. +*/ + +package timing + +import ( + "bytes" + "fmt" + "sync" + "text/template" + "time" +) + +// For testing +var currentTimeFunc = time.Now + +// DefaultRun is the default "singleton" TimedRun instance. +var DefaultRun = NewTimedRun() + +// TimedRun provides a running store of how long is spent in each category. +type TimedRun struct { + cl sync.Mutex + categories map[string]time.Duration // protected by cl +} + +// Stop stops the specified timer and increments the time spent in that category. +func (tr *TimedRun) Stop(t *Timer) { + stop := currentTimeFunc() + if _, ok := tr.categories[t.category]; !ok { + tr.categories[t.category] = 0 + } + fmt.Println(stop) + tr.cl.Lock() + defer tr.cl.Unlock() + tr.categories[t.category] += stop.Sub(t.startTime) +} + +// Start starts a new Timer and returns it. +func Start(category string) *Timer { + t := Timer{ + category: category, + startTime: currentTimeFunc(), + } + return &t +} + +// NewTimedRun returns an initialized TimedRun instance. +func NewTimedRun() *TimedRun { + tr := TimedRun{ + categories: map[string]time.Duration{}, + } + return &tr +} + +// Timer represents a running timer. +type Timer struct { + category string + startTime time.Time +} + +// DefaultFormat is a default format string used by Summary. +var DefaultFormat = template.Must(template.New("").Parse("{{range $c, $t := .}}{{$c}}: {{$t}}\n{{end}}")) + +// Summary outputs a summary of the DefaultTimedRun. +func Summary() string { + return DefaultRun.Summary() +} + +// Summary outputs a summary of the specified TimedRun. +func (tr *TimedRun) Summary() string { + b := bytes.Buffer{} + + tr.cl.Lock() + defer tr.cl.Unlock() + DefaultFormat.Execute(&b, tr.categories) + return b.String() +} diff --git a/pkg/timing/timing_test.go b/pkg/timing/timing_test.go new file mode 100644 index 000000000..43082c191 --- /dev/null +++ b/pkg/timing/timing_test.go @@ -0,0 +1,139 @@ +/* +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. +*/ +package timing + +import ( + "testing" + "time" +) + +func patchTime(timeFunc func() time.Time) func() { + old := currentTimeFunc + currentTimeFunc = timeFunc + return func() { + currentTimeFunc = old + } +} + +func mockTimeFunc(t time.Time) func() time.Time { + return func() time.Time { + return t + } +} + +func TestTimedRun_StartStop(t *testing.T) { + type args struct { + categories map[string]time.Duration + category string + waitTime time.Duration + } + tests := []struct { + name string + args args + want time.Duration + }{ + { + name: "new category", + args: args{ + categories: map[string]time.Duration{}, + category: "foo", + waitTime: 3 * time.Second, + }, + want: 3 * time.Second, + }, + { + name: "existing category", + args: args{ + categories: map[string]time.Duration{ + "foo": 4 * time.Second, + }, + category: "foo", + waitTime: 2 * time.Second, + }, + want: 6 * time.Second, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tr := &TimedRun{ + categories: tt.args.categories, + } + + timer := Timer{ + category: tt.args.category, + startTime: time.Time{}, + } + + defer patchTime(mockTimeFunc(timer.startTime.Add(tt.args.waitTime)))() + tr.Stop(&timer) + if got := tr.categories[tt.args.category]; got != tt.want { + t.Errorf("Expected %d, got %d", tt.want, got) + } + }) + } +} + +func TestTimedRun_Summary(t *testing.T) { + type fields struct { + categories map[string]time.Duration + } + tests := []struct { + name string + fields fields + want string + }{ + { + name: "single key", + fields: fields{ + categories: map[string]time.Duration{ + "foo": 3 * time.Second, + }, + }, + want: "foo: 3s\n", + }, + { + name: "two keys", + fields: fields{ + categories: map[string]time.Duration{ + "foo": 3 * time.Second, + "bar": 1 * time.Second, + }, + }, + want: "bar: 1s\nfoo: 3s\n", + }, + { + name: "units", + fields: fields{ + categories: map[string]time.Duration{ + "foo": 3 * time.Second, + "bar": 1 * time.Millisecond, + }, + }, + want: "bar: 1ms\nfoo: 3s\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tr := &TimedRun{ + categories: tt.fields.categories, + } + if got := tr.Summary(); got != tt.want { + t.Errorf("TimedRun.Summary() = %v, want %v", got, tt.want) + } + }) + } +} From c4b35c729838ead95fc2a32ef763ed0db2d3076e Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 28 Sep 2018 09:43:16 -0700 Subject: [PATCH 32/35] Check --cache-repo is provided with --cache and --no-push As described in #373, kaniko panics when provided with --cache and --no-push since it tries to infer a cache repo from the destination, which doesn't exist. To fix this, I added a check to make sure --cache-repo is passed in when both these flags are provided. --- cmd/executor/cmd/root.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cmd/executor/cmd/root.go b/cmd/executor/cmd/root.go index b2e623f1c..62136522e 100644 --- a/cmd/executor/cmd/root.go +++ b/cmd/executor/cmd/root.go @@ -55,6 +55,9 @@ var RootCmd = &cobra.Command{ if !opts.NoPush && len(opts.Destinations) == 0 { return errors.New("You must provide --destination, or use --no-push") } + if err := cacheFlagsValid(); err != nil { + return errors.Wrap(err, "cache flags invalid") + } if err := resolveSourceContext(); err != nil { return errors.Wrap(err, "error resolving source context") } @@ -112,6 +115,19 @@ func checkContained() bool { return err == nil } +// cacheFlagsValid makes sure the flags passed in related to caching are valid +func cacheFlagsValid() error { + if !opts.Cache { + return nil + } + // If --cache=true and --no-push=true, then cache repo must be provided + // since cache can't be inferred from destination + if opts.CacheRepo == "" && opts.NoPush { + return errors.New("if using cache with --no-push, specify cache repo with --cache-repo") + } + return nil +} + // resolveDockerfilePath resolves the Dockerfile path to an absolute path func resolveDockerfilePath() error { if util.FilepathExists(opts.DockerfilePath) { From e1b0f7732e898e46da514735be4b5a1917d25c46 Mon Sep 17 00:00:00 2001 From: dlorenc Date: Fri, 28 Sep 2018 11:42:07 -0700 Subject: [PATCH 33/35] Fixes a whitelist issue when untarring files in ADD commands. (#371) * Fixes a whitelist issue when untarring files in ADD commands. * Add go-cmp test tool. * Make the integration test tolerate some file differences. --- Gopkg.lock | 15 + integration/context/tars/sys.tar.gz | Bin 0 -> 166 bytes integration/dockerfiles/Dockerfile_test_add | 4 + integration/integration_test.go | 49 +- pkg/util/fs_util.go | 35 +- testutil/util.go | 5 +- vendor/github.com/google/go-cmp/LICENSE | 27 + .../github.com/google/go-cmp/cmp/compare.go | 553 ++++++++++++++++++ .../go-cmp/cmp/internal/diff/debug_disable.go | 17 + .../go-cmp/cmp/internal/diff/debug_enable.go | 122 ++++ .../google/go-cmp/cmp/internal/diff/diff.go | 363 ++++++++++++ .../go-cmp/cmp/internal/function/func.go | 49 ++ .../go-cmp/cmp/internal/value/format.go | 277 +++++++++ .../google/go-cmp/cmp/internal/value/sort.go | 111 ++++ .../github.com/google/go-cmp/cmp/options.go | 453 ++++++++++++++ vendor/github.com/google/go-cmp/cmp/path.go | 309 ++++++++++ .../github.com/google/go-cmp/cmp/reporter.go | 53 ++ .../google/go-cmp/cmp/unsafe_panic.go | 15 + .../google/go-cmp/cmp/unsafe_reflect.go | 23 + 19 files changed, 2456 insertions(+), 24 deletions(-) create mode 100644 integration/context/tars/sys.tar.gz create mode 100644 vendor/github.com/google/go-cmp/LICENSE create mode 100644 vendor/github.com/google/go-cmp/cmp/compare.go create mode 100644 vendor/github.com/google/go-cmp/cmp/internal/diff/debug_disable.go create mode 100644 vendor/github.com/google/go-cmp/cmp/internal/diff/debug_enable.go create mode 100644 vendor/github.com/google/go-cmp/cmp/internal/diff/diff.go create mode 100644 vendor/github.com/google/go-cmp/cmp/internal/function/func.go create mode 100644 vendor/github.com/google/go-cmp/cmp/internal/value/format.go create mode 100644 vendor/github.com/google/go-cmp/cmp/internal/value/sort.go create mode 100644 vendor/github.com/google/go-cmp/cmp/options.go create mode 100644 vendor/github.com/google/go-cmp/cmp/path.go create mode 100644 vendor/github.com/google/go-cmp/cmp/reporter.go create mode 100644 vendor/github.com/google/go-cmp/cmp/unsafe_panic.go create mode 100644 vendor/github.com/google/go-cmp/cmp/unsafe_reflect.go diff --git a/Gopkg.lock b/Gopkg.lock index 6ae76a504..176d4132a 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -416,6 +416,19 @@ pruneopts = "NUT" revision = "e89373fe6b4a7413d7acd6da1725b83ef713e6e4" +[[projects]] + digest = "1:2e3c336fc7fde5c984d2841455a658a6d626450b1754a854b3b32e7a8f49a07a" + name = "github.com/google/go-cmp" + packages = [ + "cmp", + "cmp/internal/diff", + "cmp/internal/function", + "cmp/internal/value", + ] + pruneopts = "NUT" + revision = "3af367b6b30c263d47e8895973edcca9a49cf029" + version = "v0.2.0" + [[projects]] branch = "master" digest = "1:8ea12d703d8f36fec3db5e0dd85c9d3cc0b8b05ea8f9d090dcd41129cd392fd2" @@ -1149,6 +1162,7 @@ "github.com/docker/docker/pkg/archive", "github.com/docker/docker/pkg/signal", "github.com/genuinetools/amicontained/container", + "github.com/google/go-cmp/cmp", "github.com/google/go-containerregistry/pkg/authn", "github.com/google/go-containerregistry/pkg/authn/k8schain", "github.com/google/go-containerregistry/pkg/name", @@ -1156,6 +1170,7 @@ "github.com/google/go-containerregistry/pkg/v1/daemon", "github.com/google/go-containerregistry/pkg/v1/empty", "github.com/google/go-containerregistry/pkg/v1/mutate", + "github.com/google/go-containerregistry/pkg/v1/partial", "github.com/google/go-containerregistry/pkg/v1/remote", "github.com/google/go-containerregistry/pkg/v1/tarball", "github.com/moby/buildkit/frontend/dockerfile/instructions", diff --git a/integration/context/tars/sys.tar.gz b/integration/context/tars/sys.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..d61da594905b599186542270a78e8f1f55e92269 GIT binary patch literal 166 zcmb2|=3rR=cuh0|^V>^~T!#zk literal 0 HcmV?d00001 diff --git a/integration/dockerfiles/Dockerfile_test_add b/integration/dockerfiles/Dockerfile_test_add index 1fbd76948..65f530ad1 100644 --- a/integration/dockerfiles/Dockerfile_test_add +++ b/integration/dockerfiles/Dockerfile_test_add @@ -14,6 +14,10 @@ ADD $contextenv/* /tmp/${contextenv}/ ADD context/tars/fil* /tars/ ADD context/tars/file.tar /tars_again +# This tar has some directories that should be whitelisted inside it. + +ADD context/tars/sys.tar.gz / + # Test with ARG ARG file COPY $file /arg diff --git a/integration/integration_test.go b/integration/integration_test.go index de2409ed7..008927bde 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -32,6 +32,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/daemon" + "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/GoogleContainerTools/kaniko/testutil" ) @@ -57,8 +58,8 @@ func (i imageDetails) String() string { func initGCPConfig() *gcpConfig { var c gcpConfig - flag.StringVar(&c.gcsBucket, "bucket", "", "The gcs bucket argument to uploaded the tar-ed contents of the `integration` dir to.") - flag.StringVar(&c.imageRepo, "repo", "", "The (docker) image repo to build and push images to during the test. `gcloud` must be authenticated with this repo.") + flag.StringVar(&c.gcsBucket, "bucket", "gs://kaniko-test-bucket", "The gcs bucket argument to uploaded the tar-ed contents of the `integration` dir to.") + flag.StringVar(&c.imageRepo, "repo", "gcr.io/kaniko-test", "The (docker) image repo to build and push images to during the test. `gcloud` must be authenticated with this repo.") flag.Parse() if c.gcsBucket == "" || c.imageRepo == "" { @@ -211,7 +212,7 @@ func TestRun(t *testing.T) { func TestLayers(t *testing.T) { offset := map[string]int{ - "Dockerfile_test_add": 10, + "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 @@ -269,13 +270,30 @@ func TestCache(t *testing.T) { } } +type fileDiff struct { + Name string + Size int +} + +type diffOutput struct { + Image1 string + Image2 string + DiffType string + Diff struct { + Adds []fileDiff + Dels []fileDiff + } +} + +var allowedDiffPaths = []string{"/sys"} + func checkContainerDiffOutput(t *testing.T, diff []byte, expected string) { // Let's compare the json objects themselves instead of strings to avoid // issues with spaces and indents t.Helper() - var diffInt interface{} - var expectedInt interface{} + diffInt := []diffOutput{} + expectedInt := []diffOutput{} err := json.Unmarshal(diff, &diffInt) if err != nil { @@ -287,9 +305,30 @@ func checkContainerDiffOutput(t *testing.T, diff []byte, expected string) { t.Error(err) } + // Some differences (whitelisted paths, etc.) are known and expected. + diffInt[0].Diff.Adds = filterDiff(diffInt[0].Diff.Adds) + diffInt[0].Diff.Dels = filterDiff(diffInt[0].Diff.Dels) + testutil.CheckErrorAndDeepEqual(t, false, nil, expectedInt, diffInt) } +func filterDiff(f []fileDiff) []fileDiff { + var newDiffs []fileDiff + for _, diff := range f { + isWhitelisted := false + for _, p := range allowedDiffPaths { + if util.HasFilepathPrefix(diff.Name, p) { + isWhitelisted = true + break + } + } + if !isWhitelisted { + newDiffs = append(newDiffs, diff) + } + } + return newDiffs +} + func checkLayers(t *testing.T, image1, image2 string, offset int) { t.Helper() img1, err := getImageDetails(image1) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index b7c2baddf..f3c5eb1a6 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -86,24 +86,6 @@ func GetFSFromImage(root string, img v1.Image) ([]string, error) { } continue } - whitelisted, err := CheckWhitelist(path) - if err != nil { - return nil, err - } - if whitelisted && !checkWhitelistRoot(root) { - logrus.Debugf("Not adding %s because it is whitelisted", path) - continue - } - if hdr.Typeflag == tar.TypeSymlink { - whitelisted, err := CheckWhitelist(hdr.Linkname) - if err != nil { - return nil, err - } - if whitelisted { - logrus.Debugf("skipping symlink from %s to %s because %s is whitelisted", hdr.Linkname, path, hdr.Linkname) - continue - } - } if err := extractFile(root, hdr, tr); err != nil { return nil, err } @@ -176,6 +158,15 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { mode := hdr.FileInfo().Mode() uid := hdr.Uid gid := hdr.Gid + + whitelisted, err := CheckWhitelist(path) + if err != nil { + return err + } + if whitelisted && !checkWhitelistRoot(dest) { + logrus.Debugf("Not adding %s because it is whitelisted", path) + return nil + } switch hdr.Typeflag { case tar.TypeReg: logrus.Debugf("creating file %s", path) @@ -223,6 +214,14 @@ func extractFile(dest string, hdr *tar.Header, tr io.Reader) error { case tar.TypeLink: logrus.Debugf("link from %s to %s", hdr.Linkname, path) + whitelisted, err := CheckWhitelist(hdr.Linkname) + if err != nil { + return err + } + if whitelisted { + logrus.Debugf("skipping symlink from %s to %s because %s is whitelisted", hdr.Linkname, path, hdr.Linkname) + return nil + } // The base directory for a link may not exist before it is created. if err := os.MkdirAll(dir, 0755); err != nil { return err diff --git a/testutil/util.go b/testutil/util.go index 5101ba415..b6106fab8 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -23,6 +23,8 @@ import ( "path/filepath" "reflect" "testing" + + "github.com/google/go-cmp/cmp" ) // SetupFiles creates files at path @@ -46,7 +48,8 @@ func CheckErrorAndDeepEqual(t *testing.T, shouldErr bool, err error, expected, a return } if !reflect.DeepEqual(expected, actual) { - t.Errorf("%T differ.\nExpected\n%+v\nActual\n%+v", expected, expected, actual) + diff := cmp.Diff(actual, expected) + t.Errorf("%T differ (-got, +want): %s", expected, diff) return } } diff --git a/vendor/github.com/google/go-cmp/LICENSE b/vendor/github.com/google/go-cmp/LICENSE new file mode 100644 index 000000000..32017f8fa --- /dev/null +++ b/vendor/github.com/google/go-cmp/LICENSE @@ -0,0 +1,27 @@ +Copyright (c) 2017 The Go Authors. All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are +met: + + * Redistributions of source code must retain the above copyright +notice, this list of conditions and the following disclaimer. + * Redistributions in binary form must reproduce the above +copyright notice, this list of conditions and the following disclaimer +in the documentation and/or other materials provided with the +distribution. + * Neither the name of Google Inc. nor the names of its +contributors may be used to endorse or promote products derived from +this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. diff --git a/vendor/github.com/google/go-cmp/cmp/compare.go b/vendor/github.com/google/go-cmp/cmp/compare.go new file mode 100644 index 000000000..7e215f220 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/compare.go @@ -0,0 +1,553 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// Package cmp determines equality of values. +// +// This package is intended to be a more powerful and safer alternative to +// reflect.DeepEqual for comparing whether two values are semantically equal. +// +// The primary features of cmp are: +// +// โ€ข When the default behavior of equality does not suit the needs of the test, +// custom equality functions can override the equality operation. +// For example, an equality function may report floats as equal so long as they +// are within some tolerance of each other. +// +// โ€ข Types that have an Equal method may use that method to determine equality. +// This allows package authors to determine the equality operation for the types +// that they define. +// +// โ€ข If no custom equality functions are used and no Equal method is defined, +// equality is determined by recursively comparing the primitive kinds on both +// values, much like reflect.DeepEqual. Unlike reflect.DeepEqual, unexported +// fields are not compared by default; they result in panics unless suppressed +// by using an Ignore option (see cmpopts.IgnoreUnexported) or explicitly compared +// using the AllowUnexported option. +package cmp + +import ( + "fmt" + "reflect" + + "github.com/google/go-cmp/cmp/internal/diff" + "github.com/google/go-cmp/cmp/internal/function" + "github.com/google/go-cmp/cmp/internal/value" +) + +// BUG(dsnet): Maps with keys containing NaN values cannot be properly compared due to +// the reflection package's inability to retrieve such entries. Equal will panic +// anytime it comes across a NaN key, but this behavior may change. +// +// See https://golang.org/issue/11104 for more details. + +var nothing = reflect.Value{} + +// Equal reports whether x and y are equal by recursively applying the +// following rules in the given order to x and y and all of their sub-values: +// +// โ€ข If two values are not of the same type, then they are never equal +// and the overall result is false. +// +// โ€ข Let S be the set of all Ignore, Transformer, and Comparer options that +// remain after applying all path filters, value filters, and type filters. +// If at least one Ignore exists in S, then the comparison is ignored. +// If the number of Transformer and Comparer options in S is greater than one, +// then Equal panics because it is ambiguous which option to use. +// If S contains a single Transformer, then use that to transform the current +// values and recursively call Equal on the output values. +// If S contains a single Comparer, then use that to compare the current values. +// Otherwise, evaluation proceeds to the next rule. +// +// โ€ข If the values have an Equal method of the form "(T) Equal(T) bool" or +// "(T) Equal(I) bool" where T is assignable to I, then use the result of +// x.Equal(y) even if x or y is nil. +// Otherwise, no such method exists and evaluation proceeds to the next rule. +// +// โ€ข Lastly, try to compare x and y based on their basic kinds. +// Simple kinds like booleans, integers, floats, complex numbers, strings, and +// channels are compared using the equivalent of the == operator in Go. +// Functions are only equal if they are both nil, otherwise they are unequal. +// Pointers are equal if the underlying values they point to are also equal. +// Interfaces are equal if their underlying concrete values are also equal. +// +// Structs are equal if all of their fields are equal. If a struct contains +// unexported fields, Equal panics unless the AllowUnexported option is used or +// an Ignore option (e.g., cmpopts.IgnoreUnexported) ignores that field. +// +// Arrays, slices, and maps are equal if they are both nil or both non-nil +// with the same length and the elements at each index or key are equal. +// Note that a non-nil empty slice and a nil slice are not equal. +// To equate empty slices and maps, consider using cmpopts.EquateEmpty. +// Map keys are equal according to the == operator. +// To use custom comparisons for map keys, consider using cmpopts.SortMaps. +func Equal(x, y interface{}, opts ...Option) bool { + s := newState(opts) + s.compareAny(reflect.ValueOf(x), reflect.ValueOf(y)) + return s.result.Equal() +} + +// Diff returns a human-readable report of the differences between two values. +// It returns an empty string if and only if Equal returns true for the same +// input values and options. The output string will use the "-" symbol to +// indicate elements removed from x, and the "+" symbol to indicate elements +// added to y. +// +// Do not depend on this output being stable. +func Diff(x, y interface{}, opts ...Option) string { + r := new(defaultReporter) + opts = Options{Options(opts), r} + eq := Equal(x, y, opts...) + d := r.String() + if (d == "") != eq { + panic("inconsistent difference and equality results") + } + return d +} + +type state struct { + // These fields represent the "comparison state". + // Calling statelessCompare must not result in observable changes to these. + result diff.Result // The current result of comparison + curPath Path // The current path in the value tree + reporter reporter // Optional reporter used for difference formatting + + // dynChecker triggers pseudo-random checks for option correctness. + // It is safe for statelessCompare to mutate this value. + dynChecker dynChecker + + // These fields, once set by processOption, will not change. + exporters map[reflect.Type]bool // Set of structs with unexported field visibility + opts Options // List of all fundamental and filter options +} + +func newState(opts []Option) *state { + s := new(state) + for _, opt := range opts { + s.processOption(opt) + } + return s +} + +func (s *state) processOption(opt Option) { + switch opt := opt.(type) { + case nil: + case Options: + for _, o := range opt { + s.processOption(o) + } + case coreOption: + type filtered interface { + isFiltered() bool + } + if fopt, ok := opt.(filtered); ok && !fopt.isFiltered() { + panic(fmt.Sprintf("cannot use an unfiltered option: %v", opt)) + } + s.opts = append(s.opts, opt) + case visibleStructs: + if s.exporters == nil { + s.exporters = make(map[reflect.Type]bool) + } + for t := range opt { + s.exporters[t] = true + } + case reporter: + if s.reporter != nil { + panic("difference reporter already registered") + } + s.reporter = opt + default: + panic(fmt.Sprintf("unknown option %T", opt)) + } +} + +// statelessCompare compares two values and returns the result. +// This function is stateless in that it does not alter the current result, +// or output to any registered reporters. +func (s *state) statelessCompare(vx, vy reflect.Value) diff.Result { + // We do not save and restore the curPath because all of the compareX + // methods should properly push and pop from the path. + // It is an implementation bug if the contents of curPath differs from + // when calling this function to when returning from it. + + oldResult, oldReporter := s.result, s.reporter + s.result = diff.Result{} // Reset result + s.reporter = nil // Remove reporter to avoid spurious printouts + s.compareAny(vx, vy) + res := s.result + s.result, s.reporter = oldResult, oldReporter + return res +} + +func (s *state) compareAny(vx, vy reflect.Value) { + // TODO: Support cyclic data structures. + + // Rule 0: Differing types are never equal. + if !vx.IsValid() || !vy.IsValid() { + s.report(vx.IsValid() == vy.IsValid(), vx, vy) + return + } + if vx.Type() != vy.Type() { + s.report(false, vx, vy) // Possible for path to be empty + return + } + t := vx.Type() + if len(s.curPath) == 0 { + s.curPath.push(&pathStep{typ: t}) + defer s.curPath.pop() + } + vx, vy = s.tryExporting(vx, vy) + + // Rule 1: Check whether an option applies on this node in the value tree. + if s.tryOptions(vx, vy, t) { + return + } + + // Rule 2: Check whether the type has a valid Equal method. + if s.tryMethod(vx, vy, t) { + return + } + + // Rule 3: Recursively descend into each value's underlying kind. + switch t.Kind() { + case reflect.Bool: + s.report(vx.Bool() == vy.Bool(), vx, vy) + return + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + s.report(vx.Int() == vy.Int(), vx, vy) + return + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: + s.report(vx.Uint() == vy.Uint(), vx, vy) + return + case reflect.Float32, reflect.Float64: + s.report(vx.Float() == vy.Float(), vx, vy) + return + case reflect.Complex64, reflect.Complex128: + s.report(vx.Complex() == vy.Complex(), vx, vy) + return + case reflect.String: + s.report(vx.String() == vy.String(), vx, vy) + return + case reflect.Chan, reflect.UnsafePointer: + s.report(vx.Pointer() == vy.Pointer(), vx, vy) + return + case reflect.Func: + s.report(vx.IsNil() && vy.IsNil(), vx, vy) + return + case reflect.Ptr: + if vx.IsNil() || vy.IsNil() { + s.report(vx.IsNil() && vy.IsNil(), vx, vy) + return + } + s.curPath.push(&indirect{pathStep{t.Elem()}}) + defer s.curPath.pop() + s.compareAny(vx.Elem(), vy.Elem()) + return + case reflect.Interface: + if vx.IsNil() || vy.IsNil() { + s.report(vx.IsNil() && vy.IsNil(), vx, vy) + return + } + if vx.Elem().Type() != vy.Elem().Type() { + s.report(false, vx.Elem(), vy.Elem()) + return + } + s.curPath.push(&typeAssertion{pathStep{vx.Elem().Type()}}) + defer s.curPath.pop() + s.compareAny(vx.Elem(), vy.Elem()) + return + case reflect.Slice: + if vx.IsNil() || vy.IsNil() { + s.report(vx.IsNil() && vy.IsNil(), vx, vy) + return + } + fallthrough + case reflect.Array: + s.compareArray(vx, vy, t) + return + case reflect.Map: + s.compareMap(vx, vy, t) + return + case reflect.Struct: + s.compareStruct(vx, vy, t) + return + default: + panic(fmt.Sprintf("%v kind not handled", t.Kind())) + } +} + +func (s *state) tryExporting(vx, vy reflect.Value) (reflect.Value, reflect.Value) { + if sf, ok := s.curPath[len(s.curPath)-1].(*structField); ok && sf.unexported { + if sf.force { + // Use unsafe pointer arithmetic to get read-write access to an + // unexported field in the struct. + vx = unsafeRetrieveField(sf.pvx, sf.field) + vy = unsafeRetrieveField(sf.pvy, sf.field) + } else { + // We are not allowed to export the value, so invalidate them + // so that tryOptions can panic later if not explicitly ignored. + vx = nothing + vy = nothing + } + } + return vx, vy +} + +func (s *state) tryOptions(vx, vy reflect.Value, t reflect.Type) bool { + // If there were no FilterValues, we will not detect invalid inputs, + // so manually check for them and append invalid if necessary. + // We still evaluate the options since an ignore can override invalid. + opts := s.opts + if !vx.IsValid() || !vy.IsValid() { + opts = Options{opts, invalid{}} + } + + // Evaluate all filters and apply the remaining options. + if opt := opts.filter(s, vx, vy, t); opt != nil { + opt.apply(s, vx, vy) + return true + } + return false +} + +func (s *state) tryMethod(vx, vy reflect.Value, t reflect.Type) bool { + // Check if this type even has an Equal method. + m, ok := t.MethodByName("Equal") + if !ok || !function.IsType(m.Type, function.EqualAssignable) { + return false + } + + eq := s.callTTBFunc(m.Func, vx, vy) + s.report(eq, vx, vy) + return true +} + +func (s *state) callTRFunc(f, v reflect.Value) reflect.Value { + v = sanitizeValue(v, f.Type().In(0)) + if !s.dynChecker.Next() { + return f.Call([]reflect.Value{v})[0] + } + + // Run the function twice and ensure that we get the same results back. + // We run in goroutines so that the race detector (if enabled) can detect + // unsafe mutations to the input. + c := make(chan reflect.Value) + go detectRaces(c, f, v) + want := f.Call([]reflect.Value{v})[0] + if got := <-c; !s.statelessCompare(got, want).Equal() { + // To avoid false-positives with non-reflexive equality operations, + // we sanity check whether a value is equal to itself. + if !s.statelessCompare(want, want).Equal() { + return want + } + fn := getFuncName(f.Pointer()) + panic(fmt.Sprintf("non-deterministic function detected: %s", fn)) + } + return want +} + +func (s *state) callTTBFunc(f, x, y reflect.Value) bool { + x = sanitizeValue(x, f.Type().In(0)) + y = sanitizeValue(y, f.Type().In(1)) + if !s.dynChecker.Next() { + return f.Call([]reflect.Value{x, y})[0].Bool() + } + + // Swapping the input arguments is sufficient to check that + // f is symmetric and deterministic. + // We run in goroutines so that the race detector (if enabled) can detect + // unsafe mutations to the input. + c := make(chan reflect.Value) + go detectRaces(c, f, y, x) + want := f.Call([]reflect.Value{x, y})[0].Bool() + if got := <-c; !got.IsValid() || got.Bool() != want { + fn := getFuncName(f.Pointer()) + panic(fmt.Sprintf("non-deterministic or non-symmetric function detected: %s", fn)) + } + return want +} + +func detectRaces(c chan<- reflect.Value, f reflect.Value, vs ...reflect.Value) { + var ret reflect.Value + defer func() { + recover() // Ignore panics, let the other call to f panic instead + c <- ret + }() + ret = f.Call(vs)[0] +} + +// sanitizeValue converts nil interfaces of type T to those of type R, +// assuming that T is assignable to R. +// Otherwise, it returns the input value as is. +func sanitizeValue(v reflect.Value, t reflect.Type) reflect.Value { + // TODO(dsnet): Remove this hacky workaround. + // See https://golang.org/issue/22143 + if v.Kind() == reflect.Interface && v.IsNil() && v.Type() != t { + return reflect.New(t).Elem() + } + return v +} + +func (s *state) compareArray(vx, vy reflect.Value, t reflect.Type) { + step := &sliceIndex{pathStep{t.Elem()}, 0, 0} + s.curPath.push(step) + + // Compute an edit-script for slices vx and vy. + es := diff.Difference(vx.Len(), vy.Len(), func(ix, iy int) diff.Result { + step.xkey, step.ykey = ix, iy + return s.statelessCompare(vx.Index(ix), vy.Index(iy)) + }) + + // Report the entire slice as is if the arrays are of primitive kind, + // and the arrays are different enough. + isPrimitive := false + switch t.Elem().Kind() { + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, + reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, + reflect.Bool, reflect.Float32, reflect.Float64, reflect.Complex64, reflect.Complex128: + isPrimitive = true + } + if isPrimitive && es.Dist() > (vx.Len()+vy.Len())/4 { + s.curPath.pop() // Pop first since we are reporting the whole slice + s.report(false, vx, vy) + return + } + + // Replay the edit-script. + var ix, iy int + for _, e := range es { + switch e { + case diff.UniqueX: + step.xkey, step.ykey = ix, -1 + s.report(false, vx.Index(ix), nothing) + ix++ + case diff.UniqueY: + step.xkey, step.ykey = -1, iy + s.report(false, nothing, vy.Index(iy)) + iy++ + default: + step.xkey, step.ykey = ix, iy + if e == diff.Identity { + s.report(true, vx.Index(ix), vy.Index(iy)) + } else { + s.compareAny(vx.Index(ix), vy.Index(iy)) + } + ix++ + iy++ + } + } + s.curPath.pop() + return +} + +func (s *state) compareMap(vx, vy reflect.Value, t reflect.Type) { + if vx.IsNil() || vy.IsNil() { + s.report(vx.IsNil() && vy.IsNil(), vx, vy) + return + } + + // We combine and sort the two map keys so that we can perform the + // comparisons in a deterministic order. + step := &mapIndex{pathStep: pathStep{t.Elem()}} + s.curPath.push(step) + defer s.curPath.pop() + for _, k := range value.SortKeys(append(vx.MapKeys(), vy.MapKeys()...)) { + step.key = k + vvx := vx.MapIndex(k) + vvy := vy.MapIndex(k) + switch { + case vvx.IsValid() && vvy.IsValid(): + s.compareAny(vvx, vvy) + case vvx.IsValid() && !vvy.IsValid(): + s.report(false, vvx, nothing) + case !vvx.IsValid() && vvy.IsValid(): + s.report(false, nothing, vvy) + default: + // It is possible for both vvx and vvy to be invalid if the + // key contained a NaN value in it. There is no way in + // reflection to be able to retrieve these values. + // See https://golang.org/issue/11104 + panic(fmt.Sprintf("%#v has map key with NaNs", s.curPath)) + } + } +} + +func (s *state) compareStruct(vx, vy reflect.Value, t reflect.Type) { + var vax, vay reflect.Value // Addressable versions of vx and vy + + step := &structField{} + s.curPath.push(step) + defer s.curPath.pop() + for i := 0; i < t.NumField(); i++ { + vvx := vx.Field(i) + vvy := vy.Field(i) + step.typ = t.Field(i).Type + step.name = t.Field(i).Name + step.idx = i + step.unexported = !isExported(step.name) + if step.unexported { + // Defer checking of unexported fields until later to give an + // Ignore a chance to ignore the field. + if !vax.IsValid() || !vay.IsValid() { + // For unsafeRetrieveField to work, the parent struct must + // be addressable. Create a new copy of the values if + // necessary to make them addressable. + vax = makeAddressable(vx) + vay = makeAddressable(vy) + } + step.force = s.exporters[t] + step.pvx = vax + step.pvy = vay + step.field = t.Field(i) + } + s.compareAny(vvx, vvy) + } +} + +// report records the result of a single comparison. +// It also calls Report if any reporter is registered. +func (s *state) report(eq bool, vx, vy reflect.Value) { + if eq { + s.result.NSame++ + } else { + s.result.NDiff++ + } + if s.reporter != nil { + s.reporter.Report(vx, vy, eq, s.curPath) + } +} + +// dynChecker tracks the state needed to periodically perform checks that +// user provided functions are symmetric and deterministic. +// The zero value is safe for immediate use. +type dynChecker struct{ curr, next int } + +// Next increments the state and reports whether a check should be performed. +// +// Checks occur every Nth function call, where N is a triangular number: +// 0 1 3 6 10 15 21 28 36 45 55 66 78 91 105 120 136 153 171 190 ... +// See https://en.wikipedia.org/wiki/Triangular_number +// +// This sequence ensures that the cost of checks drops significantly as +// the number of functions calls grows larger. +func (dc *dynChecker) Next() bool { + ok := dc.curr == dc.next + if ok { + dc.curr = 0 + dc.next++ + } + dc.curr++ + return ok +} + +// makeAddressable returns a value that is always addressable. +// It returns the input verbatim if it is already addressable, +// otherwise it creates a new value and returns an addressable copy. +func makeAddressable(v reflect.Value) reflect.Value { + if v.CanAddr() { + return v + } + vc := reflect.New(v.Type()).Elem() + vc.Set(v) + return vc +} diff --git a/vendor/github.com/google/go-cmp/cmp/internal/diff/debug_disable.go b/vendor/github.com/google/go-cmp/cmp/internal/diff/debug_disable.go new file mode 100644 index 000000000..42afa4960 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/internal/diff/debug_disable.go @@ -0,0 +1,17 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// +build !debug + +package diff + +var debug debugger + +type debugger struct{} + +func (debugger) Begin(_, _ int, f EqualFunc, _, _ *EditScript) EqualFunc { + return f +} +func (debugger) Update() {} +func (debugger) Finish() {} diff --git a/vendor/github.com/google/go-cmp/cmp/internal/diff/debug_enable.go b/vendor/github.com/google/go-cmp/cmp/internal/diff/debug_enable.go new file mode 100644 index 000000000..fd9f7f177 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/internal/diff/debug_enable.go @@ -0,0 +1,122 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// +build debug + +package diff + +import ( + "fmt" + "strings" + "sync" + "time" +) + +// The algorithm can be seen running in real-time by enabling debugging: +// go test -tags=debug -v +// +// Example output: +// === RUN TestDifference/#34 +// โ”Œโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ” +// โ”‚ \ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท # ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท \ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท \ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท X # ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท # \ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท # # ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท # \ ยท ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท \ ยท ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท \ ยท ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท ยท \ ยท ยท ยท ยท ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท \ ยท ยท # ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท \ # # ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท # # # ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท # # # # ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท ยท # # # # # ยท โ”‚ +// โ”‚ ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท ยท \ โ”‚ +// โ””โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”˜ +// [.Y..M.XY......YXYXY.|] +// +// The grid represents the edit-graph where the horizontal axis represents +// list X and the vertical axis represents list Y. The start of the two lists +// is the top-left, while the ends are the bottom-right. The 'ยท' represents +// an unexplored node in the graph. The '\' indicates that the two symbols +// from list X and Y are equal. The 'X' indicates that two symbols are similar +// (but not exactly equal) to each other. The '#' indicates that the two symbols +// are different (and not similar). The algorithm traverses this graph trying to +// make the paths starting in the top-left and the bottom-right connect. +// +// The series of '.', 'X', 'Y', and 'M' characters at the bottom represents +// the currently established path from the forward and reverse searches, +// separated by a '|' character. + +const ( + updateDelay = 100 * time.Millisecond + finishDelay = 500 * time.Millisecond + ansiTerminal = true // ANSI escape codes used to move terminal cursor +) + +var debug debugger + +type debugger struct { + sync.Mutex + p1, p2 EditScript + fwdPath, revPath *EditScript + grid []byte + lines int +} + +func (dbg *debugger) Begin(nx, ny int, f EqualFunc, p1, p2 *EditScript) EqualFunc { + dbg.Lock() + dbg.fwdPath, dbg.revPath = p1, p2 + top := "โ”Œโ”€" + strings.Repeat("โ”€โ”€", nx) + "โ”\n" + row := "โ”‚ " + strings.Repeat("ยท ", nx) + "โ”‚\n" + btm := "โ””โ”€" + strings.Repeat("โ”€โ”€", nx) + "โ”˜\n" + dbg.grid = []byte(top + strings.Repeat(row, ny) + btm) + dbg.lines = strings.Count(dbg.String(), "\n") + fmt.Print(dbg) + + // Wrap the EqualFunc so that we can intercept each result. + return func(ix, iy int) (r Result) { + cell := dbg.grid[len(top)+iy*len(row):][len("โ”‚ ")+len("ยท ")*ix:][:len("ยท")] + for i := range cell { + cell[i] = 0 // Zero out the multiple bytes of UTF-8 middle-dot + } + switch r = f(ix, iy); { + case r.Equal(): + cell[0] = '\\' + case r.Similar(): + cell[0] = 'X' + default: + cell[0] = '#' + } + return + } +} + +func (dbg *debugger) Update() { + dbg.print(updateDelay) +} + +func (dbg *debugger) Finish() { + dbg.print(finishDelay) + dbg.Unlock() +} + +func (dbg *debugger) String() string { + dbg.p1, dbg.p2 = *dbg.fwdPath, dbg.p2[:0] + for i := len(*dbg.revPath) - 1; i >= 0; i-- { + dbg.p2 = append(dbg.p2, (*dbg.revPath)[i]) + } + return fmt.Sprintf("%s[%v|%v]\n\n", dbg.grid, dbg.p1, dbg.p2) +} + +func (dbg *debugger) print(d time.Duration) { + if ansiTerminal { + fmt.Printf("\x1b[%dA", dbg.lines) // Reset terminal cursor + } + fmt.Print(dbg) + time.Sleep(d) +} diff --git a/vendor/github.com/google/go-cmp/cmp/internal/diff/diff.go b/vendor/github.com/google/go-cmp/cmp/internal/diff/diff.go new file mode 100644 index 000000000..260befea2 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/internal/diff/diff.go @@ -0,0 +1,363 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// Package diff implements an algorithm for producing edit-scripts. +// The edit-script is a sequence of operations needed to transform one list +// of symbols into another (or vice-versa). The edits allowed are insertions, +// deletions, and modifications. The summation of all edits is called the +// Levenshtein distance as this problem is well-known in computer science. +// +// This package prioritizes performance over accuracy. That is, the run time +// is more important than obtaining a minimal Levenshtein distance. +package diff + +// EditType represents a single operation within an edit-script. +type EditType uint8 + +const ( + // Identity indicates that a symbol pair is identical in both list X and Y. + Identity EditType = iota + // UniqueX indicates that a symbol only exists in X and not Y. + UniqueX + // UniqueY indicates that a symbol only exists in Y and not X. + UniqueY + // Modified indicates that a symbol pair is a modification of each other. + Modified +) + +// EditScript represents the series of differences between two lists. +type EditScript []EditType + +// String returns a human-readable string representing the edit-script where +// Identity, UniqueX, UniqueY, and Modified are represented by the +// '.', 'X', 'Y', and 'M' characters, respectively. +func (es EditScript) String() string { + b := make([]byte, len(es)) + for i, e := range es { + switch e { + case Identity: + b[i] = '.' + case UniqueX: + b[i] = 'X' + case UniqueY: + b[i] = 'Y' + case Modified: + b[i] = 'M' + default: + panic("invalid edit-type") + } + } + return string(b) +} + +// stats returns a histogram of the number of each type of edit operation. +func (es EditScript) stats() (s struct{ NI, NX, NY, NM int }) { + for _, e := range es { + switch e { + case Identity: + s.NI++ + case UniqueX: + s.NX++ + case UniqueY: + s.NY++ + case Modified: + s.NM++ + default: + panic("invalid edit-type") + } + } + return +} + +// Dist is the Levenshtein distance and is guaranteed to be 0 if and only if +// lists X and Y are equal. +func (es EditScript) Dist() int { return len(es) - es.stats().NI } + +// LenX is the length of the X list. +func (es EditScript) LenX() int { return len(es) - es.stats().NY } + +// LenY is the length of the Y list. +func (es EditScript) LenY() int { return len(es) - es.stats().NX } + +// EqualFunc reports whether the symbols at indexes ix and iy are equal. +// When called by Difference, the index is guaranteed to be within nx and ny. +type EqualFunc func(ix int, iy int) Result + +// Result is the result of comparison. +// NSame is the number of sub-elements that are equal. +// NDiff is the number of sub-elements that are not equal. +type Result struct{ NSame, NDiff int } + +// Equal indicates whether the symbols are equal. Two symbols are equal +// if and only if NDiff == 0. If Equal, then they are also Similar. +func (r Result) Equal() bool { return r.NDiff == 0 } + +// Similar indicates whether two symbols are similar and may be represented +// by using the Modified type. As a special case, we consider binary comparisons +// (i.e., those that return Result{1, 0} or Result{0, 1}) to be similar. +// +// The exact ratio of NSame to NDiff to determine similarity may change. +func (r Result) Similar() bool { + // Use NSame+1 to offset NSame so that binary comparisons are similar. + return r.NSame+1 >= r.NDiff +} + +// Difference reports whether two lists of lengths nx and ny are equal +// given the definition of equality provided as f. +// +// This function returns an edit-script, which is a sequence of operations +// needed to convert one list into the other. The following invariants for +// the edit-script are maintained: +// โ€ข eq == (es.Dist()==0) +// โ€ข nx == es.LenX() +// โ€ข ny == es.LenY() +// +// This algorithm is not guaranteed to be an optimal solution (i.e., one that +// produces an edit-script with a minimal Levenshtein distance). This algorithm +// favors performance over optimality. The exact output is not guaranteed to +// be stable and may change over time. +func Difference(nx, ny int, f EqualFunc) (es EditScript) { + // This algorithm is based on traversing what is known as an "edit-graph". + // See Figure 1 from "An O(ND) Difference Algorithm and Its Variations" + // by Eugene W. Myers. Since D can be as large as N itself, this is + // effectively O(N^2). Unlike the algorithm from that paper, we are not + // interested in the optimal path, but at least some "decent" path. + // + // For example, let X and Y be lists of symbols: + // X = [A B C A B B A] + // Y = [C B A B A C] + // + // The edit-graph can be drawn as the following: + // A B C A B B A + // โ”Œโ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ” + // C โ”‚_|_|\|_|_|_|_โ”‚ 0 + // B โ”‚_|\|_|_|\|\|_โ”‚ 1 + // A โ”‚\|_|_|\|_|_|\โ”‚ 2 + // B โ”‚_|\|_|_|\|\|_โ”‚ 3 + // A โ”‚\|_|_|\|_|_|\โ”‚ 4 + // C โ”‚ | |\| | | | โ”‚ 5 + // โ””โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”€โ”˜ 6 + // 0 1 2 3 4 5 6 7 + // + // List X is written along the horizontal axis, while list Y is written + // along the vertical axis. At any point on this grid, if the symbol in + // list X matches the corresponding symbol in list Y, then a '\' is drawn. + // The goal of any minimal edit-script algorithm is to find a path from the + // top-left corner to the bottom-right corner, while traveling through the + // fewest horizontal or vertical edges. + // A horizontal edge is equivalent to inserting a symbol from list X. + // A vertical edge is equivalent to inserting a symbol from list Y. + // A diagonal edge is equivalent to a matching symbol between both X and Y. + + // Invariants: + // โ€ข 0 โ‰ค fwdPath.X โ‰ค (fwdFrontier.X, revFrontier.X) โ‰ค revPath.X โ‰ค nx + // โ€ข 0 โ‰ค fwdPath.Y โ‰ค (fwdFrontier.Y, revFrontier.Y) โ‰ค revPath.Y โ‰ค ny + // + // In general: + // โ€ข fwdFrontier.X < revFrontier.X + // โ€ข fwdFrontier.Y < revFrontier.Y + // Unless, it is time for the algorithm to terminate. + fwdPath := path{+1, point{0, 0}, make(EditScript, 0, (nx+ny)/2)} + revPath := path{-1, point{nx, ny}, make(EditScript, 0)} + fwdFrontier := fwdPath.point // Forward search frontier + revFrontier := revPath.point // Reverse search frontier + + // Search budget bounds the cost of searching for better paths. + // The longest sequence of non-matching symbols that can be tolerated is + // approximately the square-root of the search budget. + searchBudget := 4 * (nx + ny) // O(n) + + // The algorithm below is a greedy, meet-in-the-middle algorithm for + // computing sub-optimal edit-scripts between two lists. + // + // The algorithm is approximately as follows: + // โ€ข Searching for differences switches back-and-forth between + // a search that starts at the beginning (the top-left corner), and + // a search that starts at the end (the bottom-right corner). The goal of + // the search is connect with the search from the opposite corner. + // โ€ข As we search, we build a path in a greedy manner, where the first + // match seen is added to the path (this is sub-optimal, but provides a + // decent result in practice). When matches are found, we try the next pair + // of symbols in the lists and follow all matches as far as possible. + // โ€ข When searching for matches, we search along a diagonal going through + // through the "frontier" point. If no matches are found, we advance the + // frontier towards the opposite corner. + // โ€ข This algorithm terminates when either the X coordinates or the + // Y coordinates of the forward and reverse frontier points ever intersect. + // + // This algorithm is correct even if searching only in the forward direction + // or in the reverse direction. We do both because it is commonly observed + // that two lists commonly differ because elements were added to the front + // or end of the other list. + // + // Running the tests with the "debug" build tag prints a visualization of + // the algorithm running in real-time. This is educational for understanding + // how the algorithm works. See debug_enable.go. + f = debug.Begin(nx, ny, f, &fwdPath.es, &revPath.es) + for { + // Forward search from the beginning. + if fwdFrontier.X >= revFrontier.X || fwdFrontier.Y >= revFrontier.Y || searchBudget == 0 { + break + } + for stop1, stop2, i := false, false, 0; !(stop1 && stop2) && searchBudget > 0; i++ { + // Search in a diagonal pattern for a match. + z := zigzag(i) + p := point{fwdFrontier.X + z, fwdFrontier.Y - z} + switch { + case p.X >= revPath.X || p.Y < fwdPath.Y: + stop1 = true // Hit top-right corner + case p.Y >= revPath.Y || p.X < fwdPath.X: + stop2 = true // Hit bottom-left corner + case f(p.X, p.Y).Equal(): + // Match found, so connect the path to this point. + fwdPath.connect(p, f) + fwdPath.append(Identity) + // Follow sequence of matches as far as possible. + for fwdPath.X < revPath.X && fwdPath.Y < revPath.Y { + if !f(fwdPath.X, fwdPath.Y).Equal() { + break + } + fwdPath.append(Identity) + } + fwdFrontier = fwdPath.point + stop1, stop2 = true, true + default: + searchBudget-- // Match not found + } + debug.Update() + } + // Advance the frontier towards reverse point. + if revPath.X-fwdFrontier.X >= revPath.Y-fwdFrontier.Y { + fwdFrontier.X++ + } else { + fwdFrontier.Y++ + } + + // Reverse search from the end. + if fwdFrontier.X >= revFrontier.X || fwdFrontier.Y >= revFrontier.Y || searchBudget == 0 { + break + } + for stop1, stop2, i := false, false, 0; !(stop1 && stop2) && searchBudget > 0; i++ { + // Search in a diagonal pattern for a match. + z := zigzag(i) + p := point{revFrontier.X - z, revFrontier.Y + z} + switch { + case fwdPath.X >= p.X || revPath.Y < p.Y: + stop1 = true // Hit bottom-left corner + case fwdPath.Y >= p.Y || revPath.X < p.X: + stop2 = true // Hit top-right corner + case f(p.X-1, p.Y-1).Equal(): + // Match found, so connect the path to this point. + revPath.connect(p, f) + revPath.append(Identity) + // Follow sequence of matches as far as possible. + for fwdPath.X < revPath.X && fwdPath.Y < revPath.Y { + if !f(revPath.X-1, revPath.Y-1).Equal() { + break + } + revPath.append(Identity) + } + revFrontier = revPath.point + stop1, stop2 = true, true + default: + searchBudget-- // Match not found + } + debug.Update() + } + // Advance the frontier towards forward point. + if revFrontier.X-fwdPath.X >= revFrontier.Y-fwdPath.Y { + revFrontier.X-- + } else { + revFrontier.Y-- + } + } + + // Join the forward and reverse paths and then append the reverse path. + fwdPath.connect(revPath.point, f) + for i := len(revPath.es) - 1; i >= 0; i-- { + t := revPath.es[i] + revPath.es = revPath.es[:i] + fwdPath.append(t) + } + debug.Finish() + return fwdPath.es +} + +type path struct { + dir int // +1 if forward, -1 if reverse + point // Leading point of the EditScript path + es EditScript +} + +// connect appends any necessary Identity, Modified, UniqueX, or UniqueY types +// to the edit-script to connect p.point to dst. +func (p *path) connect(dst point, f EqualFunc) { + if p.dir > 0 { + // Connect in forward direction. + for dst.X > p.X && dst.Y > p.Y { + switch r := f(p.X, p.Y); { + case r.Equal(): + p.append(Identity) + case r.Similar(): + p.append(Modified) + case dst.X-p.X >= dst.Y-p.Y: + p.append(UniqueX) + default: + p.append(UniqueY) + } + } + for dst.X > p.X { + p.append(UniqueX) + } + for dst.Y > p.Y { + p.append(UniqueY) + } + } else { + // Connect in reverse direction. + for p.X > dst.X && p.Y > dst.Y { + switch r := f(p.X-1, p.Y-1); { + case r.Equal(): + p.append(Identity) + case r.Similar(): + p.append(Modified) + case p.Y-dst.Y >= p.X-dst.X: + p.append(UniqueY) + default: + p.append(UniqueX) + } + } + for p.X > dst.X { + p.append(UniqueX) + } + for p.Y > dst.Y { + p.append(UniqueY) + } + } +} + +func (p *path) append(t EditType) { + p.es = append(p.es, t) + switch t { + case Identity, Modified: + p.add(p.dir, p.dir) + case UniqueX: + p.add(p.dir, 0) + case UniqueY: + p.add(0, p.dir) + } + debug.Update() +} + +type point struct{ X, Y int } + +func (p *point) add(dx, dy int) { p.X += dx; p.Y += dy } + +// zigzag maps a consecutive sequence of integers to a zig-zag sequence. +// [0 1 2 3 4 5 ...] => [0 -1 +1 -2 +2 ...] +func zigzag(x int) int { + if x&1 != 0 { + x = ^x + } + return x >> 1 +} diff --git a/vendor/github.com/google/go-cmp/cmp/internal/function/func.go b/vendor/github.com/google/go-cmp/cmp/internal/function/func.go new file mode 100644 index 000000000..4c35ff11e --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/internal/function/func.go @@ -0,0 +1,49 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// Package function identifies function types. +package function + +import "reflect" + +type funcType int + +const ( + _ funcType = iota + + ttbFunc // func(T, T) bool + tibFunc // func(T, I) bool + trFunc // func(T) R + + Equal = ttbFunc // func(T, T) bool + EqualAssignable = tibFunc // func(T, I) bool; encapsulates func(T, T) bool + Transformer = trFunc // func(T) R + ValueFilter = ttbFunc // func(T, T) bool + Less = ttbFunc // func(T, T) bool +) + +var boolType = reflect.TypeOf(true) + +// IsType reports whether the reflect.Type is of the specified function type. +func IsType(t reflect.Type, ft funcType) bool { + if t == nil || t.Kind() != reflect.Func || t.IsVariadic() { + return false + } + ni, no := t.NumIn(), t.NumOut() + switch ft { + case ttbFunc: // func(T, T) bool + if ni == 2 && no == 1 && t.In(0) == t.In(1) && t.Out(0) == boolType { + return true + } + case tibFunc: // func(T, I) bool + if ni == 2 && no == 1 && t.In(0).AssignableTo(t.In(1)) && t.Out(0) == boolType { + return true + } + case trFunc: // func(T) R + if ni == 1 && no == 1 { + return true + } + } + return false +} diff --git a/vendor/github.com/google/go-cmp/cmp/internal/value/format.go b/vendor/github.com/google/go-cmp/cmp/internal/value/format.go new file mode 100644 index 000000000..657e50877 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/internal/value/format.go @@ -0,0 +1,277 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// Package value provides functionality for reflect.Value types. +package value + +import ( + "fmt" + "reflect" + "strconv" + "strings" + "unicode" +) + +var stringerIface = reflect.TypeOf((*fmt.Stringer)(nil)).Elem() + +// Format formats the value v as a string. +// +// This is similar to fmt.Sprintf("%+v", v) except this: +// * Prints the type unless it can be elided +// * Avoids printing struct fields that are zero +// * Prints a nil-slice as being nil, not empty +// * Prints map entries in deterministic order +func Format(v reflect.Value, conf FormatConfig) string { + conf.printType = true + conf.followPointers = true + conf.realPointers = true + return formatAny(v, conf, nil) +} + +type FormatConfig struct { + UseStringer bool // Should the String method be used if available? + printType bool // Should we print the type before the value? + PrintPrimitiveType bool // Should we print the type of primitives? + followPointers bool // Should we recursively follow pointers? + realPointers bool // Should we print the real address of pointers? +} + +func formatAny(v reflect.Value, conf FormatConfig, visited map[uintptr]bool) string { + // TODO: Should this be a multi-line printout in certain situations? + + if !v.IsValid() { + return "" + } + if conf.UseStringer && v.Type().Implements(stringerIface) && v.CanInterface() { + if (v.Kind() == reflect.Ptr || v.Kind() == reflect.Interface) && v.IsNil() { + return "" + } + + const stringerPrefix = "s" // Indicates that the String method was used + s := v.Interface().(fmt.Stringer).String() + return stringerPrefix + formatString(s) + } + + switch v.Kind() { + case reflect.Bool: + return formatPrimitive(v.Type(), v.Bool(), conf) + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return formatPrimitive(v.Type(), v.Int(), conf) + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: + if v.Type().PkgPath() == "" || v.Kind() == reflect.Uintptr { + // Unnamed uints are usually bytes or words, so use hexadecimal. + return formatPrimitive(v.Type(), formatHex(v.Uint()), conf) + } + return formatPrimitive(v.Type(), v.Uint(), conf) + case reflect.Float32, reflect.Float64: + return formatPrimitive(v.Type(), v.Float(), conf) + case reflect.Complex64, reflect.Complex128: + return formatPrimitive(v.Type(), v.Complex(), conf) + case reflect.String: + return formatPrimitive(v.Type(), formatString(v.String()), conf) + case reflect.UnsafePointer, reflect.Chan, reflect.Func: + return formatPointer(v, conf) + case reflect.Ptr: + if v.IsNil() { + if conf.printType { + return fmt.Sprintf("(%v)(nil)", v.Type()) + } + return "" + } + if visited[v.Pointer()] || !conf.followPointers { + return formatPointer(v, conf) + } + visited = insertPointer(visited, v.Pointer()) + return "&" + formatAny(v.Elem(), conf, visited) + case reflect.Interface: + if v.IsNil() { + if conf.printType { + return fmt.Sprintf("%v(nil)", v.Type()) + } + return "" + } + return formatAny(v.Elem(), conf, visited) + case reflect.Slice: + if v.IsNil() { + if conf.printType { + return fmt.Sprintf("%v(nil)", v.Type()) + } + return "" + } + if visited[v.Pointer()] { + return formatPointer(v, conf) + } + visited = insertPointer(visited, v.Pointer()) + fallthrough + case reflect.Array: + var ss []string + subConf := conf + subConf.printType = v.Type().Elem().Kind() == reflect.Interface + for i := 0; i < v.Len(); i++ { + s := formatAny(v.Index(i), subConf, visited) + ss = append(ss, s) + } + s := fmt.Sprintf("{%s}", strings.Join(ss, ", ")) + if conf.printType { + return v.Type().String() + s + } + return s + case reflect.Map: + if v.IsNil() { + if conf.printType { + return fmt.Sprintf("%v(nil)", v.Type()) + } + return "" + } + if visited[v.Pointer()] { + return formatPointer(v, conf) + } + visited = insertPointer(visited, v.Pointer()) + + var ss []string + keyConf, valConf := conf, conf + keyConf.printType = v.Type().Key().Kind() == reflect.Interface + keyConf.followPointers = false + valConf.printType = v.Type().Elem().Kind() == reflect.Interface + for _, k := range SortKeys(v.MapKeys()) { + sk := formatAny(k, keyConf, visited) + sv := formatAny(v.MapIndex(k), valConf, visited) + ss = append(ss, fmt.Sprintf("%s: %s", sk, sv)) + } + s := fmt.Sprintf("{%s}", strings.Join(ss, ", ")) + if conf.printType { + return v.Type().String() + s + } + return s + case reflect.Struct: + var ss []string + subConf := conf + subConf.printType = true + for i := 0; i < v.NumField(); i++ { + vv := v.Field(i) + if isZero(vv) { + continue // Elide zero value fields + } + name := v.Type().Field(i).Name + subConf.UseStringer = conf.UseStringer + s := formatAny(vv, subConf, visited) + ss = append(ss, fmt.Sprintf("%s: %s", name, s)) + } + s := fmt.Sprintf("{%s}", strings.Join(ss, ", ")) + if conf.printType { + return v.Type().String() + s + } + return s + default: + panic(fmt.Sprintf("%v kind not handled", v.Kind())) + } +} + +func formatString(s string) string { + // Use quoted string if it the same length as a raw string literal. + // Otherwise, attempt to use the raw string form. + qs := strconv.Quote(s) + if len(qs) == 1+len(s)+1 { + return qs + } + + // Disallow newlines to ensure output is a single line. + // Only allow printable runes for readability purposes. + rawInvalid := func(r rune) bool { + return r == '`' || r == '\n' || !unicode.IsPrint(r) + } + if strings.IndexFunc(s, rawInvalid) < 0 { + return "`" + s + "`" + } + return qs +} + +func formatPrimitive(t reflect.Type, v interface{}, conf FormatConfig) string { + if conf.printType && (conf.PrintPrimitiveType || t.PkgPath() != "") { + return fmt.Sprintf("%v(%v)", t, v) + } + return fmt.Sprintf("%v", v) +} + +func formatPointer(v reflect.Value, conf FormatConfig) string { + p := v.Pointer() + if !conf.realPointers { + p = 0 // For deterministic printing purposes + } + s := formatHex(uint64(p)) + if conf.printType { + return fmt.Sprintf("(%v)(%s)", v.Type(), s) + } + return s +} + +func formatHex(u uint64) string { + var f string + switch { + case u <= 0xff: + f = "0x%02x" + case u <= 0xffff: + f = "0x%04x" + case u <= 0xffffff: + f = "0x%06x" + case u <= 0xffffffff: + f = "0x%08x" + case u <= 0xffffffffff: + f = "0x%010x" + case u <= 0xffffffffffff: + f = "0x%012x" + case u <= 0xffffffffffffff: + f = "0x%014x" + case u <= 0xffffffffffffffff: + f = "0x%016x" + } + return fmt.Sprintf(f, u) +} + +// insertPointer insert p into m, allocating m if necessary. +func insertPointer(m map[uintptr]bool, p uintptr) map[uintptr]bool { + if m == nil { + m = make(map[uintptr]bool) + } + m[p] = true + return m +} + +// isZero reports whether v is the zero value. +// This does not rely on Interface and so can be used on unexported fields. +func isZero(v reflect.Value) bool { + switch v.Kind() { + case reflect.Bool: + return v.Bool() == false + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return v.Int() == 0 + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: + return v.Uint() == 0 + case reflect.Float32, reflect.Float64: + return v.Float() == 0 + case reflect.Complex64, reflect.Complex128: + return v.Complex() == 0 + case reflect.String: + return v.String() == "" + case reflect.UnsafePointer: + return v.Pointer() == 0 + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Ptr, reflect.Map, reflect.Slice: + return v.IsNil() + case reflect.Array: + for i := 0; i < v.Len(); i++ { + if !isZero(v.Index(i)) { + return false + } + } + return true + case reflect.Struct: + for i := 0; i < v.NumField(); i++ { + if !isZero(v.Field(i)) { + return false + } + } + return true + } + return false +} diff --git a/vendor/github.com/google/go-cmp/cmp/internal/value/sort.go b/vendor/github.com/google/go-cmp/cmp/internal/value/sort.go new file mode 100644 index 000000000..fe8aa27a0 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/internal/value/sort.go @@ -0,0 +1,111 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +package value + +import ( + "fmt" + "math" + "reflect" + "sort" +) + +// SortKeys sorts a list of map keys, deduplicating keys if necessary. +// The type of each value must be comparable. +func SortKeys(vs []reflect.Value) []reflect.Value { + if len(vs) == 0 { + return vs + } + + // Sort the map keys. + sort.Sort(valueSorter(vs)) + + // Deduplicate keys (fails for NaNs). + vs2 := vs[:1] + for _, v := range vs[1:] { + if isLess(vs2[len(vs2)-1], v) { + vs2 = append(vs2, v) + } + } + return vs2 +} + +// TODO: Use sort.Slice once Google AppEngine is on Go1.8 or above. +type valueSorter []reflect.Value + +func (vs valueSorter) Len() int { return len(vs) } +func (vs valueSorter) Less(i, j int) bool { return isLess(vs[i], vs[j]) } +func (vs valueSorter) Swap(i, j int) { vs[i], vs[j] = vs[j], vs[i] } + +// isLess is a generic function for sorting arbitrary map keys. +// The inputs must be of the same type and must be comparable. +func isLess(x, y reflect.Value) bool { + switch x.Type().Kind() { + case reflect.Bool: + return !x.Bool() && y.Bool() + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return x.Int() < y.Int() + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: + return x.Uint() < y.Uint() + case reflect.Float32, reflect.Float64: + fx, fy := x.Float(), y.Float() + return fx < fy || math.IsNaN(fx) && !math.IsNaN(fy) + case reflect.Complex64, reflect.Complex128: + cx, cy := x.Complex(), y.Complex() + rx, ix, ry, iy := real(cx), imag(cx), real(cy), imag(cy) + if rx == ry || (math.IsNaN(rx) && math.IsNaN(ry)) { + return ix < iy || math.IsNaN(ix) && !math.IsNaN(iy) + } + return rx < ry || math.IsNaN(rx) && !math.IsNaN(ry) + case reflect.Ptr, reflect.UnsafePointer, reflect.Chan: + return x.Pointer() < y.Pointer() + case reflect.String: + return x.String() < y.String() + case reflect.Array: + for i := 0; i < x.Len(); i++ { + if isLess(x.Index(i), y.Index(i)) { + return true + } + if isLess(y.Index(i), x.Index(i)) { + return false + } + } + return false + case reflect.Struct: + for i := 0; i < x.NumField(); i++ { + if isLess(x.Field(i), y.Field(i)) { + return true + } + if isLess(y.Field(i), x.Field(i)) { + return false + } + } + return false + case reflect.Interface: + vx, vy := x.Elem(), y.Elem() + if !vx.IsValid() || !vy.IsValid() { + return !vx.IsValid() && vy.IsValid() + } + tx, ty := vx.Type(), vy.Type() + if tx == ty { + return isLess(x.Elem(), y.Elem()) + } + if tx.Kind() != ty.Kind() { + return vx.Kind() < vy.Kind() + } + if tx.String() != ty.String() { + return tx.String() < ty.String() + } + if tx.PkgPath() != ty.PkgPath() { + return tx.PkgPath() < ty.PkgPath() + } + // This can happen in rare situations, so we fallback to just comparing + // the unique pointer for a reflect.Type. This guarantees deterministic + // ordering within a program, but it is obviously not stable. + return reflect.ValueOf(vx.Type()).Pointer() < reflect.ValueOf(vy.Type()).Pointer() + default: + // Must be Func, Map, or Slice; which are not comparable. + panic(fmt.Sprintf("%T is not comparable", x.Type())) + } +} diff --git a/vendor/github.com/google/go-cmp/cmp/options.go b/vendor/github.com/google/go-cmp/cmp/options.go new file mode 100644 index 000000000..91d4b066e --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/options.go @@ -0,0 +1,453 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +package cmp + +import ( + "fmt" + "reflect" + "runtime" + "strings" + + "github.com/google/go-cmp/cmp/internal/function" +) + +// Option configures for specific behavior of Equal and Diff. In particular, +// the fundamental Option functions (Ignore, Transformer, and Comparer), +// configure how equality is determined. +// +// The fundamental options may be composed with filters (FilterPath and +// FilterValues) to control the scope over which they are applied. +// +// The cmp/cmpopts package provides helper functions for creating options that +// may be used with Equal and Diff. +type Option interface { + // filter applies all filters and returns the option that remains. + // Each option may only read s.curPath and call s.callTTBFunc. + // + // An Options is returned only if multiple comparers or transformers + // can apply simultaneously and will only contain values of those types + // or sub-Options containing values of those types. + filter(s *state, vx, vy reflect.Value, t reflect.Type) applicableOption +} + +// applicableOption represents the following types: +// Fundamental: ignore | invalid | *comparer | *transformer +// Grouping: Options +type applicableOption interface { + Option + + // apply executes the option, which may mutate s or panic. + apply(s *state, vx, vy reflect.Value) +} + +// coreOption represents the following types: +// Fundamental: ignore | invalid | *comparer | *transformer +// Filters: *pathFilter | *valuesFilter +type coreOption interface { + Option + isCore() +} + +type core struct{} + +func (core) isCore() {} + +// Options is a list of Option values that also satisfies the Option interface. +// Helper comparison packages may return an Options value when packing multiple +// Option values into a single Option. When this package processes an Options, +// it will be implicitly expanded into a flat list. +// +// Applying a filter on an Options is equivalent to applying that same filter +// on all individual options held within. +type Options []Option + +func (opts Options) filter(s *state, vx, vy reflect.Value, t reflect.Type) (out applicableOption) { + for _, opt := range opts { + switch opt := opt.filter(s, vx, vy, t); opt.(type) { + case ignore: + return ignore{} // Only ignore can short-circuit evaluation + case invalid: + out = invalid{} // Takes precedence over comparer or transformer + case *comparer, *transformer, Options: + switch out.(type) { + case nil: + out = opt + case invalid: + // Keep invalid + case *comparer, *transformer, Options: + out = Options{out, opt} // Conflicting comparers or transformers + } + } + } + return out +} + +func (opts Options) apply(s *state, _, _ reflect.Value) { + const warning = "ambiguous set of applicable options" + const help = "consider using filters to ensure at most one Comparer or Transformer may apply" + var ss []string + for _, opt := range flattenOptions(nil, opts) { + ss = append(ss, fmt.Sprint(opt)) + } + set := strings.Join(ss, "\n\t") + panic(fmt.Sprintf("%s at %#v:\n\t%s\n%s", warning, s.curPath, set, help)) +} + +func (opts Options) String() string { + var ss []string + for _, opt := range opts { + ss = append(ss, fmt.Sprint(opt)) + } + return fmt.Sprintf("Options{%s}", strings.Join(ss, ", ")) +} + +// FilterPath returns a new Option where opt is only evaluated if filter f +// returns true for the current Path in the value tree. +// +// The option passed in may be an Ignore, Transformer, Comparer, Options, or +// a previously filtered Option. +func FilterPath(f func(Path) bool, opt Option) Option { + if f == nil { + panic("invalid path filter function") + } + if opt := normalizeOption(opt); opt != nil { + return &pathFilter{fnc: f, opt: opt} + } + return nil +} + +type pathFilter struct { + core + fnc func(Path) bool + opt Option +} + +func (f pathFilter) filter(s *state, vx, vy reflect.Value, t reflect.Type) applicableOption { + if f.fnc(s.curPath) { + return f.opt.filter(s, vx, vy, t) + } + return nil +} + +func (f pathFilter) String() string { + fn := getFuncName(reflect.ValueOf(f.fnc).Pointer()) + return fmt.Sprintf("FilterPath(%s, %v)", fn, f.opt) +} + +// FilterValues returns a new Option where opt is only evaluated if filter f, +// which is a function of the form "func(T, T) bool", returns true for the +// current pair of values being compared. If the type of the values is not +// assignable to T, then this filter implicitly returns false. +// +// The filter function must be +// symmetric (i.e., agnostic to the order of the inputs) and +// deterministic (i.e., produces the same result when given the same inputs). +// If T is an interface, it is possible that f is called with two values with +// different concrete types that both implement T. +// +// The option passed in may be an Ignore, Transformer, Comparer, Options, or +// a previously filtered Option. +func FilterValues(f interface{}, opt Option) Option { + v := reflect.ValueOf(f) + if !function.IsType(v.Type(), function.ValueFilter) || v.IsNil() { + panic(fmt.Sprintf("invalid values filter function: %T", f)) + } + if opt := normalizeOption(opt); opt != nil { + vf := &valuesFilter{fnc: v, opt: opt} + if ti := v.Type().In(0); ti.Kind() != reflect.Interface || ti.NumMethod() > 0 { + vf.typ = ti + } + return vf + } + return nil +} + +type valuesFilter struct { + core + typ reflect.Type // T + fnc reflect.Value // func(T, T) bool + opt Option +} + +func (f valuesFilter) filter(s *state, vx, vy reflect.Value, t reflect.Type) applicableOption { + if !vx.IsValid() || !vy.IsValid() { + return invalid{} + } + if (f.typ == nil || t.AssignableTo(f.typ)) && s.callTTBFunc(f.fnc, vx, vy) { + return f.opt.filter(s, vx, vy, t) + } + return nil +} + +func (f valuesFilter) String() string { + fn := getFuncName(f.fnc.Pointer()) + return fmt.Sprintf("FilterValues(%s, %v)", fn, f.opt) +} + +// Ignore is an Option that causes all comparisons to be ignored. +// This value is intended to be combined with FilterPath or FilterValues. +// It is an error to pass an unfiltered Ignore option to Equal. +func Ignore() Option { return ignore{} } + +type ignore struct{ core } + +func (ignore) isFiltered() bool { return false } +func (ignore) filter(_ *state, _, _ reflect.Value, _ reflect.Type) applicableOption { return ignore{} } +func (ignore) apply(_ *state, _, _ reflect.Value) { return } +func (ignore) String() string { return "Ignore()" } + +// invalid is a sentinel Option type to indicate that some options could not +// be evaluated due to unexported fields. +type invalid struct{ core } + +func (invalid) filter(_ *state, _, _ reflect.Value, _ reflect.Type) applicableOption { return invalid{} } +func (invalid) apply(s *state, _, _ reflect.Value) { + const help = "consider using AllowUnexported or cmpopts.IgnoreUnexported" + panic(fmt.Sprintf("cannot handle unexported field: %#v\n%s", s.curPath, help)) +} + +// Transformer returns an Option that applies a transformation function that +// converts values of a certain type into that of another. +// +// The transformer f must be a function "func(T) R" that converts values of +// type T to those of type R and is implicitly filtered to input values +// assignable to T. The transformer must not mutate T in any way. +// +// To help prevent some cases of infinite recursive cycles applying the +// same transform to the output of itself (e.g., in the case where the +// input and output types are the same), an implicit filter is added such that +// a transformer is applicable only if that exact transformer is not already +// in the tail of the Path since the last non-Transform step. +// +// The name is a user provided label that is used as the Transform.Name in the +// transformation PathStep. If empty, an arbitrary name is used. +func Transformer(name string, f interface{}) Option { + v := reflect.ValueOf(f) + if !function.IsType(v.Type(), function.Transformer) || v.IsNil() { + panic(fmt.Sprintf("invalid transformer function: %T", f)) + } + if name == "" { + name = "ฮป" // Lambda-symbol as place-holder for anonymous transformer + } + if !isValid(name) { + panic(fmt.Sprintf("invalid name: %q", name)) + } + tr := &transformer{name: name, fnc: reflect.ValueOf(f)} + if ti := v.Type().In(0); ti.Kind() != reflect.Interface || ti.NumMethod() > 0 { + tr.typ = ti + } + return tr +} + +type transformer struct { + core + name string + typ reflect.Type // T + fnc reflect.Value // func(T) R +} + +func (tr *transformer) isFiltered() bool { return tr.typ != nil } + +func (tr *transformer) filter(s *state, _, _ reflect.Value, t reflect.Type) applicableOption { + for i := len(s.curPath) - 1; i >= 0; i-- { + if t, ok := s.curPath[i].(*transform); !ok { + break // Hit most recent non-Transform step + } else if tr == t.trans { + return nil // Cannot directly use same Transform + } + } + if tr.typ == nil || t.AssignableTo(tr.typ) { + return tr + } + return nil +} + +func (tr *transformer) apply(s *state, vx, vy reflect.Value) { + // Update path before calling the Transformer so that dynamic checks + // will use the updated path. + s.curPath.push(&transform{pathStep{tr.fnc.Type().Out(0)}, tr}) + defer s.curPath.pop() + + vx = s.callTRFunc(tr.fnc, vx) + vy = s.callTRFunc(tr.fnc, vy) + s.compareAny(vx, vy) +} + +func (tr transformer) String() string { + return fmt.Sprintf("Transformer(%s, %s)", tr.name, getFuncName(tr.fnc.Pointer())) +} + +// Comparer returns an Option that determines whether two values are equal +// to each other. +// +// The comparer f must be a function "func(T, T) bool" and is implicitly +// filtered to input values assignable to T. If T is an interface, it is +// possible that f is called with two values of different concrete types that +// both implement T. +// +// The equality function must be: +// โ€ข Symmetric: equal(x, y) == equal(y, x) +// โ€ข Deterministic: equal(x, y) == equal(x, y) +// โ€ข Pure: equal(x, y) does not modify x or y +func Comparer(f interface{}) Option { + v := reflect.ValueOf(f) + if !function.IsType(v.Type(), function.Equal) || v.IsNil() { + panic(fmt.Sprintf("invalid comparer function: %T", f)) + } + cm := &comparer{fnc: v} + if ti := v.Type().In(0); ti.Kind() != reflect.Interface || ti.NumMethod() > 0 { + cm.typ = ti + } + return cm +} + +type comparer struct { + core + typ reflect.Type // T + fnc reflect.Value // func(T, T) bool +} + +func (cm *comparer) isFiltered() bool { return cm.typ != nil } + +func (cm *comparer) filter(_ *state, _, _ reflect.Value, t reflect.Type) applicableOption { + if cm.typ == nil || t.AssignableTo(cm.typ) { + return cm + } + return nil +} + +func (cm *comparer) apply(s *state, vx, vy reflect.Value) { + eq := s.callTTBFunc(cm.fnc, vx, vy) + s.report(eq, vx, vy) +} + +func (cm comparer) String() string { + return fmt.Sprintf("Comparer(%s)", getFuncName(cm.fnc.Pointer())) +} + +// AllowUnexported returns an Option that forcibly allows operations on +// unexported fields in certain structs, which are specified by passing in a +// value of each struct type. +// +// Users of this option must understand that comparing on unexported fields +// from external packages is not safe since changes in the internal +// implementation of some external package may cause the result of Equal +// to unexpectedly change. However, it may be valid to use this option on types +// defined in an internal package where the semantic meaning of an unexported +// field is in the control of the user. +// +// For some cases, a custom Comparer should be used instead that defines +// equality as a function of the public API of a type rather than the underlying +// unexported implementation. +// +// For example, the reflect.Type documentation defines equality to be determined +// by the == operator on the interface (essentially performing a shallow pointer +// comparison) and most attempts to compare *regexp.Regexp types are interested +// in only checking that the regular expression strings are equal. +// Both of these are accomplished using Comparers: +// +// Comparer(func(x, y reflect.Type) bool { return x == y }) +// Comparer(func(x, y *regexp.Regexp) bool { return x.String() == y.String() }) +// +// In other cases, the cmpopts.IgnoreUnexported option can be used to ignore +// all unexported fields on specified struct types. +func AllowUnexported(types ...interface{}) Option { + if !supportAllowUnexported { + panic("AllowUnexported is not supported on purego builds, Google App Engine Standard, or GopherJS") + } + m := make(map[reflect.Type]bool) + for _, typ := range types { + t := reflect.TypeOf(typ) + if t.Kind() != reflect.Struct { + panic(fmt.Sprintf("invalid struct type: %T", typ)) + } + m[t] = true + } + return visibleStructs(m) +} + +type visibleStructs map[reflect.Type]bool + +func (visibleStructs) filter(_ *state, _, _ reflect.Value, _ reflect.Type) applicableOption { + panic("not implemented") +} + +// reporter is an Option that configures how differences are reported. +type reporter interface { + // TODO: Not exported yet. + // + // Perhaps add PushStep and PopStep and change Report to only accept + // a PathStep instead of the full-path? Adding a PushStep and PopStep makes + // it clear that we are traversing the value tree in a depth-first-search + // manner, which has an effect on how values are printed. + + Option + + // Report is called for every comparison made and will be provided with + // the two values being compared, the equality result, and the + // current path in the value tree. It is possible for x or y to be an + // invalid reflect.Value if one of the values is non-existent; + // which is possible with maps and slices. + Report(x, y reflect.Value, eq bool, p Path) +} + +// normalizeOption normalizes the input options such that all Options groups +// are flattened and groups with a single element are reduced to that element. +// Only coreOptions and Options containing coreOptions are allowed. +func normalizeOption(src Option) Option { + switch opts := flattenOptions(nil, Options{src}); len(opts) { + case 0: + return nil + case 1: + return opts[0] + default: + return opts + } +} + +// flattenOptions copies all options in src to dst as a flat list. +// Only coreOptions and Options containing coreOptions are allowed. +func flattenOptions(dst, src Options) Options { + for _, opt := range src { + switch opt := opt.(type) { + case nil: + continue + case Options: + dst = flattenOptions(dst, opt) + case coreOption: + dst = append(dst, opt) + default: + panic(fmt.Sprintf("invalid option type: %T", opt)) + } + } + return dst +} + +// getFuncName returns a short function name from the pointer. +// The string parsing logic works up until Go1.9. +func getFuncName(p uintptr) string { + fnc := runtime.FuncForPC(p) + if fnc == nil { + return "" + } + name := fnc.Name() // E.g., "long/path/name/mypkg.(mytype).(long/path/name/mypkg.myfunc)-fm" + if strings.HasSuffix(name, ")-fm") || strings.HasSuffix(name, ")ยทfm") { + // Strip the package name from method name. + name = strings.TrimSuffix(name, ")-fm") + name = strings.TrimSuffix(name, ")ยทfm") + if i := strings.LastIndexByte(name, '('); i >= 0 { + methodName := name[i+1:] // E.g., "long/path/name/mypkg.myfunc" + if j := strings.LastIndexByte(methodName, '.'); j >= 0 { + methodName = methodName[j+1:] // E.g., "myfunc" + } + name = name[:i] + methodName // E.g., "long/path/name/mypkg.(mytype)." + "myfunc" + } + } + if i := strings.LastIndexByte(name, '/'); i >= 0 { + // Strip the package name. + name = name[i+1:] // E.g., "mypkg.(mytype).myfunc" + } + return name +} diff --git a/vendor/github.com/google/go-cmp/cmp/path.go b/vendor/github.com/google/go-cmp/cmp/path.go new file mode 100644 index 000000000..c08a3cf80 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/path.go @@ -0,0 +1,309 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +package cmp + +import ( + "fmt" + "reflect" + "strings" + "unicode" + "unicode/utf8" +) + +type ( + // Path is a list of PathSteps describing the sequence of operations to get + // from some root type to the current position in the value tree. + // The first Path element is always an operation-less PathStep that exists + // simply to identify the initial type. + // + // When traversing structs with embedded structs, the embedded struct will + // always be accessed as a field before traversing the fields of the + // embedded struct themselves. That is, an exported field from the + // embedded struct will never be accessed directly from the parent struct. + Path []PathStep + + // PathStep is a union-type for specific operations to traverse + // a value's tree structure. Users of this package never need to implement + // these types as values of this type will be returned by this package. + PathStep interface { + String() string + Type() reflect.Type // Resulting type after performing the path step + isPathStep() + } + + // SliceIndex is an index operation on a slice or array at some index Key. + SliceIndex interface { + PathStep + Key() int // May return -1 if in a split state + + // SplitKeys returns the indexes for indexing into slices in the + // x and y values, respectively. These indexes may differ due to the + // insertion or removal of an element in one of the slices, causing + // all of the indexes to be shifted. If an index is -1, then that + // indicates that the element does not exist in the associated slice. + // + // Key is guaranteed to return -1 if and only if the indexes returned + // by SplitKeys are not the same. SplitKeys will never return -1 for + // both indexes. + SplitKeys() (x int, y int) + + isSliceIndex() + } + // MapIndex is an index operation on a map at some index Key. + MapIndex interface { + PathStep + Key() reflect.Value + isMapIndex() + } + // TypeAssertion represents a type assertion on an interface. + TypeAssertion interface { + PathStep + isTypeAssertion() + } + // StructField represents a struct field access on a field called Name. + StructField interface { + PathStep + Name() string + Index() int + isStructField() + } + // Indirect represents pointer indirection on the parent type. + Indirect interface { + PathStep + isIndirect() + } + // Transform is a transformation from the parent type to the current type. + Transform interface { + PathStep + Name() string + Func() reflect.Value + + // Option returns the originally constructed Transformer option. + // The == operator can be used to detect the exact option used. + Option() Option + + isTransform() + } +) + +func (pa *Path) push(s PathStep) { + *pa = append(*pa, s) +} + +func (pa *Path) pop() { + *pa = (*pa)[:len(*pa)-1] +} + +// Last returns the last PathStep in the Path. +// If the path is empty, this returns a non-nil PathStep that reports a nil Type. +func (pa Path) Last() PathStep { + return pa.Index(-1) +} + +// Index returns the ith step in the Path and supports negative indexing. +// A negative index starts counting from the tail of the Path such that -1 +// refers to the last step, -2 refers to the second-to-last step, and so on. +// If index is invalid, this returns a non-nil PathStep that reports a nil Type. +func (pa Path) Index(i int) PathStep { + if i < 0 { + i = len(pa) + i + } + if i < 0 || i >= len(pa) { + return pathStep{} + } + return pa[i] +} + +// String returns the simplified path to a node. +// The simplified path only contains struct field accesses. +// +// For example: +// MyMap.MySlices.MyField +func (pa Path) String() string { + var ss []string + for _, s := range pa { + if _, ok := s.(*structField); ok { + ss = append(ss, s.String()) + } + } + return strings.TrimPrefix(strings.Join(ss, ""), ".") +} + +// GoString returns the path to a specific node using Go syntax. +// +// For example: +// (*root.MyMap["key"].(*mypkg.MyStruct).MySlices)[2][3].MyField +func (pa Path) GoString() string { + var ssPre, ssPost []string + var numIndirect int + for i, s := range pa { + var nextStep PathStep + if i+1 < len(pa) { + nextStep = pa[i+1] + } + switch s := s.(type) { + case *indirect: + numIndirect++ + pPre, pPost := "(", ")" + switch nextStep.(type) { + case *indirect: + continue // Next step is indirection, so let them batch up + case *structField: + numIndirect-- // Automatic indirection on struct fields + case nil: + pPre, pPost = "", "" // Last step; no need for parenthesis + } + if numIndirect > 0 { + ssPre = append(ssPre, pPre+strings.Repeat("*", numIndirect)) + ssPost = append(ssPost, pPost) + } + numIndirect = 0 + continue + case *transform: + ssPre = append(ssPre, s.trans.name+"(") + ssPost = append(ssPost, ")") + continue + case *typeAssertion: + // As a special-case, elide type assertions on anonymous types + // since they are typically generated dynamically and can be very + // verbose. For example, some transforms return interface{} because + // of Go's lack of generics, but typically take in and return the + // exact same concrete type. + if s.Type().PkgPath() == "" { + continue + } + } + ssPost = append(ssPost, s.String()) + } + for i, j := 0, len(ssPre)-1; i < j; i, j = i+1, j-1 { + ssPre[i], ssPre[j] = ssPre[j], ssPre[i] + } + return strings.Join(ssPre, "") + strings.Join(ssPost, "") +} + +type ( + pathStep struct { + typ reflect.Type + } + + sliceIndex struct { + pathStep + xkey, ykey int + } + mapIndex struct { + pathStep + key reflect.Value + } + typeAssertion struct { + pathStep + } + structField struct { + pathStep + name string + idx int + + // These fields are used for forcibly accessing an unexported field. + // pvx, pvy, and field are only valid if unexported is true. + unexported bool + force bool // Forcibly allow visibility + pvx, pvy reflect.Value // Parent values + field reflect.StructField // Field information + } + indirect struct { + pathStep + } + transform struct { + pathStep + trans *transformer + } +) + +func (ps pathStep) Type() reflect.Type { return ps.typ } +func (ps pathStep) String() string { + if ps.typ == nil { + return "" + } + s := ps.typ.String() + if s == "" || strings.ContainsAny(s, "{}\n") { + return "root" // Type too simple or complex to print + } + return fmt.Sprintf("{%s}", s) +} + +func (si sliceIndex) String() string { + switch { + case si.xkey == si.ykey: + return fmt.Sprintf("[%d]", si.xkey) + case si.ykey == -1: + // [5->?] means "I don't know where X[5] went" + return fmt.Sprintf("[%d->?]", si.xkey) + case si.xkey == -1: + // [?->3] means "I don't know where Y[3] came from" + return fmt.Sprintf("[?->%d]", si.ykey) + default: + // [5->3] means "X[5] moved to Y[3]" + return fmt.Sprintf("[%d->%d]", si.xkey, si.ykey) + } +} +func (mi mapIndex) String() string { return fmt.Sprintf("[%#v]", mi.key) } +func (ta typeAssertion) String() string { return fmt.Sprintf(".(%v)", ta.typ) } +func (sf structField) String() string { return fmt.Sprintf(".%s", sf.name) } +func (in indirect) String() string { return "*" } +func (tf transform) String() string { return fmt.Sprintf("%s()", tf.trans.name) } + +func (si sliceIndex) Key() int { + if si.xkey != si.ykey { + return -1 + } + return si.xkey +} +func (si sliceIndex) SplitKeys() (x, y int) { return si.xkey, si.ykey } +func (mi mapIndex) Key() reflect.Value { return mi.key } +func (sf structField) Name() string { return sf.name } +func (sf structField) Index() int { return sf.idx } +func (tf transform) Name() string { return tf.trans.name } +func (tf transform) Func() reflect.Value { return tf.trans.fnc } +func (tf transform) Option() Option { return tf.trans } + +func (pathStep) isPathStep() {} +func (sliceIndex) isSliceIndex() {} +func (mapIndex) isMapIndex() {} +func (typeAssertion) isTypeAssertion() {} +func (structField) isStructField() {} +func (indirect) isIndirect() {} +func (transform) isTransform() {} + +var ( + _ SliceIndex = sliceIndex{} + _ MapIndex = mapIndex{} + _ TypeAssertion = typeAssertion{} + _ StructField = structField{} + _ Indirect = indirect{} + _ Transform = transform{} + + _ PathStep = sliceIndex{} + _ PathStep = mapIndex{} + _ PathStep = typeAssertion{} + _ PathStep = structField{} + _ PathStep = indirect{} + _ PathStep = transform{} +) + +// isExported reports whether the identifier is exported. +func isExported(id string) bool { + r, _ := utf8.DecodeRuneInString(id) + return unicode.IsUpper(r) +} + +// isValid reports whether the identifier is valid. +// Empty and underscore-only strings are not valid. +func isValid(id string) bool { + ok := id != "" && id != "_" + for j, c := range id { + ok = ok && (j > 0 || !unicode.IsDigit(c)) + ok = ok && (c == '_' || unicode.IsLetter(c) || unicode.IsDigit(c)) + } + return ok +} diff --git a/vendor/github.com/google/go-cmp/cmp/reporter.go b/vendor/github.com/google/go-cmp/cmp/reporter.go new file mode 100644 index 000000000..20e9f18e0 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/reporter.go @@ -0,0 +1,53 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +package cmp + +import ( + "fmt" + "reflect" + "strings" + + "github.com/google/go-cmp/cmp/internal/value" +) + +type defaultReporter struct { + Option + diffs []string // List of differences, possibly truncated + ndiffs int // Total number of differences + nbytes int // Number of bytes in diffs + nlines int // Number of lines in diffs +} + +var _ reporter = (*defaultReporter)(nil) + +func (r *defaultReporter) Report(x, y reflect.Value, eq bool, p Path) { + if eq { + return // Ignore equal results + } + const maxBytes = 4096 + const maxLines = 256 + r.ndiffs++ + if r.nbytes < maxBytes && r.nlines < maxLines { + sx := value.Format(x, value.FormatConfig{UseStringer: true}) + sy := value.Format(y, value.FormatConfig{UseStringer: true}) + if sx == sy { + // Unhelpful output, so use more exact formatting. + sx = value.Format(x, value.FormatConfig{PrintPrimitiveType: true}) + sy = value.Format(y, value.FormatConfig{PrintPrimitiveType: true}) + } + s := fmt.Sprintf("%#v:\n\t-: %s\n\t+: %s\n", p, sx, sy) + r.diffs = append(r.diffs, s) + r.nbytes += len(s) + r.nlines += strings.Count(s, "\n") + } +} + +func (r *defaultReporter) String() string { + s := strings.Join(r.diffs, "") + if r.ndiffs == len(r.diffs) { + return s + } + return fmt.Sprintf("%s... %d more differences ...", s, r.ndiffs-len(r.diffs)) +} diff --git a/vendor/github.com/google/go-cmp/cmp/unsafe_panic.go b/vendor/github.com/google/go-cmp/cmp/unsafe_panic.go new file mode 100644 index 000000000..d1518eb3a --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/unsafe_panic.go @@ -0,0 +1,15 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// +build purego appengine js + +package cmp + +import "reflect" + +const supportAllowUnexported = false + +func unsafeRetrieveField(reflect.Value, reflect.StructField) reflect.Value { + panic("unsafeRetrieveField is not implemented") +} diff --git a/vendor/github.com/google/go-cmp/cmp/unsafe_reflect.go b/vendor/github.com/google/go-cmp/cmp/unsafe_reflect.go new file mode 100644 index 000000000..579b65507 --- /dev/null +++ b/vendor/github.com/google/go-cmp/cmp/unsafe_reflect.go @@ -0,0 +1,23 @@ +// Copyright 2017, The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE.md file. + +// +build !purego,!appengine,!js + +package cmp + +import ( + "reflect" + "unsafe" +) + +const supportAllowUnexported = true + +// unsafeRetrieveField uses unsafe to forcibly retrieve any field from a struct +// such that the value has read-write permissions. +// +// The parent struct, v, must be addressable, while f must be a StructField +// describing the field to retrieve. +func unsafeRetrieveField(v reflect.Value, f reflect.StructField) reflect.Value { + return reflect.NewAt(f.Type, unsafe.Pointer(v.UnsafeAddr()+f.Offset)).Elem() +} From 5a0c9b2a13e505e4e46f8644ebd6f2b7aa083ab5 Mon Sep 17 00:00:00 2001 From: Jason Hall Date: Mon, 1 Oct 2018 14:11:26 -0400 Subject: [PATCH 34/35] Update go-containerregistry dep and remove unnecessary Options --- Gopkg.lock | 4 ++-- pkg/executor/build.go | 2 +- pkg/executor/push.go | 4 ++-- .../pkg/v1/daemon/write.go | 10 ++-------- .../pkg/v1/mutate/rebase.go | 6 +----- .../pkg/v1/remote/delete.go | 9 +-------- .../pkg/v1/remote/write.go | 12 +---------- .../pkg/v1/tarball/write.go | 20 +++++++------------ 8 files changed, 17 insertions(+), 50 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 176d4132a..219622492 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -431,7 +431,7 @@ [[projects]] branch = "master" - digest = "1:8ea12d703d8f36fec3db5e0dd85c9d3cc0b8b05ea8f9d090dcd41129cd392fd2" + digest = "1:edf64d541c12aaf4f279642ea9939f035dcc9fc2edf649aba295e9cbca2c28d4" name = "github.com/google/go-containerregistry" packages = [ "pkg/authn", @@ -450,7 +450,7 @@ "pkg/v1/v1util", ] pruneopts = "NUT" - revision = "6cfedf31db7d9e06c5beba68e9e1987a44fd844f" + revision = "03167950e20ac82689f50828811e69cdd9e02af2" [[projects]] branch = "master" diff --git a/pkg/executor/build.go b/pkg/executor/build.go index 5e10d16b5..080d24b8b 100644 --- a/pkg/executor/build.go +++ b/pkg/executor/build.go @@ -307,7 +307,7 @@ func saveStageAsTarball(stageIndex int, image v1.Image) error { } tarPath := filepath.Join(constants.KanikoIntermediateStagesDir, strconv.Itoa(stageIndex)) logrus.Infof("Storing source image from stage %d at path %s", stageIndex, tarPath) - return tarball.WriteToFile(tarPath, destRef, image, nil) + return tarball.WriteToFile(tarPath, destRef, image) } func getHasher(snapshotMode string) (func(string) (string, error), error) { diff --git a/pkg/executor/push.go b/pkg/executor/push.go index aa68c723c..6cfd39e9e 100644 --- a/pkg/executor/push.go +++ b/pkg/executor/push.go @@ -66,7 +66,7 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error { for _, destRef := range destRefs { tagToImage[destRef] = image } - return tarball.MultiWriteToFile(opts.TarPath, tagToImage, nil) + return tarball.MultiWriteToFile(opts.TarPath, tagToImage) } // continue pushing unless an error occurs @@ -98,7 +98,7 @@ func DoPush(image v1.Image, opts *config.KanikoOptions) error { } rt := &withUserAgent{t: tr} - if err := remote.Write(destRef, image, pushAuth, rt, remote.WriteOptions{}); err != nil { + if err := remote.Write(destRef, image, pushAuth, rt); err != nil { return errors.Wrap(err, fmt.Sprintf("failed to push to destination %s", destRef)) } } diff --git a/vendor/github.com/google/go-containerregistry/pkg/v1/daemon/write.go b/vendor/github.com/google/go-containerregistry/pkg/v1/daemon/write.go index df61a7de4..7310b7057 100644 --- a/vendor/github.com/google/go-containerregistry/pkg/v1/daemon/write.go +++ b/vendor/github.com/google/go-containerregistry/pkg/v1/daemon/write.go @@ -44,14 +44,8 @@ var GetImageLoader = func() (ImageLoader, error) { return cli, nil } -// WriteOptions are used to expose optional information to guide or -// control the image write. -type WriteOptions struct { - // TODO(dlorenc): What kinds of knobs does the daemon expose? -} - // Write saves the image into the daemon as the given tag. -func Write(tag name.Tag, img v1.Image, wo WriteOptions) (string, error) { +func Write(tag name.Tag, img v1.Image) (string, error) { cli, err := GetImageLoader() if err != nil { return "", err @@ -59,7 +53,7 @@ func Write(tag name.Tag, img v1.Image, wo WriteOptions) (string, error) { pr, pw := io.Pipe() go func() { - pw.CloseWithError(tarball.Write(tag, img, &tarball.WriteOptions{}, pw)) + pw.CloseWithError(tarball.Write(tag, img, pw)) }() // write the image in docker save format first, then load it diff --git a/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/rebase.go b/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/rebase.go index 9a4d4656a..d6c8a7040 100644 --- a/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/rebase.go +++ b/vendor/github.com/google/go-containerregistry/pkg/v1/mutate/rebase.go @@ -21,11 +21,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/empty" ) -type RebaseOptions struct { - // TODO(jasonhall): Rebase seam hint. -} - -func Rebase(orig, oldBase, newBase v1.Image, opts *RebaseOptions) (v1.Image, error) { +func Rebase(orig, oldBase, newBase v1.Image) (v1.Image, error) { // Verify that oldBase's layers are present in orig, otherwise orig is // not based on oldBase at all. origLayers, err := orig.Layers() diff --git a/vendor/github.com/google/go-containerregistry/pkg/v1/remote/delete.go b/vendor/github.com/google/go-containerregistry/pkg/v1/remote/delete.go index 5108a05de..2032e276e 100644 --- a/vendor/github.com/google/go-containerregistry/pkg/v1/remote/delete.go +++ b/vendor/github.com/google/go-containerregistry/pkg/v1/remote/delete.go @@ -25,15 +25,8 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote/transport" ) -// DeleteOptions are used to expose optional information to guide or -// control the image deletion. -type DeleteOptions struct { - // TODO(mattmoor): Fail on not found? - // TODO(mattmoor): Delete tag and manifest? -} - // Delete removes the specified image reference from the remote registry. -func Delete(ref name.Reference, auth authn.Authenticator, t http.RoundTripper, do DeleteOptions) error { +func Delete(ref name.Reference, auth authn.Authenticator, t http.RoundTripper) error { scopes := []string{ref.Scope(transport.DeleteScope)} tr, err := transport.New(ref.Context().Registry, auth, t, scopes) if err != nil { diff --git a/vendor/github.com/google/go-containerregistry/pkg/v1/remote/write.go b/vendor/github.com/google/go-containerregistry/pkg/v1/remote/write.go index af61e361b..1fd633c0d 100644 --- a/vendor/github.com/google/go-containerregistry/pkg/v1/remote/write.go +++ b/vendor/github.com/google/go-containerregistry/pkg/v1/remote/write.go @@ -28,16 +28,8 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote/transport" ) -// WriteOptions are used to expose optional information to guide or -// control the image write. -type WriteOptions struct { - // TODO(mattmoor): Expose "threads" to limit parallelism? -} - // Write pushes the provided img to the specified image reference. -func Write(ref name.Reference, img v1.Image, auth authn.Authenticator, t http.RoundTripper, - wo WriteOptions) error { - +func Write(ref name.Reference, img v1.Image, auth authn.Authenticator, t http.RoundTripper) error { ls, err := img.Layers() if err != nil { return err @@ -52,7 +44,6 @@ func Write(ref name.Reference, img v1.Image, auth authn.Authenticator, t http.Ro ref: ref, client: &http.Client{Transport: tr}, img: img, - options: wo, } bs, err := img.BlobSet() @@ -92,7 +83,6 @@ type writer struct { ref name.Reference client *http.Client img v1.Image - options WriteOptions } // url returns a url.Url for the specified path in the context of this remote image reference. diff --git a/vendor/github.com/google/go-containerregistry/pkg/v1/tarball/write.go b/vendor/github.com/google/go-containerregistry/pkg/v1/tarball/write.go index aa4313f16..b0d45061e 100644 --- a/vendor/github.com/google/go-containerregistry/pkg/v1/tarball/write.go +++ b/vendor/github.com/google/go-containerregistry/pkg/v1/tarball/write.go @@ -26,39 +26,33 @@ import ( "github.com/google/go-containerregistry/pkg/v1" ) -// WriteOptions are used to expose optional information to guide or -// control the image write. -type WriteOptions struct { - // TODO(mattmoor): Whether to store things compressed? -} - // WriteToFile writes in the compressed format to a tarball, on disk. // This is just syntactic sugar wrapping tarball.Write with a new file. -func WriteToFile(p string, tag name.Tag, img v1.Image, wo *WriteOptions) error { +func WriteToFile(p string, tag name.Tag, img v1.Image) error { w, err := os.Create(p) if err != nil { return err } defer w.Close() - return Write(tag, img, wo, w) + return Write(tag, img, w) } // MultiWriteToFile writes in the compressed format to a tarball, on disk. // This is just syntactic sugar wrapping tarball.MultiWrite with a new file. -func MultiWriteToFile(p string, tagToImage map[name.Tag]v1.Image, wo *WriteOptions) error { +func MultiWriteToFile(p string, tagToImage map[name.Tag]v1.Image) error { w, err := os.Create(p) if err != nil { return err } defer w.Close() - return MultiWrite(tagToImage, wo, w) + return MultiWrite(tagToImage, w) } // Write is a wrapper to write a single image and tag to a tarball. -func Write(tag name.Tag, img v1.Image, wo *WriteOptions, w io.Writer) error { - return MultiWrite(map[name.Tag]v1.Image{tag: img}, wo, w) +func Write(tag name.Tag, img v1.Image, w io.Writer) error { + return MultiWrite(map[name.Tag]v1.Image{tag: img}, w) } // MultiWrite writes the contents of each image to the provided reader, in the compressed format. @@ -66,7 +60,7 @@ func Write(tag name.Tag, img v1.Image, wo *WriteOptions, w io.Writer) error { // One manifest.json file at the top level containing information about several images. // One file for each layer, named after the layer's SHA. // One file for the config blob, named after its SHA. -func MultiWrite(tagToImage map[name.Tag]v1.Image, wo *WriteOptions, w io.Writer) error { +func MultiWrite(tagToImage map[name.Tag]v1.Image, w io.Writer) error { tf := tar.NewWriter(w) defer tf.Close() From 589197b4161001485f6708b80442e96aca33eeba Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 1 Oct 2018 14:56:46 -0700 Subject: [PATCH 35/35] Fix travis @ HEAD I merged a contributor's PR which modifed the HasFilepathPrefix function to take an additional argument, but the PR hadn't been rebased. One of the liting tests in Travis caught this bug. --- integration/integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/integration_test.go b/integration/integration_test.go index 67cb7ebbe..e6e4f8959 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -314,7 +314,7 @@ func filterDiff(f []fileDiff) []fileDiff { for _, diff := range f { isWhitelisted := false for _, p := range allowedDiffPaths { - if util.HasFilepathPrefix(diff.Name, p) { + if util.HasFilepathPrefix(diff.Name, p, false) { isWhitelisted = true break }