fix: for target template parsing for complex matchers (#5735)

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -646,8 +647,28 @@ func (r *Reconciler) reconcileGenericTarget(ctx context.Context, externalSecret
 		}
 	}
 
+	// Check if we need to fetch existing resource first (for Merge and Owner/Orphan policies)
+	var existing *unstructured.Unstructured
+	if externalSecret.Spec.Target.CreationPolicy == esv1.CreatePolicyMerge ||
+		externalSecret.Spec.Target.CreationPolicy == esv1.CreatePolicyOrphan ||
+		externalSecret.Spec.Target.CreationPolicy == esv1.CreatePolicyOwner {
+		var getErr error
+		existing, getErr = r.getGenericResource(ctx, log, externalSecret)
+		if getErr != nil && !apierrors.IsNotFound(getErr) {
+			r.markAsFailed("could not get target resource", getErr, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
+			return ctrl.Result{}, getErr
+		}
+	}
+
+	// For Merge policy with existing resource, pass it to applyTemplateToManifest
+	// so templates are applied to the existing resource instead of creating a new one
+	var baseObj *unstructured.Unstructured
+	if externalSecret.Spec.Target.CreationPolicy == esv1.CreatePolicyMerge && existing != nil {
+		baseObj = existing
+	}
+
 	// render the template for the manifest
-	obj, err := r.applyTemplateToManifest(ctx, externalSecret, dataMap)
+	obj, err := r.applyTemplateToManifest(ctx, externalSecret, dataMap, baseObj)
 	if err != nil {
 		r.markAsFailed("could not apply template to manifest", err, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
 		return ctrl.Result{}, err
@@ -661,12 +682,6 @@ func (r *Reconciler) reconcileGenericTarget(ctx context.Context, externalSecret
 
 	case esv1.CreatePolicyMerge:
 		// for Merge policy, only update if resource exists
-		existing, getErr := r.getGenericResource(ctx, log, externalSecret)
-		if getErr != nil && !apierrors.IsNotFound(getErr) {
-			r.markAsFailed("could not get target resource", getErr, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
-			return ctrl.Result{}, getErr
-		}
-
 		if existing == nil || existing.GetUID() == "" {
 			r.markAsDone(externalSecret, start, log, esv1.ConditionReasonResourceMissing, "resource will not be created due to CreationPolicy=Merge")
 			return r.getRequeueResult(externalSecret), nil
@@ -678,12 +693,6 @@ func (r *Reconciler) reconcileGenericTarget(ctx context.Context, externalSecret
 		// update the existing resource
 		err = r.updateGenericResource(ctx, log, externalSecret, obj)
 	case esv1.CreatePolicyOrphan, esv1.CreatePolicyOwner:
-		existing, getErr := r.getGenericResource(ctx, log, externalSecret)
-		if getErr != nil && !apierrors.IsNotFound(getErr) {
-			r.markAsFailed("could not get target resource", getErr, externalSecret, syncCallsError.With(resourceLabels), esv1.ConditionReasonResourceSyncedError)
-			return ctrl.Result{}, getErr
-		}
-
 		if existing != nil {
 			obj.SetResourceVersion(existing.GetResourceVersion())
 			obj.SetUID(existing.GetUID())

pkg/controllers/externalsecret/externalsecret_controller_manifest.go

@@ -168,16 +168,29 @@ func (r *Reconciler) deleteGenericResource(ctx context.Context, log logr.Logger,
 }
 
 // applyTemplateToManifest renders templates for generic resources and returns an unstructured object.
-func (r *Reconciler) applyTemplateToManifest(ctx context.Context, es *esv1.ExternalSecret, dataMap map[string][]byte) (*unstructured.Unstructured, error) {
-	gvk := getTargetGVK(es)
-
-	obj := &unstructured.Unstructured{}
-	obj.SetGroupVersionKind(gvk)
-	obj.SetName(getTargetName(es))
-	obj.SetNamespace(es.Namespace)
+// If existingObj is provided, templates will be applied to it (for merge behavior).
+// Otherwise, a new object is created.
+func (r *Reconciler) applyTemplateToManifest(ctx context.Context, es *esv1.ExternalSecret, dataMap map[string][]byte, existingObj *unstructured.Unstructured) (*unstructured.Unstructured, error) {
+	var obj *unstructured.Unstructured
+	if existingObj != nil {
+		// use the existing object for merge behavior if it exists
+		obj = existingObj.DeepCopy()
+	} else {
+		gvk := getTargetGVK(es)
+		obj = &unstructured.Unstructured{}
+		obj.SetGroupVersionKind(gvk)
+		obj.SetName(getTargetName(es))
+		obj.SetNamespace(es.Namespace)
+	}
 
-	labels := make(map[string]string)
-	annotations := make(map[string]string)
+	labels := obj.GetLabels()
+	if labels == nil {
+		labels = make(map[string]string)
+	}
+	annotations := obj.GetAnnotations()
+	if annotations == nil {
+		annotations = make(map[string]string)
+	}
 
 	if es.Spec.Target.Template != nil {
 		for k, v := range es.Spec.Target.Template.Metadata.Labels {

pkg/controllers/externalsecret/externalsecret_controller_manifest_test.go

@@ -369,7 +369,7 @@ func TestApplyTemplateToManifest_SimpleConfigMap(t *testing.T) {
 	}
 
 	// Execute
-	result, err := r.applyTemplateToManifest(context.Background(), es, dataMap)
+	result, err := r.applyTemplateToManifest(context.Background(), es, dataMap, nil)
 
 	// Verify
 	require.NoError(t, err)
@@ -431,7 +431,7 @@ func TestApplyTemplateToManifest_WithMetadata(t *testing.T) {
 	}
 
 	// Execute
-	result, err := r.applyTemplateToManifest(context.Background(), es, dataMap)
+	result, err := r.applyTemplateToManifest(context.Background(), es, dataMap, nil)
 
 	// Verify
 	require.NoError(t, err)
@@ -600,7 +600,7 @@ template:
 		"version":  []byte("1.21"),
 	}
 
-	result, err := r.applyTemplateToManifest(context.Background(), es, dataMap)
+	result, err := r.applyTemplateToManifest(context.Background(), es, dataMap, nil)
 
 	require.NoError(t, err)
 	assert.NotNil(t, result)
@@ -627,3 +627,91 @@ template:
 
 	t.Logf("Result spec: %+v", spec)
 }
+
+func TestApplyTemplateToManifest_MergeBehavior(t *testing.T) {
+	_ = esv1.AddToScheme(scheme.Scheme)
+	fakeClient := fakeclient.NewClientBuilder().WithScheme(scheme.Scheme).Build()
+
+	r := &Reconciler{
+		Client: fakeClient,
+	}
+
+	es := &esv1.ExternalSecret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-es",
+			Namespace: "default",
+		},
+		Spec: esv1.ExternalSecretSpec{
+			Target: esv1.ExternalSecretTarget{
+				Name: "test-slack-config",
+				Manifest: &esv1.ManifestReference{
+					APIVersion: "notification.toolkit.fluxcd.io/v1beta1",
+					Kind:       "Provider",
+				},
+				Template: &esv1.ExternalSecretTemplate{
+					EngineVersion: esv1.TemplateEngineV2,
+					TemplateFrom: []esv1.TemplateFrom{
+						{
+							Target:  "spec.slack",
+							Literal: ptr.To(`api_url: {{ .url }}`),
+						},
+					},
+				},
+			},
+		},
+	}
+
+	existingResource := &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": "notification.toolkit.fluxcd.io/v1beta1",
+			"kind":       "Provider",
+			"metadata": map[string]interface{}{
+				"name":            "test-slack-config",
+				"namespace":       "default",
+				"resourceVersion": "12345",
+				"uid":             "test-uid-123",
+			},
+			"spec": map[string]interface{}{
+				"type": "slack",
+				"slack": map[string]interface{}{
+					"channel":  "general",
+					"username": "bot",
+				},
+			},
+		},
+	}
+
+	dataMap := map[string][]byte{
+		"url": []byte("https://hooks.slack.com/services/XXX"),
+	}
+
+	result, err := r.applyTemplateToManifest(context.Background(), es, dataMap, existingResource)
+
+	require.NoError(t, err)
+	assert.NotNil(t, result)
+	assert.Equal(t, "Provider", result.GetKind())
+	assert.Equal(t, "test-slack-config", result.GetName())
+
+	specType, found, err := unstructured.NestedString(result.Object, "spec", "type")
+	require.NoError(t, err)
+	require.True(t, found, "spec.type should be preserved")
+	assert.Equal(t, "slack", specType, "spec.type should be preserved from existing resource")
+
+	slackChannel, found, err := unstructured.NestedString(result.Object, "spec", "slack", "channel")
+	require.NoError(t, err)
+	require.True(t, found, "spec.slack.channel should be preserved")
+	assert.Equal(t, "general", slackChannel, "spec.slack.channel should be preserved from existing resource")
+
+	slackUsername, found, err := unstructured.NestedString(result.Object, "spec", "slack", "username")
+	require.NoError(t, err)
+	require.True(t, found, "spec.slack.username should be preserved")
+	assert.Equal(t, "bot", slackUsername, "spec.slack.username should be preserved from existing resource")
+
+	apiURL, found, err := unstructured.NestedString(result.Object, "spec", "slack", "api_url")
+	require.NoError(t, err)
+	require.True(t, found, "spec.slack.api_url should be added from template")
+	assert.Equal(t, "https://hooks.slack.com/services/XXX", apiURL, "spec.slack.api_url should come from template")
+	assert.Equal(t, "12345", result.GetResourceVersion(), "resourceVersion should be preserved")
+	assert.Equal(t, "test-uid-123", string(result.GetUID()), "uid should be preserved")
+	t.Logf("Result spec: %+v", result.Object["spec"])
+}