Skip to content

Commit d42e3b2

Browse files
committed
✨ Reconcile HCloudMachines when hcloud token secret changes
1 parent 7a6a529 commit d42e3b2

2 files changed

Lines changed: 208 additions & 0 deletions

File tree

controllers/hcloudmachine_controller.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626

2727
"github.com/go-logr/logr"
2828
"github.com/google/go-cmp/cmp"
29+
corev1 "k8s.io/api/core/v1"
2930
apierrors "k8s.io/apimachinery/pkg/api/errors"
3031
"k8s.io/apimachinery/pkg/util/wait"
3132
"k8s.io/klog/v2"
@@ -320,6 +321,11 @@ func (r *HCloudMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl
320321
handler.EnqueueRequestsFromMapFunc(clusterToObjectFunc),
321322
builder.WithPredicates(predicates.ClusterPausedTransitionsOrInfrastructureReady(mgr.GetScheme(), log)),
322323
).
324+
Watches(
325+
&corev1.Secret{},
326+
handler.EnqueueRequestsFromMapFunc(r.HetznerSecretToHCloudMachines(ctx)),
327+
builder.WithPredicates(IgnoreInsignificantSecretUpdates(log)),
328+
).
323329
Complete(r)
324330
if err != nil {
325331
return fmt.Errorf("error creating controller: %w", err)
@@ -380,6 +386,66 @@ func (r *HCloudMachineReconciler) HetznerClusterToHCloudMachines(_ context.Conte
380386
}
381387
}
382388

