🌱Add checks in ValidCreate and ValidUpdate in hbmmt webhook

api/v1beta1/hcloudmachine_validation.go

@@ -0,0 +1,56 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"reflect"
+
+	"k8s.io/apimachinery/pkg/util/validation/field"
+)
+
+func validateHCloudMachineSpec(oldSpec, newSpec HCloudMachineSpec) field.ErrorList {
+	var allErrs field.ErrorList
+	// Type is immutable
+	if !reflect.DeepEqual(oldSpec.Type, newSpec.Type) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "type"), newSpec.Type, "field is immutable"),
+		)
+	}
+
+	// ImageName is immutable
+	if !reflect.DeepEqual(oldSpec.ImageName, newSpec.ImageName) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "imageName"), newSpec.ImageName, "field is immutable"),
+		)
+	}
+
+	// SSHKeys is immutable
+	if !reflect.DeepEqual(oldSpec.SSHKeys, newSpec.SSHKeys) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "sshKeys"), newSpec.SSHKeys, "field is immutable"),
+		)
+	}
+
+	// Placement group name is immutable
+	if !reflect.DeepEqual(oldSpec.PlacementGroupName, newSpec.PlacementGroupName) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "placementGroupName"), newSpec.PlacementGroupName, "field is immutable"),
+		)
+	}
+
+	return allErrs
+}

api/v1beta1/hcloudmachine_validation_test.go

