Skip to content

Commit f98ead5

Browse files
danwinshipdanwinship
committed
Require network API objects to have IPv4 addresses
1 parent f0b6abd commit f98ead5

File tree

2 files changed

+100
-10
lines changed

2 files changed

+100
-10
lines changed

‎pkg/network/apis/network/validation/validation.go

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,28 @@ import (
1616
"github.com/openshift/origin/pkg/util/netutils"
1717
)
1818

19+
func validateCIDRv4(cidr string) (*net.IPNet, error) {
20+
ipnet, err := netutils.ParseCIDRMask(cidr)
21+
if err != nil {
22+
return nil, err
23+
}
24+
if ipnet.IP.To4() == nil {
25+
return nil, fmt.Errorf("must be an IPv4 network")
26+
}
27+
return ipnet, nil
28+
}
29+
30+
func validateIPv4(ip string) (net.IP, error) {
31+
bytes := net.ParseIP(ip)
32+
if bytes == nil {
33+
return nil, fmt.Errorf("invalid IP address")
34+
}
35+
if bytes.To4() == nil {
36+
return nil, fmt.Errorf("must be an IPv4 address")
37+
}
38+
return bytes, nil
39+
}
40+
1941
var defaultClusterNetwork *networkapi.ClusterNetwork
2042

2143
// SetDefaultClusterNetwork sets the expected value of the default ClusterNetwork record
@@ -30,7 +52,7 @@ func ValidateClusterNetwork(clusterNet *networkapi.ClusterNetwork) field.ErrorLi
3052

