From 4873c59364dbe82a05d83078b1cf324158efec41 Mon Sep 17 00:00:00 2001 From: Mauren Berti Date: Wed, 12 Jul 2023 17:19:35 -0400 Subject: [PATCH] Changes from code review. * Refactor the usage of the Kubernetes client/fetcher for better separation of concerns. * Add new test cases for failure modes and multiple signatures. * Use the same value as cosign CLI for timeout. --- cmd/controller/main.go | 8 +- pkg/cnb/create_builder_test.go | 36 +- pkg/cosign/image_signer.go | 26 +- pkg/cosign/image_signer_test.go | 4 +- pkg/reconciler/builder/builder.go | 4 +- pkg/reconciler/builder/builder_test.go | 15 +- .../clusterbuilder/clusterbuilder.go | 4 +- .../clusterbuilder/clusterbuilder_test.go | 16 +- .../testhelpers/fake_builder_creator.go | 4 +- pkg/secret/fetcher.go | 4 + pkg/secret/secretfakes/fake_fetcher.go | 27 + test/cosign_e2e_test.go | 505 +++++++++++++++++- test/execute_build_test.go | 20 + 13 files changed, 616 insertions(+), 57 deletions(-) create mode 100644 pkg/secret/secretfakes/fake_fetcher.go diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 7389364c4..8e439f2dc 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -9,6 +9,8 @@ import ( "os" "time" + "github.com/pivotal/kpack/pkg/secret" + "github.com/pivotal/kpack/pkg/cosign" "github.com/sigstore/cosign/v2/cmd/cosign/cli/sign" ociremote "github.com/sigstore/cosign/v2/pkg/oci/remote" @@ -200,12 +202,14 @@ func main() { ImageSigner: cosign.NewImageSigner(sign.SignCmd, ociremote.SignatureTag), } + secretFetcher := &secret.Fetcher{Client: k8sClient} + buildController := build.NewController(ctx, options, k8sClient, buildInformer, podInformer, metadataRetriever, buildpodGenerator, keychainFactory, *injectedSidecarSupport) imageController := image.NewController(ctx, options, k8sClient, imageInformer, buildInformer, duckBuilderInformer, sourceResolverInformer, pvcInformer, *enablePriorityClasses) sourceResolverController := sourceresolver.NewController(ctx, options, sourceResolverInformer, gitResolver, blobResolver, registryResolver) - builderController, builderResync := builder.NewController(ctx, options, k8sClient, builderInformer, builderCreator, keychainFactory, clusterStoreInformer, buildpackInformer, clusterBuildpackInformer, clusterStackInformer) + builderController, builderResync := builder.NewController(ctx, options, builderInformer, builderCreator, keychainFactory, clusterStoreInformer, buildpackInformer, clusterBuildpackInformer, clusterStackInformer, secretFetcher) buildpackController := buildpack.NewController(ctx, options, keychainFactory, buildpackInformer, remoteStoreReader) - clusterBuilderController, clusterBuilderResync := clusterbuilder.NewController(ctx, options, k8sClient, clusterBuilderInformer, builderCreator, keychainFactory, clusterStoreInformer, clusterBuildpackInformer, clusterStackInformer) + clusterBuilderController, clusterBuilderResync := clusterbuilder.NewController(ctx, options, clusterBuilderInformer, builderCreator, keychainFactory, clusterStoreInformer, clusterBuildpackInformer, clusterStackInformer, secretFetcher) clusterBuildpackController := clusterbuildpack.NewController(ctx, options, keychainFactory, clusterBuildpackInformer, remoteStoreReader) clusterStoreController := clusterstore.NewController(ctx, options, keychainFactory, clusterStoreInformer, remoteStoreReader) clusterStackController := clusterstack.NewController(ctx, options, keychainFactory, clusterStackInformer, remoteStackReader) diff --git a/pkg/cnb/create_builder_test.go b/pkg/cnb/create_builder_test.go index 225e8c8f7..a35d16e4d 100644 --- a/pkg/cnb/create_builder_test.go +++ b/pkg/cnb/create_builder_test.go @@ -178,7 +178,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { KeychainFactory: keychainFactory, LifecycleProvider: lifecycleProvider, ImageSigner: &fakeBuilderSigner{ - signBuilder: func(ctx context.Context, s string, secrets []corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) { + signBuilder: func(ctx context.Context, s string, secrets []*corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) { // no-op return nil, nil }, @@ -352,7 +352,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, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.NoError(t, err) assert.Len(t, builderRecord.Buildpacks, 4) @@ -642,11 +642,11 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }) it("creates images deterministically ", func() { - original, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + original, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.NoError(t, err) for i := 1; i <= 50; i++ { - other, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + other, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.NoError(t, err) @@ -677,7 +677,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, } - _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.stack@v4: stack io.buildpacks.stacks.some-stack is not supported") }) @@ -701,7 +701,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.mixin@v4: stack missing mixin(s): something-missing-mixin, something-missing-mixin2") }) @@ -746,7 +746,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.Nil(t, err) }) @@ -771,7 +771,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, builderKeychain, nil, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + _, err := subject.CreateBuilder(ctx, builderKeychain, nil, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) 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") }) @@ -794,7 +794,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.EqualError(t, err, "validating buildpack io.buildpack.unsupported.buildpack.api@v4: unsupported buildpack api: 0.1, expecting: 0.2, 0.3") }) @@ -837,7 +837,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }}, }} - _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.NoError(t, err) }) }) @@ -864,14 +864,14 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, } - _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) 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") }) }) when("signing a builder image", func() { it("does not populate the signature paths when no secrets were present", func() { - builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{}) + builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{}) require.NoError(t, err) require.NotNil(t, builderRecord) require.Empty(t, builderRecord.SignaturePaths) @@ -879,7 +879,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { it("returns an error if signing fails", func() { subject.ImageSigner = &fakeBuilderSigner{ - signBuilder: func(ctx context.Context, s string, secrets []corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) { + signBuilder: func(ctx context.Context, s string, secrets []*corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) { return nil, fmt.Errorf("failed to sign builder") }, } @@ -891,7 +891,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, } - _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{fakeSecret}) + _, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{&fakeSecret}) require.Error(t, err) }) @@ -904,7 +904,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { } subject.ImageSigner = &fakeBuilderSigner{ - signBuilder: func(ctx context.Context, s string, secrets []corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) { + signBuilder: func(ctx context.Context, s string, secrets []*corev1.Secret, keychain authn.Keychain) ([]buildapi.CosignSignature, error) { return []buildapi.CosignSignature{ { SigningSecret: fmt.Sprintf("k8s://%s/%s", fakeSecret.Namespace, fakeSecret.Name), @@ -914,7 +914,7 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { }, } - builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []corev1.Secret{fakeSecret}) + builderRecord, err := subject.CreateBuilder(ctx, builderKeychain, stackKeychain, fetcher, stack, clusterBuilderSpec, []*corev1.Secret{&fakeSecret}) require.NoError(t, err) require.NotNil(t, builderRecord) require.NotEmpty(t, builderRecord.SignaturePaths) @@ -924,10 +924,10 @@ func testCreateBuilderOs(os string, t *testing.T, when spec.G, it spec.S) { } type fakeBuilderSigner struct { - signBuilder func(context.Context, string, []corev1.Secret, authn.Keychain) ([]buildapi.CosignSignature, error) + signBuilder func(context.Context, string, []*corev1.Secret, authn.Keychain) ([]buildapi.CosignSignature, error) } -func (s *fakeBuilderSigner) SignBuilder(ctx context.Context, imageReference string, signingSecrets []corev1.Secret, builderKeychain authn.Keychain) ([]buildapi.CosignSignature, error) { +func (s *fakeBuilderSigner) SignBuilder(ctx context.Context, imageReference string, signingSecrets []*corev1.Secret, builderKeychain authn.Keychain) ([]buildapi.CosignSignature, error) { return s.signBuilder(ctx, imageReference, signingSecrets, builderKeychain) } diff --git a/pkg/cosign/image_signer.go b/pkg/cosign/image_signer.go index 6d2f821fd..d2eb96d6c 100644 --- a/pkg/cosign/image_signer.go +++ b/pkg/cosign/image_signer.go @@ -14,12 +14,12 @@ import ( "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" "github.com/pkg/errors" - "github.com/sigstore/cosign/v2/cmd/cosign/cli/options" + cosignoptions "github.com/sigstore/cosign/v2/cmd/cosign/cli/options" cosignremote "github.com/sigstore/cosign/v2/pkg/oci/remote" corev1 "k8s.io/api/core/v1" ) -type SignFunc func(*options.RootOptions, options.KeyOpts, options.SignOptions, []string) error +type SignFunc func(*cosignoptions.RootOptions, cosignoptions.KeyOpts, cosignoptions.SignOptions, []string) error type FetchSignatureFunc func(name.Reference, ...cosignremote.Option) (name.Tag, error) @@ -39,7 +39,7 @@ func NewImageSigner(signFunc SignFunc, fetchSignatureFunc FetchSignatureFunc) *I } } -func (s *ImageSigner) Sign(ro *options.RootOptions, report platform.ExportReport, secretLocation string, annotations, cosignRepositories, cosignDockerMediaTypes map[string]interface{}) error { +func (s *ImageSigner) Sign(ro *cosignoptions.RootOptions, report platform.ExportReport, secretLocation string, annotations, cosignRepositories, cosignDockerMediaTypes map[string]interface{}) error { cosignSecrets, err := findCosignSecrets(secretLocation) if err != nil { return errors.Errorf("no keys found for cosign signing: %v\n", err) @@ -64,11 +64,11 @@ func (s *ImageSigner) Sign(ro *options.RootOptions, report platform.ExportReport return nil } -func (s *ImageSigner) sign(ro *options.RootOptions, refImage, secretLocation, cosignSecret string, annotations, cosignRepositories, cosignDockerMediaTypes map[string]interface{}) error { +func (s *ImageSigner) sign(ro *cosignoptions.RootOptions, refImage, secretLocation, cosignSecret string, annotations, cosignRepositories, cosignDockerMediaTypes map[string]interface{}) error { cosignKeyFile := fmt.Sprintf("%s/%s/cosign.key", secretLocation, cosignSecret) cosignPasswordFile := fmt.Sprintf("%s/%s/cosign.password", secretLocation, cosignSecret) - ko := options.KeyOpts{KeyRef: cosignKeyFile, PassFunc: func(bool) ([]byte, error) { + ko := cosignoptions.KeyOpts{KeyRef: cosignKeyFile, PassFunc: func(bool) ([]byte, error) { content, err := ioutil.ReadFile(cosignPasswordFile) // When password file is not available, default empty password is used if err != nil { @@ -97,9 +97,9 @@ func (s *ImageSigner) sign(ro *options.RootOptions, refImage, secretLocation, co cosignAnnotations = append(cosignAnnotations, fmt.Sprintf("%s=%s", key, value)) } - signOptions := options.SignOptions{ - Registry: options.RegistryOptions{KubernetesKeychain: true}, - AnnotationOptions: options.AnnotationOptions{ + signOptions := cosignoptions.SignOptions{ + Registry: cosignoptions.RegistryOptions{KubernetesKeychain: true}, + AnnotationOptions: cosignoptions.AnnotationOptions{ Annotations: cosignAnnotations, }, Upload: true, @@ -129,7 +129,7 @@ func (s *ImageSigner) SignBuilder( for _, cosignSecret := range cosignSecrets { keyRef := fmt.Sprintf("k8s://%s/%s", cosignSecret.Namespace, cosignSecret.Name) - keyOpts := options.KeyOpts{ + keyOpts := cosignoptions.KeyOpts{ KeyRef: keyRef, PassFunc: func(bool) ([]byte, error) { if password, ok := cosignSecret.Data[cosignutil.SecretDataCosignPassword]; ok { @@ -152,17 +152,17 @@ func (s *ImageSigner) SignBuilder( } } - registryOptions := options.RegistryOptions{KubernetesKeychain: true, Keychain: builderKeychain} + registryOptions := cosignoptions.RegistryOptions{KubernetesKeychain: true, Keychain: builderKeychain} - signOptions := options.SignOptions{ + signOptions := cosignoptions.SignOptions{ Registry: registryOptions, - AnnotationOptions: options.AnnotationOptions{}, + AnnotationOptions: cosignoptions.AnnotationOptions{}, Upload: true, Recursive: false, TlogUpload: false, } - rootOptions := options.RootOptions{} + rootOptions := cosignoptions.RootOptions{Timeout: cosignoptions.DefaultTimeout} if err := s.signFunc( &rootOptions, diff --git a/pkg/cosign/image_signer_test.go b/pkg/cosign/image_signer_test.go index c9ec053a1..0e940eae2 100644 --- a/pkg/cosign/image_signer_test.go +++ b/pkg/cosign/image_signer_test.go @@ -527,7 +527,7 @@ func testImageSigner(t *testing.T, when spec.G, it spec.S) { } fakeSecret := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName, testNamespaceName, "", nil) - cosignCreds := []corev1.Secret{fakeSecret} + cosignCreds := []*corev1.Secret{&fakeSecret} cosignSA := corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: cosignServiceAccountName, @@ -605,7 +605,7 @@ func testImageSigner(t *testing.T, when spec.G, it spec.S) { } fakeSecret := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName, testNamespaceName, "", annotations) - cosignCreds := []corev1.Secret{fakeSecret} + cosignCreds := []*corev1.Secret{&fakeSecret} cosignSA := corev1.ServiceAccount{ ObjectMeta: metav1.ObjectMeta{ Name: cosignServiceAccountName, diff --git a/pkg/reconciler/builder/builder.go b/pkg/reconciler/builder/builder.go index c7842e19f..486bcac8d 100644 --- a/pkg/reconciler/builder/builder.go +++ b/pkg/reconciler/builder/builder.go @@ -50,7 +50,7 @@ func NewController( buildpackInformer buildinformers.BuildpackInformer, clusterBuildpackInformer buildinformers.ClusterBuildpackInformer, clusterStackInformer buildinformers.ClusterStackInformer, - secretFetcher *secret.Fetcher, + secretFetcher secret.IFetcher, ) (*controller.Impl, func()) { c := &Reconciler{ Client: opt.Client, @@ -114,7 +114,7 @@ type Reconciler struct { BuildpackLister buildlisters.BuildpackLister ClusterBuildpackLister buildlisters.ClusterBuildpackLister ClusterStackLister buildlisters.ClusterStackLister - SecretFetcher *secret.Fetcher + SecretFetcher secret.IFetcher } func (c *Reconciler) Reconcile(ctx context.Context, key string) error { diff --git a/pkg/reconciler/builder/builder_test.go b/pkg/reconciler/builder/builder_test.go index 623397d66..2e96346e6 100644 --- a/pkg/reconciler/builder/builder_test.go +++ b/pkg/reconciler/builder/builder_test.go @@ -4,6 +4,8 @@ import ( "errors" "testing" + "github.com/pivotal/kpack/pkg/secret/secretfakes" + k8sfake "k8s.io/client-go/kubernetes/fake" "github.com/sclevine/spec" @@ -44,9 +46,12 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { ) var ( - builderCreator = &testhelpers.FakeBuilderCreator{} - keychainFactory = ®istryfakes.FakeKeychainFactory{} - fakeTracker = &testhelpers.FakeTracker{} + builderCreator = &testhelpers.FakeBuilderCreator{} + keychainFactory = ®istryfakes.FakeKeychainFactory{} + fakeTracker = &testhelpers.FakeTracker{} + fakeSecretFetcher = &secretfakes.FakeFetchSecret{ + FakeSecrets: []*corev1.Secret{}, + } ) rt := testhelpers.ReconcilerTester(t, @@ -64,7 +69,7 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { BuildpackLister: listers.GetBuildpackLister(), ClusterBuildpackLister: listers.GetClusterBuildpackLister(), ClusterStackLister: listers.GetClusterStackLister(), - K8sClient: k8sfakeClient, + SecretFetcher: fakeSecretFetcher, } return &kreconciler.NetworkErrorReconciler{Reconciler: r}, rtesting.ActionRecorderList{fakeClient, k8sfakeClient}, rtesting.EventList{Recorder: record.NewFakeRecorder(10)} }) @@ -290,7 +295,7 @@ func testBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Fetcher: expectedFetcher, ClusterStack: clusterStack, BuilderSpec: builder.Spec.BuilderSpec, - SigningSecrets: []corev1.Secret{}, + SigningSecrets: []*corev1.Secret{}, }}, builderCreator.CreateBuilderCalls) }) diff --git a/pkg/reconciler/clusterbuilder/clusterbuilder.go b/pkg/reconciler/clusterbuilder/clusterbuilder.go index fd1a40841..787e3e095 100644 --- a/pkg/reconciler/clusterbuilder/clusterbuilder.go +++ b/pkg/reconciler/clusterbuilder/clusterbuilder.go @@ -48,7 +48,7 @@ func NewController( clusterStoreInformer buildinformers.ClusterStoreInformer, clusterBuildpackInformer buildinformers.ClusterBuildpackInformer, clusterStackInformer buildinformers.ClusterStackInformer, - secretFetcher *secret.Fetcher, + secretFetcher secret.IFetcher, ) (*controller.Impl, func()) { c := &Reconciler{ Client: opt.Client, @@ -106,7 +106,7 @@ type Reconciler struct { ClusterStoreLister buildlisters.ClusterStoreLister ClusterBuildpackLister buildlisters.ClusterBuildpackLister ClusterStackLister buildlisters.ClusterStackLister - SecretFetcher *secret.Fetcher + SecretFetcher secret.IFetcher } func (c *Reconciler) Reconcile(ctx context.Context, key string) error { diff --git a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go index 974d4ea4b..42f6d48e7 100644 --- a/pkg/reconciler/clusterbuilder/clusterbuilder_test.go +++ b/pkg/reconciler/clusterbuilder/clusterbuilder_test.go @@ -5,7 +5,7 @@ import ( "errors" "testing" - k8sfake "k8s.io/client-go/kubernetes/fake" + "github.com/pivotal/kpack/pkg/secret/secretfakes" "github.com/sclevine/spec" "github.com/stretchr/testify/assert" @@ -43,16 +43,18 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { ) var ( - builderCreator = &testhelpers.FakeBuilderCreator{} - keychainFactory = ®istryfakes.FakeKeychainFactory{} - fakeTracker = &testhelpers.FakeTracker{} + builderCreator = &testhelpers.FakeBuilderCreator{} + keychainFactory = ®istryfakes.FakeKeychainFactory{} + fakeTracker = &testhelpers.FakeTracker{} + fakeSecretFetcher = &secretfakes.FakeFetchSecret{ + FakeSecrets: []*corev1.Secret{}, + } ) rt := testhelpers.ReconcilerTester(t, func(t *testing.T, row *rtesting.TableRow) (reconciler controller.Reconciler, lists rtesting.ActionRecorderList, list rtesting.EventList) { listers := testhelpers.NewListers(row.Objects) fakeClient := fake.NewSimpleClientset(listers.BuildServiceObjects()...) - fakeK8sClient := k8sfake.NewSimpleClientset(listers.GetKubeObjects()...) r := &clusterbuilder.Reconciler{ Client: fakeClient, ClusterBuilderLister: listers.GetClusterBuilderLister(), @@ -62,7 +64,7 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { ClusterStoreLister: listers.GetClusterStoreLister(), ClusterBuildpackLister: listers.GetClusterBuildpackLister(), ClusterStackLister: listers.GetClusterStackLister(), - K8sClient: fakeK8sClient, + SecretFetcher: fakeSecretFetcher, } return &kreconciler.NetworkErrorReconciler{Reconciler: r}, rtesting.ActionRecorderList{fakeClient}, rtesting.EventList{Recorder: record.NewFakeRecorder(10)} }) @@ -280,7 +282,7 @@ func testClusterBuilderReconciler(t *testing.T, when spec.G, it spec.S) { Fetcher: expectedFetcher, ClusterStack: clusterStack, BuilderSpec: builder.Spec.BuilderSpec, - SigningSecrets: []corev1.Secret{}, + SigningSecrets: []*corev1.Secret{}, }}, builderCreator.CreateBuilderCalls) }) diff --git a/pkg/reconciler/testhelpers/fake_builder_creator.go b/pkg/reconciler/testhelpers/fake_builder_creator.go index ed6f272ff..f7a07f53c 100644 --- a/pkg/reconciler/testhelpers/fake_builder_creator.go +++ b/pkg/reconciler/testhelpers/fake_builder_creator.go @@ -25,10 +25,10 @@ type CreateBuilderArgs struct { Fetcher cnb.RemoteBuildpackFetcher ClusterStack *buildapi.ClusterStack BuilderSpec buildapi.BuilderSpec - SigningSecrets []corev1.Secret + SigningSecrets []*corev1.Secret } -func (f *FakeBuilderCreator) CreateBuilder(ctx context.Context, builderKeychain authn.Keychain, stackKeychain authn.Keychain, fetcher cnb.RemoteBuildpackFetcher, clusterStack *buildapi.ClusterStack, spec buildapi.BuilderSpec, signingSecrets []corev1.Secret) (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, signingSecrets []*corev1.Secret) (buildapi.BuilderRecord, error) { f.CreateBuilderCalls = append(f.CreateBuilderCalls, CreateBuilderArgs{ Context: ctx, BuilderKeychain: builderKeychain, diff --git a/pkg/secret/fetcher.go b/pkg/secret/fetcher.go index 62d1afde1..3d6968332 100644 --- a/pkg/secret/fetcher.go +++ b/pkg/secret/fetcher.go @@ -9,6 +9,10 @@ import ( k8sclient "k8s.io/client-go/kubernetes" ) +type IFetcher interface { + SecretsForServiceAccount(context.Context, string, string) ([]*corev1.Secret, error) +} + type Fetcher struct { Client k8sclient.Interface } diff --git a/pkg/secret/secretfakes/fake_fetcher.go b/pkg/secret/secretfakes/fake_fetcher.go new file mode 100644 index 000000000..29656db3b --- /dev/null +++ b/pkg/secret/secretfakes/fake_fetcher.go @@ -0,0 +1,27 @@ +package secretfakes + +import ( + "context" + + corev1 "k8s.io/api/core/v1" +) + +type FakeFetchSecret struct { + FakeSecrets []*corev1.Secret + ShouldError bool + ErrorOut error + + SecretsForServiceAccountFunc func(context.Context, string, string) ([]*corev1.Secret, error) +} + +func (f *FakeFetchSecret) SecretsForServiceAccount(ctx context.Context, serviceAccount, namespace string) ([]*corev1.Secret, error) { + if f.SecretsForServiceAccountFunc != nil { + return f.SecretsForServiceAccount(ctx, serviceAccount, namespace) + } + + if f.ShouldError { + return nil, f.ErrorOut + } + + return f.FakeSecrets, nil +} diff --git a/test/cosign_e2e_test.go b/test/cosign_e2e_test.go index c639e811b..665d9b1e8 100644 --- a/test/cosign_e2e_test.go +++ b/test/cosign_e2e_test.go @@ -6,6 +6,7 @@ import ( "testing" cosigntesting "github.com/pivotal/kpack/pkg/cosign/testing" + cosignutil "github.com/pivotal/kpack/pkg/cosign/util" buildapi "github.com/pivotal/kpack/pkg/apis/build/v1alpha2" corev1alpha1 "github.com/pivotal/kpack/pkg/apis/core/v1alpha1" @@ -29,6 +30,7 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { builderName = "custom-signed-builder" clusterBuilderName = "custom-signed-cluster-builder" cosignSecretName = "cosign-creds" + secretRefFormat = "k8s://%s/%s" ) var ( @@ -280,7 +282,7 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { assert.NotEmpty(t, updatedBuilder.Status.SignaturePaths) assert.NotNil(t, updatedBuilder.Status.SignaturePaths[0]) - err = cosigntesting.Verify(t, fmt.Sprintf("k8s://%s/%s", testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) require.NoError(t, err) }) @@ -396,10 +398,260 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { assert.NotEmpty(t, updatedBuilder.Status.SignaturePaths) assert.NotNil(t, updatedBuilder.Status.SignaturePaths[0]) - err = cosigntesting.Verify(t, fmt.Sprintf("k8s://%s/%s", testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) require.NoError(t, err) }) + it("Generates more than one signature for a Builder image successfully when multiple secrets are present", func() { + const CosignKeyPassword = "password" + const cosignSecretName1 = "cosign-credentials-1" + const cosignSecretName2 = "cosign-credentials-2" + + cosignCredSecret1 := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName1, testNamespace, CosignKeyPassword, nil) + _, err := clients.k8sClient.CoreV1().Secrets(testNamespace).Create(ctx, &cosignCredSecret1, metav1.CreateOptions{}) + require.NoError(t, err) + + cosignCredSecret2 := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName2, testNamespace, CosignKeyPassword, nil) + _, err = clients.k8sClient.CoreV1().Secrets(testNamespace).Create(ctx, &cosignCredSecret2, metav1.CreateOptions{}) + require.NoError(t, err) + + serviceAccount, err := clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Get(ctx, serviceAccountName, metav1.GetOptions{}) + require.NoError(t, err) + + if serviceAccount.Secrets == nil { + serviceAccount.Secrets = make([]corev1.ObjectReference, 0) + } + serviceAccount.Secrets = append(serviceAccount.Secrets, + corev1.ObjectReference{Name: cosignCredSecret1.Name}, + corev1.ObjectReference{Name: cosignCredSecret2.Name}) + + _, err = clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Update(ctx, serviceAccount, metav1.UpdateOptions{}) + require.NoError(t, err) + + builder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Create(ctx, &buildapi.Builder{ + ObjectMeta: metav1.ObjectMeta{ + Name: builderName, + Namespace: testNamespace, + }, + Spec: buildapi.NamespacedBuilderSpec{ + BuilderSpec: buildapi.BuilderSpec{ + Tag: cfg.newImageTag(), + Stack: corev1.ObjectReference{ + Name: clusterStackName, + Kind: "ClusterStack", + }, + Store: corev1.ObjectReference{ + Name: clusterStoreName, + Kind: "ClusterStore", + }, + Order: []buildapi.BuilderOrderEntry{ + { + Group: []buildapi.BuilderBuildpackRef{ + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/nodejs", + }, + }, + }, + }, + }, + { + Group: []buildapi.BuilderBuildpackRef{ + { + ObjectReference: corev1.ObjectReference{ + Name: buildpackName, + Kind: "Buildpack", + }, + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/bellsoft-liberica", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/gradle", + }, + Optional: true, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/syft", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/executable-jar", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/dist-zip", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/spring-boot", + }, + }, + }, + }, + }, + }, + }, + ServiceAccountName: serviceAccountName, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + waitUntilReady(t, ctx, clients, builder) + + updatedBuilder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Get(ctx, builderName, metav1.GetOptions{}) + require.NoError(t, err) + + assert.NotEmpty(t, updatedBuilder.Status.SignaturePaths) + assert.Equal(t, 2, len(updatedBuilder.Status.SignaturePaths)) + assert.NotNil(t, updatedBuilder.Status.SignaturePaths[0]) + assert.NotNil(t, updatedBuilder.Status.SignaturePaths[1]) + + // tag is assigned to a single signature, but both are still verifiable + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName1), updatedBuilder.Status.LatestImage, nil) + require.NoError(t, err) + + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName2), updatedBuilder.Status.LatestImage, nil) + require.NoError(t, err) + }) + + it("Saves a failure in the Builder record when signing fails", func() { + const CosignKeyPassword = "password" + const invalidPassword = "wrong-password" + const expectedErrorMessage = "unable to sign" + + cosignCredSecret := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName, testNamespace, CosignKeyPassword, nil) + cosignCredSecret.Data[cosignutil.SecretDataCosignPassword] = []byte(invalidPassword) + + _, err := clients.k8sClient.CoreV1().Secrets(testNamespace).Create(ctx, &cosignCredSecret, metav1.CreateOptions{}) + require.NoError(t, err) + + serviceAccount, err := clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Get(ctx, serviceAccountName, metav1.GetOptions{}) + require.NoError(t, err) + + if serviceAccount.Secrets == nil { + serviceAccount.Secrets = make([]corev1.ObjectReference, 0) + } + serviceAccount.Secrets = append(serviceAccount.Secrets, + corev1.ObjectReference{Name: cosignCredSecret.Name}) + + _, err = clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Update(ctx, serviceAccount, metav1.UpdateOptions{}) + require.NoError(t, err) + + builder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Create(ctx, &buildapi.Builder{ + ObjectMeta: metav1.ObjectMeta{ + Name: builderName, + Namespace: testNamespace, + }, + Spec: buildapi.NamespacedBuilderSpec{ + BuilderSpec: buildapi.BuilderSpec{ + Tag: cfg.newImageTag(), + Stack: corev1.ObjectReference{ + Name: clusterStackName, + Kind: "ClusterStack", + }, + Store: corev1.ObjectReference{ + Name: clusterStoreName, + Kind: "ClusterStore", + }, + Order: []buildapi.BuilderOrderEntry{ + { + Group: []buildapi.BuilderBuildpackRef{ + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/nodejs", + }, + }, + }, + }, + }, + { + Group: []buildapi.BuilderBuildpackRef{ + { + ObjectReference: corev1.ObjectReference{ + Name: buildpackName, + Kind: "Buildpack", + }, + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/bellsoft-liberica", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/gradle", + }, + Optional: true, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/syft", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/executable-jar", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/dist-zip", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/spring-boot", + }, + }, + }, + }, + }, + }, + }, + ServiceAccountName: serviceAccountName, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + waitUntilFailed(t, ctx, clients, expectedErrorMessage, builder) + + updatedBuilder, err := clients.client.KpackV1alpha2().Builders(testNamespace).Get(ctx, builderName, metav1.GetOptions{}) + require.NoError(t, err) + require.NotNil(t, updatedBuilder.Status) + + readyConditionBuilder := updatedBuilder.Status.GetCondition(corev1alpha1.ConditionReady) + require.False(t, readyConditionBuilder.IsTrue()) + require.Contains(t, readyConditionBuilder.Message, expectedErrorMessage) + }) + it("Signs a ClusterBuilder image successfully when the key is not password-protected", func() { cosignCredSecret := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName, testNamespace, "", nil) _, err := clients.k8sClient.CoreV1().Secrets(testNamespace).Create(ctx, &cosignCredSecret, metav1.CreateOptions{}) @@ -507,7 +759,7 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { assert.NotEmpty(t, updatedBuilder.Status.SignaturePaths) assert.NotNil(t, updatedBuilder.Status.SignaturePaths[0]) - err = cosigntesting.Verify(t, fmt.Sprintf("k8s://%s/%s", testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) require.NoError(t, err) }) @@ -620,7 +872,252 @@ func testSignBuilder(t *testing.T, _ spec.G, it spec.S) { assert.NotEmpty(t, updatedBuilder.Status.SignaturePaths) assert.NotNil(t, updatedBuilder.Status.SignaturePaths[0]) - err = cosigntesting.Verify(t, fmt.Sprintf("k8s://%s/%s", testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName), updatedBuilder.Status.LatestImage, nil) + require.NoError(t, err) + }) + + it("Generates more than one signature for a ClusterBuilder image successfully when multiple secrets are present", func() { + const cosignKeyPassword = "password" + const cosignSecretName1 = "cosign-credentials-1" + const cosignSecretName2 = "cosign-credentials-2" + + cosignCredSecret1 := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName1, testNamespace, cosignKeyPassword, nil) + _, err := clients.k8sClient.CoreV1().Secrets(testNamespace).Create(ctx, &cosignCredSecret1, metav1.CreateOptions{}) + require.NoError(t, err) + + cosignCredSecret2 := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName2, testNamespace, cosignKeyPassword, nil) + _, err = clients.k8sClient.CoreV1().Secrets(testNamespace).Create(ctx, &cosignCredSecret2, metav1.CreateOptions{}) + require.NoError(t, err) + + serviceAccount, err := clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Get(ctx, serviceAccountName, metav1.GetOptions{}) + require.NoError(t, err) + + if serviceAccount.Secrets == nil { + serviceAccount.Secrets = make([]corev1.ObjectReference, 0) + } + serviceAccount.Secrets = append( + serviceAccount.Secrets, + corev1.ObjectReference{Name: cosignCredSecret1.Name}, + corev1.ObjectReference{Name: cosignCredSecret2.Name}) + + _, err = clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Update(ctx, serviceAccount, metav1.UpdateOptions{}) + require.NoError(t, err) + + clusterBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Create(ctx, &buildapi.ClusterBuilder{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterBuilderName, + }, + Spec: buildapi.ClusterBuilderSpec{ + BuilderSpec: buildapi.BuilderSpec{ + Tag: cfg.newImageTag(), + Stack: corev1.ObjectReference{ + Name: clusterStackName, + Kind: "ClusterStack", + }, + Store: corev1.ObjectReference{ + Name: clusterStoreName, + Kind: "ClusterStore", + }, + Order: []buildapi.BuilderOrderEntry{ + { + Group: []buildapi.BuilderBuildpackRef{ + { + ObjectReference: corev1.ObjectReference{ + Name: clusterBuildpackName, + Kind: "ClusterBuildpack", + }, + }, + }, + }, + { + Group: []buildapi.BuilderBuildpackRef{ + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/bellsoft-liberica", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/gradle", + }, + Optional: true, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/syft", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/executable-jar", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/dist-zip", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/spring-boot", + }, + }, + }, + }, + }, + }, + }, + ServiceAccountRef: corev1.ObjectReference{ + Namespace: testNamespace, + Name: serviceAccountName, + }, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + waitUntilReady(t, ctx, clients, clusterBuilder) + + updatedBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Get(ctx, clusterBuilderName, metav1.GetOptions{}) require.NoError(t, err) + + assert.NotEmpty(t, updatedBuilder.Status.SignaturePaths) + assert.Equal(t, 2, len(updatedBuilder.Status.SignaturePaths)) + assert.NotNil(t, updatedBuilder.Status.SignaturePaths[0]) + assert.NotNil(t, updatedBuilder.Status.SignaturePaths[1]) + + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName1), updatedBuilder.Status.LatestImage, nil) + require.NoError(t, err) + + err = cosigntesting.Verify(t, fmt.Sprintf(secretRefFormat, testNamespace, cosignSecretName2), updatedBuilder.Status.LatestImage, nil) + require.NoError(t, err) + }) + + it("Saves a failure in the ClusterBuilder record when signing fails", func() { + const cosignKeyPassword = "password" + const invalidPassword = "wrong-password" + const expectedErrorMessage = "unable to sign" + + cosignCredSecret := cosigntesting.GenerateFakeKeyPair(t, cosignSecretName, testNamespace, cosignKeyPassword, nil) + cosignCredSecret.Data[cosignutil.SecretDataCosignPassword] = []byte(invalidPassword) + + _, err = clients.k8sClient.CoreV1().Secrets(testNamespace).Create(ctx, &cosignCredSecret, metav1.CreateOptions{}) + require.NoError(t, err) + + serviceAccount, err := clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Get(ctx, serviceAccountName, metav1.GetOptions{}) + require.NoError(t, err) + + if serviceAccount.Secrets == nil { + serviceAccount.Secrets = make([]corev1.ObjectReference, 0) + } + serviceAccount.Secrets = append( + serviceAccount.Secrets, + corev1.ObjectReference{Name: cosignCredSecret.Name}) + + _, err = clients.k8sClient.CoreV1().ServiceAccounts(testNamespace).Update(ctx, serviceAccount, metav1.UpdateOptions{}) + require.NoError(t, err) + + clusterBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Create(ctx, &buildapi.ClusterBuilder{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterBuilderName, + }, + Spec: buildapi.ClusterBuilderSpec{ + BuilderSpec: buildapi.BuilderSpec{ + Tag: cfg.newImageTag(), + Stack: corev1.ObjectReference{ + Name: clusterStackName, + Kind: "ClusterStack", + }, + Store: corev1.ObjectReference{ + Name: clusterStoreName, + Kind: "ClusterStore", + }, + Order: []buildapi.BuilderOrderEntry{ + { + Group: []buildapi.BuilderBuildpackRef{ + { + ObjectReference: corev1.ObjectReference{ + Name: clusterBuildpackName, + Kind: "ClusterBuildpack", + }, + }, + }, + }, + { + Group: []buildapi.BuilderBuildpackRef{ + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/bellsoft-liberica", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/gradle", + }, + Optional: true, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/syft", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/executable-jar", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/dist-zip", + }, + }, + }, + { + BuildpackRef: corev1alpha1.BuildpackRef{ + BuildpackInfo: corev1alpha1.BuildpackInfo{ + Id: "paketo-buildpacks/spring-boot", + }, + }, + }, + }, + }, + }, + }, + ServiceAccountRef: corev1.ObjectReference{ + Namespace: testNamespace, + Name: serviceAccountName, + }, + }, + }, metav1.CreateOptions{}) + require.NoError(t, err) + + waitUntilFailed(t, ctx, clients, expectedErrorMessage, clusterBuilder) + + updatedBuilder, err := clients.client.KpackV1alpha2().ClusterBuilders().Get(ctx, clusterBuilderName, metav1.GetOptions{}) + require.NoError(t, err) + require.NotNil(t, updatedBuilder.Status) + + readyConditionBuilder := updatedBuilder.Status.GetCondition(corev1alpha1.ConditionReady) + require.False(t, readyConditionBuilder.IsTrue()) + require.Contains(t, readyConditionBuilder.Message, expectedErrorMessage) }) } diff --git a/test/execute_build_test.go b/test/execute_build_test.go index 372cf6d91..55120506c 100644 --- a/test/execute_build_test.go +++ b/test/execute_build_test.go @@ -658,6 +658,26 @@ func waitUntilReady(t *testing.T, ctx context.Context, clients *clients, objects } } +func waitUntilFailed(t *testing.T, ctx context.Context, clients *clients, expectedMessage string, objects ...kmeta.OwnerRefable) { + for _, ob := range objects { + namespace := ob.GetObjectMeta().GetNamespace() + name := ob.GetObjectMeta().GetName() + gvr, _ := meta.UnsafeGuessKindToResource(ob.GetGroupVersionKind()) + + eventually(t, func() bool { + unstructured, err := clients.dynamicClient.Resource(gvr).Namespace(namespace).Get(ctx, name, metav1.GetOptions{}) + require.NoError(t, err) + + kResource := &duckv1.KResource{} + err = duck.FromUnstructured(unstructured, kResource) + require.NoError(t, err) + + condition := kResource.Status.GetCondition(apis.ConditionReady) + return condition.IsFalse() && "" != condition.Message && strings.Contains(condition.Message, expectedMessage) + }, 1*time.Second, 8*time.Minute) + } +} + func validateImageCreate(t *testing.T, clients *clients, image *buildapi.Image, expectedResources corev1.ResourceRequirements) string { ctx, cancel := context.WithCancel(context.Background()) defer cancel()