feat: support labels/annotations on connection secrets for modern MRs and claims

Add metadata.labels and metadata.annotations to writeConnectionSecretToRef
for modern (namespaced) managed resources and claims via LocalSecretReference.

- LocalConnectionSecretFor clones labels/annotations from ref.Metadata
- connectionSecretNeedsUpdate detects metadata drift when manageMetadata=true
- Omitted metadata preserves existing secret labels/annotations (merge patch)
- Set metadata declaratively reconciles (merge patch replaces full map)
- Shared publishConnectionSecret helper deduplicates both publishers
- XRD schema includes metadata in claim writeConnectionSecretToRef validation

Closes: #1008
Signed-off-by: Florian Fl Bauer <florian.fl.bauer@deutschebahn.com>

Author: derbauer97

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,65 @@ 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,
+			},
+		},
 		"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,20 @@ 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 {
+		labels = maps.Clone(ref.Metadata.Labels)
+		annotations = maps.Clone(ref.Metadata.Annotations)
+	}
+
 	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 +121,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,24 @@ 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,
@@ -132,7 +142,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,
 			},

pkg/xcrd/crd_test.go

@@ -2893,6 +2893,25 @@ func TestForCompositeResourceClaim(t *testing.T) {
 													Required: []string{"name"},
 													Properties: map[string]extv1.JSONSchemaProps{
 														"name": {Type: "string"},
+														"metadata": {
+															Type: "object",
+															Properties: map[string]extv1.JSONSchemaProps{
+																"labels": {
+																	Type: "object",
+																	AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+																		Allows: true,
+																		Schema: &extv1.JSONSchemaProps{Type: "string"},
+																	},
+																},
+																"annotations": {
+																	Type: "object",
+																	AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+																		Allows: true,
+																		Schema: &extv1.JSONSchemaProps{Type: "string"},
+																	},
+																},
+															},
+														},
 													},
 												},
 											},
@@ -3151,6 +3170,25 @@ func TestForCompositeResourceClaim(t *testing.T) {
 													Required: []string{"name"},
 													Properties: map[string]extv1.JSONSchemaProps{
 														"name": {Type: "string"},
+														"metadata": {
+															Type: "object",
+															Properties: map[string]extv1.JSONSchemaProps{
+																"labels": {
+																	Type: "object",
+																	AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+																		Allows: true,
+																		Schema: &extv1.JSONSchemaProps{Type: "string"},
+																	},
+																},
+																"annotations": {
+																	Type: "object",
+																	AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+																		Allows: true,
+																		Schema: &extv1.JSONSchemaProps{Type: "string"},
+																	},
+																},
+															},
+														},
 													},
 												},
 											},
@@ -3426,14 +3464,33 @@ func TestForCompositeResourceClaimEmptyXrd(t *testing.T) {
 												"name":       {Type: "string"},
 											},
 										},
-										"writeConnectionSecretToRef": {
-											Type:     "object",
-											Required: []string{"name"},
-											Properties: map[string]extv1.JSONSchemaProps{
-												"name": {Type: "string"},
+									"writeConnectionSecretToRef": {
+										Type:     "object",
+										Required: []string{"name"},
+										Properties: map[string]extv1.JSONSchemaProps{
+											"name": {Type: "string"},
+											"metadata": {
+												Type: "object",
+												Properties: map[string]extv1.JSONSchemaProps{
+													"labels": {
+														Type: "object",
+														AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+															Allows: true,
+															Schema: &extv1.JSONSchemaProps{Type: "string"},
+														},
+													},
+													"annotations": {
+														Type: "object",
+														AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+															Allows: true,
+															Schema: &extv1.JSONSchemaProps{Type: "string"},
+														},
+													},
+												},
 											},
 										},
 									},
+									},
 								},
 								"status": {
 									Type:        "object",

pkg/xcrd/schemas.go

@@ -283,6 +283,25 @@ func CompositeResourceClaimSpecProps(defaultPol *xpv2.CompositeDeletePolicy) map
 			Required: []string{"name"},
 			Properties: map[string]extv1.JSONSchemaProps{
 				"name": {Type: "string"},
+				"metadata": {
+					Type: "object",
+					Properties: map[string]extv1.JSONSchemaProps{
+						"labels": {
+							Type: "object",
+							AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+								Allows: true,
+								Schema: &extv1.JSONSchemaProps{Type: "string"},
+							},
+						},
+						"annotations": {
+							Type: "object",
+							AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
+								Allows: true,
+								Schema: &extv1.JSONSchemaProps{Type: "string"},
+							},
+						},
+					},
+				},
 			},
 		},
 	}