Repository: syself/cluster-api-provider-hetzner (go)

Issue:
Webhook testing that not multiple HetznerBareMetalHost objects share the same serverID
## Description

A webhook that tests for HetznerBareMetalHosts on creation whether other hosts exist that have the same ServerID in the spec. If so, we should return an error that this must not happen.

If the client.List call of objects fails, we should emit a warning that we could not verify that the host has a unique serverID

Pull request:
✨ Add webhook to check if same serverID baremetal host exist or not
<!-- If this is your first PR, welcome! Please make sure you read the [contributing guidelines](../CONTRIBUTING.md#contributing-a-patch). -->
<!-- please add an icon to the title of this PR and delete this line and similar ones -->
<!-- the icon will be either ⚠️ (:warning:, major or breaking changes), ✨ (:sparkles:, feature additions), 🐛 (:bug:, patches and bugfixes), 📖 (:book:, documentation or proposals), or 🌱 (:seedling:, minor or other) -->

**What this PR does / why we need it**:

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes #928 

**Special notes for your reviewer**:

_Please confirm that if this PR changes any image versions, then that's the sole change this PR makes._

**TODOs**:

- [ ] squash commits
- [ ] include documentation
- [ ] add unit tests



Patch:
diff --git a/api/v1beta1/hetznerbaremetalhost_webhook.go b/api/v1beta1/hetznerbaremetalhost_webhook.go
index 8fdbf474..b6379682 100644
--- a/api/v1beta1/hetznerbaremetalhost_webhook.go
+++ b/api/v1beta1/hetznerbaremetalhost_webhook.go
@@ -17,19 +17,31 @@ limitations under the License.
 package v1beta1
 
 import (
-	"reflect"
+	"context"
+	"fmt"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
+// HetznerBareMetalHostWebhook implements validating and defaulting webhook for HetznerBareMetalHost.
+// +k8s:deepcopy-gen=false
+type HetznerBareMetalHostWebhook struct {
+	c client.Client
+}
+
 // SetupWebhookWithManager initializes webhook manager for HetznerBareMetalHost.
-func (host *HetznerBareMetalHost) SetupWebhookWithManager(mgr ctrl.Manager) error {
+func (hw *HetznerBareMetalHostWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	hw.c = mgr.GetClient()
+
 	return ctrl.NewWebhookManagedBy(mgr).
-		For(host).
+		For(&HetznerBareMetalHost{}).
+		WithValidator(hw).
 		Complete()
 }
 
@@ -43,28 +55,57 @@ func (host *HetznerBareMetalHost) Default() {
 
 //+kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-hetznerbaremetalhost,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=hetznerbaremetalhosts,verbs=create;update,versions=v1beta1,name=validation.hetznerbaremetalhost.infrastructure.cluster.x-k8s.io,admissionReviewVersions={v1,v1beta1}
 
-var _ webhook.Validator = &HetznerBareMetalHost{}
+var _ webhook.CustomValidator = &HetznerBareMetalHostWebhook{}
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (host *HetznerBareMetalHost) ValidateCreate() (admission.Warnings, error) {
-	return nil, nil
+func (hw *HetznerBareMetalHostWebhook) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	host, ok := (obj).(*HetznerBareMetalHost)
+	if !ok {
+		return admission.Warnings{}, apierrors.NewBadRequest(fmt.Sprintf("expected HetznerBareMetalHost, but got %T", host))
+	}
+
+	var allErrs field.ErrorList
+
+	hetznerBareMetalHostList := &HetznerBareMetalHostList{}
+	if err := hw.c.List(ctx, hetznerBareMetalHostList, &client.ListOptions{}); err != nil {
+		return admission.Warnings{fmt.Sprintf("could not verify that the host has a unique serverID: %s", err.Error())}, nil
+	}
+
+	for _, hetznerBareMetalHost := range hetznerBareMetalHostList.Items {
+		if hetznerBareMetalHost.Spec.ServerID == host.Spec.ServerID {
+			allErrs = append(allErrs,
+				field.Invalid(field.NewPath("spec", "ServerID"), host.Spec.ServerID, fmt.Sprintf("%q host exist with same serverID: %d", hetznerBareMetalHost.Name, host.Spec.ServerID)),
+			)
+		}
+	}
+
+	return nil, aggregateObjErrors(hetznerBareMetalHostList.GroupVersionKind().GroupKind(), host.Name, allErrs)
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (host *HetznerBareMetalHost) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
+func (hw *HetznerBareMetalHostWebhook) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
+	oldHost, ok := oldObj.(*HetznerBareMetalHost)
+	if !ok {
+		return admission.Warnings{}, apierrors.NewBadRequest(fmt.Sprintf("expected an ClusterStack but got a %T", oldObj))
+	}
+
+	newHost, ok := newObj.(*HetznerBareMetalHost)
+	if !ok {
+		return admission.Warnings{}, apierrors.NewBadRequest(fmt.Sprintf("expected an ClusterStack but got a %T", newHost))
+	}
+
 	var allErrs field.ErrorList
 
-	oldHetznerBareMetalHost := old.(*HetznerBareMetalHost)
-	if !reflect.DeepEqual(host.Spec.ServerID, oldHetznerBareMetalHost.Spec.ServerID) {
+	if newHost.Spec.ServerID != oldHost.Spec.ServerID {
 		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "serverID"), host.Spec.ServerID, "ServerID is immutable"),
+			field.Invalid(field.NewPath("spec", "serverID"), newHost.Spec.ServerID, "serverID is immutable"),
 		)
 	}
 
-	return nil, aggregateObjErrors(host.GroupVersionKind().GroupKind(), host.Name, allErrs)
+	return nil, aggregateObjErrors(newHost.GroupVersionKind().GroupKind(), newHost.Name, allErrs)
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (host *HetznerBareMetalHost) ValidateDelete() (admission.Warnings, error) {
+func (hw *HetznerBareMetalHostWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
 	return nil, nil
 }
diff --git a/main.go b/main.go
index efbe2284..ba5fba69 100644
--- a/main.go
+++ b/main.go
@@ -237,7 +237,7 @@ func setUpWebhookWithManager(mgr ctrl.Manager) {
 		setupLog.Error(err, "unable to create webhook", "webhook", "HCloudMachineTemplate")
 		os.Exit(1)
 	}
-	if err := (&infrastructurev1beta1.HetznerBareMetalHost{}).SetupWebhookWithManager(mgr); err != nil {
+	if err := (&infrastructurev1beta1.HetznerBareMetalHostWebhook{}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "HetznerBareMetalHost")
 		os.Exit(1)
 	}
diff --git a/pkg/services/baremetal/host/host.go b/pkg/services/baremetal/host/host.go
index 9f29e888..a4a8574f 100644
--- a/pkg/services/baremetal/host/host.go
+++ b/pkg/services/baremetal/host/host.go
@@ -1387,8 +1387,8 @@ func (s *Service) actionDeleting() actionResult {
 }
 
 func (s *Service) handleRateLimitExceeded(err error, functionName string) {
-	if models.IsError(err, models.ErrorCodeRateLimitExceeded) || strings.Contains(err.Error(), "server responded with status code 403") {
-		msg := fmt.Sprintf("exceeded rate limit with calling function %q: %s", functionName, err.Error())
+	if models.IsError(err, models.ErrorCodeRateLimitExceeded) {
+		msg := fmt.Sprintf("exceeded rate limit with calling function %q", functionName)
 		conditions.MarkFalse(
 			s.scope.HetznerBareMetalHost,
 			infrav1.HetznerAPIReachableCondition,
diff --git a/pkg/services/hcloud/client/client.go b/pkg/services/hcloud/client/client.go
index bf3dde08..599954bf 100644
--- a/pkg/services/hcloud/client/client.go
+++ b/pkg/services/hcloud/client/client.go
@@ -24,8 +24,6 @@ import (
 	"strings"
 
 	"github.com/hetznercloud/hcloud-go/v2/hcloud"
-	"github.com/prometheus/client_golang/prometheus"
-	"sigs.k8s.io/controller-runtime/pkg/metrics"
 )
 
 const errStringUnauthorized = "(unauthorized)"
@@ -78,17 +76,7 @@ type Factory interface {
 
 // NewClient creates new HCloud clients.
 func (f *factory) NewClient(hcloudToken string) Client {
-	opts := []hcloud.ClientOption{
-		hcloud.WithToken(hcloudToken),
-	}
-
-	// controller-runtime hides their default prometheus registry it uses (and exposes via HTTP) behind a custom
-	// interface. hcloud-go wants the actual registry struct, so we need to get it from the controller-runtime package.
-	if registry, ok := metrics.Registry.(*prometheus.Registry); ok {
-		opts = append(opts, hcloud.WithInstrumentation(registry))
-	}
-
-	return &realClient{client: hcloud.NewClient(opts...)}
+	return &realClient{client: hcloud.NewClient(hcloud.WithToken(hcloudToken))}
 }
 
 type factory struct{}
diff --git a/pkg/services/hcloud/placementgroup/placementgroup.go b/pkg/services/hcloud/placementgroup/placementgroup.go
index c3321643..23ce2ac2 100644
--- a/pkg/services/hcloud/placementgroup/placementgroup.go
+++ b/pkg/services/hcloud/placementgroup/placementgroup.go
@@ -120,10 +120,7 @@ func (s *Service) Reconcile(ctx context.Context) (err error) {
 	}
 
 	// Update status
-	if len(toCreate) > 0 || len(toDelete) > 0 {
-		// No need to update status if nothing changed
-		placementGroups, err = s.findPlacementGroups(ctx)
-	}
+	placementGroups, err = s.findPlacementGroups(ctx)
 	if err != nil {
 		return fmt.Errorf("failed to find placement groups: %w", err)
 	}


Test patch:
diff --git a/controllers/hetznerbaremetalhost_controller_test.go b/controllers/hetznerbaremetalhost_controller_test.go
index 2ac03a40..47348ffb 100644
--- a/controllers/hetznerbaremetalhost_controller_test.go
+++ b/controllers/hetznerbaremetalhost_controller_test.go
@@ -139,7 +139,7 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
 		osSSHClientAfterInstallImage = testEnv.OSSSHClientAfterInstallImage
 		osSSHClientAfterCloudInit = testEnv.OSSSHClientAfterCloudInit
 
-		robotClient.On("GetBMServer", 1).Return(&models.Server{
+		robotClient.On("GetBMServer", mock.Anything).Return(&models.Server{
 			ServerNumber: 1,
 			ServerIP:     "1.2.3.4",
 			Rescue:       true,
@@ -151,12 +151,12 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
 				Data:        "my-public-key",
 			},
 		}, nil)
-		robotClient.On("GetReboot", 1).Return(&models.Reset{Type: []string{"hw", "sw"}}, nil)
+		robotClient.On("GetReboot", mock.Anything).Return(&models.Reset{Type: []string{"hw", "sw"}}, nil)
 		robotClient.On("GetBootRescue", 1).Return(&models.Rescue{Active: true}, nil)
-		robotClient.On("SetBootRescue", 1, mock.Anything).Return(&models.Rescue{Active: true}, nil)
-		robotClient.On("DeleteBootRescue", 1).Return(&models.Rescue{Active: false}, nil)
+		robotClient.On("SetBootRescue", mock.Anything, mock.Anything).Return(&models.Rescue{Active: true}, nil)
+		robotClient.On("DeleteBootRescue", mock.Anything).Return(&models.Rescue{Active: false}, nil)
 		robotClient.On("RebootBMServer", mock.Anything, mock.Anything).Return(&models.ResetPost{}, nil)
-		robotClient.On("SetBMServerName", 1, mock.Anything).Return(nil, nil)
+		robotClient.On("SetBMServerName", mock.Anything, mock.Anything).Return(nil, nil)
 
 		configureRescueSSHClient(rescueSSHClient)
 
@@ -202,6 +202,10 @@ var _ = Describe("HetznerBareMetalHostReconciler", func() {
 			key = client.ObjectKey{Namespace: testNs.Name, Name: host.Name}
 		})
 
+		AfterEach(func() {
+			Expect(testEnv.Cleanup(ctx, host)).To(Succeed())
+		})
+
 		It("creates the host machine", func() {
 			Eventually(func() bool {
 				if err := testEnv.Get(ctx, key, host); err != nil {
@@ -637,7 +641,7 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
 		robotClient = testEnv.RobotClient
 		rescueSSHClient = testEnv.RescueSSHClient
 
-		robotClient.On("GetBMServer", 1).Return(&models.Server{
+		robotClient.On("GetBMServer", mock.Anything).Return(&models.Server{
 			ServerNumber: 1,
 			ServerIP:     "1.2.3.4",
 			Rescue:       true,
@@ -649,9 +653,9 @@ var _ = Describe("HetznerBareMetalHostReconciler - missing secrets", func() {
 				Data:        "my-public-key",
 			},
 		}, nil)
-		robotClient.On("GetReboot", 1).Return(&models.Reset{Type: []string{"hw", "sw"}}, nil)
-		robotClient.On("SetBootRescue", 1, mock.Anything).Return(&models.Rescue{Active: true}, nil)
-		robotClient.On("DeleteBootRescue", 1).Return(&models.Rescue{Active: true}, nil)
+		robotClient.On("GetReboot", mock.Anything).Return(&models.Reset{Type: []string{"hw", "sw"}}, nil)
+		robotClient.On("SetBootRescue", mock.Anything, mock.Anything).Return(&models.Rescue{Active: true}, nil)
+		robotClient.On("DeleteBootRescue", mock.Anything).Return(&models.Rescue{Active: true}, nil)
 		robotClient.On("RebootBMServer", mock.Anything, mock.Anything).Return(&models.ResetPost{}, nil)
 
 		configureRescueSSHClient(rescueSSHClient)
diff --git a/controllers/hetznerbaremetalmachine_controller_test.go b/controllers/hetznerbaremetalmachine_controller_test.go
index a30bf216..023f5c26 100644
--- a/controllers/hetznerbaremetalmachine_controller_test.go
+++ b/controllers/hetznerbaremetalmachine_controller_test.go
@@ -128,7 +128,7 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() {
 		rescueSSHClient = testEnv.RescueSSHClient
 		osSSHClient = testEnv.OSSSHClientAfterInstallImage
 
-		robotClient.On("GetBMServer", 1).Return(&models.Server{
+		robotClient.On("GetBMServer", mock.Anything).Return(&models.Server{
 			ServerNumber: 1,
 			ServerIP:     "1.2.3.4",
 			Rescue:       true,
@@ -140,12 +140,12 @@ var _ = Describe("HetznerBareMetalMachineReconciler", func() {
 				Data:        "my-public-key",
 			},
 		}, nil)
-		robotClient.On("GetReboot", 1).Return(&models.Reset{Type: []string{"hw", "sw"}}, nil)
+		robotClient.On("GetReboot", mock.Anything).Return(&models.Reset{Type: []string{"hw", "sw"}}, nil)
 		robotClient.On("GetBootRescue", 1).Return(&models.Rescue{Active: true}, nil)
-		robotClient.On("SetBootRescue", 1, mock.Anything).Return(&models.Rescue{Active: true}, nil)
-		robotClient.On("DeleteBootRescue", 1).Return(&models.Rescue{Active: false}, nil)
+		robotClient.On("SetBootRescue", mock.Anything, mock.Anything).Return(&models.Rescue{Active: true}, nil)
+		robotClient.On("DeleteBootRescue", mock.Anything).Return(&models.Rescue{Active: false}, nil)
 		robotClient.On("RebootBMServer", mock.Anything, mock.Anything).Return(&models.ResetPost{}, nil)
-		robotClient.On("SetBMServerName", 1, mock.Anything).Return(nil, nil)
+		robotClient.On("SetBMServerName", mock.Anything, mock.Anything).Return(nil, nil)
 
 		configureRescueSSHClient(rescueSSHClient)
 
diff --git a/pkg/services/baremetal/host/host_suite_test.go b/pkg/services/baremetal/host/host_suite_test.go
index 2bb281f0..db1e9dce 100644
--- a/pkg/services/baremetal/host/host_suite_test.go
+++ b/pkg/services/baremetal/host/host_suite_test.go
@@ -36,7 +36,6 @@ import (
 )
 
 const (
-	bareMetalHostID  = 1
 	sshFingerprint   = "my-fingerprint"
 	osSSHKeyName     = "os-sshkey"
 	rescueSSHKeyName = "rescue-sshkey"
diff --git a/pkg/services/baremetal/host/host_test.go b/pkg/services/baremetal/host/host_test.go
index 14bab7cf..6ec90861 100644
--- a/pkg/services/baremetal/host/host_test.go
+++ b/pkg/services/baremetal/host/host_test.go
@@ -201,9 +201,9 @@ var _ = Describe("handleIncompleteBoot", func() {
 		DescribeTable("hostName = rescue, varying error type and ssh client response - robot client giving all positive results, no timeouts",
 			func(tc testCaseHandleIncompleteBootCorrectHostname) {
 				robotMock := robotmock.Client{}
-				robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil)
-				robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil)
-				robotMock.On("RebootBMServer", bareMetalHostID, mock.Anything).Return(nil, nil)
+				robotMock.On("SetBootRescue", mock.Anything, sshFingerprint).Return(nil, nil)
+				robotMock.On("GetBootRescue", mock.Anything).Return(&models.Rescue{Active: true}, nil)
+				robotMock.On("RebootBMServer", mock.Anything, mock.Anything).Return(nil, nil)
 
 				host := helpers.BareMetalHost("test-host", "default",
 					helpers.WithRebootTypes([]infrav1.RebootType{
@@ -295,9 +295,9 @@ var _ = Describe("handleIncompleteBoot", func() {
 		DescribeTable("Different reset types",
 			func(tc testCaseHandleIncompleteBootDifferentResetTypes) {
 				robotMock := robotmock.Client{}
-				robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil)
-				robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil)
-				robotMock.On("RebootBMServer", bareMetalHostID, mock.Anything).Return(nil, nil)
+				robotMock.On("SetBootRescue", mock.Anything, sshFingerprint).Return(nil, nil)
+				robotMock.On("GetBootRescue", mock.Anything).Return(&models.Rescue{Active: true}, nil)
+				robotMock.On("RebootBMServer", mock.Anything, mock.Anything).Return(nil, nil)
 
 				host := helpers.BareMetalHost("test-host", "default",
 					helpers.WithSSHSpec(),
@@ -311,9 +311,9 @@ var _ = Describe("handleIncompleteBoot", func() {
 				Expect(service.handleIncompleteBoot(true, tc.isTimeOut, tc.isConnectionRefused)).To(Succeed())
 				Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedHostErrorType))
 				if tc.expectedRebootType != infrav1.RebootType("") {
-					Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, tc.expectedRebootType)).To(BeTrue())
+					Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", mock.Anything, tc.expectedRebootType)).To(BeTrue())
 				} else {
-					Expect(robotMock.AssertNotCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, mock.Anything)).To(BeTrue())
+					Expect(robotMock.AssertNotCalled(GinkgoT(), "RebootBMServer", mock.Anything, mock.Anything)).To(BeTrue())
 				}
 			},
 			Entry("timeout, no errorType, only hw reset", testCaseHandleIncompleteBootDifferentResetTypes{
@@ -377,9 +377,9 @@ var _ = Describe("handleIncompleteBoot", func() {
 		DescribeTable("Different timeouts",
 			func(tc testCaseHandleIncompleteBootDifferentTimeouts) {
 				robotMock := robotmock.Client{}
-				robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil)
-				robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil)
-				robotMock.On("RebootBMServer", bareMetalHostID, mock.Anything).Return(nil, nil)
+				robotMock.On("SetBootRescue", mock.Anything, sshFingerprint).Return(nil, nil)
+				robotMock.On("GetBootRescue", mock.Anything).Return(&models.Rescue{Active: true}, nil)
+				robotMock.On("RebootBMServer", mock.Anything, mock.Anything).Return(nil, nil)
 
 				host := helpers.BareMetalHost("test-host", "default",
 					helpers.WithRebootTypes([]infrav1.RebootType{
@@ -396,9 +396,9 @@ var _ = Describe("handleIncompleteBoot", func() {
 				Expect(service.handleIncompleteBoot(true, true, false)).To(Succeed())
 				Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedHostErrorType))
 				if tc.expectedRebootType != infrav1.RebootType("") {
-					Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, tc.expectedRebootType)).To(BeTrue())
+					Expect(robotMock.AssertCalled(GinkgoT(), "RebootBMServer", mock.Anything, tc.expectedRebootType)).To(BeTrue())
 				} else {
-					Expect(robotMock.AssertNotCalled(GinkgoT(), "RebootBMServer", bareMetalHostID, mock.Anything)).To(BeTrue())
+					Expect(robotMock.AssertNotCalled(GinkgoT(), "RebootBMServer", mock.Anything, mock.Anything)).To(BeTrue())
 				}
 			},
 			Entry("timed out hw reset", testCaseHandleIncompleteBootDifferentTimeouts{
@@ -440,9 +440,9 @@ var _ = Describe("handleIncompleteBoot", func() {
 		DescribeTable("vary hostname and see whether rescue gets triggered",
 			func(tc testCaseHandleIncompleteBoot) {
 				robotMock := robotmock.Client{}
-				robotMock.On("SetBootRescue", bareMetalHostID, sshFingerprint).Return(nil, nil)
-				robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil)
-				robotMock.On("RebootBMServer", bareMetalHostID, mock.Anything).Return(nil, nil)
+				robotMock.On("SetBootRescue", mock.Anything, sshFingerprint).Return(nil, nil)
+				robotMock.On("GetBootRescue", mock.Anything).Return(&models.Rescue{Active: true}, nil)
+				robotMock.On("RebootBMServer", mock.Anything, mock.Anything).Return(nil, nil)
 
 				host := helpers.BareMetalHost("test-host", "default",
 					helpers.WithRebootTypes([]infrav1.RebootType{
@@ -463,9 +463,9 @@ var _ = Describe("handleIncompleteBoot", func() {
 				}
 				Expect(host.Spec.Status.ErrorType).To(Equal(tc.expectedHostErrorType))
 				if tc.expectsRescueCall {
-					Expect(robotMock.AssertCalled(GinkgoT(), "GetBootRescue", bareMetalHostID)).To(BeTrue())
+					Expect(robotMock.AssertCalled(GinkgoT(), "GetBootRescue", mock.Anything)).To(BeTrue())
 				} else {
-					Expect(robotMock.AssertNotCalled(GinkgoT(), "GetBootRescue", bareMetalHostID)).To(BeTrue())
+					Expect(robotMock.AssertNotCalled(GinkgoT(), "GetBootRescue", mock.Anything)).To(BeTrue())
 				}
 			},
 			Entry("hostname == rescue", testCaseHandleIncompleteBoot{
@@ -642,7 +642,7 @@ var _ = Describe("analyzeSSHOutputInstallImage", func() {
 			)
 
 			robotMock := robotmock.Client{}
-			robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: tc.rescueActive}, nil)
+			robotMock.On("GetBootRescue", mock.Anything).Return(&models.Rescue{Active: tc.rescueActive}, nil)
 
 			service := newTestService(host, &robotMock, nil, nil, nil)
 
@@ -719,7 +719,7 @@ var _ = Describe("analyzeSSHOutputInstallImage", func() {
 			)
 
 			robotMock := robotmock.Client{}
-			robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: true}, nil)
+			robotMock.On("GetBootRescue", mock.Anything).Return(&models.Rescue{Active: true}, nil)
 
 			service := newTestService(host, &robotMock, nil, nil, nil)
 
@@ -1141,7 +1141,7 @@ var _ = Describe("actionRegistering", func() {
 			sshMock.On("GetHostName").Return(tc.getHostNameOutput)
 
 			robotMock := robotmock.Client{}
-			robotMock.On("GetBootRescue", bareMetalHostID).Return(&models.Rescue{Active: false}, nil)
+			robotMock.On("GetBootRescue", mock.Anything).Return(&models.Rescue{Active: false}, nil)
 
 			service := newTestService(host, &robotMock, bmmock.NewSSHFactory(sshMock, sshMock, sshMock), nil, helpers.GetDefaultSSHSecret(rescueSSHKeyName, "default"))
 
@@ -1282,7 +1282,7 @@ var _ = Describe("actionEnsureProvisioned", func() {
 			oldSSHMock.On("Reboot").Return(sshclient.Output{})
 
 			robotMock := robotmock.Client{}
-			robotMock.On("SetBMServerName", bareMetalHostID, infrav1.BareMetalHostNamePrefix+host.Spec.ConsumerRef.Name).Return(nil, nil)
+			robotMock.On("SetBMServerName", mock.Anything, infrav1.BareMetalHostNamePrefix+host.Spec.ConsumerRef.Name).Return(nil, nil)
 
 			service := newTestService(host, &robotMock, bmmock.NewSSHFactory(sshMock, oldSSHMock, sshMock), helpers.GetDefaultSSHSecret(osSSHKeyName, "default"), nil)
 
diff --git a/test/helpers/defaults.go b/test/helpers/defaults.go
index 3481a854..5bc1a822 100644
--- a/test/helpers/defaults.go
+++ b/test/helpers/defaults.go
@@ -22,6 +22,7 @@ import (
 	infrav1 "github.com/syself/cluster-api-provider-hetzner/api/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kuberand "k8s.io/apimachinery/pkg/util/rand"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 )
 
@@ -47,7 +48,7 @@ func BareMetalHost(name, namespace string, opts ...HostOpts) *infrav1.HetznerBar
 			Namespace: namespace,
 		},
 		Spec: infrav1.HetznerBareMetalHostSpec{
-			ServerID: bareMetalHostID,
+			ServerID: kuberand.Intn(1000),
 		},
 	}
 	for _, o := range opts {
diff --git a/test/helpers/envtest.go b/test/helpers/envtest.go
index 274db0d0..54bd853b 100644
--- a/test/helpers/envtest.go
+++ b/test/helpers/envtest.go
@@ -161,7 +161,7 @@ func NewTestEnvironment() *TestEnvironment {
 	if err := (&infrav1.HetznerBareMetalMachineTemplateWebhook{}).SetupWebhookWithManager(mgr); err != nil {
 		klog.Fatalf("failed to set up webhook with manager for HetznerBareMetalMachineTemplate: %s", err)
 	}
-	if err := (&infrav1.HetznerBareMetalHost{}).SetupWebhookWithManager(mgr); err != nil {
+	if err := (&infrav1.HetznerBareMetalHostWebhook{}).SetupWebhookWithManager(mgr); err != nil {
 		klog.Fatalf("failed to set up webhook with manager for HetznerBareMetalHost: %s", err)
 	}
 	if err := (&infrav1.HetznerBareMetalRemediation{}).SetupWebhookWithManager(mgr); err != nil {