Warn at master startup if cluster/service CIDR is mis-specified

Previously you could say things like

  clusterNetworkCIDR: "10.128.32.99/14"

and the "extra" bits in the address would just be ignored and that
would count as "10.128.0.0/14". Make it warn about this now (but still
accept it).

pkg/sdn/plugin/common.go

@@ -10,6 +10,7 @@ import (
 
 	osclient "github.com/openshift/origin/pkg/client"
 	osapi "github.com/openshift/origin/pkg/sdn/api"
+	"github.com/openshift/origin/pkg/util/netutils"
 
 	kapi "k8s.io/kubernetes/pkg/api"
 	"k8s.io/kubernetes/pkg/apis/extensions"
@@ -39,13 +40,21 @@ type NetworkInfo struct {
 }
 
 func parseNetworkInfo(clusterNetwork string, serviceNetwork string) (*NetworkInfo, error) {
-	_, cn, err := net.ParseCIDR(clusterNetwork)
+	cn, err := netutils.ParseCIDRMask(clusterNetwork)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse ClusterNetwork CIDR %s: %v", clusterNetwork, err)
+		_, cn, err := net.ParseCIDR(clusterNetwork)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse ClusterNetwork CIDR %s: %v", clusterNetwork, err)
+		}
+		glog.Errorf("Configured clusterNetworkCIDR value %q is invalid; treating it as %q", clusterNetwork, cn.String())
 	}
-	_, sn, err := net.ParseCIDR(serviceNetwork)
+	sn, err := netutils.ParseCIDRMask(serviceNetwork)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse ServiceNetwork CIDR %s: %v", serviceNetwork, err)
+		_, sn, err := net.ParseCIDR(serviceNetwork)
+		if err != nil {
+			return nil, fmt.Errorf("failed to parse ServiceNetwork CIDR %s: %v", serviceNetwork, err)
+		}
+		glog.Errorf("Configured serviceNetworkCIDR value %q is invalid; treating it as %q", serviceNetwork, sn.String())
 	}
 
 	return &NetworkInfo{

pkg/util/netutils/common.go

@@ -94,3 +94,22 @@ func GetNodeIP(nodeName string) (string, error) {
 	}
 	return ip.String(), nil
 }
+
+// ParseCIDRMask parses a CIDR string and ensures that it has no bits set beyond the
+// network mask length. Use this when the input is supposed to be either a description of
+// a subnet (eg, "192.168.1.0/24", meaning "192.168.1.0 to 192.168.1.255"), or a mask for
+// matching against (eg, "192.168.1.15/32", meaning "must match all 32 bits of the address
+// "192.168.1.15"). Use net.ParseCIDR() when the input is a host address that also
+// describes the subnet that it is on (eg, "192.168.1.15/24", meaning "the address
+// 192.168.1.15 on the network 192.168.1.0/24").
+func ParseCIDRMask(cidr string) (*net.IPNet, error) {
+	ip, net, err := net.ParseCIDR(cidr)
+	if err != nil {
+		return nil, err
+	}
+	if !ip.Equal(net.IP) {
+		maskLen, addrLen := net.Mask.Size()
+		return nil, fmt.Errorf("CIDR network specification %q is not in canonical form (should be %s/%d or %s/%d?)", cidr, ip.Mask(net.Mask).String(), maskLen, ip.String(), addrLen)
+	}
+	return net, nil
+}

pkg/util/netutils/common_test.go

@@ -2,6 +2,7 @@ package netutils
 
 import (
 	"net"
+	"strings"
 	"testing"
 )
 
@@ -40,3 +41,50 @@ func TestGenerateGateway(t *testing.T) {
 		t.Fatalf("Did not get expected gateway IP Address (gatewayIP=%s)", gatewayIP.String())
 	}
 }
+
+func TestParseCIDRMask(t *testing.T) {
+	tests := []struct {
+		cidr       string
+		fixedShort string
+		fixedLong  string
+	}{
+		{
+			cidr: "192.168.0.0/16",
+		},
+		{
+			cidr: "192.168.1.0/24",
+		},
+		{
+			cidr: "192.168.1.1/32",
+		},
+		{
+			cidr:       "192.168.1.0/16",
+			fixedShort: "192.168.0.0/16",
+			fixedLong:  "192.168.1.0/32",
+		},
+		{
+			cidr:       "192.168.1.1/24",
+			fixedShort: "192.168.1.0/24",
+			fixedLong:  "192.168.1.1/32",
+		},
+	}
+
+	for _, test := range tests {
+		_, err := ParseCIDRMask(test.cidr)
+		if test.fixedShort == "" && test.fixedLong == "" {
+			if err != nil {
+				t.Fatalf("unexpected error parsing CIDR mask %q: %v", test.cidr, err)
+			}
+		} else {
+			if err == nil {
+				t.Fatalf("unexpected lack of error parsing CIDR mask %q", test.cidr)
+			}
+			if !strings.Contains(err.Error(), test.fixedShort) {
+				t.Fatalf("error does not contain expected string %q: %v", test.fixedShort, err)
+			}
+			if !strings.Contains(err.Error(), test.fixedLong) {
+				t.Fatalf("error does not contain expected string %q: %v", test.fixedLong, err)
+			}
+		}
+	}
+}