From 38e8df86342a88fbd2fd32ce0bca6a62a955d9a4 Mon Sep 17 00:00:00 2001 From: "Brad P. Crochet" Date: Thu, 1 Aug 2019 09:20:38 -0400 Subject: [PATCH] baremetal: Validate VIPs are in MachineCIDR The API, DNS, and Ingress virtual IPs (VIPs) should be contained within the MachineCIDR, but not ClusterNetworks, or ServiceNetworks. This adds those validations for the baremetal platform. Closes: #2006 --- docs/user/metal/install_ipi.md | 2 + pkg/types/baremetal/validation/platform.go | 26 ++++- pkg/types/validation/installconfig.go | 6 +- pkg/types/validation/installconfig_test.go | 111 +++++++++++++++++++++ 4 files changed, 141 insertions(+), 4 deletions(-) diff --git a/docs/user/metal/install_ipi.md b/docs/user/metal/install_ipi.md index 70cae9d3137..07836c0e682 100644 --- a/docs/user/metal/install_ipi.md +++ b/docs/user/metal/install_ipi.md @@ -126,6 +126,8 @@ apiVersion: v1beta4 baseDomain: test.metalkube.org metadata: name: ostest +networking: + machineCIDR: 192.168.111.0/24 compute: - name: worker replicas: 1 diff --git a/pkg/types/baremetal/validation/platform.go b/pkg/types/baremetal/validation/platform.go index 9528903adc6..1a48f9dda8b 100644 --- a/pkg/types/baremetal/validation/platform.go +++ b/pkg/types/baremetal/validation/platform.go @@ -1,13 +1,24 @@ package validation import ( + "fmt" + "net" + + "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/baremetal" "github.com/openshift/installer/pkg/validate" "k8s.io/apimachinery/pkg/util/validation/field" ) +func validateIPinMachineCIDR(vip string, n *types.Networking) error { + if !n.MachineCIDR.Contains(net.ParseIP(vip)) { + return fmt.Errorf("the virtual IP is expected to be in %s subnet", n.MachineCIDR.String()) + } + return nil +} + // ValidatePlatform checks that the specified platform is valid. -func ValidatePlatform(p *baremetal.Platform, fldPath *field.Path) field.ErrorList { +func ValidatePlatform(p *baremetal.Platform, n *types.Networking, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if err := validate.URI(p.LibvirtURI); err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("libvirtURI"), p.LibvirtURI, err.Error())) @@ -37,12 +48,25 @@ func ValidatePlatform(p *baremetal.Platform, fldPath *field.Path) field.ErrorLis allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVIP"), p.APIVIP, err.Error())) } + if err := validateIPinMachineCIDR(p.APIVIP, n); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("apiVIP"), p.APIVIP, err.Error())) + } + if err := validate.IP(p.IngressVIP); err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("ingressVIP"), p.IngressVIP, err.Error())) } + if err := validateIPinMachineCIDR(p.IngressVIP, n); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("ingressVIP"), p.IngressVIP, err.Error())) + } + if err := validate.IP(p.DNSVIP); err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("dnsVIP"), p.DNSVIP, err.Error())) } + + if err := validateIPinMachineCIDR(p.DNSVIP, n); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("dnsVIP"), p.DNSVIP, err.Error())) + } + return allErrs } diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index 0bbcb6e40ab..2f19c5ebac2 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -79,7 +79,7 @@ func ValidateInstallConfig(c *types.InstallConfig, openStackValidValuesFetcher o } else { allErrs = append(allErrs, field.Required(field.NewPath("networking"), "networking is required")) } - allErrs = append(allErrs, validatePlatform(&c.Platform, field.NewPath("platform"), openStackValidValuesFetcher)...) + allErrs = append(allErrs, validatePlatform(&c.Platform, field.NewPath("platform"), openStackValidValuesFetcher, c.Networking)...) if c.ControlPlane != nil { allErrs = append(allErrs, validateControlPlane(&c.Platform, c.ControlPlane, field.NewPath("controlPlane"))...) } else { @@ -201,7 +201,7 @@ func validateCompute(platform *types.Platform, pools []types.MachinePool, fldPat return allErrs } -func validatePlatform(platform *types.Platform, fldPath *field.Path, openStackValidValuesFetcher openstackvalidation.ValidValuesFetcher) field.ErrorList { +func validatePlatform(platform *types.Platform, fldPath *field.Path, openStackValidValuesFetcher openstackvalidation.ValidValuesFetcher, network *types.Networking) field.ErrorList { allErrs := field.ErrorList{} activePlatform := platform.Name() platforms := make([]string, len(types.PlatformNames)) @@ -240,7 +240,7 @@ func validatePlatform(platform *types.Platform, fldPath *field.Path, openStackVa } if platform.BareMetal != nil { validate(baremetal.Name, platform.BareMetal, func(f *field.Path) field.ErrorList { - return baremetalvalidation.ValidatePlatform(platform.BareMetal, f) + return baremetalvalidation.ValidatePlatform(platform.BareMetal, network, f) }) } return allErrs diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index 71a4606ba07..3d29261bdbc 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -2,6 +2,7 @@ package validation import ( "fmt" + "net" "testing" "github.com/golang/mock/gomock" @@ -12,6 +13,7 @@ import ( "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/aws" + "github.com/openshift/installer/pkg/types/baremetal" "github.com/openshift/installer/pkg/types/gcp" "github.com/openshift/installer/pkg/types/libvirt" "github.com/openshift/installer/pkg/types/none" @@ -86,6 +88,22 @@ func validVSpherePlatform() *vsphere.Platform { } } +func validBareMetalPlatform() *baremetal.Platform { + iface, _ := net.Interfaces() + return &baremetal.Platform{ + LibvirtURI: "qemu+tcp://192.168.122.1/system", + IronicURI: "https://localhost:6385", + Hosts: []*baremetal.Host{}, + Image: baremetal.Image{}, + ExternalBridge: iface[0].Name, + ProvisioningBridge: iface[0].Name, + DefaultMachinePlatform: &baremetal.MachinePool{}, + APIVIP: "10.0.0.5", + IngressVIP: "10.0.0.4", + DNSVIP: "10.0.0.2", + } +} + func TestValidateInstallConfig(t *testing.T) { cases := []struct { name string @@ -443,6 +461,99 @@ func TestValidateInstallConfig(t *testing.T) { expectedError: `^platform\.openstack\.cloud: Unsupported value: "": supported values: "test-cloud"$`, }, { + name: "valid baremetal platform", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + return c + }(), + }, + { + name: "invalid baremetal platform", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + c.Platform.BareMetal.APIVIP = "" + return c + }(), + expectedError: `^\[platform\.baremetal\.apiVIP: Invalid value: "": '' is not a valid IP, platform\.baremetal\.apiVIP: Invalid value: "": the virtual IP is expected to be in 10\.0\.0\.0/16 subnet]$`, + }, + { + name: "baremetal API VIP not an IP", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + c.Platform.BareMetal.APIVIP = "test" + return c + }(), + expectedError: `^\[platform\.baremetal\.apiVIP: Invalid value: "test": 'test' is not a valid IP, platform\.baremetal\.apiVIP: Invalid value: "test": the virtual IP is expected to be in 10\.0\.0\.0/16 subnet]$`, + }, + { + name: "baremetal API VIP set to an incorrect value", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + c.Platform.BareMetal.APIVIP = "10.1.0.5" + return c + }(), + expectedError: `^platform\.baremetal\.apiVIP: Invalid value: "10\.1\.0\.5": the virtual IP is expected to be in 10\.0\.0\.0/16 subnet$`, + }, + { + name: "baremetal DNS VIP not an IP", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + c.Platform.BareMetal.DNSVIP = "test" + return c + }(), + expectedError: `^\[platform\.baremetal\.dnsVIP: Invalid value: "test": 'test' is not a valid IP, platform\.baremetal\.dnsVIP: Invalid value: "test": the virtual IP is expected to be in 10\.0\.0\.0/16 subnet]$`, + }, + { + name: "baremetal DNS VIP set to an incorrect value", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + c.Platform.BareMetal.DNSVIP = "10.1.0.6" + return c + }(), + expectedError: `^platform\.baremetal\.dnsVIP: Invalid value: "10\.1\.0\.6": the virtual IP is expected to be in 10\.0\.0\.0/16 subnet$`, + }, + { + name: "baremetal Ingress VIP not an IP", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + c.Platform.BareMetal.IngressVIP = "test" + return c + }(), + expectedError: `^\[platform\.baremetal\.ingressVIP: Invalid value: "test": 'test' is not a valid IP, platform\.baremetal\.ingressVIP: Invalid value: "test": the virtual IP is expected to be in 10\.0\.0\.0/16 subnet]$`, + }, + { + name: "baremetal Ingress VIP set to an incorrect value", + installConfig: func() *types.InstallConfig { + c := validInstallConfig() + c.Platform = types.Platform{ + BareMetal: validBareMetalPlatform(), + } + c.Platform.BareMetal.IngressVIP = "10.1.0.7" + return c + }(), + expectedError: `^platform\.baremetal\.ingressVIP: Invalid value: "10\.1\.0\.7": the virtual IP is expected to be in 10\.0\.0\.0/16 subnet$`, + }, { name: "valid vsphere platform", installConfig: func() *types.InstallConfig { c := validInstallConfig()