FIX: Allow builds to reconcile even if the secret doesn't have build.shipwright.io/referenced.secret=true annotation

docs/development/authentication.md

@@ -9,7 +9,7 @@ SPDX-License-Identifier: Apache-2.0
 The following document provides an introduction around the different authentication methods that can take place during an image build when using the Build controller.
 
 - [Overview](#overview)
-- [Build Secrets Annotation](#build-secrets-annotation)
+
 - [Authentication for Git](#authentication-for-git)
   - [SSH authentication](#ssh-authentication)
   - [Basic authentication](#basic-authentication)
@@ -23,30 +23,6 @@ The following document provides an introduction around the different authenticat
 
 There are two places where users might need to define authentication when building images. Authentication to a container registry is the most common one, but also users might have the need to define authentications for pulling source-code from Git. Overall, the authentication is done via the definition of [secrets](https://kubernetes.io/docs/concepts/configuration/secret/) in which the required sensitive data will be stored.
 
-## Build Secrets Annotation
-
-Users need to add an annotation `build.shipwright.io/referenced.secret: "true"` to a build secret so that build controller can decide to take a reconcile action when a secret event (`create`, `update` and `delete`) happens. Below is a secret example with build annotation:
-
-```yaml
-apiVersion: v1
-data:
-  .dockerconfigjson: xxxxx
-kind: Secret
-metadata:
-  annotations:
-    build.shipwright.io/referenced.secret: "true"
-  name: secret-docker
-type: kubernetes.io/dockerconfigjson
-```
-
-This annotation will help us filter secrets which are not referenced on a Build instance. That means if a secret doesn't have this annotation, then although event happens on this secret, Build controller will not reconcile. Being able to reconcile on secrets events allow the Build controller to re-trigger validations on the Build configuration, allowing users to understand if a dependency is missing.
-
-If you are using `kubectl` command create secrets, then you can first create build secret using `kubectl create secret` command and annotate this secret using `kubectl annotate secrets`. Below is an example:
-
-```sh
-kubectl -n ${namespace} create secret docker-registry example-secret --docker-server=${docker-server} --docker-username="${username}" --docker-password="${password}" --docker-email=me@here.com
-kubectl -n ${namespace} annotate secrets example-secret build.shipwright.io/referenced.secret='true'
-```
 
 ## Authentication for Git
 
@@ -66,8 +42,6 @@ apiVersion: v1
 kind: Secret
 metadata:
   name: secret-git-ssh-auth
-  annotations:
-    build.shipwright.io/referenced.secret: "true"
 type: kubernetes.io/ssh-auth
 data:
   ssh-privatekey: <base64 <~/.ssh/id_rsa>
@@ -88,8 +62,6 @@ apiVersion: v1
 kind: Secret
 metadata:
   name: secret-git-basic-auth
-  annotations:
-    build.shipwright.io/referenced.secret: "true"
 type: kubernetes.io/basic-auth
 stringData:
   username: <cleartext username>
@@ -144,7 +116,6 @@ kubectl --namespace <YOUR_NAMESPACE> create secret docker-registry <CONTAINER_RE
   --docker-username=<USERNAME> \
   --docker-password=<PASSWORD> \
   --docker-email=me@here.com
-kubectl --namespace <YOUR_NAMESPACE> annotate secrets <CONTAINER_REGISTRY_SECRET_NAME> build.shipwright.io/referenced.secret='true'
 ```
 
 _Notes:_ When generating a secret to access docker hub, the `REGISTRY_HOST` value should be `https://index.docker.io/v1/`, the username is the Docker ID.

pkg/apis/build/v1alpha1/build_types.go

@@ -101,6 +101,8 @@ const (
 
 	// AnnotationBuildRefSecret is an annotation that tells the Build Controller to reconcile on
 	// events of the secret only if is referenced by a Build in the same namespace
+	//
+	// Deprecated: this annotation is no longer required and has no effect.
 	AnnotationBuildRefSecret = BuildDomain + "/referenced.secret"
 
 	// AnnotationBuildVerifyRepository tells the Build Controller to check a remote repository. If the annotation is not set

pkg/apis/build/v1beta1/build_types.go

@@ -126,6 +126,8 @@ const (
 
 	// AnnotationBuildRefSecret is an annotation that tells the Build Controller to reconcile on
 	// events of the secret only if is referenced by a Build in the same namespace
+	//
+	// Deprecated: this annotation is no longer required and has no effect.
 	AnnotationBuildRefSecret = BuildDomain + "/referenced.secret"
 
 	// AnnotationBuildVerifyRepository tells the Build Controller to check a remote repository. If the annotation is not set

pkg/reconciler/build/controller.go

@@ -123,40 +123,6 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxCo
 		return err
 	}
 
-	preSecret := predicate.TypedFuncs[*corev1.Secret]{
-		// Only filter events where the secret have the Build specific annotation
-		CreateFunc: func(e event.TypedCreateEvent[*corev1.Secret]) bool {
-			objectAnnotations := e.Object.GetAnnotations()
-			if _, ok := buildCredentialsAnnotationExist(objectAnnotations); ok {
-				return true
-			}
-			return false
-		},
-
-		// Only filter events where the secret have the Build specific annotation,
-		// but only if the Build specific annotation changed
-		UpdateFunc: func(e event.TypedUpdateEvent[*corev1.Secret]) bool {
-			oldAnnotations := e.ObjectOld.GetAnnotations()
-			newAnnotations := e.ObjectNew.GetAnnotations()
-
-			if _, oldBuildKey := buildCredentialsAnnotationExist(oldAnnotations); !oldBuildKey {
-				if _, newBuildKey := buildCredentialsAnnotationExist(newAnnotations); newBuildKey {
-					return true
-				}
-			}
-			return false
-		},
-
-		// Only filter events where the secret have the Build specific annotation
-		DeleteFunc: func(e event.TypedDeleteEvent[*corev1.Secret]) bool {
-			objectAnnotations := e.Object.GetAnnotations()
-			if _, ok := buildCredentialsAnnotationExist(objectAnnotations); ok {
-				return true
-			}
-			return false
-		},
-	}
-
 	return c.Watch(source.Kind(mgr.GetCache(), &corev1.Secret{}, handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, secret *corev1.Secret) []reconcile.Request {
 		buildList := &buildapi.BuildList{}
 
@@ -176,20 +142,12 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxCo
 		// any Build in the same namespaces
 
 		reconcileList := []reconcile.Request{}
-		flagReconcile := false
 
 		for _, build := range buildList.Items {
-			if build.GetSourceCredentials() != nil && *build.GetSourceCredentials() == secret.Name {
-				flagReconcile = true
-			}
+			// Check if this specific Build references the Secret in source or output
+			if (build.GetSourceCredentials() != nil && *build.GetSourceCredentials() == secret.Name) ||
+				(build.Spec.Output.PushSecret != nil && *build.Spec.Output.PushSecret == secret.Name) {
 
-			if build.Spec.Output.PushSecret != nil {
-				if *build.Spec.Output.PushSecret == secret.Name {
-					flagReconcile = true
-				}
-			}
-
-			if flagReconcile {
 				reconcileList = append(reconcileList, reconcile.Request{
 					NamespacedName: types.NamespacedName{
 						Name:      build.Name,
@@ -199,12 +157,5 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxCo
 			}
 		}
 		return reconcileList
-	}), preSecret))
-}
-
-func buildCredentialsAnnotationExist(annotation map[string]string) (string, bool) {
-	if val, ok := annotation[buildapi.AnnotationBuildRefSecret]; ok {
-		return val, true
-	}
-	return "", false
+	})))
 }

test/integration/build_to_git_test.go

@@ -272,7 +272,7 @@ var _ = Describe("Integration tests Build and referenced Source url", func() {
 				},
 			}
 
-			sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace)
+			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace)
 			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
 
 			Expect(tb.CreateBuild(buildObject)).To(BeNil())

test/integration/build_to_secrets_test.go

@@ -36,7 +36,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 		Expect(err).To(BeNil())
 	})
 
