From 8afd0b2a7fd66f91433b6b8fb40b22fe7bf1c575 Mon Sep 17 00:00:00 2001 From: bennerv <10840174+bennerv@users.noreply.github.com> Date: Wed, 6 Sep 2023 18:31:15 -0400 Subject: [PATCH] Fix nil pointer dereference when addressPrefixes is set, but addressPrefix is not in a subnet --- pkg/validate/dynamic/dynamic.go | 46 ++++++++++++++++++++-------- pkg/validate/dynamic/dynamic_test.go | 29 ++++++++++++++++++ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/pkg/validate/dynamic/dynamic.go b/pkg/validate/dynamic/dynamic.go index 5e8c3e72ed6..4b1df4bccca 100644 --- a/pkg/validate/dynamic/dynamic.go +++ b/pkg/validate/dynamic/dynamic.go @@ -51,6 +51,8 @@ var ( errMsgInvalidVNetLocation = "The vnet location '%s' must match the cluster location '%s'." ) +const minimumSubnetMaskSize int = 27 + type Subnet struct { // ID is a resource id of the subnet ID string @@ -764,23 +766,41 @@ func (dv *dynamic) ValidateSubnets(ctx context.Context, oc *api.OpenShiftCluster ) } - _, net, err := net.ParseCIDR(*ss.AddressPrefix) - if err != nil { - return err + // Handle both addressPrefix & addressPrefixes + if ss.AddressPrefix == nil { + for _, address := range *ss.AddressPrefixes { + if err = validateSubnetSize(s, address); err != nil { + return err + } + } + } else { + if err = validateSubnetSize(s, *ss.AddressPrefix); err != nil { + return err + } } + } - ones, _ := net.Mask.Size() - if ones > 27 { - return api.NewCloudError( - http.StatusBadRequest, - api.CloudErrorCodeInvalidLinkedVNet, - s.Path, - errMsgSubnetInvalidSize, - s.ID, - ) - } + return nil +} + +// validateSubnetSize checks if the subnet mask is >27, and returns +// an error if so as it is too small for OCP +func validateSubnetSize(s Subnet, address string) error { + _, net, err := net.ParseCIDR(address) + if err != nil { + return err } + ones, _ := net.Mask.Size() + if ones > minimumSubnetMaskSize { + return api.NewCloudError( + http.StatusBadRequest, + api.CloudErrorCodeInvalidLinkedVNet, + s.Path, + errMsgSubnetInvalidSize, + s.ID, + ) + } return nil } diff --git a/pkg/validate/dynamic/dynamic_test.go b/pkg/validate/dynamic/dynamic_test.go index 1e69c6c86a1..d24190385a9 100644 --- a/pkg/validate/dynamic/dynamic_test.go +++ b/pkg/validate/dynamic/dynamic_test.go @@ -6,6 +6,7 @@ package dynamic import ( "context" "errors" + "fmt" "net/http" "testing" "time" @@ -1727,3 +1728,31 @@ func TestCheckBYONsg(t *testing.T) { }) } } + +func TestValidateSubnetSize(t *testing.T) { + subnetId := "id" + subnetPath := "path" + for _, tt := range []struct { + name string + address string + subnet Subnet + wantErr string + }{ + { + name: "subnet size is too small", + address: "10.0.0.0/32", + subnet: Subnet{ID: subnetId, Path: subnetPath}, + wantErr: fmt.Sprintf("400: InvalidLinkedVNet: %s: The provided subnet '%s' is invalid: must be /27 or larger.", subnetPath, subnetId), + }, + { + name: "subnet size is gucci gang", + address: "10.0.0.0/27", + subnet: Subnet{ID: subnetId, Path: subnetPath}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + err := validateSubnetSize(tt.subnet, tt.address) + utilerror.AssertErrorMessage(t, err, tt.wantErr) + }) + } +}