diff --git a/Makefile b/Makefile index d71ac38a..c38376b6 100644 --- a/Makefile +++ b/Makefile @@ -177,7 +177,12 @@ ensure-cert-manager: ensure-kind $(BINDIR)/kubeconfig.yaml | $(BINDIR)/helm-$(HE .PHONY: ensure-trust-manager ensure-trust-manager: ensure-kind kind-load ensure-cert-manager | $(BINDIR)/helm-$(HELM_VERSION)/helm ## ensure trust-manager is available on cluster, built from local checkout $(BINDIR)/helm-$(HELM_VERSION)/helm uninstall --kubeconfig $(BINDIR)/kubeconfig.yaml -n cert-manager trust-manager || : - $(BINDIR)/helm-$(HELM_VERSION)/helm upgrade --kubeconfig $(BINDIR)/kubeconfig.yaml -i -n cert-manager trust-manager deploy/charts/trust-manager/. --set image.tag=latest --set defaultTrustPackage.tag=latest$(DEBIAN_TRUST_PACKAGE_SUFFIX) --set app.logLevel=2 --wait + $(BINDIR)/helm-$(HELM_VERSION)/helm upgrade --kubeconfig $(BINDIR)/kubeconfig.yaml -i -n cert-manager trust-manager deploy/charts/trust-manager/. \ + --set image.tag=latest \ + --set defaultTrustPackage.tag=latest$(DEBIAN_TRUST_PACKAGE_SUFFIX) \ + --set app.logLevel=2 \ + --set secretTargets.enabled=true --set secretTargets.authorizedSecretsAll=true \ + --wait # When running in our CI environment the Docker network's subnet choice # causees issues with routing. diff --git a/deploy/charts/trust-manager/README.md b/deploy/charts/trust-manager/README.md index 7706ba57..8585c86d 100644 --- a/deploy/charts/trust-manager/README.md +++ b/deploy/charts/trust-manager/README.md @@ -58,5 +58,8 @@ Kubernetes: `>= 1.25.0-0` | nodeSelector | object | `{"kubernetes.io/os":"linux"}` | Configure the nodeSelector; defaults to any Linux node (trust-manager doesn't support Windows nodes) | | replicaCount | int | `1` | Number of replicas of trust-manager to run. | | resources | object | `{}` | | +| secretTargets.authorizedSecrets | list | `[]` | A list of secret names which trust-manager will be permitted to read and write across all namespaces. These will be the only allowable Secrets that can be used as targets. If the list is empty, trust-manager will not be able to write to secrets and will only be able to read secrets in the trust namespace for use as sources. | +| secretTargets.authorizedSecretsAll | bool | `false` | If set to true, grant read/write permission to all secrets across the cluster. Use with caution! When set, ignores authorizedSecrets list setting. | +| secretTargets.enabled | bool | `false` | If set to true, enable writing trust bundles to Kubernetes Secrets as a target. trust-manager can only write to secrets which are explicitly allowed. - see either authorizedSecrets or authorizedSecretsAll. | | tolerations | list | `[]` | List of Kubernetes Tolerations; see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#toleration-v1-core | | topologySpreadConstraints | list | `[]` | List of Kubernetes TopologySpreadConstraints; see https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#topologyspreadconstraint-v1-core | diff --git a/deploy/charts/trust-manager/templates/clusterrole.yaml b/deploy/charts/trust-manager/templates/clusterrole.yaml index 4a483c02..02c71049 100644 --- a/deploy/charts/trust-manager/templates/clusterrole.yaml +++ b/deploy/charts/trust-manager/templates/clusterrole.yaml @@ -43,3 +43,25 @@ rules: resources: - "events" verbs: ["create", "patch"] + +- apiGroups: + - "" + resources: + - "secrets" + verbs: ["create", "list", "watch"] +{{- if .Values.secretTargets.enabled }} +{{- if .Values.secretTargets.authorizedSecretsAll }} +- apiGroups: + - "" + resources: + - "secrets" + verbs: ["get", "update", "delete", "patch"] +{{- else if .Values.secretTargets.authorizedSecrets }} +- apiGroups: + - "" + resources: + - "secrets" + verbs: ["get", "update", "delete", "patch"] + resourceNames: {{ .Values.secretTargets.authorizedSecrets | toYaml | nindent 2 }} +{{- end -}} +{{- end -}} diff --git a/deploy/charts/trust-manager/templates/trust.cert-manager.io_bundles.yaml b/deploy/charts/trust-manager/templates/trust.cert-manager.io_bundles.yaml index cd5ec7e7..678e2c48 100644 --- a/deploy/charts/trust-manager/templates/trust.cert-manager.io_bundles.yaml +++ b/deploy/charts/trust-manager/templates/trust.cert-manager.io_bundles.yaml @@ -136,6 +136,15 @@ spec: type: object additionalProperties: type: string + secret: + description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace. + type: object + required: + - key + properties: + key: + description: Key is the key of the entry in the object's `data` field to be used. + type: string status: description: Status of the Bundle. This is set and managed automatically. type: object @@ -220,6 +229,15 @@ spec: type: object additionalProperties: type: string + secret: + description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace. + type: object + required: + - key + properties: + key: + description: Key is the key of the entry in the object's `data` field to be used. + type: string served: true storage: true subresources: diff --git a/deploy/charts/trust-manager/values.yaml b/deploy/charts/trust-manager/values.yaml index a47bf158..104aceed 100644 --- a/deploy/charts/trust-manager/values.yaml +++ b/deploy/charts/trust-manager/values.yaml @@ -101,6 +101,20 @@ app: # -- If false, disables the default seccomp profile, which might be required to run on certain platforms seccompProfileEnabled: true +secretTargets: + # -- If set to true, enable writing trust bundles to Kubernetes Secrets as a target. + # trust-manager can only write to secrets which are explicitly allowed. + # - see either authorizedSecrets or authorizedSecretsAll. + enabled: false + # -- If set to true, grant read/write permission to all secrets across the cluster. Use with caution! + # When set, ignores authorizedSecrets list setting. + authorizedSecretsAll: false + # -- A list of secret names which trust-manager will be permitted to read and write across all namespaces. + # These will be the only allowable Secrets that can be used as targets. + # If the list is empty, trust-manager will not be able to write to secrets and will only be able to + # read secrets in the trust namespace for use as sources. + authorizedSecrets: [] + resources: {} # -- Kubernetes pod resource limits for trust. # limits: diff --git a/deploy/crds/trust.cert-manager.io_bundles.yaml b/deploy/crds/trust.cert-manager.io_bundles.yaml index ca58140c..b5d9b73d 100644 --- a/deploy/crds/trust.cert-manager.io_bundles.yaml +++ b/deploy/crds/trust.cert-manager.io_bundles.yaml @@ -135,6 +135,15 @@ spec: type: object additionalProperties: type: string + secret: + description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace. + type: object + required: + - key + properties: + key: + description: Key is the key of the entry in the object's `data` field to be used. + type: string status: description: Status of the Bundle. This is set and managed automatically. type: object @@ -219,6 +228,15 @@ spec: type: object additionalProperties: type: string + secret: + description: Secret is the target Secret that all Bundle source data will be synced to. Using Secrets as targets is only supported if enabled at trust-manager startup. By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace. + type: object + required: + - key + properties: + key: + description: Key is the key of the entry in the object's `data` field to be used. + type: string served: true storage: true subresources: diff --git a/pkg/apis/trust/v1alpha1/types_bundle.go b/pkg/apis/trust/v1alpha1/types_bundle.go index fd6ae052..0ab3e8d6 100644 --- a/pkg/apis/trust/v1alpha1/types_bundle.go +++ b/pkg/apis/trust/v1alpha1/types_bundle.go @@ -98,6 +98,11 @@ type BundleTarget struct { // data will be synced to. ConfigMap *KeySelector `json:"configMap,omitempty"` + // Secret is the target Secret that all Bundle source data will be synced to. + // Using Secrets as targets is only supported if enabled at trust-manager startup. + // By default, trust-manager has no permissions for writing to secrets and can only read secrets in the trust namespace. + Secret *KeySelector `json:"secret,omitempty"` + // AdditionalFormats specifies any additional formats to write to the target // +optional AdditionalFormats *AdditionalFormats `json:"additionalFormats,omitempty"` diff --git a/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go index e0083e4a..2ee173e1 100644 --- a/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/trust/v1alpha1/zz_generated.deepcopy.go @@ -225,6 +225,11 @@ func (in *BundleTarget) DeepCopyInto(out *BundleTarget) { *out = new(KeySelector) **out = **in } + if in.Secret != nil { + in, out := &in.Secret, &out.Secret + *out = new(KeySelector) + **out = **in + } if in.AdditionalFormats != nil { in, out := &in.AdditionalFormats, &out.AdditionalFormats *out = new(AdditionalFormats) diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go index 83c75503..3fe6a107 100644 --- a/pkg/bundle/bundle.go +++ b/pkg/bundle/bundle.go @@ -64,7 +64,7 @@ type bundle struct { // a direct Kubernetes client (only used for CSA to CSA migration) directClient client.Client - // targetCache is a cache.Cache that holds cached ConfigMap + // targetCache is a cache.Cache that holds cached ConfigMap and Secret // resources that are used as targets for Bundles. targetCache client.Reader @@ -204,7 +204,7 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result } // Find all old existing ConfigMap targetResources. - { + if bundle.Spec.Target.ConfigMap != nil { configMapList := &metav1.PartialObjectMetadataList{ TypeMeta: metav1.TypeMeta{ APIVersion: "v1", @@ -250,32 +250,105 @@ func (b *bundle) reconcileBundle(ctx context.Context, req ctrl.Request) (result } } + // Find all old existing Secret targetResources. + if bundle.Spec.Target.Secret != nil { + secretLists := &metav1.PartialObjectMetadataList{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, + } + err := b.targetCache.List(ctx, secretLists, &client.ListOptions{ + LabelSelector: labels.SelectorFromSet(map[string]string{ + trustapi.BundleLabelKey: bundle.Name, + }), + }) + if err != nil { + log.Error(err, "failed to list secrets") + b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SecretListError", "Failed to list secrets: %s", err) + return ctrl.Result{}, nil, fmt.Errorf("failed to list Secrets: %w", err) + } + + for _, secret := range secretLists.Items { + key := types.NamespacedName{ + Name: secret.Name, + Namespace: secret.Namespace, + } + + secretLog := log.WithValues("secret", key) + + if _, ok := targetResources[key]; ok { + // This Secret is still a target, so we don't need to remove it. + continue + } + + // Don't reconcile target for Secrets that are being deleted. + if secret.GetDeletionTimestamp() != nil { + secretLog.V(2).WithValues("deletionTimestamp", secret.GetDeletionTimestamp()).Info("skipping sync for secret as it is being deleted") + continue + } + + if !metav1.IsControlledBy(&secret, &bundle) { + secretLog.V(2).Info("skipping sync for configmap as it is not controlled by bundle") + continue + } + + targetResources[key] = false + } + } + var needsUpdate bool for target, shouldExist := range targetResources { targetLog := log.WithValues("target", target) + var cmSynced, secretSynced bool + var err error + + if bundle.Spec.Target.ConfigMap != nil { + cmSynced, err = b.syncConfigMapTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle.data, shouldExist) + if err != nil { + targetLog.Error(err, "failed sync bundle to ConfigMap target namespace") + b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SyncConfigMapTargetFailed", "Failed to sync target in Namespace %q: %s", target.Namespace, err) + + b.setBundleCondition( + bundle.Status.Conditions, + &statusPatch.Conditions, + trustapi.BundleCondition{ + Type: trustapi.BundleConditionSynced, + Status: metav1.ConditionFalse, + Reason: "SyncConfigMapTargetFailed", + Message: fmt.Sprintf("Failed to sync bundle to namespace %q: %s", target.Namespace, err), + ObservedGeneration: bundle.Generation, + }, + ) + + return ctrl.Result{Requeue: true}, statusPatch, nil + } + } - synced, err := b.syncTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle.data, shouldExist) - if err != nil { - targetLog.Error(err, "failed sync bundle to target namespace") - b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SyncTargetFailed", "Failed to sync target in Namespace %q: %s", target.Namespace, err) - - b.setBundleCondition( - bundle.Status.Conditions, - &statusPatch.Conditions, - trustapi.BundleCondition{ - Type: trustapi.BundleConditionSynced, - Status: metav1.ConditionFalse, - Reason: "SyncTargetFailed", - Message: fmt.Sprintf("Failed to sync bundle to namespace %q: %s", target.Namespace, err), - ObservedGeneration: bundle.Generation, - }, - ) - - return ctrl.Result{Requeue: true}, statusPatch, nil + if bundle.Spec.Target.Secret != nil { + secretSynced, err = b.syncSecretTarget(ctx, targetLog, &bundle, target.Name, target.Namespace, resolvedBundle.data, shouldExist) + if err != nil { + targetLog.Error(err, "failed sync bundle to Secret target namespace") + b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SyncSecretTargetFailed", "Failed to sync target in Namespace %q: %s", target.Namespace, err) + + b.setBundleCondition( + bundle.Status.Conditions, + &statusPatch.Conditions, + trustapi.BundleCondition{ + Type: trustapi.BundleConditionSynced, + Status: metav1.ConditionFalse, + Reason: "SyncSecretTargetFailed", + Message: fmt.Sprintf("Failed to sync bundle to namespace %q: %s", target.Namespace, err), + ObservedGeneration: bundle.Generation, + }, + ) + + return ctrl.Result{Requeue: true}, statusPatch, nil + } } - if synced { + if cmSynced || secretSynced { // We need to update if any target is synced. needsUpdate = true } diff --git a/pkg/bundle/controller.go b/pkg/bundle/controller.go index 10d98e43..b38065bb 100644 --- a/pkg/bundle/controller.go +++ b/pkg/bundle/controller.go @@ -98,6 +98,19 @@ func AddBundleController( builder.OnlyMetadata, ). + // Reconcile a Bundle on events against a Secret that it + // owns. Only cache Secret metadata. + WatchesRawSource( + source.Kind(targetCache, &corev1.Secret{}), + handler.EnqueueRequestForOwner( + mgr.GetScheme(), + mgr.GetRESTMapper(), + &trustapi.Bundle{}, + handler.OnlyControllerOwner(), + ), + builder.OnlyMetadata, + ). + ////// Sources ////// // Reconcile trust.cert-manager.io Bundles diff --git a/pkg/bundle/internal/ssa_client/configmap.go b/pkg/bundle/internal/ssa_client/configmap.go index c69c3076..346085dd 100644 --- a/pkg/bundle/internal/ssa_client/configmap.go +++ b/pkg/bundle/internal/ssa_client/configmap.go @@ -47,3 +47,26 @@ func GenerateConfigMapPatch( return configmap, applyPatch{encodedPatch}, nil } + +func GenerateSecretPatch( + secretPatch *coreapplyconfig.SecretApplyConfiguration, +) (*corev1.Secret, client.Patch, error) { + if secretPatch == nil || secretPatch.Name == nil || secretPatch.Namespace == nil { + panic("secretPatch must be non-nil and have a name and namespace") + } + + // This object is used to deduce the name & namespace + unmarshall the return value in + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: *secretPatch.Name, + Namespace: *secretPatch.Namespace, + }, + } + + encodedPatch, err := json.Marshal(secretPatch) + if err != nil { + return secret, nil, err + } + + return secret, applyPatch{encodedPatch}, nil +} diff --git a/pkg/bundle/sync.go b/pkg/bundle/sync.go index 226013fb..c00a343c 100644 --- a/pkg/bundle/sync.go +++ b/pkg/bundle/sync.go @@ -265,11 +265,11 @@ func (e pkcs12Encoder) encode(trustBundle string) ([]byte, error) { return pkcs12.EncodeTrustStoreEntries(rand.Reader, entries, e.password) } -// syncTarget syncs the given data to the target ConfigMap in the given namespace. +// syncConfigMapTarget syncs the given data to the target ConfigMap in the given namespace. // The name of the ConfigMap is the same as the Bundle. // Ensures the ConfigMap is owned by the given Bundle, and the data is up to date. // Returns true if the ConfigMap has been created or was updated. -func (b *bundle) syncTarget( +func (b *bundle) syncConfigMapTarget( ctx context.Context, log logr.Logger, bundle *trustapi.Bundle, @@ -331,94 +331,131 @@ func (b *bundle) syncTarget( target.ConfigMap.Key: data, } configmapBinData := map[string][]byte{} + if err := populateAdditionalFormatData(data, target, configmapBinData); err != nil { + return false, err + } - if target.AdditionalFormats != nil { - if target.AdditionalFormats.JKS != nil { - encoded, err := jksEncoder{password: []byte(DefaultJKSPassword)}.encode(data) - if err != nil { - return false, fmt.Errorf("failed to encode JKS: %w", err) - } - - configmapBinData[target.AdditionalFormats.JKS.Key] = encoded + // If the ConfigMap doesn't exist, create it. + if !apierrors.IsNotFound(err) { + // Exit early if no update is needed + if exit, err := b.needsUpdate(ctx, log, configMap, bundle, dataHash); err != nil { + return false, err + } else if !exit { + return false, nil } + } - if target.AdditionalFormats.PKCS12 != nil { - encoded, err := pkcs12Encoder{password: DefaultPKCS12Password}.encode(data) - if err != nil { - return false, fmt.Errorf("failed to encode PKCS12: %w", err) - } + configMapPatch := coreapplyconfig. + ConfigMap(name, namespace). + WithLabels(map[string]string{ + trustapi.BundleLabelKey: bundle.Name, + }). + WithAnnotations(map[string]string{ + trustapi.BundleHashAnnotationKey: dataHash, + }). + WithOwnerReferences( + v1.OwnerReference(). + WithAPIVersion(trustapi.SchemeGroupVersion.String()). + WithKind(trustapi.BundleKind). + WithName(bundle.GetName()). + WithUID(bundle.GetUID()). + WithBlockOwnerDeletion(true). + WithController(true), + ). + WithData(configmapData). + WithBinaryData(configmapBinData) - configmapBinData[target.AdditionalFormats.PKCS12.Key] = encoded - } + if err = b.patchResource(ctx, configMapPatch); err != nil { + return false, fmt.Errorf("failed to patch configMap %s/%s: %w", namespace, bundle.Name, err) } - // If the ConfigMap doesn't exist, create it. - needsPatch := apierrors.IsNotFound(err) - if !needsPatch { - if !metav1.IsControlledBy(configMap, bundle) { - needsPatch = true - } + log.V(2).Info("synced bundle to namespace for target ConfigMap") - if configMap.Labels[trustapi.BundleLabelKey] != bundle.Name { - needsPatch = true - } + return true, nil +} - if configMap.Annotations[trustapi.BundleHashAnnotationKey] != dataHash { - needsPatch = true - } +// syncSecretTarget syncs the given data to the target Secret in the given namespace. +// The name of the Secret is the same as the Bundle. +// Ensures the Secret is owned by the given Bundle, and the data is up to date. +// Returns true if the Secret has been created or was updated. +func (b *bundle) syncSecretTarget( + ctx context.Context, + log logr.Logger, + bundle *trustapi.Bundle, + name string, + namespace string, + data string, + shouldExist bool, +) (bool, error) { + secret := &metav1.PartialObjectMetadata{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + } + err := b.targetCache.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, secret) + if err != nil && !apierrors.IsNotFound(err) { + return false, fmt.Errorf("failed to get Secret %s/%s: %w", namespace, name, err) + } - { - properties, err := listManagedProperties(configMap, fieldManager, "data") - if err != nil { - return false, fmt.Errorf("failed to list managed properties: %w", err) - } + // If the target obj exists, but the Bundle is being deleted, delete the Secret. + if apierrors.IsNotFound(err) && !shouldExist { + return false, nil + } - expectedProperties := sets.New[string](target.ConfigMap.Key) + // If the Secret should not exist, but it does, delete it. + if !apierrors.IsNotFound(err) && !shouldExist { + // apply empty patch to remove the key + secretPatch := coreapplyconfig. + Secret(name, namespace). + WithLabels(map[string]string{ + trustapi.BundleLabelKey: bundle.Name, + }). + WithOwnerReferences( + v1.OwnerReference(). + WithAPIVersion(trustapi.SchemeGroupVersion.String()). + WithKind(trustapi.BundleKind). + WithName(bundle.GetName()). + WithUID(bundle.GetUID()). + WithBlockOwnerDeletion(true). + WithController(true), + ) - if !properties.Equal(expectedProperties) { - needsPatch = true - } + if err = b.patchSecretResource(ctx, secretPatch); err != nil { + return false, fmt.Errorf("failed to patch secret %s/%s: %w", namespace, bundle.Name, err) } - { - properties, err := listManagedProperties(configMap, fieldManager, "binaryData") - if err != nil { - return false, fmt.Errorf("failed to list managed properties: %w", err) - } - - expectedProperties := sets.New[string]() - - if target.AdditionalFormats != nil && target.AdditionalFormats.JKS != nil { - expectedProperties.Insert(target.AdditionalFormats.JKS.Key) - } + return true, nil + } - if target.AdditionalFormats != nil && target.AdditionalFormats.PKCS12 != nil { - expectedProperties.Insert(target.AdditionalFormats.PKCS12.Key) - } + target := bundle.Spec.Target + if target.Secret == nil { + return false, errors.New("target not defined") + } - if !properties.Equal(expectedProperties) { - needsPatch = true - } - } + // Generated JKS is not deterministic - best we can do here is update if the pem cert has + // changed (hence not checking if JKS matches) + dataHash := fmt.Sprintf("%x", sha256.Sum256([]byte(data))) + targetData := map[string][]byte{ + target.Secret.Key: []byte(data), + } - if bundle.Status.Target != nil && bundle.Status.Target.ConfigMap != nil { - // Check if we need to migrate the ConfigMap managed fields to the Apply field operation - if didMigrate, err := b.migrateConfigMapToApply(ctx, configMap, bundle.Status.Target.ConfigMap.Key); err != nil { - return false, fmt.Errorf("failed to migrate ConfigMap %s/%s to Apply: %w", namespace, name, err) - } else if didMigrate { - log.V(2).Info("migrated configmap from CSA to SSA") - needsPatch = true - } - } + if additionalFormatsErr := populateAdditionalFormatData(data, target, targetData); additionalFormatsErr != nil { + return false, err } - // Exit early if no update is needed - if !needsPatch { - return false, nil + // If the Secret doesn't exist, create it. + if !apierrors.IsNotFound(err) { + // Exit early if no update is needed + if exit, err := b.needsUpdate(ctx, log, secret, bundle, dataHash); err != nil { + return false, err + } else if !exit { + return false, nil + } } - configMapPatch := coreapplyconfig. - ConfigMap(name, namespace). + secretPatch := coreapplyconfig. + Secret(name, namespace). WithLabels(map[string]string{ trustapi.BundleLabelKey: bundle.Name, }). @@ -434,18 +471,95 @@ func (b *bundle) syncTarget( WithBlockOwnerDeletion(true). WithController(true), ). - WithData(configmapData). - WithBinaryData(configmapBinData) + WithData(targetData) - if err = b.patchResource(ctx, configMapPatch); err != nil { - return false, fmt.Errorf("failed to patch configMap %s/%s: %w", namespace, bundle.Name, err) + if err = b.patchSecretResource(ctx, secretPatch); err != nil { + return false, fmt.Errorf("failed to patch Secret %s/%s: %w", namespace, bundle.Name, err) } - log.V(2).Info("synced bundle to namespace") + log.V(2).Info("synced bundle to namespace for target secret") return true, nil } +func populateAdditionalFormatData(data string, target trustapi.BundleTarget, targetMap map[string][]byte) error { + if target.AdditionalFormats != nil { + if target.AdditionalFormats.JKS != nil { + encoded, err := jksEncoder{password: []byte(DefaultJKSPassword)}.encode(data) + if err != nil { + return fmt.Errorf("failed to encode JKS: %w", err) + } + targetMap[target.AdditionalFormats.JKS.Key] = encoded + } + + if target.AdditionalFormats.PKCS12 != nil { + encoded, err := pkcs12Encoder{password: DefaultPKCS12Password}.encode(data) + if err != nil { + return fmt.Errorf("failed to encode PKCS12: %w", err) + } + targetMap[target.AdditionalFormats.PKCS12.Key] = encoded + } + } + return nil +} + +func (b *bundle) needsUpdate(ctx context.Context, log logr.Logger, obj *metav1.PartialObjectMetadata, bundle *trustapi.Bundle, dataHash string) (bool, error) { + needsUpdate := false + if !metav1.IsControlledBy(obj, bundle) { + needsUpdate = true + } + + if obj.GetLabels()[trustapi.BundleLabelKey] != bundle.Name { + needsUpdate = true + } + + if obj.GetAnnotations()[trustapi.BundleHashAnnotationKey] != dataHash { + needsUpdate = true + } + + { + properties, err := listManagedProperties(obj, fieldManager, "data") + if err != nil { + return false, fmt.Errorf("failed to list managed properties: %w", err) + } + + key := "" + targetType := obj.TypeMeta.Kind + if targetType == "ConfigMap" { + key = bundle.Spec.Target.ConfigMap.Key + } else if targetType == "Secret" { + key = bundle.Spec.Target.Secret.Key + } else { + return false, fmt.Errorf("unknown targetType: %s", targetType) + } + expectedProperties := sets.New[string](key) + if bundle.Spec.Target.AdditionalFormats != nil && bundle.Spec.Target.AdditionalFormats.JKS != nil { + expectedProperties.Insert(bundle.Spec.Target.AdditionalFormats.JKS.Key) + } + + if bundle.Spec.Target.AdditionalFormats != nil && bundle.Spec.Target.AdditionalFormats.PKCS12 != nil { + expectedProperties.Insert(bundle.Spec.Target.AdditionalFormats.PKCS12.Key) + } + + if !properties.Equal(expectedProperties) { + needsUpdate = true + } + + if targetType == "ConfigMap" { + if bundle.Status.Target != nil && bundle.Status.Target.ConfigMap != nil { + // Check if we need to migrate the ConfigMap managed fields to the Apply field operation + if didMigrate, err := b.migrateConfigMapToApply(ctx, obj, bundle.Status.Target.ConfigMap.Key); err != nil { + return false, fmt.Errorf("failed to migrate ConfigMap %s/%s to Apply: %w", obj.Namespace, obj.Name, err) + } else if didMigrate { + log.V(2).Info("migrated configmap from CSA to SSA") + needsUpdate = true + } + } + } + } + return needsUpdate, nil +} + func listManagedProperties(configmap *metav1.PartialObjectMetadata, fieldManager string, fieldName string) (sets.Set[string], error) { properties := sets.New[string]() @@ -507,6 +621,37 @@ func (b *bundle) patchResource(ctx context.Context, obj interface{}) error { return nil } +func (b *bundle) patchSecretResource(ctx context.Context, obj interface{}) error { + if b.patchResourceOverwrite != nil { + return b.patchResourceOverwrite(ctx, obj) + } + + applyConfig, ok := obj.(*coreapplyconfig.SecretApplyConfiguration) + if !ok { + return fmt.Errorf("expected *coreapplyconfig.ConfigMapApplyConfiguration, got %T", obj) + } + + secret, patch, err := ssa_client.GenerateSecretPatch(applyConfig) + if err != nil { + return fmt.Errorf("failed to generate patch: %w", err) + } + + err = b.client.Patch(ctx, secret, patch, &client.PatchOptions{ + FieldManager: fieldManager, + Force: ptr.To(true), + }) + if err != nil { + return err + } + + // If the secret is empty, delete it + if len(secret.Data) == 0 { + return b.client.Delete(ctx, secret) + } + + return nil +} + // MIGRATION: This is a migration function that migrates the ownership of // fields from the Update operation to the Apply operation. This is required // to ensure that the apply operations will also remove fields that were diff --git a/pkg/bundle/sync_test.go b/pkg/bundle/sync_test.go index 91cd33da..7a4d308d 100644 --- a/pkg/bundle/sync_test.go +++ b/pkg/bundle/sync_test.go @@ -46,6 +46,14 @@ import ( "github.com/cert-manager/trust-manager/test/dummy" ) +const ( + bundleName = "test-bundle" + key = "trust.pem" + jksKey = "trust.jks" + pkcs12Key = "trust.p12" + data = dummy.TestCertificate1 +) + func managedFieldEntries(fields []string, dataFields []string) []metav1.ManagedFieldsEntry { fieldset := fieldpath.NewSet() for _, property := range fields { @@ -75,14 +83,7 @@ func managedFieldEntries(fields []string, dataFields []string) []metav1.ManagedF } } -func Test_syncTarget(t *testing.T) { - const ( - bundleName = "test-bundle" - key = "trust.pem" - jksKey = "trust.jks" - pkcs12Key = "trust.p12" - data = dummy.TestCertificate1 - ) +func Test_syncConfigMapTarget(t *testing.T) { dataHash := fmt.Sprintf("%x", sha256.Sum256([]byte(data))) tests := map[string]struct { @@ -624,7 +625,7 @@ func Test_syncTarget(t *testing.T) { spec.Target.AdditionalFormats.PKCS12 = &trustapi.KeySelector{Key: pkcs12Key} } - needsUpdate, err := b.syncTarget(context.TODO(), klogr.New(), &trustapi.Bundle{ + needsUpdate, err := b.syncConfigMapTarget(context.TODO(), klogr.New(), &trustapi.Bundle{ ObjectMeta: metav1.ObjectMeta{Name: bundleName}, Spec: spec, }, bundleName, test.namespace.Name, data, test.shouldExist) @@ -707,6 +708,634 @@ func Test_syncTarget(t *testing.T) { } } +func Test_syncSecretTarget(t *testing.T) { + dataHash := fmt.Sprintf("%x", sha256.Sum256([]byte(data))) + const ( + bundleName = "test-bundle" + key = "key" + data = dummy.TestCertificate1 + ) + + tests := map[string]struct { + object runtime.Object + namespace corev1.Namespace + shouldExist bool + // Add JKS to AdditionalFormats + withJKS bool + // Add PKCS12 to AdditionalFormats + withPKCS12 bool + // Expect the secret to exist at the end of the sync. + expExists bool + // Expect JKS to exist in the secret at the end of the sync. + expJKS bool + // Expect PKCS12 to exist in the secret at the end of the sync. + expPKCS12 bool + expEvent string + // Expect the owner reference of the secret to point to the bundle. + expOwnerReference bool + expNeedsUpdate bool + }{ + "if object doesn't exist, expect update": { + object: nil, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object doesn't exist with JKS, expect update": { + object: nil, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withJKS: true, + expExists: true, + expJKS: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object doesn't exist with PKCS12, expect update": { + object: nil, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withPKCS12: true, + expExists: true, + expPKCS12: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists but without data or owner, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + ManagedFields: managedFieldEntries(nil, nil), + }, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with data but no owner, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with owner but no data, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + }, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with owner but wrong data, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: "wrong hash"}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte("wrong data")}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with owner but without JKS, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withJKS: true, + expExists: true, + expJKS: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with owner but without PKCS12, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withPKCS12: true, + expExists: true, + expPKCS12: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with owner but wrong key, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{"wrong key"}, nil), + }, + Data: map[string][]byte{"wrong key": []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with owner but wrong JKS key, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, []string{"wrong key"}), + }, + Data: map[string][]byte{ + key: []byte(data), + "wrong-key": []byte(data), + }, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withJKS: true, + expExists: true, + expJKS: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with owner but wrong PKCS12 key, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key, "wrong key"}, nil), + }, + Data: map[string][]byte{ + key: []byte(data), + "wrong-key": []byte(data), + }, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withPKCS12: true, + expExists: true, + expPKCS12: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with correct data, expect no update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: false, + }, + "if object exists without JKS, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withJKS: true, + expExists: true, + expJKS: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists without PKCS12, expect update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + withPKCS12: true, + expExists: true, + expPKCS12: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object exists with correct data and some extra data (not owned by our fieldmanager) and owner, expect no update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: "another-bundle", + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{ + key: []byte(data), + "another-key": []byte("another-data"), + }, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace"}}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: false, + }, + "if object doesn't exist and labels match, expect update": { + object: nil, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + Labels: map[string]string{"foo": "bar"}, + }}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: true, + }, + "if object doesn't exist and labels don't match, don't expect update": { + object: nil, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + Labels: map[string]string{"bar": "foo"}, + }}, + shouldExist: false, + expExists: false, + expOwnerReference: true, + expNeedsUpdate: false, + }, + "if object exists with correct data and labels match, expect no update": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + Labels: map[string]string{"foo": "bar"}, + }}, + shouldExist: true, + expExists: true, + expOwnerReference: true, + expNeedsUpdate: false, + }, + "if object exists with correct data but labels don't match, expect deletion": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + OwnerReferences: []metav1.OwnerReference{ + { + Kind: "Bundle", + APIVersion: "trust.cert-manager.io/v1alpha1", + Name: bundleName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + Labels: map[string]string{"bar": "foo"}, + }}, + shouldExist: false, + expExists: false, + expOwnerReference: false, + expNeedsUpdate: true, + }, + "if object exists and labels don't match, expect empty patch": { + object: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: bundleName, + Namespace: "test-namespace", + Labels: map[string]string{trustapi.BundleLabelKey: bundleName}, + Annotations: map[string]string{trustapi.BundleHashAnnotationKey: dataHash}, + ManagedFields: managedFieldEntries([]string{key}, nil), + }, + Data: map[string][]byte{key: []byte(data)}, + }, + namespace: corev1.Namespace{ObjectMeta: metav1.ObjectMeta{ + Name: "test-namespace", + Labels: map[string]string{"bar": "foo"}, + }}, + shouldExist: false, + expExists: false, + expOwnerReference: false, + expNeedsUpdate: true, + }, + } + + for name, test := range tests { + test := test + t.Run(name, func(t *testing.T) { + t.Parallel() + + clientBuilder := fakeclient.NewClientBuilder(). + WithScheme(trustapi.GlobalScheme) + if test.object != nil { + clientBuilder.WithRuntimeObjects(test.object) + } + + fakeclient := clientBuilder.Build() + fakerecorder := record.NewFakeRecorder(1) + + var ( + logMutex sync.Mutex + resourcePatches []interface{} + ) + + b := &bundle{ + client: fakeclient, + targetCache: fakeclient, + recorder: fakerecorder, + patchResourceOverwrite: func(ctx context.Context, obj interface{}) error { + logMutex.Lock() + defer logMutex.Unlock() + + resourcePatches = append(resourcePatches, obj) + return nil + }, + } + + spec := trustapi.BundleSpec{ + Target: trustapi.BundleTarget{ + Secret: &trustapi.KeySelector{Key: key}, + AdditionalFormats: &trustapi.AdditionalFormats{}, + }, + } + if test.withJKS { + spec.Target.AdditionalFormats.JKS = &trustapi.KeySelector{Key: jksKey} + } + if test.withPKCS12 { + spec.Target.AdditionalFormats.PKCS12 = &trustapi.KeySelector{Key: pkcs12Key} + } + + needsUpdate, err := b.syncSecretTarget(context.TODO(), klogr.New(), &trustapi.Bundle{ + ObjectMeta: metav1.ObjectMeta{Name: bundleName}, + Spec: spec, + }, bundleName, test.namespace.Name, data, test.shouldExist) + assert.NoError(t, err) + + assert.Equalf(t, test.expNeedsUpdate, needsUpdate, "unexpected needsUpdate, exp=%t got=%t", test.expNeedsUpdate, needsUpdate) + + if len(resourcePatches) > 1 { + t.Fatalf("expected only one patch, got %d", len(resourcePatches)) + } + + if len(resourcePatches) == 1 { + secret := resourcePatches[0].(*coreapplyconfig.SecretApplyConfiguration) + + if test.expExists { + assert.Equal(t, data, string(secret.Data[key])) + } + + expectedOwnerReference := metav1applyconfig. + OwnerReference(). + WithAPIVersion(trustapi.SchemeGroupVersion.String()). + WithKind(trustapi.BundleKind). + WithName(bundleName). + WithUID(""). + WithBlockOwnerDeletion(true). + WithController(true) + + if test.expOwnerReference { + assert.Equal(t, expectedOwnerReference, &secret.OwnerReferences[0]) + } else { + assert.NotContains(t, secret.OwnerReferences, expectedOwnerReference) + } + + jksData, jksExists := secret.Data[jksKey] + assert.Equal(t, test.expJKS, jksExists) + + if test.expJKS { + reader := bytes.NewReader(jksData) + + ks := jks.New() + err := ks.Load(reader, []byte(DefaultJKSPassword)) + assert.Nil(t, err) + + entryNames := ks.Aliases() + + assert.Len(t, entryNames, 1) + assert.True(t, ks.IsTrustedCertificateEntry(entryNames[0])) + + // Safe to ignore errors here, we've tested that it's present and a TrustedCertificateEntry + cert, _ := ks.GetTrustedCertificateEntry(entryNames[0]) + + // Only one certificate block for this test, so we can safely ignore the `remaining` byte array + p, _ := pem.Decode([]byte(data)) + assert.Equal(t, p.Bytes, cert.Certificate.Content) + } + + pkcs12Data, pkcs12Exists := secret.Data[pkcs12Key] + assert.Equal(t, test.expPKCS12, pkcs12Exists) + + if test.expPKCS12 { + cas, err := pkcs12.DecodeTrustStore(pkcs12Data, DefaultPKCS12Password) + assert.Nil(t, err) + assert.Len(t, cas, 1) + + // Only one certificate block for this test, so we can safely ignore the `remaining` byte array + p, _ := pem.Decode([]byte(data)) + assert.Equal(t, p.Bytes, cas[0].Raw) + } + } + + var event string + select { + case event = <-fakerecorder.Events: + default: + } + assert.Equal(t, test.expEvent, event) + }) + } +} + func Test_buildSourceBundle(t *testing.T) { tests := map[string]struct { bundle *trustapi.Bundle diff --git a/pkg/webhook/validation.go b/pkg/webhook/validation.go index d3a0f0eb..04119f6c 100644 --- a/pkg/webhook/validation.go +++ b/pkg/webhook/validation.go @@ -45,6 +45,29 @@ func (v *validator) ValidateCreate(ctx context.Context, obj runtime.Object) (adm } func (v *validator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + oldBundle, ok := oldObj.(*trustapi.Bundle) + if !ok { + return nil, fmt.Errorf("expected a Bundle, but got a %T", oldBundle) + } + newBundle, ok := newObj.(*trustapi.Bundle) + if !ok { + return nil, fmt.Errorf("expected a Bundle, but got a %T", newBundle) + } + + var ( + el field.ErrorList + path = field.NewPath("spec") + ) + // Target removal are not allowed. + if oldBundle.Spec.Target.ConfigMap != nil && newBundle.Spec.Target.ConfigMap == nil { + el = append(el, field.Invalid(path.Child("target", "configmap"), "", "target configMap removal is not allowed")) + return nil, el.ToAggregate() + } + // Target removal are not allowed. + if oldBundle.Spec.Target.Secret != nil && newBundle.Spec.Target.Secret == nil { + el = append(el, field.Invalid(path.Child("target", "secret"), "", "target secret removal is not allowed")) + return nil, el.ToAggregate() + } return v.validate(ctx, newObj) } @@ -141,13 +164,28 @@ func (v *validator) validate(ctx context.Context, obj runtime.Object) (admission } } - if configMap := bundle.Spec.Target.ConfigMap; configMap == nil { - el = append(el, field.Invalid(path.Child("target", "configMap"), configMap, "target configMap must be defined")) - } else if len(configMap.Key) == 0 { + configMap := bundle.Spec.Target.ConfigMap + secret := bundle.Spec.Target.Secret + + if configMap == nil && secret == nil { + el = append(el, field.Invalid(path.Child("target"), bundle.Spec.Target, "must define at least one target")) + } + + if configMap != nil && len(configMap.Key) == 0 { el = append(el, field.Invalid(path.Child("target", "configMap", "key"), configMap.Key, "target configMap key must be defined")) - } else if bundle.Spec.Target.AdditionalFormats != nil { - targetKeys := map[string]struct{}{ - configMap.Key: {}, + } + + if secret != nil && len(secret.Key) == 0 { + el = append(el, field.Invalid(path.Child("target", "secret", "key"), configMap.Key, "target secret key must be defined")) + } + + if bundle.Spec.Target.AdditionalFormats != nil { + targetKeys := map[string]struct{}{} + if configMap != nil { + targetKeys[configMap.Key] = struct{}{} + } + if secret != nil { + targetKeys[secret.Key] = struct{}{} } formats := map[string]*trustapi.KeySelector{ "jks": bundle.Spec.Target.AdditionalFormats.JKS, diff --git a/pkg/webhook/validation_test.go b/pkg/webhook/validation_test.go index 60625d23..f9c71fd6 100644 --- a/pkg/webhook/validation_test.go +++ b/pkg/webhook/validation_test.go @@ -33,9 +33,6 @@ import ( ) func Test_validate(t *testing.T) { - var ( - nilKeySelector *trustapi.KeySelector - ) tests := map[string]struct { bundle runtime.Object expErr *string @@ -51,7 +48,7 @@ func Test_validate(t *testing.T) { }, expErr: ptr.To(field.ErrorList{ field.Forbidden(field.NewPath("spec", "sources"), "must define at least one source"), - field.Invalid(field.NewPath("spec", "target", "configMap"), nilKeySelector, "target configMap must be defined"), + field.Invalid(field.NewPath("spec", "target"), trustapi.BundleTarget{}, "must define at least one target"), }.ToAggregate().Error()), }, "sources with multiple types defined in items": { @@ -384,3 +381,55 @@ func Test_validate(t *testing.T) { }) } } + +func Test_validate_update(t *testing.T) { + tests := map[string]struct { + oldBundle runtime.Object + newBundle runtime.Object + expErr *string + expWarnings admission.Warnings + }{ + "if the target configmap is removed during update": { + oldBundle: &trustapi.Bundle{ + ObjectMeta: metav1.ObjectMeta{Name: "testing"}, + Spec: trustapi.BundleSpec{ + Target: trustapi.BundleTarget{ + ConfigMap: &trustapi.KeySelector{ + Key: "bar", + }, + }, + }, + }, + newBundle: &trustapi.Bundle{}, + expErr: ptr.To("spec.target.configmap: Invalid value: \"\": target configMap removal is not allowed"), + }, + "if the target secret is removed during update": { + oldBundle: &trustapi.Bundle{ + ObjectMeta: metav1.ObjectMeta{Name: "testing"}, + Spec: trustapi.BundleSpec{ + Target: trustapi.BundleTarget{ + Secret: &trustapi.KeySelector{ + Key: "bar", + }, + }, + }, + }, + newBundle: &trustapi.Bundle{}, + expErr: ptr.To("spec.target.secret: Invalid value: \"\": target secret removal is not allowed"), + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + v := &validator{log: klogr.New()} + gotWarnings, gotErr := v.ValidateUpdate(context.TODO(), test.oldBundle, test.newBundle) + if test.expErr == nil && gotErr != nil { + t.Errorf("got an unexpected error: %v", gotErr) + } else if test.expErr != nil && (gotErr == nil || *test.expErr != gotErr.Error()) { + t.Errorf("wants error: %v got: %v", *test.expErr, gotErr) + } + assert.Equal(t, test.expWarnings, gotWarnings) + + }) + } +} diff --git a/test/env/data.go b/test/env/data.go index b55b1612..0e2ca021 100644 --- a/test/env/data.go +++ b/test/env/data.go @@ -39,7 +39,7 @@ import ( ) const ( - EventuallyTimeout = "30s" + EventuallyTimeout = "90s" EventuallyPollInterval = "100ms" ) @@ -78,7 +78,7 @@ func DefaultTrustData() TestData { // newTestBundle creates a new Bundle in the API using the input test data. // Returns the create Bundle object. -func NewTestBundle(ctx context.Context, cl client.Client, opts bundle.Options, td TestData) *trustapi.Bundle { +func newTestBundle(ctx context.Context, cl client.Client, opts bundle.Options, td TestData, targetType string) *trustapi.Bundle { By("creating trust Bundle") configMap := corev1.ConfigMap{ @@ -132,22 +132,53 @@ func NewTestBundle(ctx context.Context, cl client.Client, opts bundle.Options, t }, }, } + if targetType == "ConfigMap" { + bundle.Spec.Target = trustapi.BundleTarget{ + ConfigMap: &td.Target, + } + } else if targetType == "Secret" { + bundle.Spec.Target = trustapi.BundleTarget{ + Secret: &td.Target, + } + } Expect(cl.Create(ctx, &bundle)).NotTo(HaveOccurred()) return &bundle } +// NewTestBundleSecretTarget creates a new Bundle in the API using the input test data. +// Returns the create Bundle object. +func NewTestBundleSecretTarget(ctx context.Context, cl client.Client, opts bundle.Options, td TestData) *trustapi.Bundle { + return newTestBundle(ctx, cl, opts, td, "Secret") +} + +// newTestBundleConfigMapTarget creates a new Bundle in the API using the input test data with target set to ConfigMap. +// Returns the create Bundle object. +func NewTestBundleConfigMapTarget(ctx context.Context, cl client.Client, opts bundle.Options, td TestData) *trustapi.Bundle { + return newTestBundle(ctx, cl, opts, td, "ConfigMap") +} + func checkBundleSyncedInternal(ctx context.Context, cl client.Client, bundleName string, namespace string, comparator func(string) error) error { var bundle trustapi.Bundle Expect(cl.Get(ctx, client.ObjectKey{Name: bundleName}, &bundle)).NotTo(HaveOccurred()) - var configMap corev1.ConfigMap - if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: bundle.Name}, &configMap); err != nil { - return fmt.Errorf("failed to get configMap %s/%s when checking bundle sync: %w", namespace, bundle.Name, err) + gotData := "" + if bundle.Spec.Target.ConfigMap != nil { + var configMap corev1.ConfigMap + if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: bundle.Name}, &configMap); err != nil { + return fmt.Errorf("failed to get configMap %s/%s when checking bundle sync: %w", namespace, bundle.Name, err) + } + gotData = configMap.Data[bundle.Spec.Target.ConfigMap.Key] + } else if bundle.Spec.Target.Secret != nil { + var secret corev1.Secret + if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace, Name: bundle.Name}, &secret); err != nil { + return fmt.Errorf("failed to get secret %s/%s when checking bundle sync: %w", namespace, bundle.Name, err) + } + gotData = string(secret.Data[bundle.Spec.Target.Secret.Key]) + } else { + return fmt.Errorf("invalid bundle spec targets: %v", bundle.Spec.Target) } - gotData := configMap.Data[bundle.Spec.Target.ConfigMap.Key] - if err := comparator(gotData); err != nil { return fmt.Errorf("configMap %s/%s didn't have expected value: %w", namespace, bundle.Name, err) } diff --git a/test/integration/bundle/suite.go b/test/integration/bundle/suite.go index 9d3cc654..471c8aab 100644 --- a/test/integration/bundle/suite.go +++ b/test/integration/bundle/suite.go @@ -129,7 +129,7 @@ var _ = Describe("Integration", func() { By("Creating Bundle for test") testData = testenv.DefaultTrustData() - testBundle = testenv.NewTestBundle(ctx, cl, opts, testData) + testBundle = testenv.NewTestBundleConfigMapTarget(ctx, cl, opts, testData) testenv.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, dummy.DefaultJoinedCerts()) diff --git a/test/smoke/suite_test.go b/test/smoke/suite_test.go index 827a9df6..4eefa0d2 100644 --- a/test/smoke/suite_test.go +++ b/test/smoke/suite_test.go @@ -41,7 +41,7 @@ const ( ) var _ = Describe("Smoke", func() { - It("should create a bundle, sync to target, and then remove all when deleted", func() { + It("should create a bundle, sync to ConfigMap target, and then remove all configmap targets when deleted", func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -53,7 +53,7 @@ var _ = Describe("Smoke", func() { By("Creating Bundle for test") testData := env.DefaultTrustData() - testBundle := env.NewTestBundle(ctx, cl, bundle.Options{ + testBundle := env.NewTestBundleConfigMapTarget(ctx, cl, bundle.Options{ Log: klogr.New(), Namespace: cnf.TrustNamespace, }, testData) @@ -61,99 +61,136 @@ var _ = Describe("Smoke", func() { By("Ensuring the Bundle has Synced") env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, dummy.DefaultJoinedCerts()) - By("Ensuring targets update when a ConfigMap source is updated") - var configMap corev1.ConfigMap + testBundleCommon(ctx, cl, testBundle) + }) + + It("should create a bundle, sync to Secret target, and then remove all secret targets when deleted", func() { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cl, err := client.New(cnf.RestConfig, client.Options{ + Scheme: trustapi.GlobalScheme, + }) + Expect(err).NotTo(HaveOccurred()) + + By("Creating Bundle for test") + testData := env.DefaultTrustData() + + testBundle := env.NewTestBundleSecretTarget(ctx, cl, bundle.Options{ + Log: klogr.New(), + Namespace: cnf.TrustNamespace, + }, testData) - Expect(cl.Get(ctx, client.ObjectKey{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[0].ConfigMap.Name}, &configMap)).NotTo(HaveOccurred()) + By("Ensuring the Bundle has Synced") + env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, dummy.DefaultJoinedCerts()) - configMap.Data[testData.Sources.ConfigMap.Key] = dummy.TestCertificate4 + testBundleCommon(ctx, cl, testBundle) + }) +}) - Expect(cl.Update(ctx, &configMap)).NotTo(HaveOccurred()) +func testBundleCommon(ctx context.Context, cl client.Client, testBundle *trustapi.Bundle) { + testData := env.DefaultTrustData() + By("Ensuring targets update when a ConfigMap source is updated") + var configMap corev1.ConfigMap - env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, dummy.JoinCerts(dummy.TestCertificate4, dummy.TestCertificate2, dummy.TestCertificate3)) + Expect(cl.Get(ctx, client.ObjectKey{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[0].ConfigMap.Name}, &configMap)).NotTo(HaveOccurred()) - By("Ensuring targets update when a Secret source is updated") - var secret corev1.Secret + configMap.Data[testData.Sources.ConfigMap.Key] = dummy.TestCertificate4 - Expect(cl.Get(ctx, client.ObjectKey{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[1].Secret.Name}, &secret)).NotTo(HaveOccurred()) + Expect(cl.Update(ctx, &configMap)).NotTo(HaveOccurred()) - secret.Data[testData.Sources.Secret.Key] = []byte(dummy.TestCertificate1) + env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, dummy.JoinCerts(dummy.TestCertificate4, dummy.TestCertificate2, dummy.TestCertificate3)) - Expect(cl.Update(ctx, &secret)).NotTo(HaveOccurred()) + By("Ensuring targets update when a Secret source is updated") + var secret corev1.Secret - env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, dummy.JoinCerts(dummy.TestCertificate4, dummy.TestCertificate1, dummy.TestCertificate3)) + Expect(cl.Get(ctx, client.ObjectKey{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[1].Secret.Name}, &secret)).NotTo(HaveOccurred()) - By("Ensuring targets update when an InLine source is updated") - Expect(cl.Get(ctx, client.ObjectKey{Name: testBundle.Name}, testBundle)).NotTo(HaveOccurred()) + secret.Data[testData.Sources.Secret.Key] = []byte(dummy.TestCertificate1) - testBundle.Spec.Sources[2].InLine = ptr.To(dummy.TestCertificate2) + Expect(cl.Update(ctx, &secret)).NotTo(HaveOccurred()) - Expect(cl.Update(ctx, testBundle)).NotTo(HaveOccurred()) + env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, dummy.JoinCerts(dummy.TestCertificate4, dummy.TestCertificate1, dummy.TestCertificate3)) - newBundle := dummy.JoinCerts(dummy.TestCertificate4, dummy.TestCertificate1, dummy.TestCertificate2) + By("Ensuring targets update when an InLine source is updated") + Expect(cl.Get(ctx, client.ObjectKey{Name: testBundle.Name}, testBundle)).NotTo(HaveOccurred()) - env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, newBundle) + testBundle.Spec.Sources[2].InLine = ptr.To(dummy.TestCertificate2) - By("Ensuring targets update when default CAs are requested") - Expect(cl.Get(ctx, client.ObjectKey{Name: testBundle.Name}, testBundle)).NotTo(HaveOccurred()) + Expect(cl.Update(ctx, testBundle)).NotTo(HaveOccurred()) - testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{UseDefaultCAs: ptr.To(true)}) + newBundle := dummy.JoinCerts(dummy.TestCertificate4, dummy.TestCertificate1, dummy.TestCertificate2) - Expect(cl.Update(ctx, testBundle)).NotTo(HaveOccurred()) + env.EventuallyBundleHasSyncedAllNamespaces(ctx, cl, testBundle.Name, newBundle) - env.EventuallyBundleHasSyncedAllNamespacesStartsWith(ctx, cl, testBundle.Name, newBundle) + By("Ensuring targets update when default CAs are requested") + Expect(cl.Get(ctx, client.ObjectKey{Name: testBundle.Name}, testBundle)).NotTo(HaveOccurred()) - By("Ensuring targets update when a Namespace is created") - testNamespace := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{GenerateName: "trust-test-smoke-random-namespace-"}} + testBundle.Spec.Sources = append(testBundle.Spec.Sources, trustapi.BundleSource{UseDefaultCAs: ptr.To(true)}) - Expect(cl.Create(ctx, &testNamespace)).NotTo(HaveOccurred()) + Expect(cl.Update(ctx, testBundle)).NotTo(HaveOccurred()) - env.EventuallyBundleHasSyncedToNamespaceStartsWith(ctx, cl, testBundle.Name, testNamespace.Name, newBundle) + env.EventuallyBundleHasSyncedAllNamespacesStartsWith(ctx, cl, testBundle.Name, newBundle) - By("Setting Namespace Selector should remove ConfigMaps from Namespaces that do not have a match") - Expect(cl.Get(ctx, client.ObjectKey{Name: testBundle.Name}, testBundle)).NotTo(HaveOccurred()) + By("Ensuring targets update when a Namespace is created") + testNamespace := corev1.Namespace{ObjectMeta: metav1.ObjectMeta{GenerateName: "trust-test-smoke-random-namespace-"}} - testBundle.Spec.Target.NamespaceSelector = &trustapi.NamespaceSelector{ - MatchLabels: map[string]string{"foo": "bar"}, - } - Expect(cl.Update(ctx, testBundle)).NotTo(HaveOccurred()) + Expect(cl.Create(ctx, &testNamespace)).NotTo(HaveOccurred()) - Eventually(func() bool { + env.EventuallyBundleHasSyncedToNamespaceStartsWith(ctx, cl, testBundle.Name, testNamespace.Name, newBundle) + + By("Setting Namespace Selector should remove Secrets from Namespaces that do not have a match") + Expect(cl.Get(ctx, client.ObjectKey{Name: testBundle.Name}, testBundle)).NotTo(HaveOccurred()) + + testBundle.Spec.Target.NamespaceSelector = &trustapi.NamespaceSelector{ + MatchLabels: map[string]string{"foo": "bar"}, + } + Expect(cl.Update(ctx, testBundle)).NotTo(HaveOccurred()) + + Eventually(func() bool { + var err error + if testBundle.Spec.Target.Secret != nil { + var secret corev1.Secret + err = cl.Get(ctx, client.ObjectKey{Namespace: testNamespace.Name, Name: testBundle.Name}, &secret) + } else if testBundle.Spec.Target.ConfigMap != nil { var cm corev1.ConfigMap + err = cl.Get(ctx, client.ObjectKey{Namespace: testNamespace.Name, Name: testBundle.Name}, &cm) + } + return apierrors.IsNotFound(err) + }, eventuallyTimeout, eventualyPollInterval).Should(BeTrue(), "checking that namespace selector resulted in non-matching namespaces having Secret/ConfigMap removed") - err := cl.Get(ctx, client.ObjectKey{Namespace: testNamespace.Name, Name: testBundle.Name}, &cm) - return apierrors.IsNotFound(err) - }, eventuallyTimeout, eventualyPollInterval).Should(BeTrue(), "checking that namespace selector resulted in non-matching namespaces having ConfigMap removed") + By("Adding matching label on Namespace should sync ConfigMap/Secret to namespace") + Expect(cl.Get(ctx, client.ObjectKey{Name: testNamespace.Name}, &testNamespace)).NotTo(HaveOccurred()) - By("Adding matching label on Namespace should sync ConfigMap to namespace") - Expect(cl.Get(ctx, client.ObjectKey{Name: testNamespace.Name}, &testNamespace)).NotTo(HaveOccurred()) + testNamespace.Labels["foo"] = "bar" - testNamespace.Labels["foo"] = "bar" + Expect(cl.Update(ctx, &testNamespace)).NotTo(HaveOccurred()) - Expect(cl.Update(ctx, &testNamespace)).NotTo(HaveOccurred()) + env.EventuallyBundleHasSyncedToNamespaceStartsWith(ctx, cl, testBundle.Name, testNamespace.Name, newBundle) - env.EventuallyBundleHasSyncedToNamespaceStartsWith(ctx, cl, testBundle.Name, testNamespace.Name, newBundle) + By("Deleting test Namespace") + Expect(cl.Delete(ctx, &testNamespace)).NotTo(HaveOccurred()) - By("Deleting test Namespace") - Expect(cl.Delete(ctx, &testNamespace)).NotTo(HaveOccurred()) + By("Deleting the created Bundle") + Expect(cl.Get(ctx, client.ObjectKeyFromObject(testBundle), testBundle)).ToNot(HaveOccurred()) - By("Deleting the created Bundle") - Expect(cl.Get(ctx, client.ObjectKeyFromObject(testBundle), testBundle)).ToNot(HaveOccurred()) + Expect(cl.Delete(ctx, testBundle)).NotTo(HaveOccurred()) - Expect(cl.Delete(ctx, testBundle)).NotTo(HaveOccurred()) + Expect(cl.Delete(ctx, &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[0].ConfigMap.Name}})).NotTo(HaveOccurred()) - Expect(cl.Delete(ctx, &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[0].ConfigMap.Name}})).NotTo(HaveOccurred()) + Expect(cl.Delete(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[1].Secret.Name}})).NotTo(HaveOccurred()) - Expect(cl.Delete(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Namespace: cnf.TrustNamespace, Name: testBundle.Spec.Sources[1].Secret.Name}})).NotTo(HaveOccurred()) + By("Ensuring all targets have been deleted") + var namespaceList corev1.NamespaceList + Expect(cl.List(ctx, &namespaceList)).ToNot(HaveOccurred()) - By("Ensuring all targets have been deleted") - var namespaceList corev1.NamespaceList - Expect(cl.List(ctx, &namespaceList)).ToNot(HaveOccurred()) + for _, namespace := range namespaceList.Items { + Eventually(func() error { + if err := cl.Get(ctx, client.ObjectKey{Namespace: namespace.Name, Name: testBundle.Name}, new(corev1.ConfigMap)); err != nil { + return err + } - for _, namespace := range namespaceList.Items { - Eventually(func() error { - return cl.Get(ctx, client.ObjectKey{Namespace: namespace.Name, Name: testBundle.Name}, new(corev1.ConfigMap)) - }, eventuallyTimeout, eventualyPollInterval).ShouldNot(BeNil()) - } - }) -}) + return cl.Get(ctx, client.ObjectKey{Namespace: namespace.Name, Name: testBundle.Name}, new(corev1.Secret)) + }, eventuallyTimeout, eventualyPollInterval).ShouldNot(BeNil()) + } +}