-	Context("when a build reference a secret with annotations for the spec output", func() {
+	Context("when a build reference a secret for the spec output", func() {
 		It("should validate the Build after secret deletion", func() {
 
 			// populate Build related vars
@@ -48,7 +48,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 			)
 			Expect(err).To(BeNil())
 
-			sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
+			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
 
 			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
 
@@ -90,128 +90,20 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SpecOutputSecretRefNotFound))
 			Expect(*buildObject.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *buildObject.Spec.Output.PushSecret)))
 
-			sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
-
-			// generate resources
-			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
-
-			// assert that the validation happened one more time
-			buildObject, err = tb.GetBuildTillRegistration(buildName, corev1.ConditionTrue)
-			Expect(err).To(BeNil())
-			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionTrue))
-			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SucceedStatus))
-		})
-	})
-
-	Context("when a build reference a secret without annotations for the spec output", func() {
-		It("should not validate the Build after a secret deletion", func() {
-
-			// populate Build related vars
-			buildName := BUILD + tb.Namespace
-			buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy(
-				buildName,
-				STRATEGY+tb.Namespace,
-				[]byte(test.BuildWithOutputRefSecret),
-			)
-			Expect(err).To(BeNil())
-
 			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
 
 			// generate resources
 			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
-			Expect(tb.CreateBuild(buildObject)).To(BeNil())
-
-			// wait until the Build finish the validation
-			buildObject, err := tb.GetBuildTillValidation(buildName)
-			Expect(err).To(BeNil())
-			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionTrue))
-			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SucceedStatus))
-
-			// delete a secret
-			Expect(tb.DeleteSecret(*buildObject.Spec.Output.PushSecret)).To(BeNil())
 
 			// assert that the validation happened one more time
