fix: v1 templates with metadata + always cleanup orphaned secrets (#4174)

Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
Co-authored-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>

apis/externalsecrets/v1beta1/externalsecret_types.go

@@ -465,11 +465,12 @@ const (
 	// ConditionReasonSecretMissing indicates that the secret is missing.
 	ConditionReasonSecretMissing = "SecretMissing"
 
-	ReasonUpdateFailed = "UpdateFailed"
-	ReasonDeprecated   = "ParameterDeprecated"
-	ReasonCreated      = "Created"
-	ReasonUpdated      = "Updated"
-	ReasonDeleted      = "Deleted"
+	ReasonUpdateFailed          = "UpdateFailed"
+	ReasonDeprecated            = "ParameterDeprecated"
+	ReasonCreated               = "Created"
+	ReasonUpdated               = "Updated"
+	ReasonDeleted               = "Deleted"
+	ReasonMissingProviderSecret = "MissingProviderSecret"
 )
 
 type ExternalSecretStatus struct {

docs/guides/templating-v1.md

@@ -4,6 +4,10 @@
 
     Templating Engine v1 is **deprecated** and will be removed in the future. Please migrate to engine v2 and take a look at our [upgrade guide](templating.md#migrating-from-v1) for changes.
 
+!!! note
+
+    Templating Engine v1 does NOT support templating the `spec.target.template.metadata` fields, or the keys of the `spec.target.template.data` map, it will treat them as plain strings.
+    To use templates in annotations/labels/data-keys, please use Templating Engine v2.
 
 With External Secrets Operator you can transform the data from the external secret provider before it is stored as `Kind=Secret`. You can do this with the `Spec.Target.Template`.
 
@@ -18,7 +22,7 @@ You can use templates to inject your secrets into a configuration file that you
 
 You can also use pre-defined functions to extract data from your secrets. Here: extract key/cert from a pkcs12 archive and store it as PEM.
 ``` yaml
-{% include 'pkcs12-template-v2-external-secret.yaml' %}
+{% include 'pkcs12-template-v1-external-secret.yaml' %}
 ```
 
 ### TemplateFrom

docs/snippets/pkcs12-template-v1-external-secret.yaml

@@ -13,6 +13,7 @@ spec:
     # this is how the Kind=Secret will look like
     template:
       type: kubernetes.io/tls
+      engineVersion: v1
       data:
         tls.crt: "{{ .mysecret | pkcs12cert | pemCertificate }}"
         tls.key: "{{ .mysecret | pkcs12key | pemPrivateKey }}"

pkg/controllers/externalsecret/externalsecret_controller.go

@@ -93,11 +93,11 @@ const (
 	logErrorUnmanagedStore       = "unable to determine if store is managed"
 
 	// error formats.
-	errConvert               = "could not apply conversion strategy to keys: %v"
-	errDecode                = "could not apply decoding strategy to %v[%d]: %v"
-	errGenerate              = "could not generate [%d]: %w"
-	errRewrite               = "could not rewrite spec.dataFrom[%d]: %v"
-	errInvalidKeys           = "secret keys from spec.dataFrom.%v[%d] can only have alphanumeric, '-', '_' or '.' characters. Convert them using rewrite (https://external-secrets.io/latest/guides/datafrom-rewrite/)"
+	errConvert               = "error applying conversion strategy %s to keys: %w"
+	errRewrite               = "error applying rewrite to keys: %w"
+	errDecode                = "error applying decoding strategy %s to data: %w"
+	errGenerate              = "error using generator: %w"
+	errInvalidKeys           = "invalid secret keys (TIP: use rewrite or conversionStrategy to change keys): %w"
 	errFetchTplFrom          = "error fetching templateFrom data: %w"
 	errApplyTemplate         = "could not apply template: %w"
 	errExecTpl               = "could not execute template: %w"
@@ -106,6 +106,14 @@ const (
 	errUpdateNotFound        = "unable to update secret %s: not found"
 	errDeleteCreatePolicy    = "unable to delete secret %s: creationPolicy=%s is not Owner"
 	errSecretCachesNotSynced = "controller caches for secret %s are not in sync"
+
+	// event messages.
+	eventCreated                  = "secret created"
+	eventUpdated                  = "secret updated"
+	eventDeleted                  = "secret deleted due to DeletionPolicy=Delete"
+	eventDeletedOrphaned          = "secret deleted because it was orphaned"
+	eventMissingProviderSecret    = "secret does not exist at provider using spec.dataFrom[%d]"
+	eventMissingProviderSecretKey = "secret does not exist at provider using spec.dataFrom[%d] (key=%s)"
 )
 
 // these errors are explicitly defined so we can detect them with `errors.Is()`.
@@ -333,17 +341,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 			// NOTE: this error cant be fixed by retrying so we don't return an error (which would requeue immediately)
 			creationPolicy := externalSecret.Spec.Target.CreationPolicy
 			if creationPolicy != esv1beta1.CreatePolicyOwner {
-				err := fmt.Errorf(errDeleteCreatePolicy, secretName, creationPolicy)
+				err = fmt.Errorf(errDeleteCreatePolicy, secretName, creationPolicy)
 				r.markAsFailed(msgErrorDeleteSecret, err, externalSecret, syncCallsError.With(resourceLabels))
 				return ctrl.Result{}, nil
 			}
 
 			// delete the secret, if it exists
 			if existingSecret.UID != "" {
-				if err := r.Delete(ctx, existingSecret); err != nil && !apierrors.IsNotFound(err) {
+				err = r.Delete(ctx, existingSecret)
+				if err != nil && !apierrors.IsNotFound(err) {
 					r.markAsFailed(msgErrorDeleteSecret, err, externalSecret, syncCallsError.With(resourceLabels))
 					return ctrl.Result{}, err
 				}
+				r.recorder.Event(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonDeleted, eventDeleted)
 			}
 
 			r.markAsDone(externalSecret, start, log, esv1beta1.ConditionReasonSecretDeleted, msgDeleted)
@@ -446,7 +456,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 		return nil
 	}
 
-	switch externalSecret.Spec.Target.CreationPolicy { //nolint:exhaustive
+	switch externalSecret.Spec.Target.CreationPolicy {
+	case esv1beta1.CreatePolicyNone:
+		log.V(1).Info("secret creation skipped due to CreationPolicy=None")
+		err = nil
 	case esv1beta1.CreatePolicyMerge:
 		// update the secret, if it exists
 		if existingSecret.UID != "" {
@@ -457,25 +470,28 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ct
 			r.markAsDone(externalSecret, start, log, esv1beta1.ConditionReasonSecretMissing, msgMissing)
 			return r.getRequeueResult(externalSecret), nil
 		}
-	case esv1beta1.CreatePolicyNone:
-		log.V(1).Info("secret creation skipped due to creationPolicy=None")
-		err = nil
-	default:
+	case esv1beta1.CreatePolicyOrphan:
 		// create the secret, if it does not exist
 		if existingSecret.UID == "" {
 			err = r.createSecret(ctx, mutationFunc, externalSecret, secretName)
+		} else {
+			// if the secret exists, we should update it
+			err = r.updateSecret(ctx, existingSecret, mutationFunc, externalSecret, secretName)
+		}
+	case esv1beta1.CreatePolicyOwner:
+		// we may have orphaned secrets to clean up,
+		// for example, if the target secret name was changed
+		err = r.deleteOrphanedSecrets(ctx, externalSecret, secretName)
+		if err != nil {
+			r.markAsFailed(msgErrorDeleteOrphaned, err, externalSecret, syncCallsError.With(resourceLabels))
+			return ctrl.Result{}, err
+		}
 
-			// we may have orphaned secrets to clean up,
-			// for example, if the target secret name was changed
-			if err == nil {
-				delErr := deleteOrphanedSecrets(ctx, r.Client, externalSecret, secretName)
-				if delErr != nil {
-					r.markAsFailed(msgErrorDeleteOrphaned, delErr, externalSecret, syncCallsError.With(resourceLabels))
-					return ctrl.Result{}, delErr
-				}
-			}
+		// create the secret, if it does not exist
+		if existingSecret.UID == "" {
+			err = r.createSecret(ctx, mutationFunc, externalSecret, secretName)
 		} else {
-			// update the secret, if it exists
+			// if the secret exists, we should update it
 			err = r.updateSecret(ctx, existingSecret, mutationFunc, externalSecret, secretName)
 		}
 	}
@@ -581,9 +597,11 @@ func (r *Reconciler) markAsFailed(msg string, err error, externalSecret *esv1bet
 	counter.Inc()
 }
 
-func deleteOrphanedSecrets(ctx context.Context, cl client.Client, externalSecret *esv1beta1.ExternalSecret, secretName string) error {
+func (r *Reconciler) deleteOrphanedSecrets(ctx context.Context, externalSecret *esv1beta1.ExternalSecret, secretName string) error {
 	ownerLabel := utils.ObjectHash(fmt.Sprintf("%v/%v", externalSecret.Namespace, externalSecret.Name))
 
+	// we use a PartialObjectMetadataList to avoid loading the full secret objects
+	// and because the Secrets partials are always cached due to WatchesMetadata() in SetupWithManager()
 	secretListPartial := &metav1.PartialObjectMetadataList{}
 	secretListPartial.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind("SecretList"))
 	listOpts := &client.ListOptions{
@@ -592,16 +610,18 @@ func deleteOrphanedSecrets(ctx context.Context, cl client.Client, externalSecret
 		}),
 		Namespace: externalSecret.Namespace,
 	}
-	if err := cl.List(ctx, secretListPartial, listOpts); err != nil {
+	if err := r.List(ctx, secretListPartial, listOpts); err != nil {
 		return err
 	}
 
 	// delete all secrets that are not the target secret
 	for _, secretPartial := range secretListPartial.Items {
 		if secretPartial.GetName() != secretName {
-			if err := cl.Delete(ctx, &secretPartial); err != nil {
+			err := r.Delete(ctx, &secretPartial)
+			if err != nil && !apierrors.IsNotFound(err) {
 				return err
 			}
+			r.recorder.Event(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonDeleted, eventDeletedOrphaned)
 		}
 	}
 
@@ -633,7 +653,7 @@ func (r *Reconciler) createSecret(ctx context.Context, mutationFunc func(secret
 	// https://github.com/external-secrets/external-secrets/pull/2263
 	es.Status.Binding = v1.LocalObjectReference{Name: newSecret.Name}
 
-	r.recorder.Event(es, v1.EventTypeNormal, esv1beta1.ReasonCreated, "Created Secret")
+	r.recorder.Event(es, v1.EventTypeNormal, esv1beta1.ReasonCreated, eventCreated)
 	return nil
 }
 
@@ -709,7 +729,7 @@ func (r *Reconciler) updateSecret(ctx context.Context, existingSecret *v1.Secret
 		return fmt.Errorf(errUpdate, updatedSecret.Name, err)
 	}
 
-	r.recorder.Event(es, v1.EventTypeNormal, esv1beta1.ReasonUpdated, "Updated Secret")
+	r.recorder.Event(es, v1.EventTypeNormal, esv1beta1.ReasonUpdated, eventUpdated)
 	return nil
 }
 

pkg/controllers/externalsecret/externalsecret_controller_secret.go

@@ -49,55 +49,68 @@ func (r *Reconciler) getProviderSecretData(ctx context.Context, externalSecret *
 		var err error
 
 		if remoteRef.Find != nil {
-			secretMap, err = r.handleFindAllSecrets(ctx, externalSecret, remoteRef, mgr, i)
+			secretMap, err = r.handleFindAllSecrets(ctx, externalSecret, remoteRef, mgr)
+			if err != nil {
+				err = fmt.Errorf("error processing spec.dataFrom[%d].find, err: %w", i, err)
+			}
 		} else if remoteRef.Extract != nil {
-			secretMap, err = r.handleExtractSecrets(ctx, externalSecret, remoteRef, mgr, i)
+			secretMap, err = r.handleExtractSecrets(ctx, externalSecret, remoteRef, mgr)
+			if err != nil {
+				err = fmt.Errorf("error processing spec.dataFrom[%d].extract, err: %w", i, err)
+			}
 		} else if remoteRef.SourceRef != nil && remoteRef.SourceRef.GeneratorRef != nil {
-			secretMap, err = r.handleGenerateSecrets(ctx, externalSecret.Namespace, remoteRef, i)
+			secretMap, err = r.handleGenerateSecrets(ctx, externalSecret.Namespace, remoteRef)
+			if err != nil {
+				err = fmt.Errorf("error processing spec.dataFrom[%d].sourceRef.generatorRef, err: %w", i, err)
+			}
 		}
+
 		if errors.Is(err, esv1beta1.NoSecretErr) && externalSecret.Spec.Target.DeletionPolicy != esv1beta1.DeletionPolicyRetain {
-			r.recorder.Event(
-				externalSecret,
-				v1.EventTypeNormal,
-				esv1beta1.ReasonDeleted,
-				fmt.Sprintf("secret does not exist at provider using .dataFrom[%d]", i),
-			)
+			r.recorder.Eventf(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonMissingProviderSecret, eventMissingProviderSecret, i)
 			continue
 		}
 		if err != nil {
 			return nil, err
 		}
+
 		providerData = utils.MergeByteMap(providerData, secretMap)
 	}
 
 	for i, secretRef := range externalSecret.Spec.Data {
-		err := r.handleSecretData(ctx, i, *externalSecret, secretRef, providerData, mgr)
+		err := r.handleSecretData(ctx, *externalSecret, secretRef, providerData, mgr)
 		if errors.Is(err, esv1beta1.NoSecretErr) && externalSecret.Spec.Target.DeletionPolicy != esv1beta1.DeletionPolicyRetain {
-			r.recorder.Event(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonDeleted, fmt.Sprintf("secret does not exist at provider using .data[%d] key=%s", i, secretRef.RemoteRef.Key))
+			r.recorder.Eventf(externalSecret, v1.EventTypeNormal, esv1beta1.ReasonMissingProviderSecret, eventMissingProviderSecretKey, i, secretRef.RemoteRef.Key)
 			continue
 		}
 		if err != nil {
-			return nil, fmt.Errorf("error retrieving secret at .data[%d], key: %s, err: %w", i, secretRef.RemoteRef.Key, err)
+			return nil, fmt.Errorf("error processing spec.data[%d] (key: %s), err: %w", i, secretRef.RemoteRef.Key, err)
 		}
 	}
 
 	return providerData, nil
 }
 
-func (r *Reconciler) handleSecretData(ctx context.Context, i int, externalSecret esv1beta1.ExternalSecret, secretRef esv1beta1.ExternalSecretData, providerData map[string][]byte, cmgr *secretstore.Manager) error {
+func (r *Reconciler) handleSecretData(ctx context.Context, externalSecret esv1beta1.ExternalSecret, secretRef esv1beta1.ExternalSecretData, providerData map[string][]byte, cmgr *secretstore.Manager) error {
 	client, err := cmgr.Get(ctx, externalSecret.Spec.SecretStoreRef, externalSecret.Namespace, toStoreGenSourceRef(secretRef.SourceRef))
 	if err != nil {
 		return err
 	}
+
+	// get a single secret from the store
 	secretData, err := client.GetSecret(ctx, secretRef.RemoteRef)
 	if err != nil {
 		return err
 	}
+
+	// decode the secret if needed
 	secretData, err = utils.Decode(secretRef.RemoteRef.DecodingStrategy, secretData)
 	if err != nil {
-		return fmt.Errorf(errDecode, "spec.data", i, err)
+		return fmt.Errorf(errDecode, secretRef.RemoteRef.DecodingStrategy, err)
 	}
+
+	// store the secret data
 	providerData[secretRef.SecretKey] = secretData
+
 	return nil
 }
 
@@ -110,83 +123,108 @@ func toStoreGenSourceRef(ref *esv1beta1.StoreSourceRef) *esv1beta1.StoreGenerato
 	}
 }
 
-func (r *Reconciler) handleGenerateSecrets(ctx context.Context, namespace string, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef, i int) (map[string][]byte, error) {
+func (r *Reconciler) handleGenerateSecrets(ctx context.Context, namespace string, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef) (map[string][]byte, error) {
 	gen, obj, err := resolvers.GeneratorRef(ctx, r.Client, r.Scheme, namespace, remoteRef.SourceRef.GeneratorRef)
 	if err != nil {
-		return nil, fmt.Errorf("unable to resolve generator: %w", err)
+		return nil, err
 	}
-	// We still pass the namespace to the generate function because it needs to create
-	// namespace based objects.
+
+	// use the generator
 	secretMap, err := gen.Generate(ctx, obj, r.Client, namespace)
 	if err != nil {
-		return nil, fmt.Errorf(errGenerate, i, err)
+		return nil, fmt.Errorf(errGenerate, err)
 	}
+
+	// rewrite the keys if needed
 	secretMap, err = utils.RewriteMap(remoteRef.Rewrite, secretMap)
 	if err != nil {
-		return nil, fmt.Errorf(errRewrite, i, err)
+		return nil, fmt.Errorf(errRewrite, err)
 	}
-	if !utils.ValidateKeys(secretMap) {
-		return nil, fmt.Errorf(errInvalidKeys, "generator", i)
+
+	// validate the keys
+	err = utils.ValidateKeys(secretMap)
+	if err != nil {
+		return nil, fmt.Errorf(errInvalidKeys, err)
 	}
+
 	return secretMap, err
 }
 
-func (r *Reconciler) handleExtractSecrets(ctx context.Context, externalSecret *esv1beta1.ExternalSecret, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef, cmgr *secretstore.Manager, i int) (map[string][]byte, error) {
+//nolint:dupl
+func (r *Reconciler) handleExtractSecrets(ctx context.Context, externalSecret *esv1beta1.ExternalSecret, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef, cmgr *secretstore.Manager) (map[string][]byte, error) {
 	client, err := cmgr.Get(ctx, externalSecret.Spec.SecretStoreRef, externalSecret.Namespace, remoteRef.SourceRef)
 	if err != nil {
 		return nil, err
 	}
+
+	// get multiple secrets from the store
 	secretMap, err := client.GetSecretMap(ctx, *remoteRef.Extract)
 	if err != nil {
 		return nil, err
 	}
+
+	// rewrite the keys if needed
 	secretMap, err = utils.RewriteMap(remoteRef.Rewrite, secretMap)
 	if err != nil {
-		return nil, fmt.Errorf(errRewrite, i, err)
+		return nil, fmt.Errorf(errRewrite, err)
 	}
 	if len(remoteRef.Rewrite) == 0 {
 		secretMap, err = utils.ConvertKeys(remoteRef.Extract.ConversionStrategy, secretMap)
 		if err != nil {
-			return nil, fmt.Errorf(errConvert, err)
+			return nil, fmt.Errorf(errConvert, remoteRef.Extract.ConversionStrategy, err)
 		}
 	}
-	if !utils.ValidateKeys(secretMap) {
-		return nil, fmt.Errorf(errInvalidKeys, "extract", i)
+
+	// validate the keys
+	err = utils.ValidateKeys(secretMap)
+	if err != nil {
+		return nil, fmt.Errorf(errInvalidKeys, err)
 	}
+
+	// decode the secrets if needed
 	secretMap, err = utils.DecodeMap(remoteRef.Extract.DecodingStrategy, secretMap)
 	if err != nil {
-		return nil, fmt.Errorf(errDecode, "spec.dataFrom", i, err)
+		return nil, fmt.Errorf(errDecode, remoteRef.Extract.DecodingStrategy, err)
 	}
+
 	return secretMap, err
 }
 
-func (r *Reconciler) handleFindAllSecrets(ctx context.Context, externalSecret *esv1beta1.ExternalSecret, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef, cmgr *secretstore.Manager, i int) (map[string][]byte, error) {
+//nolint:dupl
+func (r *Reconciler) handleFindAllSecrets(ctx context.Context, externalSecret *esv1beta1.ExternalSecret, remoteRef esv1beta1.ExternalSecretDataFromRemoteRef, cmgr *secretstore.Manager) (map[string][]byte, error) {
 	client, err := cmgr.Get(ctx, externalSecret.Spec.SecretStoreRef, externalSecret.Namespace, remoteRef.SourceRef)
 	if err != nil {
 		return nil, err
 	}
+
+	// get all secrets from the store that match the selector
 	secretMap, err := client.GetAllSecrets(ctx, *remoteRef.Find)
 	if err != nil {
 		return nil, err
 	}
+
+	// rewrite the keys if needed
 	secretMap, err = utils.RewriteMap(remoteRef.Rewrite, secretMap)
 	if err != nil {
-		return nil, fmt.Errorf(errRewrite, i, err)
+		return nil, fmt.Errorf(errRewrite, err)
 	}
 	if len(remoteRef.Rewrite) == 0 {
-		// ConversionStrategy is deprecated. Use RewriteMap instead.
-		r.recorder.Event(externalSecret, v1.EventTypeWarning, esv1beta1.ReasonDeprecated, fmt.Sprintf("dataFrom[%d].find.conversionStrategy=%v is deprecated and will be removed in further releases. Use dataFrom.rewrite instead", i, remoteRef.Find.ConversionStrategy))
 		secretMap, err = utils.ConvertKeys(remoteRef.Find.ConversionStrategy, secretMap)
 		if err != nil {
-			return nil, fmt.Errorf(errConvert, err)
+			return nil, fmt.Errorf(errConvert, remoteRef.Find.ConversionStrategy, err)
 		}
 	}
-	if !utils.ValidateKeys(secretMap) {
-		return nil, fmt.Errorf(errInvalidKeys, "find", i)
+
+	// validate the keys
+	err = utils.ValidateKeys(secretMap)
+	if err != nil {
+		return nil, fmt.Errorf(errInvalidKeys, err)
 	}
+
+	// decode the secrets if needed
 	secretMap, err = utils.DecodeMap(remoteRef.Find.DecodingStrategy, secretMap)
 	if err != nil {
-		return nil, fmt.Errorf(errDecode, "spec.dataFrom", i, err)
+		return nil, fmt.Errorf(errDecode, remoteRef.Find.DecodingStrategy, err)
 	}
 	return secretMap, err
 }

pkg/controllers/externalsecret/externalsecret_controller_template.go

@@ -79,18 +79,23 @@ func (r *Reconciler) applyTemplate(ctx context.Context, es *esv1beta1.ExternalSe
 	if err != nil {
 		return fmt.Errorf(errFetchTplFrom, err)
 	}
-	// explicitly defined template.Data takes precedence over templateFrom
+
+	// apply data templates
+	// NOTE: explicitly defined template.data templates take precedence over templateFrom
 	err = p.MergeMap(es.Spec.Target.Template.Data, esv1beta1.TemplateTargetData)
 	if err != nil {
 		return fmt.Errorf(errExecTpl, err)
 	}
 
-	// get template data for labels
+	// apply templates for labels
+	// NOTE: this only works for v2 templates
 	err = p.MergeMap(es.Spec.Target.Template.Metadata.Labels, esv1beta1.TemplateTargetLabels)
 	if err != nil {
 		return fmt.Errorf(errExecTpl, err)
 	}
-	// get template data for annotations
+
+	// apply template for annotations
+	// NOTE: this only works for v2 templates
 	err = p.MergeMap(es.Spec.Target.Template.Metadata.Annotations, esv1beta1.TemplateTargetAnnotations)
 	if err != nil {
 		return fmt.Errorf(errExecTpl, err)

pkg/template/engine.go

@@ -14,6 +14,8 @@ limitations under the License.
 package template
 
 import (
+	"fmt"
+
 	corev1 "k8s.io/api/core/v1"
 
 	esapi "github.com/external-secrets/external-secrets/apis/externalsecrets/v1beta1"
@@ -30,8 +32,5 @@ func EngineForVersion(version esapi.TemplateEngineVersion) (ExecFunc, error) {
 	case esapi.TemplateEngineV2:
 		return v2.Execute, nil
 	}
-
-	// in case we run with a old v1alpha1 CRD
-	// we must return v1 as default
-	return v1.Execute, nil
+	return nil, fmt.Errorf("unsupported template engine version: %s", version)
 }

pkg/template/v1/template.go

@@ -73,16 +73,28 @@ const (
 )
 
 // Execute renders the secret data as template. If an error occurs processing is stopped immediately.
-func Execute(tpl, data map[string][]byte, _ esapi.TemplateScope, _ esapi.TemplateTarget, secret *corev1.Secret) error {
-	if tpl == nil {
+func Execute(tpl, data map[string][]byte, scope esapi.TemplateScope, target esapi.TemplateTarget, secret *corev1.Secret) error {
+	if len(tpl) == 0 {
 		return nil
 	}
-	for k, v := range tpl {
-		val, err := execute(k, string(v), data)
-		if err != nil {
-			return fmt.Errorf(errExecute, k, err)
+
+	if scope != "" && scope != esapi.TemplateScopeValues {
+		return fmt.Errorf("template scope %s is not supported in v1 templates, please only use Values", scope)
+	}
+
+	switch target {
+	case esapi.TemplateTargetAnnotations:
+		// Annotations are not supported in v1 templates
+	case esapi.TemplateTargetLabels:
+		// Labels are not supported in v1 templates
+	case esapi.TemplateTargetData, "":
+		for k, v := range tpl {
+			val, err := execute(k, string(v), data)
+			if err != nil {
+				return fmt.Errorf(errExecute, k, err)
+			}
+			secret.Data[k] = val
 		}
-		secret.Data[k] = val
 	}
 	return nil
 }

pkg/utils/utils.go

@@ -192,19 +192,23 @@ func Decode(strategy esv1beta1.ExternalSecretDecodingStrategy, in []byte) ([]byt
 	}
 }
 
-func ValidateKeys(in map[string][]byte) bool {
+// ValidateKeys checks if the keys in the secret map are valid keys for a Kubernetes secret.
+func ValidateKeys(in map[string][]byte) error {
 	for key := range in {
-		for _, v := range key {
-			if !unicode.IsNumber(v) &&
-				!unicode.IsLetter(v) &&
-				v != '-' &&
-				v != '.' &&
-				v != '_' {
-				return false
+		keyLength := len(key)
+		if keyLength == 0 {
+			return fmt.Errorf("found empty key")
+		}
+		if keyLength > 253 {
+			return fmt.Errorf("key has length %d but max is 253: (following is truncated): %s", keyLength, key[:253])
+		}
+		for _, c := range key {
+			if !unicode.IsLetter(c) && !unicode.IsNumber(c) && c != '-' && c != '.' && c != '_' {
+				return fmt.Errorf("key has invalid character %c, only alphanumeric, '-', '.' and '_' are allowed: %s", c, key)
 			}
 		}
 	}
-	return true
+	return nil
 }
 
 // ConvertKeys converts a secret map into a valid key.