Skip to content

Commit

Permalink
🌱Add checks in ValidCreate and ValidUpdate in hbmmt webhook (#1319)
Browse files Browse the repository at this point in the history 
🌱Add validation checks and tests for hbmmt and hcloudmachine webhook

Added validation checks for hbmm , hbmmt , hcloudmachine and hcloud
machinetemplate webhooks . Also added the unit tests for the same
  • Loading branch information
@yrs147
yrs147 authored Jul 17, 2024
1 parent abdc74f commit 99ac110
Show file tree
Hide file tree
Showing 12 changed files with 1,034 additions and 112 deletions.
56 changes: 56 additions & 0 deletions api/v1beta1/hcloudmachine_validation.go
View file
Original file line number Diff line number Diff line change
@@ -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
}
151 changes: 151 additions & 0 deletions api/v1beta1/hcloudmachine_validation_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
/*
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.Error
}{
{
name: "Immutable Type",
args: args{
oldSpec: HCloudMachineSpec{
Type: "cpx11",
},
newSpec: HCloudMachineSpec{
Type: "cx21",
},
},
want: 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.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.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.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"}},
PlacementGroupName: createPlacementGroupName("placement-group-1"),
},
newSpec: HCloudMachineSpec{
Type: "cpx11",
ImageName: "ubuntu-20.04",
SSHKeys: []SSHKey{{Name: "ssh-key-1"}},
PlacementGroupName: createPlacementGroupName("placement-group-1"),
},
},
want: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := validateHCloudMachineSpec(tt.args.oldSpec, tt.args.newSpec)

if len(got) == 0 {
assert.Empty(t, got)
}

if len(got) > 1 {
t.Errorf("got length: %d greater than 1", len(got))
}

// assert if length of got is 1
if len(got) == 1 {
assert.Equal(t, tt.want.Type, got[0].Type)
assert.Equal(t, tt.want.Field, got[0].Field)
assert.Equal(t, tt.want.Detail, got[0].Detail)
}
})
}
}

func createPlacementGroupName(name string) *string {
return &name
}
31 changes: 1 addition & 30 deletions api/v1beta1/hcloudmachine_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package v1beta1

import (
"fmt"
"reflect"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions api/v1beta1/hcloudmachinetemplate_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ func (r *HCloudMachineTemplateWebhook) ValidateUpdate(ctx context.Context, oldRa
if err != nil {
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a admission.Request inside context: %v", err))
}

var allErrs field.ErrorList

if !topology.ShouldSkipImmutabilityChecks(req, newHCloudMachineTemplate) && !reflect.DeepEqual(newHCloudMachineTemplate.Spec, oldHCloudMachineTemplate.Spec) {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec"), newHCloudMachineTemplate, "HCloudMachineTemplate.Spec is immutable"))
}

allErrs = append(allErrs, validateHCloudMachineSpec(oldHCloudMachineTemplate.Spec.Template.Spec, newHCloudMachineTemplate.Spec.Template.Spec)...)

return nil, aggregateObjErrors(newHCloudMachineTemplate.GroupVersionKind().GroupKind(), newHCloudMachineTemplate.Name, allErrs)
}

Expand Down
90 changes: 90 additions & 0 deletions api/v1beta1/hetznerbaremetalmachine_validation.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
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.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
}
Loading

0 comments on commit 99ac110

Please sign in to comment.