fix: allow using tpl metadata

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -39,7 +39,6 @@ import (
 	// Loading registered providers.
 	_ "github.com/external-secrets/external-secrets/pkg/provider/register"
 	schema "github.com/external-secrets/external-secrets/pkg/provider/schema"
-	"github.com/external-secrets/external-secrets/pkg/template"
 	utils "github.com/external-secrets/external-secrets/pkg/utils"
 )
 
@@ -58,6 +57,7 @@ const (
 	errSetCtrlReference      = "could not set ExternalSecret controller reference: %w"
 	errFetchTplFrom          = "error fetching templateFrom data: %w"
 	errGetSecretData         = "could not get secret data from provider: %w"
+	errApplyTemplate         = "could not apply template: %w"
 	errExecTpl               = "could not execute template: %w"
 	errPolicyMergeNotFound   = "the desired secret %s was not found. With creationPolicy=Merge the secret won't be created"
 	errPolicyMergeGetSecret  = "unable to get secret %s: %w"
@@ -188,37 +188,17 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 				return fmt.Errorf(errSetCtrlReference, err)
 			}
 		}
-		mergeMetadata(secret, externalSecret)
-		var tplMap map[string][]byte
-		var dataMap map[string][]byte
 
-		// get data
-		dataMap, err = r.getProviderSecretData(ctx, secretClient, &externalSecret)
+		dataMap, err := r.getProviderSecretData(ctx, secretClient, &externalSecret)
 		if err != nil {
 			return fmt.Errorf(errGetSecretData, err)
 		}
 
-		// no template: copy data and return
-		if externalSecret.Spec.Target.Template == nil {
-			secret.Data = dataMap
-			return nil
+		err = r.applyTemplate(ctx, &externalSecret, secret, dataMap)
+		if err != nil {
+			return fmt.Errorf(errApplyTemplate, err)
 		}
 
-		// template: fetch & execute templates
-		tplMap, err = r.getTemplateData(ctx, &externalSecret)
-		if err != nil {
-			return fmt.Errorf(errFetchTplFrom, err)
-		}
-		// override templateFrom data with template data
-		for k, v := range externalSecret.Spec.Target.Template.Data {
-			tplMap[k] = []byte(v)
-		}
-
-		log.V(1).Info("found template data", "tpl_data", tplMap)
-		err = template.Execute(tplMap, dataMap, secret)
-		if err != nil {
-			return fmt.Errorf(errExecTpl, err)
-		}
 		return nil
 	}
 
@@ -327,26 +307,6 @@ func shouldRefresh(es esv1alpha1.ExternalSecret) bool {
 	return !es.Status.RefreshTime.Add(es.Spec.RefreshInterval.Duration).After(time.Now())
 }
 
