From 3b5d253b6ac8ba9c83c361660ee132068bd057ab Mon Sep 17 00:00:00 2001 From: Andrea Fasano Date: Wed, 4 Mar 2020 13:17:00 +0100 Subject: [PATCH] validation: add host validations baremetal.Host is validated using go-playground/validator module and a custom defined rule for duplicates --- pkg/types/baremetal/platform.go | 10 +- pkg/types/baremetal/validation/platform.go | 58 +++- .../baremetal/validation/platform_test.go | 248 +++++++++++++++++- 3 files changed, 309 insertions(+), 7 deletions(-) diff --git a/pkg/types/baremetal/platform.go b/pkg/types/baremetal/platform.go index 37e8673a4b0..cc3f04d75dc 100644 --- a/pkg/types/baremetal/platform.go +++ b/pkg/types/baremetal/platform.go @@ -6,18 +6,18 @@ import ( // BMC stores the information about a baremetal host's management controller. type BMC struct { - Username string `json:"username"` - Password string `json:"password"` - Address string `json:"address"` + Username string `json:"username" validate:"required"` + Password string `json:"password" validate:"required"` + Address string `json:"address" validate:"required,uniqueField"` DisableCertificateVerification bool `json:"disableCertificateVerification"` } // Host stores all the configuration data for a baremetal host. type Host struct { - Name string `json:"name,omitempty"` + Name string `json:"name,omitempty" validate:"required,uniqueField"` BMC BMC `json:"bmc"` Role string `json:"role"` - BootMACAddress string `json:"bootMACAddress"` + BootMACAddress string `json:"bootMACAddress" validate:"required,uniqueField"` HardwareProfile string `json:"hardwareProfile"` } diff --git a/pkg/types/baremetal/validation/platform.go b/pkg/types/baremetal/validation/platform.go index cd4fe590813..835715655cf 100644 --- a/pkg/types/baremetal/validation/platform.go +++ b/pkg/types/baremetal/validation/platform.go @@ -4,12 +4,15 @@ import ( "fmt" "net" "net/url" + "reflect" + "strings" + + "github.com/go-playground/validator/v10" "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" - "strings" ) // dynamicValidator is a function that validates certain fields in the platform. @@ -61,6 +64,57 @@ func validateOSImageURI(uri string) error { return nil } +// validateHosts checks that hosts have all required fields set with appropriate values +func validateHosts(hosts []*baremetal.Host, fldPath *field.Path) field.ErrorList { + hostErrs := field.ErrorList{} + + values := make(map[string]map[interface{}]struct{}) + + //Initialize a new validator and register a custom validation rule for the tag `uniqueField` + validate := validator.New() + validate.RegisterValidation("uniqueField", func(fl validator.FieldLevel) bool { + valueFound := false + fieldName := fl.Parent().Type().Name() + "." + fl.FieldName() + fieldValue := fl.Field().Interface() + + if fl.Field().Type().Comparable() { + if _, present := values[fieldName]; !present { + values[fieldName] = make(map[interface{}]struct{}) + } + + fieldValues := values[fieldName] + if _, valueFound = fieldValues[fieldValue]; !valueFound { + fieldValues[fieldValue] = struct{}{} + } + } else { + panic(fmt.Sprintf("Cannot apply validation rule 'uniqueField' on field %s", fl.FieldName())) + } + + return !valueFound + }) + + //Apply validations and translate errors + fldPath = fldPath.Child("hosts") + + for idx, host := range hosts { + err := validate.Struct(host) + if err != nil { + hostType := reflect.TypeOf(hosts).Elem().Elem().Name() + for _, err := range err.(validator.ValidationErrors) { + childName := fldPath.Index(idx).Child(err.Namespace()[len(hostType)+1:]) + switch err.Tag() { + case "required": + hostErrs = append(hostErrs, field.Required(childName, "missing "+err.Field())) + case "uniqueField": + hostErrs = append(hostErrs, field.Duplicate(childName, err.Value())) + } + } + } + } + + return hostErrs +} + // ValidatePlatform checks that the specified platform is valid. func ValidatePlatform(p *baremetal.Platform, n *types.Networking, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -156,6 +210,8 @@ func ValidatePlatform(p *baremetal.Platform, n *types.Networking, fldPath *field } } + allErrs = append(allErrs, validateHosts(p.Hosts, fldPath)...) + for _, validator := range dynamicValidators { allErrs = append(allErrs, validator(p, fldPath)...) } diff --git a/pkg/types/baremetal/validation/platform_test.go b/pkg/types/baremetal/validation/platform_test.go index 8e45525f9d8..b7bdcfa1b2b 100644 --- a/pkg/types/baremetal/validation/platform_test.go +++ b/pkg/types/baremetal/validation/platform_test.go @@ -481,11 +481,257 @@ func TestValidatePlatform(t *testing.T) { network: network, expected: "Invalid value: \"192.168.128.1\": \"192.168.128.1\" is not in the provisioning network", }, + { + name: "duplicate_bmc_address", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + Name: "host1", + BootMACAddress: "CA:FE:CA:FE:00:00", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.1", + }, + }, + { + Name: "host2", + BootMACAddress: "CA:FE:CA:FE:00:01", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.1", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[1\\].BMC.Address: Duplicate value: \"ipmi://192.168.111.1\"", + }, + { + name: "bmc_address_required", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + Name: "host1", + BootMACAddress: "CA:FE:CA:FE:00:00", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[0\\].BMC.Address: Required value: missing Address", + }, + { + name: "bmc_username_required", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + Name: "host1", + BootMACAddress: "CA:FE:CA:FE:00:00", + BMC: baremetal.BMC{ + Password: "password", + Address: "ipmi://192.168.111.1", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[0\\].BMC.Username: Required value: missing Username", + }, + { + name: "bmc_password_required", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + Name: "host1", + BootMACAddress: "CA:FE:CA:FE:00:00", + BMC: baremetal.BMC{ + Username: "root", + Address: "ipmi://192.168.111.1", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[0\\].BMC.Password: Required value: missing Password", + }, + { + name: "duplicate_host_name", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + Name: "host1", + BootMACAddress: "CA:FE:CA:FE:00:00", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.1", + }, + }, + { + Name: "host1", + BootMACAddress: "CA:FE:CA:FE:00:01", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.2", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[1\\].Name: Duplicate value: \"host1\"", + }, + { + name: "duplicate_host_mac", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + Name: "host1", + BootMACAddress: "CA:FE:CA:FE:CA:FE", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.1", + }, + }, + { + Name: "host2", + BootMACAddress: "CA:FE:CA:FE:CA:FE", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.2", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[1\\].BootMACAddress: Duplicate value: \"CA:FE:CA:FE:CA:FE\"", + }, + { + name: "missing_name", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + BootMACAddress: "CA:FE:CA:FE:CA:FE", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.1", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[0\\].Name: Required value: missing Name", + }, + { + name: "missing_mac", + platform: &baremetal.Platform{ + APIVIP: "192.168.111.2", + DNSVIP: "192.168.111.3", + IngressVIP: "192.168.111.4", + Hosts: []*baremetal.Host{ + { + Name: "host1", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.1", + }, + }, + }, + LibvirtURI: "qemu://system", + ProvisioningNetworkCIDR: ipnet.MustParseCIDR("172.22.0.0/24"), + ClusterProvisioningIP: "172.22.0.3", + BootstrapProvisioningIP: "172.22.0.2", + ExternalBridge: "br0", + ProvisioningBridge: "br1", + ProvisioningNetworkInterface: "ens3", + }, + network: network, + expected: "baremetal.hosts\\[0\\].BootMACAddress: Required value: missing BootMACAddress", + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - err := ValidatePlatform(tc.platform, tc.network, field.NewPath("test-path")).ToAggregate() + err := ValidatePlatform(tc.platform, tc.network, field.NewPath("baremetal")).ToAggregate() if tc.expected == "" { assert.NoError(t, err) } else {