feat: support labels/annotations on connection secrets

pkg/reconciler/managed/api.go

@@ -106,25 +106,7 @@ func (a *APISecretPublisher) PublishConnection(ctx context.Context, o resource.C
 	s := resource.ConnectionSecretFor(o, resource.MustGetKind(o, a.typer))
 	s.Data = c
 
-	err := a.secret.Apply(ctx, s,
-		resource.ConnectionSecretMustBeControllableBy(o.GetUID()),
-		resource.AllowUpdateIf(func(current, desired runtime.Object) bool {
-			// We consider the update to be a no-op and don't allow it if the
-			// current and existing secret data are identical.
-			//nolint:forcetypeassert // Will always be a secret.
-			return !cmp.Equal(current.(*corev1.Secret).Data, desired.(*corev1.Secret).Data, cmpopts.EquateEmpty())
-		}),
-	)
-	if resource.IsNotAllowed(err) {
-		// The update was not allowed because it was a no-op.
-		return false, nil
-	}
-
-	if err != nil {
-		return false, errors.Wrap(err, errCreateOrUpdateSecret)
-	}
-
-	return true, nil
+	return publishConnectionSecret(ctx, a.secret, s, o.GetUID(), false)
 }
 
 // UnpublishConnection is no-op since PublishConnection only creates resources
