From 0c0191c5a2bf09abb8268fcc09eca5c6862a8127 Mon Sep 17 00:00:00 2001 From: fabioaraujopt Date: Thu, 12 Mar 2026 18:56:28 -0700 Subject: [PATCH] Preserve v2 connection secret refs across pipeline steps. Reuse the previously desired connection Secret reference as the base when composing v2 XR connection details in subsequent patch-and-transform steps, preventing accidental namespace clobbering while keeping explicit input and patch overrides intact. Signed-off-by: fabioaraujopt --- connection.go | 38 +++++++++++++++++------- connection_test.go | 63 +++++++++++++++++++++++++++++++++++++-- fn.go | 14 +++++++-- fn_test.go | 74 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 174 insertions(+), 15 deletions(-) diff --git a/connection.go b/connection.go index de071f2..cf077fd 100644 --- a/connection.go +++ b/connection.go @@ -93,7 +93,7 @@ func supportsConnectionDetails(xr *resource.Composite) bool { // composeConnectionSecret creates a Secret composed resource containing the // provided connection details. -func composeConnectionSecret(xr *resource.Composite, details resource.ConnectionDetails, ref *v1beta1.WriteConnectionSecretToRef) (*resource.DesiredComposed, error) { +func composeConnectionSecret(xr *resource.Composite, details resource.ConnectionDetails, ref *v1beta1.WriteConnectionSecretToRef, existingRef *xpv1.SecretReference) (*resource.DesiredComposed, error) { if len(details) == 0 { return nil, nil } @@ -102,7 +102,7 @@ func composeConnectionSecret(xr *resource.Composite, details resource.Connection secret.SetAPIVersion("v1") secret.SetKind("Secret") - secretRef, err := getConnectionSecretRef(xr, ref) + secretRef, err := getConnectionSecretRef(xr, ref, existingRef) if err != nil { return nil, errors.Wrap(err, "cannot generate connection secret reference") } @@ -126,9 +126,9 @@ func composeConnectionSecret(xr *resource.Composite, details resource.Connection // getConnectionSecretRef creates a connection secret reference from the given // XR and input. The patches for the reference will be applied before the // reference is returned. -func getConnectionSecretRef(xr *resource.Composite, input *v1beta1.WriteConnectionSecretToRef) (xpv1.SecretReference, error) { +func getConnectionSecretRef(xr *resource.Composite, input *v1beta1.WriteConnectionSecretToRef, existingRef *xpv1.SecretReference) (xpv1.SecretReference, error) { // Get the base connection secret ref to start with - ref := getBaseConnectionSecretRef(xr, input) + ref := getBaseConnectionSecretRef(xr, input, existingRef) // Apply patches to the base connection secret ref if they've been provided if input != nil && len(input.Patches) > 0 { @@ -146,10 +146,11 @@ func getConnectionSecretRef(xr *resource.Composite, input *v1beta1.WriteConnecti // 1. xr.spec.writeConnectionSecretToRef - this is no longer automatically added // to v2 XR schemas, but the community has been adding it manually, so if // it's present we will use it. -// 2. function input.writeConnectionSecretToRef - if name or namespace is provided -// then the whole ref will be used -// 3. generate the reference from scratch, based on the XR name and namespace -func getBaseConnectionSecretRef(xr *resource.Composite, input *v1beta1.WriteConnectionSecretToRef) xpv1.SecretReference { +// 2. existing desired composed connection secret (from a previous function step) +// 3. function input.writeConnectionSecretToRef - if name/namespace is provided, +// non-empty values override the base ref field-by-field +// 4. generate the reference from scratch, based on the XR name and namespace +func getBaseConnectionSecretRef(xr *resource.Composite, input *v1beta1.WriteConnectionSecretToRef, existingRef *xpv1.SecretReference) xpv1.SecretReference { // Check if XR author manually added writeConnectionSecretToRef to the XR's // schema and just use that if it exists xrRef := xr.Resource.GetWriteConnectionSecretToReference() @@ -157,12 +158,29 @@ func getBaseConnectionSecretRef(xr *resource.Composite, input *v1beta1.WriteConn return *xrRef } - // Use the input values if at least one of name or namespace has been provided + // Reuse an existing desired secret reference from an earlier pipeline step. + // This prevents accidentally dropping a non-empty namespace in subsequent + // patch-and-transform steps that omit writeConnectionSecretToRef values. + if existingRef != nil { + ref := *existingRef + if input != nil { + if input.Name != "" { + ref.Name = input.Name + } + if input.Namespace != "" { + ref.Namespace = input.Namespace + } + } + return ref + } + + // Preserve existing behavior when there is no previously desired secret: + // if at least one input field is provided, use the full input ref as-is. if input != nil && (input.Name != "" || input.Namespace != "") { return xpv1.SecretReference{Name: input.Name, Namespace: input.Namespace} } - // Nothing has been provided, so generate a default name using the name of the XR + // Nothing has been provided, so generate a default name using the name of the XR. return xpv1.SecretReference{ Name: xr.Resource.GetName() + "-connection", Namespace: xr.Resource.GetNamespace(), diff --git a/connection_test.go b/connection_test.go index 7a41940..93ca55f 100644 --- a/connection_test.go +++ b/connection_test.go @@ -143,8 +143,9 @@ func TestExtractConnectionDetails(t *testing.T) { func TestGetConnectionSecretRef(t *testing.T) { type args struct { - xr *resource.Composite - input *v1beta1.WriteConnectionSecretToRef + xr *resource.Composite + input *v1beta1.WriteConnectionSecretToRef + existingRef *xpv1.SecretReference } type want struct { ref xpv1.SecretReference @@ -256,6 +257,62 @@ func TestGetConnectionSecretRef(t *testing.T) { }, }, }, + "ExistingRefProvidedNoInput": { + reason: "Should reuse an existing desired secret reference when no input values are provided", + args: args{ + xr: &resource.Composite{ + Resource: func() *composite.Unstructured { + xr := composite.New() + _ = json.Unmarshal([]byte(`{ + "apiVersion":"example.org/v1", + "kind":"XR", + "metadata":{"name":"my-xr","uid":"test-uid-existing"} + }`), xr) + return xr + }(), + }, + input: nil, + existingRef: &xpv1.SecretReference{ + Name: "existing-secret", + Namespace: "tenant-a", + }, + }, + want: want{ + ref: xpv1.SecretReference{ + Name: "existing-secret", + Namespace: "tenant-a", + }, + }, + }, + "ExistingRefWithPartialInputOverride": { + reason: "Should keep existing namespace when input overrides only the name", + args: args{ + xr: &resource.Composite{ + Resource: func() *composite.Unstructured { + xr := composite.New() + _ = json.Unmarshal([]byte(`{ + "apiVersion":"example.org/v1", + "kind":"XR", + "metadata":{"name":"my-xr","uid":"test-uid-partial"} + }`), xr) + return xr + }(), + }, + input: &v1beta1.WriteConnectionSecretToRef{ + Name: "new-secret-name", + }, + existingRef: &xpv1.SecretReference{ + Name: "existing-secret", + Namespace: "tenant-a", + }, + }, + want: want{ + ref: xpv1.SecretReference{ + Name: "new-secret-name", + Namespace: "tenant-a", + }, + }, + }, "PatchFromCompositeFieldPath": { reason: "Should apply patches to transform the secret name using XR metadata", args: args{ @@ -415,7 +472,7 @@ func TestGetConnectionSecretRef(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - got, err := getConnectionSecretRef(tc.args.xr, tc.args.input) + got, err := getConnectionSecretRef(tc.args.xr, tc.args.input, tc.args.existingRef) if diff := cmp.Diff(tc.want.err, err, cmpopts.EquateErrors()); diff != "" { t.Errorf("\n%s\ngetConnectionSecretRef(...): -want, +got:\n%s", tc.reason, diff) } diff --git a/fn.go b/fn.go index ec678c6..fa856ca 100644 --- a/fn.go +++ b/fn.go @@ -6,6 +6,7 @@ import ( "maps" "github.com/crossplane-contrib/function-patch-and-transform/input/v1beta1" + xpv1 "github.com/crossplane/crossplane-runtime/v2/apis/common/v1" "github.com/crossplane/crossplane-runtime/v2/pkg/errors" "github.com/crossplane/crossplane-runtime/v2/pkg/fieldpath" "github.com/crossplane/crossplane-runtime/v2/pkg/logging" @@ -269,7 +270,17 @@ func (f *Function) RunFunction(ctx context.Context, req *fnv1.RunFunctionRequest // details, we'll transparently create a composed secret resource to store // the connection details and add it to the set of desired resources. if !supportsConnectionDetails(oxr) && len(dxr.ConnectionDetails) > 0 { - connectionSecret, err := composeConnectionSecret(oxr, dxr.ConnectionDetails, input.WriteConnectionSecretToRef) + composedResourceName := resource.Name(fmt.Sprintf("%s-connection-secret", oxr.Resource.GetName())) + + var existingConnectionSecretRef *xpv1.SecretReference + if dcd, ok := desired[composedResourceName]; ok && dcd != nil && dcd.Resource != nil { + existingConnectionSecretRef = &xpv1.SecretReference{ + Name: dcd.Resource.GetName(), + Namespace: dcd.Resource.GetNamespace(), + } + } + + connectionSecret, err := composeConnectionSecret(oxr, dxr.ConnectionDetails, input.WriteConnectionSecretToRef, existingConnectionSecretRef) if err != nil { response.Fatal(rsp, errors.Wrap(err, "cannot compose connection secret")) return rsp, nil @@ -278,7 +289,6 @@ func (f *Function) RunFunction(ctx context.Context, req *fnv1.RunFunctionRequest if connectionSecret != nil { // Add the connection secret as a composed resource with a special name // We use a prefix to avoid conflicts with user-defined resource names - composedResourceName := resource.Name(fmt.Sprintf("%s-connection-secret", oxr.Resource.GetName())) desired[composedResourceName] = connectionSecret log.Debug("Added connection secret to desired composed resources", "composed-resource-name", composedResourceName, "secret-name", connectionSecret.Resource.GetName(), "secret-namespace", connectionSecret.Resource.GetNamespace()) diff --git a/fn_test.go b/fn_test.go index 90ca4b9..4e361dd 100644 --- a/fn_test.go +++ b/fn_test.go @@ -795,6 +795,80 @@ func TestRunFunction(t *testing.T) { }, }, }, + "ExtractCompositeConnectionDetailsV2XRPreservesExistingSecretNamespace": { + reason: "A later patch-and-transform step should preserve an existing desired connection secret namespace when input.writeConnectionSecretToRef is omitted.", + args: args{ + req: &fnv1.RunFunctionRequest{ + Input: resource.MustStructObject(&v1beta1.Resources{ + Resources: []v1beta1.ComposedTemplate{ + { + Name: "cool-resource", + Base: &runtime.RawExtension{Raw: []byte(`{"apiVersion":"example.org/v1","kind":"CD"}`)}, + ConnectionDetails: []v1beta1.ConnectionDetail{ + { + Type: v1beta1.ConnectionDetailTypeFromConnectionSecretKey, + Name: "very", + FromConnectionSecretKey: ptr.To[string]("very"), + }, + }, + }, + }, + }), + Observed: &fnv1.State{ + Composite: &fnv1.Resource{ + // v2 XR without namespace (cluster-scoped style). + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"XR","metadata":{"name":"cool-xr-42"},"spec":{"foo":"bar","crossplane":{}}}`), + }, + Resources: map[string]*fnv1.Resource{ + "cool-resource": { + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","metadata":{"namespace":"default","name":"cool-42"}}`), + ConnectionDetails: map[string][]byte{ + "very": []byte("secret"), + }, + }, + }, + }, + Desired: &fnv1.State{ + Composite: &fnv1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"XR","metadata":{"name":"cool-xr-42"},"spec":{"foo":"bar","crossplane":{}}}`), + ConnectionDetails: map[string][]byte{ + "existing": []byte("supersecretvalue"), + }, + }, + Resources: map[string]*fnv1.Resource{ + "cool-xr-42-connection-secret": { + Ready: fnv1.Ready_READY_TRUE, + Resource: resource.MustStructJSON(`{"apiVersion":"v1","kind":"Secret","metadata":{"name":"cool-conn-secret","namespace":"tenant-a"},"data":{"old":"dmFsdWU="},"type":"connection.crossplane.io/v1alpha1"}`), + }, + }, + }, + }, + }, + want: want{ + rsp: &fnv1.RunFunctionResponse{ + Meta: &fnv1.ResponseMeta{Ttl: durationpb.New(response.DefaultTTL)}, + Desired: &fnv1.State{ + Composite: &fnv1.Resource{ + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"XR","metadata":{"name":"cool-xr-42"},"spec":{"foo":"bar","crossplane":{}}}`), + ConnectionDetails: map[string][]byte{ + "existing": []byte("supersecretvalue"), + "very": []byte("secret"), + }, + }, + Resources: map[string]*fnv1.Resource{ + "cool-resource": { + Resource: resource.MustStructJSON(`{"apiVersion":"example.org/v1","kind":"CD","metadata":{"namespace":"default","name":"cool-42"}}`), + }, + "cool-xr-42-connection-secret": { + Ready: fnv1.Ready_READY_TRUE, + Resource: resource.MustStructJSON(`{"apiVersion":"v1","kind":"Secret","metadata":{"name":"cool-conn-secret","namespace":"tenant-a"},"data":{"existing":"c3VwZXJzZWNyZXR2YWx1ZQ==","very":"c2VjcmV0"},"type":"connection.crossplane.io/v1alpha1"}`), + }, + }, + }, + Context: contextWithEnvironment(nil), + }, + }, + }, "PatchToComposite": { reason: "A basic ToCompositeFieldPath patch should work.", args: args{