-			buildObject, err = tb.GetBuild(buildName)
-			Expect(err).To(BeNil())
-			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionTrue))
-		})
-
-		It("should not validate when a missing secret is recreated without annotation", func() {
-			// populate Build related vars
-			buildName := BUILD + tb.Namespace
-			buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy(
-				buildName,
-				STRATEGY+tb.Namespace,
-				[]byte(test.BuildCBSMinimalWithFakeSecret),
-			)
-			Expect(err).To(BeNil())
-
-			Expect(tb.CreateBuild(buildObject)).To(BeNil())
-
-			// wait until the Build finish the validation
-			buildObject, err := tb.GetBuildTillValidation(buildName)
-			Expect(err).To(BeNil())
-			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse))
-			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SpecOutputSecretRefNotFound))
-			Expect(*buildObject.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *buildObject.Spec.Output.PushSecret)))
-
-			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
-
-			// generate resources
-			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
-
-			// // assert that the validation happened one more time
-			buildObject, err = tb.GetBuildTillRegistration(buildName, corev1.ConditionFalse)
-			Expect(err).To(BeNil())
-			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse))
-			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SpecOutputSecretRefNotFound))
-			Expect(*buildObject.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *buildObject.Spec.Output.PushSecret)))
-		})
-
-		It("should validate when a missing secret is recreated with annotation", func() {
-			// populate Build related vars
-			buildName := BUILD + tb.Namespace
-			buildObject, err = tb.Catalog.LoadBuildWithNameAndStrategy(
-				buildName,
-				STRATEGY+tb.Namespace,
-				[]byte(test.BuildCBSMinimalWithFakeSecret),
-			)
-			Expect(err).To(BeNil())
-
-			Expect(tb.CreateBuild(buildObject)).To(BeNil())
-
-			// wait until the Build finish the validation
-			buildObject, err := tb.GetBuildTillValidation(buildName)
-			Expect(err).To(BeNil())
-			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse))
-			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SpecOutputSecretRefNotFound))
-			Expect(*buildObject.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", "fake-secret")))
-
-			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
-
-			// generate resources
-			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
-			// validate build status again
-			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse))
-			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SpecOutputSecretRefNotFound))
-			Expect(*buildObject.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", "fake-secret")))
-
-			// we modify the annotation so automatic delete does not take place
-			data := []byte(fmt.Sprintf(`{"metadata":{"annotations":{"%s":"true"}}}`, buildapi.AnnotationBuildRefSecret))
-
-			_, err = tb.PatchSecret(*buildObject.Spec.Output.PushSecret, data)
-			Expect(err).To(BeNil())
-
-			// // assert that the validation happened one more time
 			buildObject, err = tb.GetBuildTillRegistration(buildName, corev1.ConditionTrue)
 			Expect(err).To(BeNil())
 			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionTrue))
 			Expect(*buildObject.Status.Reason).To(Equal(buildapi.SucceedStatus))
