Supporting a secret target

This was requested in cert-manager#10

Motivation:
Many helm charts/operators support a certificate authority mount only in the form of a Secret, and do not support ConfigMap. This is due to the fact that certificate authority is often shipped with TLS-type secrets (along with their certificate and key).
cert-manager also require a certificate authority in the form of a Secret for its Vault issuer.

trust-manager should support the target being a Secret, along with a ConfigMap, to guarantee maximum usability.

For security concerns, we will allow trust-manager to only view/update a given list of managed secrets in the helm chart.

Signed-off-by: Guillaume Ludinard <guillaume.ludinard@japannext.co.jp>

deploy/charts/trust-manager/README.md

@@ -38,6 +38,7 @@ Kubernetes: `>= 1.22.0-0`
 | app.webhook.port | int | `6443` | Port that the webhook listens on. |
 | app.webhook.service | object | `{"type":"ClusterIP"}` | Type of Kubernetes Service used by the Webhook |
 | app.webhook.timeoutSeconds | int | `5` | Timeout of webhook HTTP request. |
+| authorizedSecrets[0] | string | `"ca-bundle"` |  |
 | crds.enabled | bool | `true` | Whether or not to install the crds. |
 | defaultPackageImage.pullPolicy | string | `"IfNotPresent"` | imagePullPolicy for the default package image |
 | defaultPackageImage.repository | string | `"quay.io/jetstack/cert-manager-package-debian"` | Repository for the default package image. This image enables the 'useDefaultCAs' source on Bundles. |

deploy/charts/trust-manager/templates/clusterrole.yaml

@@ -31,6 +31,17 @@ rules:
   - "configmaps"
   verbs: ["get", "list", "create", "update", "watch", "delete"]
 
+- apiGroups:
+  - ""
+  resources:
+  - "secrets"
+  verbs: ["get", "list", "update", "watch", "delete"]
+  resourceNames: {{ .Values.authorizedSecrets | toYaml | nindent 2 }}
+- apiGroups:
+  - ""
+  resources:
+  - "secrets"
+  verbs: ["create"]
 - apiGroups:
   - ""
   resources:

deploy/charts/trust-manager/templates/trust.cert-manager.io_bundles.yaml

@@ -115,6 +115,15 @@ spec:
                           type: object
                           additionalProperties:
                             type: string
+                    secret:
+                      description: Secret is the target Secret in Namespaces that all Bundle source data will be synced to.
+                      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
@@ -174,6 +183,15 @@ spec:
                           type: object
                           additionalProperties:
                             type: string
+                    secret:
+                      description: Secret is the target Secret in Namespaces that all Bundle source data will be synced to.
+                      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:

deploy/charts/trust-manager/values.yaml

@@ -67,6 +67,8 @@ app:
     # -- If false, disables the default seccomp profile, which might be required to run on certain platforms
     seccompProfileEnabled: true
 
+authorizedSecrets: ['ca-bundle']
+
 resources: {}
   # -- Kubernetes pod resource limits for trust.
   # limits:

pkg/apis/trust/v1alpha1/types_bundle.go

@@ -96,6 +96,10 @@ type BundleTarget struct {
 	// data will be synced to.
 	ConfigMap *KeySelector `json:"configMap,omitempty"`
 
+    // Secret is the target Secret in Namespaces that all Bundle source
+    // data will be synced to.
+    Secret *KeySelector `json:"secret,omitempty"`
+
 	// NamespaceSelector will, if set, only sync the target resource in
 	// Namespaces which match the selector.
 	// +optional

pkg/bundle/bundle.go

@@ -202,7 +202,26 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 			return ctrl.Result{Requeue: true}, b.targetDirectClient.Status().Update(ctx, &bundle)
 		}
 
