Restrict daemon reading the pull secret from a merged secret

cmd/controller/main.go

@@ -3,6 +3,7 @@
 package main
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"os"
@@ -21,6 +22,7 @@ import (
 	bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1"
 	"github.com/bootc-dev/bootc-operator/internal/config"
 	"github.com/bootc-dev/bootc-operator/internal/controller"
+	"github.com/bootc-dev/bootc-operator/internal/pullsecret"
 	"github.com/bootc-dev/bootc-operator/internal/registry"
 	"github.com/bootc-dev/bootc-operator/internal/version"
 )
@@ -118,6 +120,10 @@ func main() {
 		os.Exit(1)
 	}
 	operatorNamespace := detectNamespace()
+	if operatorNamespace == "" {
+		setupLog.Error(nil, "Operator namespace unknown; set POD_NAMESPACE")
+		os.Exit(1)
+	}
 	pullSecretNamespaces, err := controller.ParsePullSecretNamespaces(
 		pullSecretNamespacesFlag,
 		operatorNamespace,
@@ -126,11 +132,6 @@ func main() {
 		setupLog.Error(err, "Invalid pull secret namespace configuration")
 		os.Exit(1)
 	}
-	if len(pullSecretNamespaces) == 0 {
-		setupLog.Error(nil, "No pull secret namespaces configured and operator namespace unknown; "+
-			"set spec.controller.pullSecretNamespaces, --pull-secret-namespaces, or POD_NAMESPACE")
-		os.Exit(1)
-	}
 	setupLog.Info("Pull secret namespaces", "namespaces", pullSecretNamespaces)
 
 	config.LogSource(
@@ -156,6 +157,14 @@ func main() {
 		os.Exit(1)
 	}
 
+	if err := pullsecret.IndexPoolPullSecretRef(
+		context.Background(),
+		mgr.GetFieldIndexer(),
+	); err != nil {
+		setupLog.Error(err, "Failed to index pools by pull secret")
+		os.Exit(1)
+	}
+
 	kubeClient, err := kubernetes.NewForConfig(mgr.GetConfig())
 	if err != nil {
 		setupLog.Error(err, "Failed to create Kubernetes clientset")
@@ -175,6 +184,15 @@ func main() {
 		os.Exit(1)
 	}
 
+	if err := (&pullsecret.PullSecretAggregator{
+		Client:               mgr.GetClient(),
+		Namespace:            operatorNamespace,
+		PullSecretNamespaces: pullSecretNamespaces,
+	}).SetupWithManager(mgr); err != nil {
+		setupLog.Error(err, "Failed to create controller", "controller", "pullsecretaggregator")
+		os.Exit(1)
+	}
+
 	// +kubebuilder:scaffold:builder
 
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {

cmd/daemon/main.go

@@ -22,6 +22,7 @@ import (
 	"github.com/bootc-dev/bootc-operator/internal/bootc"
 	"github.com/bootc-dev/bootc-operator/internal/config"
 	"github.com/bootc-dev/bootc-operator/internal/daemon"
+	"github.com/bootc-dev/bootc-operator/internal/pullsecret"
 	"github.com/bootc-dev/bootc-operator/internal/version"
 )
 
@@ -127,9 +128,9 @@ func main() {
 		Scheme:        mgr.GetScheme(),
 		NodeName:      nodeName,
 		HostRoot:      "/proc/1/root",
+		PullSecretDir: pullsecret.MountDir,
 		Executor:      executor,
 		StatusWatcher: watcher,
-		APIReader:     mgr.GetAPIReader(),
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "Failed to create controller", "controller", "bootcnode")
 		os.Exit(1)

config/daemon/daemon.yaml

@@ -42,6 +42,17 @@ spec:
           requests:
             cpu: 10m
             memory: 64Mi
+        volumeMounts:
+        # No subPath: subPath mounts do not receive Secret updates.
+        - name: pull-secrets
+          mountPath: /var/run/bootc-pull-secrets
+          readOnly: true
+      volumes:
+      - name: pull-secrets
+        secret:
+          secretName: bootc-pull-secrets
+          # The daemon must start before the controller creates the Secret.
+          optional: true
       tolerations:
       - operator: Exists
       serviceAccountName: daemon

config/rbac/daemon_role.yaml

@@ -12,12 +12,6 @@ rules:
   - bootcoperatorconfigs
   verbs:
   - list
-- apiGroups:
-  - ""
-  resources:
-  - secrets
-  verbs:
-  - get
 - apiGroups:
   - node.bootc.dev
   resources:

config/rbac/role.yaml

@@ -101,6 +101,16 @@ rules:
   resources:
   - secrets
   verbs:
+  - create
   - get
   - list
   - watch
+- apiGroups:
+  - ""
+  resourceNames:
+  - bootc-pull-secrets
+  resources:
+  - secrets
+  verbs:
+  - patch
+  - update

docs/src/helm.md

@@ -201,6 +201,15 @@ helm uninstall bootc-operator --namespace bootc-operator
 > preserves the CRDs and configuration. Uninstalling `bootc-operator-crds`
 > deletes the CRDs and all their instances.
 
+The controller creates the `bootc-pull-secrets` Secret in the release
+namespace, which Helm does not track, and `helm uninstall` does not
+delete the namespace created by `--create-namespace`. Delete the namespace
+to remove that Secret, along with any pull secrets created there:
+
+```bash
+kubectl delete namespace bootc-operator
+```
+
 ## Regenerating the chart
 
 Most Helm templates are generated from the kustomize manifests. After

hack/test-helm.sh

@@ -22,7 +22,7 @@ render() {
 # check MANIFESTS FLAG_ARGUMENTS NAMESPACES...
 #   Asserts the namespace flag arguments in order (empty means absent), each namespace has
 #   exactly one secret-reading Role bound to the controller ServiceAccount,
-#   and that the controller ClusterRole does not grant access to secrets.
+#   and that no ClusterRole (controller or daemon) grants access to secrets.
 check() {
     local manifests="$1" flag="$2"; shift 2
     local want got roles
@@ -45,9 +45,8 @@ check() {
     [ "$got" = "$roles" ] || fail "controller RoleBindings [$got], want [$roles]"
 
     got=$(echo "$manifests" | "$YQ" -N 'select(.kind == "ClusterRole") |
-        select(.metadata.name == "'"$SA"'") |
         select(.rules | any_c(.resources | any_c(. == "secrets"))) | .metadata.name')
-    [ -z "$got" ] || fail "controller ClusterRole grants secrets access"
+    [ -z "$got" ] || fail "ClusterRoles grant secrets access: [$got]"
 }
 
 "$HELM" dependency build "$CHART_DIR" >/dev/null

internal/controller/bootcnodepool_controller.go

@@ -39,6 +39,7 @@ import (
 
 	bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1"
 	"github.com/bootc-dev/bootc-operator/internal/karpenter"
+	"github.com/bootc-dev/bootc-operator/internal/pullsecret"
 )
 
 // drainStatus tracks an in-progress drain goroutine for a single node.
@@ -112,7 +113,7 @@ func (r *BootcNodePoolReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		For(&bootcv1alpha1.BootcNodePool{}).
 		Owns(&bootcv1alpha1.BootcNode{}).
 		Watches(&corev1.Node{}, handler.EnqueueRequestsFromMapFunc(r.mapNodeToPoolRequests), builder.WithPredicates(nodePredicates())).
-		Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.mapSecretToPoolRequests), builder.WithPredicates(secretTypePredicate())).
+		Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.mapSecretToPoolRequests)).
 		WatchesRawSource(source.Channel(r.drainCh, &handler.EnqueueRequestForObject{}))
 
 	// Karpenter is optional. Only register the NodeClaim watch when its CRD is
@@ -269,35 +270,27 @@ func (r *BootcNodePoolReconciler) mapNodeToPoolRequests(
 }
 
 // mapSecretToPoolRequests maps a Secret event to the BootcNodePool(s) that
-// should be reconciled. It enqueues pools whose pullSecretRef matches the
-// changed Secret's name and namespace.
+// should be reconciled: the pools whose pullSecretRef names the Secret,
+// whatever its type or content, so that a Secret gaining, changing or losing
+// its credentials is always noticed. It needs the pullsecret.PoolRefIndex
+// field index.
 func (r *BootcNodePoolReconciler) mapSecretToPoolRequests(
 	ctx context.Context,
 	obj client.Object,
 ) []reconcile.Request {
-	secret, ok := obj.(*corev1.Secret)
-	if !ok {
-		logf.FromContext(ctx).Error(nil, "Unexpected object type in secret mapper", "object", obj)
-		return nil
-	}
-
 	var pools bootcv1alpha1.BootcNodePoolList
-	if err := r.List(ctx, &pools); err != nil {
-		log := logf.FromContext(ctx)
-		log.Error(err, "Failed to list BootcNodePools in secret mapper")
+	if err := r.List(ctx, &pools, client.MatchingFields{
+		pullsecret.PoolRefIndex: pullsecret.PoolRefIndexValue(obj.GetNamespace(), obj.GetName()),
+	}); err != nil {
+		logf.FromContext(ctx).Error(err, "Failed to list BootcNodePools in secret mapper")
 		return nil
 	}
 
-	var requests []reconcile.Request
+	requests := make([]reconcile.Request, 0, len(pools.Items))
 	for i := range pools.Items {
-		pool := &pools.Items[i]
-		if pool.Spec.PullSecretRef != nil &&
-			pool.Spec.PullSecretRef.Name == secret.Name &&
-			pool.Spec.PullSecretRef.Namespace == secret.Namespace {
-			requests = append(requests, reconcile.Request{
-				NamespacedName: types.NamespacedName{Name: pool.Name},
-			})
-		}
+		requests = append(requests, reconcile.Request{
+			NamespacedName: types.NamespacedName{Name: pools.Items[i].Name},
+		})
 	}
 	return requests
 }
@@ -380,19 +373,6 @@ func hasKarpenterRebootTaint(node *corev1.Node) bool {
 	return false
 }
 
-// secretTypePredicate returns a predicate that accepts only Secrets of
-// type kubernetes.io/dockerconfigjson.
-func secretTypePredicate() predicate.Predicate {
-	return predicate.NewPredicateFuncs(func(obj client.Object) bool {
-		secret, ok := obj.(*corev1.Secret)
-		if !ok {
-			logf.Log.Error(nil, "Unexpected object type in secret predicate", "object", obj)
-			return false
-		}
-		return secret.Type == corev1.SecretTypeDockerConfigJson
-	})
-}
-
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 func (r *BootcNodePoolReconciler) Reconcile(
@@ -826,9 +806,11 @@ func (r *BootcNodePoolReconciler) listAllBootcNodes(
 	return all, nil
 }
 
-// fetchPullSecretData fetches the .dockerconfigjson data from the pull
-// secret referenced by the pool. Returns nil if no pull secret is set, and
-// an error if the secret's namespace is not in PullSecretNamespaces.
+// fetchPullSecretData fetches the credentials of the pull secret referenced
+// by the pool, normalized to .dockerconfigjson format (see
+// pullsecret.AuthData). Returns nil if no pull secret is set, and
+// an error if the secret's namespace is not in PullSecretNamespaces or the
+// reference cannot be stored in the aggregated pull secret.
 func (r *BootcNodePoolReconciler) fetchPullSecretData(
 	ctx context.Context,
 	pool *bootcv1alpha1.BootcNodePool,
@@ -845,6 +827,13 @@ func (r *BootcNodePoolReconciler) fetchPullSecretData(
 		)
 	}
 
+	if _, err := pullsecret.Key(
+		pool.Spec.PullSecretRef.Namespace,
+		pool.Spec.PullSecretRef.Name,
+	); err != nil {
+		return nil, err
+	}
+
 	key := types.NamespacedName{
 		Name:      pool.Spec.PullSecretRef.Name,
 		Namespace: pool.Spec.PullSecretRef.Namespace,
@@ -854,14 +843,9 @@ func (r *BootcNodePoolReconciler) fetchPullSecretData(
 		return nil, fmt.Errorf("fetching pull secret %s/%s: %w", key.Namespace, key.Name, err)
 	}
 
-	data, ok := secret.Data[corev1.DockerConfigJsonKey]
-	if !ok {
-		return nil, fmt.Errorf(
-			"pull secret %s/%s missing %s key",
-			key.Namespace,
-			key.Name,
-			corev1.DockerConfigJsonKey,
-		)
+	data, err := pullsecret.AuthData(&secret)
+	if err != nil {
+		return nil, fmt.Errorf("pull secret %s/%s: %w", key.Namespace, key.Name, err)
 	}
 
 	return data, nil