diff --git a/pkg/controller/ipam/validate.go b/pkg/controller/ipam/validate.go index abdb04296c4..83b0405bd0e 100644 --- a/pkg/controller/ipam/validate.go +++ b/pkg/controller/ipam/validate.go @@ -195,6 +195,17 @@ func ipVersion(ip net.IP) int { } func validateIPRange(r crdv1alpha2.SubnetIPRange, poolIPVersion int) (bool, string) { + if poolIPVersion == 4 { + if r.PrefixLength <= 0 || r.PrefixLength >= 32 { + return false, fmt.Sprintf("Invalid prefix length %d", r.PrefixLength) + } + } else if poolIPVersion == 6 { + if r.PrefixLength <= 0 || r.PrefixLength >= 128 { + return false, fmt.Sprintf("Invalid prefix length %d", r.PrefixLength) + } + } else { + return false, fmt.Sprintf("Invalid IP version %d", poolIPVersion) + } // Validate the integrity the IP range: // Verify that all the IP ranges have the same IP family as the IP pool // Verify that the gateway IP is reachable from the IP range @@ -209,28 +220,27 @@ func validateIPRange(r crdv1alpha2.SubnetIPRange, poolIPVersion int) (bool, stri if r.CIDR != "" { _, cidr, _ := net.ParseCIDR(r.CIDR) - + if ipVersion(cidr.IP) != poolIPVersion { + return false, fmt.Sprintf( + "Range is invalid. IP version of range %s differs from Pool IP version", r.CIDR) + } if !netCIDR.Contains(cidr.IP) { return false, fmt.Sprintf( "Range is invalid. CIDR %s is not contained within subnet %s/%d", r.CIDR, netCIDR.IP.String(), r.PrefixLength) } - if ipVersion(cidr.IP) != poolIPVersion { - return false, fmt.Sprintf( - "Range is invalid. IP version of range %s differs from Pool IP version", r.CIDR) - } } else { rStart := net.ParseIP(r.Start) rEnd := net.ParseIP(r.End) + if ipVersion(rStart) != poolIPVersion || ipVersion(rEnd) != poolIPVersion { + return false, fmt.Sprintf( + "Range is invalid. IP version of range %s-%s differs from Pool IP version", r.Start, r.End) + } if !netCIDR.Contains(rStart) || !netCIDR.Contains(rEnd) { return false, fmt.Sprintf( "Range is invalid. range %s-%s is not contained within subnet %s/%d", r.Start, r.End, netCIDR.IP.String(), r.PrefixLength) } - if ipVersion(rStart) != poolIPVersion || ipVersion(rEnd) != poolIPVersion { - return false, fmt.Sprintf( - "Range is invalid. IP version of range %s-%s differs from Pool IP version", r.Start, r.End) - } } return true, "" } diff --git a/pkg/controller/ipam/validate_test.go b/pkg/controller/ipam/validate_test.go index 8b86d477912..260e317cd6f 100644 --- a/pkg/controller/ipam/validate_test.go +++ b/pkg/controller/ipam/validate_test.go @@ -92,6 +92,30 @@ func TestEgressControllerValidateExternalIPPool(t *testing.T) { }, expectedResponse: &admv1.AdmissionResponse{Allowed: true}, }, + { + name: "CREATE operation with invalid prefix length should not be allowed", + request: &admv1.AdmissionRequest{ + Name: "foo", + Operation: "CREATE", + Object: runtime.RawExtension{Raw: marshal(copyAndMutateIPPool(testIPPool, func(pool *crdv1alpha2.IPPool) { + pool.Spec.IPRanges = append(pool.Spec.IPRanges, crdv1alpha2.SubnetIPRange{ + IPRange: crdv1alpha2.IPRange{ + CIDR: "192.168.3.0/26", + }, + SubnetInfo: crdv1alpha2.SubnetInfo{ + Gateway: "192.168.3.1", + PrefixLength: 32, + }, + }) + }))}, + }, + expectedResponse: &admv1.AdmissionResponse{ + Allowed: false, + Result: &metav1.Status{ + Message: "Invalid prefix length 32", + }, + }, + }, { name: "CREATE operation with CIDR partially overlap with IP range should not be allowed", request: &admv1.AdmissionRequest{ @@ -152,7 +176,7 @@ func TestEgressControllerValidateExternalIPPool(t *testing.T) { }, SubnetInfo: crdv1alpha2.SubnetInfo{ Gateway: "10:2400::01", - PrefixLength: 64, + PrefixLength: 24, }, }) }))},