-// we do not want to force-override the label/annotations
-// and only copy the necessary key/value pairs.
-func mergeMetadata(secret *v1.Secret, externalSecret esv1alpha1.ExternalSecret) {
-	if secret.ObjectMeta.Labels == nil {
-		secret.ObjectMeta.Labels = make(map[string]string)
-	}
-	if secret.ObjectMeta.Annotations == nil {
-		secret.ObjectMeta.Annotations = make(map[string]string)
-	}
-	if externalSecret.Spec.Target.Template == nil {
-		utils.MergeStringMap(secret.ObjectMeta.Labels, externalSecret.ObjectMeta.Labels)
-		utils.MergeStringMap(secret.ObjectMeta.Annotations, externalSecret.ObjectMeta.Annotations)
-		return
-	}
-	// if template is defined: use those labels/annotations
-	secret.Type = externalSecret.Spec.Target.Template.Type
-	utils.MergeStringMap(secret.ObjectMeta.Labels, externalSecret.Spec.Target.Template.Metadata.Labels)
-	utils.MergeStringMap(secret.ObjectMeta.Annotations, externalSecret.Spec.Target.Template.Metadata.Annotations)
-}
-
 // getStore returns the store with the provided ExternalSecret.
 func (r *Reconciler) getStore(ctx context.Context, externalSecret *esv1alpha1.ExternalSecret) (esv1alpha1.GenericStore, error) {
 	ref := types.NamespacedName{
@@ -398,50 +358,6 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, providerClient p
 	return providerData, nil
 }
 
-func (r *Reconciler) getTemplateData(ctx context.Context, externalSecret *esv1alpha1.ExternalSecret) (map[string][]byte, error) {
-	out := make(map[string][]byte)
-	if externalSecret.Spec.Target.Template == nil {
-		return out, nil
-	}
-	for _, tpl := range externalSecret.Spec.Target.Template.TemplateFrom {
-		if tpl.ConfigMap != nil {
-			var cm v1.ConfigMap
-			err := r.Client.Get(ctx, types.NamespacedName{
-				Name:      tpl.ConfigMap.Name,
-				Namespace: externalSecret.Namespace,
-			}, &cm)
-			if err != nil {
-				return nil, err
-			}
-			for _, k := range tpl.ConfigMap.Items {
-				val, ok := cm.Data[k.Key]
-				if !ok {
-					return nil, fmt.Errorf(errTplCMMissingKey, tpl.ConfigMap.Name, k.Key)
-				}
-				out[k.Key] = []byte(val)
-			}
-		}
-		if tpl.Secret != nil {
-			var sec v1.Secret
-			err := r.Client.Get(ctx, types.NamespacedName{
-				Name:      tpl.Secret.Name,
-				Namespace: externalSecret.Namespace,
-			}, &sec)
-			if err != nil {
-				return nil, err
-			}
-			for _, k := range tpl.Secret.Items {
-				val, ok := sec.Data[k.Key]
-				if !ok {
-					return nil, fmt.Errorf(errTplSecMissingKey, tpl.Secret.Name, k.Key)
-				}
-				out[k.Key] = val
-			}
-		}
-	}
-	return out, nil
-}
-
 // SetupWithManager returns a new controller builder that will be started by the provided Manager.
 func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).

pkg/controllers/externalsecret/externalsecret_controller_template.go

