From 2d5829b790b8bfa6dababa95654c798ac553e331 Mon Sep 17 00:00:00 2001 From: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com> Date: Fri, 6 Dec 2024 13:22:59 -0800 Subject: [PATCH] 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> --- .../v1beta1/externalsecret_types.go | 11 +- docs/guides/templating-v1.md | 6 +- .../pkcs12-template-v1-external-secret.yaml | 1 + .../externalsecret_controller.go | 76 +++++++----- .../externalsecret_controller_secret.go | 110 ++++++++++++------ .../externalsecret_controller_template.go | 11 +- pkg/template/engine.go | 7 +- pkg/template/v1/template.go | 26 +++-- pkg/utils/utils.go | 22 ++-- 9 files changed, 177 insertions(+), 93 deletions(-) diff --git a/apis/externalsecrets/v1beta1/externalsecret_types.go b/apis/externalsecrets/v1beta1/externalsecret_types.go index 54e37e5b2..f97689f0e 100644 --- a/apis/externalsecrets/v1beta1/externalsecret_types.go +++ b/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 { diff --git a/docs/guides/templating-v1.md b/docs/guides/templating-v1.md index b08246305..237982142 100644 --- a/docs/guides/templating-v1.md +++ b/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 diff --git a/docs/snippets/pkcs12-template-v1-external-secret.yaml b/docs/snippets/pkcs12-template-v1-external-secret.yaml index 04dbf2fa5..7d45400cf 100644 --- a/docs/snippets/pkcs12-template-v1-external-secret.yaml +++ b/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 }}" diff --git a/pkg/controllers/externalsecret/externalsecret_controller.go b/pkg/controllers/externalsecret/externalsecret_controller.go index b9a948976..d18e1a4b8 100644 --- a/pkg/controllers/externalsecret/externalsecret_controller.go +++ b/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) - - // 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 - } - } } else { - // update the secret, if it exists + // 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 + } + + // 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) } } @@ -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 } diff --git a/pkg/controllers/externalsecret/externalsecret_controller_secret.go b/pkg/controllers/externalsecret/externalsecret_controller_secret.go index 00c99bf71..ff9ccf8a1 100644 --- a/pkg/controllers/externalsecret/externalsecret_controller_secret.go +++ b/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 } diff --git a/pkg/controllers/externalsecret/externalsecret_controller_template.go b/pkg/controllers/externalsecret/externalsecret_controller_template.go index 19eb8c48a..cbbbf2854 100644 --- a/pkg/controllers/externalsecret/externalsecret_controller_template.go +++ b/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) diff --git a/pkg/template/engine.go b/pkg/template/engine.go index b406a962b..be7298fb0 100644 --- a/pkg/template/engine.go +++ b/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) } diff --git a/pkg/template/v1/template.go b/pkg/template/v1/template.go index df49cbf7b..f703147b9 100644 --- a/pkg/template/v1/template.go +++ b/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 } diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index 7a6485931..7af21c1fc 100644 --- a/pkg/utils/utils.go +++ b/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.