Skip to content

Commit d3c87a2

Browse files
feat(controller): roll pods on auth.existingSecret changes (closes #10) (#11)
A user-managed spec.auth.existingSecret had no watch, so an external password rotation never enqueued the referencing ValkeyCluster: the generated ConfigMap (and its config hash) stayed stale and pods kept the old credential until some unrelated reconcile happened. Extend the existing Secret watch mapper (mapTLSSecretToCluster -> mapSecretToCluster) to also enqueue any cluster whose enabled auth.existingSecret matches the changed Secret, mirroring the TLS path. The reconcile re-renders valkey.conf with the new password, the config hash changes, and the StatefulSet rolls. Operator-owned Secrets keep flowing through Owns(); the existingSecret name match skips them.
1 parent e07f0a0 commit d3c87a2

2 files changed

Lines changed: 73 additions & 12 deletions

File tree

internal/controller/reconciler_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,3 +589,55 @@ func TestReconcileClusterReshardOnAnnotation(t *testing.T) {
589589
t.Fatalf("reshard job not created on annotation: %v", err)
590590
}
591591
}
592+
593+
func TestMapSecretToCluster(t *testing.T) {
594+
scheme := newTestScheme(t)
595+
const ns = "ns"
596+
mkVC := func(name string, auth *cachev1beta1.AuthSpec, tls *cachev1beta1.TLSSpec) *cachev1beta1.ValkeyCluster {
597+
return &cachev1beta1.ValkeyCluster{
598+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns},
599+
Spec: cachev1beta1.ValkeyClusterSpec{Auth: auth, TLS: tls},
600+
}
601+
}
602+
authExt := func() *cachev1beta1.AuthSpec {
603+
return &cachev1beta1.AuthSpec{Enabled: true, ExistingSecret: "shared-auth"}
604+
}
605+
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(
606+
mkVC("c1", authExt(), nil), // references shared-auth
607+
mkVC("c2", authExt(), nil), // also references shared-auth
608+
mkVC("c3", &cachev1beta1.AuthSpec{Enabled: true}, nil), // generated secret, no existingSecret
609+
mkVC("c4", nil, &cachev1beta1.TLSSpec{Enabled: true, ExistingSecret: "tls-ext"}), // TLS existingSecret
610+
).Build()
611+
r := &ValkeyClusterReconciler{Client: c, Scheme: scheme}
612+
613+
names := func(reqs []ctrl.Request) []string {
614+
out := make([]string, 0, len(reqs))
615+
for _, req := range reqs {
616+
out = append(out, req.Name)
617+
}
618+
slices.Sort(out)
619+
return out
620+
}
621+
mapped := func(secretName string) []string {
622+
s := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: ns}}
623+
return names(r.mapSecretToCluster(context.Background(), s))
624+
}
625+
626+
// A user-managed auth.existingSecret update enqueues every cluster referencing it.
627+
if got := mapped("shared-auth"); !slices.Equal(got, []string{"c1", "c2"}) {
628+
t.Errorf("auth existingSecret shared-auth → %v, want [c1 c2]", got)
629+
}
630+
// The TLS cert Secret still enqueues its cluster (no regression).
631+
if got := mapped("tls-ext"); !slices.Equal(got, []string{"c4"}) {
632+
t.Errorf("tls existingSecret tls-ext → %v, want [c4]", got)
633+
}
634+
// An unreferenced Secret enqueues nothing.
635+
if got := mapped("unrelated"); len(got) != 0 {
636+
t.Errorf("unreferenced secret → %v, want none", got)
637+
}
638+
// The operator-managed generated auth Secret for c3 is "c3-auth"; it must NOT be
639+
// enqueued through this path — Owns() already covers operator-owned Secrets.
640+
if got := mapped("c3-auth"); len(got) != 0 {
641+
t.Errorf("generated auth secret c3-auth → %v, want none (handled by Owns)", got)
642+
}
643+
}

internal/controller/valkeycluster_controller.go

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -747,29 +747,38 @@ func (r *ValkeyClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
747747
// slow: the pod can be recreated faster than that, and the survivors then
748748
// resync from the empty primary (data loss — see cha-02/cha-03 chaos).
749749
Watches(&corev1.Pod{}, handler.EnqueueRequestsFromMapFunc(mapPodToCluster)).
750-
// Watch the TLS cert Secret. cert-manager renews it in place (owned by the
751-
// Certificate, not the ValkeyCluster, so Owns() misses it); a renewal must
752-
// enqueue a reconcile so the operator reloads the cert onto live pods
753-
// without a restart.
754-
Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.mapTLSSecretToCluster)).
750+
// Watch user-managed Secrets the operator does not own: the TLS cert Secret
751+
// (cert-manager renews it in place, owned by the Certificate, so Owns()
752+
// misses it — a renewal reloads the cert onto live pods without a restart)
753+
// and an auth.existingSecret (an external password rotation must re-render
754+
// the config and roll the pods). Operator-managed Secrets are owned by the
755+
// ValkeyCluster and already covered by Owns() above.
756+
Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.mapSecretToCluster)).
755757
Named("valkeycluster").
756758
Complete(r)
757759
}
758760

759-
// mapTLSSecretToCluster routes a Secret event to any ValkeyCluster in the same
760-
// namespace that uses it as its TLS cert Secret. The cert Secret carries no
761-
// operator labels (cert-manager owns it), so we match by the resolved TLS secret
762-
// name; the per-namespace list is cheap under the one-cluster-per-namespace DBaaS
763-
// layout.
764-
func (r *ValkeyClusterReconciler) mapTLSSecretToCluster(ctx context.Context, obj client.Object) []reconcile.Request {
761+
// mapSecretToCluster routes a Secret event to any ValkeyCluster in the same
762+
// namespace that references it as a user-managed Secret the operator does not
763+
// own: either the TLS cert Secret (cert-manager owns it) or an
764+
// auth.existingSecret (the user owns it). These carry no operator owner ref, so
765+
// Owns() misses them; we match by the resolved name instead. The operator's own
766+
// generated Secrets ARE owned and covered by Owns(); they use a different name
767+
// than any existingSecret, so the name match here naturally skips them. The
768+
// per-namespace list is cheap under the one-cluster-per-namespace layout.
769+
func (r *ValkeyClusterReconciler) mapSecretToCluster(ctx context.Context, obj client.Object) []reconcile.Request {
765770
var list cachev1beta1.ValkeyClusterList
766771
if err := r.List(ctx, &list, client.InNamespace(obj.GetNamespace())); err != nil {
767772
return nil
768773
}
774+
name := obj.GetName()
769775
var reqs []reconcile.Request
770776
for i := range list.Items {
771777
vc := &list.Items[i]
772-
if tlsEnabled(vc) && tlsSecretName(vc) == obj.GetName() {
778+
tlsMatch := tlsEnabled(vc) && tlsSecretName(vc) == name
779+
authMatch := vc.Spec.Auth != nil && vc.Spec.Auth.Enabled &&
780+
vc.Spec.Auth.ExistingSecret != "" && vc.Spec.Auth.ExistingSecret == name
781+
if tlsMatch || authMatch {
773782
reqs = append(reqs, reconcile.Request{
774783
NamespacedName: types.NamespacedName{Namespace: vc.Namespace, Name: vc.Name},
775784
})

0 commit comments

Comments
 (0)