@@ -0,0 +1,155 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package externalsecret
+
+import (
+	"context"
+	"fmt"
+
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	esv1alpha1 "github.com/external-secrets/external-secrets/apis/externalsecrets/v1alpha1"
+
+	// Loading registered providers.
+	_ "github.com/external-secrets/external-secrets/pkg/provider/register"
+	"github.com/external-secrets/external-secrets/pkg/template"
+	utils "github.com/external-secrets/external-secrets/pkg/utils"
+)
+
+// merge template in the following order:
+// * template.Data (highest precedence)
+// * template.templateFrom
+// * secret via es.data or es.dataFrom.
+func (r *Reconciler) applyTemplate(ctx context.Context, es *esv1alpha1.ExternalSecret, secret *v1.Secret, dataMap map[string][]byte) error {
+	mergeMetadata(secret, es)
+
+	// no template: copy data and return
+	if es.Spec.Target.Template == nil {
+		secret.Data = dataMap
+		return nil
+	}
+
+	// fetch templates defined in template.templateFrom
+	tplMap, err := r.getTemplateData(ctx, es)
+	if err != nil {
+		return fmt.Errorf(errFetchTplFrom, err)
+	}
+
+	// explicitly defined template.Data takes precedence over templateFrom
+	for k, v := range es.Spec.Target.Template.Data {
+		tplMap[k] = []byte(v)
+	}
+	r.Log.V(1).Info("found template data", "tpl_data", tplMap)
+
+	err = template.Execute(tplMap, dataMap, secret)
+	if err != nil {
+		return fmt.Errorf(errExecTpl, err)
+	}
+
+	// if no data was provided by template fallback
+	// to value from the provider
+	if len(es.Spec.Target.Template.Data) == 0 {
+		for k, v := range dataMap {
+			secret.Data[k] = v
+		}
+	}
+
+	return nil
+}
+
+// we do not want to force-override the label/annotations
+// and only copy the necessary key/value pairs.
+func mergeMetadata(secret *v1.Secret, externalSecret *esv1alpha1.ExternalSecret) {
+	if secret.ObjectMeta.Labels == nil {
+		secret.ObjectMeta.Labels = make(map[string]string)
+	}
+	if secret.ObjectMeta.Annotations == nil {
+		secret.ObjectMeta.Annotations = make(map[string]string)
+	}
+	if externalSecret.Spec.Target.Template == nil {
+		utils.MergeStringMap(secret.ObjectMeta.Labels, externalSecret.ObjectMeta.Labels)
+		utils.MergeStringMap(secret.ObjectMeta.Annotations, externalSecret.ObjectMeta.Annotations)
+		return
+	}
+	// if template is defined: use those labels/annotations
+	secret.Type = externalSecret.Spec.Target.Template.Type
+	utils.MergeStringMap(secret.ObjectMeta.Labels, externalSecret.Spec.Target.Template.Metadata.Labels)
+	utils.MergeStringMap(secret.ObjectMeta.Annotations, externalSecret.Spec.Target.Template.Metadata.Annotations)
+}
+
+func (r *Reconciler) getTemplateData(ctx context.Context, externalSecret *esv1alpha1.ExternalSecret) (map[string][]byte, error) {
+	out := make(map[string][]byte)
+	if externalSecret.Spec.Target.Template == nil {
+		return out, nil
+	}
+	for _, tpl := range externalSecret.Spec.Target.Template.TemplateFrom {
+		err := mergeConfigMap(ctx, r.Client, externalSecret, tpl, out)
+		if err != nil {
+			return nil, err
+		}
+		err = mergeSecret(ctx, r.Client, externalSecret, tpl, out)
+		if err != nil {
+			return nil, err
+		}
+	}
+	return out, nil
+}
+
+func mergeConfigMap(ctx context.Context, k8sClient client.Client, es *esv1alpha1.ExternalSecret, tpl esv1alpha1.TemplateFrom, out map[string][]byte) error {
+	if tpl.ConfigMap == nil {
+		return nil
+	}
+
+	var cm v1.ConfigMap
+	err := k8sClient.Get(ctx, types.NamespacedName{
+		Name:      tpl.ConfigMap.Name,
+		Namespace: es.Namespace,
+	}, &cm)
+	if err != nil {
+		return err
+	}
+	for _, k := range tpl.ConfigMap.Items {
+		val, ok := cm.Data[k.Key]
+		if !ok {
+			return fmt.Errorf(errTplCMMissingKey, tpl.ConfigMap.Name, k.Key)
+		}
+		out[k.Key] = []byte(val)
+	}
+	return nil
+}
+
+func mergeSecret(ctx context.Context, k8sClient client.Client, es *esv1alpha1.ExternalSecret, tpl esv1alpha1.TemplateFrom, out map[string][]byte) error {
+	if tpl.Secret == nil {
+		return nil
+	}
+	var sec v1.Secret
+	err := k8sClient.Get(ctx, types.NamespacedName{
+		Name:      tpl.Secret.Name,
+		Namespace: es.Namespace,
+	}, &sec)
+	if err != nil {
+		return err
+	}
+	for _, k := range tpl.Secret.Items {
+		val, ok := sec.Data[k.Key]
+		if !ok {
+			return fmt.Errorf(errTplSecMissingKey, tpl.Secret.Name, k.Key)
+		}
+		out[k.Key] = val
+	}
+	return nil
+}

pkg/controllers/externalsecret/externalsecret_controller_test.go

@@ -341,17 +341,29 @@ var _ = Describe("ExternalSecret controller", func() {
 		const tplStaticKey = "tplstatickey"
 		const tplStaticVal = "tplstaticvalue"
 		const tplFromCMName = "template-cm"
+		const tplFromSecretName = "template-secret"
 		const tplFromKey = "tpl-from-key"
+		const tplFromSecKey = "tpl-from-sec-key"
 		const tplFromVal = "tpl-from-value: {{ .targetProperty | toString }} // {{ .bar | toString }}"
+		const tplFromSecVal = "tpl-from-sec-value: {{ .targetProperty | toString }} // {{ .bar | toString }}"
 		Expect(k8sClient.Create(context.Background(), &v1.ConfigMap{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      "template-cm",
+				Name:      tplFromCMName,
 				Namespace: ExternalSecretNamespace,
 			},
 			Data: map[string]string{
 				tplFromKey: tplFromVal,
 			},
 		})).To(Succeed())
+		Expect(k8sClient.Create(context.Background(), &v1.Secret{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      tplFromSecretName,
+				Namespace: ExternalSecretNamespace,
+			},
+			Data: map[string][]byte{
+				tplFromSecKey: []byte(tplFromSecVal),
+			},
+		})).To(Succeed())
 		tc.externalSecret.Spec.Target.Template = &esv1alpha1.ExternalSecretTemplate{
 			Metadata: esv1alpha1.ExternalSecretTemplateMetadata{},
 			Type:     v1.SecretTypeOpaque,
@@ -366,6 +378,16 @@ var _ = Describe("ExternalSecret controller", func() {
 						},
 					},
 				},
