From cf4822c31cbfd0a79b9a0c4a59dbc250d258b4c1 Mon Sep 17 00:00:00 2001 From: Tejal Desai Date: Tue, 19 Oct 2021 12:25:42 -0700 Subject: [PATCH] Run docker_credentials_gcr in warmer (#1780) * run docker_credentials_gcr in warmer * fix tests * fix dockerfiles * fix boilerplate * mend * fix * another lint --- cmd/warmer/cmd/root.go | 17 +++++ deploy/Dockerfile | 1 - deploy/Dockerfile_debug | 2 - deploy/Dockerfile_warmer | 1 - hack/boilerplate/boilerplate.py | 2 +- pkg/executor/push.go | 41 +--------- pkg/executor/push_test.go | 122 +----------------------------- pkg/util/gcr_util.go | 69 +++++++++++++++++ pkg/util/gcr_util_test.go | 130 ++++++++++++++++++++++++++++++++ 9 files changed, 222 insertions(+), 163 deletions(-) create mode 100644 pkg/util/gcr_util.go create mode 100644 pkg/util/gcr_util_test.go diff --git a/cmd/warmer/cmd/root.go b/cmd/warmer/cmd/root.go index 0ccfdd1421..61333daeec 100644 --- a/cmd/warmer/cmd/root.go +++ b/cmd/warmer/cmd/root.go @@ -19,11 +19,13 @@ package cmd import ( "fmt" "os" + "strings" "time" "github.com/GoogleContainerTools/kaniko/pkg/cache" "github.com/GoogleContainerTools/kaniko/pkg/config" "github.com/GoogleContainerTools/kaniko/pkg/logging" + "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/pkg/errors" "github.com/spf13/cobra" ) @@ -63,9 +65,24 @@ var RootCmd = &cobra.Command{ exit(errors.Wrap(err, "Failed to create cache directory")) } } + isGCR := false + for _, image := range opts.Images { + if strings.Contains(image, "gcr.io") || strings.Contains(image, ".pkg.dev") { + isGCR = true + break + } + } + // Historically kaniko was pre-configured by default with gcr credential helper, + // in here we keep the backwards compatibility by enabling the GCR helper only + // when gcr.io (or pkg.dev) is in one of the destinations. + if isGCR { + util.ConfigureGCR("") + } + if err := cache.WarmCache(opts); err != nil { exit(errors.Wrap(err, "Failed warming cache")) } + }, } diff --git a/deploy/Dockerfile b/deploy/Dockerfile index fb605cd1bd..331b10622e 100644 --- a/deploy/Dockerfile +++ b/deploy/Dockerfile @@ -76,6 +76,5 @@ ENV SSL_CERT_DIR=/kaniko/ssl/certs ENV DOCKER_CONFIG /kaniko/.docker/ ENV DOCKER_CREDENTIAL_GCR_CONFIG /kaniko/.config/gcloud/docker_credential_gcr_config.json WORKDIR /workspace -RUN ["/kaniko/docker-credential-gcr", "config", "--token-source=env"] ENTRYPOINT ["/kaniko/executor"] \ No newline at end of file diff --git a/deploy/Dockerfile_debug b/deploy/Dockerfile_debug index d7ca55650c..25eb0cdcc3 100644 --- a/deploy/Dockerfile_debug +++ b/deploy/Dockerfile_debug @@ -71,7 +71,6 @@ 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/local/docker-credential-ecr-login /kaniko/docker-credential-ecr-login COPY --from=0 /go/src/github.com/chrismellard/docker-credential-acr-env/build/docker-credential-acr-env /kaniko/docker-credential-acr COPY --from=busybox:1.32.0 /bin /busybox - # Declare /busybox as a volume to get it automatically in the path to ignore VOLUME /busybox @@ -85,7 +84,6 @@ ENV SSL_CERT_DIR=/kaniko/ssl/certs ENV DOCKER_CONFIG /kaniko/.docker/ ENV DOCKER_CREDENTIAL_GCR_CONFIG /kaniko/.config/gcloud/docker_credential_gcr_config.json WORKDIR /workspace -RUN ["/kaniko/docker-credential-gcr", "config", "--token-source=env"] RUN ["/busybox/mkdir", "-p", "/bin"] RUN ["/busybox/ln", "-s", "/busybox/sh", "/bin/sh"] ENTRYPOINT ["/kaniko/executor"] diff --git a/deploy/Dockerfile_warmer b/deploy/Dockerfile_warmer index cf1e2b1934..42d4114863 100644 --- a/deploy/Dockerfile_warmer +++ b/deploy/Dockerfile_warmer @@ -76,5 +76,4 @@ ENV SSL_CERT_DIR=/kaniko/ssl/certs ENV DOCKER_CONFIG /kaniko/.docker/ ENV DOCKER_CREDENTIAL_GCR_CONFIG /kaniko/.config/gcloud/docker_credential_gcr_config.json WORKDIR /workspace -RUN ["/kaniko/docker-credential-gcr", "config", "--token-source=env"] ENTRYPOINT ["/kaniko/warmer"] diff --git a/hack/boilerplate/boilerplate.py b/hack/boilerplate/boilerplate.py index 7b7ea474aa..1769544ea5 100644 --- a/hack/boilerplate/boilerplate.py +++ b/hack/boilerplate/boilerplate.py @@ -147,7 +147,7 @@ def get_regexs(): # Search for "YEAR" which exists in the boilerplate, but shouldn't in the real thing regexs["year"] = re.compile( 'YEAR' ) # dates can be 2018, 2019, 2020 company holder names can be anything - regexs["date"] = re.compile( '(2018|2019|2020)' ) + regexs["date"] = re.compile( '(2018|2019|2020|2021)' ) # strip // go:build \n\n build constraints regexs["go_build_constraints_go"] = re.compile(r"^(//go\:build.*)+\n", re.MULTILINE) # strip // +build \n\n build constraints diff --git a/pkg/executor/push.go b/pkg/executor/push.go index a989db9204..d504025165 100644 --- a/pkg/executor/push.go +++ b/pkg/executor/push.go @@ -17,13 +17,11 @@ limitations under the License. package executor import ( - "bytes" "encoding/json" "fmt" "io/ioutil" "net/http" "os" - "os/exec" "path/filepath" "strings" "time" @@ -61,28 +59,6 @@ const ( UpstreamClientUaKey = "UPSTREAM_CLIENT_TYPE" ) -// DockerConfLocation returns the file system location of the Docker -// configuration file under the directory set in the DOCKER_CONFIG environment -// variable. If that variable is not set, it returns the OS-equivalent of -// "/kaniko/.docker/config.json". -func DockerConfLocation() string { - configFile := "config.json" - if dockerConfig := os.Getenv("DOCKER_CONFIG"); dockerConfig != "" { - file, err := os.Stat(dockerConfig) - if err == nil { - if file.IsDir() { - return filepath.Join(dockerConfig, configFile) - } - } else { - if os.IsNotExist(err) { - return string(os.PathSeparator) + filepath.Join("kaniko", ".docker", configFile) - } - } - return filepath.Clean(dockerConfig) - } - return string(os.PathSeparator) + filepath.Join("kaniko", ".docker", configFile) -} - func (w *withUserAgent) RoundTrip(r *http.Request) (*http.Response, error) { ua := []string{fmt.Sprintf("kaniko/%s", version.Version())} if upstream := os.Getenv(UpstreamClientUaKey); upstream != "" { @@ -95,7 +71,6 @@ func (w *withUserAgent) RoundTrip(r *http.Request) (*http.Response, error) { // for testing var ( fs = afero.NewOsFs() - execCommand = exec.Command checkRemotePushPermission = remote.CheckPushPermission ) @@ -110,8 +85,6 @@ func CheckPushPermissions(opts *config.KanikoOptions) error { } checked := map[string]bool{} - _, err := fs.Stat(DockerConfLocation()) - dockerConfNotExists := os.IsNotExist(err) for _, destination := range targets { destRef, err := name.NewTag(destination, name.WeakValidation) if err != nil { @@ -126,18 +99,8 @@ func CheckPushPermissions(opts *config.KanikoOptions) error { // in here we keep the backwards compatibility by enabling the GCR helper only // when gcr.io (or pkg.dev) is in one of the destinations. if registryName == "gcr.io" || strings.HasSuffix(registryName, ".gcr.io") || strings.HasSuffix(registryName, ".pkg.dev") { - // Checking for existence of docker.config as it's normally required for - // authenticated registries and prevent overwriting user provided docker conf - if dockerConfNotExists { - flags := fmt.Sprintf("--registries=%s", registryName) - cmd := execCommand("docker-credential-gcr", "configure-docker", flags) - var out bytes.Buffer - cmd.Stderr = &out - if err := cmd.Run(); err != nil { - return errors.Wrap(err, fmt.Sprintf("error while configuring docker-credential-gcr helper: %s : %s", cmd.String(), out.String())) - } - } else { - logrus.Warnf("\nSkip running docker-credential-gcr as user provided docker configuration exists at %s", DockerConfLocation()) + if err := util.ConfigureGCR(fmt.Sprintf("--registries=%s", registryName)); err != nil { + return err } } if opts.Insecure || opts.InsecureRegistries.Contains(registryName) { diff --git a/pkg/executor/push_test.go b/pkg/executor/push_test.go index 940c870a61..8b5f25269c 100644 --- a/pkg/executor/push_test.go +++ b/pkg/executor/push_test.go @@ -22,11 +22,11 @@ import ( "io/ioutil" "net/http" "os" - "os/exec" "path/filepath" "testing" "github.com/GoogleContainerTools/kaniko/pkg/config" + "github.com/GoogleContainerTools/kaniko/pkg/util" "github.com/GoogleContainerTools/kaniko/testutil" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" @@ -36,11 +36,6 @@ import ( "github.com/spf13/afero" ) -const ( - DefaultKanikoDockerConfigJSON = "/kaniko/.docker/config.json" - DockerConfigEnvKey = "DOCKER_CONFIG" -) - func mustTag(t *testing.T, s string) name.Tag { tag, err := name.NewTag(s, name.StrictValidation) if err != nil { @@ -49,107 +44,6 @@ func mustTag(t *testing.T, s string) name.Tag { return tag } -func TestDockerConfLocationWithFileLocation(t *testing.T) { - originalDockerConfig := os.Getenv(DockerConfigEnvKey) - if err := os.Unsetenv(DockerConfigEnvKey); err != nil { - t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) - } - file, err := ioutil.TempFile("", "docker.conf") - if err != nil { - t.Fatalf("could not create temp file: %s", err) - } - defer os.Remove(file.Name()) - if err := os.Setenv(DockerConfigEnvKey, file.Name()); err != nil { - t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) - } - unset := DockerConfLocation() - unsetExpected := file.Name() - if unset != unsetExpected { - t.Errorf("Unexpected default Docker configuration file location: expected:'%s' got:'%s'", unsetExpected, unset) - } - restoreOriginalDockerConfigEnv(t, originalDockerConfig) -} - -func TestDockerConfLocationWithInvalidFileLocation(t *testing.T) { - originalDockerConfig := os.Getenv(DockerConfigEnvKey) - if err := os.Unsetenv(DockerConfigEnvKey); err != nil { - t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) - } - tmpDir, err := ioutil.TempDir("", "*") - if err != nil { - t.Fatalf("could not create temp dir: %s", err) - } - defer os.RemoveAll(tmpDir) - random := "fdgdsfrdfgdf-fdfsf-24dsgfd" //replace with a really random string - file := filepath.Join(tmpDir, random) // an random file name, shouldn't exist - if err := os.Setenv(DockerConfigEnvKey, file); err != nil { - t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) - } - unset := DockerConfLocation() - // as file doesn't point to a real file, DockerConfLocation() should return the default kaniko path for config.json - unsetExpected := DefaultKanikoDockerConfigJSON - if unset != unsetExpected { - t.Errorf("Unexpected default Docker configuration file location: expected:'%s' got:'%s'", unsetExpected, unset) - } - restoreOriginalDockerConfigEnv(t, originalDockerConfig) -} - -func TestDockerConfLocation(t *testing.T) { - originalDockerConfig := os.Getenv(DockerConfigEnvKey) - - if err := os.Unsetenv(DockerConfigEnvKey); err != nil { - t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) - } - unset := DockerConfLocation() - unsetExpected := DefaultKanikoDockerConfigJSON // will fail on Windows - if unset != unsetExpected { - t.Errorf("Unexpected default Docker configuration file location: expected:'%s' got:'%s'", unsetExpected, unset) - } - tmpDir, err := ioutil.TempDir("", "*") - if err != nil { - t.Fatalf("could not create temp dir: %s", err) - } - defer os.RemoveAll(tmpDir) - - dir := filepath.Join(tmpDir, "/kaniko/.docker") - os.MkdirAll(dir, os.ModePerm) - if err := os.Setenv(DockerConfigEnvKey, dir); err != nil { - t.Fatalf("Failed to set DOCKER_CONFIG: %v", err) - } - kanikoDefault := DockerConfLocation() - kanikoDefaultExpected := filepath.Join(tmpDir, DefaultKanikoDockerConfigJSON) // will fail on Windows - if kanikoDefault != kanikoDefaultExpected { - t.Errorf("Unexpected kaniko default Docker conf file location: expected:'%s' got:'%s'", kanikoDefaultExpected, kanikoDefault) - } - - differentPath, err := ioutil.TempDir("", "differentPath") - if err != nil { - t.Fatalf("could not create temp dir: %s", err) - } - defer os.RemoveAll(differentPath) - if err := os.Setenv(DockerConfigEnvKey, differentPath); err != nil { - t.Fatalf("Failed to set DOCKER_CONFIG: %v", err) - } - set := DockerConfLocation() - setExpected := filepath.Join(differentPath, "config.json") // will fail on Windows ? - if set != setExpected { - t.Errorf("Unexpected DOCKER_CONF-based file location: expected:'%s' got:'%s'", setExpected, set) - } - restoreOriginalDockerConfigEnv(t, originalDockerConfig) -} - -func restoreOriginalDockerConfigEnv(t *testing.T, originalDockerConfig string) { - if originalDockerConfig != "" { - if err := os.Setenv(DockerConfigEnvKey, originalDockerConfig); err != nil { - t.Fatalf("Failed to set DOCKER_CONFIG back to original value '%s': %v", originalDockerConfig, err) - } - } else { - if err := os.Unsetenv(DockerConfigEnvKey); err != nil { - t.Fatalf("Failed to unset DOCKER_CONFIG after testing: %v", err) - } - } -} - func TestWriteImageOutputs(t *testing.T) { img, err := random.Image(1024, 3) if err != nil { @@ -370,15 +264,6 @@ func setCalledFalse() { calledCheckPushPermission = false } -func fakeExecCommand(command string, args ...string) *exec.Cmd { - calledExecCommand = append(calledExecCommand, true) - cs := []string{"-test.run=TestHelperProcess", "--", command} - cs = append(cs, args...) - cmd := exec.Command(os.Args[0], cs...) - cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"} - return cmd -} - func fakeCheckPushPermission(ref name.Reference, kc authn.Keychain, t http.RoundTripper) error { calledCheckPushPermission = true return nil @@ -421,7 +306,6 @@ func TestCheckPushPermissions(t *testing.T) { }, } - execCommand = fakeExecCommand checkRemotePushPermission = fakeCheckPushPermission for _, test := range tests { t.Run(test.description, func(t *testing.T) { @@ -431,8 +315,8 @@ func TestCheckPushPermissions(t *testing.T) { Destinations: test.Destination, } if test.ExistingConfig { - afero.WriteFile(fs, DockerConfLocation(), []byte(""), os.FileMode(0644)) - defer fs.Remove(DockerConfLocation()) + afero.WriteFile(fs, util.DockerConfLocation(), []byte(""), os.FileMode(0644)) + defer fs.Remove(util.DockerConfLocation()) } CheckPushPermissions(&opts) for i, shdCall := range test.ShouldCallExecCommand { diff --git a/pkg/util/gcr_util.go b/pkg/util/gcr_util.go new file mode 100644 index 0000000000..7627ac6de4 --- /dev/null +++ b/pkg/util/gcr_util.go @@ -0,0 +1,69 @@ +/* +Copyright 2021 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 util + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "path/filepath" + + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "github.com/spf13/afero" +) + +// DockerConfLocation returns the file system location of the Docker +// configuration file under the directory set in the DOCKER_CONFIG environment +// variable. If that variable is not set, it returns the OS-equivalent of +// "/kaniko/.docker/config.json". +func DockerConfLocation() string { + configFile := "config.json" + if dockerConfig := os.Getenv("DOCKER_CONFIG"); dockerConfig != "" { + file, err := os.Stat(dockerConfig) + if err == nil { + if file.IsDir() { + return filepath.Join(dockerConfig, configFile) + } + } else { + if os.IsNotExist(err) { + return string(os.PathSeparator) + filepath.Join("kaniko", ".docker", configFile) + } + } + return filepath.Clean(dockerConfig) + } + return string(os.PathSeparator) + filepath.Join("kaniko", ".docker", configFile) +} + +func ConfigureGCR(flags string) error { + // Checking for existence of docker.config as it's normally required for + // authenticated registries and prevent overwriting user provided docker conf + _, err := afero.NewOsFs().Stat(DockerConfLocation()) + dockerConfNotExists := os.IsNotExist(err) + if dockerConfNotExists { + cmd := exec.Command("docker-credential-gcr", "configure-docker", flags) + var out bytes.Buffer + cmd.Stderr = &out + if err := cmd.Run(); err != nil { + return errors.Wrap(err, fmt.Sprintf("error while configuring docker-credential-gcr helper: %s : %s", cmd.String(), out.String())) + } + } else { + logrus.Warnf("\nSkip running docker-credential-gcr as user provided docker configuration exists at %s", DockerConfLocation()) + } + return nil +} diff --git a/pkg/util/gcr_util_test.go b/pkg/util/gcr_util_test.go new file mode 100644 index 0000000000..96ebb67eaf --- /dev/null +++ b/pkg/util/gcr_util_test.go @@ -0,0 +1,130 @@ +/* +Copyright 2021 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 util + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" +) + +const ( + DefaultKanikoDockerConfigJSON = "/kaniko/.docker/config.json" + DockerConfigEnvKey = "DOCKER_CONFIG" +) + +func TestDockerConfLocationWithInvalidFileLocation(t *testing.T) { + originalDockerConfig := os.Getenv(DockerConfigEnvKey) + if err := os.Unsetenv(DockerConfigEnvKey); err != nil { + t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) + } + tmpDir, err := ioutil.TempDir("", "*") + if err != nil { + t.Fatalf("could not create temp dir: %s", err) + } + defer os.RemoveAll(tmpDir) + random := "fdgdsfrdfgdf-fdfsf-24dsgfd" //replace with a really random string + file := filepath.Join(tmpDir, random) // an random file name, shouldn't exist + if err := os.Setenv(DockerConfigEnvKey, file); err != nil { + t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) + } + unset := DockerConfLocation() + // as file doesn't point to a real file, DockerConfLocation() should return the default kaniko path for config.json + unsetExpected := DefaultKanikoDockerConfigJSON + if unset != unsetExpected { + t.Errorf("Unexpected default Docker configuration file location: expected:'%s' got:'%s'", unsetExpected, unset) + } + restoreOriginalDockerConfigEnv(t, originalDockerConfig) +} + +func TestDockerConfLocation(t *testing.T) { + originalDockerConfig := os.Getenv(DockerConfigEnvKey) + + if err := os.Unsetenv(DockerConfigEnvKey); err != nil { + t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) + } + unset := DockerConfLocation() + unsetExpected := DefaultKanikoDockerConfigJSON // will fail on Windows + if unset != unsetExpected { + t.Errorf("Unexpected default Docker configuration file location: expected:'%s' got:'%s'", unsetExpected, unset) + } + tmpDir, err := ioutil.TempDir("", "*") + if err != nil { + t.Fatalf("could not create temp dir: %s", err) + } + defer os.RemoveAll(tmpDir) + + dir := filepath.Join(tmpDir, "/kaniko/.docker") + os.MkdirAll(dir, os.ModePerm) + if err := os.Setenv(DockerConfigEnvKey, dir); err != nil { + t.Fatalf("Failed to set DOCKER_CONFIG: %v", err) + } + kanikoDefault := DockerConfLocation() + kanikoDefaultExpected := filepath.Join(tmpDir, DefaultKanikoDockerConfigJSON) // will fail on Windows + if kanikoDefault != kanikoDefaultExpected { + t.Errorf("Unexpected kaniko default Docker conf file location: expected:'%s' got:'%s'", kanikoDefaultExpected, kanikoDefault) + } + + differentPath, err := ioutil.TempDir("", "differentPath") + if err != nil { + t.Fatalf("could not create temp dir: %s", err) + } + defer os.RemoveAll(differentPath) + if err := os.Setenv(DockerConfigEnvKey, differentPath); err != nil { + t.Fatalf("Failed to set DOCKER_CONFIG: %v", err) + } + set := DockerConfLocation() + setExpected := filepath.Join(differentPath, "config.json") // will fail on Windows ? + if set != setExpected { + t.Errorf("Unexpected DOCKER_CONF-based file location: expected:'%s' got:'%s'", setExpected, set) + } + restoreOriginalDockerConfigEnv(t, originalDockerConfig) +} + +func restoreOriginalDockerConfigEnv(t *testing.T, originalDockerConfig string) { + if originalDockerConfig != "" { + if err := os.Setenv(DockerConfigEnvKey, originalDockerConfig); err != nil { + t.Fatalf("Failed to set DOCKER_CONFIG back to original value '%s': %v", originalDockerConfig, err) + } + } else { + if err := os.Unsetenv(DockerConfigEnvKey); err != nil { + t.Fatalf("Failed to unset DOCKER_CONFIG after testing: %v", err) + } + } +} + +func TestDockerConfLocationWithFileLocation(t *testing.T) { + originalDockerConfig := os.Getenv(DockerConfigEnvKey) + if err := os.Unsetenv(DockerConfigEnvKey); err != nil { + t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) + } + file, err := ioutil.TempFile("", "docker.conf") + if err != nil { + t.Fatalf("could not create temp file: %s", err) + } + defer os.Remove(file.Name()) + if err := os.Setenv(DockerConfigEnvKey, file.Name()); err != nil { + t.Fatalf("Failed to unset DOCKER_CONFIG: %v", err) + } + unset := DockerConfLocation() + unsetExpected := file.Name() + if unset != unsetExpected { + t.Errorf("Unexpected default Docker configuration file location: expected:'%s' got:'%s'", unsetExpected, unset) + } + restoreOriginalDockerConfigEnv(t, originalDockerConfig) +}