3153
if len(clusterNet.Network) != 0 || clusterNet.HostSubnetLength != 0 {
3254
//In the case that a user manually makes a clusterNetwork object with clusterNet.Network and clusterNet.HostubnetLength at least make sure they are valid values
33-
clusterIPNet, err := netutils.ParseCIDRMask(clusterNet.Network)
55+
clusterIPNet, err := validateCIDRv4(clusterNet.Network)
3456
if err != nil {
3557
allErrs = append(allErrs, field.Invalid(field.NewPath("network"), clusterNet.Network, err.Error()))
3658
} else {
@@ -46,12 +68,12 @@ func ValidateClusterNetwork(clusterNet *networkapi.ClusterNetwork) field.ErrorLi
4668
if len(clusterNet.ClusterNetworks) == 0 && len(clusterNet.Network) == 0 {
4769
allErrs = append(allErrs, field.Invalid(field.NewPath("clusterNetworks"), clusterNet.ClusterNetworks, "must have at least one cluster network CIDR"))
4870
}
49-
serviceIPNet, err := netutils.ParseCIDRMask(clusterNet.ServiceNetwork)
71+
serviceIPNet, err := validateCIDRv4(clusterNet.ServiceNetwork)
5072
if err != nil {
5173
allErrs = append(allErrs, field.Invalid(field.NewPath("serviceNetwork"), clusterNet.ServiceNetwork, err.Error()))
5274
}
5375
for i, cn := range clusterNet.ClusterNetworks {
54-
clusterIPNet, err := netutils.ParseCIDRMask(cn.CIDR)
76+
clusterIPNet, err := validateCIDRv4(cn.CIDR)
5577
if err != nil {
5678
allErrs = append(allErrs, field.Invalid(field.NewPath("clusterNetworks").Index(i).Child("cidr"), cn.CIDR, err.Error()))
5779
continue
@@ -115,18 +137,19 @@ func ValidateHostSubnet(hs *networkapi.HostSubnet) field.ErrorList {
115137
allErrs = append(allErrs, field.Invalid(field.NewPath("subnet"), hs.Subnet, "field cannot be empty"))
116138
}
117139
} else {
118-
_, err := netutils.ParseCIDRMask(hs.Subnet)
140+
_, err := validateCIDRv4(hs.Subnet)
119141
if err != nil {
120142
allErrs = append(allErrs, field.Invalid(field.NewPath("subnet"), hs.Subnet, err.Error()))
121143
}
122144
}
145+
// In theory this has to be IPv4, but it's possible some clusters might be limping along with IPv6 values?
123146
if net.ParseIP(hs.HostIP) == nil {
124147
allErrs = append(allErrs, field.Invalid(field.NewPath("hostIP"), hs.HostIP, "invalid IP address"))
125148
}
126149

127150
for i, egressIP := range hs.EgressIPs {
128-
if net.ParseIP(egressIP) == nil {
129-
allErrs = append(allErrs, field.Invalid(field.NewPath("egressIPs").Index(i), egressIP, "invalid IP address"))
151+
if _, err := validateIPv4(egressIP); err != nil {
152+
allErrs = append(allErrs, field.Invalid(field.NewPath("egressIPs").Index(i), egressIP, err.Error()))
130153
}
131154
}
132155

@@ -157,8 +180,8 @@ func ValidateNetNamespace(netnamespace *networkapi.NetNamespace) field.ErrorList
157180
}
158181

159182
for i, ip := range netnamespace.EgressIPs {
160-
if net.ParseIP(ip) == nil {
161-
allErrs = append(allErrs, field.Invalid(field.NewPath("egressIPs").Index(i), ip, "invalid IP address"))
183+
if _, err := validateIPv4(ip); err != nil {
184+
allErrs = append(allErrs, field.Invalid(field.NewPath("egressIPs").Index(i), ip, err.Error()))
162185
}
163186
}
164187

‎pkg/network/apis/network/validation/validation_test.go

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,24 @@ func TestValidateClusterNetwork(t *testing.T) {
154154
},
155155
expectedErrors: 1,
156156
},
157+
{
158+
name: "IPv6 ClusterNetwork",
159+
cn: &networkapi.ClusterNetwork{
160+
ObjectMeta: metav1.ObjectMeta{Name: "any"},
161+
ClusterNetworks: []networkapi.ClusterNetworkEntry{{CIDR: "fe80:1234::/64", HostSubnetLength: 8}},
162+
ServiceNetwork: "172.30.0.0/16",
163+
},
164+
expectedErrors: 1,
165+
},
166+
{
167+
name: "IPv6 ServiceNetwork",
168+
cn: &networkapi.ClusterNetwork{
169+
ObjectMeta: metav1.ObjectMeta{Name: "any"},
170+
ClusterNetworks: []networkapi.ClusterNetworkEntry{{CIDR: "10.20.0.0/16", HostSubnetLength: 8}},
171+
ServiceNetwork: "fe80:1234::/64",
172+
},
173+
expectedErrors: 1,
174+
},
157175
}
158176

159177
for _, tc := range tests {
@@ -331,6 +349,34 @@ func TestValidateHostSubnet(t *testing.T) {
331349
},
332350
expectedErrors: 2,
333351
},
352+
{
353+
name: "IPv6 subnet",
354+
hs: &networkapi.HostSubnet{
355+
ObjectMeta: metav1.ObjectMeta{
356+
Name: "abc.def.com",
357+
},
358+
Host: "abc.def.com",
359+
HostIP: "10.20.30.40",
360+
Subnet: "fe80:1234::/64",
361+
},
362+
expectedErrors: 1,
363+
},
364+
{
365+
name: "IPv6 EgressIP",
366+
hs: &networkapi.HostSubnet{
367+
ObjectMeta: metav1.ObjectMeta{
368+
Name: "abc.def.com",
369+
},
370+
Host: "abc.def.com",
371+
HostIP: "10.20.30.40",
372+
Subnet: "8.8.8.0/24",
373+
EgressIPs: []string{
374+
"192.168.1.99",
375+
"1234::5678",
376+
},
377+
},
378+
expectedErrors: 1,
379+
},
334380
}
335381

336382
for _, tc := range tests {
@@ -412,9 +458,9 @@ func TestValidateNetNamespace(t *testing.T) {
412458
},
413459
NetName: "abc",
414460
NetID: 12345,
415-
EgressIPs: []string{"example.com", ""},
461+
EgressIPs: []string{"example.com", "", "1234::5678"},
416462
},
417-
expectedErrors: 2,
463+
expectedErrors: 3,
418464
},
419465
}
420466

@@ -615,6 +661,27 @@ func TestValidateEgressNetworkPolicy(t *testing.T) {
615661
},
616662
expectedErrors: 1,
617663
},
664+
{
665+
// IPv6 CIDRSelectors are currently useless, but they don't break anything
666+
name: "IPv6 CIDR",
667+
fw: &networkapi.EgressNetworkPolicy{
668+
ObjectMeta: metav1.ObjectMeta{
669+
Name: "default",
670+
Namespace: "testing",
671+
},
672+
Spec: networkapi.EgressNetworkPolicySpec{
673+
Egress: []networkapi.EgressNetworkPolicyRule{
674+
{
675+
Type: networkapi.EgressNetworkPolicyRuleAllow,
676+
To: networkapi.EgressNetworkPolicyPeer{
677+
CIDRSelector: "fe80::/64",
678+
},
679+
},
680+
},
681+
},
682+
},
683+
expectedErrors: 0,
684+
},
618685
}
619686

620687
for _, tc := range tests {

0 commit comments

Comments
 (0)