Skip to content

Commit

Permalink
add more generic validation funcs
Browse files Browse the repository at this point in the history
  • Loading branch information
@jackfrancis
jackfrancis committed Oct 17, 2022
1 parent f9a6449 commit 5e9873a
Show file tree
Hide file tree
Showing 10 changed files with 556 additions and 276 deletions.
2 changes: 2 additions & 0 deletions .golangci.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ linters-settings:
alias: infrav1alpha4exp
- pkg: sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1
alias: infrav1exp
- pkg: sigs.k8s.io/cluster-api-provider-azure/util/webhook
alias: webhookutils
gocritic:
enabled-tags:
- "experimental"
Expand Down
31 changes: 16 additions & 15 deletions api/v1beta1/azurecluster_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
webhookutils "sigs.k8s.io/cluster-api-provider-azure/util/webhook"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)
Expand Down Expand Up @@ -54,25 +55,25 @@ func (c *AzureCluster) ValidateUpdate(oldRaw runtime.Object) error {
var allErrs field.ErrorList
old := oldRaw.(*AzureCluster)

if !reflect.DeepEqual(c.Spec.ResourceGroup, old.Spec.ResourceGroup) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "ResourceGroup"),
c.Spec.ResourceGroup, "field is immutable"),
)
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "ResourceGroup"),
old.Spec.ResourceGroup,
c.Spec.ResourceGroup); err != nil {
allErrs = append(allErrs, err)
}

if !reflect.DeepEqual(c.Spec.SubscriptionID, old.Spec.SubscriptionID) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "SubscriptionID"),
c.Spec.SubscriptionID, "field is immutable"),
)
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "SubscriptionID"),
old.Spec.SubscriptionID,
c.Spec.SubscriptionID); err != nil {
allErrs = append(allErrs, err)
}

if !reflect.DeepEqual(c.Spec.Location, old.Spec.Location) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "Location"),
c.Spec.Location, "field is immutable"),
)
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "Location"),
old.Spec.Location,
c.Spec.Location); err != nil {
allErrs = append(allErrs, err)
}

if old.Spec.ControlPlaneEndpoint.Host != "" && c.Spec.ControlPlaneEndpoint.Host != old.Spec.ControlPlaneEndpoint.Host {
Expand Down
31 changes: 16 additions & 15 deletions api/v1beta1/azuremachine_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
webhookutils "sigs.k8s.io/cluster-api-provider-azure/util/webhook"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)
Expand Down Expand Up @@ -81,11 +82,11 @@ func (m *AzureMachine) ValidateUpdate(oldRaw runtime.Object) error {
)
}

if !reflect.DeepEqual(m.Spec.RoleAssignmentName, old.Spec.RoleAssignmentName) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "roleAssignmentName"),
m.Spec.RoleAssignmentName, "field is immutable"),
)
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "RoleAssignmentName"),
old.Spec.RoleAssignmentName,
m.Spec.RoleAssignmentName); err != nil {
allErrs = append(allErrs, err)
}

if !reflect.DeepEqual(m.Spec.OSDisk, old.Spec.OSDisk) {
Expand All @@ -102,11 +103,11 @@ func (m *AzureMachine) ValidateUpdate(oldRaw runtime.Object) error {
)
}

if !reflect.DeepEqual(m.Spec.SSHPublicKey, old.Spec.SSHPublicKey) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "sshPublicKey"),
m.Spec.SSHPublicKey, "field is immutable"),
)
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "SSHPublicKey"),
old.Spec.SSHPublicKey,
m.Spec.SSHPublicKey); err != nil {
allErrs = append(allErrs, err)
}

if !reflect.DeepEqual(m.Spec.AllocatePublicIP, old.Spec.AllocatePublicIP) {
Expand All @@ -123,11 +124,11 @@ func (m *AzureMachine) ValidateUpdate(oldRaw runtime.Object) error {
)
}

if !reflect.DeepEqual(m.Spec.AcceleratedNetworking, old.Spec.AcceleratedNetworking) {
allErrs = append(allErrs,
field.Invalid(field.NewPath("spec", "acceleratedNetworking"),
m.Spec.AcceleratedNetworking, "field is immutable"),
)
if err := webhookutils.ValidateBoolPtrImmutable(
field.NewPath("Spec", "AcceleratedNetworking"),
old.Spec.AcceleratedNetworking,
m.Spec.AcceleratedNetworking); err != nil {
allErrs = append(allErrs, err)
}

if !reflect.DeepEqual(m.Spec.SpotVMOptions, old.Spec.SpotVMOptions) {
Expand Down
158 changes: 51 additions & 107 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/validation/field"
infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
webhookutils "sigs.k8s.io/cluster-api-provider-azure/util/webhook"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -88,131 +89,74 @@ func (m *AzureManagedControlPlane) ValidateUpdate(oldRaw runtime.Object, client
var allErrs field.ErrorList
old := oldRaw.(*AzureManagedControlPlane)

if m.Name != old.Name {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Name"),
m.Name,
"field is immutable"))
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Name"),
old.Name,
m.Name); err != nil {
allErrs = append(allErrs, err)
}

if m.Spec.SubscriptionID != old.Spec.SubscriptionID {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "SubscriptionID"),
m.Spec.SubscriptionID,
"field is immutable"))
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "SubscriptionID"),
old.Spec.SubscriptionID,
m.Spec.SubscriptionID); err != nil {
allErrs = append(allErrs, err)
}

