🌱 Update names of finalizers. (#1455)

* 🌱 Update names of finalizers.

old: foo.infrastructure.cluster.x-k8s.io
new: infrastructure.cluster.x-k8s.io/foo

Related: #1450

api/v1beta1/hcloudmachine_types.go

@@ -24,10 +24,13 @@ import (
 )
 
 const (
-	// MachineFinalizer allows ReconcileHCloudMachine to clean up HCloud
+	// HCloudMachineFinalizer allows ReconcileHCloudMachine to clean up HCloud
 	// resources associated with HCloudMachine before removing it from the
 	// apiserver.
-	MachineFinalizer = "hcloudmachine.infrastructure.cluster.x-k8s.io"
+	HCloudMachineFinalizer = "infrastructure.cluster.x-k8s.io/hcloudmachine"
+	// DeprecatedHCloudMachineFinalizer contains the old string.
+	// The controller will automatically update to the new string.
+	DeprecatedHCloudMachineFinalizer = "hcloudmachine.infrastructure.cluster.x-k8s.io"
 )
 
 // HCloudMachineSpec defines the desired state of HCloudMachine.

api/v1beta1/hcloudremediation_types.go

@@ -22,10 +22,6 @@ import (
 )
 
 const (
-	// HCloudRemediationFinalizer allows HCloudRemediationReconciler to clean up resources associated with HCloudRemediation before
-	// removing it from the apiserver.
-	HCloudRemediationFinalizer = "hcloudremediation.infrastructure.cluster.x-k8s.io"
-
 	// HCloudRebootAnnotation indicates that a bare metal host object should be rebooted.
 	HCloudRebootAnnotation = "reboot.hcloud.infrastructure.cluster.x-k8s.io"
 )

api/v1beta1/hetznerbaremetalhost_types.go

@@ -31,10 +31,14 @@ import (
 )
 
 const (
-	// BareMetalHostFinalizer is the name of the finalizer added to
+	// HetznerBareMetalHostFinalizer is the name of the finalizer added to
 	// hosts to block delete operations until the physical host can be
 	// deprovisioned.
-	BareMetalHostFinalizer string = "hetznerbaremetalhost.infrastructure.cluster.x-k8s.io"
+	HetznerBareMetalHostFinalizer = "infrastructure.cluster.x-k8s.io/hetznerbaremetalhost"
+
+	// DeprecatedBareMetalHostFinalizer contains the old string.
+	// The controller will automatically update to the new string.
+	DeprecatedBareMetalHostFinalizer = "hetznerbaremetalhost.infrastructure.cluster.x-k8s.io"
 
 	// HostAnnotation is the key for an annotation that should go on a HetznerBareMetalMachine to
 	// reference what HetznerBareMetalHost it corresponds to.
@@ -132,17 +136,17 @@ const (
 
 const (
 	// ErrorMessageMissingRootDeviceHints specifies the error message when no root device hints are specified.
-	ErrorMessageMissingRootDeviceHints string = "no root device hints specified"
+	ErrorMessageMissingRootDeviceHints = "no root device hints specified"
 	// ErrorMessageInvalidRootDeviceHints specifies the error message when invalid root device hints are specified.
-	ErrorMessageInvalidRootDeviceHints string = "invalid root device hints specified"
+	ErrorMessageInvalidRootDeviceHints = "invalid root device hints specified"
 	// ErrorMessageMissingHetznerSecret specifies the error message when no Hetzner secret is found.
-	ErrorMessageMissingHetznerSecret string = "could not find HetznerSecret"
+	ErrorMessageMissingHetznerSecret = "could not find HetznerSecret"
 	// ErrorMessageMissingRescueSSHSecret specifies the error message when no RescueSSH secret is found.
-	ErrorMessageMissingRescueSSHSecret string = "could not find RescueSSHSecret"
+	ErrorMessageMissingRescueSSHSecret = "could not find RescueSSHSecret"
 	// ErrorMessageMissingOSSSHSecret specifies the error message when no OSSSH secret is found.
-	ErrorMessageMissingOSSSHSecret string = "could not find OSSSHSecret"
+	ErrorMessageMissingOSSSHSecret = "could not find OSSSHSecret"
 	// ErrorMessageMissingOrInvalidSecretData specifies the error message when no data in secret is missing or invalid.
-	ErrorMessageMissingOrInvalidSecretData string = "invalid or not specified information in secret"
+	ErrorMessageMissingOrInvalidSecretData = "invalid or not specified information in secret"
 )
 
 // ProvisioningState defines the states of provisioning of the host.

api/v1beta1/hetznerbaremetalmachine_types.go

@@ -29,9 +29,13 @@ import (
 )
 
 const (
-	// BareMetalMachineFinalizer allows Reconcilehetznerbaremetalmachine to clean up resources associated with hetznerbaremetalmachine before
+	// HetznerBareMetalMachineFinalizer allows Reconcilehetznerbaremetalmachine to clean up resources associated with hetznerbaremetalmachine before
 	// removing it from the apiserver.
-	BareMetalMachineFinalizer = "hetznerbaremetalmachine.infrastructure.cluster.x-k8s.io"
+	HetznerBareMetalMachineFinalizer = "infrastructure.cluster.x-k8s.io/hetznerbaremetalmachine"
+
+	// DeprecatedBareMetalMachineFinalizer contains the old string.
+	// The controller will automatically update to the new string.
+	DeprecatedBareMetalMachineFinalizer = "hetznerbaremetalmachine.infrastructure.cluster.x-k8s.io"
 
 	// BareMetalHostNamePrefix is a prefix for all hostNames of bare metal servers.
 	BareMetalHostNamePrefix = "bm-"

api/v1beta1/hetznerbaremetalremediation_types.go

@@ -21,10 +21,6 @@ import (
 )
 
 const (
-	// RemediationFinalizer allows HetznerBareMetalRemediationReconciler to clean up resources associated with HetznerBareMetalRemediation before
-	// removing it from the apiserver.
-	RemediationFinalizer = "hetznerbaremetalremediation.infrastructure.cluster.x-k8s.io"
-
 	// RebootAnnotation indicates that a bare metal host object should be rebooted.
 	RebootAnnotation = "reboot.hetznerbaremetalhost.infrastructure.cluster.x-k8s.io"
 

api/v1beta1/hetznercluster_types.go

@@ -22,10 +22,15 @@ import (
 )
 
 const (
-	// ClusterFinalizer allows ReconcileHetznerCluster to clean up HCloud
+	// HetznerClusterFinalizer allows ReconcileHetznerCluster to clean up HCloud
 	// resources associated with HetznerCluster before removing it from the
 	// apiserver.
-	ClusterFinalizer = "hetznercluster.infrastructure.cluster.x-k8s.io"
+	HetznerClusterFinalizer = "infrastructure.cluster.x-k8s.io/hetznercluster"
+
+	// DeprecatedHetznerClusterFinalizer contains the old string.
+	// The controller will automatically update to the new string.
+	DeprecatedHetznerClusterFinalizer = "hetznercluster.infrastructure.cluster.x-k8s.io"
+
 	// AllowEmptyControlPlaneAddressAnnotation allows HetznerCluster Webhook
 	// to skip some validation steps for externally managed control planes.
 	AllowEmptyControlPlaneAddressAnnotation = "capi.syself.com/allow-empty-control-plane-address"

controllers/hcloudmachine_controller.go

@@ -187,7 +187,8 @@ func (r *HCloudMachineReconciler) reconcileDelete(ctx context.Context, machineSc
 		return result, nil
 	}
 	// Machine is deleted so remove the finalizer.
-	controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.MachineFinalizer)
+	controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.HCloudMachineFinalizer)
+	controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.DeprecatedHCloudMachineFinalizer)
 
 	return reconcile.Result{}, nil
 }
@@ -196,7 +197,8 @@ func (r *HCloudMachineReconciler) reconcileNormal(ctx context.Context, machineSc
 	hcloudMachine := machineScope.HCloudMachine
 
 	// If the HCloudMachine doesn't have our finalizer, add it.
-	controllerutil.AddFinalizer(machineScope.HCloudMachine, infrav1.MachineFinalizer)
+	controllerutil.AddFinalizer(machineScope.HCloudMachine, infrav1.HCloudMachineFinalizer)
+	controllerutil.RemoveFinalizer(machineScope.HCloudMachine, infrav1.DeprecatedHCloudMachineFinalizer)
 
 	// Register the finalizer immediately to avoid orphaning HCloud resources on delete.
 	if err := machineScope.PatchObject(ctx); err != nil {

controllers/hcloudmachinetemplate_controller.go

@@ -78,7 +78,8 @@ func (r *HCloudMachineTemplateReconciler) Reconcile(ctx context.Context, req rec
 	}()
 
 	// removing finalizer that was set in previous versions but is not needed
-	controllerutil.RemoveFinalizer(machineTemplate, infrav1.MachineFinalizer)
+	// We can remove that code in 2025.
+	controllerutil.RemoveFinalizer(machineTemplate, infrav1.DeprecatedHCloudMachineFinalizer)
 
 	// Check whether owner is a ClusterClass. In that case there is nothing to do.
 	if hasOwnerClusterClass(machineTemplate.ObjectMeta) {

controllers/hcloudremediation_controller_test.go

@@ -258,7 +258,7 @@ var _ = Describe("HCloudRemediationReconciler", func() {
 			}, timeout).Should(BeTrue())
 		})
 
-		It("should delete machine if retry limit reached and reboot timed out", func() {
+		It("should delete machine if retry limit reached and reboot timed out (hcloud)", func() {
 			By("creating hcloudRemediation")
 			Expect(testEnv.Create(ctx, hcloudRemediation)).To(Succeed())
 

controllers/hetznerbaremetalhost_controller.go

@@ -35,6 +35,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -46,7 +47,6 @@ import (
 	robotclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/robot"
 	sshclient "github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/client/ssh"
 	"github.com/syself/cluster-api-provider-hetzner/pkg/services/baremetal/host"
-	"github.com/syself/cluster-api-provider-hetzner/pkg/utils"
 )
 
 // HetznerBareMetalHostReconciler reconciles a HetznerBareMetalHost object.
@@ -87,12 +87,12 @@ func (r *HetznerBareMetalHostReconciler) Reconcile(ctx context.Context, req ctrl
 	}
 
 	// Add a finalizer to newly created objects.
-	if bmHost.DeletionTimestamp.IsZero() && !hostHasFinalizer(bmHost) {
-		bmHost.Finalizers = append(bmHost.Finalizers,
-			infrav1.BareMetalHostFinalizer)
+	if bmHost.DeletionTimestamp.IsZero() &&
+		(controllerutil.AddFinalizer(bmHost, infrav1.HetznerBareMetalHostFinalizer) ||
+			controllerutil.RemoveFinalizer(bmHost, infrav1.DeprecatedBareMetalHostFinalizer)) {
 		err := r.Update(ctx, bmHost)
 		if err != nil {
-			return reconcile.Result{}, fmt.Errorf("failed to add finalizer: %w", err)
+			return reconcile.Result{}, fmt.Errorf("failed to update finalizer: %w", err)
 		}
 		return ctrl.Result{Requeue: true}, nil
 	}
@@ -230,21 +230,20 @@ func (r *HetznerBareMetalHostReconciler) reconcileSelectedStates(ctx context.Con
 		if needsUpdate {
 			err := r.Update(ctx, bmHost)
 			if err != nil {
-				return reconcile.Result{}, fmt.Errorf("failed to add finalizer: %w", err)
+				return reconcile.Result{}, fmt.Errorf("Update() failed after setting ProvisioningState: %w", err)
 			}
 		}
 
 		return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 
 		// Handle StateDeleting
 	case infrav1.StateDeleting:
-		if !utils.StringInList(bmHost.Finalizers, infrav1.BareMetalHostFinalizer) {
-			return reconcile.Result{Requeue: true}, nil
-		}
-
-		bmHost.Finalizers = utils.FilterStringFromList(bmHost.Finalizers, infrav1.BareMetalHostFinalizer)
-		if err := r.Update(ctx, bmHost); err != nil {
-			return reconcile.Result{}, fmt.Errorf("failed to remove finalizer: %w", err)
+		if controllerutil.RemoveFinalizer(bmHost, infrav1.HetznerBareMetalHostFinalizer) ||
+			controllerutil.RemoveFinalizer(bmHost, infrav1.DeprecatedBareMetalHostFinalizer) {
+			// at least one finalizer was removed.
+			if err := r.Update(ctx, bmHost); err != nil {
+				return reconcile.Result{}, fmt.Errorf("failed to remove finalizer: %w", err)
+			}
 		}
 		return reconcile.Result{Requeue: true}, nil
 	}
@@ -411,10 +410,6 @@ func hetznerSecretErrorResult(
 	return reconcile.Result{}, fmt.Errorf("hetznerSecretErrorResult: an unhandled failure occurred: %T %w", err, err)
 }
 
-func hostHasFinalizer(host *infrav1.HetznerBareMetalHost) bool {
-	return utils.StringInList(host.Finalizers, infrav1.BareMetalHostFinalizer)
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *HetznerBareMetalHostReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
 	return ctrl.NewControllerManagedBy(mgr).

controllers/hetznerbaremetalhost_controller_test.go

@@ -226,7 +226,7 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
 					return false
 				}
 				for _, finalizer := range host.GetFinalizers() {
-					if finalizer == infrav1.BareMetalHostFinalizer {
+					if finalizer == infrav1.HetznerBareMetalHostFinalizer {
 						return true
 					}
 				}

controllers/hetznerbaremetalmachine_controller.go

@@ -178,14 +178,16 @@ func (r *HetznerBareMetalMachineReconciler) reconcileDelete(ctx context.Context,
 		return result, nil
 	}
 	// Machine is deleted so remove the finalizer.
-	controllerutil.RemoveFinalizer(machineScope.BareMetalMachine, infrav1.BareMetalMachineFinalizer)
+	controllerutil.RemoveFinalizer(machineScope.BareMetalMachine, infrav1.HetznerBareMetalMachineFinalizer)
+	controllerutil.RemoveFinalizer(machineScope.BareMetalMachine, infrav1.DeprecatedBareMetalMachineFinalizer)
 
 	return result, nil
 }
 
 func (r *HetznerBareMetalMachineReconciler) reconcileNormal(ctx context.Context, machineScope *scope.BareMetalMachineScope) (reconcile.Result, error) {
 	// If the HetznerBareMetalMachine doesn't have our finalizer, add it.
-	controllerutil.AddFinalizer(machineScope.BareMetalMachine, infrav1.BareMetalMachineFinalizer)
+	controllerutil.AddFinalizer(machineScope.BareMetalMachine, infrav1.HetznerBareMetalMachineFinalizer)
+	controllerutil.RemoveFinalizer(machineScope.BareMetalMachine, infrav1.DeprecatedBareMetalMachineFinalizer)
 
 	// Register the finalizer immediately to avoid orphaning HetznerBareMetal resources on delete
 	if err := machineScope.PatchObject(ctx); err != nil {

controllers/hetznerbaremetalmachine_controller_test.go

@@ -336,7 +336,7 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() {
 						return false
 					}
 					for _, finalizer := range bmMachine.GetFinalizers() {
-						if finalizer == infrav1.BareMetalMachineFinalizer {
+						if finalizer == infrav1.HetznerBareMetalMachineFinalizer {
 							return true
 						}
 					}

controllers/hetznerbaremetalremediation_controller_test.go

@@ -358,7 +358,7 @@ var _ = Describe("HetznerBareMetalRemediationReconciler", func() {
 					}, timeout).Should(BeTrue())
 				})
 
-				It("should delete machine if retry limit reached and reboot timed out", func() {
+				It("should delete machine if retry limit reached and reboot timed out (bm)", func() {
 					By("creating hetznerBareMetalRemediation object")
 					Expect(testEnv.Create(ctx, hetznerBareMetalRemediation)).To(Succeed())
 

controllers/hetznercluster_controller.go

@@ -178,7 +178,9 @@ func (r *HetznerClusterReconciler) reconcileNormal(ctx context.Context, clusterS
 	hetznerCluster := clusterScope.HetznerCluster
 
 	// If the HetznerCluster doesn't have our finalizer, add it.
-	controllerutil.AddFinalizer(hetznerCluster, infrav1.ClusterFinalizer)
+	controllerutil.AddFinalizer(hetznerCluster, infrav1.HetznerClusterFinalizer)
+	controllerutil.RemoveFinalizer(hetznerCluster, infrav1.DeprecatedHetznerClusterFinalizer)
+
 	if err := clusterScope.PatchObject(ctx); err != nil {
 		return reconcile.Result{}, err
 	}
@@ -366,7 +368,8 @@ func (r *HetznerClusterReconciler) reconcileDelete(ctx context.Context, clusterS
 	}
 
 	// Cluster is deleted so remove the finalizer.
-	controllerutil.RemoveFinalizer(clusterScope.HetznerCluster, infrav1.ClusterFinalizer)
+	controllerutil.RemoveFinalizer(clusterScope.HetznerCluster, infrav1.HetznerClusterFinalizer)
+	controllerutil.RemoveFinalizer(clusterScope.HetznerCluster, infrav1.DeprecatedHetznerClusterFinalizer)
 
 	return reconcile.Result{}, nil
 }

pkg/secrets/secret.go

@@ -35,7 +35,11 @@ import (
 
 const (
 	// SecretFinalizer is the finalizer for secrets.
-	SecretFinalizer = infrav1.ClusterFinalizer + "/secret"
+	SecretFinalizer = "infrastructure.cluster.x-k8s.io/caph-secret"
+
+	// DeprecatedSecretFinalizer contains the old string.
+	// The controller will automatically update to the new string.
+	DeprecatedSecretFinalizer = infrav1.DeprecatedHetznerClusterFinalizer + "/secret"
 )
 
 // SecretManager is a type for fetching Secrets whether or not they are in the
@@ -91,8 +95,8 @@ func (sm *SecretManager) claimSecret(ctx context.Context, secret *corev1.Secret,
 		}
 	}
 
-	if addFinalizer && !utils.StringInList(secret.Finalizers, SecretFinalizer) {
-		secret.Finalizers = append(secret.Finalizers, SecretFinalizer)
+	if addFinalizer && (controllerutil.AddFinalizer(secret, SecretFinalizer) ||
+		controllerutil.RemoveFinalizer(secret, DeprecatedSecretFinalizer)) {
 		needsUpdate = true
 	}
 
@@ -180,8 +184,8 @@ func (sm *SecretManager) ReleaseSecret(ctx context.Context, secret *corev1.Secre
 
 	// remove finalizer from secret to allow deletion if no other owner exists
 	if !foundOtherHetznerClusterOwner {
-		secret.Finalizers = utils.FilterStringFromList(
-			secret.Finalizers, SecretFinalizer)
+		controllerutil.RemoveFinalizer(secret, SecretFinalizer)
+		controllerutil.RemoveFinalizer(secret, DeprecatedSecretFinalizer)
 	}
 
 	secret.OwnerReferences = newOwnerRefs

pkg/services/baremetal/host/host.go

@@ -40,6 +40,7 @@ import (
 	"sigs.k8s.io/cluster-api/util/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
@@ -1803,7 +1804,8 @@ func (s *Service) actionDeprovisioning(_ context.Context) actionResult {
 }
 
 func (s *Service) actionDeleting(_ context.Context) actionResult {
-	s.scope.HetznerBareMetalHost.Finalizers = utils.FilterStringFromList(s.scope.HetznerBareMetalHost.Finalizers, infrav1.BareMetalHostFinalizer)
+	controllerutil.RemoveFinalizer(s.scope.HetznerBareMetalHost, infrav1.HetznerBareMetalHostFinalizer)
+	controllerutil.RemoveFinalizer(s.scope.HetznerBareMetalHost, infrav1.DeprecatedBareMetalHostFinalizer)
 	return deleteComplete{}
 }
 

pkg/utils/utils.go

@@ -172,17 +172,6 @@ func StringInList(list []string, strToSearch string) bool {
 	return false
 }
 
-// FilterStringFromList produces a new string slice that does not
-// include the strToFilter argument.
-func FilterStringFromList(list []string, strToFilter string) (newList []string) {
-	for _, item := range list {
-		if item != strToFilter {
-			newList = append(newList, item)
-		}
-	}
-	return
-}
-
 // GenerateName takes a name as string pointer. It returns name if pointer is not nil, otherwise it returns fallback with random suffix.
 func GenerateName(name *string, fallback string) string {
 	if name != nil {

pkg/utils/utils_test.go

@@ -177,29 +177,6 @@ var _ = Describe("StringInList", func() {
 		}))
 })
 
-var _ = Describe("FilterStringFromList", func() {
-	type testCaseFilterStringFromList struct {
-		list            []string
-		str             string
-		expectedOutcome []string
-	}
-	DescribeTable("Test filter string from list",
-		func(tc testCaseFilterStringFromList) {
-			out := utils.FilterStringFromList(tc.list, tc.str)
-			Expect(out).To(Equal(tc.expectedOutcome))
-		},
-		Entry("entry1", testCaseFilterStringFromList{
-			list:            []string{"a", "b", "c"},
-			str:             "a",
-			expectedOutcome: []string{"b", "c"},
-		}),
-		Entry("entry2", testCaseFilterStringFromList{
-			list:            []string{"a", "b", "c"},
-			str:             "d",
-			expectedOutcome: []string{"a", "b", "c"},
-		}))
-})
-
 var _ = Describe("Test removeOwnerRefFromList", func() {
 	type testCaseRemoveOwnerRefFromList struct {
 		RefList         []metav1.OwnerReference