-			Expect(*buildObject.Status.Message).To(Equal("all validations succeeded"))
 		})
 	})
 
-	Context("when a build reference a secret with annotations for the spec source", func() {
+	Context("when a build reference a secret for the spec source", func() {
 		It("should validate the Build after secret deletion", func() {
 
 			// populate Build related vars
@@ -223,7 +115,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 			)
 			Expect(err).To(BeNil())
 
-			sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace)
+			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace)
 
 			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
 
@@ -266,7 +158,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 			// Status reason sometimes returns message "there are no secrets in namespace..."
 			// Expect(buildObject.Status.Reason).To(Equal(fmt.Sprintf("secret %s does not exist", buildObject.Spec.Source.Credentials.Name)))
 
-			sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace)
+			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace)
 
 			// generate resources
 			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
@@ -279,7 +171,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 		})
 	})
 
-	Context("when multiple builds reference a secret with annotations for the spec.source", func() {
+	Context("when multiple builds reference a secret for the spec.source", func() {
 		It("should validate the Builds after secret deletion", func() {
 
 			// populate Build related vars
@@ -300,7 +192,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 			)
 			Expect(err).To(BeNil())
 
-			specSourceSecret := tb.Catalog.SecretWithAnnotation(*firstBuildObject.Spec.Source.Git.CloneSecret, firstBuildObject.Namespace)
+			specSourceSecret := tb.Catalog.SecretWithoutAnnotation(*firstBuildObject.Spec.Source.Git.CloneSecret, firstBuildObject.Namespace)
 
 			Expect(tb.CreateSecret(specSourceSecret)).To(BeNil())
 
@@ -366,7 +258,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() {
 			Expect(err).To(BeNil())
 			Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse))
 
-			specSourceSecret := tb.Catalog.SecretWithAnnotation(*firstBuildObject.Spec.Source.Git.CloneSecret, firstBuildObject.Namespace)
+			specSourceSecret := tb.Catalog.SecretWithoutAnnotation(*firstBuildObject.Spec.Source.Git.CloneSecret, firstBuildObject.Namespace)
 
 			// generate resources
 			Expect(tb.CreateSecret(specSourceSecret)).To(BeNil())

test/integration/buildruns_to_sa_test.go

@@ -82,7 +82,7 @@ var _ = Describe("Integration tests BuildRuns and Service-accounts", func() {
 				return false
 			}
 
-			sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
+			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
 
 			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())
 
@@ -130,7 +130,7 @@ var _ = Describe("Integration tests BuildRuns and Service-accounts", func() {
 				return false
 			}
 
-			sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
+			sampleSecret := tb.Catalog.SecretWithoutAnnotation(*buildObject.Spec.Output.PushSecret, buildObject.Namespace)
 
 			Expect(tb.CreateSecret(sampleSecret)).To(BeNil())