@@ -164,35 +146,48 @@ func (a *APILocalSecretPublisher) PublishConnection(ctx context.Context, o resou
 	s := resource.LocalConnectionSecretFor(o, resource.MustGetKind(o, a.typer))
 	s.Data = c
 
-	err := a.secret.Apply(ctx, s,
-		resource.ConnectionSecretMustBeControllableBy(o.GetUID()),
+	return publishConnectionSecret(ctx, a.secret, s, o.GetUID(), o.GetWriteConnectionSecretToReference().Metadata != nil)
+}
+
+// UnpublishConnection is no-op since PublishConnection only creates resources
+// that will be garbage collected by Kubernetes when the managed resource is
+// deleted.
+func (a *APILocalSecretPublisher) UnpublishConnection(_ context.Context, _ resource.LocalConnectionSecretOwner, _ ConnectionDetails) error {
+	return nil
+}
+
+func connectionSecretNeedsUpdate(current, desired *corev1.Secret, manageMetadata bool) bool {
+	if !cmp.Equal(current.Data, desired.Data, cmpopts.EquateEmpty()) {
+		return true
+	}
+
+	if !manageMetadata {
+		return false
+	}
+
+	return !cmp.Equal(current.Labels, desired.Labels, cmpopts.EquateEmpty()) ||
+		!cmp.Equal(current.Annotations, desired.Annotations, cmpopts.EquateEmpty())
+}
+
+// publishConnectionSecret applies the desired Secret via the given applicator,
+// returning whether the secret was published (i.e. changed) and any error.
+func publishConnectionSecret(ctx context.Context, applicator resource.Applicator, s *corev1.Secret, uid types.UID, manageMetadata bool) (bool, error) {
+	err := applicator.Apply(ctx, s,
+		resource.ConnectionSecretMustBeControllableBy(uid),
 		resource.AllowUpdateIf(func(current, desired runtime.Object) bool {
-			// We consider the update to be a no-op and don't allow it if the
-			// current and existing secret data are identical.
 			//nolint:forcetypeassert // Will always be a secret.
-			// NOTE(erhancagirici): cmp package is not recommended for production use
-			return !cmp.Equal(current.(*corev1.Secret).Data, desired.(*corev1.Secret).Data, cmpopts.EquateEmpty())
+			return connectionSecretNeedsUpdate(current.(*corev1.Secret), desired.(*corev1.Secret), manageMetadata)
 		}),
 	)
 	if resource.IsNotAllowed(err) {
-		// The update was not allowed because it was a no-op.
 		return false, nil
 	}
-
 	if err != nil {
 		return false, errors.Wrap(err, errCreateOrUpdateSecret)
 	}
-
 	return true, nil
 }
 
-// UnpublishConnection is no-op since PublishConnection only creates resources
-// that will be garbage collected by Kubernetes when the managed resource is
-// deleted.
-func (a *APILocalSecretPublisher) UnpublishConnection(_ context.Context, _ resource.LocalConnectionSecretOwner, _ ConnectionDetails) error {
-	return nil
-}
-
 // An APISimpleReferenceResolver resolves references from one managed resource
 // to others by calling the referencing resource's ResolveReferences method, if
 // any.

pkg/reconciler/managed/api_test.go

@@ -253,6 +253,10 @@ func TestAPILocalSecretPublisher(t *testing.T) {
 	mg := &fake.ModernManaged{
 		LocalConnectionSecretWriterTo: fake.LocalConnectionSecretWriterTo{Ref: &xpv2.LocalSecretReference{
 			Name: "coolsecret",
+			Metadata: &xpv2.SecretReferenceMetadata{
+				Labels:      map[string]string{"desired": "label"},
+				Annotations: map[string]string{"desired": "annotation"},
+			},
 		}},
 	}
 
@@ -328,6 +332,105 @@ func TestAPILocalSecretPublisher(t *testing.T) {
 				err:       nil,
 			},
 		},
+		"MetadataChanged": {
+			reason: "Metadata-only changes should publish an updated local secret",
+			fields: fields{
+				secret: resource.ApplyFn(func(ctx context.Context, o client.Object, ao ...resource.ApplyOption) error {
+					current := resource.LocalConnectionSecretFor(mg, fake.GVK(mg))
+					current.Data = cd
+					current.Labels = map[string]string{"stale": "label"}
+					current.Annotations = map[string]string{"stale": "annotation"}
+
+					for _, fn := range ao {
+						if err := fn(ctx, current, o); err != nil {
+							return err
+						}
+					}
+
+					return nil
+				}),
+				typer: fake.SchemeWith(&fake.ModernManaged{}),
+			},
+			args: args{
+				ctx: context.Background(),
+				mg:  mg,
+				c:   cd,
+			},
+			want: want{published: true},
+		},
+		"MetadataOmittedPreservesExisting": {
+			reason: "Omitting metadata should preserve existing local secret metadata and remain a no-op when data matches",
+			fields: fields{
+				secret: resource.ApplyFn(func(ctx context.Context, o client.Object, ao ...resource.ApplyOption) error {
+					withoutMetadata := mg.DeepCopyObject().(*fake.ModernManaged)
+					withoutMetadata.LocalConnectionSecretWriterTo.Ref = &xpv2.LocalSecretReference{Name: "coolsecret"}
+					want := resource.LocalConnectionSecretFor(withoutMetadata, fake.GVK(withoutMetadata))
+					want.Data = cd
+					want.Labels = map[string]string{"keep": "label"}
+					want.Annotations = map[string]string{"keep": "annotation"}
+
+					for _, fn := range ao {
+						if err := fn(ctx, want, o); err != nil {
+							return err
+						}
+					}
+
+					return nil
+				}),
+				typer: fake.SchemeWith(&fake.ModernManaged{}),
+			},
+			args: args{
+				ctx: context.Background(),
+				mg: &fake.ModernManaged{
+					LocalConnectionSecretWriterTo: fake.LocalConnectionSecretWriterTo{Ref: &xpv2.LocalSecretReference{Name: "coolsecret"}},
+				},
+				c: cd,
+			},
+			want: want{
+				published: false,
+				err:       nil,
+			},
+		},
+		"LabelsOnlyMetadataClearsAnnotations": {
+			reason: "Metadata with only labels specified should clear existing annotations",
+			fields: fields{
+				secret: resource.ApplyFn(func(ctx context.Context, o client.Object, ao ...resource.ApplyOption) error {
+					labelsOnly := &fake.ModernManaged{
+						LocalConnectionSecretWriterTo: fake.LocalConnectionSecretWriterTo{Ref: &xpv2.LocalSecretReference{
+							Name: "coolsecret",
+							Metadata: &xpv2.SecretReferenceMetadata{
+								Labels: map[string]string{"desired": "label"},
+							},
+						}},
+					}
+					current := resource.LocalConnectionSecretFor(labelsOnly, fake.GVK(labelsOnly))
+					current.Data = cd
+					current.Annotations = map[string]string{"stale": "annotation"}
+
+					for _, fn := range ao {
+						if err := fn(ctx, current, o); err != nil {
+							return err
+						}
+					}
+
+					return nil
+				}),
+				typer: fake.SchemeWith(&fake.ModernManaged{}),
+			},
+			args: args{
+				ctx: context.Background(),
+				mg: &fake.ModernManaged{
+					LocalConnectionSecretWriterTo: fake.LocalConnectionSecretWriterTo{Ref: &xpv2.LocalSecretReference{
+						Name: "coolsecret",
+						Metadata: &xpv2.SecretReferenceMetadata{
+							Labels: map[string]string{"desired": "label"},
+						},
+					}},
+				},
+				c: cd,
+			},
+			want: want{published: true},
+		},
 		"Success": {
 			reason: "A successful application of the connection secret should result in no error",
 			fields: fields{

pkg/resource/resource.go

@@ -19,6 +19,7 @@ package resource
 import (
 	"context"
 	"fmt"
+	"maps"
 	"sort"
 	"strings"
 
@@ -94,10 +95,28 @@ type LocalConnectionSecretOwner interface {
 // LocalConnectionSecretFor creates a connection secret in the namespace of the
 // supplied LocalConnectionSecretOwner, assumed to be of the supplied kind.
 func LocalConnectionSecretFor(o LocalConnectionSecretOwner, kind schema.GroupVersionKind) *corev1.Secret {
+	ref := o.GetWriteConnectionSecretToReference()
+
+	var labels, annotations map[string]string
+	if ref.Metadata != nil {
+		if ref.Metadata.Labels != nil {
+			labels = maps.Clone(ref.Metadata.Labels)
+		} else {
+			labels = map[string]string{}
+		}
+		if ref.Metadata.Annotations != nil {
+			annotations = maps.Clone(ref.Metadata.Annotations)
+		} else {
+			annotations = map[string]string{}
+		}
+	}
+
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace:       o.GetNamespace(),
-			Name:            o.GetWriteConnectionSecretToReference().Name,
+			Name:            ref.Name,
+			Labels:          labels,
+			Annotations:     annotations,
 			OwnerReferences: []metav1.OwnerReference{meta.AsController(meta.TypedReferenceTo(o, kind))},
 		},
 		Type: SecretTypeConnection,
@@ -110,10 +129,12 @@ func LocalConnectionSecretFor(o LocalConnectionSecretOwner, kind schema.GroupVer
 // written to 'default' namespace if the ConnectionSecretOwner does not specify
 // a namespace.
 func ConnectionSecretFor(o ConnectionSecretOwner, kind schema.GroupVersionKind) *corev1.Secret {
+	ref := o.GetWriteConnectionSecretToReference()
+
 	return &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
-			Namespace:       o.GetWriteConnectionSecretToReference().Namespace,
-			Name:            o.GetWriteConnectionSecretToReference().Name,
+			Namespace:       ref.Namespace,
+			Name:            ref.Name,
 			OwnerReferences: []metav1.OwnerReference{meta.AsController(meta.TypedReferenceTo(o, kind))},
 		},
 		Type: SecretTypeConnection,

pkg/resource/resource_test.go

@@ -78,14 +78,94 @@ func TestLocalConnectionSecretFor(t *testing.T) {
 						Name:      name,
 						UID:       uid,
 					},
-					Ref: &xpv2.LocalSecretReference{Name: secretName},
+					Ref: &xpv2.LocalSecretReference{
+						Name: secretName,
+						Metadata: &xpv2.SecretReferenceMetadata{
+							Labels:      map[string]string{"l": "v"},
+							Annotations: map[string]string{"a": "v"},
+						},
+					},
 				},
 				kind: MockOwnerGVK,
 			},
 			want: &corev1.Secret{
 				ObjectMeta: metav1.ObjectMeta{
 					Namespace: namespace,
 					Name:      secretName,
+					Labels:    map[string]string{"l": "v"},
+					Annotations: map[string]string{
+						"a": "v",
+					},
+					OwnerReferences: []metav1.OwnerReference{{
+						APIVersion:         MockOwnerGVK.GroupVersion().String(),
+						Kind:               MockOwnerGVK.Kind,
+						Name:               name,
+						UID:                uid,
+						Controller:         &controller,
+						BlockOwnerDeletion: &controller,
+					}},
+				},
+				Type: SecretTypeConnection,
+				Data: map[string][]byte{},
+			},
+		},
+		"LabelsOnlyCoercesAnnotationsToEmpty": {
+			args: args{
+				o: &fake.MockLocalConnectionSecretOwner{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: namespace,
+						Name:      name,
+						UID:       uid,
+					},
+					Ref: &xpv2.LocalSecretReference{
+						Name: secretName,
+						Metadata: &xpv2.SecretReferenceMetadata{
+							Labels: map[string]string{"l": "v"},
+						},
+					},
+				},
+				kind: MockOwnerGVK,
+			},
+			want: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   namespace,
+					Name:        secretName,
+					Labels:      map[string]string{"l": "v"},
+					Annotations: map[string]string{},
+					OwnerReferences: []metav1.OwnerReference{{
+						APIVersion:         MockOwnerGVK.GroupVersion().String(),
+						Kind:               MockOwnerGVK.Kind,
+						Name:               name,
+						UID:                uid,
+						Controller:         &controller,
+						BlockOwnerDeletion: &controller,
+					}},
+				},
+				Type: SecretTypeConnection,
+				Data: map[string][]byte{},
+			},
+		},
+		"EmptyMetadataCoercesBothToEmpty": {
+			args: args{
+				o: &fake.MockLocalConnectionSecretOwner{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: namespace,
+						Name:      name,
+						UID:       uid,
+					},
+					Ref: &xpv2.LocalSecretReference{
+						Name:     secretName,
+						Metadata: &xpv2.SecretReferenceMetadata{},
+					},
+				},
+				kind: MockOwnerGVK,
+			},
+			want: &corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace:   namespace,
+					Name:        secretName,
+					Labels:      map[string]string{},
+					Annotations: map[string]string{},
 					OwnerReferences: []metav1.OwnerReference{{
 						APIVersion:         MockOwnerGVK.GroupVersion().String(),
 						Kind:               MockOwnerGVK.Kind,
@@ -132,7 +212,10 @@ func TestConnectionSecretFor(t *testing.T) {
 						Name:      name,
 						UID:       uid,
 					},
-					WriterTo: &xpv2.SecretReference{Namespace: namespace, Name: secretName},
+					WriterTo: &xpv2.SecretReference{
+						Namespace: namespace,
+						Name:      secretName,
+					},
 				},
 				kind: MockOwnerGVK,
 			},