From 4655ffb8f68435f8da3deab3ec7d8b720337f1fc Mon Sep 17 00:00:00 2001 From: Sukhil Suresh Date: Mon, 27 Apr 2020 16:23:21 -0400 Subject: [PATCH 1/2] Resolve issue #356 Improve logging for the prepare step Before ``` $ logs -image tutorial-image -build 1 [prepare] prepare:fetch.go:88: Successfully cloned "https://github.com/spring-projects/spring-petclinic" @ "82cb521d636b282340378d80a6307a08e3d4a4c4" in path "/workspace" ``` After: ``` $ logs -image tutorial-image -build 5 [prepare] Loading secrets for "https://index.docker.io/v1/" from secret "tutorial-registry-credentials" [prepare] Successfully cloned "https://github.com/spring-projects/spring-petclinic" @ "400e3028f48a6c23f5156f6598dd10cb5e6a2849" in path "/workspace" ``` Signed-off-by: Sukhil Suresh --- cmd/build-init/main.go | 4 ++-- cmd/rebase/main.go | 8 +++++--- pkg/dockercreds/parse_annoted_secrets.go | 7 +++++-- pkg/dockercreds/parse_annoted_secrets_test.go | 10 +++++++++- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/cmd/build-init/main.go b/cmd/build-init/main.go index 8fba8b288..91b3799d6 100644 --- a/cmd/build-init/main.go +++ b/cmd/build-init/main.go @@ -55,9 +55,9 @@ const ( func main() { flag.Parse() - logger := log.New(os.Stdout, "prepare:", log.Lshortfile) + logger := log.New(os.Stdout, "", 0) - creds, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials) + creds, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials, logger) if err != nil { logger.Fatal(err) } diff --git a/cmd/rebase/main.go b/cmd/rebase/main.go index eeded40a5..420d153b7 100644 --- a/cmd/rebase/main.go +++ b/cmd/rebase/main.go @@ -2,6 +2,7 @@ package main import ( "flag" + "log" "os" "github.com/buildpacks/imgutil/remote" @@ -34,16 +35,17 @@ func init() { func main() { flag.Parse() tags := flag.Args() + logger := log.New(os.Stdout, "", 0) - cmd.Exit(rebase(tags)) + cmd.Exit(rebase(tags, logger)) } -func rebase(tags []string) error { +func rebase(tags []string, logger *log.Logger) error { if len(tags) < 1 { return cmd.FailCode(cmd.CodeInvalidArgs, "must provide one or more image tags") } - keychain, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials) + keychain, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials, logger) if err != nil { return cmd.FailErrCode(err, cmd.CodeInvalidArgs) } diff --git a/pkg/dockercreds/parse_annoted_secrets.go b/pkg/dockercreds/parse_annoted_secrets.go index e2b16d219..5e65e74a5 100644 --- a/pkg/dockercreds/parse_annoted_secrets.go +++ b/pkg/dockercreds/parse_annoted_secrets.go @@ -1,14 +1,16 @@ package dockercreds import ( + "log" "strings" "github.com/google/go-containerregistry/pkg/authn" - "github.com/pivotal/kpack/pkg/secret" "github.com/pkg/errors" + + "github.com/pivotal/kpack/pkg/secret" ) -func ParseMountedAnnotatedSecrets(volumeName string, secrets []string) (DockerCreds, error) { +func ParseMountedAnnotatedSecrets(volumeName string, secrets []string, logger *log.Logger) (DockerCreds, error) { var dockerCreds = DockerCreds{} for _, s := range secrets { splitSecret := strings.Split(s, "=") @@ -18,6 +20,7 @@ func ParseMountedAnnotatedSecrets(volumeName string, secrets []string) (DockerCr secretName := splitSecret[0] domain := splitSecret[1] + logger.Printf("Loading secrets for %q from secret %q", domain, secretName) auth, err := secret.ReadBasicAuthSecret(volumeName, secretName) if err != nil { return nil, err diff --git a/pkg/dockercreds/parse_annoted_secrets_test.go b/pkg/dockercreds/parse_annoted_secrets_test.go index 5f37622a5..7ea7a4281 100644 --- a/pkg/dockercreds/parse_annoted_secrets_test.go +++ b/pkg/dockercreds/parse_annoted_secrets_test.go @@ -1,7 +1,9 @@ package dockercreds_test import ( + "bytes" "io/ioutil" + "log" "os" "path" "testing" @@ -44,10 +46,16 @@ func testParseAnnotatedSecrets(t *testing.T, when spec.G, it spec.S) { when("ParseMountedAnnotatedSecrets", func() { it("parses the volume mounted creds", func() { + + logger := log.New(&bytes.Buffer{}, "", 0) + creds, err := dockercreds.ParseMountedAnnotatedSecrets(testDir, []string{ "gcr-creds=gcr.io", - "dockerhub-creds=index.docker.io"}) + "dockerhub-creds=index.docker.io", + }, + logger, + ) require.NoError(t, err) assert.Equal(t, dockercreds.DockerCreds{ From 908282b410cac73e17b8ce4d6d6ba39f6f048bf1 Mon Sep 17 00:00:00 2001 From: Sukhil Suresh Date: Wed, 29 Apr 2020 15:13:50 -0400 Subject: [PATCH 2/2] Incorporte feedback from PR - Log loading of dockeconfigjson and dockercfg secrets - Log loading of git secrets - Refactored to avoid passing logger around Sample logging: ``` $ logs -image tutorial-image -build 7 [prepare] Loading secrets for "https://index.docker.io/v1/" from secret "tutorial-registry-credentials" [prepare] Loading secrets for "https://index.docker.io/v1/" from secret "docker-configjson" [prepare] Loading secrets for "registry.pivotal.io" from secret "docker-configjson" [prepare] Loading secrets for "git@github.com" from secret "git-ssh-auth" [prepare] Successfully cloned "https://github.com/sukhil-suresh/spring-petclinic" @ "6108a8a7c2f82ed37968db017e3ff120fb5b2265" in path "/workspace" ``` Signed-off-by: Sukhil Suresh --- cmd/build-init/main.go | 27 ++++++++++++++++--- cmd/rebase/main.go | 2 +- pkg/apis/build/v1alpha1/build_pod.go | 6 ++--- pkg/apis/build/v1alpha1/build_pod_test.go | 8 +++--- pkg/dockercreds/parse_annoted_secrets.go | 4 +-- pkg/dockercreds/parse_annoted_secrets_test.go | 8 ++---- 6 files changed, 34 insertions(+), 21 deletions(-) diff --git a/cmd/build-init/main.go b/cmd/build-init/main.go index 91b3799d6..dab184dc9 100644 --- a/cmd/build-init/main.go +++ b/cmd/build-init/main.go @@ -5,6 +5,8 @@ import ( "log" "os" "path" + "path/filepath" + "strings" "github.com/google/go-containerregistry/pkg/authn" "github.com/pkg/errors" @@ -51,23 +53,27 @@ const ( builderPullSecretsDir = "/builderPullSecrets" projectMetadataDir = "/projectMetadata" ) - func main() { flag.Parse() logger := log.New(os.Stdout, "", 0) - creds, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials, logger) + logLoadingSecrets(logger, dockerCredentials) + creds, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials) if err != nil { logger.Fatal(err) } for _, c := range append(dockerCfgCredentials, dockerConfigCredentials...) { - dockerCfgCreds, err := dockercreds.ParseDockerPullSecrets(c) + dockerCfgCreds, err := dockercreds.ParseDockerPullSecrets(filepath.Join(buildSecretsDir, c)) if err != nil { logger.Fatal(err) } + for domain := range dockerCfgCreds { + logger.Printf("Loading secrets for %q from secret %q", domain, c) + } + creds, err = creds.Append(dockerCfgCreds) if err != nil { logger.Fatal(err) @@ -122,6 +128,7 @@ func fetchSource(logger *log.Logger, serviceAccountCreds dockercreds.DockerCreds switch { case *gitURL != "": + logLoadingSecrets(logger, basicGitCredentials, sshGitCredentials) gitKeychain, err := git.NewMountedSecretGitKeychain(buildSecretsDir, basicGitCredentials, sshGitCredentials) if err != nil { return err @@ -152,3 +159,17 @@ func fetchSource(logger *log.Logger, serviceAccountCreds dockercreds.DockerCreds return errors.New("no git url, blob url, or registry image provided") } } + +func logLoadingSecrets(logger *log.Logger, secretsSlices ...[]string) { + for _, secretsSlice := range secretsSlices { + for _, secret := range secretsSlice { + splitSecret := strings.Split(secret, "=") + if len(splitSecret) == 2 { + secretName := splitSecret[0] + domain := splitSecret[1] + logger.Printf("Loading secrets for %q from secret %q", domain, secretName) + } + } + } +} + diff --git a/cmd/rebase/main.go b/cmd/rebase/main.go index 420d153b7..7d58ac510 100644 --- a/cmd/rebase/main.go +++ b/cmd/rebase/main.go @@ -45,7 +45,7 @@ func rebase(tags []string, logger *log.Logger) error { return cmd.FailCode(cmd.CodeInvalidArgs, "must provide one or more image tags") } - keychain, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials, logger) + keychain, err := dockercreds.ParseMountedAnnotatedSecrets(buildSecretsDir, dockerCredentials) if err != nil { return cmd.FailErrCode(err, cmd.CodeInvalidArgs) } diff --git a/pkg/apis/build/v1alpha1/build_pod.go b/pkg/apis/build/v1alpha1/build_pod.go index 6ed48c2b4..8c14c28b2 100644 --- a/pkg/apis/build/v1alpha1/build_pod.go +++ b/pkg/apis/build/v1alpha1/build_pod.go @@ -455,11 +455,9 @@ func (b *Build) setupSecretVolumesAndArgs(secrets []corev1.Secret, filter func(s args = append(args, fmt.Sprintf("-basic-%s=%s=%s", "docker", secret.Name, secret.Annotations[DOCKERSecretAnnotationPrefix])) case secret.Type == corev1.SecretTypeDockerConfigJson: - args = append(args, - fmt.Sprintf("-dockerconfig=%s", fmt.Sprintf(SecretPathName, secret.Name))) + args = append(args, fmt.Sprintf("-dockerconfig=%s", secret.Name)) case secret.Type == corev1.SecretTypeDockercfg: - args = append(args, - fmt.Sprintf("-dockercfg=%s", fmt.Sprintf(SecretPathName, secret.Name))) + args = append(args, fmt.Sprintf("-dockercfg=%s", secret.Name)) case secret.Type == corev1.SecretTypeBasicAuth: annotatedUrl := secret.Annotations[GITSecretAnnotationPrefix] args = append(args, fmt.Sprintf("-basic-%s=%s=%s", "git", secret.Name, annotatedUrl)) diff --git a/pkg/apis/build/v1alpha1/build_pod_test.go b/pkg/apis/build/v1alpha1/build_pod_test.go index 28dfdb081..ad5784fbc 100644 --- a/pkg/apis/build/v1alpha1/build_pod_test.go +++ b/pkg/apis/build/v1alpha1/build_pod_test.go @@ -234,8 +234,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { "-basic-git=git-secret-1=https://github.com", "-ssh-git=git-secret-2=https://bitbucket.com", "-basic-docker=docker-secret-1=acr.io", - "-dockerconfig=/var/build-secrets/docker-secret-2", - "-dockercfg=/var/build-secrets/docker-secret-3", + "-dockerconfig=docker-secret-2", + "-dockercfg=docker-secret-3", }, pod.Spec.InitContainers[0].Args) assert.Contains(t, @@ -708,8 +708,8 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) { "--last-built-image", build.Spec.LastBuild.Image, "-basic-docker=docker-secret-1=acr.io", - "-dockerconfig=/var/build-secrets/docker-secret-2", - "-dockercfg=/var/build-secrets/docker-secret-3", + "-dockerconfig=docker-secret-2", + "-dockercfg=docker-secret-3", "someimage/name", "someimage/name:tag2", "someimage/name:tag3"}, ImagePullPolicy: corev1.PullIfNotPresent, WorkingDir: "/workspace", diff --git a/pkg/dockercreds/parse_annoted_secrets.go b/pkg/dockercreds/parse_annoted_secrets.go index 5e65e74a5..3d26bb1d6 100644 --- a/pkg/dockercreds/parse_annoted_secrets.go +++ b/pkg/dockercreds/parse_annoted_secrets.go @@ -1,7 +1,6 @@ package dockercreds import ( - "log" "strings" "github.com/google/go-containerregistry/pkg/authn" @@ -10,7 +9,7 @@ import ( "github.com/pivotal/kpack/pkg/secret" ) -func ParseMountedAnnotatedSecrets(volumeName string, secrets []string, logger *log.Logger) (DockerCreds, error) { +func ParseMountedAnnotatedSecrets(volumeName string, secrets []string) (DockerCreds, error) { var dockerCreds = DockerCreds{} for _, s := range secrets { splitSecret := strings.Split(s, "=") @@ -20,7 +19,6 @@ func ParseMountedAnnotatedSecrets(volumeName string, secrets []string, logger *l secretName := splitSecret[0] domain := splitSecret[1] - logger.Printf("Loading secrets for %q from secret %q", domain, secretName) auth, err := secret.ReadBasicAuthSecret(volumeName, secretName) if err != nil { return nil, err diff --git a/pkg/dockercreds/parse_annoted_secrets_test.go b/pkg/dockercreds/parse_annoted_secrets_test.go index 7ea7a4281..df553c82b 100644 --- a/pkg/dockercreds/parse_annoted_secrets_test.go +++ b/pkg/dockercreds/parse_annoted_secrets_test.go @@ -1,9 +1,7 @@ package dockercreds_test import ( - "bytes" "io/ioutil" - "log" "os" "path" "testing" @@ -47,14 +45,12 @@ func testParseAnnotatedSecrets(t *testing.T, when spec.G, it spec.S) { when("ParseMountedAnnotatedSecrets", func() { it("parses the volume mounted creds", func() { - logger := log.New(&bytes.Buffer{}, "", 0) - - creds, err := dockercreds.ParseMountedAnnotatedSecrets(testDir, + creds, err := dockercreds.ParseMountedAnnotatedSecrets( + testDir, []string{ "gcr-creds=gcr.io", "dockerhub-creds=index.docker.io", }, - logger, ) require.NoError(t, err)