Skip to content

Preserve v2 connection Secret reference across pipeline steps #299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fabioaraujopt wants to merge 1 commit into
base: main
Choose a base branch
from
+174 −15
Open

Preserve v2 connection Secret reference across pipeline steps #299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions connection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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")
}
Expand All @@ -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 {
Expand All @@ -146,23 +146,41 @@ 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()
if xrRef != nil {
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(),
Expand Down
63 changes: 60 additions & 3 deletions connection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)
}
Expand Down
14 changes: 12 additions & 2 deletions fn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
}
}
Comment on lines +276 to +281
Copy link
Collaborator

@phisco phisco Mar 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we get an empty namespace by mistake by a previous step? should we guard here for name != "" && namespace != ""?


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
Expand All @@ -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())
Expand Down
74 changes: 74 additions & 0 deletions fn_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
Loading