@@ -0,0 +1,155 @@
+/*
+Copyright 2024 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+)
+
+type args struct {
+	oldSpec HCloudMachineSpec
+	newSpec HCloudMachineSpec
+}
+
+func TestValidateHCloudMachineSpec(t *testing.T) {
+	tests := []struct {
+		name string
+		args args
+		want field.ErrorList
+	}{
+		{
+			name: "Immutable Type",
+			args: args{
+				oldSpec: HCloudMachineSpec{
+					Type: "cpx11",
+				},
+				newSpec: HCloudMachineSpec{
+					Type: "cx21",
+				},
+			},
+			want: field.ErrorList{
+				field.Invalid(field.NewPath("spec", "type"), "cx21", "field is immutable"),
+			},
+		},
+		{
+			name: "Immutable ImageName",
+			args: args{
+				oldSpec: HCloudMachineSpec{
+					ImageName: "ubuntu-20.04",
+				},
+				newSpec: HCloudMachineSpec{
+					ImageName: "centos-7",
+				},
+			},
+			want: field.ErrorList{
+				field.Invalid(field.NewPath("spec", "imageName"), "centos-7", "field is immutable"),
+			},
+		},
+		{
+			name: "Immutable SSHKeys",
+			args: args{
+				oldSpec: HCloudMachineSpec{
+					SSHKeys: []SSHKey{
+						{
+							Name:        "ssh-key-1",
+							Fingerprint: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC",
+						},
+					},
+				},
+				newSpec: HCloudMachineSpec{
+					SSHKeys: []SSHKey{
+						{
+							Name:        "ssh-key-1",
+							Fingerprint: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC",
+						},
+						{
+							Name:        "ssh-key-2",
+							Fingerprint: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC",
+						},
+					},
+				},
+			},
+			want: field.ErrorList{
+				field.Invalid(field.NewPath("spec", "sshKeys"), []SSHKey{
+					{
+						Name:        "ssh-key-1",
+						Fingerprint: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC",
+					},
+					{
+						Name:        "ssh-key-2",
+						Fingerprint: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC",
+					},
+				}, "field is immutable"),
+			},
+		},
+		{
+			name: "Immutable PlacementGroupName",
+			args: args{
+				oldSpec: HCloudMachineSpec{
+					PlacementGroupName: createPlacementGroupName("placement-group-1"),
+				},
+				newSpec: HCloudMachineSpec{
+					PlacementGroupName: createPlacementGroupName("placement-group-2"),
+				},
+			},
+			want: field.ErrorList{
+				field.Invalid(field.NewPath("spec", "placementGroupName"), "placement-group-2", "field is immutable"),
+			},
+		},
+		{
+			name: "No Errors",
+			args: args{
+				oldSpec: HCloudMachineSpec{
+					Type:               "cpx11",
+					ImageName:          "ubuntu-20.04",
+					SSHKeys:            []SSHKey{{Name: "ssh-key-1", Fingerprint: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC"}},
+					PlacementGroupName: createPlacementGroupName("placement-group-1"),
+				},
+				newSpec: HCloudMachineSpec{
+					Type:               "cpx11",
+					ImageName:          "ubuntu-20.04",
+					SSHKeys:            []SSHKey{{Name: "ssh-key-1", Fingerprint: "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQC"}},
+					PlacementGroupName: createPlacementGroupName("placement-group-1"),
+				},
+			},
+			want: field.ErrorList{},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := validateHCloudMachineSpec(tt.args.oldSpec, tt.args.newSpec)
+
+			if len(got) != len(tt.want) {
+				t.Errorf("validateHCloudMachineSpec() = %v, want %v", got, tt.want)
+				return
+			}
+
+			for i, err := range got {
+				assert.Equal(t, tt.want[i].Type, err.Type)
+				assert.Equal(t, tt.want[i].Field, err.Field)
+				assert.Equal(t, tt.want[i].Detail, err.Detail)
+			}
+		})
+	}
+}
+
+func createPlacementGroupName(name string) *string {
+	return &name
+}

api/v1beta1/hcloudmachine_webhook.go

@@ -18,7 +18,6 @@ package v1beta1
 
 import (
 	"fmt"
-	"reflect"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -82,35 +81,7 @@ func (r *HCloudMachine) ValidateUpdate(old runtime.Object) (admission.Warnings,
 		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected an HCloudMachine but got a %T", old))
 	}
 
-	var allErrs field.ErrorList
-
-	// Type is immutable
-	if !reflect.DeepEqual(oldM.Spec.Type, r.Spec.Type) {
-		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "type"), r.Spec.Type, "field is immutable"),
-		)
-	}
-
-	// ImageName is immutable
-	if !reflect.DeepEqual(oldM.Spec.ImageName, r.Spec.ImageName) {
-		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "imageName"), r.Spec.ImageName, "field is immutable"),
-		)
-	}
-
-	// SSHKeys is immutable
-	if !reflect.DeepEqual(oldM.Spec.SSHKeys, r.Spec.SSHKeys) {
-		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "sshKeys"), r.Spec.SSHKeys, "field is immutable"),
-		)
-	}
-
-	// Placement group name is immutable
-	if !reflect.DeepEqual(oldM.Spec.PlacementGroupName, r.Spec.PlacementGroupName) {
-		allErrs = append(allErrs,
-			field.Invalid(field.NewPath("spec", "placementGroupName"), r.Spec.PlacementGroupName, "field is immutable"),
-		)
-	}
+	allErrs := validateHCloudMachineSpec(oldM.Spec, r.Spec)
 
 	return nil, aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs)
 }

api/v1beta1/hcloudmachinetemplate_webhook.go

@@ -67,7 +67,7 @@ func (r *HCloudMachineTemplateWebhook) ValidateUpdate(ctx context.Context, oldRa
 		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a admission.Request inside context: %v", err))
 	}
 
-	var allErrs field.ErrorList
+	allErrs := validateHCloudMachineSpec(oldHCloudMachineTemplate.Spec.Template.Spec, newHCloudMachineTemplate.Spec.Template.Spec)
 
 	if !topology.ShouldSkipImmutabilityChecks(req, newHCloudMachineTemplate) && !reflect.DeepEqual(newHCloudMachineTemplate.Spec, oldHCloudMachineTemplate.Spec) {
 		allErrs = append(allErrs, field.Invalid(field.NewPath("spec"), newHCloudMachineTemplate, "HCloudMachineTemplate.Spec is immutable"))

api/v1beta1/hetznerbaremetalmachine_validation.go

@@ -0,0 +1,94 @@
+/*
+Copyright 2021 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1beta1
+
+import (
+	"fmt"
+	"reflect"
+	"strings"
+
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/apimachinery/pkg/selection"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+)
+
+func validateHetznerBareMetalMachineSpecCreate(spec HetznerBareMetalMachineSpec) field.ErrorList {
+	var allErrs field.ErrorList
+
+	if spec.SSHSpec.PortAfterCloudInit == 0 {
+		spec.SSHSpec.PortAfterCloudInit = spec.SSHSpec.PortAfterInstallImage
+	}
+
+	if (spec.InstallImage.Image.Name == "" || spec.InstallImage.Image.URL == "") &&
+		spec.InstallImage.Image.Path == "" {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "installImage", "image"), spec.InstallImage.Image,
+				"have to specify either image name and url or path"),
+		)
+	}
+
+	if spec.InstallImage.Image.URL != "" {
+		if _, err := GetImageSuffix(spec.InstallImage.Image.URL); err != nil {
+			allErrs = append(allErrs,
+				field.Invalid(field.NewPath("spec", "installImage", "image", "url"), spec.InstallImage.Image.URL,
+					"unknown image type in URL"),
+			)
+		}
+	}
+
+	// validate host selector
+	for labelKey, labelVal := range spec.HostSelector.MatchLabels {
+		if _, err := labels.NewRequirement(labelKey, selection.Equals, []string{labelVal}); err != nil {
+			allErrs = append(allErrs, field.Invalid(
+				field.NewPath("spec", "hostSelector", "matchLabels"), spec.HostSelector.MatchLabels,
+				fmt.Sprintf("invalid match label: %s", err.Error()),
+			))
+		}
+	}
+	for _, req := range spec.HostSelector.MatchExpressions {
+		lowercaseOperator := selection.Operator(strings.ToLower(string(req.Operator)))
+		if _, err := labels.NewRequirement(req.Key, lowercaseOperator, req.Values); err != nil {
+			allErrs = append(allErrs, field.Invalid(
+				field.NewPath("spec", "hostSelector", "matchExpressions"), spec.HostSelector.MatchExpressions,
+				fmt.Sprintf("invalid match expression: %s", err.Error()),
+			))
+		}
+	}
+
+	return allErrs
+}
+
+func validateHetznerBareMetalMachineSpecUpdate(oldSpec, newSpec HetznerBareMetalMachineSpec) field.ErrorList {
+	var allErrs field.ErrorList
+	if !reflect.DeepEqual(newSpec.InstallImage, oldSpec.InstallImage) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "installImage"), newSpec.InstallImage, "installImage immutable"),
+		)
+	}
+	if !reflect.DeepEqual(newSpec.SSHSpec, oldSpec.SSHSpec) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "sshSpec"), newSpec.SSHSpec, "sshSpec immutable"),
+		)
+	}
+	if !reflect.DeepEqual(newSpec.HostSelector, oldSpec.HostSelector) {
+		allErrs = append(allErrs,
+			field.Invalid(field.NewPath("spec", "hostSelector"), newSpec.HostSelector, "hostSelector immutable"),
+		)
+	}
+
+	return allErrs
+}