-		if synced {
+        secretSynced := false
+        if bundle.Spec.Target.Secret != nil {
+            var syncErr error
+            secretSynced, syncErr = b.syncSecretTarget(ctx, log, &bundle, namespaceSelector, &namespace, []byte(resolvedBundle.data))
+            if syncErr != nil {
+                log.Error(syncErr, "failed sync bundle to target namespace")
+                b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SyncTargetFailed", "Failed to sync secret target in Namespace %q: %s", namespace.Name, syncErr)
+
+                b.setBundleCondition(&bundle, trustapi.BundleCondition{
+                    Type:    trustapi.BundleConditionSynced,
+                    Status:  corev1.ConditionFalse,
+                    Reason:  "SyncTargetFailed",
+                    Message: fmt.Sprintf("Failed to sync bundle target secret to namespace %q: %s", namespace.Name, syncErr),
+                })
+
+                return ctrl.Result{Requeue: true}, b.targetDirectClient.Status().Update(ctx, &bundle)
+            }
+        }
+
+		if synced || secretSynced {
 			// We need to update if any target is synced.
 			needsUpdate = true
 		}

pkg/bundle/sync.go

@@ -18,6 +18,7 @@ package bundle
 
 import (
 	"context"
+    "bytes"
 	"errors"
 	"fmt"
 	"strings"
@@ -210,7 +211,7 @@ func (b *bundle) syncTarget(ctx context.Context, log logr.Logger,
 	}
 
 	// Match, return do nothing
-	if cmdata, ok := configMap.Data[target.ConfigMap.Key]; !ok || cmdata != data {
+	if cmdata, ok2 := configMap.Data[target.ConfigMap.Key]; !ok2 || cmdata != data {
 		if configMap.Data == nil {
 			configMap.Data = make(map[string]string)
 		}
@@ -223,11 +224,100 @@ func (b *bundle) syncTarget(ctx context.Context, log logr.Logger,
 		return false, nil
 	}
 
-	if err := b.targetDirectClient.Update(ctx, &configMap); err != nil {
+	if err = b.targetDirectClient.Update(ctx, &configMap); err != nil {
 		return true, fmt.Errorf("failed to update configmap %s/%s with bundle: %w", namespace, bundle.Name, err)
 	}
 
 	log.V(2).Info("synced bundle to namespace")
 
 	return true, nil
 }
+
+// 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,
+	namespaceSelector labels.Selector,
+	namespace *corev1.Namespace,
+	data []byte,
+) (bool, error) {
+	target := bundle.Spec.Target
+
+	if target.Secret == nil {
+        // Fail silently, since target.Secret is optional
+		return false, nil
+	}
+
+	matchNamespace := namespaceSelector.Matches(labels.Set(namespace.Labels))
+
+	var secret corev1.Secret
+	err := b.targetDirectClient.Get(ctx, client.ObjectKey{Namespace: namespace.Name, Name: bundle.Name}, &secret)
+
+	// If the Secret doesn't exist yet, create it.
+	if apierrors.IsNotFound(err) {
+		// If the namespace doesn't match selector we do nothing since we don't
+		// want to create it, and it also doesn't exist.
+		if !matchNamespace {
+			log.V(4).Info("ignoring namespace as it doesn't match selector", "labels", namespace.Labels)
+			return false, nil
+		}
+
+        secret = corev1.Secret{
+            ObjectMeta: metav1.ObjectMeta{
+                Name:           bundle.Name,
+                Namespace:      namespace.Name,
+                OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(bundle, trustapi.SchemeGroupVersion.WithKind("Bundle"))},
+            },
+            Type: "Opaque",
+            Data: map[string][]byte{
+                target.Secret.Key: data,
+            },
+        }
+
+		return true, b.targetDirectClient.Create(ctx, &secret)
+	}
+
+	// Here, the secret exists, but the selector doesn't match the namespace.
+	if !matchNamespace {
+		// The ConfigMap is owned by this controller- delete it.
+		if metav1.IsControlledBy(&secret, bundle) {
+			log.V(2).Info("deleting bundle from Namespace since namespaceSelector does not match")
+			return true, b.targetDirectClient.Delete(ctx, &secret)
+		}
+		// The ConfigMap isn't owned by us, so we shouldn't delete it. Return that
+		// we did nothing.
+		b.recorder.Eventf(&secret, corev1.EventTypeWarning, "NotOwned", "Secret is not owned by trust.cert-manager.io so ignoring")
+		return false, nil
+	}
+
+	var needsUpdate bool
+	// If ConfigMap is missing OwnerReference, add it back.
+	if !metav1.IsControlledBy(&secret, bundle) {
+		secret.OwnerReferences = append(secret.OwnerReferences, *metav1.NewControllerRef(bundle, trustapi.SchemeGroupVersion.WithKind("Bundle")))
+		needsUpdate = true
+	}
+
+	// Match, return do nothing
+	if cmdata, ok := secret.Data[target.Secret.Key]; !ok || bytes.Compare(cmdata, data) != 0 {
+		if secret.Data == nil {
+			secret.Data = make(map[string][]byte)
+		}
+		secret.Data[target.Secret.Key] = data
+		needsUpdate = true
+	}
+
+	// Exit early if no update is needed
+	if !needsUpdate {
+		return false, nil
+	}
+
+	if err := b.targetDirectClient.Update(ctx, &secret); err != nil {
+		return true, fmt.Errorf("failed to update secret %s/%s with bundle: %w", namespace, bundle.Name, err)
+	}
+
+	log.V(2).Info("synced bundle to namespace")
+
+	return true, nil
+}