diff --git a/Gopkg.lock b/Gopkg.lock index c681dd90f4a..e6e5b471a1b 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -878,6 +878,7 @@ "k8s.io/apimachinery/pkg/util/errors", "k8s.io/apimachinery/pkg/util/sets", "k8s.io/apimachinery/pkg/util/validation", + "k8s.io/apimachinery/pkg/util/validation/field", "k8s.io/apimachinery/pkg/util/wait", "k8s.io/apimachinery/pkg/watch", "k8s.io/client-go/kubernetes", diff --git a/pkg/asset/ignition/machine/master_test.go b/pkg/asset/ignition/machine/master_test.go index f0ce4eed3e9..ae4da49d3b9 100644 --- a/pkg/asset/ignition/machine/master_test.go +++ b/pkg/asset/ignition/machine/master_test.go @@ -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{ diff --git a/pkg/asset/ignition/machine/worker_test.go b/pkg/asset/ignition/machine/worker_test.go index 3858c035819..ba79aaa2813 100644 --- a/pkg/asset/ignition/machine/worker_test.go +++ b/pkg/asset/ignition/machine/worker_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{ diff --git a/pkg/asset/installconfig/aws/aws.go b/pkg/asset/installconfig/aws/aws.go index e357381ea8a..54a47266673 100644 --- a/pkg/asset/installconfig/aws/aws.go +++ b/pkg/asset/installconfig/aws/aws.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 { diff --git a/pkg/asset/installconfig/installconfig.go b/pkg/asset/installconfig/installconfig.go index 359056f7b4a..533154bb8e8 100644 --- a/pkg/asset/installconfig/installconfig.go +++ b/pkg/asset/installconfig/installconfig.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 } diff --git a/pkg/asset/installconfig/libvirt/libvirt.go b/pkg/asset/installconfig/libvirt/libvirt.go index f12d8da9758..a9e86acace3 100644 --- a/pkg/asset/installconfig/libvirt/libvirt.go +++ b/pkg/asset/installconfig/libvirt/libvirt.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, diff --git a/pkg/asset/installconfig/openstack/openstack.go b/pkg/asset/installconfig/openstack/openstack.go index 2af3f81df1e..f1de960a53e 100644 --- a/pkg/asset/installconfig/openstack/openstack.go +++ b/pkg/asset/installconfig/openstack/openstack.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, diff --git a/pkg/asset/machines/libvirt/machines.go b/pkg/asset/machines/libvirt/machines.go index 718f5bdc741..bd18c139d20 100644 --- a/pkg/asset/machines/libvirt/machines.go +++ b/pkg/asset/machines/libvirt/machines.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, } diff --git a/pkg/ipnet/ipnet.go b/pkg/ipnet/ipnet.go index eff16ee0c8f..354fbfa67a3 100644 --- a/pkg/ipnet/ipnet.go +++ b/pkg/ipnet/ipnet.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 +} diff --git a/pkg/tfvars/libvirt/libvirt.go b/pkg/tfvars/libvirt/libvirt.go index 12f4442c19a..8643daf077d 100644 --- a/pkg/tfvars/libvirt/libvirt.go +++ b/pkg/tfvars/libvirt/libvirt.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. diff --git a/pkg/tfvars/tfvars.go b/pkg/tfvars/tfvars.go index 8825ec267cf..4a0122f1cd7 100644 --- a/pkg/tfvars/tfvars.go +++ b/pkg/tfvars/tfvars.go @@ -82,9 +82,14 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e } config.AWS = aws.AWS{ - Region: cfg.Platform.AWS.Region, - ExtraTags: cfg.Platform.AWS.UserTags, - VPCCIDRBlock: cfg.Platform.AWS.VPCCIDRBlock, + Region: cfg.Platform.AWS.Region, + ExtraTags: cfg.Platform.AWS.UserTags, + VPCCIDRBlock: func() string { + if cfg.Platform.AWS.VPCCIDRBlock == nil { + return "" + } + return cfg.Platform.AWS.VPCCIDRBlock.String() + }(), EC2AMIOverride: ami, } } else if cfg.Platform.Libvirt != nil { @@ -96,7 +101,7 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e URI: cfg.Platform.Libvirt.URI, Network: libvirt.Network{ IfName: cfg.Platform.Libvirt.Network.IfName, - IPRange: cfg.Platform.Libvirt.Network.IPRange, + IPRange: cfg.Platform.Libvirt.Network.IPRange.String(), }, Image: cfg.Platform.Libvirt.DefaultMachinePlatform.Image, MasterIPs: masterIPs, @@ -110,7 +115,7 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e } else if cfg.Platform.OpenStack != nil { config.OpenStack = openstack.OpenStack{ Region: cfg.Platform.OpenStack.Region, - NetworkCIDRBlock: cfg.Platform.OpenStack.NetworkCIDRBlock, + NetworkCIDRBlock: cfg.Platform.OpenStack.NetworkCIDRBlock.String(), BaseImage: cfg.Platform.OpenStack.BaseImage, } config.OpenStack.Credentials.Cloud = cfg.Platform.OpenStack.Cloud diff --git a/pkg/types/aws/machinepool.go b/pkg/types/aws/machinepool.go index 1e5331b512c..b19dcea1ad2 100644 --- a/pkg/types/aws/machinepool.go +++ b/pkg/types/aws/machinepool.go @@ -53,10 +53,10 @@ func (a *MachinePool) Set(required *MachinePool) { // EC2RootVolume defines the storage for an ec2 instance. type EC2RootVolume struct { - // IOPS defines the iops for the instance. + // IOPS defines the iops for the storage. IOPS int `json:"iops"` - // Size defines the size of the instance. + // Size defines the size of the storage. Size int `json:"size"` - // Type defines the type of the instance. + // Type defines the type of the storage. Type string `json:"type"` } diff --git a/pkg/types/aws/platform.go b/pkg/types/aws/platform.go index 407834f46c7..d99ca07ce08 100644 --- a/pkg/types/aws/platform.go +++ b/pkg/types/aws/platform.go @@ -1,5 +1,35 @@ package aws +import ( + "github.com/openshift/installer/pkg/ipnet" +) + +var ( + // ValidRegions is a map of the known AWS regions. The key of the map is + // the short name of the region. The value of the map is the long name of + // the region. + ValidRegions = 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", + } +) + // Platform stores all the global configuration that all machinesets // use. type Platform struct { @@ -15,5 +45,6 @@ type Platform struct { DefaultMachinePlatform *MachinePool `json:"defaultMachinePlatform,omitempty"` // VPCCIDRBlock - VPCCIDRBlock string `json:"vpcCIDRBlock"` + // +optional + VPCCIDRBlock *ipnet.IPNet `json:"vpcCIDRBlock,omitempty"` } diff --git a/pkg/types/aws/validation/machinepool.go b/pkg/types/aws/validation/machinepool.go new file mode 100644 index 00000000000..813659dc1e6 --- /dev/null +++ b/pkg/types/aws/validation/machinepool.go @@ -0,0 +1,19 @@ +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types/aws" +) + +// ValidateMachinePool checks that the specified machine pool is valid. +func ValidateMachinePool(p *aws.MachinePool, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if p.IOPS < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("iops"), p.IOPS, "Storage IOPS must be positive")) + } + if p.Size < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("size"), p.IOPS, "Storage size must be positive")) + } + return allErrs +} diff --git a/pkg/types/aws/validation/platform.go b/pkg/types/aws/validation/platform.go new file mode 100644 index 00000000000..e662acfd0e1 --- /dev/null +++ b/pkg/types/aws/validation/platform.go @@ -0,0 +1,37 @@ +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/validate" +) + +var ( + validRegionValues = func() []string { + validValues := make([]string, len(aws.ValidRegions)) + i := 0 + for r := range aws.ValidRegions { + validValues[i] = r + i++ + } + return validValues + }() +) + +// ValidatePlatform checks that the specified platform is valid. +func ValidatePlatform(p *aws.Platform, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if _, ok := aws.ValidRegions[p.Region]; !ok { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("region"), p.Region, validRegionValues)) + } + if p.DefaultMachinePlatform != nil { + allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...) + } + if p.VPCCIDRBlock != nil { + if err := validate.SubnetCIDR(&p.VPCCIDRBlock.IPNet); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("vpcCIDRBlock"), p.VPCCIDRBlock, err.Error())) + } + } + return allErrs +} diff --git a/pkg/types/libvirt/network.go b/pkg/types/libvirt/network.go index 98198f7ff20..e584ee25227 100644 --- a/pkg/types/libvirt/network.go +++ b/pkg/types/libvirt/network.go @@ -1,9 +1,13 @@ package libvirt +import ( + "github.com/openshift/installer/pkg/ipnet" +) + // Network is the configuration of the libvirt network. type Network struct { // IfName is the name of the network interface. IfName string `json:"if"` // IPRange is the range of IPs to use. - IPRange string `json:"ipRange"` + IPRange ipnet.IPNet `json:"ipRange"` } diff --git a/pkg/types/libvirt/platform.go b/pkg/types/libvirt/platform.go index 0a41a14d378..e9dbe6fbac9 100644 --- a/pkg/types/libvirt/platform.go +++ b/pkg/types/libvirt/platform.go @@ -21,5 +21,5 @@ type Platform struct { Network Network `json:"network"` // MasterIPs - MasterIPs []net.IP `json:"masterIPs"` + MasterIPs []net.IP `json:"masterIPs,omitempty"` } diff --git a/pkg/types/libvirt/validation/machinepool.go b/pkg/types/libvirt/validation/machinepool.go new file mode 100644 index 00000000000..42c330a7668 --- /dev/null +++ b/pkg/types/libvirt/validation/machinepool.go @@ -0,0 +1,19 @@ +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types/libvirt" + "github.com/openshift/installer/pkg/validate" +) + +// ValidateMachinePool checks that the specified machine pool is valid. +func ValidateMachinePool(p *libvirt.MachinePool, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if p.Image != "" { + if err := validate.URI(p.Image); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("image"), p.Image, err.Error())) + } + } + return allErrs +} diff --git a/pkg/types/libvirt/validation/platform.go b/pkg/types/libvirt/validation/platform.go new file mode 100644 index 00000000000..626efe0068e --- /dev/null +++ b/pkg/types/libvirt/validation/platform.go @@ -0,0 +1,26 @@ +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types/libvirt" + "github.com/openshift/installer/pkg/validate" +) + +// ValidatePlatform checks that the specified platform is valid. +func ValidatePlatform(p *libvirt.Platform, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if err := validate.URI(p.URI); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("uri"), p.URI, err.Error())) + } + if p.DefaultMachinePlatform != nil { + allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...) + } + if p.Network.IfName == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("network").Child("if"), p.Network.IfName)) + } + if err := validate.SubnetCIDR(&p.Network.IPRange.IPNet); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("network").Child("ipRange"), p.Network.IPRange, err.Error())) + } + return allErrs +} diff --git a/pkg/types/openstack/platform.go b/pkg/types/openstack/platform.go index 8a65d3e19f4..f8dcb670c55 100644 --- a/pkg/types/openstack/platform.go +++ b/pkg/types/openstack/platform.go @@ -1,5 +1,9 @@ package openstack +import ( + "github.com/openshift/installer/pkg/ipnet" +) + // Platform stores all the global configuration that all // machinesets use. type Platform struct { @@ -12,8 +16,7 @@ type Platform struct { DefaultMachinePlatform *MachinePool `json:"defaultMachinePlatform,omitempty"` // NetworkCIDRBlock - // +optional - NetworkCIDRBlock string `json:"NetworkCIDRBlock"` + NetworkCIDRBlock ipnet.IPNet `json:"NetworkCIDRBlock"` // BaseImage // Name of image to use from OpenStack cloud diff --git a/pkg/types/openstack/validation/machinepool.go b/pkg/types/openstack/validation/machinepool.go new file mode 100644 index 00000000000..9e099dcecb3 --- /dev/null +++ b/pkg/types/openstack/validation/machinepool.go @@ -0,0 +1,19 @@ +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types/openstack" +) + +// ValidateMachinePool checks that the specified machine pool is valid. +func ValidateMachinePool(p *openstack.MachinePool, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if p.RootVolume.IOPS < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("rootVolume").Child("iops"), p.RootVolume.IOPS, "Root volume IOPS must be positive")) + } + if p.RootVolume.Size < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("rootVolume").Child("size"), p.RootVolume.IOPS, "Root volume size must be positive")) + } + return allErrs +} diff --git a/pkg/types/openstack/validation/platform.go b/pkg/types/openstack/validation/platform.go new file mode 100644 index 00000000000..fc31868da92 --- /dev/null +++ b/pkg/types/openstack/validation/platform.go @@ -0,0 +1,23 @@ +package validation + +import ( + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types/openstack" + "github.com/openshift/installer/pkg/validate" +) + +// ValidatePlatform checks that the specified platform is valid. +func ValidatePlatform(p *openstack.Platform, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if p.DefaultMachinePlatform != nil { + allErrs = append(allErrs, ValidateMachinePool(p.DefaultMachinePlatform, fldPath.Child("defaultMachinePlatform"))...) + } + if err := validate.SubnetCIDR(&p.NetworkCIDRBlock.IPNet); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("NetworkCIDRBlock"), p.NetworkCIDRBlock, err.Error())) + } + if err := validate.URI(p.BaseImage); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("baseImage"), p.BaseImage, err.Error())) + } + return allErrs +} diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go new file mode 100644 index 00000000000..7c12f4705ab --- /dev/null +++ b/pkg/types/validation/installconfig.go @@ -0,0 +1,126 @@ +package validation + +import ( + "fmt" + "net" + + netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" + awsvalidation "github.com/openshift/installer/pkg/types/aws/validation" + "github.com/openshift/installer/pkg/types/libvirt" + libvirtvalidation "github.com/openshift/installer/pkg/types/libvirt/validation" + "github.com/openshift/installer/pkg/types/openstack" + openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation" + "github.com/openshift/installer/pkg/validate" +) + +// ValidateInstallConfig checks that the specified install config is valid. +func ValidateInstallConfig(c *types.InstallConfig) field.ErrorList { + allErrs := field.ErrorList{} + if c.ObjectMeta.Name == "" { + allErrs = append(allErrs, field.Required(field.NewPath("metadata", "name"), "cluster name required")) + } + if c.ClusterID == "" { + allErrs = append(allErrs, field.Required(field.NewPath("clusterID"), "cluster ID required")) + } + if c.SSHKey != "" { + if err := validate.SSHPublicKey(c.SSHKey); err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("sshKey"), c.SSHKey, err.Error())) + } + } + if err := validate.DomainName(c.BaseDomain); err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("baseDomain"), c.BaseDomain, err.Error())) + } + allErrs = append(allErrs, validateNetworking(&c.Networking, field.NewPath("networking"))...) + allErrs = append(allErrs, validateMachinePools(c.Machines, field.NewPath("machines"), c.Platform.Name())...) + allErrs = append(allErrs, validatePlatform(&c.Platform, field.NewPath("platform"))...) + if err := validate.ImagePullSecret(c.PullSecret); err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("pullSecret"), c.PullSecret, err.Error())) + } + return allErrs +} + +func validateNetworking(n *types.Networking, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + if !validate.ValidNetworkTypes[n.Type] { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("type"), n.Type, validate.ValidNetworkTypeValues)) + } + if err := validate.SubnetCIDR(&n.ServiceCIDR.IPNet); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("serviceCIDR"), n.ServiceCIDR, err.Error())) + } + for i, cn := range n.ClusterNetworks { + allErrs = append(allErrs, validateClusterNetwork(&cn, fldPath.Child("clusterNetworks").Index(i), &n.ServiceCIDR.IPNet)...) + } + if !n.PodCIDR.IPNet.IP.IsUnspecified() { + if err := validate.SubnetCIDR(&n.PodCIDR.IPNet); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("podCIDR"), n.PodCIDR, err.Error())) + } + if validate.DoCIDRsOverlap(&n.ServiceCIDR.IPNet, &n.PodCIDR.IPNet) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("podCIDR"), n.PodCIDR, "podCIDR must not overlap with serviceCIDR")) + } + } + if len(n.ClusterNetworks) == 0 && n.PodCIDR.IPNet.IP.IsUnspecified() { + allErrs = append(allErrs, field.Invalid(fldPath, n, "either clusterNetworks or podCIDR is required")) + } + if len(n.ClusterNetworks) != 0 && !n.PodCIDR.IPNet.IP.IsUnspecified() { + allErrs = append(allErrs, field.Invalid(fldPath.Child("podCIDR"), n.PodCIDR, "cannot use podCIDR when clusterNetworks is used")) + } + return allErrs +} + +func validateClusterNetwork(cn *netopv1.ClusterNetwork, fldPath *field.Path, serviceCIDR *net.IPNet) field.ErrorList { + allErrs := field.ErrorList{} + _, cidr, err := net.ParseCIDR(cn.CIDR) + if err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("cidr"), cn.CIDR, err.Error())) + } + if validate.DoCIDRsOverlap(cidr, serviceCIDR) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("cidr"), cn.CIDR, "cluster network CIDR must not overlap with serviceCIDR")) + } + if cn.HostSubnetLength <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("hostSubnetLength"), cn.HostSubnetLength, "cluster network host subnet length must be positive")) + } + if _, bits := cidr.Mask.Size(); cn.HostSubnetLength > uint32(bits) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("hostSubnetLength"), cn.HostSubnetLength, "cluster network host subnet lenght must not be greater than CIDR length")) + } + return allErrs +} + +func validateMachinePools(pools []types.MachinePool, fldPath *field.Path, platform string) field.ErrorList { + allErrs := field.ErrorList{} + if len(pools) != 2 { + allErrs = append(allErrs, field.Required(fldPath, "must have a single 'master' and a single 'worker' machine pool")) + } + for i, p := range pools { + allErrs = append(allErrs, ValidateMachinePool(&p, fldPath.Index(i), platform)...) + } + return allErrs +} + +func validatePlatform(platform *types.Platform, fldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + activePlatform := "" + validate := func(n string, value interface{}, validation func(*field.Path) field.ErrorList) { + if activePlatform != "" { + allErrs = append(allErrs, field.Invalid(fldPath, platform, fmt.Sprintf("must only specify a single type of platform; cannot use both %q and %q", activePlatform, n))) + } else { + activePlatform = n + } + validation(fldPath.Child(n)) + } + if platform.AWS != nil { + validate(aws.Name, platform.AWS, func(f *field.Path) field.ErrorList { return awsvalidation.ValidatePlatform(platform.AWS, f) }) + } + if platform.Libvirt != nil { + validate(libvirt.Name, platform.Libvirt, func(f *field.Path) field.ErrorList { return libvirtvalidation.ValidatePlatform(platform.Libvirt, f) }) + } + if platform.OpenStack != nil { + validate(openstack.Name, platform.OpenStack, func(f *field.Path) field.ErrorList { + return openstackvalidation.ValidatePlatform(platform.OpenStack, f) + }) + } + return allErrs +} diff --git a/pkg/types/validation/machinepools.go b/pkg/types/validation/machinepools.go new file mode 100644 index 00000000000..58306285f5e --- /dev/null +++ b/pkg/types/validation/machinepools.go @@ -0,0 +1,69 @@ +package validation + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/openshift/installer/pkg/types" + "github.com/openshift/installer/pkg/types/aws" + awsvalidation "github.com/openshift/installer/pkg/types/aws/validation" + "github.com/openshift/installer/pkg/types/libvirt" + libvirtvalidation "github.com/openshift/installer/pkg/types/libvirt/validation" + "github.com/openshift/installer/pkg/types/openstack" + openstackvalidation "github.com/openshift/installer/pkg/types/openstack/validation" +) + +var ( + validMachinePoolNames = map[string]bool{ + "master": true, + "worker": true, + } + + validMachinePoolNameValues = func() []string { + validValues := make([]string, len(validMachinePoolNames)) + i := 0 + for n := range validMachinePoolNames { + validValues[i] = n + i++ + } + return validValues + }() +) + +// ValidateMachinePool checks that the specified machine pool is valid. +func ValidateMachinePool(p *types.MachinePool, fldPath *field.Path, platform string) field.ErrorList { + allErrs := field.ErrorList{} + if !validMachinePoolNames[p.Name] { + allErrs = append(allErrs, field.NotSupported(fldPath.Child("name"), p.Name, validMachinePoolNameValues)) + } + if p.Replicas != nil { + if *p.Replicas <= 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("replicas"), p.Replicas, "number of replicas must be positive")) + } + } + allErrs = append(allErrs, validateMachinePoolPlatform(&p.Platform, fldPath.Child("platform"), platform)...) + return allErrs +} + +func validateMachinePoolPlatform(p *types.MachinePoolPlatform, fldPath *field.Path, platform string) field.ErrorList { + allErrs := field.ErrorList{} + validate := func(n string, value interface{}, validation func(*field.Path) field.ErrorList) { + f := fldPath.Child(n) + if platform == n { + allErrs = append(allErrs, validation(f)...) + } else { + allErrs = append(allErrs, field.Invalid(f, value, fmt.Sprintf("cannot specify %q for machine pool when cluster is using %q", n, platform))) + } + } + if p.AWS != nil { + validate(aws.Name, p.AWS, func(f *field.Path) field.ErrorList { return awsvalidation.ValidateMachinePool(p.AWS, f) }) + } + if p.Libvirt != nil { + validate(libvirt.Name, p.Libvirt, func(f *field.Path) field.ErrorList { return libvirtvalidation.ValidateMachinePool(p.Libvirt, f) }) + } + if p.OpenStack != nil { + validate(openstack.Name, p.OpenStack, func(f *field.Path) field.ErrorList { return openstackvalidation.ValidateMachinePool(p.OpenStack, f) }) + } + return allErrs +} diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go index dc89541f916..bc744b64c99 100644 --- a/pkg/validate/validate.go +++ b/pkg/validate/validate.go @@ -8,15 +8,42 @@ import ( "net" "net/url" "regexp" - "strconv" "strings" "unicode/utf8" + netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "golang.org/x/crypto/ssh" k8serrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation" ) +var ( + dockerBridgeCIDR = func() *net.IPNet { + _, cidr, _ := net.ParseCIDR("172.17.0.0/16") + return cidr + }() + + // ValidNetworkTypes is a collection of the valid network types. + ValidNetworkTypes = map[netopv1.NetworkType]bool{ + netopv1.NetworkTypeOpenshiftSDN: true, + netopv1.NetworkTypeOVNKubernetes: true, + netopv1.NetworkTypeCalico: true, + netopv1.NetworkTypeKuryr: true, + } + + // ValidNetworkTypeValues is a slice filled with the valid network types as + // strings. + ValidNetworkTypeValues = func() []string { + validValues := make([]string, len(ValidNetworkTypes)) + i := 0 + for t := range ValidNetworkTypes { + validValues[i] = string(t) + i++ + } + return validValues + }() +) + func validateSubdomain(v string) error { validationMessages := validation.IsDNS1123Subdomain(v) if len(validationMessages) == 0 { @@ -105,80 +132,23 @@ func nonEmpty(v string) error { return nil } -// SubnetCIDR checks if the given string is a valid CIDR for a master nodes or worker nodes subnet and returns an error if not. -func SubnetCIDR(v string) error { - if err := nonEmpty(v); err != nil { - return err - } - - split := strings.Split(v, "/") - - if len(split) == 1 { - return errors.New("must provide a CIDR netmask (eg, /24)") +// SubnetCIDR checks if the given IP net is a valid CIDR for a master nodes or worker nodes subnet and returns an error if not. +func SubnetCIDR(cidr *net.IPNet) error { + if cidr.IP.To4() == nil { + return errors.New("must use IPv4") } - - if len(split) != 2 { - return errors.New("invalid IPv4 address") - } - - ip := split[0] - - if err := IPv4(ip); err != nil { - return errors.New("invalid IPv4 address") - } - - if mask, err := strconv.Atoi(split[1]); err != nil || mask < 0 || mask > 32 { - return errors.New("invalid netmask size (must be between 0 and 32)") - } - - // Catch any invalid CIDRs not caught by the checks above - if _, _, err := net.ParseCIDR(v); err != nil { - return errors.New("invalid CIDR") + if cidr.IP.IsUnspecified() { + return errors.New("address must be specified") } - - if strings.HasPrefix(ip, "172.17.") { - return errors.New("overlaps with default Docker Bridge subnet (172.17.0.0/16)") + if DoCIDRsOverlap(cidr, dockerBridgeCIDR) { + return fmt.Errorf("overlaps with default Docker Bridge subnet (%v)", cidr.String()) } - return nil } -// CIDRsDontOverlap ensures two given CIDRs don't overlap -// with one another. CIDR starting IPs are canonicalized -// before being compared. -func CIDRsDontOverlap(acidr, bcidr string) error { - _, a, err := net.ParseCIDR(acidr) - if err != nil { - return fmt.Errorf("invalid CIDR %q: %v", acidr, err) - } - if err := canonicalizeIP(&a.IP); err != nil { - return fmt.Errorf("invalid CIDR %q: %v", acidr, err) - } - _, b, err := net.ParseCIDR(bcidr) - if err != nil { - return fmt.Errorf("invalid CIDR %q: %v", bcidr, err) - } - if err := canonicalizeIP(&b.IP); err != nil { - return fmt.Errorf("invalid CIDR %q: %v", bcidr, err) - } - err = fmt.Errorf("%q and %q overlap", acidr, bcidr) - // IPs are of different families. - if len(a.IP) != len(b.IP) { - return nil - } - if a.Contains(b.IP) { - return err - } - if a.Contains(lastIP(b)) { - return err - } - if b.Contains(a.IP) { - return err - } - if b.Contains(lastIP(a)) { - return err - } - return nil +// DoCIDRsOverlap returns true if one of the CIDRs is a subset of the other. +func DoCIDRsOverlap(acidr, bcidr *net.IPNet) bool { + return acidr.Contains(bcidr.IP) || bcidr.Contains(acidr.IP) } // IPv4 checks if the given string is a valid IP v4 address and returns an error if not. diff --git a/pkg/validate/validate_test.go b/pkg/validate/validate_test.go index b2d3ff2a1c4..363b56bc1ef 100644 --- a/pkg/validate/validate_test.go +++ b/pkg/validate/validate_test.go @@ -3,7 +3,6 @@ package validate import ( "fmt" "net" - "regexp" "strings" "testing" @@ -144,34 +143,36 @@ func TestIPv4(t *testing.T) { } func TestSubnetCIDR(t *testing.T) { - const netmaskSizeMsg = "invalid netmask size (must be between 0 and 32)" - - tests := []test{ - {"", emptyMsg}, - {" ", emptyMsg}, - {"/16", invalidIPMsg}, - {"0.0.0.0/0", ""}, - {"0.0.0.0/32", ""}, - {"1.2.3.4", noCIDRNetmaskMsg}, - {"1.2.3.", noCIDRNetmaskMsg}, - {"1.2.3.4.", noCIDRNetmaskMsg}, - {"1.2.3.4/0", ""}, - {"1.2.3.4/1", ""}, - {"1.2.3.4/31", ""}, - {"1.2.3.4/32", ""}, - {"1.2.3./16", invalidIPMsg}, - {"1.2.3.4./16", invalidIPMsg}, - {"1.2.3.4/33", netmaskSizeMsg}, - {"1.2.3.4/-1", netmaskSizeMsg}, - {"1.2.3.4/abc", netmaskSizeMsg}, - {"172.17.1.2", noCIDRNetmaskMsg}, - {"172.17.1.2/", netmaskSizeMsg}, - {"172.17.1.2/33", netmaskSizeMsg}, - {"172.17.1.2/20", "overlaps with default Docker Bridge subnet (172.17.0.0/16)"}, - {"255.255.255.255/1", ""}, - {"255.255.255.255/32", ""}, + cases := []struct { + cidr string + valid bool + }{ + {"0.0.0.0/32", false}, + {"1.2.3.4/0", false}, + {"1.2.3.4/1", false}, + {"1.2.3.4/31", true}, + {"1.2.3.4/32", true}, + {"0:0:0:0:0:1:102:304/116", false}, + {"0:0:0:0:0:ffff:102:304/116", true}, + {"172.17.1.2/20", false}, + {"172.17.1.2/8", false}, + {"255.255.255.255/1", false}, + {"255.255.255.255/32", true}, + } + for _, tc := range cases { + t.Run(tc.cidr, func(t *testing.T) { + _, cidr, err := net.ParseCIDR(tc.cidr) + if err != nil { + t.Fatalf("could not parse cidr: %v", err) + } + err = SubnetCIDR(cidr) + if tc.valid { + assert.NoError(t, err) + } else { + assert.Error(t, err) + } + }) } - runTests(t, "SubnetCIDR", SubnetCIDR, tests) } func TestDomainName(t *testing.T) { @@ -236,48 +237,45 @@ func TestEmail(t *testing.T) { runTests(t, "Email", Email, tests) } -func TestCIDRsDontOverlap(t *testing.T) { +func TestDoCIDRsOverlap(t *testing.T) { cases := []struct { - a string - b string - err *regexp.Regexp + a string + b string + overlap bool }{ { - a: "192.168.0.0/24", - b: "192.168.0.0/24", - err: regexp.MustCompile("^\"192.168.0.0/24\" and \"192.168.0.0/24\" overlap$"), + a: "192.168.0.0/30", + b: "192.168.0.3/30", + overlap: true, }, { - a: "192.168.0.0/24", - b: "192.168.0.3/24", - err: regexp.MustCompile("^\"192.168.0.0/24\" and \"192.168.0.3/24\" overlap$"), + a: "192.168.0.0/30", + b: "192.168.0.4/30", + overlap: false, }, { - a: "192.168.0.0/30", - b: "192.168.0.3/30", - err: regexp.MustCompile("^\"192.168.0.0/30\" and \"192.168.0.3/30\" overlap$"), + a: "192.168.0.0/29", + b: "192.168.0.4/30", + overlap: true, }, { - a: "192.168.0.0/30", - b: "192.168.0.4/30", - }, - { - a: "0.0.0.0/0", - b: "192.168.0.0/24", - err: regexp.MustCompile("^\"0.0.0.0/0\" and \"192.168.0.0/24\" overlap$"), + a: "0.0.0.0/0", + b: "192.168.0.0/24", + overlap: true, }, } - - for _, testCase := range cases { - t.Run(fmt.Sprintf("%s %s", testCase.a, testCase.b), func(t *testing.T) { - err := CIDRsDontOverlap(testCase.a, testCase.b) - if testCase.err == nil { - if err != nil { - t.Fatal(err) - } - } else { - assert.Regexp(t, testCase.err, err) + for _, tc := range cases { + t.Run(fmt.Sprintf("%s %s", tc.a, tc.b), func(t *testing.T) { + _, a, err := net.ParseCIDR(tc.a) + if err != nil { + t.Fatalf("could not parse cidr %q: %v", tc.a, err) + } + _, b, err := net.ParseCIDR(tc.b) + if err != nil { + t.Fatalf("could not parse cidr %q: %v", tc.b, err) } + actual := DoCIDRsOverlap(a, b) + assert.Equal(t, tc.overlap, actual) }) } }