+				{
+					Secret: &esv1alpha1.TemplateRef{
+						Name: tplFromSecretName,
+						Items: []esv1alpha1.TemplateRefItem{
+							{
+								Key: tplFromSecKey,
+							},
+						},
+					},
+				},
 			},
 			Data: map[string]string{
 				// this should be the data value, not dataFrom
@@ -392,6 +414,7 @@ var _ = Describe("ExternalSecret controller", func() {
 			Expect(string(secret.Data[tplStaticKey])).To(Equal(tplStaticVal))
 			Expect(string(secret.Data["bar"])).To(Equal("value from map: map-bar-value"))
 			Expect(string(secret.Data[tplFromKey])).To(Equal("tpl-from-value: someValue // map-bar-value"))
+			Expect(string(secret.Data[tplFromSecKey])).To(Equal("tpl-from-sec-value: someValue // map-bar-value"))
 		}
 	}
 
@@ -451,6 +474,26 @@ var _ = Describe("ExternalSecret controller", func() {
 		}
 	}
 
+	onlyMetadataFromTemplate := func(tc *testCase) {
+		const secretVal = "someValue"
+		tc.externalSecret.Spec.RefreshInterval = &metav1.Duration{Duration: time.Second}
+		tc.externalSecret.Spec.Target.Template = &esv1alpha1.ExternalSecretTemplate{
+			Metadata: esv1alpha1.ExternalSecretTemplateMetadata{
+				Labels:      map[string]string{"foo": "bar"},
+				Annotations: map[string]string{"foo": "bar"},
+			},
+		}
+		fakeProvider.WithGetSecret([]byte(secretVal), nil)
+		tc.checkSecret = func(es *esv1alpha1.ExternalSecret, secret *v1.Secret) {
+			// check values
+			Expect(string(secret.Data[targetProp])).To(Equal(secretVal))
+
+			// labels/annotations should be taken from the template
+			Expect(secret.ObjectMeta.Labels).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Labels))
+			Expect(secret.ObjectMeta.Annotations).To(BeEquivalentTo(es.Spec.Target.Template.Metadata.Annotations))
+		}
+	}
+
 	// when the provider secret changes the Kind=Secret value
 	// must change, too.
 	refreshSecretValue := func(tc *testCase) {
@@ -703,6 +746,7 @@ var _ = Describe("ExternalSecret controller", func() {
 		Entry("should sync with template", syncWithTemplate),
 		Entry("should sync template with correct value precedence", syncWithTemplatePrecedence),
 		Entry("should refresh secret from template", refreshWithTemplate),
+		Entry("should be able to use only metadata from template", onlyMetadataFromTemplate),
 		Entry("should refresh secret value when provider secret changes", refreshSecretValue),
 		Entry("should not refresh secret value when provider secret changes but refreshInterval is zero", refreshintervalZero),
 		Entry("should fetch secret using dataFrom", syncWithDataFrom),

pkg/utils/utils.go

@@ -17,11 +17,11 @@ package utils
 import "reflect"
 
 // MergeByteMap merges map of byte slices.
-func MergeByteMap(src, dst map[string][]byte) map[string][]byte {
-	for k, v := range dst {
-		src[k] = v
+func MergeByteMap(dst, src map[string][]byte) map[string][]byte {
+	for k, v := range src {
+		dst[k] = v
 	}
-	return src
+	return dst
 }
 
 // MergeStringMap performs a deep clone from src to dest.