389+
// HetznerSecretToHCloudMachines is a handler.ToRequestsFunc to be used to enqueue requests for reconciliation
390+
// of HCloudMachines when the referenced HetznerSecret changes (e.g. after a token rotation).
391+
func (r *HCloudMachineReconciler) HetznerSecretToHCloudMachines(_ context.Context) handler.MapFunc {
392+
return func(ctx context.Context, o client.Object) []reconcile.Request {
393+
log := log.FromContext(ctx)
394+
395+
secret, ok := o.(*corev1.Secret)
396+
if !ok {
397+
log.Error(fmt.Errorf("expected a Secret but got a %T", o), "failed to get HCloudMachine for Secret")
398+
return nil
399+
}
400+
401+
log = log.WithValues("objectMapper", "hetznerSecretToHCloudMachine", "namespace", secret.Namespace, "secret", secret.Name)
402+
403+
hetznerClusterList := &infrav1.HetznerClusterList{}
404+
if err := r.List(ctx, hetznerClusterList, client.InNamespace(secret.Namespace)); err != nil {
405+
log.Error(err, "failed to list HetznerClusters, skipping mapping")
406+
return nil
407+
}
408+
409+
result := []reconcile.Request{}
410+
toRequests := r.HetznerClusterToHCloudMachines(ctx)
411+
for i := range hetznerClusterList.Items {
412+
hc := &hetznerClusterList.Items[i]
413+
if hc.Spec.HetznerSecret.Name != secret.Name {
414+
continue
415+
}
416+
result = append(result, toRequests(ctx, hc)...)
417+
}
418+
return result
419+
}
420+
}
421+
422+
// IgnoreInsignificantSecretUpdates is a predicate that only fires when the Secret's Data
423+
// actually changes, so HCloudMachines do not reconcile for ManagedFields or metadata-only
424+
// Secret updates.
425+
func IgnoreInsignificantSecretUpdates(logger logr.Logger) predicate.Funcs {
426+
return predicate.Funcs{
427+
UpdateFunc: func(e event.UpdateEvent) bool {
428+
oldSecret, ok := e.ObjectOld.(*corev1.Secret)
429+
if !ok {
430+
return false
431+
}
432+
newSecret, ok := e.ObjectNew.(*corev1.Secret)
433+
if !ok {
434+
return false
435+
}
436+
if reflect.DeepEqual(oldSecret.Data, newSecret.Data) {
437+
return false
438+
}
439+
logger.V(1).Info("Secret data changed, will enqueue HCloudMachines",
440+
"namespace", newSecret.GetNamespace(), "name", newSecret.GetName())
441+
return true
442+
},
443+
CreateFunc: func(_ event.CreateEvent) bool { return true },
444+
DeleteFunc: func(_ event.DeleteEvent) bool { return true },
445+
GenericFunc: func(_ event.GenericEvent) bool { return false },
446+
}
447+
}
448+
383449
// IgnoreInsignificantHetznerClusterUpdates is a predicate used for ignoring insignificant HetznerCluster.Status updates.
384450
func IgnoreInsignificantHetznerClusterUpdates(logger logr.Logger) predicate.Funcs {
385451
return predicate.Funcs{

controllers/hcloudmachine_controller_test.go

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,29 @@ limitations under the License.
1717
package controllers
1818

1919
import (
20+
"context"
2021
"fmt"
2122
"testing"
2223

2324
"github.com/hetznercloud/hcloud-go/v2/hcloud"
2425
. "github.com/onsi/ginkgo/v2"
2526
. "github.com/onsi/gomega"
27+
"github.com/stretchr/testify/require"
2628
corev1 "k8s.io/api/core/v1"
2729
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
30+
"k8s.io/apimachinery/pkg/runtime"
31+
"k8s.io/apimachinery/pkg/types"
32+
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
2833
"k8s.io/klog/v2"
2934
"k8s.io/utils/ptr"
3035
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
3136
"sigs.k8s.io/cluster-api/util/conditions"
3237
"sigs.k8s.io/cluster-api/util/patch"
3338
"sigs.k8s.io/controller-runtime/pkg/client"
39+
fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
3440
"sigs.k8s.io/controller-runtime/pkg/event"
3541
"sigs.k8s.io/controller-runtime/pkg/predicate"
42+
"sigs.k8s.io/controller-runtime/pkg/reconcile"
3643

3744
infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
3845
hcloudclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/hcloud/client"
@@ -223,6 +230,141 @@ func TestIgnoreInsignificantMachineStatusUpdates(t *testing.T) {
223230
}
224231
}
225232

233+
func TestIgnoreInsignificantSecretUpdates(t *testing.T) {
234+
p := IgnoreInsignificantSecretUpdates(klog.Background())
235+
236+
makeSecret := func(data map[string][]byte, rv string) *corev1.Secret {
237+
return &corev1.Secret{
238+
ObjectMeta: metav1.ObjectMeta{
239+
Name: "hetzner",
240+
Namespace: "default",
241+
ResourceVersion: rv,
242+
},
243+
Data: data,
244+
}
245+
}
246+
247+
testCases := []struct {
248+
name string
249+
oldObj *corev1.Secret
250+
newObj *corev1.Secret
251+
expected bool
252+
}{
253+
{
254+
name: "Data changed",
255+
oldObj: makeSecret(map[string][]byte{"hcloud-token": []byte("old")}, "1"),
256+
newObj: makeSecret(map[string][]byte{"hcloud-token": []byte("new")}, "2"),
257+
expected: true,
258+
},
259+
{
260+
name: "Only ResourceVersion changed",
261+
oldObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same")}, "1"),
262+
newObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same")}, "2"),
263+
expected: false,
264+
},
265+
{
266+
name: "Unrelated data key added",
267+
oldObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same")}, "1"),
268+
newObj: makeSecret(map[string][]byte{"hcloud-token": []byte("same"), "other": []byte("x")}, "2"),
269+
expected: true,
270+
},
271+
}
272+
273+
for _, tc := range testCases {
274+
t.Run(tc.name, func(t *testing.T) {
275+
got := p.Update(event.UpdateEvent{ObjectOld: tc.oldObj, ObjectNew: tc.newObj})
276+
require.Equal(t, tc.expected, got)
277+
})
278+
}
279+
280+
require.True(t, p.Create(event.CreateEvent{Object: makeSecret(nil, "1")}))
281+
require.True(t, p.Delete(event.DeleteEvent{Object: makeSecret(nil, "1")}))
282+
require.False(t, p.Generic(event.GenericEvent{Object: makeSecret(nil, "1")}))
283+
}
284+
285+
func TestHetznerSecretToHCloudMachines(t *testing.T) {
286+
ctx := context.Background()
287+
288+
testScheme := runtime.NewScheme()
289+
utilruntime.Must(corev1.AddToScheme(testScheme))
290+
utilruntime.Must(infrav1.AddToScheme(testScheme))
291+
utilruntime.Must(clusterv1.AddToScheme(testScheme))
292+
293+
const (
294+
ns = "default"
295+
secretName = "hetzner"
296+
clusterName = "cluster-a"
297+
)
298+
299+
newCluster := func(name string) *clusterv1.Cluster {
300+
return &clusterv1.Cluster{
301+
ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns, UID: types.UID(name + "-uid")},
302+
}
303+
}
304+
newHetznerCluster := func(name, clusterOwner, secret string) *infrav1.HetznerCluster {
305+
return &infrav1.HetznerCluster{
306+
ObjectMeta: metav1.ObjectMeta{
307+
Name: name,
308+
Namespace: ns,
309+
OwnerReferences: []metav1.OwnerReference{
310+
{APIVersion: clusterv1.GroupVersion.String(), Kind: "Cluster", Name: clusterOwner, UID: types.UID(clusterOwner + "-uid")},
311+
},
312+
},
313+
Spec: infrav1.HetznerClusterSpec{
314+
HetznerSecret: infrav1.HetznerSecretRef{
315+
Name: secret,
316+
Key: infrav1.HetznerSecretKeyRef{HCloudToken: "hcloud-token"},
317+
},
318+
},
319+
}
320+
}
321+
newMachine := func(name, clusterOwner, infraName string) *clusterv1.Machine {
322+
return &clusterv1.Machine{
323+
ObjectMeta: metav1.ObjectMeta{
324+
Name: name,
325+
Namespace: ns,
326+
Labels: map[string]string{clusterv1.ClusterNameLabel: clusterOwner},
327+
},
328+
Spec: clusterv1.MachineSpec{
329+
ClusterName: clusterOwner,
330+
InfrastructureRef: corev1.ObjectReference{
331+
APIVersion: infrav1.GroupVersion.String(),
332+
Kind: "HCloudMachine",
333+
Name: infraName,
334+
},
335+
},
336+
}
337+
}
338+
339+
capiClusterA := newCluster(clusterName)
340+
capiClusterB := newCluster("cluster-b")
341+
hcA := newHetznerCluster("hc-a", clusterName, secretName)
342+
hcB := newHetznerCluster("hc-b", "cluster-b", secretName)
343+
hcUnrelated := newHetznerCluster("hc-u", clusterName, "other-secret")
344+
hcmA := &infrav1.HCloudMachine{ObjectMeta: metav1.ObjectMeta{Name: "m-a", Namespace: ns}}
345+
hcmB := &infrav1.HCloudMachine{ObjectMeta: metav1.ObjectMeta{Name: "m-b", Namespace: ns}}
346+
cmA := newMachine("cm-a", clusterName, hcmA.Name)
347+
cmB := newMachine("cm-b", "cluster-b", hcmB.Name)
348+
matchingSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: secretName, Namespace: ns}}
349+
otherSecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "no-ref", Namespace: ns}}
350+
351+
c := fakeclient.NewClientBuilder().
352+
WithScheme(testScheme).
353+
WithObjects(capiClusterA, capiClusterB, hcA, hcB, hcUnrelated, hcmA, hcmB, cmA, cmB).
354+
Build()
355+
356+
r := &HCloudMachineReconciler{Client: c}
357+
mapper := r.HetznerSecretToHCloudMachines(ctx)
358+
359+
got := mapper(ctx, matchingSecret)
360+
require.ElementsMatch(t, []reconcile.Request{
361+
{NamespacedName: client.ObjectKey{Namespace: ns, Name: hcmA.Name}},
362+
{NamespacedName: client.ObjectKey{Namespace: ns, Name: hcmB.Name}},
363+
}, got)
364+
365+
require.Empty(t, mapper(ctx, otherSecret))
366+
}
367+
226368
var _ = Describe("HCloudMachineReconciler", func() {
227369
var (
228370
capiCluster *clusterv1.Cluster

0 commit comments

Comments
 (0)