feat: apply ACL changes live without rolling pods

api/v1alpha1/valkeynode_types.go

@@ -190,6 +190,11 @@ const (
 	// of Spec.Config has been successfully applied via CONFIG SET. The cluster
 	// controller blocks one-at-a-time progress until this condition is True.
 	ValkeyNodeConditionLiveConfigApplied = "LiveConfigApplied"
+	// ValkeyNodeConditionACLApplied indicates that the ACL the cluster controller
+	// wrote to the mounted Secret is live on the server. It is applied via ACL
+	// LOAD without a pod roll, so ACL never enters Spec.WorkloadRevision. True
+	// means the desired user set and their password hashes are live.
+	ValkeyNodeConditionACLApplied = "ACLApplied"
 	// ValkeyNodeConditionWorkloadRollPending indicates a rolling pod-template update
 	// is intentionally deferred: the desired template differs from live, and
 	// Spec.WorkloadRevision has not yet authorized that template. Status True means

docs/status-conditions.md

@@ -199,6 +199,27 @@ Common reasons when `LiveConfigApplied=True`:
 
 > **Note:** A `False` condition blocks one-at-a-time progress in the cluster controller (the same way `Ready=False` does during a rolling update). The node controller retries with exponential backoff and emits a `LiveConfigApplyFailed` warning event on each failure. The condition clears in either of two ways: once `CONFIG SET` succeeds it transitions to `True`, or if the offending key is removed from `spec.config` (leaving no allowlisted keys) the condition is removed and reverts to absent. Either way the cluster advances.
 
+#### `ACLApplied`
+Indicates whether the ACL the cluster controller wrote for `spec.users` is live on the running Valkey process. The ACL is applied with `ACL LOAD` on the mounted aclfile, so an ACL change takes effect without a pod roll and does not enter `spec.workloadRevision`.
+
+This condition is set once the node is ready and mounts an ACL Secret. An edit reads `False` until the mounted aclfile refreshes (the projected Secret volume is updated lazily by kubelet) and the server loads it.
+
+| Status | Meaning |
+|---|---|
+| `True` | The desired user set and their password hashes are live on the server. |
+| `False` | The reload ran but the running ACL does not yet match the desired users and passwords (for example the mounted aclfile has not refreshed yet), or the reload failed. |
+
+Common reasons when `ACLApplied=False`:
+- `PendingPropagation` – the reload read a stale mounted aclfile; the node retries until the projected volume catches up.
+- `ApplyFailed` – `ACL LOAD` or the follow-up read returned an error. The message field contains the exact error.
+
+Common reasons when `ACLApplied=True`:
+- `Applied` – the desired users and passwords are live.
+
+> **Note:** `ACLApplied` compares the user set and password hashes exactly, which is what a password rotation waits on. It is informational and does not block the cluster controller's one-at-a-time progress. On a failed apply the node controller emits a `LiveACLApplyFailed` warning event and retries with backoff.
+>
+> **Limitation:** Only the user set and password hashes drive the condition, so only those changes move it through `PendingPropagation` back to `True`: adding or removing a user or a password. A change to a user's `enabled` flag or permissions still takes effect on the server (the reload is unconditional), but the condition does not report a transient `PendingPropagation` for it, so it is not a signal to wait on for those fields. Comparing rules directly would mean reimplementing Valkey's normalized ACL rendering; making the condition track every field is left as a follow-up.
+
 #### `WorkloadRollPending`
 Indicates that a rolling pod-template update is intentionally deferred: the ValkeyNode controller has built a pod template that differs from the live StatefulSet or Deployment, but `spec.workloadRevision` has not yet authorized that template. This is expected staging while the cluster advances rolls one node at a time, not an error.
 
@@ -212,7 +233,7 @@ The ValkeyCluster controller owns `spec.workloadRevision` (a hash of the fully b
 Common reasons when `WorkloadRollPending=True`:
 - `AwaitingWorkloadRevision` – waiting for the ValkeyCluster controller to set `spec.workloadRevision` to the desired template hash.
 
-> **Note:** First-time backfill of an empty `spec.workloadRevision` (operator upgrade onto this feature) is bookkeeping only and does not fail over primaries. A non-empty revision change (for example after an ACL secret hash or image-driven template change) is staged like any other Spec roll.
+> **Note:** First-time backfill of an empty `spec.workloadRevision` (operator upgrade onto this feature) is bookkeeping only and does not fail over primaries. A non-empty revision change (for example an image-driven template change) is staged like any other Spec roll. ACL edits do not change the template and are applied live instead (see [`ACLApplied`](#aclapplied)).
 
 Example commands:
 
@@ -372,6 +393,7 @@ These events are emitted during ACL user management.
 | `InternalSecretsUpdated` | Normal | Internal ACL secret synchronized |
 | `InternalSecretsCreationFailed` | Warning | Failed to create or take ownership of internal ACL secret |
 | `InternalSecretsUpdateFailed` | Warning | Failed to update internal ACL secret |
+| `LiveACLApplyFailed` | Warning | `ACL LOAD` (or the follow-up verification) failed on a node; the `ACLApplied` condition is set to `False` |
 
 ### Viewing events
 

docs/valkeycluster.md

@@ -352,6 +352,10 @@ users:
 - `channels` — pub/sub channel patterns
 - `permissions` — raw ACL string appended after any generated rules
 
+ACL changes are applied to running nodes with `ACL LOAD` (no pod restart), the same way live-settable config is applied without rolling pods. Each node reports an [`ACLApplied`](status-conditions.md#aclapplied) condition once the change is live on the server.
+
+> **Upgrade note:** Live application relies on the operator's `_operator` user holding the `acl|load`, `acl|getuser`, and `acl|users` commands, which older operator versions did not grant. Upgrading onto this version rewrites the pod template (it drops a now-unused annotation), so every existing cluster rolls once and picks up the new grants on restart, after which ACL changes apply live. The exception is a cluster old enough to predate that annotation entirely: it gets no automatic roll, so it needs a one-time manual pod restart after the upgrade before live ACL applies.
+
 #### Constraints
 
 - Usernames cannot start with `_` (reserved for operator-managed system users)

internal/controller/failover.go

@@ -180,7 +180,7 @@ func needsProactiveFailoverForRoll(current, desired *valkeyiov1alpha1.ValkeyNode
 // matches) does not qualify.
 //
 // liveTemplateHashes maps ValkeyNode name -> hash of live pod template.
-func anyNodeRequiresFailoverAwareRoll(cluster *valkeyiov1alpha1.ValkeyCluster, nodeList *valkeyiov1alpha1.ValkeyNodeList, configHash string, aclSecret *corev1.Secret, liveTemplateHashes map[string]string) bool {
+func anyNodeRequiresFailoverAwareRoll(cluster *valkeyiov1alpha1.ValkeyCluster, nodeList *valkeyiov1alpha1.ValkeyNodeList, configHash string, liveTemplateHashes map[string]string) bool {
 	byName := make(map[string]*valkeyiov1alpha1.ValkeyNode, len(nodeList.Items))
 	for i := range nodeList.Items {
 		byName[nodeList.Items[i].Name] = &nodeList.Items[i]
@@ -190,7 +190,7 @@ func anyNodeRequiresFailoverAwareRoll(cluster *valkeyiov1alpha1.ValkeyCluster, n
 		for nodeIndex := range nodesPerShard {
 			desired := buildClusterValkeyNode(cluster, shardIndex, nodeIndex)
 			desired.Spec.ServerConfigHash = configHash
-			if err := setDesiredWorkloadRevision(desired, aclSecret); err != nil {
+			if err := setDesiredWorkloadRevision(desired); err != nil {
 				return true
 			}
 			if current, ok := byName[desired.Name]; ok {

internal/controller/failover_test.go

@@ -184,7 +184,7 @@ func TestAnyNodeRequiresFailoverAwareRoll(t *testing.T) {
 	steadyStateNode := func() valkeyiov1alpha1.ValkeyNode {
 		n := buildClusterValkeyNode(cluster, 0, 0)
 		n.Spec.ServerConfigHash = configHash
-		require.NoError(t, setDesiredWorkloadRevision(n, nil))
+		require.NoError(t, setDesiredWorkloadRevision(n))
 		n.Status.PodIP = "10.0.0.1"
 		return *n
 	}
@@ -193,7 +193,7 @@ func TestAnyNodeRequiresFailoverAwareRoll(t *testing.T) {
 		n := steadyStateNode()
 		nodes := &valkeyiov1alpha1.ValkeyNodeList{Items: []valkeyiov1alpha1.ValkeyNode{n}}
 		live := map[string]string{n.Name: n.Spec.WorkloadRevision}
-		assert.False(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, nil, live))
+		assert.False(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, live))
 	})
 
 	t.Run("empty WorkloadRevision backfill when live matches does not need failover-aware roll", func(t *testing.T) {
@@ -202,11 +202,11 @@ func TestAnyNodeRequiresFailoverAwareRoll(t *testing.T) {
 		n.Spec.WorkloadRevision = ""
 		nodes := &valkeyiov1alpha1.ValkeyNodeList{Items: []valkeyiov1alpha1.ValkeyNode{n}}
 		live := map[string]string{n.Name: authorized}
-		assert.False(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, nil, live))
+		assert.False(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, live))
 		assert.True(t, nodeRequiresRoll(&n, func() *valkeyiov1alpha1.ValkeyNode {
 			d := buildClusterValkeyNode(cluster, 0, 0)
 			d.Spec.ServerConfigHash = configHash
-			require.NoError(t, setDesiredWorkloadRevision(d, nil))
+			require.NoError(t, setDesiredWorkloadRevision(d))
 			return d
 		}()))
 	})
@@ -217,21 +217,21 @@ func TestAnyNodeRequiresFailoverAwareRoll(t *testing.T) {
 		nodes := &valkeyiov1alpha1.ValkeyNodeList{Items: []valkeyiov1alpha1.ValkeyNode{n}}
 		// Live still on an older template (e.g. ACL hash change before backfill).
 		live := map[string]string{n.Name: "old-live-template-hash"}
-		assert.True(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, nil, live))
+		assert.True(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, live))
 	})
 
 	t.Run("stale config hash needs failover-aware roll", func(t *testing.T) {
 		n := steadyStateNode()
 		n.Spec.ServerConfigHash = "stale"
 		nodes := &valkeyiov1alpha1.ValkeyNodeList{Items: []valkeyiov1alpha1.ValkeyNode{n}}
-		assert.True(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, nil, nil))
+		assert.True(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, nil))
 	})
 
 	t.Run("stale WorkloadRevision needs failover-aware roll", func(t *testing.T) {
 		n := steadyStateNode()
 		n.Spec.WorkloadRevision = "not-the-real-hash"
 		nodes := &valkeyiov1alpha1.ValkeyNodeList{Items: []valkeyiov1alpha1.ValkeyNode{n}}
-		assert.True(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, nil, nil))
+		assert.True(t, anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, nil))
 	})
 }
 

internal/controller/users.go

@@ -64,6 +64,9 @@ var (
 			"+cluster|set-config-epoch",  // set epoch on new nodes
 			"+config|set",                // apply live config changes
 			"+config|get",                // verify applied config / audit current state
+			"+acl|load",                  // reload the aclfile live to apply ACL changes without a pod roll
+			"+acl|getuser",               // read back a user's password hashes to verify the reload landed
+			"+acl|users",                 // read the user set to verify membership after a reload
 			"+info",                      // node info and replication status
 			"+role",                      // current replication role
 		}, " "),

internal/controller/valkeycluster_controller.go

@@ -548,18 +548,6 @@ func (r *ValkeyClusterReconciler) reconcileValkeyNodes(ctx context.Context, clus
 	nodesPerShard := 1 + int(cluster.Spec.Replicas)
 	totalCreated := 0
 
-	// One ACL secret snapshot for preflight and per-node WorkloadRevision so the
-	// scrape decision and authorized hash cannot disagree mid-reconcile.
-	aclSecret, err := r.getClusterACLSecret(ctx, cluster)
-	if err != nil {
-		if !apierrors.IsNotFound(err) {
-			return false, fmt.Errorf("get ACL secret for workload revision: %w", err)
-		}
-		// Bootstrap: secret may not exist yet; hash without ACL annotations.
-		log.V(1).Info("ACL secret not ready for roll preflight, hashing without it", "err", err)
-		aclSecret = nil
-	}
-
 	// Scrape cluster state once for proactive failover decisions, but only
 	// when at least one node needs a failover-aware roll. During initial
 	// bootstrap no nodes exist, so state stays nil. The snapshot is safe to
@@ -571,7 +559,7 @@ func (r *ValkeyClusterReconciler) reconcileValkeyNodes(ctx context.Context, clus
 	if err != nil {
 		return false, err
 	}
-	if anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, aclSecret, liveHashes) {
+	if anyNodeRequiresFailoverAwareRoll(cluster, nodes, configHash, liveHashes) {
 		operatorPassword, err := fetchSystemUserPassword(ctx, operatorUser, r.Client, cluster.Name, cluster.Namespace)
 		if err != nil {
 			return false, fmt.Errorf("failed to fetch operator password for proactive failover: %w", err)
@@ -593,7 +581,7 @@ func (r *ValkeyClusterReconciler) reconcileValkeyNodes(ctx context.Context, clus
 		// actual primary (which may differ from node-index=0 after a failover)
 		// and place it last.
 		for _, nodeIndex := range replicaFirstNodeOrder(shardIndex, nodesPerShard, nodes, clusterState) {
-			result, err := r.reconcileValkeyNode(ctx, cluster, shardIndex, nodeIndex, clusterState, configHash, aclSecret, liveHashes)
+			result, err := r.reconcileValkeyNode(ctx, cluster, shardIndex, nodeIndex, clusterState, configHash, liveHashes)
 			if err != nil {
 				return false, err
 			}
@@ -634,14 +622,14 @@ const (
 
 // reconcileValkeyNode reconciles a single ValkeyNode for (shardIndex, nodeIndex).
 // Returns a nodeResult signaling the outcome or required next action.
-// aclSecret and liveTemplateHashes are the reconcileValkeyNodes snapshot so
-// WorkloadRevision and failover decisions stay consistent for the whole pass.
-func (r *ValkeyClusterReconciler) reconcileValkeyNode(ctx context.Context, cluster *valkeyiov1alpha1.ValkeyCluster, shardIndex, nodeIndex int, clusterState *valkey.ClusterState, configHash string, aclSecret *corev1.Secret, liveTemplateHashes map[string]string) (nodeResult, error) {
+// liveTemplateHashes is the reconcileValkeyNodes snapshot so WorkloadRevision
+// and failover decisions stay consistent for the whole pass.
+func (r *ValkeyClusterReconciler) reconcileValkeyNode(ctx context.Context, cluster *valkeyiov1alpha1.ValkeyCluster, shardIndex, nodeIndex int, clusterState *valkey.ClusterState, configHash string, liveTemplateHashes map[string]string) (nodeResult, error) {
 	log := logf.FromContext(ctx)
 
 	desired := buildClusterValkeyNode(cluster, shardIndex, nodeIndex)
 	desired.Spec.ServerConfigHash = configHash
-	if err := setDesiredWorkloadRevision(desired, aclSecret); err != nil {
+	if err := setDesiredWorkloadRevision(desired); err != nil {
 		return nodeUnchanged, err
 	}
 
@@ -808,16 +796,6 @@ func (r *ValkeyClusterReconciler) livePodTemplateHash(ctx context.Context, node
 	}
 }
 
-// getClusterACLSecret loads the cluster internal ACL secret used for template annotations.
-func (r *ValkeyClusterReconciler) getClusterACLSecret(ctx context.Context, cluster *valkeyiov1alpha1.ValkeyCluster) (*corev1.Secret, error) {
-	aclSecret := &corev1.Secret{}
-	name := getInternalSecretName(cluster.Name)
-	if err := r.Get(ctx, client.ObjectKey{Name: name, Namespace: cluster.Namespace}, aclSecret); err != nil {
-		return nil, err
-	}
-	return aclSecret, nil
-}
-
 const (
 	// gracePeriodBufferSeconds is added on top of cluster-manual-failover-timeout
 	// so the SIGTERM-triggered failover has headroom to finish before SIGKILL.

internal/controller/valkeycluster_controller_test.go

@@ -1183,7 +1183,7 @@ var _ = Describe("reconcileValkeyNode", func() {
 	}
 
 	It("creates the ValkeyNode and emits ValkeyNodeCreated event", func() {
-		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(result).To(Equal(nodeCreated))
 
@@ -1193,12 +1193,12 @@ var _ = Describe("reconcileValkeyNode", func() {
 	})
 
 	It("updates the ValkeyNode spec, emits ValkeyNodeUpdated event, and signals requeue", func() {
-		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		collectEvents(fakeRecorder) // drain creation event
 
 		cluster.Spec.Image = "valkey/valkey:9.1.0"
-		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(result).To(Equal(nodeRequeued))
 
@@ -1207,31 +1207,31 @@ var _ = Describe("reconcileValkeyNode", func() {
 	})
 
 	It("signals requeue when node is unchanged but not yet ready", func() {
-		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		collectEvents(fakeRecorder) // drain creation event
 
 		// Status.Ready defaults to false after creation
-		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(result).To(Equal(nodeRequeued))
 	})
 
 	It("does not requeue when node is unchanged and ready", func() {
-		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		collectEvents(fakeRecorder) // drain creation event
 
 		setNodeReady(true)
 
-		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(result).To(Equal(nodeUnchanged))
 	})
 
 	It("signals requeue when node is unchanged but ObservedGeneration is stale", func() {
 		// Create the node
-		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		_, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 
 		// Mark ready but leave ObservedGeneration at 0
@@ -1246,7 +1246,7 @@ var _ = Describe("reconcileValkeyNode", func() {
 		// Because ObservedGeneration > 0 guard: newly created node with
 		// ObservedGeneration=0 falls through to the Ready check, which
 		// passes (Ready=true). No requeue.
-		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		result, err := r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(result).To(Equal(nodeUnchanged))
 
@@ -1261,13 +1261,13 @@ var _ = Describe("reconcileValkeyNode", func() {
 
 		// Change cluster spec to trigger an update on next reconcile
 		cluster.Spec.Image = "valkey/valkey:9.1.0"
-		result, err = r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		result, err = r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(result).To(Equal(nodeRequeued), "should requeue after updating node")
 
 		// Next reconcile: spec matches (OperationResultNone), but
 		// Generation (2) != ObservedGeneration (1) — must requeue.
-		result, err = r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil, nil)
+		result, err = r.reconcileValkeyNode(testCtx, cluster, shardIndex, nodeIndex, nil, "", nil)
 		Expect(err).NotTo(HaveOccurred())
 		Expect(result).To(Equal(nodeRequeued), "should requeue while ObservedGeneration is stale")
 	})