validate: simplify CIDR validation

Much of the code that made of the CIDR validation was there to give a custom
description of what made the CIDR provided invalid. This has been removed in
favor of using the error returned by the golang library's ParseCIDR function.

Additionally, the code to determine whether two CIDRs overlap was unnecessary
complex because it did not take advantage of the fact that CIDRs only overlap
if one is a subset of the other.

https://jira.coreos.com/browse/CORS-850

Author: staebler

Gopkg.lock

@@ -1,7 +1,6 @@
 package machine
 
 import (
-	"net"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -24,12 +23,7 @@ func TestMasterGenerate(t *testing.T) {
 			},
 			BaseDomain: "test-domain",
 			Networking: types.Networking{
-				ServiceCIDR: ipnet.IPNet{
-					IPNet: func(s string) net.IPNet {
-						_, cidr, _ := net.ParseCIDR(s)
-						return *cidr
-					}("10.0.1.0/24"),
-				},
+				ServiceCIDR: *ipnet.MustParseCIDR("10.0.1.0/24"),
 			},
 			Platform: types.Platform{
 				AWS: &aws.Platform{

pkg/asset/ignition/machine/master_test.go

@@ -1,7 +1,6 @@
 package machine
 
 import (
-	"net"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -19,12 +18,7 @@ func TestWorkerGenerate(t *testing.T) {
 	installConfig := &installconfig.InstallConfig{
 		Config: &types.InstallConfig{
 			Networking: types.Networking{
-				ServiceCIDR: ipnet.IPNet{
-					IPNet: func(s string) net.IPNet {
-						_, cidr, _ := net.ParseCIDR(s)
-						return *cidr
-					}("10.0.1.0/24"),
-				},
+				ServiceCIDR: *ipnet.MustParseCIDR("10.0.1.0/24"),
 			},
 			Platform: types.Platform{
 				AWS: &aws.Platform{

pkg/asset/ignition/machine/worker_test.go

@@ -16,41 +16,19 @@ import (
 	"github.com/sirupsen/logrus"
 	survey "gopkg.in/AlecAivazis/survey.v1"
 
+	"github.com/openshift/installer/pkg/ipnet"
 	"github.com/openshift/installer/pkg/types/aws"
 )
 
-const (
-	defaultVPCCIDR = "10.0.0.0/16"
-)
-
 var (
-	validAWSRegions = map[string]string{
-		"ap-northeast-1": "Tokyo",
-		"ap-northeast-2": "Seoul",
-		"ap-northeast-3": "Osaka-Local",
-		"ap-south-1":     "Mumbai",
-		"ap-southeast-1": "Singapore",
-		"ap-southeast-2": "Sydney",
-		"ca-central-1":   "Central",
-		"cn-north-1":     "Beijing",
-		"cn-northwest-1": "Ningxia",
-		"eu-central-1":   "Frankfurt",
-		"eu-west-1":      "Ireland",
-		"eu-west-2":      "London",
-		"eu-west-3":      "Paris",
-		"sa-east-1":      "São Paulo",
-		"us-east-1":      "N. Virginia",
-		"us-east-2":      "Ohio",
-		"us-west-1":      "N. California",
-		"us-west-2":      "Oregon",
-	}
+	defaultVPCCIDR = ipnet.MustParseCIDR("10.0.0.0/16")
 )
 
 // Platform collects AWS-specific configuration.
 func Platform() (*aws.Platform, error) {
-	longRegions := make([]string, 0, len(validAWSRegions))
-	shortRegions := make([]string, 0, len(validAWSRegions))
-	for id, location := range validAWSRegions {
+	longRegions := make([]string, 0, len(aws.ValidRegions))
+	shortRegions := make([]string, 0, len(aws.ValidRegions))
+	for id, location := range aws.ValidRegions {
 		longRegions = append(longRegions, fmt.Sprintf("%s (%s)", id, location))
 		shortRegions = append(shortRegions, id)
 	}
@@ -59,7 +37,7 @@ func Platform() (*aws.Platform, error) {
 	})
 
 	defaultRegion := "us-east-1"
-	_, ok := validAWSRegions[defaultRegion]
+	_, ok := aws.ValidRegions[defaultRegion]
 	if !ok {
 		panic(fmt.Sprintf("installer bug: invalid default AWS region %q", defaultRegion))
 	}
@@ -81,7 +59,7 @@ func Platform() (*aws.Platform, error) {
 
 	defaultRegionPointer := ssn.Config.Region
 	if defaultRegionPointer != nil && *defaultRegionPointer != "" {
-		_, ok := validAWSRegions[*defaultRegionPointer]
+		_, ok := aws.ValidRegions[*defaultRegionPointer]
 		if ok {
 			defaultRegion = *defaultRegionPointer
 		} else {
@@ -98,7 +76,7 @@ func Platform() (*aws.Platform, error) {
 			Prompt: &survey.Select{
 				Message: "Region",
 				Help:    "The AWS region to be used for installation.",
-				Default: fmt.Sprintf("%s (%s)", defaultRegion, validAWSRegions[defaultRegion]),
+				Default: fmt.Sprintf("%s (%s)", defaultRegion, aws.ValidRegions[defaultRegion]),
 				Options: longRegions,
 			},
 			Validate: survey.ComposeValidators(survey.Required, func(ans interface{}) error {

pkg/asset/installconfig/aws/aws.go

@@ -12,6 +12,7 @@ import (
 	"github.com/openshift/installer/pkg/asset"
 	"github.com/openshift/installer/pkg/ipnet"
 	"github.com/openshift/installer/pkg/types"
+	"github.com/openshift/installer/pkg/types/validation"
 )
 
 const (
@@ -156,6 +157,10 @@ func (a *InstallConfig) Load(f asset.FileFetcher) (found bool, err error) {
 		return false, errors.Wrapf(err, "failed to unmarshal")
 	}
 
+	if err := validation.ValidateInstallConfig(config).ToAggregate(); err != nil {
+		return false, errors.Wrapf(err, "invalid %q file", installConfigFilename)
+	}
+
 	a.File, a.Config = file, config
 	return true, nil
 }

pkg/asset/installconfig/installconfig.go

@@ -7,14 +7,18 @@ import (
 	"github.com/pkg/errors"
 	survey "gopkg.in/AlecAivazis/survey.v1"
 
+	"github.com/openshift/installer/pkg/ipnet"
 	"github.com/openshift/installer/pkg/rhcos"
 	"github.com/openshift/installer/pkg/types/libvirt"
 	"github.com/openshift/installer/pkg/validate"
 )
 
 const (
-	defaultNetworkIfName  = "tt0"
-	defaultNetworkIPRange = "192.168.126.0/24"
+	defaultNetworkIfName = "tt0"
+)
+
+var (
+	defaultNetworkIPRange = ipnet.MustParseCIDR("192.168.126.0/24")
 )
 
 // Platform collects libvirt-specific configuration.
@@ -42,7 +46,7 @@ func Platform() (*libvirt.Platform, error) {
 	return &libvirt.Platform{
 		Network: libvirt.Network{
 			IfName:  defaultNetworkIfName,
-			IPRange: defaultNetworkIPRange,
+			IPRange: *defaultNetworkIPRange,
 		},
 		DefaultMachinePlatform: &libvirt.MachinePool{
 			Image: qcowImage,

pkg/asset/installconfig/libvirt/libvirt.go

@@ -12,11 +12,12 @@ import (
 	"github.com/pkg/errors"
 	survey "gopkg.in/AlecAivazis/survey.v1"
 
+	"github.com/openshift/installer/pkg/ipnet"
 	"github.com/openshift/installer/pkg/types/openstack"
 )
 
-const (
-	defaultVPCCIDR = "10.0.0.0/16"
+var (
+	defaultNetworkCIDR = ipnet.MustParseCIDR("10.0.0.0/16")
 )
 
 // Read the valid cloud names from the clouds.yaml
@@ -235,7 +236,7 @@ func Platform() (*openstack.Platform, error) {
 	}
 
 	return &openstack.Platform{
-		NetworkCIDRBlock: defaultVPCCIDR,
+		NetworkCIDRBlock: *defaultNetworkCIDR,
 		Region:           region,
 		BaseImage:        image,
 		Cloud:            cloud,

pkg/asset/installconfig/openstack/openstack.go

@@ -77,7 +77,7 @@ func provider(clusterName string, platform *libvirt.Platform, userDataSecret str
 			BaseVolumeID: fmt.Sprintf("/var/lib/libvirt/images/%s-base", clusterName),
 		},
 		NetworkInterfaceName:    clusterName,
-		NetworkInterfaceAddress: platform.Network.IPRange,
+		NetworkInterfaceAddress: platform.Network.IPRange.String(),
 		Autostart:               false,
 		URI:                     platform.URI,
 	}

pkg/asset/machines/libvirt/machines.go

@@ -70,3 +70,22 @@ func (ipnet *IPNet) UnmarshalJSON(b []byte) (err error) {
 
 	return nil
 }
+
+// ParseCIDR parses a CIDR from its string representation.
+func ParseCIDR(s string) (*IPNet, error) {
+	_, cidr, err := net.ParseCIDR(s)
+	if err != nil {
+		return nil, err
+	}
+	return &IPNet{IPNet: *cidr}, nil
+}
+
+// MustParseCIDR parses a CIDR from its string representation. If the parse fails,
+// the function will panic.
+func MustParseCIDR(s string) *IPNet {
+	cidr, err := ParseCIDR(s)
+	if err != nil {
+		panic(err)
+	}
+	return cidr
+}

pkg/ipnet/ipnet.go

@@ -18,8 +18,8 @@ type Libvirt struct {
 
 // Network describes a libvirt network configuration.
 type Network struct {
-	IfName  string `json:"libvirt_network_if,omitempty"`
-	IPRange string `json:"libvirt_ip_range,omitempty"`
+	IfName  string `json:"libvirt_network_if"`
+	IPRange string `json:"libvirt_ip_range"`
 }
 
 // TFVars fills in computed Terraform variables.