From 5fc8e78ded3bc70670d8e0d349b9ccf3ad4266a2 Mon Sep 17 00:00:00 2001 From: Matthew McNew Date: Mon, 30 Sep 2019 13:51:21 -0600 Subject: [PATCH] Utilize k8schain from ggcr - Allow auth for images to be fetched via kubernetes/credentialprovider - Allow images/builds to be built with unauthenticated registries - Resolves #143 - Remove ImageRef interface - Images are fetched with concrete SecretRef - Builder auth can be fetched with default service account imagePullSecrets --- cmd/build-init/main.go | 14 +- go.mod | 2 + go.sum | 11 ++ pkg/apis/build/v1alpha1/abstract_builder.go | 14 ++ pkg/apis/build/v1alpha1/build.go | 20 -- pkg/apis/build/v1alpha1/build_pod.go | 2 +- pkg/apis/build/v1alpha1/builder.go | 27 +-- pkg/apis/build/v1alpha1/cluster_builder.go | 39 ++-- pkg/apis/build/v1alpha1/image_builds.go | 10 +- pkg/apis/build/v1alpha1/image_builds_test.go | 2 +- .../build/v1alpha1/image_reconcile_build.go | 2 +- pkg/buildpod/generator.go | 4 +- pkg/buildpod/generator_test.go | 4 +- pkg/cnb/cnb_metadata.go | 15 +- pkg/cnb/cnb_metadata_test.go | 53 +++++- pkg/cnb/file_permission_setup_test.go | 9 +- pkg/cnb/file_permisson_setup.go | 2 +- pkg/reconciler/v1alpha1/build/build.go | 12 +- pkg/reconciler/v1alpha1/build/build_test.go | 4 +- .../buildfakes/fake_metadata_retriever.go | 14 +- pkg/reconciler/v1alpha1/builder/builder.go | 7 +- .../builderfakes/fake_metadata_retriever.go | 14 +- .../v1alpha1/clusterbuilder/clusterbuilder.go | 3 +- .../fake_metadata_retriever.go | 14 +- pkg/reconciler/v1alpha1/image/image.go | 2 +- pkg/reconciler/v1alpha1/image/image_test.go | 16 +- pkg/registry/image_factory.go | 62 +++---- .../fake_remote_image_factory.go | 100 +++++++++- pkg/secret/secret_manager.go | 42 +---- pkg/secret/secret_manager_test.go | 105 ----------- pkg/secret/secrets_keychain.go | 69 +++---- pkg/secret/secrets_keychain_test.go | 173 +++++++++++------- 32 files changed, 408 insertions(+), 459 deletions(-) create mode 100644 pkg/apis/build/v1alpha1/abstract_builder.go delete mode 100644 pkg/secret/secret_manager_test.go diff --git a/cmd/build-init/main.go b/cmd/build-init/main.go index a38e03422..c2ba5a3d7 100644 --- a/cmd/build-init/main.go +++ b/cmd/build-init/main.go @@ -7,8 +7,6 @@ import ( "os/user" "path/filepath" - "github.com/google/go-containerregistry/pkg/authn" - "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/dockercreds" "github.com/pivotal/kpack/pkg/registry" @@ -54,9 +52,7 @@ func main() { log.Fatal(err) } - remoteImageFactory := ®istry.ImageFactory{ - KeychainFactory: keychainFactory{builderCreds}, - } + remoteImageFactory := ®istry.ImageFactory{} filePermissionSetup := &cnb.FilePermissionSetup{ RemoteImageFactory: remoteImageFactory, @@ -76,14 +72,6 @@ func main() { } } -type keychainFactory struct { - keychain authn.Keychain -} - -func (k keychainFactory) KeychainForImageRef(registry.ImageRef) authn.Keychain { - return k.keychain -} - type realOs struct { } diff --git a/go.mod b/go.mod index f17964986..924353969 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( cloud.google.com/go v0.46.3 // indirect contrib.go.opencensus.io/exporter/prometheus v0.1.0 // indirect contrib.go.opencensus.io/exporter/stackdriver v0.12.2 // indirect + github.com/Azure/azure-sdk-for-go v11.3.0-beta+incompatible // indirect github.com/aws/aws-sdk-go v1.25.1 // indirect github.com/buildpack/imgutil v0.0.0-20190827204914-36282d0caea7 // indirect github.com/buildpack/lifecycle v0.4.0 @@ -53,6 +54,7 @@ require ( k8s.io/gengo v0.0.0-20190822140433-26a664648505 // indirect k8s.io/klog v1.0.0 // indirect k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf // indirect + k8s.io/kubernetes v1.10.2 // indirect k8s.io/utils v0.0.0-20190923111123-69764acb6e8e // indirect knative.dev/pkg v0.0.0-20190927181044-f6eb4a55ec68 ) diff --git a/go.sum b/go.sum index d84118e21..a127262fa 100644 --- a/go.sum +++ b/go.sum @@ -14,6 +14,9 @@ contrib.go.opencensus.io/exporter/prometheus v0.1.0/go.mod h1:cGFniUXGZlKRjzOyuZ contrib.go.opencensus.io/exporter/stackdriver v0.12.2 h1:jU1p9F07ASK11wYgSTPKtFlTvTtCDj6R1d3nRt0ZHDE= contrib.go.opencensus.io/exporter/stackdriver v0.12.2/go.mod h1:iwB6wGarfphGGe/e5CWqyUk/cLzKnWsOKPVW3no6OTw= contrib.go.opencensus.io/resource v0.1.1/go.mod h1:F361eGI91LCmW1I/Saf+rX0+OFcigGlFvXwEGEnkRLA= +github.com/Azure/azure-sdk-for-go v11.3.0-beta+incompatible h1:F+Xs1GMaEJnaBa8gY+ogJSCeK34w4PXPYspY0huefbM= +github.com/Azure/azure-sdk-for-go v11.3.0-beta+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc= +github.com/Azure/azure-sdk-for-go v34.0.0+incompatible h1:4uQN/1HmJCkxYOnK3MUBUhHW7dxWUABOOr/LgxkkYKM= github.com/Azure/go-ansiterm v0.0.0-20170929234023-d6e3b3328b78/go.mod h1:LmzpDX56iTiv29bbRTIsUNlaFfuhWRQBWjQdVyAevI8= github.com/Azure/go-autorest v11.1.2+incompatible h1:viZ3tV5l4gE2Sw0xrasFHytCGtzYCrT+um/rrSQ1BfA= github.com/Azure/go-autorest v11.1.2+incompatible/go.mod h1:r+4oMnoxhatjLLJ6zxSWATqVooLgysK6ZNox3g/xq24= @@ -58,9 +61,12 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/dgrijalva/jwt-go v0.0.0-20160705203006-01aeca54ebda h1:NyywMz59neOoVRFDz+ccfKWxn784fiHMDnZSy6T+JXY= github.com/dgrijalva/jwt-go v0.0.0-20160705203006-01aeca54ebda/go.mod h1:E3ru+11k8xSBh+hMPgOLZmtrrCbhqsmaPHjLKYnJCaQ= github.com/docker/distribution v2.7.1+incompatible/go.mod h1:J2gT2udsDAN96Uj4KfcMRqY0/ypR+oyYUYmja8H+y+w= +github.com/docker/docker v0.7.3-0.20190307005417-54dddadc7d5d h1:nIS6IF5oAIkXtSTHeRgFDtNKHpTjvZe1q3RCA0cm1rQ= github.com/docker/docker v0.7.3-0.20190307005417-54dddadc7d5d/go.mod h1:eEKB0N0r5NX/I1kEveEz05bcu8tLC/8azJZsviup8Sk= +github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKohAFqRJQ= github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec= github.com/docker/go-units v0.3.3/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= +github.com/docker/go-units v0.4.0 h1:3uh0PgVws3nIA0Q+MwDC8yjEPf9zjRfZZWXZYDct3Tw= github.com/docker/go-units v0.4.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= github.com/docker/spdystream v0.0.0-20160310174837-449fdfce4d96/go.mod h1:Qh8CwZgvJUkLughtfhJv5dyTYa91l1fOUCrgjqmcifM= github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3ebgob9U8Nd0kOddGdZWjyMGR8Wziv+TBNwSE= @@ -76,6 +82,7 @@ github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjr github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= +github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680 h1:ZktWZesgun21uEDrwW7iEV1zPCGQldM2atlJZ3TdvVM= github.com/ghodss/yaml v0.0.0-20150909031657-73d445a93680/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= github.com/gliderlabs/ssh v0.2.2 h1:6zsha5zo/TWhRhwqCD3+EarCAgZ2yN28ipRnGPnwkI0= github.com/gliderlabs/ssh v0.2.2/go.mod h1:U7qILu1NlMHj9FlMhZLlkCdDnU1DBEAqr0aevW3Awn0= @@ -205,7 +212,9 @@ github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGV github.com/onsi/gomega v0.0.0-20190113212917-5533ce8a0da3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.5.0 h1:izbySO9zDPmjJ8rDjLvkA2zJHIo+HkYXHnf7eN7SSyo= github.com/onsi/gomega v1.5.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= +github.com/opencontainers/go-digest v1.0.0-rc1 h1:WzifXhOVOEOuFYOJAW6aQqW0TooG2iki3E3Ii+WN7gQ= github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= +github.com/opencontainers/image-spec v1.0.1 h1:JMemWkRwHx4Zj+fVxWoMCFm/8sYGGrUVojFA6h/TRcI= github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0= github.com/pelletier/go-buffruneio v0.2.0/go.mod h1:JkE26KsDizTr40EUHkXVtNPvgGtbSNq5BcowyYOWdKo= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= @@ -475,6 +484,8 @@ k8s.io/klog v1.0.0/go.mod h1:4Bi6QPql/J/LkTDqv7R/cd3hPo4k2DG6Ptcz060Ez5I= k8s.io/kube-openapi v0.0.0-20190228160746-b3a7cee44a30/go.mod h1:BXM9ceUBTj2QnfH2MK1odQs778ajze1RxcmP6S8RVVc= k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf h1:EYm5AW/UUDbnmnI+gK0TJDVK9qPLhM+sRHYanNKw0EQ= k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E= +k8s.io/kubernetes v1.10.2 h1:ad/NtArD2zm//BVvQpiWHHpSxryJrSd73B68Z18WFzM= +k8s.io/kubernetes v1.10.2/go.mod h1:ocZa8+6APFNC2tX1DZASIbocyYT5jHzqFVsY5aoB7Jk= k8s.io/utils v0.0.0-20190221042446-c2654d5206da/go.mod h1:8k8uAuAQ0rXslZKaEWd0c3oVhZz7sSzSiPnVZayjIX0= k8s.io/utils v0.0.0-20190923111123-69764acb6e8e h1:BXSmdH6S3YGLlhC89DZp+sNdYSmwNeDU6Xu5ZpzGOlM= k8s.io/utils v0.0.0-20190923111123-69764acb6e8e/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= diff --git a/pkg/apis/build/v1alpha1/abstract_builder.go b/pkg/apis/build/v1alpha1/abstract_builder.go new file mode 100644 index 000000000..2af6ae7f6 --- /dev/null +++ b/pkg/apis/build/v1alpha1/abstract_builder.go @@ -0,0 +1,14 @@ +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type AbstractBuilder interface { + metav1.ObjectMetaAccessor + BuilderImage() BuilderImage + Image() string + ImagePullSecrets() []string + Ready() bool + BuildpackMetadata() BuildpackMetadataList +} diff --git a/pkg/apis/build/v1alpha1/build.go b/pkg/apis/build/v1alpha1/build.go index 44c77fea8..8b047d7fb 100644 --- a/pkg/apis/build/v1alpha1/build.go +++ b/pkg/apis/build/v1alpha1/build.go @@ -31,30 +31,10 @@ func (*Build) GetGroupVersionKind() schema.GroupVersionKind { return SchemeGroupVersion.WithKind("Build") } -func (b *Build) ServiceAccount() string { - return b.Spec.ServiceAccount -} - -func (b *Build) Image() string { - return b.Tag() -} - func (b *Build) Tag() string { return b.Spec.Tags[0] } -func (b *Build) HasSecret() bool { - return true -} - -func (b *Build) Namespace() string { - return b.ObjectMeta.Namespace -} - -func (b *Build) SecretName() string { - return "" // Needed only for ImagePullSecrets Keychain -} - func (b *Build) IsRunning() bool { if b == nil { return false diff --git a/pkg/apis/build/v1alpha1/build_pod.go b/pkg/apis/build/v1alpha1/build_pod.go index 004924a69..cd034ea86 100644 --- a/pkg/apis/build/v1alpha1/build_pod.go +++ b/pkg/apis/build/v1alpha1/build_pod.go @@ -97,7 +97,7 @@ func (b *Build) BuildPod(config BuildPodConfig, secrets []corev1.Secret, builder return &corev1.Pod{ ObjectMeta: v1.ObjectMeta{ Name: b.PodName(), - Namespace: b.Namespace(), + Namespace: b.ObjectMeta.Namespace, Labels: b.labels(map[string]string{ BuildLabel: b.Name, }), diff --git a/pkg/apis/build/v1alpha1/builder.go b/pkg/apis/build/v1alpha1/builder.go index b870437af..572d8478a 100644 --- a/pkg/apis/build/v1alpha1/builder.go +++ b/pkg/apis/build/v1alpha1/builder.go @@ -9,40 +9,25 @@ func (b *Builder) Ready() bool { (b.Generation == b.Status.ObservedGeneration) } -func (b *Builder) ImageRef() BuilderImage { +func (b *Builder) BuilderImage() BuilderImage { return BuilderImage{ Image: b.Status.LatestImage, ImagePullSecrets: b.Spec.ImagePullSecrets, } } -func (b *Builder) SecretName() string { - if b.HasSecret() { - return b.Spec.ImagePullSecrets[0].Name +func (b *Builder) ImagePullSecrets() []string { + var secrets []string + for _, s := range b.Spec.ImagePullSecrets { + secrets = append(secrets, s.Name) } - return "" -} - -func (b *Builder) ServiceAccount() string { - return "" -} - -func (b *Builder) Namespace() string { - return b.ObjectMeta.Namespace + return secrets } func (b *Builder) Image() string { return b.Spec.Image } -func (b *Builder) HasSecret() bool { - return len(b.Spec.ImagePullSecrets) > 0 -} - func (b *Builder) BuildpackMetadata() BuildpackMetadataList { return b.Status.BuilderMetadata } - -func (b *Builder) GetName() string { - return b.ObjectMeta.Name -} diff --git a/pkg/apis/build/v1alpha1/cluster_builder.go b/pkg/apis/build/v1alpha1/cluster_builder.go index c6e3e2d14..9b2b1e8f9 100644 --- a/pkg/apis/build/v1alpha1/cluster_builder.go +++ b/pkg/apis/build/v1alpha1/cluster_builder.go @@ -4,42 +4,25 @@ import ( duckv1alpha1 "knative.dev/pkg/apis/duck/v1alpha1" ) -func (in *ClusterBuilder) ServiceAccount() string { - return "" +func (c *ClusterBuilder) Image() string { + return c.Spec.Image } -func (in *ClusterBuilder) Namespace() string { - return "" -} - -func (in *ClusterBuilder) Image() string { - return in.Spec.Image -} - -func (in *ClusterBuilder) HasSecret() bool { - return false -} - -func (in *ClusterBuilder) SecretName() string { - return "" -} - -func (in *ClusterBuilder) ImageRef() BuilderImage { +func (c *ClusterBuilder) BuilderImage() BuilderImage { return BuilderImage{ - Image: in.Status.LatestImage, - ImagePullSecrets: nil, + Image: c.Status.LatestImage, } } -func (in *ClusterBuilder) BuildpackMetadata() BuildpackMetadataList { - return in.Status.BuilderMetadata +func (c *ClusterBuilder) BuildpackMetadata() BuildpackMetadataList { + return c.Status.BuilderMetadata } -func (in *ClusterBuilder) Ready() bool { - return in.Status.GetCondition(duckv1alpha1.ConditionReady).IsTrue() && - (in.Generation == in.Status.ObservedGeneration) +func (c *ClusterBuilder) Ready() bool { + return c.Status.GetCondition(duckv1alpha1.ConditionReady).IsTrue() && + (c.Generation == c.Status.ObservedGeneration) } -func (in *ClusterBuilder) GetName() string { - return in.ObjectMeta.Name +func (c *ClusterBuilder) ImagePullSecrets() []string { + return nil } diff --git a/pkg/apis/build/v1alpha1/image_builds.go b/pkg/apis/build/v1alpha1/image_builds.go index a16da2d55..e33e41dfc 100644 --- a/pkg/apis/build/v1alpha1/image_builds.go +++ b/pkg/apis/build/v1alpha1/image_builds.go @@ -23,14 +23,6 @@ const ( BuildReasonBuildpack = "BUILDPACK" ) -type AbstractBuilder interface { - metav1.ObjectMetaAccessor - ImageRef() BuilderImage - Ready() bool - BuildpackMetadata() BuildpackMetadataList - GetName() string -} - func (im *Image) buildNeeded(lastBuild *Build, sourceResolver *SourceResolver, builder AbstractBuilder) ([]string, bool) { if !sourceResolver.Ready() { return []string{}, false @@ -96,7 +88,7 @@ func (im *Image) build(sourceResolver *SourceResolver, builder AbstractBuilder, }, Spec: BuildSpec{ Tags: im.generateTags(buildNumber), - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), Env: im.Spec.Build.Env, Resources: im.Spec.Build.Resources, ServiceAccount: im.Spec.ServiceAccount, diff --git a/pkg/apis/build/v1alpha1/image_builds_test.go b/pkg/apis/build/v1alpha1/image_builds_test.go index db468c5d5..a7c47e92e 100644 --- a/pkg/apis/build/v1alpha1/image_builds_test.go +++ b/pkg/apis/build/v1alpha1/image_builds_test.go @@ -87,7 +87,7 @@ func testImageBuilds(t *testing.T, when spec.G, it spec.S) { }, Spec: BuildSpec{ Tags: []string{"some/image"}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: "some/serviceaccount", Env: []v1.EnvVar{ { diff --git a/pkg/apis/build/v1alpha1/image_reconcile_build.go b/pkg/apis/build/v1alpha1/image_reconcile_build.go index 1fd8e39bd..106d5e274 100644 --- a/pkg/apis/build/v1alpha1/image_reconcile_build.go +++ b/pkg/apis/build/v1alpha1/image_reconcile_build.go @@ -90,7 +90,7 @@ func (r upToDateBuild) builderCondition() duckv1alpha1.Condition { Type: ConditionBuilderReady, Status: corev1.ConditionFalse, Reason: BuilderNotReady, - Message: fmt.Sprintf("Builder %s is not ready", r.builder.GetName()), + Message: fmt.Sprintf("Builder %s is not ready", r.builder.GetObjectMeta().GetName()), } } return duckv1alpha1.Condition{ diff --git a/pkg/buildpod/generator.go b/pkg/buildpod/generator.go index ad1c37105..b71e57b91 100644 --- a/pkg/buildpod/generator.go +++ b/pkg/buildpod/generator.go @@ -24,12 +24,12 @@ func (g *Generator) Generate(build *v1alpha1.Build) (*v1.Pod, error) { func (g *Generator) getBuildSecrets(build *v1alpha1.Build) ([]corev1.Secret, error) { var secrets []corev1.Secret - serviceAccount, err := g.K8sClient.CoreV1().ServiceAccounts(build.Namespace()).Get(build.ServiceAccount(), metav1.GetOptions{}) + serviceAccount, err := g.K8sClient.CoreV1().ServiceAccounts(build.ObjectMeta.Namespace).Get(build.Spec.ServiceAccount, metav1.GetOptions{}) if err != nil { return nil, err } for _, secretRef := range serviceAccount.Secrets { - secret, err := g.K8sClient.CoreV1().Secrets(build.Namespace()).Get(secretRef.Name, metav1.GetOptions{}) + secret, err := g.K8sClient.CoreV1().Secrets(build.ObjectMeta.Namespace).Get(secretRef.Name, metav1.GetOptions{}) if err != nil { return nil, err } diff --git a/pkg/buildpod/generator_test.go b/pkg/buildpod/generator_test.go index 21adcb393..d9bf6a1d1 100644 --- a/pkg/buildpod/generator_test.go +++ b/pkg/buildpod/generator_test.go @@ -103,7 +103,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { "image/name", "additional/names", }, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: serviceAccountName, Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -136,7 +136,7 @@ func testGenerator(t *testing.T, when spec.G, it spec.S) { expectedPod, err := build.BuildPod(buildPodConfig, []corev1.Secret{ *gitSecret, *dockerSecret, - }, builder.ImageRef()) + }, builder.BuilderImage()) require.NoError(t, err) require.Equal(t, expectedPod, pod) }) diff --git a/pkg/cnb/cnb_metadata.go b/pkg/cnb/cnb_metadata.go index f3877b86a..5248c4fea 100644 --- a/pkg/cnb/cnb_metadata.go +++ b/pkg/cnb/cnb_metadata.go @@ -7,6 +7,7 @@ import ( lcyclemd "github.com/buildpack/lifecycle/metadata" "github.com/pkg/errors" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha1" "github.com/pivotal/kpack/pkg/registry" ) @@ -32,8 +33,11 @@ type RemoteMetadataRetriever struct { RemoteImageFactory registry.RemoteImageFactory } -func (r *RemoteMetadataRetriever) GetBuilderImage(repo registry.ImageRef) (BuilderImage, error) { - img, err := r.RemoteImageFactory.NewRemote(repo) +func (r *RemoteMetadataRetriever) GetBuilderImage(builder v1alpha1.AbstractBuilder) (BuilderImage, error) { + img, err := r.RemoteImageFactory.NewRemote(builder.Image(), registry.SecretRef{ + Namespace: builder.GetObjectMeta().GetNamespace(), + ImagePullSecrets: builder.ImagePullSecrets(), + }) if err != nil { return BuilderImage{}, errors.Wrap(err, "unable to fetch remote builder image") } @@ -61,8 +65,11 @@ func (r *RemoteMetadataRetriever) GetBuilderImage(repo registry.ImageRef) (Build }, nil } -func (r *RemoteMetadataRetriever) GetBuiltImage(ref registry.ImageRef) (BuiltImage, error) { - img, err := r.RemoteImageFactory.NewRemote(ref) +func (r *RemoteMetadataRetriever) GetBuiltImage(ref *v1alpha1.Build) (BuiltImage, error) { + img, err := r.RemoteImageFactory.NewRemote(ref.Tag(), registry.SecretRef{ + ServiceAccount: ref.Spec.ServiceAccount, + Namespace: ref.ObjectMeta.Namespace, + }) if err != nil { return BuiltImage{}, err } diff --git a/pkg/cnb/cnb_metadata_test.go b/pkg/cnb/cnb_metadata_test.go index 3351bee86..f2129f32b 100644 --- a/pkg/cnb/cnb_metadata_test.go +++ b/pkg/cnb/cnb_metadata_test.go @@ -3,9 +3,12 @@ package cnb_test import ( "testing" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha1" "github.com/sclevine/spec" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/registry" @@ -23,39 +26,72 @@ func testMetadataRetriever(t *testing.T, when spec.G, it spec.S) { when("RemoteMetadataRetriever", func() { when("retrieving from a builder image", func() { + var builder = &v1alpha1.Builder{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "builderNamespace", + }, + Spec: v1alpha1.BuilderWithSecretsSpec{ + BuilderSpec: v1alpha1.BuilderSpec{ + Image: "builder/name", + }, + ImagePullSecrets: []v1.LocalObjectReference{ + { + Name: "Secret-1", + }, + { + Name: "Secret-2", + }, + }, + }, + } + it("gets buildpacks from a local image", func() { fakeImage := registryfakes.NewFakeRemoteImage("index.docker.io/builder/image", "sha256:2bc85afc0ee0aec012b3889cf5f2e9690bb504c9d19ce90add2f415b85990895") err := fakeImage.SetLabel("io.buildpacks.builder.metadata", `{"buildpacks": [{"id": "test.id", "version": "1.2.3"}]}`) assert.NoError(t, err) - imageRef := registry.NewNoAuthImageRef("test-repo-name") mockFactory.NewRemoteReturns(fakeImage, nil) subject := cnb.RemoteMetadataRetriever{RemoteImageFactory: mockFactory} - builderImage, err := subject.GetBuilderImage(imageRef) + builderImage, err := subject.GetBuilderImage(builder) assert.NoError(t, err) require.Len(t, builderImage.BuilderBuildpackMetadata, 1) assert.Equal(t, builderImage.BuilderBuildpackMetadata[0].ID, "test.id") assert.Equal(t, builderImage.BuilderBuildpackMetadata[0].Version, "1.2.3") - assert.Equal(t, mockFactory.NewRemoteArgsForCall(0), imageRef) + image, secretRef := mockFactory.NewRemoteArgsForCall(0) + assert.Equal(t, image, "builder/name") + assert.Equal(t, secretRef, registry.SecretRef{ + Namespace: "builderNamespace", + ImagePullSecrets: []string{"Secret-1", "Secret-2"}, + }) assert.Equal(t, "index.docker.io/builder/image@sha256:2bc85afc0ee0aec012b3889cf5f2e9690bb504c9d19ce90add2f415b85990895", builderImage.Identifier) }) }) when("GetBuiltImage", func() { + var build = &v1alpha1.Build{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace-name", + }, + Spec: v1alpha1.BuildSpec{ + Tags: []string{"image/name"}, + ServiceAccount: "service-account", + }, + Status: v1alpha1.BuildStatus{}, + } + it("retrieves the metadata from the registry", func() { fakeImage := registryfakes.NewFakeRemoteImage("index.docker.io/built/image", "sha256:dc7e5e790001c71c2cfb175854dd36e65e0b71c58294b331a519be95bdec4ef4") err := fakeImage.SetLabel("io.buildpacks.build.metadata", `{"buildpacks": [{"id": "test.id", "version": "1.2.3"}]}`) assert.NoError(t, err) - fakeImageRef := registry.NewNoAuthImageRef("built/image:tag") mockFactory.NewRemoteReturns(fakeImage, nil) subject := cnb.RemoteMetadataRetriever{RemoteImageFactory: mockFactory} - result, err := subject.GetBuiltImage(fakeImageRef) + result, err := subject.GetBuiltImage(build) assert.NoError(t, err) metadata := result.BuildpackMetadata @@ -68,7 +104,12 @@ func testMetadataRetriever(t *testing.T, when spec.G, it spec.S) { assert.Equal(t, result.CompletedAt, createdAtTime) assert.Equal(t, result.Identifier, "index.docker.io/built/image@sha256:dc7e5e790001c71c2cfb175854dd36e65e0b71c58294b331a519be95bdec4ef4") - assert.Equal(t, mockFactory.NewRemoteArgsForCall(0), fakeImageRef) + image, secretRef := mockFactory.NewRemoteArgsForCall(0) + assert.Equal(t, image, "image/name") + assert.Equal(t, secretRef, registry.SecretRef{ + ServiceAccount: "service-account", + Namespace: "namespace-name", + }) }) }) }) diff --git a/pkg/cnb/file_permission_setup_test.go b/pkg/cnb/file_permission_setup_test.go index 193e77ed1..7f97834b2 100644 --- a/pkg/cnb/file_permission_setup_test.go +++ b/pkg/cnb/file_permission_setup_test.go @@ -11,7 +11,6 @@ import ( "github.com/stretchr/testify/require" "github.com/pivotal/kpack/pkg/cnb" - "github.com/pivotal/kpack/pkg/registry" "github.com/pivotal/kpack/pkg/registry/registryfakes" ) @@ -32,7 +31,7 @@ func testFilePermissionSetup(t *testing.T, when spec.G, it spec.S) { }) it.After(func() { - os.RemoveAll(testVolume) + require.NoError(t, os.RemoveAll(testVolume)) }) when("#setup", func() { @@ -41,7 +40,7 @@ func testFilePermissionSetup(t *testing.T, when spec.G, it spec.S) { require.NoError(t, fakeImage.SetEnv("CNB_USER_ID", "1234")) require.NoError(t, fakeImage.SetEnv("CNB_GROUP_ID", "5678")) - fakeRemoteImageFactory.NewRemoteReturns(fakeImage, nil) + fakeRemoteImageFactory.NewRemoteWithDefaultKeychainReturns(fakeImage, nil) chowner := &osSpy{ chowned: make(map[string]string), @@ -56,8 +55,8 @@ func testFilePermissionSetup(t *testing.T, when spec.G, it spec.S) { require.Equal(t, chowner.chowned[testVolume], "1234:5678") - require.Equal(t, fakeRemoteImageFactory.NewRemoteCallCount(), 1) - assert.Equal(t, fakeRemoteImageFactory.NewRemoteArgsForCall(0), registry.NewNoAuthImageRef("builder/builder")) + require.Equal(t, fakeRemoteImageFactory.NewRemoteWithDefaultKeychainCallCount(), 1) + assert.Equal(t, fakeRemoteImageFactory.NewRemoteWithDefaultKeychainArgsForCall(0), "builder/builder") }) }) diff --git a/pkg/cnb/file_permisson_setup.go b/pkg/cnb/file_permisson_setup.go index 1e30eb46a..21064e0d2 100644 --- a/pkg/cnb/file_permisson_setup.go +++ b/pkg/cnb/file_permisson_setup.go @@ -21,7 +21,7 @@ const cnbUserId = "CNB_USER_ID" const cnbGroupId = "CNB_GROUP_ID" func (p *FilePermissionSetup) Setup(builder string, volumes ...string) error { - image, err := p.RemoteImageFactory.NewRemote(registry.NewNoAuthImageRef(builder)) + image, err := p.RemoteImageFactory.NewRemoteWithDefaultKeychain(builder) if err != nil { return err } diff --git a/pkg/reconciler/v1alpha1/build/build.go b/pkg/reconciler/v1alpha1/build/build.go index 385f14cb3..a906bb720 100644 --- a/pkg/reconciler/v1alpha1/build/build.go +++ b/pkg/reconciler/v1alpha1/build/build.go @@ -21,7 +21,6 @@ import ( v1alpha1lister "github.com/pivotal/kpack/pkg/client/listers/build/v1alpha1" "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/reconciler" - "github.com/pivotal/kpack/pkg/registry" ) const ( @@ -29,8 +28,9 @@ const ( Kind = "Build" ) +//go:generate counterfeiter . MetadataRetriever type MetadataRetriever interface { - GetBuiltImage(repoName registry.ImageRef) (cnb.BuiltImage, error) + GetBuiltImage(repoName *v1alpha1.Build) (cnb.BuiltImage, error) } type PodGenerator interface { @@ -112,7 +112,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { } func (c *Reconciler) reconcileBuildPod(build *v1alpha1.Build) (*corev1.Pod, error) { - pod, err := c.PodLister.Pods(build.Namespace()).Get(build.PodName()) + pod, err := c.PodLister.Pods(build.ObjectMeta.Namespace).Get(build.PodName()) if err != nil && !k8s_errors.IsNotFound(err) { return nil, err } else if k8s_errors.IsNotFound(err) { @@ -120,7 +120,7 @@ func (c *Reconciler) reconcileBuildPod(build *v1alpha1.Build) (*corev1.Pod, erro if err != nil { return nil, err } - return c.K8sClient.CoreV1().Pods(build.Namespace()).Create(podConfig) + return c.K8sClient.CoreV1().Pods(build.ObjectMeta.Namespace).Create(podConfig) } return pod, nil @@ -174,7 +174,7 @@ func stepCompleted(pod *corev1.Pod) []string { } func (c *Reconciler) updateStatus(desired *v1alpha1.Build) error { - original, err := c.Lister.Builds(desired.Namespace()).Get(desired.Name) + original, err := c.Lister.Builds(desired.ObjectMeta.Namespace).Get(desired.Name) if err != nil { return err } @@ -183,7 +183,7 @@ func (c *Reconciler) updateStatus(desired *v1alpha1.Build) error { return nil } - _, err = c.Client.BuildV1alpha1().Builds(desired.Namespace()).UpdateStatus(desired) + _, err = c.Client.BuildV1alpha1().Builds(desired.ObjectMeta.Namespace).UpdateStatus(desired) return err } diff --git a/pkg/reconciler/v1alpha1/build/build_test.go b/pkg/reconciler/v1alpha1/build/build_test.go index 02bfcc1db..dd8e9bfcb 100644 --- a/pkg/reconciler/v1alpha1/build/build_test.go +++ b/pkg/reconciler/v1alpha1/build/build_test.go @@ -111,7 +111,7 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) { Spec: v1alpha1.BuildSpec{ Tags: []string{"someimage/name", "someimage/name:tag2", "someimage/name:tag3"}, ServiceAccount: serviceAccountName, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), Env: []corev1.EnvVar{ {Name: "keyA", Value: "valueA"}, {Name: "keyB", Value: "valueB"}, @@ -743,7 +743,7 @@ func (testPodGenerator) Generate(build *v1alpha1.Build) (*corev1.Pod, error) { TypeMeta: metav1.TypeMeta{}, ObjectMeta: v1.ObjectMeta{ Name: build.PodName(), - Namespace: build.Namespace(), + Namespace: build.ObjectMeta.Namespace, Labels: build.Labels, OwnerReferences: []metav1.OwnerReference{ *kmeta.NewControllerRef(build), diff --git a/pkg/reconciler/v1alpha1/build/buildfakes/fake_metadata_retriever.go b/pkg/reconciler/v1alpha1/build/buildfakes/fake_metadata_retriever.go index e8ea8ed32..e99047f6a 100644 --- a/pkg/reconciler/v1alpha1/build/buildfakes/fake_metadata_retriever.go +++ b/pkg/reconciler/v1alpha1/build/buildfakes/fake_metadata_retriever.go @@ -4,16 +4,16 @@ package buildfakes import ( "sync" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha1" "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/reconciler/v1alpha1/build" - "github.com/pivotal/kpack/pkg/registry" ) type FakeMetadataRetriever struct { - GetBuiltImageStub func(registry.ImageRef) (cnb.BuiltImage, error) + GetBuiltImageStub func(*v1alpha1.Build) (cnb.BuiltImage, error) getBuiltImageMutex sync.RWMutex getBuiltImageArgsForCall []struct { - arg1 registry.ImageRef + arg1 *v1alpha1.Build } getBuiltImageReturns struct { result1 cnb.BuiltImage @@ -27,11 +27,11 @@ type FakeMetadataRetriever struct { invocationsMutex sync.RWMutex } -func (fake *FakeMetadataRetriever) GetBuiltImage(arg1 registry.ImageRef) (cnb.BuiltImage, error) { +func (fake *FakeMetadataRetriever) GetBuiltImage(arg1 *v1alpha1.Build) (cnb.BuiltImage, error) { fake.getBuiltImageMutex.Lock() ret, specificReturn := fake.getBuiltImageReturnsOnCall[len(fake.getBuiltImageArgsForCall)] fake.getBuiltImageArgsForCall = append(fake.getBuiltImageArgsForCall, struct { - arg1 registry.ImageRef + arg1 *v1alpha1.Build }{arg1}) fake.recordInvocation("GetBuiltImage", []interface{}{arg1}) fake.getBuiltImageMutex.Unlock() @@ -51,13 +51,13 @@ func (fake *FakeMetadataRetriever) GetBuiltImageCallCount() int { return len(fake.getBuiltImageArgsForCall) } -func (fake *FakeMetadataRetriever) GetBuiltImageCalls(stub func(registry.ImageRef) (cnb.BuiltImage, error)) { +func (fake *FakeMetadataRetriever) GetBuiltImageCalls(stub func(*v1alpha1.Build) (cnb.BuiltImage, error)) { fake.getBuiltImageMutex.Lock() defer fake.getBuiltImageMutex.Unlock() fake.GetBuiltImageStub = stub } -func (fake *FakeMetadataRetriever) GetBuiltImageArgsForCall(i int) registry.ImageRef { +func (fake *FakeMetadataRetriever) GetBuiltImageArgsForCall(i int) *v1alpha1.Build { fake.getBuiltImageMutex.RLock() defer fake.getBuiltImageMutex.RUnlock() argsForCall := fake.getBuiltImageArgsForCall[i] diff --git a/pkg/reconciler/v1alpha1/builder/builder.go b/pkg/reconciler/v1alpha1/builder/builder.go index 2f5cb781c..5b960d1ba 100644 --- a/pkg/reconciler/v1alpha1/builder/builder.go +++ b/pkg/reconciler/v1alpha1/builder/builder.go @@ -17,7 +17,6 @@ import ( v1alpha1Listers "github.com/pivotal/kpack/pkg/client/listers/build/v1alpha1" "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/reconciler" - "github.com/pivotal/kpack/pkg/registry" ) const ( @@ -27,7 +26,7 @@ const ( //go:generate counterfeiter . MetadataRetriever type MetadataRetriever interface { - GetBuilderImage(repo registry.ImageRef) (cnb.BuilderImage, error) + GetBuilderImage(builder v1alpha1.AbstractBuilder) (cnb.BuilderImage, error) } func NewController(opt reconciler.Options, builderInformer v1alpha1informers.BuilderInformer, metadataRetriever MetadataRetriever) *controller.Impl { @@ -92,7 +91,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { } func (c *Reconciler) updateStatus(desired *v1alpha1.Builder) error { - original, err := c.BuilderLister.Builders(desired.Namespace()).Get(desired.Name) + original, err := c.BuilderLister.Builders(desired.ObjectMeta.Namespace).Get(desired.Name) if err != nil { return err } @@ -101,7 +100,7 @@ func (c *Reconciler) updateStatus(desired *v1alpha1.Builder) error { return nil } - _, err = c.Client.BuildV1alpha1().Builders(desired.Namespace()).UpdateStatus(desired) + _, err = c.Client.BuildV1alpha1().Builders(desired.ObjectMeta.Namespace).UpdateStatus(desired) return err } diff --git a/pkg/reconciler/v1alpha1/builder/builderfakes/fake_metadata_retriever.go b/pkg/reconciler/v1alpha1/builder/builderfakes/fake_metadata_retriever.go index 912d76711..5492e6976 100644 --- a/pkg/reconciler/v1alpha1/builder/builderfakes/fake_metadata_retriever.go +++ b/pkg/reconciler/v1alpha1/builder/builderfakes/fake_metadata_retriever.go @@ -4,16 +4,16 @@ package builderfakes import ( "sync" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha1" "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/reconciler/v1alpha1/builder" - "github.com/pivotal/kpack/pkg/registry" ) type FakeMetadataRetriever struct { - GetBuilderImageStub func(registry.ImageRef) (cnb.BuilderImage, error) + GetBuilderImageStub func(v1alpha1.AbstractBuilder) (cnb.BuilderImage, error) getBuilderImageMutex sync.RWMutex getBuilderImageArgsForCall []struct { - arg1 registry.ImageRef + arg1 v1alpha1.AbstractBuilder } getBuilderImageReturns struct { result1 cnb.BuilderImage @@ -27,11 +27,11 @@ type FakeMetadataRetriever struct { invocationsMutex sync.RWMutex } -func (fake *FakeMetadataRetriever) GetBuilderImage(arg1 registry.ImageRef) (cnb.BuilderImage, error) { +func (fake *FakeMetadataRetriever) GetBuilderImage(arg1 v1alpha1.AbstractBuilder) (cnb.BuilderImage, error) { fake.getBuilderImageMutex.Lock() ret, specificReturn := fake.getBuilderImageReturnsOnCall[len(fake.getBuilderImageArgsForCall)] fake.getBuilderImageArgsForCall = append(fake.getBuilderImageArgsForCall, struct { - arg1 registry.ImageRef + arg1 v1alpha1.AbstractBuilder }{arg1}) fake.recordInvocation("GetBuilderImage", []interface{}{arg1}) fake.getBuilderImageMutex.Unlock() @@ -51,13 +51,13 @@ func (fake *FakeMetadataRetriever) GetBuilderImageCallCount() int { return len(fake.getBuilderImageArgsForCall) } -func (fake *FakeMetadataRetriever) GetBuilderImageCalls(stub func(registry.ImageRef) (cnb.BuilderImage, error)) { +func (fake *FakeMetadataRetriever) GetBuilderImageCalls(stub func(v1alpha1.AbstractBuilder) (cnb.BuilderImage, error)) { fake.getBuilderImageMutex.Lock() defer fake.getBuilderImageMutex.Unlock() fake.GetBuilderImageStub = stub } -func (fake *FakeMetadataRetriever) GetBuilderImageArgsForCall(i int) registry.ImageRef { +func (fake *FakeMetadataRetriever) GetBuilderImageArgsForCall(i int) v1alpha1.AbstractBuilder { fake.getBuilderImageMutex.RLock() defer fake.getBuilderImageMutex.RUnlock() argsForCall := fake.getBuilderImageArgsForCall[i] diff --git a/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilder.go b/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilder.go index bb564a4d5..0357f164a 100644 --- a/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilder.go +++ b/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilder.go @@ -16,7 +16,6 @@ import ( v1alpha1Listers "github.com/pivotal/kpack/pkg/client/listers/build/v1alpha1" "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/reconciler" - "github.com/pivotal/kpack/pkg/registry" ) const ( @@ -26,7 +25,7 @@ const ( //go:generate counterfeiter . MetadataRetriever type MetadataRetriever interface { - GetBuilderImage(repo registry.ImageRef) (cnb.BuilderImage, error) + GetBuilderImage(builder v1alpha1.AbstractBuilder) (cnb.BuilderImage, error) } func NewController(opt reconciler.Options, clusterBuilderInformer v1alpha1informers.ClusterBuilderInformer, metadataRetriever MetadataRetriever) *controller.Impl { diff --git a/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilderfakes/fake_metadata_retriever.go b/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilderfakes/fake_metadata_retriever.go index 16cc81081..c248ff3bb 100644 --- a/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilderfakes/fake_metadata_retriever.go +++ b/pkg/reconciler/v1alpha1/clusterbuilder/clusterbuilderfakes/fake_metadata_retriever.go @@ -4,16 +4,16 @@ package clusterbuilderfakes import ( "sync" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha1" "github.com/pivotal/kpack/pkg/cnb" "github.com/pivotal/kpack/pkg/reconciler/v1alpha1/clusterbuilder" - "github.com/pivotal/kpack/pkg/registry" ) type FakeMetadataRetriever struct { - GetBuilderImageStub func(registry.ImageRef) (cnb.BuilderImage, error) + GetBuilderImageStub func(v1alpha1.AbstractBuilder) (cnb.BuilderImage, error) getBuilderImageMutex sync.RWMutex getBuilderImageArgsForCall []struct { - arg1 registry.ImageRef + arg1 v1alpha1.AbstractBuilder } getBuilderImageReturns struct { result1 cnb.BuilderImage @@ -27,11 +27,11 @@ type FakeMetadataRetriever struct { invocationsMutex sync.RWMutex } -func (fake *FakeMetadataRetriever) GetBuilderImage(arg1 registry.ImageRef) (cnb.BuilderImage, error) { +func (fake *FakeMetadataRetriever) GetBuilderImage(arg1 v1alpha1.AbstractBuilder) (cnb.BuilderImage, error) { fake.getBuilderImageMutex.Lock() ret, specificReturn := fake.getBuilderImageReturnsOnCall[len(fake.getBuilderImageArgsForCall)] fake.getBuilderImageArgsForCall = append(fake.getBuilderImageArgsForCall, struct { - arg1 registry.ImageRef + arg1 v1alpha1.AbstractBuilder }{arg1}) fake.recordInvocation("GetBuilderImage", []interface{}{arg1}) fake.getBuilderImageMutex.Unlock() @@ -51,13 +51,13 @@ func (fake *FakeMetadataRetriever) GetBuilderImageCallCount() int { return len(fake.getBuilderImageArgsForCall) } -func (fake *FakeMetadataRetriever) GetBuilderImageCalls(stub func(registry.ImageRef) (cnb.BuilderImage, error)) { +func (fake *FakeMetadataRetriever) GetBuilderImageCalls(stub func(v1alpha1.AbstractBuilder) (cnb.BuilderImage, error)) { fake.getBuilderImageMutex.Lock() defer fake.getBuilderImageMutex.Unlock() fake.GetBuilderImageStub = stub } -func (fake *FakeMetadataRetriever) GetBuilderImageArgsForCall(i int) registry.ImageRef { +func (fake *FakeMetadataRetriever) GetBuilderImageArgsForCall(i int) v1alpha1.AbstractBuilder { fake.getBuilderImageMutex.RLock() defer fake.getBuilderImageMutex.RUnlock() argsForCall := fake.getBuilderImageArgsForCall[i] diff --git a/pkg/reconciler/v1alpha1/image/image.go b/pkg/reconciler/v1alpha1/image/image.go index 8c869dfe6..2efc6e3c8 100644 --- a/pkg/reconciler/v1alpha1/image/image.go +++ b/pkg/reconciler/v1alpha1/image/image.go @@ -335,5 +335,5 @@ func buildCacheEqual(desiredBuildCache *corev1.PersistentVolumeClaim, buildCache } func (c *Reconciler) CreateBuild(build *v1alpha1.Build) (*v1alpha1.Build, error) { - return c.Client.BuildV1alpha1().Builds(build.Namespace()).Create(build) + return c.Client.BuildV1alpha1().Builds(build.ObjectMeta.Namespace).Create(build) } diff --git a/pkg/reconciler/v1alpha1/image/image_test.go b/pkg/reconciler/v1alpha1/image/image_test.go index b141020c1..3dcf1f969 100644 --- a/pkg/reconciler/v1alpha1/image/image_test.go +++ b/pkg/reconciler/v1alpha1/image/image_test.go @@ -680,7 +680,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: image.Spec.ServiceAccount, Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -744,7 +744,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: image.Spec.ServiceAccount, Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -810,7 +810,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: "old-service-account", Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -852,7 +852,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: image.Spec.ServiceAccount, Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -919,7 +919,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: image.Spec.ServiceAccount, Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -961,7 +961,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: image.Spec.ServiceAccount, Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -1153,7 +1153,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: "old-service-account", Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ @@ -1205,7 +1205,7 @@ func testImageReconciler(t *testing.T, when spec.G, it spec.S) { }, Spec: v1alpha1.BuildSpec{ Tags: []string{image.Spec.Tag}, - Builder: builder.ImageRef(), + Builder: builder.BuilderImage(), ServiceAccount: image.Spec.ServiceAccount, Source: v1alpha1.SourceConfig{ Git: &v1alpha1.Git{ diff --git a/pkg/registry/image_factory.go b/pkg/registry/image_factory.go index a60b0bdb7..d6d599d74 100644 --- a/pkg/registry/image_factory.go +++ b/pkg/registry/image_factory.go @@ -10,49 +10,20 @@ type ImageFactory struct { KeychainFactory KeychainFactory } -func (f *ImageFactory) NewRemote(imageRef ImageRef) (RemoteImage, error) { - remoteImage, err := NewGoContainerRegistryImage(imageRef.Image(), f.KeychainFactory.KeychainForImageRef(imageRef)) - return remoteImage, err +func (f *ImageFactory) NewRemote(image string, secretRef SecretRef) (RemoteImage, error) { + keychain, err := f.KeychainFactory.KeychainForSecretRef(secretRef) + if err != nil { + return nil, err + } + return NewGoContainerRegistryImage(image, keychain) } -type KeychainFactory interface { - KeychainForImageRef(ImageRef) authn.Keychain -} - -type ImageRef interface { - ServiceAccount() string - Namespace() string - Image() string - HasSecret() bool - SecretName() string -} - -type noAuthImageRef struct { - identifier string -} - -func (na *noAuthImageRef) SecretName() string { - return "" -} - -func NewNoAuthImageRef(identifier string) *noAuthImageRef { - return &noAuthImageRef{identifier: identifier} -} - -func (na *noAuthImageRef) Image() string { - return na.identifier -} - -func (noAuthImageRef) ServiceAccount() string { - return "" -} - -func (noAuthImageRef) HasSecret() bool { - return false +func (f *ImageFactory) NewRemoteWithDefaultKeychain(image string) (RemoteImage, error) { + return NewGoContainerRegistryImage(image, authn.DefaultKeychain) } -func (noAuthImageRef) Namespace() string { - return "" +type KeychainFactory interface { + KeychainForSecretRef(SecretRef) (authn.Keychain, error) } type RemoteImage interface { @@ -64,5 +35,16 @@ type RemoteImage interface { //go:generate counterfeiter . RemoteImageFactory type RemoteImageFactory interface { - NewRemote(imageRef ImageRef) (RemoteImage, error) + NewRemote(image string, secretRef SecretRef) (RemoteImage, error) + NewRemoteWithDefaultKeychain(image string) (RemoteImage, error) +} + +type SecretRef struct { + ServiceAccount string + Namespace string + ImagePullSecrets []string +} + +func (ref SecretRef) HasNamespace() bool { + return ref.Namespace != "" } diff --git a/pkg/registry/registryfakes/fake_remote_image_factory.go b/pkg/registry/registryfakes/fake_remote_image_factory.go index 781039817..4d5477f22 100644 --- a/pkg/registry/registryfakes/fake_remote_image_factory.go +++ b/pkg/registry/registryfakes/fake_remote_image_factory.go @@ -8,10 +8,11 @@ import ( ) type FakeRemoteImageFactory struct { - NewRemoteStub func(registry.ImageRef) (registry.RemoteImage, error) + NewRemoteStub func(string, registry.SecretRef) (registry.RemoteImage, error) newRemoteMutex sync.RWMutex newRemoteArgsForCall []struct { - arg1 registry.ImageRef + arg1 string + arg2 registry.SecretRef } newRemoteReturns struct { result1 registry.RemoteImage @@ -21,20 +22,34 @@ type FakeRemoteImageFactory struct { result1 registry.RemoteImage result2 error } + NewRemoteWithDefaultKeychainStub func(string) (registry.RemoteImage, error) + newRemoteWithDefaultKeychainMutex sync.RWMutex + newRemoteWithDefaultKeychainArgsForCall []struct { + arg1 string + } + newRemoteWithDefaultKeychainReturns struct { + result1 registry.RemoteImage + result2 error + } + newRemoteWithDefaultKeychainReturnsOnCall map[int]struct { + result1 registry.RemoteImage + result2 error + } invocations map[string][][]interface{} invocationsMutex sync.RWMutex } -func (fake *FakeRemoteImageFactory) NewRemote(arg1 registry.ImageRef) (registry.RemoteImage, error) { +func (fake *FakeRemoteImageFactory) NewRemote(arg1 string, arg2 registry.SecretRef) (registry.RemoteImage, error) { fake.newRemoteMutex.Lock() ret, specificReturn := fake.newRemoteReturnsOnCall[len(fake.newRemoteArgsForCall)] fake.newRemoteArgsForCall = append(fake.newRemoteArgsForCall, struct { - arg1 registry.ImageRef - }{arg1}) - fake.recordInvocation("NewRemote", []interface{}{arg1}) + arg1 string + arg2 registry.SecretRef + }{arg1, arg2}) + fake.recordInvocation("NewRemote", []interface{}{arg1, arg2}) fake.newRemoteMutex.Unlock() if fake.NewRemoteStub != nil { - return fake.NewRemoteStub(arg1) + return fake.NewRemoteStub(arg1, arg2) } if specificReturn { return ret.result1, ret.result2 @@ -49,17 +64,17 @@ func (fake *FakeRemoteImageFactory) NewRemoteCallCount() int { return len(fake.newRemoteArgsForCall) } -func (fake *FakeRemoteImageFactory) NewRemoteCalls(stub func(registry.ImageRef) (registry.RemoteImage, error)) { +func (fake *FakeRemoteImageFactory) NewRemoteCalls(stub func(string, registry.SecretRef) (registry.RemoteImage, error)) { fake.newRemoteMutex.Lock() defer fake.newRemoteMutex.Unlock() fake.NewRemoteStub = stub } -func (fake *FakeRemoteImageFactory) NewRemoteArgsForCall(i int) registry.ImageRef { +func (fake *FakeRemoteImageFactory) NewRemoteArgsForCall(i int) (string, registry.SecretRef) { fake.newRemoteMutex.RLock() defer fake.newRemoteMutex.RUnlock() argsForCall := fake.newRemoteArgsForCall[i] - return argsForCall.arg1 + return argsForCall.arg1, argsForCall.arg2 } func (fake *FakeRemoteImageFactory) NewRemoteReturns(result1 registry.RemoteImage, result2 error) { @@ -88,11 +103,76 @@ func (fake *FakeRemoteImageFactory) NewRemoteReturnsOnCall(i int, result1 regist }{result1, result2} } +func (fake *FakeRemoteImageFactory) NewRemoteWithDefaultKeychain(arg1 string) (registry.RemoteImage, error) { + fake.newRemoteWithDefaultKeychainMutex.Lock() + ret, specificReturn := fake.newRemoteWithDefaultKeychainReturnsOnCall[len(fake.newRemoteWithDefaultKeychainArgsForCall)] + fake.newRemoteWithDefaultKeychainArgsForCall = append(fake.newRemoteWithDefaultKeychainArgsForCall, struct { + arg1 string + }{arg1}) + fake.recordInvocation("NewRemoteWithDefaultKeychain", []interface{}{arg1}) + fake.newRemoteWithDefaultKeychainMutex.Unlock() + if fake.NewRemoteWithDefaultKeychainStub != nil { + return fake.NewRemoteWithDefaultKeychainStub(arg1) + } + if specificReturn { + return ret.result1, ret.result2 + } + fakeReturns := fake.newRemoteWithDefaultKeychainReturns + return fakeReturns.result1, fakeReturns.result2 +} + +func (fake *FakeRemoteImageFactory) NewRemoteWithDefaultKeychainCallCount() int { + fake.newRemoteWithDefaultKeychainMutex.RLock() + defer fake.newRemoteWithDefaultKeychainMutex.RUnlock() + return len(fake.newRemoteWithDefaultKeychainArgsForCall) +} + +func (fake *FakeRemoteImageFactory) NewRemoteWithDefaultKeychainCalls(stub func(string) (registry.RemoteImage, error)) { + fake.newRemoteWithDefaultKeychainMutex.Lock() + defer fake.newRemoteWithDefaultKeychainMutex.Unlock() + fake.NewRemoteWithDefaultKeychainStub = stub +} + +func (fake *FakeRemoteImageFactory) NewRemoteWithDefaultKeychainArgsForCall(i int) string { + fake.newRemoteWithDefaultKeychainMutex.RLock() + defer fake.newRemoteWithDefaultKeychainMutex.RUnlock() + argsForCall := fake.newRemoteWithDefaultKeychainArgsForCall[i] + return argsForCall.arg1 +} + +func (fake *FakeRemoteImageFactory) NewRemoteWithDefaultKeychainReturns(result1 registry.RemoteImage, result2 error) { + fake.newRemoteWithDefaultKeychainMutex.Lock() + defer fake.newRemoteWithDefaultKeychainMutex.Unlock() + fake.NewRemoteWithDefaultKeychainStub = nil + fake.newRemoteWithDefaultKeychainReturns = struct { + result1 registry.RemoteImage + result2 error + }{result1, result2} +} + +func (fake *FakeRemoteImageFactory) NewRemoteWithDefaultKeychainReturnsOnCall(i int, result1 registry.RemoteImage, result2 error) { + fake.newRemoteWithDefaultKeychainMutex.Lock() + defer fake.newRemoteWithDefaultKeychainMutex.Unlock() + fake.NewRemoteWithDefaultKeychainStub = nil + if fake.newRemoteWithDefaultKeychainReturnsOnCall == nil { + fake.newRemoteWithDefaultKeychainReturnsOnCall = make(map[int]struct { + result1 registry.RemoteImage + result2 error + }) + } + fake.newRemoteWithDefaultKeychainReturnsOnCall[i] = struct { + result1 registry.RemoteImage + result2 error + }{result1, result2} +} + func (fake *FakeRemoteImageFactory) Invocations() map[string][][]interface{} { fake.invocationsMutex.RLock() defer fake.invocationsMutex.RUnlock() fake.newRemoteMutex.RLock() defer fake.newRemoteMutex.RUnlock() + fake.newRemoteWithDefaultKeychainMutex.RLock() + defer fake.newRemoteWithDefaultKeychainMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} for key, value := range fake.invocations { copiedInvocations[key] = value diff --git a/pkg/secret/secret_manager.go b/pkg/secret/secret_manager.go index 9b042ec0a..1696f1790 100644 --- a/pkg/secret/secret_manager.go +++ b/pkg/secret/secret_manager.go @@ -1,7 +1,6 @@ package secret import ( - "encoding/json" "fmt" "k8s.io/api/core/v1" @@ -30,7 +29,7 @@ func (m *SecretManager) SecretForServiceAccountAndURL(serviceAccount, namespace return nil, err } - registryUser := NewURLAndUser(url, string(secret.Data["username"]), string(secret.Data["password"])) + registryUser := NewURLAndUser(url, string(secret.Data[v1.BasicAuthUsernameKey]), string(secret.Data[v1.BasicAuthPasswordKey])) return ®istryUser, nil } @@ -41,47 +40,10 @@ func (m *SecretManager) secretForServiceAccount(account *v1.ServiceAccount, url return nil, err } - if m.Matcher(url, secret.ObjectMeta.Annotations[m.AnnotationKey]) { + if m.Matcher(url, secret.ObjectMeta.Annotations[m.AnnotationKey]) && secret.Type == v1.SecretTypeBasicAuth { return secret, nil } } return nil, k8serrors.NewNotFound(schema.GroupResource{Group: "", Resource: "Secret"}, fmt.Sprintf("secret for %s", url)) } - -type dockerConfigJson struct { - Auths dockerConfig `json:"auths"` -} - -type dockerConfig map[string]dockerConfigEntry - -type dockerConfigEntry struct { - Auth string -} - -func (m *SecretManager) SecretForImagePull(namespace, secretName, registryName string) (string, error) { - secret, err := m.Client.CoreV1().Secrets(namespace).Get(secretName, meta_v1.GetOptions{}) - if err != nil { - return "", err - } - - var config dockerConfigJson - switch secret.Type { - case v1.SecretTypeDockercfg: - var auths dockerConfig - err = json.Unmarshal(secret.Data[v1.DockerConfigKey], &auths) - config.Auths = auths - case v1.SecretTypeDockerConfigJson: - err = json.Unmarshal(secret.Data[v1.DockerConfigJsonKey], &config) - } - if err != nil { - return "", err - } - - for registry, registryAuth := range config.Auths { - if m.Matcher(registryName, registry) { - return registryAuth.Auth, nil - } - } - return "", fmt.Errorf("no secret configuration for registry: %s", registryName) -} diff --git a/pkg/secret/secret_manager_test.go b/pkg/secret/secret_manager_test.go deleted file mode 100644 index 3a1ef72d0..000000000 --- a/pkg/secret/secret_manager_test.go +++ /dev/null @@ -1,105 +0,0 @@ -package secret_test - -import ( - "strings" - "testing" - - "github.com/sclevine/spec" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes/fake" - - "github.com/pivotal/kpack/pkg/secret" -) - -func TestSecretManagerFactory(t *testing.T) { - spec.Run(t, "SecretManager", testSecretManager) -} -func testSecretManager(t *testing.T, when spec.G, it spec.S) { - const ( - namespace = "some-namespace" - secretName = "some-secret-name" - ) - var ( - fakeClient = fake.NewSimpleClientset(&v1.Secret{}) - - subject = secret.SecretManager{ - Client: fakeClient, - Matcher: fakeMatch, - } - ) - - when("ImagePull Secret", func() { - it("retrieves the secret from dockerconfigjson", func() { - _, err := fakeClient.CoreV1().Secrets(namespace).Create(&v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - }, - Data: map[string][]byte{ - v1.DockerConfigJsonKey: []byte(`{ "auths": { "some-registry": { "auth": "some-base64-secret" }, "some-other-registry": { "auth": "some-base64-secret" } } }`), - }, - Type: v1.SecretTypeDockerConfigJson, - }) - require.NoError(t, err) - - auth, err := subject.SecretForImagePull(namespace, secretName, "some-registry") - require.NoError(t, err) - assert.Equal(t, "some-base64-secret", auth) - }) - - it("retrieves the secret from dockercfg", func() { - _, err := fakeClient.CoreV1().Secrets(namespace).Create(&v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - }, - Data: map[string][]byte{ - v1.DockerConfigKey: []byte(`{ "some-registry": { "auth": "some-base64-secret" }, "some-other-registry": { "auth": "some-base64-secret" } }`), - }, - Type: v1.SecretTypeDockercfg, - }) - require.NoError(t, err) - - auth, err := subject.SecretForImagePull(namespace, secretName, "some-registry") - require.NoError(t, err) - assert.Equal(t, "some-base64-secret", auth) - }) - - it("errors when registry secret is not available from dockerconfigjson", func() { - _, err := fakeClient.CoreV1().Secrets(namespace).Create(&v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - }, - Data: map[string][]byte{ - v1.DockerConfigJsonKey: []byte(`{ "auths": { "some-registry": { "auth": "some-base64-secret" } } }`), - }, - Type: v1.SecretTypeDockerConfigJson, - }) - require.NoError(t, err) - - _, err = subject.SecretForImagePull(namespace, secretName, "some-other-registry") - assert.EqualError(t, err, "no secret configuration for registry: some-other-registry") - }) - - it("errors when registry secret is not available from dockercfg", func() { - _, err := fakeClient.CoreV1().Secrets(namespace).Create(&v1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: secretName, - }, - Data: map[string][]byte{ - v1.DockerConfigKey: []byte(`{ "some-registry": { "auth": "some-base64-secret" } }`), - }, - Type: v1.SecretTypeDockercfg, - }) - require.NoError(t, err) - - _, err = subject.SecretForImagePull(namespace, secretName, "some-other-registry") - assert.EqualError(t, err, "no secret configuration for registry: some-other-registry") - }) - }) -} - -func fakeMatch(url, annotatedUrl string) bool { - return strings.Contains(annotatedUrl, url) -} diff --git a/pkg/secret/secrets_keychain.go b/pkg/secret/secrets_keychain.go index 0985c61b7..ecc47b32b 100644 --- a/pkg/secret/secrets_keychain.go +++ b/pkg/secret/secrets_keychain.go @@ -2,6 +2,8 @@ package secret import ( "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/authn/k8schain" + k8serrors "k8s.io/apimachinery/pkg/api/errors" k8sclient "k8s.io/client-go/kubernetes" "github.com/pivotal/kpack/pkg/apis/build/v1alpha1" @@ -9,60 +11,49 @@ import ( "github.com/pivotal/kpack/pkg/registry" ) -type SecretKeychainFactory struct { - secretManager *SecretManager +type k8sKeychainFactory struct { + client k8sclient.Interface } -func NewSecretKeychainFactory(client k8sclient.Interface) *SecretKeychainFactory { - return &SecretKeychainFactory{ - secretManager: &SecretManager{ - Client: client, - AnnotationKey: v1alpha1.DOCKERSecretAnnotationPrefix, - Matcher: dockercreds.RegistryMatch, - }, +func (f *k8sKeychainFactory) KeychainForSecretRef(ref registry.SecretRef) (authn.Keychain, error) { + if !ref.HasNamespace() { + return k8schain.New(nil, k8schain.Options{}) // k8s keychain with no secrets } -} - -type pullSecretKeychain struct { - imageRef registry.ImageRef - secretManager *SecretManager -} -func (k *pullSecretKeychain) Resolve(registry authn.Resource) (authn.Authenticator, error) { - base64Auth, err := k.secretManager.SecretForImagePull(k.imageRef.Namespace(), k.imageRef.SecretName(), registry.RegistryStr()) - if err != nil { - return nil, err + annotatedBasicAuthKeychain := &annotatedBasicAuthKeychain{ + secretManager: &SecretManager{Client: f.client, AnnotationKey: v1alpha1.DOCKERSecretAnnotationPrefix, Matcher: dockercreds.RegistryMatch}, } - return dockercreds.Auth(base64Auth), nil -} - -type serviceAccountKeychain struct { - imageRef registry.ImageRef - secretManager *SecretManager -} -func (k *serviceAccountKeychain) Resolve(res authn.Resource) (authn.Authenticator, error) { - creds, err := k.secretManager.SecretForServiceAccountAndURL(k.imageRef.ServiceAccount(), k.imageRef.Namespace(), res.RegistryStr()) + k8sKeychain, err := k8schain.New(f.client, k8schain.Options{ + Namespace: ref.Namespace, + ServiceAccountName: ref.ServiceAccount, + ImagePullSecrets: ref.ImagePullSecrets, + }) if err != nil { return nil, err } - return &authn.Basic{Username: creds.Username, Password: creds.Password}, nil + return authn.NewMultiKeychain(annotatedBasicAuthKeychain, k8sKeychain), nil } -func (f *SecretKeychainFactory) KeychainForImageRef(ref registry.ImageRef) authn.Keychain { - if !ref.HasSecret() { - return &anonymousKeychain{} +func NewSecretKeychainFactory(client k8sclient.Interface) *k8sKeychainFactory { + return &k8sKeychainFactory{ + client: client, } - if ref.ServiceAccount() == "" { - return &pullSecretKeychain{imageRef: ref, secretManager: f.secretManager} - } - return &serviceAccountKeychain{imageRef: ref, secretManager: f.secretManager} } -type anonymousKeychain struct { +type annotatedBasicAuthKeychain struct { + secretRef registry.SecretRef + secretManager *SecretManager } -func (anonymousKeychain) Resolve(authn.Resource) (authn.Authenticator, error) { - return authn.Anonymous, nil +func (k *annotatedBasicAuthKeychain) Resolve(res authn.Resource) (authn.Authenticator, error) { + creds, err := k.secretManager.SecretForServiceAccountAndURL(k.secretRef.ServiceAccount, k.secretRef.Namespace, res.RegistryStr()) + if err != nil && !k8serrors.IsNotFound(err) { + return nil, err + } else if k8serrors.IsNotFound(err) { + return authn.Anonymous, nil + } + + return &authn.Basic{Username: creds.Username, Password: creds.Password}, nil } diff --git a/pkg/secret/secrets_keychain_test.go b/pkg/secret/secrets_keychain_test.go index 17001e702..3e5e0d38d 100644 --- a/pkg/secret/secrets_keychain_test.go +++ b/pkg/secret/secrets_keychain_test.go @@ -6,108 +6,147 @@ import ( "testing" "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/authn/k8schain" "github.com/google/go-containerregistry/pkg/name" + "github.com/pivotal/kpack/pkg/apis/build/v1alpha1" + "github.com/pivotal/kpack/pkg/registry" "github.com/sclevine/spec" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" "github.com/pivotal/kpack/pkg/secret" - secrethelper "github.com/pivotal/kpack/pkg/secret/testhelpers" ) func TestSecretKeychainFactory(t *testing.T) { - spec.Run(t, "SecretKeychainFactory", testSecretKeychain) + spec.Run(t, "k8sKeychainFactory", testSecretKeychain) } func testSecretKeychain(t *testing.T, when spec.G, it spec.S) { const ( serviceAccountName = "some-service-account" + testNamespace = "test-namespace" ) - var ( - testNamespace = "namespace" - keychainFactory *secret.SecretKeychainFactory - fakeClient = fake.NewSimpleClientset(&v1.Secret{}) - ) - - when("SecretKeychainFactory", func() { - it.Before(func() { - keychainFactory = secret.NewSecretKeychainFactory(fakeClient) - - err := secrethelper.SaveDockerSecrets(fakeClient, testNamespace, serviceAccountName, - []secret.URLAndUser{ - secret.NewURLAndUser("https://godoker.reg.com", "foobar", "foobar321"), - secret.NewURLAndUser("https://redhook.port", "brooklyn", "nothip"), + when("#KeychainForSecretRef", func() { + it("keychain provides auth from annotated basic auth secrets", func() { + fakeClient := fake.NewSimpleClientset(&v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "secret-1", + Namespace: testNamespace, + Annotations: map[string]string{ + v1alpha1.DOCKERSecretAnnotationPrefix: "annotated.io", + }, + }, + Type: v1.SecretTypeBasicAuth, + Data: map[string][]byte{ + v1.BasicAuthUsernameKey: []byte("annotated-username"), + v1.BasicAuthPasswordKey: []byte("annotated-password"), + }, + }, + &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceAccountName, + Namespace: testNamespace, + }, + Secrets: []v1.ObjectReference{ + {Name: "secret-1"}, + }, }) - assert.NoError(t, err) - }) - - when("#NewImage", func() { - it("returns a keychain that provides auth credentials", func() { - keychain := keychainFactory.KeychainForImageRef(&fakeImageRef{serviceAccountName: serviceAccountName, namespace: testNamespace, hasSecret: true}) - - reference, err := name.ParseReference("redhook.port/name", name.WeakValidation) - assert.NoError(t, err) + keychainFactory := secret.NewSecretKeychainFactory(fakeClient) - authenticator, err := keychain.Resolve(reference.Context().Registry) - assert.NoError(t, err) + keychain, err := keychainFactory.KeychainForSecretRef(registry.SecretRef{ + ServiceAccount: serviceAccountName, + Namespace: testNamespace, + }) - encoded := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", "brooklyn", "nothip"))) + reg, err := name.NewRegistry("annotated.io") + require.NoError(t, err) - auth, err := authenticator.Authorization() - assert.NoError(t, err) - assert.Equal(t, auth, fmt.Sprintf("Basic %s", encoded)) - }) + authenticator, err := keychain.Resolve(reg) + require.NoError(t, err) - it("returns an error if no credentials are provided for the registry", func() { - keychain := keychainFactory.KeychainForImageRef(&fakeImageRef{serviceAccountName: serviceAccountName, namespace: testNamespace, hasSecret: true}) + encoded := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", "annotated-username", "annotated-password"))) - reference, err := name.ParseReference("notareal.reg/name", name.WeakValidation) - assert.NoError(t, err) + auth, err := authenticator.Authorization() + require.NoError(t, err) + assert.Equal(t, auth, fmt.Sprintf("Basic %s", encoded)) + }) - _, err = keychain.Resolve(reference.Context().Registry) - assert.Error(t, err, "credentials not found for: notareal.reg") + it("keychain provides auth from ImagePull secrets", func() { + fakeClient := fake.NewSimpleClientset(&v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "image-pull-secret", + Namespace: testNamespace, + }, + Type: v1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + v1.DockerConfigJsonKey: []byte("{\"auths\": {\"imagepull.io\": {\"username\": \"image-pull-user\", \"password\": \"image-pull-password\"}}}"), + }, + }, + &v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceAccountName, + Namespace: testNamespace, + }, + }) + keychainFactory := secret.NewSecretKeychainFactory(fakeClient) + keychain, err := keychainFactory.KeychainForSecretRef(registry.SecretRef{ + ServiceAccount: serviceAccountName, + Namespace: testNamespace, + ImagePullSecrets: []string{"image-pull-secret"}, }) + require.NoError(t, err) - it("returns anonymous auth if does not have a secret", func() { - keychain := keychainFactory.KeychainForImageRef(&fakeImageRef{serviceAccountName: "asd", namespace: testNamespace, hasSecret: false}) + reg, err := name.NewRegistry("imagepull.io") + require.NoError(t, err) - reference, err := name.ParseReference("notareal.reg/name", name.WeakValidation) - assert.NoError(t, err) + authenticator, err := keychain.Resolve(reg) + require.NoError(t, err) - authenticator, err := keychain.Resolve(reference.Context().Registry) - assert.NoError(t, err) + auth, err := authenticator.Authorization() + require.NoError(t, err) - assert.Equal(t, authenticator, authn.Anonymous) - }) + encoded := base64.StdEncoding.EncodeToString([]byte(fmt.Sprintf("%s:%s", "image-pull-user", "image-pull-password"))) + assert.Equal(t, fmt.Sprintf("Basic %s", encoded), auth) }) - }) -} -type fakeImageRef struct { - serviceAccountName string - namespace string - hasSecret bool -} + it("keychain provides Anonymous auth for no matching credentials", func() { + keychainFactory := secret.NewSecretKeychainFactory(fake.NewSimpleClientset(&v1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: serviceAccountName, + Namespace: testNamespace, + }, + })) + + keychain, err := keychainFactory.KeychainForSecretRef(registry.SecretRef{ + ServiceAccount: serviceAccountName, + Namespace: testNamespace, + }) + require.NoError(t, err) -func (f *fakeImageRef) SecretName() string { - return "" -} + reg, err := name.NewRegistry("nosecret.io") + require.NoError(t, err) + auth, err := keychain.Resolve(reg) + require.NoError(t, err) -func (f *fakeImageRef) Namespace() string { - return f.namespace -} + assert.Equal(t, auth, authn.Anonymous) + }) -func (f *fakeImageRef) Image() string { - return "NOT-NEEDED" -} + it("returns an empty k8schain when no namespace is provided to leverage k8s.io/kubernetes/pkg/credentialprovider", func() { + keychainFactory := secret.NewSecretKeychainFactory(fake.NewSimpleClientset()) -func (f *fakeImageRef) ServiceAccount() string { - return f.serviceAccountName -} + keychain, err := keychainFactory.KeychainForSecretRef(registry.SecretRef{ + Namespace: "", + }) + require.NoError(t, err) -func (f *fakeImageRef) HasSecret() bool { - return f.hasSecret + expected, err := k8schain.New(nil, k8schain.Options{}) + require.NoError(t, err) + assert.Equal(t, expected, keychain) + }) + }) }