if m.Spec.ResourceGroupName != old.Spec.ResourceGroupName {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "ResourceGroupName"),
m.Spec.ResourceGroupName,
"field is immutable"))
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "ResourceGroupName"),
old.Spec.ResourceGroupName,
m.Spec.ResourceGroupName); err != nil {
allErrs = append(allErrs, err)
}

if m.Spec.NodeResourceGroupName != old.Spec.NodeResourceGroupName {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "NodeResourceGroupName"),
m.Spec.NodeResourceGroupName,
"field is immutable"))
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "NodeResourceGroupName"),
old.Spec.NodeResourceGroupName,
m.Spec.NodeResourceGroupName); err != nil {
allErrs = append(allErrs, err)
}

if m.Spec.Location != old.Spec.Location {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "Location"),
m.Spec.Location,
"field is immutable"))
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "Location"),
old.Spec.Location,
m.Spec.Location); err != nil {
allErrs = append(allErrs, err)
}

if old.Spec.SSHPublicKey != "" {
// Prevent SSH key modification if it was already set to some value
if m.Spec.SSHPublicKey != old.Spec.SSHPublicKey {
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "SSHPublicKey"),
m.Spec.SSHPublicKey,
"field is immutable"))
}
if err := webhookutils.ValidateStringImmutable(
field.NewPath("Spec", "SSHPublicKey"),
old.Spec.SSHPublicKey,
m.Spec.SSHPublicKey); err != nil {
allErrs = append(allErrs, err)
}

if old.Spec.DNSServiceIP != nil {
// Prevent DNSServiceIP modification if it was already set to some value
if m.Spec.DNSServiceIP == nil {
// unsetting the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "DNSServiceIP"),
m.Spec.DNSServiceIP,
"field is immutable, unsetting is not allowed"))
} else if *m.Spec.DNSServiceIP != *old.Spec.DNSServiceIP {
// changing the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "DNSServiceIP"),
*m.Spec.DNSServiceIP,
"field is immutable"))
}
if err := webhookutils.ValidateStringPtrImmutable(
field.NewPath("Spec", "DNSServiceIP"),
old.Spec.DNSServiceIP,
m.Spec.DNSServiceIP); err != nil {
allErrs = append(allErrs, err)
}

if old.Spec.NetworkPlugin != nil {
// Prevent NetworkPlugin modification if it was already set to some value
if m.Spec.NetworkPlugin == nil {
// unsetting the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "NetworkPlugin"),
m.Spec.NetworkPlugin,
"field is immutable, unsetting is not allowed"))
} else if *m.Spec.NetworkPlugin != *old.Spec.NetworkPlugin {
// changing the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "NetworkPlugin"),
*m.Spec.NetworkPlugin,
"field is immutable"))
}
if err := webhookutils.ValidateStringPtrImmutable(
field.NewPath("Spec", "NetworkPlugin"),
old.Spec.NetworkPlugin,
m.Spec.NetworkPlugin); err != nil {
allErrs = append(allErrs, err)
}

if old.Spec.NetworkPolicy != nil {
// Prevent NetworkPolicy modification if it was already set to some value
if m.Spec.NetworkPolicy == nil {
// unsetting the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "NetworkPolicy"),
m.Spec.NetworkPolicy,
"field is immutable, unsetting is not allowed"))
} else if *m.Spec.NetworkPolicy != *old.Spec.NetworkPolicy {
// changing the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "NetworkPolicy"),
*m.Spec.NetworkPolicy,
"field is immutable"))
}
if err := webhookutils.ValidateStringPtrImmutable(
field.NewPath("Spec", "NetworkPolicy"),
old.Spec.NetworkPolicy,
m.Spec.NetworkPolicy); err != nil {
allErrs = append(allErrs, err)
}

if old.Spec.LoadBalancerSKU != nil {
// Prevent LoadBalancerSKU modification if it was already set to some value
if m.Spec.LoadBalancerSKU == nil {
// unsetting the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "LoadBalancerSKU"),
m.Spec.LoadBalancerSKU,
"field is immutable, unsetting is not allowed"))
} else if *m.Spec.LoadBalancerSKU != *old.Spec.LoadBalancerSKU {
// changing the field is not allowed
allErrs = append(allErrs,
field.Invalid(
field.NewPath("Spec", "LoadBalancerSKU"),
*m.Spec.LoadBalancerSKU,
"field is immutable"))
}
if err := webhookutils.ValidateStringPtrImmutable(
field.NewPath("Spec", "LoadBalancerSKU"),
old.Spec.LoadBalancerSKU,
m.Spec.LoadBalancerSKU); err != nil {
allErrs = append(allErrs, err)
}

if old.Spec.AADProfile != nil {
Expand Down
12 changes: 0 additions & 12 deletions exp/api/v1beta1/azuremanagedcontrolplane_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,18 +353,6 @@ func TestAzureManagedControlPlane_ValidateUpdate(t *testing.T) {
amcp *AzureManagedControlPlane
wantErr bool
}{
{
name: "AzureManagedControlPlane with valid SSHPublicKey",
oldAMCP: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", ""),
amcp: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", generateSSHPublicKey(true)),
wantErr: false,
},
{
name: "AzureManagedControlPlane with invalid SSHPublicKey",
oldAMCP: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", ""),
amcp: createAzureManagedControlPlane("192.168.0.0", "v1.18.0", generateSSHPublicKey(false)),
wantErr: true,
},
{
name: "AzureManagedControlPlane with invalid serviceIP",
oldAMCP: createAzureManagedControlPlane("", "v1.18.0", ""),
Expand Down
Loading

0 comments on commit 5e9873a

Please sign in to comment.