From 5a60919ff318d6b0e1fe9314b92eebe337442181 Mon Sep 17 00:00:00 2001 From: Fabricio Aguiar Date: Wed, 28 Aug 2024 16:02:07 +0100 Subject: [PATCH] Validate nodeset networks order closes OSPRH-9455 Signed-off-by: Fabricio Aguiar --- apis/dataplane/v1beta1/const.go | 22 ++++++++++++++++ .../openstackdataplanenodeset_types.go | 26 +++++++++++++++++++ .../openstackdataplanenodeset_webhook.go | 2 ++ pkg/dataplane/baremetal.go | 4 +-- pkg/dataplane/cert.go | 2 +- pkg/dataplane/const.go | 4 --- pkg/dataplane/inventory.go | 6 ++--- pkg/dataplane/ipam.go | 4 +-- 8 files changed, 58 insertions(+), 12 deletions(-) create mode 100644 apis/dataplane/v1beta1/const.go diff --git a/apis/dataplane/v1beta1/const.go b/apis/dataplane/v1beta1/const.go new file mode 100644 index 000000000..c3c69bb0b --- /dev/null +++ b/apis/dataplane/v1beta1/const.go @@ -0,0 +1,22 @@ +/* +Copyright 2023. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1beta1 + +const ( + // CtlPlaneNetwork - default ctlplane Network Name in NetConfig + CtlPlaneNetwork = "ctlplane" +) diff --git a/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go b/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go index ff12ce94f..210001b1f 100644 --- a/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go +++ b/apis/dataplane/v1beta1/openstackdataplanenodeset_types.go @@ -347,3 +347,29 @@ func (r *OpenStackDataPlaneNodeSetSpec) TLSMatch(controlPlane openstackv1.OpenSt } return nil } + +// Validate NodeSet networks +func (r *OpenStackDataPlaneNodeSetSpec) ValidateNetworks() (errors field.ErrorList) { + for _, node := range r.Nodes { + if len(node.Networks) > 0 && node.Networks[0].Name != CtlPlaneNetwork { + errors = append(errors, field.Invalid( + field.NewPath("spec").Child("nodes"), + node.Networks, + fmt.Sprintf( + "node %s error: networks should start with %s got %s instead", + node.HostName, CtlPlaneNetwork, node.Networks[0].Name, + ))) + } + } + if len(r.NodeTemplate.Networks) > 0 && r.NodeTemplate.Networks[0].Name != CtlPlaneNetwork { + errors = append(errors, field.Invalid( + field.NewPath("spec").Child("nodeTemplate"), + r.NodeTemplate.Networks, + fmt.Sprintf( + "networks should start with %s got %s instead", + CtlPlaneNetwork, r.NodeTemplate.Networks[0].Name, + ))) + } + + return errors +} diff --git a/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go b/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go index 6104b0e3b..c24ffa87f 100644 --- a/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go +++ b/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go @@ -114,6 +114,7 @@ func (r *OpenStackDataPlaneNodeSet) ValidateCreate() (admission.Warnings, error) if err != nil { return nil, err } + errors = append(errors, r.Spec.ValidateNetworks()...) // Check if OpenStackDataPlaneNodeSet name matches RFC1123 for use in labels validate := validator.New() @@ -184,6 +185,7 @@ func (r *OpenStackDataPlaneNodeSet) ValidateUpdate(old runtime.Object) (admissio return nil, err } + errors = append(errors, r.Spec.ValidateNetworks()...) errors = append(errors, r.Spec.ValidateUpdate(&oldNodeSet.Spec)...) if errors != nil { diff --git a/pkg/dataplane/baremetal.go b/pkg/dataplane/baremetal.go index a91d5d47b..9781c47fe 100644 --- a/pkg/dataplane/baremetal.go +++ b/pkg/dataplane/baremetal.go @@ -77,7 +77,7 @@ func DeployBaremetalSet( instanceSpec.CtlPlaneIP = fmt.Sprintf("%s/24", node.Ansible.AnsibleHost) } else { for _, res := range ipSet.Status.Reservation { - if strings.ToLower(string(res.Network)) == CtlPlaneNetwork { + if strings.ToLower(string(res.Network)) == dataplanev1.CtlPlaneNetwork { _, ipNet, err := net.ParseCIDR(res.Cidr) if err != nil { return err @@ -85,7 +85,7 @@ func DeployBaremetalSet( ipPrefix, _ := ipNet.Mask.Size() instanceSpec.CtlPlaneIP = fmt.Sprintf("%s/%d", res.Address, ipPrefix) if res.Gateway == nil { - return fmt.Errorf("%s gateway is missing", CtlPlaneNetwork) + return fmt.Errorf("%s gateway is missing", dataplanev1.CtlPlaneNetwork) } baremetalSet.Spec.CtlplaneGateway = *res.Gateway baremetalSet.Spec.BootstrapDNS = dnsAddresses diff --git a/pkg/dataplane/cert.go b/pkg/dataplane/cert.go index 3ef43257f..1b3f49107 100644 --- a/pkg/dataplane/cert.go +++ b/pkg/dataplane/cert.go @@ -164,7 +164,7 @@ func EnsureTLSCerts(ctx context.Context, helper *helper.Helper, // NOTE: we are assuming that there will always be a ctlplane network // that means if you are not using network isolation with multiple networks // you should still need to have a ctlplane network at a minimum to use tls-e - baseName, ok := dnsNames[CtlPlaneNetwork] + baseName, ok := dnsNames[dataplanev1.CtlPlaneNetwork] if !ok { return &result, fmt.Errorf( "control plane network not found for node %s , tls-e requires a control plane network to be present", diff --git a/pkg/dataplane/const.go b/pkg/dataplane/const.go index 79f24c64a..85973445d 100644 --- a/pkg/dataplane/const.go +++ b/pkg/dataplane/const.go @@ -17,10 +17,6 @@ limitations under the License. package deployment const ( - - // CtlPlaneNetwork - default ctlplane Network Name in NetConfig - CtlPlaneNetwork = "ctlplane" - // ValidateNetworkLabel for ValidateNetwork OpenStackAnsibleEE ValidateNetworkLabel = "validate-network" diff --git a/pkg/dataplane/inventory.go b/pkg/dataplane/inventory.go index 44b62bfaa..b8a4fecab 100644 --- a/pkg/dataplane/inventory.go +++ b/pkg/dataplane/inventory.go @@ -213,14 +213,14 @@ func populateInventoryFromIPAM( netCidr, _ := ipnet.Mask.Size() host.Vars[entry+"_cidr"] = netCidr } - if res.Vlan != nil || entry != CtlPlaneNetwork { + if res.Vlan != nil || entry != dataplanev1.CtlPlaneNetwork { host.Vars[entry+"_vlan_id"] = res.Vlan } host.Vars[entry+"_mtu"] = res.MTU host.Vars[entry+"_gateway_ip"] = res.Gateway host.Vars[entry+"_host_routes"] = res.Routes - if entry == CtlPlaneNetwork { + if entry == dataplanev1.CtlPlaneNetwork { host.Vars[entry+"_dns_nameservers"] = dnsAddresses if dataplanev1.NodeHostNameIsFQDN(hostName) { host.Vars["canonical_hostname"] = hostName @@ -355,7 +355,7 @@ func buildNetworkVars(networks []infranetworkv1.IPSetNetwork) ([]string, map[str var nets []string for _, network := range networks { netName := string(network.Name) - if strings.EqualFold(netName, CtlPlaneNetwork) { + if strings.EqualFold(netName, dataplanev1.CtlPlaneNetwork) { continue } nets = append(nets, netName) diff --git a/pkg/dataplane/ipam.go b/pkg/dataplane/ipam.go index f4493f67a..686d2ddb4 100644 --- a/pkg/dataplane/ipam.go +++ b/pkg/dataplane/ipam.go @@ -128,7 +128,7 @@ func createOrPatchDNSData(ctx context.Context, helper *helper.Helper, fqdnNames = append(fqdnNames, fqdnName) dnsDetails.Hostnames[hostName][infranetworkv1.NetNameStr(netLower)] = fqdnName } - if dataplanev1.NodeHostNameIsFQDN(hostName) && netLower == CtlPlaneNetwork { + if dataplanev1.NodeHostNameIsFQDN(hostName) && netLower == dataplanev1.CtlPlaneNetwork { fqdnNames = append(fqdnNames, hostName) dnsDetails.Hostnames[hostName][infranetworkv1.NetNameStr(netLower)] = hostName } @@ -137,7 +137,7 @@ func createOrPatchDNSData(ctx context.Context, helper *helper.Helper, allDNSRecords = append(allDNSRecords, dnsRecord) // Adding only ctlplane domain for ansibleee. // TODO (rabi) This is not very efficient. - if netLower == CtlPlaneNetwork && ctlplaneSearchDomain == "" { + if netLower == dataplanev1.CtlPlaneNetwork && ctlplaneSearchDomain == "" { dnsDetails.CtlplaneSearchDomain = res.DNSDomain } }