diff --git a/pkg/cnb/create_builder.go b/pkg/cnb/create_builder.go index 3dbf96538..21c05c197 100644 --- a/pkg/cnb/create_builder.go +++ b/pkg/cnb/create_builder.go @@ -5,6 +5,7 @@ import ( "github.com/google/go-containerregistry/pkg/authn" ggcrv1 "github.com/google/go-containerregistry/pkg/v1" + buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1" "github.com/pivotal/kpack/pkg/registry" @@ -26,13 +27,8 @@ type RemoteBuilderCreator struct { KeychainFactory registry.KeychainFactory } -func (r *RemoteBuilderCreator) CreateBuilder( - ctx context.Context, - builderKeychain authn.Keychain, - fetcher RemoteBuildpackFetcher, - clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec, -) (buildapi.BuilderRecord, error) { - buildImage, _, err := r.RegistryClient.Fetch(builderKeychain, clusterStack.Status.BuildImage.LatestImage) +func (r *RemoteBuilderCreator) CreateBuilder(ctx context.Context, builderKeychain authn.Keychain, stackKeychain authn.Keychain, fetcher RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec) (buildapi.BuilderRecord, error) { + buildImage, _, err := r.RegistryClient.Fetch(stackKeychain, clusterStack.Status.BuildImage.LatestImage) if err != nil { return buildapi.BuilderRecord{}, err } diff --git a/pkg/cnb/create_builder_test.go b/pkg/cnb/create_builder_test.go index 469171be2..f3f256eb1 100644 --- a/pkg/cnb/create_builder_test.go +++ b/pkg/cnb/create_builder_test.go @@ -57,7 +57,8 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { registryClient = registryfakes.NewFakeClient() keychainFactory = ®istryfakes.FakeKeychainFactory{} - keychain = authn.NewMultiKeychain(authn.DefaultKeychain) + builderKeychain = authn.NewMultiKeychain(authn.DefaultKeychain) + stackKeychain = authn.NewMultiKeychain(authn.DefaultKeychain) secretRef = registry.SecretRef{} ctx = context.Background() @@ -190,7 +191,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { ) it.Before(func() { - keychainFactory.AddKeychainForSecretRef(t, secretRef, keychain) + keychainFactory.AddKeychainForSecretRef(t, secretRef, builderKeychain) buildpack1 := buildpackLayer{ v1Layer: buildpack1Layer, @@ -267,7 +268,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { fetcher.AddBuildpack(t, "io.buildpack.2", "v2", []buildpackLayer{buildpack3, buildpack2}) }) - registryClient.AddSaveKeychain("custom/example", keychain) + registryClient.AddSaveKeychain("custom/example", builderKeychain) when("CreateBuilder", func() { var ( @@ -286,7 +287,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { config.OS = os buildImg, err = mutate.ConfigFile(buildImg, config) - registryClient.AddImage(buildImage, buildImg, keychain) + registryClient.AddImage(buildImage, buildImg, stackKeychain) lifecycleProvider.metadata = LifecycleMetadata{ LifecycleInfo: LifecycleInfo{ @@ -314,7 +315,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }) it("creates a custom builder", func() { - builderRecord, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.NoError(t, err) assert.Len(t, builderRecord.Buildpacks, 3) @@ -576,11 +577,11 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }) it("creates images deterministically ", func() { - original, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + original, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.NoError(t, err) for i := 1; i <= 50; i++ { - other, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + other, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.NoError(t, err) require.Equal(t, original.Image, other.Image) @@ -610,7 +611,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, } - _, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.stack@v4: stack io.buildpacks.stacks.some-stack is not supported") }) @@ -634,7 +635,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.mixin@v4: stack missing mixin(s): something-missing-mixin, something-missing-mixin2") }) @@ -679,7 +680,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.Nil(t, err) }) @@ -704,7 +705,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + _, err := subject.CreateBuilder(ctx, builderKeychain, nil, fetcher, stack, clusterBuilderSpec) require.Error(t, err, "validating buildpack io.buildpack.relaxed.old.mixin@v4: stack missing mixin(s): build:common-mixin, run:common-mixin, another-common-mixin") }) @@ -727,7 +728,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.buildpack.api@v4: unsupported buildpack api: 0.1, expecting: 0.2, 0.3") }) @@ -770,7 +771,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.NoError(t, err) }) }) @@ -797,7 +798,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, } - _, err := subject.CreateBuilder(ctx, keychain, fetcher, stack, clusterBuilderSpec) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec) require.EqualError(t, err, "unsupported platform apis in kpack lifecycle: 0.1, 0.2, 0.999, expecting one of: 0.3, 0.4, 0.5, 0.6, 0.7, 0.8") }) }) diff --git a/pkg/reconciler/builder/builder.go b/pkg/reconciler/builder/builder.go index 2c2fa287a..e78dc4a58 100644 --- a/pkg/reconciler/builder/builder.go +++ b/pkg/reconciler/builder/builder.go @@ -33,7 +33,7 @@ const ( ) type BuilderCreator interface { - CreateBuilder(ctx context.Context, keychain authn.Keychain, fetcher cnb.RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec) (buildapi.BuilderRecord, error) + CreateBuilder(ctx context.Context, builderKeychain authn.Keychain, keychain authn.Keychain, fetcher cnb.RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec) (buildapi.BuilderRecord, error) } func NewController( @@ -203,7 +203,7 @@ func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Bui return buildapi.BuilderRecord{}, errors.Errorf("stack %s is not ready", clusterStack.Name) } - keychain, err := c.KeychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{ + builderKeychain, err := c.KeychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{ ServiceAccount: builder.Spec.ServiceAccount(), Namespace: builder.Namespace, }) @@ -211,9 +211,20 @@ func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Bui return buildapi.BuilderRecord{}, err } + stackKeychain := builderKeychain + if clusterStack.Spec.ServiceAccountRef != nil { + stackKeychain, err = c.KeychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{ + ServiceAccount: clusterStack.Spec.ServiceAccountRef.Name, + Namespace: clusterStack.Spec.ServiceAccountRef.Namespace, + }) + if err != nil { + return buildapi.BuilderRecord{}, err + } + } + fetcher := cnb.NewRemoteBuildpackFetcher(c.KeychainFactory, clusterStore, buildpacks, clusterBuildpacks) - buildRecord, err := c.BuilderCreator.CreateBuilder(ctx, keychain, fetcher, clusterStack, builder.Spec.BuilderSpec) + buildRecord, err := c.BuilderCreator.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, clusterStack, builder.Spec.BuilderSpec) if err != nil { return buildapi.BuilderRecord{}, err } diff --git a/pkg/reconciler/builder/builder_test.go b/pkg/reconciler/builder/builder_test.go index 500063926..4d87cdefa 100644 --- a/pkg/reconciler/builder/builder_test.go +++ b/pkg/reconciler/builder/builder_test.go @@ -85,6 +85,12 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Kind: "ClusterStack", APIVersion: "kpack.io/v1alpha2", }, + Spec: buildapi.ClusterStackSpec{ + ServiceAccountRef: &corev1.ObjectReference{ + Name: "some-service-account", + Namespace: testNamespace, + }, + }, Status: buildapi.ClusterStackStatus{ Status: corev1alpha1.Status{ ObservedGeneration: 0, @@ -247,11 +253,12 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { }) assert.Equal(t, []testhelpers.CreateBuilderArgs{{ - Context: context.Background(), - Keychain: ®istryfakes.FakeKeychain{}, - Fetcher: expectedFetcher, - ClusterStack: clusterStack, - BuilderSpec: builder.Spec.BuilderSpec, + Context: context.Background(), + BuilderKeychain: ®istryfakes.FakeKeychain{}, + StackKeychain: ®istryfakes.FakeKeychain{}, + Fetcher: expectedFetcher, + ClusterStack: clusterStack, + BuilderSpec: builder.Spec.BuilderSpec, }}, builderCreator.CreateBuilderCalls) }) diff --git a/pkg/reconciler/clusterbuilder/clusterbuilder.go b/pkg/reconciler/clusterbuilder/clusterbuilder.go index d22f6839e..fc3564acd 100644 --- a/pkg/reconciler/clusterbuilder/clusterbuilder.go +++ b/pkg/reconciler/clusterbuilder/clusterbuilder.go @@ -34,7 +34,7 @@ const ( ) type BuilderCreator interface { - CreateBuilder(ctx context.Context, keychain authn.Keychain, fetcher cnb.RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec) (buildapi.BuilderRecord, error) + CreateBuilder(ctx context.Context, builderKeychain authn.Keychain, stackKeychain authn.Keychain, fetcher cnb.RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec) (buildapi.BuilderRecord, error) } func NewController( @@ -188,7 +188,7 @@ func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Clu return buildapi.BuilderRecord{}, errors.Errorf("stack %s is not ready", clusterStack.Name) } - keychain, err := c.KeychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{ + builderKeychain, err := c.KeychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{ ServiceAccount: builder.Spec.ServiceAccountRef.Name, Namespace: builder.Spec.ServiceAccountRef.Namespace, }) @@ -196,9 +196,20 @@ func (c *Reconciler) reconcileBuilder(ctx context.Context, builder *buildapi.Clu return buildapi.BuilderRecord{}, err } + stackKeychain := builderKeychain + if clusterStack.Spec.ServiceAccountRef != nil { + stackKeychain, err = c.KeychainFactory.KeychainForSecretRef(ctx, registry.SecretRef{ + ServiceAccount: clusterStack.Spec.ServiceAccountRef.Name, + Namespace: clusterStack.Spec.ServiceAccountRef.Namespace, + }) + if err != nil { + return buildapi.BuilderRecord{}, err + } + } + fetcher := cnb.NewRemoteBuildpackFetcher(c.KeychainFactory, clusterStore, nil, clusterBuildpacks) - buildRecord, err := c.BuilderCreator.CreateBuilder(ctx, keychain, fetcher, clusterStack, builder.Spec.BuilderSpec) + buildRecord, err := c.BuilderCreator.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, clusterStack, builder.Spec.BuilderSpec) if err != nil { return buildapi.BuilderRecord{}, err } diff --git a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go index 7bada2c00..e15afa58b 100644 --- a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go +++ b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go @@ -83,6 +83,12 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Kind: "ClusterStack", APIVersion: "kpack.io/v1alpha2", }, + Spec: buildapi.ClusterStackSpec{ + ServiceAccountRef: &corev1.ObjectReference{ + Namespace: "some-sa-namespace", + Name: "some-sa-name", + }, + }, Status: buildapi.ClusterStackStatus{ Status: corev1alpha1.Status{ ObservedGeneration: 0, @@ -243,11 +249,12 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { }) assert.Equal(t, []testhelpers.CreateBuilderArgs{{ - Context: context.Background(), - Keychain: ®istryfakes.FakeKeychain{}, - Fetcher: expectedFetcher, - ClusterStack: clusterStack, - BuilderSpec: builder.Spec.BuilderSpec, + Context: context.Background(), + BuilderKeychain: ®istryfakes.FakeKeychain{}, + StackKeychain: ®istryfakes.FakeKeychain{}, + Fetcher: expectedFetcher, + ClusterStack: clusterStack, + BuilderSpec: builder.Spec.BuilderSpec, }}, builderCreator.CreateBuilderCalls) }) diff --git a/pkg/reconciler/testhelpers/fake_builder_creator.go b/pkg/reconciler/testhelpers/fake_builder_creator.go index a44bc7321..65623f5cb 100644 --- a/pkg/reconciler/testhelpers/fake_builder_creator.go +++ b/pkg/reconciler/testhelpers/fake_builder_creator.go @@ -19,20 +19,22 @@ type FakeBuilderCreator struct { } type CreateBuilderArgs struct { - Context context.Context - Keychain authn.Keychain - Fetcher cnb.RemoteBuildpackFetcher - ClusterStack *buildapi.ClusterStack - BuilderSpec buildapi.BuilderSpec + Context context.Context + BuilderKeychain authn.Keychain + StackKeychain authn.Keychain + Fetcher cnb.RemoteBuildpackFetcher + ClusterStack *buildapi.ClusterStack + BuilderSpec buildapi.BuilderSpec } -func (f *FakeBuilderCreator) CreateBuilder(ctx context.Context, keychain authn.Keychain, fetcher cnb.RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, builder buildapi.BuilderSpec) (buildapi.BuilderRecord, error) { +func (f *FakeBuilderCreator) CreateBuilder(ctx context.Context, builderKeychain authn.Keychain, stackKeychain authn.Keychain, fetcher cnb.RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec) (buildapi.BuilderRecord, error) { f.CreateBuilderCalls = append(f.CreateBuilderCalls, CreateBuilderArgs{ - Context: ctx, - Keychain: keychain, - Fetcher: fetcher, - ClusterStack: clusterStack, - BuilderSpec: builder, + Context: ctx, + BuilderKeychain: builderKeychain, + StackKeychain: stackKeychain, + Fetcher: fetcher, + ClusterStack: clusterStack, + BuilderSpec: spec, }) return f.Record, f.CreateErr