Validate all referenced secrets (#389)

pkg/controller/build/build_controller.go

@@ -1,14 +1,14 @@
 // Copyright The Shipwright Contributors
-// 
+//
 // SPDX-License-Identifier: Apache-2.0
 
 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
 }
 

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() {