From 8934598ea5eebcda8b0feda46d3c5328454e060c Mon Sep 17 00:00:00 2001 From: Lennart Jern Date: Mon, 4 Jul 2022 09:38:17 +0300 Subject: [PATCH] =?UTF-8?q?=E2=9A=A0=20Remove=20PortOpts.SecurityGroupFilt?= =?UTF-8?q?ers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This turns PortOpts.SecurityGroups in v1alpha6 into *[]SecurityGroupParam instead of *[]string and removes the SecurityGroupFilters field which would contain the same information otherwise. Co-authored-by: Anwar Hassen --- api/v1alpha4/conversion_test.go | 2 +- api/v1alpha4/zz_generated.conversion.go | 21 +- api/v1alpha5/conversion.go | 35 +++ api/v1alpha5/zz_generated.conversion.go | 240 +++++++++++++++--- api/v1alpha6/conversion.go | 20 ++ api/v1alpha6/types.go | 9 +- api/v1alpha6/zz_generated.deepcopy.go | 9 +- ...re.cluster.x-k8s.io_openstackclusters.yaml | 44 +--- ...er.x-k8s.io_openstackclustertemplates.yaml | 13 +- ...re.cluster.x-k8s.io_openstackmachines.yaml | 13 +- ...er.x-k8s.io_openstackmachinetemplates.yaml | 11 +- pkg/cloud/services/networking/port.go | 50 +--- pkg/cloud/services/networking/port_test.go | 16 +- .../services/networking/securitygroups.go | 7 +- test/e2e/shared/openstack.go | 28 +- test/e2e/suites/e2e/e2e_test.go | 13 +- 16 files changed, 354 insertions(+), 177 deletions(-) diff --git a/api/v1alpha4/conversion_test.go b/api/v1alpha4/conversion_test.go index b068edde13..083d340ae0 100644 --- a/api/v1alpha4/conversion_test.go +++ b/api/v1alpha4/conversion_test.go @@ -307,7 +307,7 @@ func TestFuzzyConversion(t *testing.T) { v1alpha6PortOpts.Network = nil } } - v1alpha6PortOpts.SecurityGroupFilters = nil + v1alpha6PortOpts.SecurityGroups = nil }, func(v1alpha6FixedIP *infrav1.FixedIP, c fuzz.Continue) { c.FuzzNoCustom(v1alpha6FixedIP) diff --git a/api/v1alpha4/zz_generated.conversion.go b/api/v1alpha4/zz_generated.conversion.go index 43f09203a6..9f929a0078 100644 --- a/api/v1alpha4/zz_generated.conversion.go +++ b/api/v1alpha4/zz_generated.conversion.go @@ -1474,7 +1474,15 @@ func autoConvert_v1alpha4_PortOpts_To_v1alpha6_PortOpts(in *PortOpts, out *v1alp } out.TenantID = in.TenantID out.ProjectID = in.ProjectID - out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) + if in.SecurityGroups != nil { + in, out := &in.SecurityGroups, &out.SecurityGroups + *out = new([]v1alpha6.SecurityGroupParam) + if err := v1alpha6.Convert_Slice_string_To_Slice_v1alpha6_SecurityGroupParam(*in, *out, s); err != nil { + return err + } + } else { + out.SecurityGroups = nil + } out.AllowedAddressPairs = *(*[]v1alpha6.AddressPair)(unsafe.Pointer(&in.AllowedAddressPairs)) out.Trunk = (*bool)(unsafe.Pointer(in.Trunk)) out.HostID = in.HostID @@ -1504,8 +1512,15 @@ func autoConvert_v1alpha6_PortOpts_To_v1alpha4_PortOpts(in *v1alpha6.PortOpts, o } out.TenantID = in.TenantID out.ProjectID = in.ProjectID - out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) - // WARNING: in.SecurityGroupFilters requires manual conversion: does not exist in peer-type + if in.SecurityGroups != nil { + in, out := &in.SecurityGroups, &out.SecurityGroups + *out = new([]string) + if err := v1alpha6.Convert_Slice_v1alpha6_SecurityGroupParam_To_Slice_string(*in, *out, s); err != nil { + return err + } + } else { + out.SecurityGroups = nil + } out.AllowedAddressPairs = *(*[]AddressPair)(unsafe.Pointer(&in.AllowedAddressPairs)) out.Trunk = (*bool)(unsafe.Pointer(in.Trunk)) out.HostID = in.HostID diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index 3e3eb3bd73..a8c5c06ad2 100644 --- a/api/v1alpha5/conversion.go +++ b/api/v1alpha5/conversion.go @@ -202,3 +202,38 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in * // Our new flag has no equivalent in v1alpha5 return autoConvert_v1alpha6_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in, out, s) } + +func Convert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(in *PortOpts, out *infrav1.PortOpts, s conversion.Scope) error { + err := autoConvert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(in, out, s) + if err != nil { + return err + } + // SecurityGroupFilters were removed in v1alpha6. It is included in the SecurityGroups field in v1alpha6. + if in.SecurityGroupFilters != nil { + for _, v1alpha5SecurityGroupParam := range in.SecurityGroupFilters { + *out.SecurityGroups = append(*out.SecurityGroups, infrav1.SecurityGroupParam{UUID: v1alpha5SecurityGroupParam.UUID}) + } + } + return nil +} + +func Convert_Slice_v1alpha5_Network_To_Slice_v1alpha6_Network(in *[]Network, out *[]infrav1.Network, s conversion.Scope) error { + *out = make([]infrav1.Network, len(*in)) + for i := range *in { + if err := Convert_v1alpha5_Network_To_v1alpha6_Network(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + + return nil +} + +func Convert_Slice_v1alpha6_Network_To_Slice_v1alpha5_Network(in *[]infrav1.Network, out *[]Network, s conversion.Scope) error { + *out = make([]Network, len(*in)) + for i := range *in { + if err := Convert_v1alpha6_Network_To_v1alpha5_Network(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + return nil +} diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index 13121335fd..5fb4b9dd45 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -304,11 +304,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*PortOpts)(nil), (*v1alpha6.PortOpts)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(a.(*PortOpts), b.(*v1alpha6.PortOpts), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*v1alpha6.PortOpts)(nil), (*PortOpts)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha6_PortOpts_To_v1alpha5_PortOpts(a.(*v1alpha6.PortOpts), b.(*PortOpts), scope) }); err != nil { @@ -404,6 +399,21 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*[]Network)(nil), (*[]v1alpha6.Network)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_v1alpha5_Network_To_Slice_v1alpha6_Network(a.(*[]Network), b.(*[]v1alpha6.Network), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*[]v1alpha6.Network)(nil), (*[]Network)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_Slice_v1alpha6_Network_To_Slice_v1alpha5_Network(a.(*[]v1alpha6.Network), b.(*[]Network), scope) + }); err != nil { + return err + } + if err := s.AddConversionFunc((*PortOpts)(nil), (*v1alpha6.PortOpts)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(a.(*PortOpts), b.(*v1alpha6.PortOpts), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha6.OpenStackClusterSpec)(nil), (*OpenStackClusterSpec)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha6_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(a.(*v1alpha6.OpenStackClusterSpec), b.(*OpenStackClusterSpec), scope) }); err != nil { @@ -540,7 +550,15 @@ func autoConvert_v1alpha5_Instance_To_v1alpha6_Instance(in *Instance, out *v1alp out.Trunk = in.Trunk out.FailureDomain = in.FailureDomain out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) - out.Networks = (*[]v1alpha6.Network)(unsafe.Pointer(in.Networks)) + if in.Networks != nil { + in, out := &in.Networks, &out.Networks + *out = new([]v1alpha6.Network) + if err := Convert_Slice_v1alpha5_Network_To_Slice_v1alpha6_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Networks = nil + } out.Subnet = in.Subnet out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.Image = in.Image @@ -569,7 +587,15 @@ func autoConvert_v1alpha6_Instance_To_v1alpha5_Instance(in *v1alpha6.Instance, o out.Trunk = in.Trunk out.FailureDomain = in.FailureDomain out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) - out.Networks = (*[]Network)(unsafe.Pointer(in.Networks)) + if in.Networks != nil { + in, out := &in.Networks, &out.Networks + *out = new([]Network) + if err := Convert_Slice_v1alpha6_Network_To_Slice_v1alpha5_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Networks = nil + } out.Subnet = in.Subnet out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.Image = in.Image @@ -625,7 +651,15 @@ func autoConvert_v1alpha5_Network_To_v1alpha6_Network(in *Network, out *v1alpha6 out.ID = in.ID out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.Subnet = (*v1alpha6.Subnet)(unsafe.Pointer(in.Subnet)) - out.PortOpts = (*v1alpha6.PortOpts)(unsafe.Pointer(in.PortOpts)) + if in.PortOpts != nil { + in, out := &in.PortOpts, &out.PortOpts + *out = new(v1alpha6.PortOpts) + if err := Convert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(*in, *out, s); err != nil { + return err + } + } else { + out.PortOpts = nil + } out.Router = (*v1alpha6.Router)(unsafe.Pointer(in.Router)) out.APIServerLoadBalancer = (*v1alpha6.LoadBalancer)(unsafe.Pointer(in.APIServerLoadBalancer)) return nil @@ -641,7 +675,15 @@ func autoConvert_v1alpha6_Network_To_v1alpha5_Network(in *v1alpha6.Network, out out.ID = in.ID out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.Subnet = (*Subnet)(unsafe.Pointer(in.Subnet)) - out.PortOpts = (*PortOpts)(unsafe.Pointer(in.PortOpts)) + if in.PortOpts != nil { + in, out := &in.PortOpts, &out.PortOpts + *out = new(PortOpts) + if err := Convert_v1alpha6_PortOpts_To_v1alpha5_PortOpts(*in, *out, s); err != nil { + return err + } + } else { + out.PortOpts = nil + } out.Router = (*Router)(unsafe.Pointer(in.Router)) out.APIServerLoadBalancer = (*LoadBalancer)(unsafe.Pointer(in.APIServerLoadBalancer)) return nil @@ -815,7 +857,15 @@ func autoConvert_v1alpha5_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec( out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.ControlPlaneEndpoint = in.ControlPlaneEndpoint out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) - out.Bastion = (*v1alpha6.Bastion)(unsafe.Pointer(in.Bastion)) + if in.Bastion != nil { + in, out := &in.Bastion, &out.Bastion + *out = new(v1alpha6.Bastion) + if err := Convert_v1alpha5_Bastion_To_v1alpha6_Bastion(*in, *out, s); err != nil { + return err + } + } else { + out.Bastion = nil + } out.IdentityRef = (*v1alpha6.OpenStackIdentityReference)(unsafe.Pointer(in.IdentityRef)) return nil } @@ -851,20 +901,52 @@ func autoConvert_v1alpha6_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec( out.ControlPlaneEndpoint = in.ControlPlaneEndpoint out.ControlPlaneAvailabilityZones = *(*[]string)(unsafe.Pointer(&in.ControlPlaneAvailabilityZones)) // WARNING: in.ControlPlaneOmitAvailabilityZone requires manual conversion: does not exist in peer-type - out.Bastion = (*Bastion)(unsafe.Pointer(in.Bastion)) + if in.Bastion != nil { + in, out := &in.Bastion, &out.Bastion + *out = new(Bastion) + if err := Convert_v1alpha6_Bastion_To_v1alpha5_Bastion(*in, *out, s); err != nil { + return err + } + } else { + out.Bastion = nil + } out.IdentityRef = (*OpenStackIdentityReference)(unsafe.Pointer(in.IdentityRef)) return nil } func autoConvert_v1alpha5_OpenStackClusterStatus_To_v1alpha6_OpenStackClusterStatus(in *OpenStackClusterStatus, out *v1alpha6.OpenStackClusterStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Network = (*v1alpha6.Network)(unsafe.Pointer(in.Network)) - out.ExternalNetwork = (*v1alpha6.Network)(unsafe.Pointer(in.ExternalNetwork)) + if in.Network != nil { + in, out := &in.Network, &out.Network + *out = new(v1alpha6.Network) + if err := Convert_v1alpha5_Network_To_v1alpha6_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Network = nil + } + if in.ExternalNetwork != nil { + in, out := &in.ExternalNetwork, &out.ExternalNetwork + *out = new(v1alpha6.Network) + if err := Convert_v1alpha5_Network_To_v1alpha6_Network(*in, *out, s); err != nil { + return err + } + } else { + out.ExternalNetwork = nil + } out.FailureDomains = *(*v1beta1.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) out.ControlPlaneSecurityGroup = (*v1alpha6.SecurityGroup)(unsafe.Pointer(in.ControlPlaneSecurityGroup)) out.WorkerSecurityGroup = (*v1alpha6.SecurityGroup)(unsafe.Pointer(in.WorkerSecurityGroup)) out.BastionSecurityGroup = (*v1alpha6.SecurityGroup)(unsafe.Pointer(in.BastionSecurityGroup)) - out.Bastion = (*v1alpha6.Instance)(unsafe.Pointer(in.Bastion)) + if in.Bastion != nil { + in, out := &in.Bastion, &out.Bastion + *out = new(v1alpha6.Instance) + if err := Convert_v1alpha5_Instance_To_v1alpha6_Instance(*in, *out, s); err != nil { + return err + } + } else { + out.Bastion = nil + } out.FailureReason = (*errors.ClusterStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) return nil @@ -877,13 +959,37 @@ func Convert_v1alpha5_OpenStackClusterStatus_To_v1alpha6_OpenStackClusterStatus( func autoConvert_v1alpha6_OpenStackClusterStatus_To_v1alpha5_OpenStackClusterStatus(in *v1alpha6.OpenStackClusterStatus, out *OpenStackClusterStatus, s conversion.Scope) error { out.Ready = in.Ready - out.Network = (*Network)(unsafe.Pointer(in.Network)) - out.ExternalNetwork = (*Network)(unsafe.Pointer(in.ExternalNetwork)) + if in.Network != nil { + in, out := &in.Network, &out.Network + *out = new(Network) + if err := Convert_v1alpha6_Network_To_v1alpha5_Network(*in, *out, s); err != nil { + return err + } + } else { + out.Network = nil + } + if in.ExternalNetwork != nil { + in, out := &in.ExternalNetwork, &out.ExternalNetwork + *out = new(Network) + if err := Convert_v1alpha6_Network_To_v1alpha5_Network(*in, *out, s); err != nil { + return err + } + } else { + out.ExternalNetwork = nil + } out.FailureDomains = *(*v1beta1.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) out.ControlPlaneSecurityGroup = (*SecurityGroup)(unsafe.Pointer(in.ControlPlaneSecurityGroup)) out.WorkerSecurityGroup = (*SecurityGroup)(unsafe.Pointer(in.WorkerSecurityGroup)) out.BastionSecurityGroup = (*SecurityGroup)(unsafe.Pointer(in.BastionSecurityGroup)) - out.Bastion = (*Instance)(unsafe.Pointer(in.Bastion)) + if in.Bastion != nil { + in, out := &in.Bastion, &out.Bastion + *out = new(Instance) + if err := Convert_v1alpha6_Instance_To_v1alpha5_Instance(*in, *out, s); err != nil { + return err + } + } else { + out.Bastion = nil + } out.FailureReason = (*errors.ClusterStatusError)(unsafe.Pointer(in.FailureReason)) out.FailureMessage = (*string)(unsafe.Pointer(in.FailureMessage)) return nil @@ -1066,7 +1172,17 @@ func Convert_v1alpha6_OpenStackMachine_To_v1alpha5_OpenStackMachine(in *v1alpha6 func autoConvert_v1alpha5_OpenStackMachineList_To_v1alpha6_OpenStackMachineList(in *OpenStackMachineList, out *v1alpha6.OpenStackMachineList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1alpha6.OpenStackMachine)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1alpha6.OpenStackMachine, len(*in)) + for i := range *in { + if err := Convert_v1alpha5_OpenStackMachine_To_v1alpha6_OpenStackMachine(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1077,7 +1193,17 @@ func Convert_v1alpha5_OpenStackMachineList_To_v1alpha6_OpenStackMachineList(in * func autoConvert_v1alpha6_OpenStackMachineList_To_v1alpha5_OpenStackMachineList(in *v1alpha6.OpenStackMachineList, out *OpenStackMachineList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]OpenStackMachine)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]OpenStackMachine, len(*in)) + for i := range *in { + if err := Convert_v1alpha6_OpenStackMachine_To_v1alpha5_OpenStackMachine(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1095,7 +1221,17 @@ func autoConvert_v1alpha5_OpenStackMachineSpec_To_v1alpha6_OpenStackMachineSpec( out.ImageUUID = in.ImageUUID out.SSHKeyName = in.SSHKeyName out.Networks = *(*[]v1alpha6.NetworkParam)(unsafe.Pointer(&in.Networks)) - out.Ports = *(*[]v1alpha6.PortOpts)(unsafe.Pointer(&in.Ports)) + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]v1alpha6.PortOpts, len(*in)) + for i := range *in { + if err := Convert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Ports = nil + } out.Subnet = in.Subnet out.FloatingIP = in.FloatingIP out.SecurityGroups = *(*[]v1alpha6.SecurityGroupParam)(unsafe.Pointer(&in.SecurityGroups)) @@ -1123,7 +1259,17 @@ func autoConvert_v1alpha6_OpenStackMachineSpec_To_v1alpha5_OpenStackMachineSpec( out.ImageUUID = in.ImageUUID out.SSHKeyName = in.SSHKeyName out.Networks = *(*[]NetworkParam)(unsafe.Pointer(&in.Networks)) - out.Ports = *(*[]PortOpts)(unsafe.Pointer(&in.Ports)) + if in.Ports != nil { + in, out := &in.Ports, &out.Ports + *out = make([]PortOpts, len(*in)) + for i := range *in { + if err := Convert_v1alpha6_PortOpts_To_v1alpha5_PortOpts(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Ports = nil + } out.Subnet = in.Subnet out.FloatingIP = in.FloatingIP out.SecurityGroups = *(*[]SecurityGroupParam)(unsafe.Pointer(&in.SecurityGroups)) @@ -1200,7 +1346,17 @@ func Convert_v1alpha6_OpenStackMachineTemplate_To_v1alpha5_OpenStackMachineTempl func autoConvert_v1alpha5_OpenStackMachineTemplateList_To_v1alpha6_OpenStackMachineTemplateList(in *OpenStackMachineTemplateList, out *v1alpha6.OpenStackMachineTemplateList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]v1alpha6.OpenStackMachineTemplate)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]v1alpha6.OpenStackMachineTemplate, len(*in)) + for i := range *in { + if err := Convert_v1alpha5_OpenStackMachineTemplate_To_v1alpha6_OpenStackMachineTemplate(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1211,7 +1367,17 @@ func Convert_v1alpha5_OpenStackMachineTemplateList_To_v1alpha6_OpenStackMachineT func autoConvert_v1alpha6_OpenStackMachineTemplateList_To_v1alpha5_OpenStackMachineTemplateList(in *v1alpha6.OpenStackMachineTemplateList, out *OpenStackMachineTemplateList, s conversion.Scope) error { out.ListMeta = in.ListMeta - out.Items = *(*[]OpenStackMachineTemplate)(unsafe.Pointer(&in.Items)) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]OpenStackMachineTemplate, len(*in)) + for i := range *in { + if err := Convert_v1alpha6_OpenStackMachineTemplate_To_v1alpha5_OpenStackMachineTemplate(&(*in)[i], &(*out)[i], s); err != nil { + return err + } + } + } else { + out.Items = nil + } return nil } @@ -1277,8 +1443,16 @@ func autoConvert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(in *PortOpts, out *v1alp out.FixedIPs = *(*[]v1alpha6.FixedIP)(unsafe.Pointer(&in.FixedIPs)) out.TenantID = in.TenantID out.ProjectID = in.ProjectID - out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) - out.SecurityGroupFilters = *(*[]v1alpha6.SecurityGroupParam)(unsafe.Pointer(&in.SecurityGroupFilters)) + if in.SecurityGroups != nil { + in, out := &in.SecurityGroups, &out.SecurityGroups + *out = new([]v1alpha6.SecurityGroupParam) + if err := v1alpha6.Convert_Slice_string_To_Slice_v1alpha6_SecurityGroupParam(*in, *out, s); err != nil { + return err + } + } else { + out.SecurityGroups = nil + } + // WARNING: in.SecurityGroupFilters requires manual conversion: does not exist in peer-type out.AllowedAddressPairs = *(*[]v1alpha6.AddressPair)(unsafe.Pointer(&in.AllowedAddressPairs)) out.Trunk = (*bool)(unsafe.Pointer(in.Trunk)) out.HostID = in.HostID @@ -1289,11 +1463,6 @@ func autoConvert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(in *PortOpts, out *v1alp return nil } -// Convert_v1alpha5_PortOpts_To_v1alpha6_PortOpts is an autogenerated conversion function. -func Convert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(in *PortOpts, out *v1alpha6.PortOpts, s conversion.Scope) error { - return autoConvert_v1alpha5_PortOpts_To_v1alpha6_PortOpts(in, out, s) -} - func autoConvert_v1alpha6_PortOpts_To_v1alpha5_PortOpts(in *v1alpha6.PortOpts, out *PortOpts, s conversion.Scope) error { out.Network = (*NetworkFilter)(unsafe.Pointer(in.Network)) out.NameSuffix = in.NameSuffix @@ -1303,8 +1472,15 @@ func autoConvert_v1alpha6_PortOpts_To_v1alpha5_PortOpts(in *v1alpha6.PortOpts, o out.FixedIPs = *(*[]FixedIP)(unsafe.Pointer(&in.FixedIPs)) out.TenantID = in.TenantID out.ProjectID = in.ProjectID - out.SecurityGroups = (*[]string)(unsafe.Pointer(in.SecurityGroups)) - out.SecurityGroupFilters = *(*[]SecurityGroupParam)(unsafe.Pointer(&in.SecurityGroupFilters)) + if in.SecurityGroups != nil { + in, out := &in.SecurityGroups, &out.SecurityGroups + *out = new([]string) + if err := v1alpha6.Convert_Slice_v1alpha6_SecurityGroupParam_To_Slice_string(*in, *out, s); err != nil { + return err + } + } else { + out.SecurityGroups = nil + } out.AllowedAddressPairs = *(*[]AddressPair)(unsafe.Pointer(&in.AllowedAddressPairs)) out.Trunk = (*bool)(unsafe.Pointer(in.Trunk)) out.HostID = in.HostID diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go index eec5d8d0fc..2c31183d2f 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -16,6 +16,8 @@ limitations under the License. package v1alpha6 +import "k8s.io/apimachinery/pkg/conversion" + // Hub marks OpenStackCluster as a conversion hub. func (*OpenStackCluster) Hub() {} @@ -39,3 +41,21 @@ func (*OpenStackMachineTemplate) Hub() {} // Hub marks OpenStackMachineTemplateList as a conversion hub. func (*OpenStackMachineTemplateList) Hub() {} + +func Convert_Slice_string_To_Slice_v1alpha6_SecurityGroupParam(in *[]string, out *[]SecurityGroupParam, s conversion.Scope) error { + if in != nil { + for _, v1alpha4SecurityGroupUID := range *in { + *out = append(*out, SecurityGroupParam{UUID: v1alpha4SecurityGroupUID}) + } + } + return nil +} + +func Convert_Slice_v1alpha6_SecurityGroupParam_To_Slice_string(in *[]SecurityGroupParam, out *[]string, s conversion.Scope) error { + if in != nil { + for _, v1alpha6SecurityGroups := range *in { + *out = append(*out, v1alpha6SecurityGroups.UUID) + } + } + return nil +} diff --git a/api/v1alpha6/types.go b/api/v1alpha6/types.go index 6e99a915bd..1ceb3312d3 100644 --- a/api/v1alpha6/types.go +++ b/api/v1alpha6/types.go @@ -117,12 +117,9 @@ type PortOpts struct { FixedIPs []FixedIP `json:"fixedIPs,omitempty"` TenantID string `json:"tenantId,omitempty"` ProjectID string `json:"projectId,omitempty"` - // The uuids of the security groups to assign to the instance - // +listType=set - SecurityGroups *[]string `json:"securityGroups,omitempty"` - // The names, uuids, filters or any combination these of the security groups to assign to the instance - SecurityGroupFilters []SecurityGroupParam `json:"securityGroupFilters,omitempty"` - AllowedAddressPairs []AddressPair `json:"allowedAddressPairs,omitempty"` + // The names, uuids, filters or any combination of these of the security groups to assign to the port + SecurityGroups *[]SecurityGroupParam `json:"securityGroups,omitempty"` + AllowedAddressPairs []AddressPair `json:"allowedAddressPairs,omitempty"` // Enables and disables trunk at port level. If not provided, openStackMachine.Spec.Trunk is inherited. Trunk *bool `json:"trunk,omitempty"` diff --git a/api/v1alpha6/zz_generated.deepcopy.go b/api/v1alpha6/zz_generated.deepcopy.go index add4e9a3ea..3af0ed7c13 100644 --- a/api/v1alpha6/zz_generated.deepcopy.go +++ b/api/v1alpha6/zz_generated.deepcopy.go @@ -832,18 +832,13 @@ func (in *PortOpts) DeepCopyInto(out *PortOpts) { } if in.SecurityGroups != nil { in, out := &in.SecurityGroups, &out.SecurityGroups - *out = new([]string) + *out = new([]SecurityGroupParam) if **in != nil { in, out := *in, *out - *out = make([]string, len(*in)) + *out = make([]SecurityGroupParam, len(*in)) copy(*out, *in) } } - if in.SecurityGroupFilters != nil { - in, out := &in.SecurityGroupFilters, &out.SecurityGroupFilters - *out = make([]SecurityGroupParam, len(*in)) - copy(*out, *in) - } if in.AllowedAddressPairs != nil { in, out := &in.AllowedAddressPairs, &out.AllowedAddressPairs *out = make([]AddressPair, len(*in)) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml index 59c90ce8ad..3c63de978b 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -4543,9 +4543,9 @@ spec: type: object projectId: type: string - securityGroupFilters: + securityGroups: description: The names, uuids, filters or any combination - these of the security groups to assign to the instance + of these of the security groups to assign to the port items: properties: filter: @@ -4587,13 +4587,6 @@ spec: type: string type: object type: array - securityGroups: - description: The uuids of the security groups to assign - to the instance - items: - type: string - type: array - x-kubernetes-list-type: set tags: description: Tags applied to the port (and corresponding trunk, if a trunk is configured.) These tags are applied @@ -5077,9 +5070,9 @@ spec: type: object projectId: type: string - securityGroupFilters: + securityGroups: description: The names, uuids, filters or any combination - these of the security groups to assign to the instance + of these of the security groups to assign to the port items: properties: filter: @@ -5121,13 +5114,6 @@ spec: type: string type: object type: array - securityGroups: - description: The uuids of the security groups to assign - to the instance - items: - type: string - type: array - x-kubernetes-list-type: set tags: description: Tags applied to the port (and corresponding trunk, if a trunk is configured.) These tags are applied @@ -5467,9 +5453,9 @@ spec: type: object projectId: type: string - securityGroupFilters: + securityGroups: description: The names, uuids, filters or any combination - these of the security groups to assign to the instance + of these of the security groups to assign to the port items: properties: filter: @@ -5511,13 +5497,6 @@ spec: type: string type: object type: array - securityGroups: - description: The uuids of the security groups to assign to - the instance - items: - type: string - type: array - x-kubernetes-list-type: set tags: description: Tags applied to the port (and corresponding trunk, if a trunk is configured.) These tags are applied in addition @@ -5771,9 +5750,9 @@ spec: type: object projectId: type: string - securityGroupFilters: + securityGroups: description: The names, uuids, filters or any combination - these of the security groups to assign to the instance + of these of the security groups to assign to the port items: properties: filter: @@ -5815,13 +5794,6 @@ spec: type: string type: object type: array - securityGroups: - description: The uuids of the security groups to assign to - the instance - items: - type: string - type: array - x-kubernetes-list-type: set tags: description: Tags applied to the port (and corresponding trunk, if a trunk is configured.) These tags are applied in addition diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml index e424193108..5407994949 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -1776,10 +1776,10 @@ spec: type: object projectId: type: string - securityGroupFilters: + securityGroups: description: The names, uuids, filters or any - combination these of the security groups to - assign to the instance + combination of these of the security groups + to assign to the port items: properties: filter: @@ -1821,13 +1821,6 @@ spec: type: string type: object type: array - securityGroups: - description: The uuids of the security groups - to assign to the instance - items: - type: string - type: array - x-kubernetes-list-type: set tags: description: Tags applied to the port (and corresponding trunk, if a trunk is configured.) These tags diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml index b7eef06399..c8e03427c5 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml @@ -1679,9 +1679,9 @@ spec: type: object projectId: type: string - securityGroupFilters: - description: The names, uuids, filters or any combination these - of the security groups to assign to the instance + securityGroups: + description: The names, uuids, filters or any combination of + these of the security groups to assign to the port items: properties: filter: @@ -1723,13 +1723,6 @@ spec: type: string type: object type: array - securityGroups: - description: The uuids of the security groups to assign to the - instance - items: - type: string - type: array - x-kubernetes-list-type: set tags: description: Tags applied to the port (and corresponding trunk, if a trunk is configured.) These tags are applied in addition diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml index c34cbbddac..fac50c65f6 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachinetemplates.yaml @@ -1389,9 +1389,9 @@ spec: type: object projectId: type: string - securityGroupFilters: + securityGroups: description: The names, uuids, filters or any combination - these of the security groups to assign to the instance + of these of the security groups to assign to the port items: properties: filter: @@ -1433,13 +1433,6 @@ spec: type: string type: object type: array - securityGroups: - description: The uuids of the security groups to assign - to the instance - items: - type: string - type: array - x-kubernetes-list-type: set tags: description: Tags applied to the port (and corresponding trunk, if a trunk is configured.) These tags are applied diff --git a/pkg/cloud/services/networking/port.go b/pkg/cloud/services/networking/port.go index 82c4b15282..92a76ccc0c 100644 --- a/pkg/cloud/services/networking/port.go +++ b/pkg/cloud/services/networking/port.go @@ -90,9 +90,15 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string MACAddress: ap.MACAddress, }) } - securityGroups, err = s.CollectPortSecurityGroups(eventObject, portOpts.SecurityGroups, portOpts.SecurityGroupFilters) - if err != nil { - return nil, err + portSecurityGroups := portOpts.SecurityGroups + if portSecurityGroups != nil { + portSecurityGroups, err := s.GetSecurityGroups(*portSecurityGroups) + if err != nil { + return nil, fmt.Errorf("error getting security groups: %v", err) + } + if len(portSecurityGroups) > 0 { + securityGroups = &portSecurityGroups + } } // inherit port security groups from the instance if not explicitly specified if securityGroups == nil || len(*securityGroups) == 0 { @@ -285,41 +291,3 @@ func (s *Service) GarbageCollectErrorInstancesPort(eventObject runtime.Object, i return nil } - -// CollectPortSecurityGroups collects distinct securityGroups from port.SecurityGroups and port.SecurityGroupFilter fields. -func (s *Service) CollectPortSecurityGroups(eventObject runtime.Object, portSecurityGroups *[]string, portSecurityGroupFilters []infrav1.SecurityGroupParam) (*[]string, error) { - var allSecurityGroupIDs []string - // security groups provided with the portSecurityGroupFilters fields - securityGroupFiltersByID, err := s.GetSecurityGroups(portSecurityGroupFilters) - if err != nil { - return portSecurityGroups, fmt.Errorf("error getting security groups: %v", err) - } - allSecurityGroupIDs = append(allSecurityGroupIDs, securityGroupFiltersByID...) - securityGroupCount := 0 - // security groups provided with the portSecurityGroups fields - if portSecurityGroups != nil { - allSecurityGroupIDs = append(allSecurityGroupIDs, *portSecurityGroups...) - } - // generate unique values - uids := make(map[string]int) - for _, sg := range allSecurityGroupIDs { - if sg == "" { - continue - } - // count distinct values - _, ok := uids[sg] - if !ok { - securityGroupCount++ - } - uids[sg] = 1 - } - distinctSecurityGroupIDs := make([]string, 0, securityGroupCount) - // collect distict values - for key := range uids { - if key == "" { - continue - } - distinctSecurityGroupIDs = append(distinctSecurityGroupIDs, key) - } - return &distinctSecurityGroupIDs, nil -} diff --git a/pkg/cloud/services/networking/port_test.go b/pkg/cloud/services/networking/port_test.go index 554b970580..d6e853a91b 100644 --- a/pkg/cloud/services/networking/port_test.go +++ b/pkg/cloud/services/networking/port_test.go @@ -46,11 +46,11 @@ func Test_GetOrCreatePort(t *testing.T) { tenantID := "62b523a7-f838-45fd-904f-d2db2bb58e04" projectID := "063171b1-0595-4882-98cd-3ee79676ff87" trunkID := "eb7541fa-5e2a-4cca-b2c3-dfa409b917ce" + portSecurityGroupID := "f51d1206-fc5a-4f7a-a5c0-2e03e44e4dc0" // Other arbitrary variables passed in to the tests instanceSecurityGroups := []string{"instance-secgroup"} - portSecurityGroups := []string{"port-secgroup"} - + portSecurityGroups := []infrav1.SecurityGroupParam{{UUID: portSecurityGroupID, Name: "port-secgroup"}} pointerToTrue := pointerTo(true) pointerToFalse := pointerTo(false) @@ -166,9 +166,8 @@ func Test_GetOrCreatePort(t *testing.T) { }, IPAddress: "192.168.0.50", }, {IPAddress: "192.168.1.50"}}, - TenantID: tenantID, - ProjectID: projectID, - SecurityGroups: &portSecurityGroups, + TenantID: tenantID, + ProjectID: projectID, AllowedAddressPairs: []infrav1.AddressPair{{ IPAddress: "10.10.10.10", MACAddress: "f1:f1:f1:f1:f1:f1", @@ -197,9 +196,8 @@ func Test_GetOrCreatePort(t *testing.T) { IPAddress: "192.168.1.50", }, }, - TenantID: tenantID, - ProjectID: projectID, - SecurityGroups: &portSecurityGroups, + TenantID: tenantID, + ProjectID: projectID, AllowedAddressPairs: []ports.AddressPair{{ IPAddress: "10.10.10.10", MACAddress: "f1:f1:f1:f1:f1:f1", @@ -313,7 +311,7 @@ func Test_GetOrCreatePort(t *testing.T) { CreateOptsBuilder: ports.CreateOpts{ Name: "foo-port-1", Description: "Created by cluster-api-provider-openstack cluster test-cluster", - SecurityGroups: &portSecurityGroups, + SecurityGroups: &[]string{portSecurityGroupID}, NetworkID: netID, AllowedAddressPairs: []ports.AddressPair{}, }, diff --git a/pkg/cloud/services/networking/securitygroups.go b/pkg/cloud/services/networking/securitygroups.go index e41a66f5cf..05ce4ef4b9 100644 --- a/pkg/cloud/services/networking/securitygroups.go +++ b/pkg/cloud/services/networking/securitygroups.go @@ -165,6 +165,7 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl return desiredSecGroups, nil } +// GetSecurityGroups finds and returns the IDs of all security groups (if any) matching the input. func (s *Service) GetSecurityGroups(securityGroupParams []infrav1.SecurityGroupParam) ([]string, error) { var sgIDs []string for _, sg := range securityGroupParams { @@ -187,11 +188,7 @@ func (s *Service) GetSecurityGroups(securityGroupParams []infrav1.SecurityGroupP if err != nil { return nil, err } - - if len(SGList) == 0 { - return nil, fmt.Errorf("security group %s not found", sg.Name) - } - + // Check for duplicates for _, group := range SGList { if isDuplicate(sgIDs, group.ID) { continue diff --git a/test/e2e/shared/openstack.go b/test/e2e/shared/openstack.go index fdbfb8c468..3fee38d014 100644 --- a/test/e2e/shared/openstack.go +++ b/test/e2e/shared/openstack.go @@ -372,17 +372,17 @@ func DumpOpenStackPorts(e2eCtx *E2EContext, filter ports.ListOpts) ([]ports.Port } // CreateOpenStackSecurityGroup creates a security group to be consumed by a worker node. -func CreateOpenStackSecurityGroup(e2eCtx *E2EContext, securityGroupName, description string) error { +func CreateOpenStackSecurityGroup(e2eCtx *E2EContext, securityGroupName, description string) (string, error) { providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx) if err != nil { - return fmt.Errorf("error creating provider client: %s", err) + return "", fmt.Errorf("error creating provider client: %s", err) } networkClient, err := openstack.NewNetworkV2(providerClient, gophercloud.EndpointOpts{ Region: clientOpts.RegionName, }) if err != nil { - return fmt.Errorf("error creating network client: %s", err) + return "", fmt.Errorf("error creating network client: %s", err) } createOpts := groups.CreateOpts{ @@ -390,7 +390,27 @@ func CreateOpenStackSecurityGroup(e2eCtx *E2EContext, securityGroupName, descrip Description: description, } - _, err = groups.Create(networkClient, createOpts).Extract() + group, err := groups.Create(networkClient, createOpts).Extract() + if err != nil { + return "", err + } + return group.ID, nil +} + +// DeleteOpenStackSecurityGroup deletes manually created security group. +func DeleteOpenStackSecurityGroup(e2eCtx *E2EContext, securityGroupID string) error { + providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx) + if err != nil { + return fmt.Errorf("error creating provider client: %s", err) + } + + networkClient, err := openstack.NewNetworkV2(providerClient, gophercloud.EndpointOpts{ + Region: clientOpts.RegionName, + }) + if err != nil { + return fmt.Errorf("error creating network client: %s", err) + } + err = groups.Delete(networkClient, securityGroupID).ExtractErr() if err != nil { return err } diff --git a/test/e2e/suites/e2e/e2e_test.go b/test/e2e/suites/e2e/e2e_test.go index 42a7461c51..b3b47ad1da 100644 --- a/test/e2e/suites/e2e/e2e_test.go +++ b/test/e2e/suites/e2e/e2e_test.go @@ -210,13 +210,19 @@ var _ = Describe("e2e tests", func() { shared.Byf("Creating MachineDeployment with custom port options") md3Name := clusterName + "-md-3" testSecurityGroupName := "testSecGroup" + var testSecurityGroupID string + var err error // create required test security group Eventually(func() int { - err := shared.CreateOpenStackSecurityGroup(e2eCtx, testSecurityGroupName, "Test security group") + testSecurityGroupID, err = shared.CreateOpenStackSecurityGroup(e2eCtx, testSecurityGroupName, "Test security group") Expect(err).To(BeNil()) return 1 }, e2eCtx.E2EConfig.GetIntervals(specName, "wait-worker-nodes")...).Should(Equal(1)) - + postClusterCleanup = append(postClusterCleanup, func() { + shared.Byf("Deleting security group %s", testSecurityGroupName) + err := shared.DeleteOpenStackSecurityGroup(e2eCtx, testSecurityGroupID) + Expect(err).NotTo(HaveOccurred()) + }) customPortOptions := &[]infrav1.PortOpts{ { Description: "primary", @@ -226,7 +232,7 @@ var _ = Describe("e2e tests", func() { Trunk: pointer.Bool(true), }, { - SecurityGroupFilters: []infrav1.SecurityGroupParam{{Name: testSecurityGroupName}}, + SecurityGroups: &[]infrav1.SecurityGroupParam{{Name: testSecurityGroupName}}, }, } @@ -244,7 +250,6 @@ var _ = Describe("e2e tests", func() { shared.Byf("Waiting for custom port to be created") var plist []ports.Port - var err error Eventually(func() int { plist, err = shared.DumpOpenStackPorts(e2eCtx, ports.ListOpts{Description: "primary", Tags: testTag}) Expect(err).To(BeNil())