Skip to content

Commit

Permalink
Fix nil pointer dereference when addressPrefixes is set, but addressP…
Browse files Browse the repository at this point in the history
…refix is not in a subnet
  • Loading branch information
bennerv committed Sep 7, 2023
1 parent 5afcf2e commit 478d0ef
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 13 deletions.
46 changes: 33 additions & 13 deletions pkg/validate/dynamic/dynamic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
29 changes: 29 additions & 0 deletions pkg/validate/dynamic/dynamic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package dynamic
import (
"context"
"errors"
"fmt"
"net/http"
"testing"
"time"
Expand Down Expand Up @@ -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)
})
}
}

0 comments on commit 478d0ef

Please sign in to comment.