From ee46bd61955f263d06be071d19e3f82ae37f0c42 Mon Sep 17 00:00:00 2001 From: Abdullah Shakeel Date: Tue, 28 Apr 2026 14:26:32 +0500 Subject: [PATCH] :sparkles: Reconcile HCloudMachines when hcloud token secret changes --- controllers/hcloudmachine_controller.go | 66 +++++++++ controllers/hcloudmachine_controller_test.go | 142 +++++++++++++++++++ 2 files changed, 208 insertions(+) diff --git a/controllers/hcloudmachine_controller.go b/controllers/hcloudmachine_controller.go index e07c6ad5e..f49d53465 100644 --- a/controllers/hcloudmachine_controller.go +++ b/controllers/hcloudmachine_controller.go @@ -26,6 +26,7 @@ import ( "github.com/go-logr/logr" "github.com/google/go-cmp/cmp" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" @@ -321,6 +322,11 @@ func (r *HCloudMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl handler.EnqueueRequestsFromMapFunc(clusterToObjectFunc), builder.WithPredicates(predicates.ClusterPausedTransitionsOrInfrastructureProvisioned(mgr.GetScheme(), log)), ). + Watches( + &corev1.Secret{}, + handler.EnqueueRequestsFromMapFunc(r.HetznerSecretToHCloudMachines(ctx)), + builder.WithPredicates(IgnoreInsignificantSecretUpdates(log)), + ). Complete(r) if err != nil { return fmt.Errorf("error creating controller: %w", err) @@ -381,6 +387,66 @@ func (r *HCloudMachineReconciler) HetznerClusterToHCloudMachines(_ context.Conte } } +// HetznerSecretToHCloudMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation +// of HCloudMachines when the referenced HetznerSecret changes (e.g. after a token rotation). +func (r *HCloudMachineReconciler) HetznerSecretToHCloudMachines(_ context.Context) handler.MapFunc { + return func(ctx context.Context, o client.Object) []reconcile.Request { + log := log.FromContext(ctx) + + secret, ok := o.(*corev1.Secret) + if !ok { + log.Error(fmt.Errorf("expected a Secret but got a %T", o), "failed to get HCloudMachine for Secret") + return nil + } + + log = log.WithValues("objectMapper", "hetznerSecretToHCloudMachine", "namespace", secret.Namespace, "secret", secret.Name) + + hetznerClusterList := &infrav1.HetznerClusterList{} + if err := r.List(ctx, hetznerClusterList, client.InNamespace(secret.Namespace)); err != nil { + log.Error(err, "failed to list HetznerClusters, skipping mapping") + return nil + } + + result := []reconcile.Request{} + toRequests := r.HetznerClusterToHCloudMachines(ctx) + for i := range hetznerClusterList.Items { + hc := &hetznerClusterList.Items[i] + if hc.Spec.HetznerSecret.Name != secret.Name { + continue + } + result = append(result, toRequests(ctx, hc)...) + } + return result + } +} + +// IgnoreInsignificantSecretUpdates is a predicate that only fires when the Secret's Data +// actually changes, so HCloudMachines do not reconcile for ManagedFields or metadata-only +// Secret updates. +func IgnoreInsignificantSecretUpdates(logger logr.Logger) predicate.Funcs { + return predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldSecret, ok := e.ObjectOld.(*corev1.Secret) + if !ok { + return false + } + newSecret, ok := e.ObjectNew.(*corev1.Secret) + if !ok { + return false + } + if reflect.DeepEqual(oldSecret.Data, newSecret.Data) { + return false + } + logger.V(1).Info("Secret data changed, will enqueue HCloudMachines", + "namespace", newSecret.GetNamespace(), "name", newSecret.GetName()) + return true + }, + CreateFunc: func(_ event.CreateEvent) bool { return true }, + DeleteFunc: func(_ event.DeleteEvent) bool { return true }, + GenericFunc: func(_ event.GenericEvent) bool { return false }, + } +} + // IgnoreInsignificantHetznerClusterUpdates is a predicate used for ignoring insignificant HetznerCluster.Status updates. func IgnoreInsignificantHetznerClusterUpdates(logger logr.Logger) predicate.Funcs { return predicate.Funcs{ diff --git a/controllers/hcloudmachine_controller_test.go b/controllers/hcloudmachine_controller_test.go index ae42d9b8e..2e5ffa1f0 100644 --- a/controllers/hcloudmachine_controller_test.go +++ b/controllers/hcloudmachine_controller_test.go @@ -17,14 +17,19 @@ limitations under the License. package controllers import ( + "context" "fmt" "testing" "github.com/hetznercloud/hcloud-go/v2/hcloud" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/klog/v2" "k8s.io/utils/ptr" clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1" @@ -32,8 +37,10 @@ import ( v1beta1conditions "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/conditions" v1beta1patch "sigs.k8s.io/cluster-api/util/deprecated/v1beta1/patch" "sigs.k8s.io/controller-runtime/pkg/client" + fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1" hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client" @@ -224,6 +231,141 @@ func TestIgnoreInsignificantMachineStatusUpdates(t *testing.T) { } } +func TestIgnoreInsignificantSecretUpdates(t *testing.T) { + p := IgnoreInsignificantSecretUpdates(klog.Background()) + + makeSecret := func(data map[string][]byte, rv string) *corev1.Secret { + return &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hetzner", + Namespace: "default", + ResourceVersion: rv, + }, + Data: data, + } + } + + testCases := []struct { + name string + oldObj *corev1.Secret + newObj *corev1.Secret + expected bool + }{ + { + name: "Data changed", + oldObj: makeSecret(map[string][]byte{"hcloud-token": []byte("old")}, "1"), + newObj: makeSecret(map[string][]byte{"hcloud-token": []byte("new")}, "2"), + expected: true, + }, + { + name: "Only ResourceVersion changed", + oldObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same")}, "1"), + newObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same")}, "2"), + expected: false, + }, + { + name: "Unrelated data key added", + oldObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same")}, "1"), + newObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same"), "other": []byte("x")}, "2"), + expected: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got := p.Update(event.UpdateEvent{ObjectOld: tc.oldObj, ObjectNew: tc.newObj}) + require.Equal(t, tc.expected, got) + }) + } + + require.True(t, p.Create(event.CreateEvent{Object: makeSecret(nil, "1")})) + require.True(t, p.Delete(event.DeleteEvent{Object: makeSecret(nil, "1")})) + require.False(t, p.Generic(event.GenericEvent{Object: makeSecret(nil, "1")})) +} + +func TestHetznerSecretToHCloudMachines(t *testing.T) { + ctx := context.Background() + + testScheme := runtime.NewScheme() + utilruntime.Must(corev1.AddToScheme(testScheme)) + utilruntime.Must(infrav1.AddToScheme(testScheme)) + utilruntime.Must(clusterv1.AddToScheme(testScheme)) + + const ( + ns = "default" + secretName = "hetzner" + clusterName = "cluster-a" + ) + + newCluster := func(name string) *clusterv1.Cluster { + return &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns, UID: types.UID(name + "-uid")}, + } + } + newHetznerCluster := func(name, clusterOwner, secret string) *infrav1.HetznerCluster { + return &infrav1.HetznerCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: clusterv1.GroupVersion.String(), Kind: "Cluster", Name: clusterOwner, UID: types.UID(clusterOwner + "-uid")}, + }, + }, + Spec: infrav1.HetznerClusterSpec{ + HetznerSecret: infrav1.HetznerSecretRef{ + Name: secret, + Key: infrav1.HetznerSecretKeyRef{HCloudToken: "hcloud-token"}, + }, + }, + } + } + newMachine := func(name, clusterOwner, infraName string) *clusterv1.Machine { + return &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: ns, + Labels: map[string]string{clusterv1.ClusterNameLabel: clusterOwner}, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: clusterOwner, + InfrastructureRef: clusterv1.ContractVersionedObjectReference{ + APIGroup: infrav1.GroupVersion.Group, + Kind: "HCloudMachine", + Name: infraName, + }, + }, + } + } + + capiClusterA := newCluster(clusterName) + capiClusterB := newCluster("cluster-b") + hcA := newHetznerCluster("hc-a", clusterName, secretName) + hcB := newHetznerCluster("hc-b", "cluster-b", secretName) + hcUnrelated := newHetznerCluster("hc-u", clusterName, "other-secret") + hcmA := &infrav1.HCloudMachine{ObjectMeta: metav1.ObjectMeta{Name: "m-a", Namespace: ns}} + hcmB := &infrav1.HCloudMachine{ObjectMeta: metav1.ObjectMeta{Name: "m-b", Namespace: ns}} + cmA := newMachine("cm-a", clusterName, hcmA.Name) + cmB := newMachine("cm-b", "cluster-b", hcmB.Name) + matchingSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: ns}} + otherSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "no-ref", Namespace: ns}} + + c := fakeclient.NewClientBuilder(). + WithScheme(testScheme). + WithObjects(capiClusterA, capiClusterB, hcA, hcB, hcUnrelated, hcmA, hcmB, cmA, cmB). + Build() + + r := &HCloudMachineReconciler{Client: c} + mapper := r.HetznerSecretToHCloudMachines(ctx) + + got := mapper(ctx, matchingSecret) + require.ElementsMatch(t, []reconcile.Request{ + {NamespacedName: client.ObjectKey{Namespace: ns, Name: hcmA.Name}}, + {NamespacedName: client.ObjectKey{Namespace: ns, Name: hcmB.Name}}, + }, got) + + require.Empty(t, mapper(ctx, otherSecret)) +} + var _ = Describe("HCloudMachineReconciler", func() { var ( capiCluster *clusterv1.Cluster