diff --git a/pkg/controller/build/build_controller.go b/pkg/controller/build/build_controller.go index 836701a69..f5ef7a8d2 100644 --- a/pkg/controller/build/build_controller.go +++ b/pkg/controller/build/build_controller.go @@ -1,5 +1,5 @@ // Copyright The Shipwright Contributors -// +// // SPDX-License-Identifier: Apache-2.0 package build @@ -7,8 +7,8 @@ package build import ( "context" "fmt" - "reflect" + "strings" "github.com/pkg/errors" build "github.com/shipwright-io/build/pkg/apis/build/v1alpha1" @@ -145,9 +145,20 @@ func (r *ReconcileBuild) Reconcile(request reconcile.Request) (reconcile.Result, b.Status.Registered = corev1.ConditionFalse b.Status.Reason = succeedStatus - // Validate if the spec.output.secretref exist in the namespace + // Validate if the referenced secrets exist in the namespace + var secretNames []string if b.Spec.Output.SecretRef != nil && b.Spec.Output.SecretRef.Name != "" { - if err := r.validateOutputSecret(ctx, b.Spec.Output.SecretRef.Name, b.Namespace); err != nil { + secretNames = append(secretNames, b.Spec.Output.SecretRef.Name) + } + if b.Spec.Source.SecretRef != nil && b.Spec.Source.SecretRef.Name != "" { + secretNames = append(secretNames, b.Spec.Source.SecretRef.Name) + } + if b.Spec.BuilderImage != nil && b.Spec.BuilderImage.SecretRef != nil && b.Spec.BuilderImage.SecretRef.Name != "" { + secretNames = append(secretNames, b.Spec.BuilderImage.SecretRef.Name) + } + + if len(secretNames) > 0 { + if err := r.validateSecrets(ctx, secretNames, b.Namespace); err != nil { b.Status.Reason = err.Error() updateErr := r.client.Status().Update(ctx, b) return reconcile.Result{}, fmt.Errorf("errors: %v %v", err, updateErr) @@ -261,7 +272,7 @@ func (r *ReconcileBuild) validateClusterBuildStrategy(ctx context.Context, n str return nil } -func (r *ReconcileBuild) validateOutputSecret(ctx context.Context, n string, ns string) error { +func (r *ReconcileBuild) validateSecrets(ctx context.Context, secretNames []string, ns string) error { list := &corev1.SecretList{} if err := r.client.List( @@ -278,14 +289,26 @@ func (r *ReconcileBuild) validateOutputSecret(ctx context.Context, n string, ns return errors.Errorf("there are no secrets in namespace %s", ns) } - if len(list.Items) > 0 { - for _, secret := range list.Items { - if secret.Name == n { - return nil - } + var lookUp = map[string]bool{} + for _, secretName := range secretNames { + lookUp[secretName] = false + } + for _, secret := range list.Items { + lookUp[secret.Name] = true + } + var missingSecrets []string + for name, found := range lookUp { + if !found { + missingSecrets = append(missingSecrets, name) } - return fmt.Errorf("secret %s does not exist", n) } + + if len(missingSecrets) > 1 { + return fmt.Errorf("secrets %s do not exist", strings.Join(missingSecrets, ", ")) + } else if len(missingSecrets) > 0 { + return fmt.Errorf("secret %s does not exist", missingSecrets[0]) + } + return nil } diff --git a/pkg/controller/build/build_controller_test.go b/pkg/controller/build/build_controller_test.go index 5e3d31e1f..7183ba35e 100644 --- a/pkg/controller/build/build_controller_test.go +++ b/pkg/controller/build/build_controller_test.go @@ -1,5 +1,5 @@ // Copyright The Shipwright Contributors -// +// // SPDX-License-Identifier: Apache-2.0 package build_test @@ -76,8 +76,134 @@ var _ = Describe("Reconcile Build", func() { }) Describe("Reconcile", func() { + Context("when source secret is specified", func() { + It("fails when the secret does not exist", func() { + buildSample.Spec.Source.SecretRef = &corev1.LocalObjectReference{ + Name: "non-existing", + } + buildSample.Spec.Output.SecretRef = nil + + // Fake some client LIST calls and ensure we populate all + // different resources we could get during reconciliation + client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error { + switch object := object.(type) { + case *corev1.SecretList: + list := ctl.FakeSecretList() + list.DeepCopyInto(object) + case *build.ClusterBuildStrategyList: + list := ctl.ClusterBuildStrategyList(buildStrategyName) + list.DeepCopyInto(object) + } + return nil + }) + + statusCall := ctl.StubFunc(corev1.ConditionFalse, "secret non-existing does not exist") + statusWriter.UpdateCalls(statusCall) + + _, err := reconciler.Reconcile(request) + Expect(err).To(HaveOccurred()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + Expect(err.Error()).To(ContainSubstring("secret non-existing does not exist")) + }) + + It("succeeds when the secret exists", func() { + buildSample.Spec.Source.SecretRef = &corev1.LocalObjectReference{ + Name: "existing", + } + buildSample.Spec.Output.SecretRef = nil + + // Fake some client LIST calls and ensure we populate all + // different resources we could get during reconciliation + client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error { + switch object := object.(type) { + case *corev1.SecretList: + list := ctl.SecretList("existing") + list.DeepCopyInto(object) + case *build.ClusterBuildStrategyList: + list := ctl.ClusterBuildStrategyList(buildStrategyName) + list.DeepCopyInto(object) + } + return nil + }) + + statusCall := ctl.StubFunc(corev1.ConditionTrue, "Succeeded") + statusWriter.UpdateCalls(statusCall) + + result, err := reconciler.Reconcile(request) + Expect(err).ToNot(HaveOccurred()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + Expect(reconcile.Result{}).To(Equal(result)) + }) + }) + + Context("when builder image secret is specified", func() { + It("fails when the secret does not exist", func() { + buildSample.Spec.BuilderImage = &build.Image{ + ImageURL: "busybox", + SecretRef: &corev1.LocalObjectReference{ + Name: "non-existing", + }, + } + buildSample.Spec.Output.SecretRef = nil + + // Fake some client LIST calls and ensure we populate all + // different resources we could get during reconciliation + client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error { + switch object := object.(type) { + case *corev1.SecretList: + list := ctl.FakeSecretList() + list.DeepCopyInto(object) + case *build.ClusterBuildStrategyList: + list := ctl.ClusterBuildStrategyList(buildStrategyName) + list.DeepCopyInto(object) + } + return nil + }) + + statusCall := ctl.StubFunc(corev1.ConditionFalse, "secret non-existing does not exist") + statusWriter.UpdateCalls(statusCall) + + _, err := reconciler.Reconcile(request) + Expect(err).To(HaveOccurred()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + Expect(err.Error()).To(ContainSubstring("secret non-existing does not exist")) + }) + + It("succeeds when the secret exists", func() { + buildSample.Spec.BuilderImage = &build.Image{ + ImageURL: "busybox", + SecretRef: &corev1.LocalObjectReference{ + Name: "existing", + }, + } + buildSample.Spec.Output.SecretRef = nil + + // Fake some client LIST calls and ensure we populate all + // different resources we could get during reconciliation + client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error { + switch object := object.(type) { + case *corev1.SecretList: + list := ctl.SecretList("existing") + list.DeepCopyInto(object) + case *build.ClusterBuildStrategyList: + list := ctl.ClusterBuildStrategyList(buildStrategyName) + list.DeepCopyInto(object) + } + return nil + }) + + statusCall := ctl.StubFunc(corev1.ConditionTrue, "Succeeded") + statusWriter.UpdateCalls(statusCall) + + result, err := reconciler.Reconcile(request) + Expect(err).ToNot(HaveOccurred()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + Expect(reconcile.Result{}).To(Equal(result)) + }) + }) + Context("when spec output registry secret is specified", func() { - It("fails when the secret does not exists", func() { + It("fails when the secret does not exist", func() { // Fake some client LIST calls and ensure we populate all // different resources we could get during reconciliation @@ -145,9 +271,41 @@ var _ = Describe("Reconcile Build", func() { Expect(err).To(HaveOccurred()) Expect(statusWriter.UpdateCallCount()).To(Equal(1)) Expect(err.Error()).To(ContainSubstring(fmt.Sprintf("there are no secrets in namespace %s", namespace))) + }) + }) + + Context("when source secret and output secret are specified", func() { + It("fails when both secrets do not exist", func() { + buildSample.Spec.Source.SecretRef = &corev1.LocalObjectReference{ + Name: "non-existing-source", + } + buildSample.Spec.Output.SecretRef = &corev1.LocalObjectReference{ + Name: "non-existing-output", + } + + // Fake some client LIST calls and ensure we populate all + // different resources we could get during reconciliation + client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error { + switch object := object.(type) { + case *corev1.SecretList: + list := ctl.FakeSecretList() + list.DeepCopyInto(object) + case *build.ClusterBuildStrategyList: + list := ctl.ClusterBuildStrategyList(buildStrategyName) + list.DeepCopyInto(object) + } + return nil + }) + _, err := reconciler.Reconcile(request) + Expect(err).To(HaveOccurred()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + Expect(err.Error()).To(ContainSubstring("do not exist")) + Expect(err.Error()).To(ContainSubstring("non-existing-source")) + Expect(err.Error()).To(ContainSubstring("non-existing-output")) }) }) + Context("when spec strategy ClusterBuildStrategy is specified", func() { It("fails when the strategy does not exists", func() {