diff --git a/docs/development/authentication.md b/docs/development/authentication.md index a3b198a3bb..256c16ce1c 100644 --- a/docs/development/authentication.md +++ b/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: @@ -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: @@ -144,7 +116,6 @@ kubectl --namespace create secret docker-registry \ --docker-password= \ --docker-email=me@here.com -kubectl --namespace annotate secrets 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. diff --git a/pkg/apis/build/v1alpha1/build_types.go b/pkg/apis/build/v1alpha1/build_types.go index e08dc94819..7ccee7517c 100644 --- a/pkg/apis/build/v1alpha1/build_types.go +++ b/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 diff --git a/pkg/apis/build/v1beta1/build_types.go b/pkg/apis/build/v1beta1/build_types.go index 5806f37521..9868b43a52 100644 --- a/pkg/apis/build/v1beta1/build_types.go +++ b/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 diff --git a/pkg/reconciler/build/controller.go b/pkg/reconciler/build/controller.go index 5e29e6b288..0f2700787e 100644 --- a/pkg/reconciler/build/controller.go +++ b/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 + }))) } diff --git a/test/integration/build_to_git_test.go b/test/integration/build_to_git_test.go index a79624bcd8..5303450025 100644 --- a/test/integration/build_to_git_test.go +++ b/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()) diff --git a/test/integration/build_to_secrets_test.go b/test/integration/build_to_secrets_test.go index ced38482e2..bc45b11c8c 100644 --- a/test/integration/build_to_secrets_test.go +++ b/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()) diff --git a/test/integration/buildruns_to_sa_test.go b/test/integration/buildruns_to_sa_test.go index 5f30ffa077..42ed19667f 100644 --- a/test/integration/buildruns_to_sa_test.go +++ b/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()) diff --git a/test/v1alpha1_samples/catalog.go b/test/v1alpha1_samples/catalog.go index 6df145562f..7c2d87e533 100644 --- a/test/v1alpha1_samples/catalog.go +++ b/test/v1alpha1_samples/catalog.go @@ -30,17 +30,6 @@ import ( // Catalog allows you to access helper functions type Catalog struct{} -// SecretWithAnnotation gives you a secret with build annotation -func (c *Catalog) SecretWithAnnotation(name string, ns string) *corev1.Secret { - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - Annotations: map[string]string{buildapialpha.AnnotationBuildRefSecret: "true"}, - }, - } -} - // SecretWithoutAnnotation gives you a secret without build annotation func (c *Catalog) SecretWithoutAnnotation(name string, ns string) *corev1.Secret { return &corev1.Secret{ diff --git a/test/v1beta1_samples/catalog.go b/test/v1beta1_samples/catalog.go index 7148cdc662..ddc5ab7d94 100644 --- a/test/v1beta1_samples/catalog.go +++ b/test/v1beta1_samples/catalog.go @@ -30,17 +30,6 @@ import ( // Catalog allows you to access helper functions type Catalog struct{} -// SecretWithAnnotation gives you a secret with build annotation -func (c *Catalog) SecretWithAnnotation(name string, ns string) *corev1.Secret { - return &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: ns, - Annotations: map[string]string{buildapi.AnnotationBuildRefSecret: "true"}, - }, - } -} - // SecretWithoutAnnotation gives you a secret without build annotation func (c *Catalog) SecretWithoutAnnotation(name string, ns string) *corev1.Secret { return &corev1.Secret{