From 418ce3d5e977f6c940917b206680c3380f47a942 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Mon, 15 Jan 2024 16:30:54 -0500 Subject: [PATCH] AllNodes security groups API Co-Authored-By: Emilien Macchi Co-Authored-By: Matthew Booth --- api/v1alpha5/conversion.go | 67 +++ api/v1alpha5/conversion_test.go | 51 +- api/v1alpha5/zz_generated.conversion.go | 156 +++-- api/v1alpha6/conversion.go | 101 ++++ api/v1alpha6/conversion_test.go | 47 ++ api/v1alpha6/zz_generated.conversion.go | 156 +++-- api/v1alpha7/conversion.go | 148 ++++- api/v1alpha7/conversion_test.go | 47 ++ api/v1alpha7/zz_generated.conversion.go | 156 +++-- api/v1alpha8/conversion.go | 28 + api/v1alpha8/openstackcluster_types.go | 25 +- api/v1alpha8/openstackcluster_webhook.go | 20 + api/v1alpha8/openstackcluster_webhook_test.go | 81 +++ api/v1alpha8/types.go | 151 ++++- api/v1alpha8/zz_generated.deepcopy.go | 155 ++++- ...re.cluster.x-k8s.io_openstackclusters.yaml | 224 ++++++-- ...er.x-k8s.io_openstackclustertemplates.yaml | 84 ++- controllers/openstackcluster_controller.go | 2 +- controllers/openstackmachine_controller.go | 2 +- .../openstackmachine_controller_test.go | 14 +- .../src/clusteropenstack/configuration.md | 75 ++- .../crd-changes/v1alpha7-to-v1alpha8.md | 49 +- .../v1alpha8/default/cluster-template.yaml | 21 +- pkg/cloud/services/compute/instance_test.go | 4 +- .../services/networking/securitygroups.go | 229 ++++++-- .../networking/securitygroups_rules.go | 130 +---- .../networking/securitygroups_test.go | 531 ++++++++++++++++++ .../cluster-template-flatcar-sysext.yaml | 21 +- templates/cluster-template-flatcar.yaml | 21 +- templates/cluster-template-without-lb.yaml | 21 +- templates/cluster-template.yaml | 21 +- templates/clusterclass-dev-test.yaml | 21 +- test/e2e/shared/openstack.go | 53 ++ test/e2e/suites/e2e/e2e_test.go | 9 + 34 files changed, 2348 insertions(+), 573 deletions(-) create mode 100644 pkg/cloud/services/networking/securitygroups_test.go diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index bfb8329cf4..4dfc78a140 100644 --- a/api/v1alpha5/conversion.go +++ b/api/v1alpha5/conversion.go @@ -20,6 +20,7 @@ import ( "strings" conversion "k8s.io/apimachinery/pkg/conversion" + "k8s.io/utils/pointer" utilconversion "sigs.k8s.io/cluster-api/util/conversion" ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion" @@ -209,6 +210,10 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in * } } + if in.ManagedSecurityGroups != nil { + out.ManagedSecurityGroups = true + } + return nil } @@ -243,6 +248,13 @@ func Convert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in * } // We're dropping DNSNameservers even if these were set as without NodeCIDR it doesn't make sense. + if in.ManagedSecurityGroups { + out.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} + if !in.AllowAllInClusterTraffic { + out.ManagedSecurityGroups.AllNodesSecurityGroupRules = infrav1.LegacyCalicoSecurityGroupRules() + } + } + return nil } @@ -554,3 +566,58 @@ func Convert_v1alpha5_Bastion_To_v1alpha8_Bastion(in *Bastion, out *infrav1.Bast in.Instance.FloatingIP = out.FloatingIP return nil } + +func Convert_v1alpha8_SecurityGroupStatus_To_v1alpha5_SecurityGroup(in *infrav1.SecurityGroupStatus, out *SecurityGroup, s conversion.Scope) error { //nolint:revive + out.ID = in.ID + out.Name = in.Name + out.Rules = make([]SecurityGroupRule, len(in.Rules)) + for i, rule := range in.Rules { + out.Rules[i] = SecurityGroupRule{ + ID: rule.ID, + Direction: rule.Direction, + } + if rule.Description != nil { + out.Rules[i].Description = *rule.Description + } + if rule.EtherType != nil { + out.Rules[i].EtherType = *rule.EtherType + } + if rule.PortRangeMin != nil { + out.Rules[i].PortRangeMin = *rule.PortRangeMin + } + if rule.PortRangeMax != nil { + out.Rules[i].PortRangeMax = *rule.PortRangeMax + } + if rule.Protocol != nil { + out.Rules[i].Protocol = *rule.Protocol + } + if rule.RemoteGroupID != nil { + out.Rules[i].RemoteGroupID = *rule.RemoteGroupID + } + if rule.RemoteIPPrefix != nil { + out.Rules[i].RemoteIPPrefix = *rule.RemoteIPPrefix + } + } + return nil +} + +func Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroupStatus(in *SecurityGroup, out *infrav1.SecurityGroupStatus, s conversion.Scope) error { //nolint:revive + out.ID = in.ID + out.Name = in.Name + out.Rules = make([]infrav1.SecurityGroupRuleStatus, len(in.Rules)) + for i, rule := range in.Rules { + out.Rules[i] = infrav1.SecurityGroupRuleStatus{ + ID: rule.ID, + Description: pointer.String(rule.Description), + Direction: rule.Direction, + EtherType: pointer.String(rule.EtherType), + PortRangeMin: pointer.Int(rule.PortRangeMin), + PortRangeMax: pointer.Int(rule.PortRangeMax), + Protocol: pointer.String(rule.Protocol), + RemoteGroupID: pointer.String(rule.RemoteGroupID), + RemoteIPPrefix: pointer.String(rule.RemoteIPPrefix), + } + } + + return nil +} diff --git a/api/v1alpha5/conversion_test.go b/api/v1alpha5/conversion_test.go index e4357782c4..b51adaff7c 100644 --- a/api/v1alpha5/conversion_test.go +++ b/api/v1alpha5/conversion_test.go @@ -49,7 +49,7 @@ func TestConvertFrom(t *testing.T) { Spec: OpenStackClusterSpec{}, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"allowAllInClusterTraffic\":false,\"apiServerLoadBalancer\":{},\"cloudName\":\"\",\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"managedSecurityGroups\":false,\"network\":{}},\"status\":{\"ready\":false}}", + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"allowAllInClusterTraffic\":false,\"apiServerLoadBalancer\":{},\"cloudName\":\"\",\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"managedSecurityGroups\":null,\"network\":{}},\"status\":{\"ready\":false}}", }, }, }, @@ -64,7 +64,7 @@ func TestConvertFrom(t *testing.T) { Spec: OpenStackClusterTemplateSpec{}, ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ - "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"allowAllInClusterTraffic\":false,\"apiServerLoadBalancer\":{},\"cloudName\":\"\",\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"managedSecurityGroups\":false,\"network\":{}}}}}", + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"allowAllInClusterTraffic\":false,\"apiServerLoadBalancer\":{},\"cloudName\":\"\",\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"managedSecurityGroups\":null,\"network\":{}}}}}", }, }, }, @@ -109,3 +109,50 @@ func TestConvertFrom(t *testing.T) { }) } } + +func TestConvert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(t *testing.T) { + tests := []struct { + name string + in *OpenStackClusterSpec + expectedOut *infrav1.OpenStackClusterSpec + }{ + { + name: "empty", + in: &OpenStackClusterSpec{}, + expectedOut: &infrav1.OpenStackClusterSpec{}, + }, + { + name: "with managed security groups and not allow all in cluster traffic", + in: &OpenStackClusterSpec{ + ManagedSecurityGroups: true, + AllowAllInClusterTraffic: false, + }, + expectedOut: &infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllNodesSecurityGroupRules: infrav1.LegacyCalicoSecurityGroupRules(), + }, + }, + }, + { + name: "with managed security groups and allow all in cluster traffic", + in: &OpenStackClusterSpec{ + ManagedSecurityGroups: true, + AllowAllInClusterTraffic: true, + }, + expectedOut: &infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{}, + AllowAllInClusterTraffic: true, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + out := &infrav1.OpenStackClusterSpec{} + err := Convert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(tt.in, out, nil) + g.Expect(err).NotTo(gomega.HaveOccurred()) + g.Expect(out).To(gomega.Equal(tt.expectedOut)) + }) + } +} diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index c9e8b952a2..0d4ef4e7b5 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -244,31 +244,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*SecurityGroup)(nil), (*v1alpha8.SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroup(a.(*SecurityGroup), b.(*v1alpha8.SecurityGroup), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroup)(nil), (*SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha8_SecurityGroup_To_v1alpha5_SecurityGroup(a.(*v1alpha8.SecurityGroup), b.(*SecurityGroup), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroupFilter)(nil), (*SecurityGroupFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha8_SecurityGroupFilter_To_v1alpha5_SecurityGroupFilter(a.(*v1alpha8.SecurityGroupFilter), b.(*SecurityGroupFilter), scope) }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*SecurityGroupRule)(nil), (*v1alpha8.SecurityGroupRule)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha5_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(a.(*SecurityGroupRule), b.(*v1alpha8.SecurityGroupRule), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroupRule)(nil), (*SecurityGroupRule)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha8_SecurityGroupRule_To_v1alpha5_SecurityGroupRule(a.(*v1alpha8.SecurityGroupRule), b.(*SecurityGroupRule), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Subnet)(nil), (*v1alpha8.Subnet)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha5_Subnet_To_v1alpha8_Subnet(a.(*Subnet), b.(*v1alpha8.Subnet), scope) }); err != nil { @@ -339,6 +319,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*SecurityGroup)(nil), (*v1alpha8.SecurityGroupStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroupStatus(a.(*SecurityGroup), b.(*v1alpha8.SecurityGroupStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*SubnetParam)(nil), (*v1alpha8.SubnetFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha5_SubnetParam_To_v1alpha8_SubnetFilter(a.(*SubnetParam), b.(*v1alpha8.SubnetFilter), scope) }); err != nil { @@ -404,6 +389,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha8.SecurityGroupStatus)(nil), (*SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha8_SecurityGroupStatus_To_v1alpha5_SecurityGroup(a.(*v1alpha8.SecurityGroupStatus), b.(*SecurityGroup), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha8.SubnetFilter)(nil), (*SubnetParam)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha8_SubnetFilter_To_v1alpha5_SubnetParam(a.(*v1alpha8.SubnetFilter), b.(*SubnetParam), scope) }); err != nil { @@ -680,7 +670,7 @@ func autoConvert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( out.APIServerFloatingIP = in.APIServerFloatingIP out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort - out.ManagedSecurityGroups = in.ManagedSecurityGroups + // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups) out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) @@ -728,7 +718,7 @@ func autoConvert_v1alpha8_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec( out.APIServerFloatingIP = in.APIServerFloatingIP out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort - out.ManagedSecurityGroups = in.ManagedSecurityGroups + // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups vs bool) out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) @@ -769,9 +759,33 @@ func autoConvert_v1alpha5_OpenStackClusterStatus_To_v1alpha8_OpenStackClusterSta out.ExternalNetwork = nil } out.FailureDomains = *(*v1beta1.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) - out.ControlPlaneSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.ControlPlaneSecurityGroup)) - out.WorkerSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.WorkerSecurityGroup)) - out.BastionSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.BastionSecurityGroup)) + if in.ControlPlaneSecurityGroup != nil { + in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.ControlPlaneSecurityGroup = nil + } + if in.WorkerSecurityGroup != nil { + in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.WorkerSecurityGroup = nil + } + if in.BastionSecurityGroup != nil { + in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.BastionSecurityGroup = nil + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(v1alpha8.BastionStatus) @@ -809,9 +823,33 @@ func autoConvert_v1alpha8_OpenStackClusterStatus_To_v1alpha5_OpenStackClusterSta // WARNING: in.Router requires manual conversion: does not exist in peer-type // WARNING: in.APIServerLoadBalancer requires manual conversion: does not exist in peer-type 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)) + if in.ControlPlaneSecurityGroup != nil { + in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha5_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.ControlPlaneSecurityGroup = nil + } + if in.WorkerSecurityGroup != nil { + in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha5_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.WorkerSecurityGroup = nil + } + if in.BastionSecurityGroup != nil { + in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha5_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.BastionSecurityGroup = nil + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(Instance) @@ -1374,30 +1412,6 @@ func Convert_v1alpha8_Router_To_v1alpha5_Router(in *v1alpha8.Router, out *Router return autoConvert_v1alpha8_Router_To_v1alpha5_Router(in, out, s) } -func autoConvert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroup(in *SecurityGroup, out *v1alpha8.SecurityGroup, s conversion.Scope) error { - out.Name = in.Name - out.ID = in.ID - out.Rules = *(*[]v1alpha8.SecurityGroupRule)(unsafe.Pointer(&in.Rules)) - return nil -} - -// Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroup is an autogenerated conversion function. -func Convert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroup(in *SecurityGroup, out *v1alpha8.SecurityGroup, s conversion.Scope) error { - return autoConvert_v1alpha5_SecurityGroup_To_v1alpha8_SecurityGroup(in, out, s) -} - -func autoConvert_v1alpha8_SecurityGroup_To_v1alpha5_SecurityGroup(in *v1alpha8.SecurityGroup, out *SecurityGroup, s conversion.Scope) error { - out.Name = in.Name - out.ID = in.ID - out.Rules = *(*[]SecurityGroupRule)(unsafe.Pointer(&in.Rules)) - return nil -} - -// Convert_v1alpha8_SecurityGroup_To_v1alpha5_SecurityGroup is an autogenerated conversion function. -func Convert_v1alpha8_SecurityGroup_To_v1alpha5_SecurityGroup(in *v1alpha8.SecurityGroup, out *SecurityGroup, s conversion.Scope) error { - return autoConvert_v1alpha8_SecurityGroup_To_v1alpha5_SecurityGroup(in, out, s) -} - func autoConvert_v1alpha5_SecurityGroupFilter_To_v1alpha8_SecurityGroupFilter(in *SecurityGroupFilter, out *v1alpha8.SecurityGroupFilter, s conversion.Scope) error { out.ID = in.ID out.Name = in.Name @@ -1432,44 +1446,6 @@ func Convert_v1alpha8_SecurityGroupFilter_To_v1alpha5_SecurityGroupFilter(in *v1 return autoConvert_v1alpha8_SecurityGroupFilter_To_v1alpha5_SecurityGroupFilter(in, out, s) } -func autoConvert_v1alpha5_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in *SecurityGroupRule, out *v1alpha8.SecurityGroupRule, s conversion.Scope) error { - out.Description = in.Description - out.ID = in.ID - out.Direction = in.Direction - out.EtherType = in.EtherType - out.SecurityGroupID = in.SecurityGroupID - out.PortRangeMin = in.PortRangeMin - out.PortRangeMax = in.PortRangeMax - out.Protocol = in.Protocol - out.RemoteGroupID = in.RemoteGroupID - out.RemoteIPPrefix = in.RemoteIPPrefix - return nil -} - -// Convert_v1alpha5_SecurityGroupRule_To_v1alpha8_SecurityGroupRule is an autogenerated conversion function. -func Convert_v1alpha5_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in *SecurityGroupRule, out *v1alpha8.SecurityGroupRule, s conversion.Scope) error { - return autoConvert_v1alpha5_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in, out, s) -} - -func autoConvert_v1alpha8_SecurityGroupRule_To_v1alpha5_SecurityGroupRule(in *v1alpha8.SecurityGroupRule, out *SecurityGroupRule, s conversion.Scope) error { - out.Description = in.Description - out.ID = in.ID - out.Direction = in.Direction - out.EtherType = in.EtherType - out.SecurityGroupID = in.SecurityGroupID - out.PortRangeMin = in.PortRangeMin - out.PortRangeMax = in.PortRangeMax - out.Protocol = in.Protocol - out.RemoteGroupID = in.RemoteGroupID - out.RemoteIPPrefix = in.RemoteIPPrefix - return nil -} - -// Convert_v1alpha8_SecurityGroupRule_To_v1alpha5_SecurityGroupRule is an autogenerated conversion function. -func Convert_v1alpha8_SecurityGroupRule_To_v1alpha5_SecurityGroupRule(in *v1alpha8.SecurityGroupRule, out *SecurityGroupRule, s conversion.Scope) error { - return autoConvert_v1alpha8_SecurityGroupRule_To_v1alpha5_SecurityGroupRule(in, out, s) -} - func autoConvert_v1alpha5_Subnet_To_v1alpha8_Subnet(in *Subnet, out *v1alpha8.Subnet, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go index 177a2f0f8c..417d161237 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -21,6 +21,7 @@ import ( "strings" apiconversion "k8s.io/apimachinery/pkg/conversion" + "k8s.io/utils/pointer" ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8" @@ -29,6 +30,16 @@ import ( const trueString = "true" +func restorev1alpha6SecurityGroup(previous *SecurityGroup, dst *SecurityGroup) { + if previous == nil || dst == nil { + return + } + + for i, rule := range previous.Rules { + dst.Rules[i].SecurityGroupID = rule.SecurityGroupID + } +} + func restorev1alpha6MachineSpec(previous *OpenStackMachineSpec, dst *OpenStackMachineSpec) { // Subnet is removed from v1alpha8 with no replacement, so can't be // losslessly converted. Restore the previously stored value on down-conversion. @@ -76,6 +87,10 @@ func restorev1alpha6ClusterStatus(previous *OpenStackClusterStatus, dst *OpenSta if previous.Bastion != nil && previous.Bastion.Networks != nil { dst.Bastion.Networks = previous.Bastion.Networks } + + restorev1alpha6SecurityGroup(previous.ControlPlaneSecurityGroup, dst.ControlPlaneSecurityGroup) + restorev1alpha6SecurityGroup(previous.WorkerSecurityGroup, dst.WorkerSecurityGroup) + restorev1alpha6SecurityGroup(previous.BastionSecurityGroup, dst.BastionSecurityGroup) } func restorev1alpha8MachineSpec(previous *infrav1.OpenStackMachineSpec, dst *infrav1.OpenStackMachineSpec) { @@ -110,6 +125,10 @@ func restorev1alpha8ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst if previous.Bastion != nil { dst.Bastion.ReferencedResources = previous.Bastion.ReferencedResources } + + dst.ControlPlaneSecurityGroup = previous.ControlPlaneSecurityGroup + dst.WorkerSecurityGroup = previous.WorkerSecurityGroup + dst.BastionSecurityGroup = previous.BastionSecurityGroup } func restorev1alpha6ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackClusterSpec) { @@ -189,6 +208,12 @@ var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStack }, restorev1alpha8Subnets, ), + "allNodesSecurityGroupRules": conversion.HashedFieldRestorer( + func(c *infrav1.OpenStackCluster) *infrav1.ManagedSecurityGroups { + return c.Spec.ManagedSecurityGroups + }, + restorev1alpha8ManagedSecurityGroups, + ), "status": conversion.HashedFieldRestorer( func(c *infrav1.OpenStackCluster) *infrav1.OpenStackClusterStatus { return &c.Status @@ -242,6 +267,10 @@ var v1alpha6OpenStackClusterTemplateRestorer = conversion.RestorerFor[*OpenStack ), } +func restorev1alpha8ManagedSecurityGroups(previous *infrav1.ManagedSecurityGroups, dst *infrav1.ManagedSecurityGroups) { + dst.AllNodesSecurityGroupRules = previous.AllNodesSecurityGroupRules +} + var v1alpha8OpenStackClusterTemplateRestorer = conversion.RestorerFor[*infrav1.OpenStackClusterTemplate]{ "externalNetwork": conversion.UnconditionalFieldRestorer( func(c *infrav1.OpenStackClusterTemplate) *infrav1.NetworkFilter { @@ -275,6 +304,12 @@ var v1alpha8OpenStackClusterTemplateRestorer = conversion.RestorerFor[*infrav1.O }, restorev1alpha8Subnets, ), + "allNodesSecurityGroupRules": conversion.HashedFieldRestorer( + func(c *infrav1.OpenStackClusterTemplate) *infrav1.ManagedSecurityGroups { + return c.Spec.Template.Spec.ManagedSecurityGroups + }, + restorev1alpha8ManagedSecurityGroups, + ), } func (r *OpenStackClusterTemplate) ConvertTo(dstRaw ctrlconversion.Hub) error { @@ -555,6 +590,10 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in * out.DNSNameservers = in.ManagedSubnets[0].DNSNameservers } + if in.ManagedSecurityGroups != nil { + out.ManagedSecurityGroups = true + } + return nil } @@ -589,6 +628,13 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in * } } + if in.ManagedSecurityGroups { + out.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} + if !in.AllowAllInClusterTraffic { + out.ManagedSecurityGroups.AllNodesSecurityGroupRules = infrav1.LegacyCalicoSecurityGroupRules() + } + } + return nil } @@ -878,3 +924,58 @@ func Convert_v1alpha8_Bastion_To_v1alpha6_Bastion(in *infrav1.Bastion, out *Bast out.Instance.FloatingIP = in.FloatingIP return nil } + +func Convert_v1alpha8_SecurityGroupStatus_To_v1alpha6_SecurityGroup(in *infrav1.SecurityGroupStatus, out *SecurityGroup, s apiconversion.Scope) error { //nolint:revive + out.ID = in.ID + out.Name = in.Name + out.Rules = make([]SecurityGroupRule, len(in.Rules)) + for i, rule := range in.Rules { + out.Rules[i] = SecurityGroupRule{ + ID: rule.ID, + Direction: rule.Direction, + } + if rule.Description != nil { + out.Rules[i].Description = *rule.Description + } + if rule.EtherType != nil { + out.Rules[i].EtherType = *rule.EtherType + } + if rule.PortRangeMin != nil { + out.Rules[i].PortRangeMin = *rule.PortRangeMin + } + if rule.PortRangeMax != nil { + out.Rules[i].PortRangeMax = *rule.PortRangeMax + } + if rule.Protocol != nil { + out.Rules[i].Protocol = *rule.Protocol + } + if rule.RemoteGroupID != nil { + out.Rules[i].RemoteGroupID = *rule.RemoteGroupID + } + if rule.RemoteIPPrefix != nil { + out.Rules[i].RemoteIPPrefix = *rule.RemoteIPPrefix + } + } + return nil +} + +func Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroupStatus(in *SecurityGroup, out *infrav1.SecurityGroupStatus, s apiconversion.Scope) error { //nolint:revive + out.ID = in.ID + out.Name = in.Name + out.Rules = make([]infrav1.SecurityGroupRuleStatus, len(in.Rules)) + for i, rule := range in.Rules { + out.Rules[i] = infrav1.SecurityGroupRuleStatus{ + ID: rule.ID, + Description: pointer.String(rule.Description), + Direction: rule.Direction, + EtherType: pointer.String(rule.EtherType), + PortRangeMin: pointer.Int(rule.PortRangeMin), + PortRangeMax: pointer.Int(rule.PortRangeMax), + Protocol: pointer.String(rule.Protocol), + RemoteGroupID: pointer.String(rule.RemoteGroupID), + RemoteIPPrefix: pointer.String(rule.RemoteIPPrefix), + } + } + + return nil +} diff --git a/api/v1alpha6/conversion_test.go b/api/v1alpha6/conversion_test.go index e6ed8f4c9e..2e684b8e35 100644 --- a/api/v1alpha6/conversion_test.go +++ b/api/v1alpha6/conversion_test.go @@ -654,3 +654,50 @@ func TestMachineConversionControllerSpecFields(t *testing.T) { }) } } + +func TestConvert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(t *testing.T) { + tests := []struct { + name string + in *OpenStackClusterSpec + expectedOut *infrav1.OpenStackClusterSpec + }{ + { + name: "empty", + in: &OpenStackClusterSpec{}, + expectedOut: &infrav1.OpenStackClusterSpec{}, + }, + { + name: "with managed security groups and not allow all in cluster traffic", + in: &OpenStackClusterSpec{ + ManagedSecurityGroups: true, + AllowAllInClusterTraffic: false, + }, + expectedOut: &infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllNodesSecurityGroupRules: infrav1.LegacyCalicoSecurityGroupRules(), + }, + }, + }, + { + name: "with managed security groups and allow all in cluster traffic", + in: &OpenStackClusterSpec{ + ManagedSecurityGroups: true, + AllowAllInClusterTraffic: true, + }, + expectedOut: &infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{}, + AllowAllInClusterTraffic: true, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + out := &infrav1.OpenStackClusterSpec{} + err := Convert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(tt.in, out, nil) + g.Expect(err).NotTo(gomega.HaveOccurred()) + g.Expect(out).To(gomega.Equal(tt.expectedOut)) + }) + } +} diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index 08c25e97cf..7df2ce7482 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -254,31 +254,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*SecurityGroup)(nil), (*v1alpha8.SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroup(a.(*SecurityGroup), b.(*v1alpha8.SecurityGroup), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroup)(nil), (*SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha8_SecurityGroup_To_v1alpha6_SecurityGroup(a.(*v1alpha8.SecurityGroup), b.(*SecurityGroup), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroupFilter)(nil), (*SecurityGroupFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha8_SecurityGroupFilter_To_v1alpha6_SecurityGroupFilter(a.(*v1alpha8.SecurityGroupFilter), b.(*SecurityGroupFilter), scope) }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*SecurityGroupRule)(nil), (*v1alpha8.SecurityGroupRule)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha6_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(a.(*SecurityGroupRule), b.(*v1alpha8.SecurityGroupRule), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroupRule)(nil), (*SecurityGroupRule)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha8_SecurityGroupRule_To_v1alpha6_SecurityGroupRule(a.(*v1alpha8.SecurityGroupRule), b.(*SecurityGroupRule), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Subnet)(nil), (*v1alpha8.Subnet)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha6_Subnet_To_v1alpha8_Subnet(a.(*Subnet), b.(*v1alpha8.Subnet), scope) }); err != nil { @@ -359,6 +339,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*SecurityGroup)(nil), (*v1alpha8.SecurityGroupStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroupStatus(a.(*SecurityGroup), b.(*v1alpha8.SecurityGroupStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*SubnetParam)(nil), (*v1alpha8.SubnetFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha6_SubnetParam_To_v1alpha8_SubnetFilter(a.(*SubnetParam), b.(*v1alpha8.SubnetFilter), scope) }); err != nil { @@ -414,6 +399,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha8.SecurityGroupStatus)(nil), (*SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha8_SecurityGroupStatus_To_v1alpha6_SecurityGroup(a.(*v1alpha8.SecurityGroupStatus), b.(*SecurityGroup), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha8.SubnetFilter)(nil), (*SubnetParam)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha8_SubnetFilter_To_v1alpha6_SubnetParam(a.(*v1alpha8.SubnetFilter), b.(*SubnetParam), scope) }); err != nil { @@ -702,7 +692,7 @@ func autoConvert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( out.APIServerFloatingIP = in.APIServerFloatingIP out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort - out.ManagedSecurityGroups = in.ManagedSecurityGroups + // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups) out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) @@ -751,7 +741,7 @@ func autoConvert_v1alpha8_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec( out.APIServerFloatingIP = in.APIServerFloatingIP out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort - out.ManagedSecurityGroups = in.ManagedSecurityGroups + // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups vs bool) out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) @@ -792,9 +782,33 @@ func autoConvert_v1alpha6_OpenStackClusterStatus_To_v1alpha8_OpenStackClusterSta out.ExternalNetwork = nil } out.FailureDomains = *(*v1beta1.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) - out.ControlPlaneSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.ControlPlaneSecurityGroup)) - out.WorkerSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.WorkerSecurityGroup)) - out.BastionSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.BastionSecurityGroup)) + if in.ControlPlaneSecurityGroup != nil { + in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.ControlPlaneSecurityGroup = nil + } + if in.WorkerSecurityGroup != nil { + in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.WorkerSecurityGroup = nil + } + if in.BastionSecurityGroup != nil { + in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.BastionSecurityGroup = nil + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(v1alpha8.BastionStatus) @@ -832,9 +846,33 @@ func autoConvert_v1alpha8_OpenStackClusterStatus_To_v1alpha6_OpenStackClusterSta // WARNING: in.Router requires manual conversion: does not exist in peer-type // WARNING: in.APIServerLoadBalancer requires manual conversion: does not exist in peer-type 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)) + if in.ControlPlaneSecurityGroup != nil { + in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha6_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.ControlPlaneSecurityGroup = nil + } + if in.WorkerSecurityGroup != nil { + in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha6_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.WorkerSecurityGroup = nil + } + if in.BastionSecurityGroup != nil { + in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha6_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.BastionSecurityGroup = nil + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(Instance) @@ -1398,30 +1436,6 @@ func Convert_v1alpha8_Router_To_v1alpha6_Router(in *v1alpha8.Router, out *Router return autoConvert_v1alpha8_Router_To_v1alpha6_Router(in, out, s) } -func autoConvert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroup(in *SecurityGroup, out *v1alpha8.SecurityGroup, s conversion.Scope) error { - out.Name = in.Name - out.ID = in.ID - out.Rules = *(*[]v1alpha8.SecurityGroupRule)(unsafe.Pointer(&in.Rules)) - return nil -} - -// Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroup is an autogenerated conversion function. -func Convert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroup(in *SecurityGroup, out *v1alpha8.SecurityGroup, s conversion.Scope) error { - return autoConvert_v1alpha6_SecurityGroup_To_v1alpha8_SecurityGroup(in, out, s) -} - -func autoConvert_v1alpha8_SecurityGroup_To_v1alpha6_SecurityGroup(in *v1alpha8.SecurityGroup, out *SecurityGroup, s conversion.Scope) error { - out.Name = in.Name - out.ID = in.ID - out.Rules = *(*[]SecurityGroupRule)(unsafe.Pointer(&in.Rules)) - return nil -} - -// Convert_v1alpha8_SecurityGroup_To_v1alpha6_SecurityGroup is an autogenerated conversion function. -func Convert_v1alpha8_SecurityGroup_To_v1alpha6_SecurityGroup(in *v1alpha8.SecurityGroup, out *SecurityGroup, s conversion.Scope) error { - return autoConvert_v1alpha8_SecurityGroup_To_v1alpha6_SecurityGroup(in, out, s) -} - func autoConvert_v1alpha6_SecurityGroupFilter_To_v1alpha8_SecurityGroupFilter(in *SecurityGroupFilter, out *v1alpha8.SecurityGroupFilter, s conversion.Scope) error { out.ID = in.ID out.Name = in.Name @@ -1456,44 +1470,6 @@ func Convert_v1alpha8_SecurityGroupFilter_To_v1alpha6_SecurityGroupFilter(in *v1 return autoConvert_v1alpha8_SecurityGroupFilter_To_v1alpha6_SecurityGroupFilter(in, out, s) } -func autoConvert_v1alpha6_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in *SecurityGroupRule, out *v1alpha8.SecurityGroupRule, s conversion.Scope) error { - out.Description = in.Description - out.ID = in.ID - out.Direction = in.Direction - out.EtherType = in.EtherType - out.SecurityGroupID = in.SecurityGroupID - out.PortRangeMin = in.PortRangeMin - out.PortRangeMax = in.PortRangeMax - out.Protocol = in.Protocol - out.RemoteGroupID = in.RemoteGroupID - out.RemoteIPPrefix = in.RemoteIPPrefix - return nil -} - -// Convert_v1alpha6_SecurityGroupRule_To_v1alpha8_SecurityGroupRule is an autogenerated conversion function. -func Convert_v1alpha6_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in *SecurityGroupRule, out *v1alpha8.SecurityGroupRule, s conversion.Scope) error { - return autoConvert_v1alpha6_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in, out, s) -} - -func autoConvert_v1alpha8_SecurityGroupRule_To_v1alpha6_SecurityGroupRule(in *v1alpha8.SecurityGroupRule, out *SecurityGroupRule, s conversion.Scope) error { - out.Description = in.Description - out.ID = in.ID - out.Direction = in.Direction - out.EtherType = in.EtherType - out.SecurityGroupID = in.SecurityGroupID - out.PortRangeMin = in.PortRangeMin - out.PortRangeMax = in.PortRangeMax - out.Protocol = in.Protocol - out.RemoteGroupID = in.RemoteGroupID - out.RemoteIPPrefix = in.RemoteIPPrefix - return nil -} - -// Convert_v1alpha8_SecurityGroupRule_To_v1alpha6_SecurityGroupRule is an autogenerated conversion function. -func Convert_v1alpha8_SecurityGroupRule_To_v1alpha6_SecurityGroupRule(in *v1alpha8.SecurityGroupRule, out *SecurityGroupRule, s conversion.Scope) error { - return autoConvert_v1alpha8_SecurityGroupRule_To_v1alpha6_SecurityGroupRule(in, out, s) -} - func autoConvert_v1alpha6_Subnet_To_v1alpha8_Subnet(in *Subnet, out *v1alpha8.Subnet, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID diff --git a/api/v1alpha7/conversion.go b/api/v1alpha7/conversion.go index 9bfa6ad90d..1e84d4f0b9 100644 --- a/api/v1alpha7/conversion.go +++ b/api/v1alpha7/conversion.go @@ -18,6 +18,7 @@ package v1alpha7 import ( apiconversion "k8s.io/apimachinery/pkg/conversion" + "k8s.io/utils/pointer" ctrlconversion "sigs.k8s.io/controller-runtime/pkg/conversion" infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8" @@ -57,6 +58,72 @@ var v1alpha7OpenStackClusterRestorer = conversion.RestorerFor[*OpenStackCluster] }, ), ), + "status": conversion.HashedFieldRestorer( + func(c *OpenStackCluster) *OpenStackClusterStatus { + return &c.Status + }, + restorev1alpha7ClusterStatus, + ), +} + +func restorev1alpha7SecurityGroup(previous *SecurityGroup, dst *SecurityGroup) { + if previous == nil || dst == nil { + return + } + + for i, rule := range previous.Rules { + dst.Rules[i].SecurityGroupID = rule.SecurityGroupID + } +} + +func restorev1alpha7ClusterStatus(previous *OpenStackClusterStatus, dst *OpenStackClusterStatus) { + restorev1alpha7SecurityGroup(previous.ControlPlaneSecurityGroup, dst.ControlPlaneSecurityGroup) + restorev1alpha7SecurityGroup(previous.WorkerSecurityGroup, dst.WorkerSecurityGroup) + restorev1alpha7SecurityGroup(previous.BastionSecurityGroup, dst.BastionSecurityGroup) +} + +func restorev1alpha8SecurityGroupStatus(previous *infrav1.SecurityGroupStatus, dst *infrav1.SecurityGroupStatus) { + if previous == nil || dst == nil { + return + } + + for i := range dst.Rules { + dstRule := &dst.Rules[i] + + // Conversion from scalar to *scalar is lossy for zero values. We need to restore only nil values. + if dstRule.Description != nil && *dstRule.Description == "" { + dstRule.Description = previous.Rules[i].Description + } + if dstRule.EtherType != nil && *dstRule.EtherType == "" { + dstRule.EtherType = previous.Rules[i].EtherType + } + if dstRule.PortRangeMin != nil && *dstRule.PortRangeMin == 0 { + dstRule.PortRangeMin = previous.Rules[i].PortRangeMin + } + if dstRule.PortRangeMax != nil && *dstRule.PortRangeMax == 0 { + dstRule.PortRangeMax = previous.Rules[i].PortRangeMax + } + if dstRule.Protocol != nil && *dstRule.Protocol == "" { + dstRule.Protocol = previous.Rules[i].Protocol + } + if dstRule.RemoteGroupID != nil && *dstRule.RemoteGroupID == "" { + dstRule.RemoteGroupID = previous.Rules[i].RemoteGroupID + } + if dstRule.RemoteIPPrefix != nil && *dstRule.RemoteIPPrefix == "" { + dstRule.RemoteIPPrefix = previous.Rules[i].RemoteIPPrefix + } + } +} + +func restorev1alpha8ClusterStatus(previous *infrav1.OpenStackClusterStatus, dst *infrav1.OpenStackClusterStatus) { + restorev1alpha8SecurityGroupStatus(previous.ControlPlaneSecurityGroup, dst.ControlPlaneSecurityGroup) + restorev1alpha8SecurityGroupStatus(previous.WorkerSecurityGroup, dst.WorkerSecurityGroup) + restorev1alpha8SecurityGroupStatus(previous.BastionSecurityGroup, dst.BastionSecurityGroup) + + // ReferencedResources have no equivalent in v1alpha7 + if dst.Bastion != nil { + dst.Bastion.ReferencedResources = previous.Bastion.ReferencedResources + } } var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStackCluster]{ @@ -85,14 +152,11 @@ var v1alpha8OpenStackClusterRestorer = conversion.RestorerFor[*infrav1.OpenStack ), ), - // No equivalent in v1alpha7 - "bastionrefresources": conversion.UnconditionalFieldRestorer( - func(c *infrav1.OpenStackCluster) *infrav1.ReferencedMachineResources { - if c.Status.Bastion == nil { - return nil - } - return &c.Status.Bastion.ReferencedResources + "status": conversion.HashedFieldRestorer( + func(c *infrav1.OpenStackCluster) *infrav1.OpenStackClusterStatus { + return &c.Status }, + restorev1alpha8ClusterStatus, ), } @@ -165,6 +229,10 @@ func restorev1alpha8ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *inf } dst.ManagedSubnets = previous.ManagedSubnets + + if previous.ManagedSecurityGroups != nil { + dst.ManagedSecurityGroups.AllNodesSecurityGroupRules = previous.ManagedSecurityGroups.AllNodesSecurityGroupRules + } } func (r *OpenStackCluster) ConvertTo(dstRaw ctrlconversion.Hub) error { @@ -509,6 +577,13 @@ func Convert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in * } } + if in.ManagedSecurityGroups { + out.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} + if !in.AllowAllInClusterTraffic { + out.ManagedSecurityGroups.AllNodesSecurityGroupRules = infrav1.LegacyCalicoSecurityGroupRules() + } + } + return nil } @@ -533,5 +608,64 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in * out.DNSNameservers = in.ManagedSubnets[0].DNSNameservers } + if in.ManagedSecurityGroups != nil { + out.ManagedSecurityGroups = true + } + + return nil +} + +func Convert_v1alpha8_SecurityGroupStatus_To_v1alpha7_SecurityGroup(in *infrav1.SecurityGroupStatus, out *SecurityGroup, s apiconversion.Scope) error { //nolint:revive + out.ID = in.ID + out.Name = in.Name + out.Rules = make([]SecurityGroupRule, len(in.Rules)) + for i, rule := range in.Rules { + out.Rules[i] = SecurityGroupRule{ + ID: rule.ID, + Direction: rule.Direction, + } + if rule.Description != nil { + out.Rules[i].Description = *rule.Description + } + if rule.EtherType != nil { + out.Rules[i].EtherType = *rule.EtherType + } + if rule.PortRangeMin != nil { + out.Rules[i].PortRangeMin = *rule.PortRangeMin + } + if rule.PortRangeMax != nil { + out.Rules[i].PortRangeMax = *rule.PortRangeMax + } + if rule.Protocol != nil { + out.Rules[i].Protocol = *rule.Protocol + } + if rule.RemoteGroupID != nil { + out.Rules[i].RemoteGroupID = *rule.RemoteGroupID + } + if rule.RemoteIPPrefix != nil { + out.Rules[i].RemoteIPPrefix = *rule.RemoteIPPrefix + } + } + return nil +} + +func Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroupStatus(in *SecurityGroup, out *infrav1.SecurityGroupStatus, s apiconversion.Scope) error { //nolint:revive + out.ID = in.ID + out.Name = in.Name + out.Rules = make([]infrav1.SecurityGroupRuleStatus, len(in.Rules)) + for i, rule := range in.Rules { + out.Rules[i] = infrav1.SecurityGroupRuleStatus{ + ID: rule.ID, + Description: pointer.String(rule.Description), + Direction: rule.Direction, + EtherType: pointer.String(rule.EtherType), + PortRangeMin: pointer.Int(rule.PortRangeMin), + PortRangeMax: pointer.Int(rule.PortRangeMax), + Protocol: pointer.String(rule.Protocol), + RemoteGroupID: pointer.String(rule.RemoteGroupID), + RemoteIPPrefix: pointer.String(rule.RemoteIPPrefix), + } + } + return nil } diff --git a/api/v1alpha7/conversion_test.go b/api/v1alpha7/conversion_test.go index 97763d7d4b..90b391f037 100644 --- a/api/v1alpha7/conversion_test.go +++ b/api/v1alpha7/conversion_test.go @@ -269,3 +269,50 @@ func TestMachineConversionControllerSpecFields(t *testing.T) { }) } } + +func TestConvert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(t *testing.T) { + tests := []struct { + name string + in *OpenStackClusterSpec + expectedOut *infrav1.OpenStackClusterSpec + }{ + { + name: "empty", + in: &OpenStackClusterSpec{}, + expectedOut: &infrav1.OpenStackClusterSpec{}, + }, + { + name: "with managed security groups and not allow all in cluster traffic", + in: &OpenStackClusterSpec{ + ManagedSecurityGroups: true, + AllowAllInClusterTraffic: false, + }, + expectedOut: &infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllNodesSecurityGroupRules: infrav1.LegacyCalicoSecurityGroupRules(), + }, + }, + }, + { + name: "with managed security groups and allow all in cluster traffic", + in: &OpenStackClusterSpec{ + ManagedSecurityGroups: true, + AllowAllInClusterTraffic: true, + }, + expectedOut: &infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{}, + AllowAllInClusterTraffic: true, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := gomega.NewWithT(t) + out := &infrav1.OpenStackClusterSpec{} + err := Convert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(tt.in, out, nil) + g.Expect(err).NotTo(gomega.HaveOccurred()) + g.Expect(out).To(gomega.Equal(tt.expectedOut)) + }) + } +} diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index de0e42be72..5ff683e8fb 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -349,16 +349,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*SecurityGroup)(nil), (*v1alpha8.SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroup(a.(*SecurityGroup), b.(*v1alpha8.SecurityGroup), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroup)(nil), (*SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha8_SecurityGroup_To_v1alpha7_SecurityGroup(a.(*v1alpha8.SecurityGroup), b.(*SecurityGroup), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*SecurityGroupFilter)(nil), (*v1alpha8.SecurityGroupFilter)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha7_SecurityGroupFilter_To_v1alpha8_SecurityGroupFilter(a.(*SecurityGroupFilter), b.(*v1alpha8.SecurityGroupFilter), scope) }); err != nil { @@ -369,16 +359,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*SecurityGroupRule)(nil), (*v1alpha8.SecurityGroupRule)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha7_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(a.(*SecurityGroupRule), b.(*v1alpha8.SecurityGroupRule), scope) - }); err != nil { - return err - } - if err := s.AddGeneratedConversionFunc((*v1alpha8.SecurityGroupRule)(nil), (*SecurityGroupRule)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1alpha8_SecurityGroupRule_To_v1alpha7_SecurityGroupRule(a.(*v1alpha8.SecurityGroupRule), b.(*SecurityGroupRule), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*Subnet)(nil), (*v1alpha8.Subnet)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha7_Subnet_To_v1alpha8_Subnet(a.(*Subnet), b.(*v1alpha8.Subnet), scope) }); err != nil { @@ -424,6 +404,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*SecurityGroup)(nil), (*v1alpha8.SecurityGroupStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroupStatus(a.(*SecurityGroup), b.(*v1alpha8.SecurityGroupStatus), scope) + }); err != nil { + return err + } if err := s.AddConversionFunc((*v1alpha8.BastionStatus)(nil), (*BastionStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1alpha8_BastionStatus_To_v1alpha7_BastionStatus(a.(*v1alpha8.BastionStatus), b.(*BastionStatus), scope) }); err != nil { @@ -449,6 +434,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1alpha8.SecurityGroupStatus)(nil), (*SecurityGroup)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1alpha8_SecurityGroupStatus_To_v1alpha7_SecurityGroup(a.(*v1alpha8.SecurityGroupStatus), b.(*SecurityGroup), scope) + }); err != nil { + return err + } return nil } @@ -894,7 +884,7 @@ func autoConvert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( out.APIServerFloatingIP = in.APIServerFloatingIP out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort - out.ManagedSecurityGroups = in.ManagedSecurityGroups + // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups) out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) @@ -933,7 +923,7 @@ func autoConvert_v1alpha8_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec( out.APIServerFloatingIP = in.APIServerFloatingIP out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort - out.ManagedSecurityGroups = in.ManagedSecurityGroups + // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (*sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups vs bool) out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) @@ -960,9 +950,33 @@ func autoConvert_v1alpha7_OpenStackClusterStatus_To_v1alpha8_OpenStackClusterSta out.Router = (*v1alpha8.Router)(unsafe.Pointer(in.Router)) out.APIServerLoadBalancer = (*v1alpha8.LoadBalancer)(unsafe.Pointer(in.APIServerLoadBalancer)) out.FailureDomains = *(*v1beta1.FailureDomains)(unsafe.Pointer(&in.FailureDomains)) - out.ControlPlaneSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.ControlPlaneSecurityGroup)) - out.WorkerSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.WorkerSecurityGroup)) - out.BastionSecurityGroup = (*v1alpha8.SecurityGroup)(unsafe.Pointer(in.BastionSecurityGroup)) + if in.ControlPlaneSecurityGroup != nil { + in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.ControlPlaneSecurityGroup = nil + } + if in.WorkerSecurityGroup != nil { + in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.WorkerSecurityGroup = nil + } + if in.BastionSecurityGroup != nil { + in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup + *out = new(v1alpha8.SecurityGroupStatus) + if err := Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroupStatus(*in, *out, s); err != nil { + return err + } + } else { + out.BastionSecurityGroup = nil + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(v1alpha8.BastionStatus) @@ -989,9 +1003,33 @@ func autoConvert_v1alpha8_OpenStackClusterStatus_To_v1alpha7_OpenStackClusterSta out.Router = (*Router)(unsafe.Pointer(in.Router)) out.APIServerLoadBalancer = (*LoadBalancer)(unsafe.Pointer(in.APIServerLoadBalancer)) 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)) + if in.ControlPlaneSecurityGroup != nil { + in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha7_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.ControlPlaneSecurityGroup = nil + } + if in.WorkerSecurityGroup != nil { + in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha7_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.WorkerSecurityGroup = nil + } + if in.BastionSecurityGroup != nil { + in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup + *out = new(SecurityGroup) + if err := Convert_v1alpha8_SecurityGroupStatus_To_v1alpha7_SecurityGroup(*in, *out, s); err != nil { + return err + } + } else { + out.BastionSecurityGroup = nil + } if in.Bastion != nil { in, out := &in.Bastion, &out.Bastion *out = new(BastionStatus) @@ -1545,30 +1583,6 @@ func Convert_v1alpha8_RouterFilter_To_v1alpha7_RouterFilter(in *v1alpha8.RouterF return autoConvert_v1alpha8_RouterFilter_To_v1alpha7_RouterFilter(in, out, s) } -func autoConvert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroup(in *SecurityGroup, out *v1alpha8.SecurityGroup, s conversion.Scope) error { - out.Name = in.Name - out.ID = in.ID - out.Rules = *(*[]v1alpha8.SecurityGroupRule)(unsafe.Pointer(&in.Rules)) - return nil -} - -// Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroup is an autogenerated conversion function. -func Convert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroup(in *SecurityGroup, out *v1alpha8.SecurityGroup, s conversion.Scope) error { - return autoConvert_v1alpha7_SecurityGroup_To_v1alpha8_SecurityGroup(in, out, s) -} - -func autoConvert_v1alpha8_SecurityGroup_To_v1alpha7_SecurityGroup(in *v1alpha8.SecurityGroup, out *SecurityGroup, s conversion.Scope) error { - out.Name = in.Name - out.ID = in.ID - out.Rules = *(*[]SecurityGroupRule)(unsafe.Pointer(&in.Rules)) - return nil -} - -// Convert_v1alpha8_SecurityGroup_To_v1alpha7_SecurityGroup is an autogenerated conversion function. -func Convert_v1alpha8_SecurityGroup_To_v1alpha7_SecurityGroup(in *v1alpha8.SecurityGroup, out *SecurityGroup, s conversion.Scope) error { - return autoConvert_v1alpha8_SecurityGroup_To_v1alpha7_SecurityGroup(in, out, s) -} - func autoConvert_v1alpha7_SecurityGroupFilter_To_v1alpha8_SecurityGroupFilter(in *SecurityGroupFilter, out *v1alpha8.SecurityGroupFilter, s conversion.Scope) error { out.ID = in.ID out.Name = in.Name @@ -1603,44 +1617,6 @@ func Convert_v1alpha8_SecurityGroupFilter_To_v1alpha7_SecurityGroupFilter(in *v1 return autoConvert_v1alpha8_SecurityGroupFilter_To_v1alpha7_SecurityGroupFilter(in, out, s) } -func autoConvert_v1alpha7_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in *SecurityGroupRule, out *v1alpha8.SecurityGroupRule, s conversion.Scope) error { - out.Description = in.Description - out.ID = in.ID - out.Direction = in.Direction - out.EtherType = in.EtherType - out.SecurityGroupID = in.SecurityGroupID - out.PortRangeMin = in.PortRangeMin - out.PortRangeMax = in.PortRangeMax - out.Protocol = in.Protocol - out.RemoteGroupID = in.RemoteGroupID - out.RemoteIPPrefix = in.RemoteIPPrefix - return nil -} - -// Convert_v1alpha7_SecurityGroupRule_To_v1alpha8_SecurityGroupRule is an autogenerated conversion function. -func Convert_v1alpha7_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in *SecurityGroupRule, out *v1alpha8.SecurityGroupRule, s conversion.Scope) error { - return autoConvert_v1alpha7_SecurityGroupRule_To_v1alpha8_SecurityGroupRule(in, out, s) -} - -func autoConvert_v1alpha8_SecurityGroupRule_To_v1alpha7_SecurityGroupRule(in *v1alpha8.SecurityGroupRule, out *SecurityGroupRule, s conversion.Scope) error { - out.Description = in.Description - out.ID = in.ID - out.Direction = in.Direction - out.EtherType = in.EtherType - out.SecurityGroupID = in.SecurityGroupID - out.PortRangeMin = in.PortRangeMin - out.PortRangeMax = in.PortRangeMax - out.Protocol = in.Protocol - out.RemoteGroupID = in.RemoteGroupID - out.RemoteIPPrefix = in.RemoteIPPrefix - return nil -} - -// Convert_v1alpha8_SecurityGroupRule_To_v1alpha7_SecurityGroupRule is an autogenerated conversion function. -func Convert_v1alpha8_SecurityGroupRule_To_v1alpha7_SecurityGroupRule(in *v1alpha8.SecurityGroupRule, out *SecurityGroupRule, s conversion.Scope) error { - return autoConvert_v1alpha8_SecurityGroupRule_To_v1alpha7_SecurityGroupRule(in, out, s) -} - func autoConvert_v1alpha7_Subnet_To_v1alpha8_Subnet(in *Subnet, out *v1alpha8.Subnet, s conversion.Scope) error { out.Name = in.Name out.ID = in.ID diff --git a/api/v1alpha8/conversion.go b/api/v1alpha8/conversion.go index 49a817623d..d59b51d96f 100644 --- a/api/v1alpha8/conversion.go +++ b/api/v1alpha8/conversion.go @@ -16,6 +16,8 @@ limitations under the License. package v1alpha8 +import "k8s.io/utils/pointer" + // Hub marks OpenStackCluster as a conversion hub. func (*OpenStackCluster) Hub() {} @@ -39,3 +41,29 @@ func (*OpenStackMachineTemplate) Hub() {} // Hub marks OpenStackMachineTemplateList as a conversion hub. func (*OpenStackMachineTemplateList) Hub() {} + +// LegacyCalicoSecurityGroupRules returns a list of security group rules for calico +// that need to be applied to the control plane and worker security groups when +// managed security groups are enabled and upgrading to v1alpha8. +func LegacyCalicoSecurityGroupRules() []SecurityGroupRuleSpec { + return []SecurityGroupRuleSpec{ + { + Name: "BGP (calico)", + Description: pointer.String("Created by cluster-api-provider-openstack API conversion - BGP (calico)"), + Direction: "ingress", + EtherType: pointer.String("IPv4"), + PortRangeMin: pointer.Int(179), + PortRangeMax: pointer.Int(179), + Protocol: pointer.String("tcp"), + RemoteManagedGroups: []ManagedSecurityGroupName{"controlplane", "worker"}, + }, + { + Name: "IP-in-IP (calico)", + Description: pointer.String("Created by cluster-api-provider-openstack API conversion - IP-in-IP (calico)"), + Direction: "ingress", + EtherType: pointer.String("IPv4"), + Protocol: pointer.String("4"), + RemoteManagedGroups: []ManagedSecurityGroupName{"controlplane", "worker"}, + }, + } +} diff --git a/api/v1alpha8/openstackcluster_types.go b/api/v1alpha8/openstackcluster_types.go index 5f25489e24..cabb2a55ca 100644 --- a/api/v1alpha8/openstackcluster_types.go +++ b/api/v1alpha8/openstackcluster_types.go @@ -122,10 +122,12 @@ type OpenStackClusterSpec struct { // ManagedSecurityGroups determines whether OpenStack security groups for the cluster // will be managed by the OpenStack provider or whether pre-existing security groups will // be specified as part of the configuration. - // By default, the managed security groups have rules that allow the Kubelet, etcd, the - // Kubernetes API server and the Calico CNI plugin to function correctly. + // By default, the managed security groups have rules that allow the Kubelet, etcd, and the + // Kubernetes API server to function correctly. + // It's possible to add additional rules to the managed security groups. + // When defined to an empty struct, the managed security groups will be created with the default rules. // +optional - ManagedSecurityGroups bool `json:"managedSecurityGroups"` + ManagedSecurityGroups *ManagedSecurityGroups `json:"managedSecurityGroups"` // AllowAllInClusterTraffic is only used when managed security groups are in use. // If set to true, the rules for the managed security groups are configured so that all @@ -191,13 +193,13 @@ type OpenStackClusterStatus struct { // ControlPlaneSecurityGroups contains all the information about the OpenStack // Security Group that needs to be applied to control plane nodes. // TODO: Maybe instead of two properties, we add a property to the group? - ControlPlaneSecurityGroup *SecurityGroup `json:"controlPlaneSecurityGroup,omitempty"` + ControlPlaneSecurityGroup *SecurityGroupStatus `json:"controlPlaneSecurityGroup,omitempty"` // WorkerSecurityGroup contains all the information about the OpenStack Security // Group that needs to be applied to worker nodes. - WorkerSecurityGroup *SecurityGroup `json:"workerSecurityGroup,omitempty"` + WorkerSecurityGroup *SecurityGroupStatus `json:"workerSecurityGroup,omitempty"` - BastionSecurityGroup *SecurityGroup `json:"bastionSecurityGroup,omitempty"` + BastionSecurityGroup *SecurityGroupStatus `json:"bastionSecurityGroup,omitempty"` Bastion *BastionStatus `json:"bastion,omitempty"` @@ -269,6 +271,17 @@ type OpenStackClusterList struct { Items []OpenStackCluster `json:"items"` } +// ManagedSecurityGroups defines the desired state of security groups and rules for the cluster. +type ManagedSecurityGroups struct { + // allNodesSecurityGroupRules defines the rules that should be applied to all nodes. + // +patchMergeKey=name + // +patchStrategy=merge + // +listType=map + // +listMapKey=name + // +optional + AllNodesSecurityGroupRules []SecurityGroupRuleSpec `json:"allNodesSecurityGroupRules" patchStrategy:"merge" patchMergeKey:"name"` +} + func init() { objectTypes = append(objectTypes, &OpenStackCluster{}, &OpenStackClusterList{}) } diff --git a/api/v1alpha8/openstackcluster_webhook.go b/api/v1alpha8/openstackcluster_webhook.go index b47c2ac612..d2051a09e5 100644 --- a/api/v1alpha8/openstackcluster_webhook.go +++ b/api/v1alpha8/openstackcluster_webhook.go @@ -63,6 +63,20 @@ func (r *OpenStackCluster) ValidateCreate() (admission.Warnings, error) { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "identityRef", "kind"), "must be a Secret")) } + if r.Spec.ManagedSecurityGroups != nil { + for _, rule := range r.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules { + if rule.RemoteManagedGroups != nil && (rule.RemoteGroupID != nil || rule.RemoteIPPrefix != nil) { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "managedSecurityGroups", "allNodesSecurityGroupRules"), "remoteManagedGroups cannot be used with remoteGroupID or remoteIPPrefix")) + } + if rule.RemoteGroupID != nil && (rule.RemoteManagedGroups != nil || rule.RemoteIPPrefix != nil) { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "managedSecurityGroups", "allNodesSecurityGroupRules"), "remoteGroupID cannot be used with remoteManagedGroups or remoteIPPrefix")) + } + if rule.RemoteIPPrefix != nil && (rule.RemoteManagedGroups != nil || rule.RemoteGroupID != nil) { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "managedSecurityGroups", "allNodesSecurityGroupRules"), "remoteIPPrefix cannot be used with remoteManagedGroups or remoteGroupID")) + } + } + } + return aggregateObjErrors(r.GroupVersionKind().GroupKind(), r.Name, allErrs) } @@ -127,6 +141,12 @@ func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warn old.Spec.Bastion = &Bastion{} r.Spec.Bastion = &Bastion{} + // Allow changes to the managed allNodesSecurityGroupRules. + if r.Spec.ManagedSecurityGroups != nil { + old.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules = []SecurityGroupRuleSpec{} + r.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules = []SecurityGroupRuleSpec{} + } + // Allow changes on AllowedCIDRs if r.Spec.APIServerLoadBalancer.Enabled { old.Spec.APIServerLoadBalancer.AllowedCIDRs = []string{} diff --git a/api/v1alpha8/openstackcluster_webhook_test.go b/api/v1alpha8/openstackcluster_webhook_test.go index 734a388763..3985e76605 100644 --- a/api/v1alpha8/openstackcluster_webhook_test.go +++ b/api/v1alpha8/openstackcluster_webhook_test.go @@ -20,6 +20,7 @@ import ( "testing" . "github.com/onsi/gomega" + "k8s.io/utils/pointer" ) func TestOpenStackCluster_ValidateUpdate(t *testing.T) { @@ -146,6 +147,44 @@ func TestOpenStackCluster_ValidateUpdate(t *testing.T) { }, wantErr: false, }, + { + name: "Changing security group rules on the OpenStackCluster.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules is allowed", + oldTemplate: &OpenStackCluster{ + Spec: OpenStackClusterSpec{ + CloudName: "foobar", + ManagedSecurityGroups: &ManagedSecurityGroups{ + AllNodesSecurityGroupRules: []SecurityGroupRuleSpec{ + { + Name: "foobar", + Description: pointer.String("foobar"), + PortRangeMin: pointer.Int(80), + PortRangeMax: pointer.Int(80), + Protocol: pointer.String("tcp"), + RemoteManagedGroups: []ManagedSecurityGroupName{"controlplane"}, + }, + }, + }, + }, + }, + newTemplate: &OpenStackCluster{ + Spec: OpenStackClusterSpec{ + CloudName: "foobar", + ManagedSecurityGroups: &ManagedSecurityGroups{ + AllNodesSecurityGroupRules: []SecurityGroupRuleSpec{ + { + Name: "foobar", + Description: pointer.String("foobar"), + PortRangeMin: pointer.Int(80), + PortRangeMax: pointer.Int(80), + Protocol: pointer.String("tcp"), + RemoteManagedGroups: []ManagedSecurityGroupName{"controlplane", "worker"}, + }, + }, + }, + }, + }, + wantErr: false, + }, { name: "Changing CIDRs on the OpenStackCluster.Spec.APIServerLoadBalancer.AllowedCIDRs is allowed", oldTemplate: &OpenStackCluster{ @@ -443,6 +482,48 @@ func TestOpenStackCluster_ValidateCreate(t *testing.T) { }, wantErr: true, }, + { + name: "OpenStackCluster.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules with correct spec on create", + template: &OpenStackCluster{ + Spec: OpenStackClusterSpec{ + CloudName: "foobar", + ManagedSecurityGroups: &ManagedSecurityGroups{ + AllNodesSecurityGroupRules: []SecurityGroupRuleSpec{ + { + Name: "foobar", + Description: pointer.String("foobar"), + PortRangeMin: pointer.Int(80), + PortRangeMax: pointer.Int(80), + Protocol: pointer.String("tcp"), + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "OpenStackCluster.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules with mutually exclusive fields on create", + template: &OpenStackCluster{ + Spec: OpenStackClusterSpec{ + CloudName: "foobar", + ManagedSecurityGroups: &ManagedSecurityGroups{ + AllNodesSecurityGroupRules: []SecurityGroupRuleSpec{ + { + Name: "foobar", + Description: pointer.String("foobar"), + PortRangeMin: pointer.Int(80), + PortRangeMax: pointer.Int(80), + Protocol: pointer.String("tcp"), + RemoteManagedGroups: []ManagedSecurityGroupName{"controlplane"}, + RemoteGroupID: pointer.String("foobar"), + }, + }, + }, + }, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/api/v1alpha8/types.go b/api/v1alpha8/types.go index 1278f83639..8c206d803d 100644 --- a/api/v1alpha8/types.go +++ b/api/v1alpha8/types.go @@ -301,39 +301,136 @@ type LoadBalancer struct { Tags []string `json:"tags,omitempty"` } -// SecurityGroup represents the basic information of the associated +// SecurityGroupStatus represents the basic information of the associated // OpenStack Neutron Security Group. -type SecurityGroup struct { - Name string `json:"name"` - ID string `json:"id"` - Rules []SecurityGroupRule `json:"rules"` +type SecurityGroupStatus struct { + // name of the security group + // +kubebuilder:validation:Required + Name string `json:"name"` + + // id of the security group + // +kubebuilder:validation:Required + ID string `json:"id"` + + // list of security group rules + // +optional + Rules []SecurityGroupRuleStatus `json:"rules,omitempty"` } -// SecurityGroupRule represent the basic information of the associated OpenStack +// SecurityGroupRuleSpec represent the basic information of the associated OpenStack // Security Group Role. -type SecurityGroupRule struct { - Description string `json:"description"` - ID string `json:"name"` - Direction string `json:"direction"` - EtherType string `json:"etherType"` - SecurityGroupID string `json:"securityGroupID"` - PortRangeMin int `json:"portRangeMin"` - PortRangeMax int `json:"portRangeMax"` - Protocol string `json:"protocol"` - RemoteGroupID string `json:"remoteGroupID"` - RemoteIPPrefix string `json:"remoteIPPrefix"` +// For now this is only used for the allNodesSecurityGroupRules but when we add +// other security groups, we'll need to add a validation because +// Remote* fields are mutually exclusive. +type SecurityGroupRuleSpec struct { + // name of the security group rule. + // It's used to identify the rule so it can be patched and will not be sent to the OpenStack API. + // +kubebuilder:validation:Required + Name string `json:"name"` + + // description of the security group rule. + // +optional + Description *string `json:"description,omitempty"` + + // direction in which the security group rule is applied. The only values + // allowed are "ingress" or "egress". For a compute instance, an ingress + // security group rule is applied to incoming (ingress) traffic for that + // instance. An egress rule is applied to traffic leaving the instance. + // +kubebuilder:validation:Required + // +kubebuilder:validation:enum=ingress;egress + Direction string `json:"direction"` + + // etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the + // ingress or egress rules. + // +kubebuilder:validation:enum=IPv4;IPv6 + // +optional + EtherType *string `json:"etherType,omitempty"` + + // portRangeMin is a number in the range that is matched by the security group + // rule. If the protocol is TCP or UDP, this value must be less than or equal + // to the value of the portRangeMax attribute. + // +optional + PortRangeMin *int `json:"portRangeMin,omitempty"` + + // portRangeMax is a number in the range that is matched by the security group + // rule. The portRangeMin attribute constrains the portRangeMax attribute. + // +optional + PortRangeMax *int `json:"portRangeMax,omitempty"` + + // protocol is the protocol that is matched by the security group rule. + // +optional + Protocol *string `json:"protocol,omitempty"` + + // remoteGroupID is the remote group ID to be associated with this security group rule. + // You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + // +optional + RemoteGroupID *string `json:"remoteGroupID,omitempty"` + + // remoteIPPrefix is the remote IP prefix to be associated with this security group rule. + // You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + // +optional + RemoteIPPrefix *string `json:"remoteIPPrefix,omitempty"` + + // remoteManagedGroups is the remote managed groups to be associated with this security group rule. + // You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + // +optional + RemoteManagedGroups []ManagedSecurityGroupName `json:"remoteManagedGroups,omitempty"` } -// Equal checks if two SecurityGroupRules are the same. -func (r SecurityGroupRule) Equal(x SecurityGroupRule) bool { - return (r.Direction == x.Direction && - r.Description == x.Description && - r.EtherType == x.EtherType && - r.PortRangeMin == x.PortRangeMin && - r.PortRangeMax == x.PortRangeMax && - r.Protocol == x.Protocol && - r.RemoteGroupID == x.RemoteGroupID && - r.RemoteIPPrefix == x.RemoteIPPrefix) +type SecurityGroupRuleStatus struct { + // id of the security group rule + // +kubebuilder:validation:Required + ID string `json:"id"` + + // description of the security group rule. + // +optional + Description *string `json:"description,omitempty"` + + // direction in which the security group rule is applied. The only values + // allowed are "ingress" or "egress". For a compute instance, an ingress + // security group rule is applied to incoming (ingress) traffic for that + // instance. An egress rule is applied to traffic leaving the instance. + // +kubebuilder:validation:Required + // +kubebuilder:validation:enum=ingress;egress + Direction string `json:"direction"` + + // etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the + // ingress or egress rules. + // +kubebuilder:validation:enum=IPv4;IPv6 + // +optional + EtherType *string `json:"etherType,omitempty"` + + // portRangeMin is a number in the range that is matched by the security group + // rule. If the protocol is TCP or UDP, this value must be less than or equal + // to the value of the portRangeMax attribute. + // +optional + PortRangeMin *int `json:"portRangeMin,omitempty"` + + // portRangeMax is a number in the range that is matched by the security group + // rule. The portRangeMin attribute constrains the portRangeMax attribute. + // +optional + PortRangeMax *int `json:"portRangeMax,omitempty"` + + // protocol is the protocol that is matched by the security group rule. + // +optional + Protocol *string `json:"protocol,omitempty"` + + // remoteGroupID is the remote group ID to be associated with this security group rule. + // You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + // +optional + RemoteGroupID *string `json:"remoteGroupID,omitempty"` + + // remoteIPPrefix is the remote IP prefix to be associated with this security group rule. + // You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + // +optional + RemoteIPPrefix *string `json:"remoteIPPrefix,omitempty"` +} + +// +kubebuilder:validation:Enum=bastion;controlplane;worker +type ManagedSecurityGroupName string + +func (m ManagedSecurityGroupName) String() string { + return string(m) } // InstanceState describes the state of an OpenStack instance. diff --git a/api/v1alpha8/zz_generated.deepcopy.go b/api/v1alpha8/zz_generated.deepcopy.go index 5be338cb7f..2b52b02cd2 100644 --- a/api/v1alpha8/zz_generated.deepcopy.go +++ b/api/v1alpha8/zz_generated.deepcopy.go @@ -246,6 +246,28 @@ func (in *LoadBalancer) DeepCopy() *LoadBalancer { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ManagedSecurityGroups) DeepCopyInto(out *ManagedSecurityGroups) { + *out = *in + if in.AllNodesSecurityGroupRules != nil { + in, out := &in.AllNodesSecurityGroupRules, &out.AllNodesSecurityGroupRules + *out = make([]SecurityGroupRuleSpec, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagedSecurityGroups. +func (in *ManagedSecurityGroups) DeepCopy() *ManagedSecurityGroups { + if in == nil { + return nil + } + out := new(ManagedSecurityGroups) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *NetworkFilter) DeepCopyInto(out *NetworkFilter) { *out = *in @@ -391,6 +413,11 @@ func (in *OpenStackClusterSpec) DeepCopyInto(out *OpenStackClusterSpec) { } out.ExternalNetwork = in.ExternalNetwork in.APIServerLoadBalancer.DeepCopyInto(&out.APIServerLoadBalancer) + if in.ManagedSecurityGroups != nil { + in, out := &in.ManagedSecurityGroups, &out.ManagedSecurityGroups + *out = new(ManagedSecurityGroups) + (*in).DeepCopyInto(*out) + } if in.Tags != nil { in, out := &in.Tags, &out.Tags *out = make([]string, len(*in)) @@ -456,17 +483,17 @@ func (in *OpenStackClusterStatus) DeepCopyInto(out *OpenStackClusterStatus) { } if in.ControlPlaneSecurityGroup != nil { in, out := &in.ControlPlaneSecurityGroup, &out.ControlPlaneSecurityGroup - *out = new(SecurityGroup) + *out = new(SecurityGroupStatus) (*in).DeepCopyInto(*out) } if in.WorkerSecurityGroup != nil { in, out := &in.WorkerSecurityGroup, &out.WorkerSecurityGroup - *out = new(SecurityGroup) + *out = new(SecurityGroupStatus) (*in).DeepCopyInto(*out) } if in.BastionSecurityGroup != nil { in, out := &in.BastionSecurityGroup, &out.BastionSecurityGroup - *out = new(SecurityGroup) + *out = new(SecurityGroupStatus) (*in).DeepCopyInto(*out) } if in.Bastion != nil { @@ -1007,51 +1034,143 @@ func (in *RouterFilter) DeepCopy() *RouterFilter { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SecurityGroup) DeepCopyInto(out *SecurityGroup) { +func (in *SecurityGroupFilter) DeepCopyInto(out *SecurityGroupFilter) { *out = *in - if in.Rules != nil { - in, out := &in.Rules, &out.Rules - *out = make([]SecurityGroupRule, len(*in)) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupFilter. +func (in *SecurityGroupFilter) DeepCopy() *SecurityGroupFilter { + if in == nil { + return nil + } + out := new(SecurityGroupFilter) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecurityGroupRuleSpec) DeepCopyInto(out *SecurityGroupRuleSpec) { + *out = *in + if in.Description != nil { + in, out := &in.Description, &out.Description + *out = new(string) + **out = **in + } + if in.EtherType != nil { + in, out := &in.EtherType, &out.EtherType + *out = new(string) + **out = **in + } + if in.PortRangeMin != nil { + in, out := &in.PortRangeMin, &out.PortRangeMin + *out = new(int) + **out = **in + } + if in.PortRangeMax != nil { + in, out := &in.PortRangeMax, &out.PortRangeMax + *out = new(int) + **out = **in + } + if in.Protocol != nil { + in, out := &in.Protocol, &out.Protocol + *out = new(string) + **out = **in + } + if in.RemoteGroupID != nil { + in, out := &in.RemoteGroupID, &out.RemoteGroupID + *out = new(string) + **out = **in + } + if in.RemoteIPPrefix != nil { + in, out := &in.RemoteIPPrefix, &out.RemoteIPPrefix + *out = new(string) + **out = **in + } + if in.RemoteManagedGroups != nil { + in, out := &in.RemoteManagedGroups, &out.RemoteManagedGroups + *out = make([]ManagedSecurityGroupName, len(*in)) copy(*out, *in) } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroup. -func (in *SecurityGroup) DeepCopy() *SecurityGroup { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupRuleSpec. +func (in *SecurityGroupRuleSpec) DeepCopy() *SecurityGroupRuleSpec { if in == nil { return nil } - out := new(SecurityGroup) + out := new(SecurityGroupRuleSpec) in.DeepCopyInto(out) return out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SecurityGroupFilter) DeepCopyInto(out *SecurityGroupFilter) { +func (in *SecurityGroupRuleStatus) DeepCopyInto(out *SecurityGroupRuleStatus) { *out = *in + if in.Description != nil { + in, out := &in.Description, &out.Description + *out = new(string) + **out = **in + } + if in.EtherType != nil { + in, out := &in.EtherType, &out.EtherType + *out = new(string) + **out = **in + } + if in.PortRangeMin != nil { + in, out := &in.PortRangeMin, &out.PortRangeMin + *out = new(int) + **out = **in + } + if in.PortRangeMax != nil { + in, out := &in.PortRangeMax, &out.PortRangeMax + *out = new(int) + **out = **in + } + if in.Protocol != nil { + in, out := &in.Protocol, &out.Protocol + *out = new(string) + **out = **in + } + if in.RemoteGroupID != nil { + in, out := &in.RemoteGroupID, &out.RemoteGroupID + *out = new(string) + **out = **in + } + if in.RemoteIPPrefix != nil { + in, out := &in.RemoteIPPrefix, &out.RemoteIPPrefix + *out = new(string) + **out = **in + } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupFilter. -func (in *SecurityGroupFilter) DeepCopy() *SecurityGroupFilter { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupRuleStatus. +func (in *SecurityGroupRuleStatus) DeepCopy() *SecurityGroupRuleStatus { if in == nil { return nil } - out := new(SecurityGroupFilter) + out := new(SecurityGroupRuleStatus) in.DeepCopyInto(out) return out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *SecurityGroupRule) DeepCopyInto(out *SecurityGroupRule) { +func (in *SecurityGroupStatus) DeepCopyInto(out *SecurityGroupStatus) { *out = *in + if in.Rules != nil { + in, out := &in.Rules, &out.Rules + *out = make([]SecurityGroupRuleStatus, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupRule. -func (in *SecurityGroupRule) DeepCopy() *SecurityGroupRule { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecurityGroupStatus. +func (in *SecurityGroupStatus) DeepCopy() *SecurityGroupStatus { if in == nil { return nil } - out := new(SecurityGroupRule) + out := new(SecurityGroupStatus) in.DeepCopyInto(out) return out } 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 35f142e9ff..a9d2d44d11 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -5456,9 +5456,87 @@ spec: ManagedSecurityGroups determines whether OpenStack security groups for the cluster will be managed by the OpenStack provider or whether pre-existing security groups will be specified as part of the configuration. - By default, the managed security groups have rules that allow the Kubelet, etcd, the - Kubernetes API server and the Calico CNI plugin to function correctly. - type: boolean + By default, the managed security groups have rules that allow the Kubelet, etcd, and the + Kubernetes API server to function correctly. + It's possible to add additional rules to the managed security groups. + When defined to an empty struct, the managed security groups will be created with the default rules. + properties: + allNodesSecurityGroupRules: + description: allNodesSecurityGroupRules defines the rules that + should be applied to all nodes. + items: + description: |- + SecurityGroupRuleSpec represent the basic information of the associated OpenStack + Security Group Role. + For now this is only used for the allNodesSecurityGroupRules but when we add + other security groups, we'll need to add a validation because + Remote* fields are mutually exclusive. + properties: + description: + description: description of the security group rule. + type: string + direction: + description: |- + direction in which the security group rule is applied. The only values + allowed are "ingress" or "egress". For a compute instance, an ingress + security group rule is applied to incoming (ingress) traffic for that + instance. An egress rule is applied to traffic leaving the instance. + type: string + etherType: + description: |- + etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the + ingress or egress rules. + type: string + name: + description: |- + name of the security group rule. + It's used to identify the rule so it can be patched and will not be sent to the OpenStack API. + type: string + portRangeMax: + description: |- + portRangeMax is a number in the range that is matched by the security group + rule. The portRangeMin attribute constrains the portRangeMax attribute. + type: integer + portRangeMin: + description: |- + portRangeMin is a number in the range that is matched by the security group + rule. If the protocol is TCP or UDP, this value must be less than or equal + to the value of the portRangeMax attribute. + type: integer + protocol: + description: protocol is the protocol that is matched by + the security group rule. + type: string + remoteGroupID: + description: |- + remoteGroupID is the remote group ID to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + type: string + remoteIPPrefix: + description: |- + remoteIPPrefix is the remote IP prefix to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + type: string + remoteManagedGroups: + description: |- + remoteManagedGroups is the remote managed groups to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + items: + enum: + - bastion + - controlplane + - worker + type: string + type: array + required: + - direction + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object managedSubnets: description: |- ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network, @@ -5640,56 +5718,70 @@ spec: type: object bastionSecurityGroup: description: |- - SecurityGroup represents the basic information of the associated + SecurityGroupStatus represents the basic information of the associated OpenStack Neutron Security Group. properties: id: + description: id of the security group type: string name: + description: name of the security group type: string rules: + description: list of security group rules items: - description: |- - SecurityGroupRule represent the basic information of the associated OpenStack - Security Group Role. properties: description: + description: description of the security group rule. type: string direction: + description: |- + direction in which the security group rule is applied. The only values + allowed are "ingress" or "egress". For a compute instance, an ingress + security group rule is applied to incoming (ingress) traffic for that + instance. An egress rule is applied to traffic leaving the instance. type: string etherType: + description: |- + etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the + ingress or egress rules. type: string - name: + id: + description: id of the security group rule type: string portRangeMax: + description: |- + portRangeMax is a number in the range that is matched by the security group + rule. The portRangeMin attribute constrains the portRangeMax attribute. type: integer portRangeMin: + description: |- + portRangeMin is a number in the range that is matched by the security group + rule. If the protocol is TCP or UDP, this value must be less than or equal + to the value of the portRangeMax attribute. type: integer protocol: + description: protocol is the protocol that is matched by + the security group rule. type: string remoteGroupID: + description: |- + remoteGroupID is the remote group ID to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. type: string remoteIPPrefix: - type: string - securityGroupID: + description: |- + remoteIPPrefix is the remote IP prefix to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. type: string required: - - description - direction - - etherType - - name - - portRangeMax - - portRangeMin - - protocol - - remoteGroupID - - remoteIPPrefix - - securityGroupID + - id type: object type: array required: - id - name - - rules type: object controlPlaneSecurityGroup: description: |- @@ -5698,52 +5790,66 @@ spec: TODO: Maybe instead of two properties, we add a property to the group? properties: id: + description: id of the security group type: string name: + description: name of the security group type: string rules: + description: list of security group rules items: - description: |- - SecurityGroupRule represent the basic information of the associated OpenStack - Security Group Role. properties: description: + description: description of the security group rule. type: string direction: + description: |- + direction in which the security group rule is applied. The only values + allowed are "ingress" or "egress". For a compute instance, an ingress + security group rule is applied to incoming (ingress) traffic for that + instance. An egress rule is applied to traffic leaving the instance. type: string etherType: + description: |- + etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the + ingress or egress rules. type: string - name: + id: + description: id of the security group rule type: string portRangeMax: + description: |- + portRangeMax is a number in the range that is matched by the security group + rule. The portRangeMin attribute constrains the portRangeMax attribute. type: integer portRangeMin: + description: |- + portRangeMin is a number in the range that is matched by the security group + rule. If the protocol is TCP or UDP, this value must be less than or equal + to the value of the portRangeMax attribute. type: integer protocol: + description: protocol is the protocol that is matched by + the security group rule. type: string remoteGroupID: + description: |- + remoteGroupID is the remote group ID to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. type: string remoteIPPrefix: - type: string - securityGroupID: + description: |- + remoteIPPrefix is the remote IP prefix to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. type: string required: - - description - direction - - etherType - - name - - portRangeMax - - portRangeMin - - protocol - - remoteGroupID - - remoteIPPrefix - - securityGroupID + - id type: object type: array required: - id - name - - rules type: object externalNetwork: description: externalNetwork contains information about the external @@ -5891,52 +5997,66 @@ spec: Group that needs to be applied to worker nodes. properties: id: + description: id of the security group type: string name: + description: name of the security group type: string rules: + description: list of security group rules items: - description: |- - SecurityGroupRule represent the basic information of the associated OpenStack - Security Group Role. properties: description: + description: description of the security group rule. type: string direction: + description: |- + direction in which the security group rule is applied. The only values + allowed are "ingress" or "egress". For a compute instance, an ingress + security group rule is applied to incoming (ingress) traffic for that + instance. An egress rule is applied to traffic leaving the instance. type: string etherType: + description: |- + etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the + ingress or egress rules. type: string - name: + id: + description: id of the security group rule type: string portRangeMax: + description: |- + portRangeMax is a number in the range that is matched by the security group + rule. The portRangeMin attribute constrains the portRangeMax attribute. type: integer portRangeMin: + description: |- + portRangeMin is a number in the range that is matched by the security group + rule. If the protocol is TCP or UDP, this value must be less than or equal + to the value of the portRangeMax attribute. type: integer protocol: + description: protocol is the protocol that is matched by + the security group rule. type: string remoteGroupID: + description: |- + remoteGroupID is the remote group ID to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. type: string remoteIPPrefix: - type: string - securityGroupID: + description: |- + remoteIPPrefix is the remote IP prefix to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. type: string required: - - description - direction - - etherType - - name - - portRangeMax - - portRangeMin - - protocol - - remoteGroupID - - remoteIPPrefix - - securityGroupID + - id type: object type: array required: - id - name - - rules type: object required: - ready 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 b0dcf7fac1..10a79fa903 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2891,9 +2891,87 @@ spec: ManagedSecurityGroups determines whether OpenStack security groups for the cluster will be managed by the OpenStack provider or whether pre-existing security groups will be specified as part of the configuration. - By default, the managed security groups have rules that allow the Kubelet, etcd, the - Kubernetes API server and the Calico CNI plugin to function correctly. - type: boolean + By default, the managed security groups have rules that allow the Kubelet, etcd, and the + Kubernetes API server to function correctly. + It's possible to add additional rules to the managed security groups. + When defined to an empty struct, the managed security groups will be created with the default rules. + properties: + allNodesSecurityGroupRules: + description: allNodesSecurityGroupRules defines the rules + that should be applied to all nodes. + items: + description: |- + SecurityGroupRuleSpec represent the basic information of the associated OpenStack + Security Group Role. + For now this is only used for the allNodesSecurityGroupRules but when we add + other security groups, we'll need to add a validation because + Remote* fields are mutually exclusive. + properties: + description: + description: description of the security group rule. + type: string + direction: + description: |- + direction in which the security group rule is applied. The only values + allowed are "ingress" or "egress". For a compute instance, an ingress + security group rule is applied to incoming (ingress) traffic for that + instance. An egress rule is applied to traffic leaving the instance. + type: string + etherType: + description: |- + etherType must be IPv4 or IPv6, and addresses represented in CIDR must match the + ingress or egress rules. + type: string + name: + description: |- + name of the security group rule. + It's used to identify the rule so it can be patched and will not be sent to the OpenStack API. + type: string + portRangeMax: + description: |- + portRangeMax is a number in the range that is matched by the security group + rule. The portRangeMin attribute constrains the portRangeMax attribute. + type: integer + portRangeMin: + description: |- + portRangeMin is a number in the range that is matched by the security group + rule. If the protocol is TCP or UDP, this value must be less than or equal + to the value of the portRangeMax attribute. + type: integer + protocol: + description: protocol is the protocol that is matched + by the security group rule. + type: string + remoteGroupID: + description: |- + remoteGroupID is the remote group ID to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + type: string + remoteIPPrefix: + description: |- + remoteIPPrefix is the remote IP prefix to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + type: string + remoteManagedGroups: + description: |- + remoteManagedGroups is the remote managed groups to be associated with this security group rule. + You can specify either remoteGroupID or remoteIPPrefix or remoteManagedGroups. + items: + enum: + - bastion + - controlplane + - worker + type: string + type: array + required: + - direction + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object managedSubnets: description: |- ManagedSubnets describe OpenStack Subnets to be created. Cluster actuator will create a network, diff --git a/controllers/openstackcluster_controller.go b/controllers/openstackcluster_controller.go index 6cd037383c..5246ba218d 100644 --- a/controllers/openstackcluster_controller.go +++ b/controllers/openstackcluster_controller.go @@ -462,7 +462,7 @@ func bastionToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, clusterNa } instanceSpec.SecurityGroups = openStackCluster.Spec.Bastion.Instance.SecurityGroups - if openStackCluster.Spec.ManagedSecurityGroups { + if openStackCluster.Spec.ManagedSecurityGroups != nil { if openStackCluster.Status.BastionSecurityGroup != nil { instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupFilter{ ID: openStackCluster.Status.BastionSecurityGroup.ID, diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go index 5d6095177c..71972fdeff 100644 --- a/controllers/openstackmachine_controller.go +++ b/controllers/openstackmachine_controller.go @@ -533,7 +533,7 @@ func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine * instanceSpec.Tags = machineTags instanceSpec.SecurityGroups = openStackMachine.Spec.SecurityGroups - if openStackCluster.Spec.ManagedSecurityGroups { + if openStackCluster.Spec.ManagedSecurityGroups != nil { var managedSecurityGroup string if util.IsControlPlaneMachine(machine) { if openStackCluster.Status.ControlPlaneSecurityGroup != nil { diff --git a/controllers/openstackmachine_controller_test.go b/controllers/openstackmachine_controller_test.go index b204111708..32db7b92bb 100644 --- a/controllers/openstackmachine_controller_test.go +++ b/controllers/openstackmachine_controller_test.go @@ -56,8 +56,8 @@ func getDefaultOpenStackCluster() *infrav1.OpenStackCluster { {ID: subnetUUID}, }, }, - ControlPlaneSecurityGroup: &infrav1.SecurityGroup{ID: controlPlaneSecurityGroupUUID}, - WorkerSecurityGroup: &infrav1.SecurityGroup{ID: workerSecurityGroupUUID}, + ControlPlaneSecurityGroup: &infrav1.SecurityGroupStatus{ID: controlPlaneSecurityGroupUUID}, + WorkerSecurityGroup: &infrav1.SecurityGroupStatus{ID: workerSecurityGroupUUID}, }, } } @@ -141,7 +141,7 @@ func Test_machineToInstanceSpec(t *testing.T) { name: "Control plane security group", openStackCluster: func() *infrav1.OpenStackCluster { c := getDefaultOpenStackCluster() - c.Spec.ManagedSecurityGroups = true + c.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} return c }, machine: func() *clusterv1.Machine { @@ -162,7 +162,7 @@ func Test_machineToInstanceSpec(t *testing.T) { name: "Worker security group", openStackCluster: func() *infrav1.OpenStackCluster { c := getDefaultOpenStackCluster() - c.Spec.ManagedSecurityGroups = true + c.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} return c }, machine: getDefaultMachine, @@ -177,7 +177,7 @@ func Test_machineToInstanceSpec(t *testing.T) { name: "Control plane security group not applied to worker", openStackCluster: func() *infrav1.OpenStackCluster { c := getDefaultOpenStackCluster() - c.Spec.ManagedSecurityGroups = true + c.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} c.Status.WorkerSecurityGroup = nil return c }, @@ -193,7 +193,7 @@ func Test_machineToInstanceSpec(t *testing.T) { name: "Worker security group not applied to control plane", openStackCluster: func() *infrav1.OpenStackCluster { c := getDefaultOpenStackCluster() - c.Spec.ManagedSecurityGroups = true + c.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} c.Status.ControlPlaneSecurityGroup = nil return c }, @@ -215,7 +215,7 @@ func Test_machineToInstanceSpec(t *testing.T) { name: "Extra security group", openStackCluster: func() *infrav1.OpenStackCluster { c := getDefaultOpenStackCluster() - c.Spec.ManagedSecurityGroups = true + c.Spec.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} return c }, machine: getDefaultMachine, diff --git a/docs/book/src/clusteropenstack/configuration.md b/docs/book/src/clusteropenstack/configuration.md index 46c7b00363..18f1269f66 100644 --- a/docs/book/src/clusteropenstack/configuration.md +++ b/docs/book/src/clusteropenstack/configuration.md @@ -16,6 +16,7 @@ - [Availability zone](#availability-zone) - [DNS server](#dns-server) - [Machine flavor](#machine-flavor) + - [CNI security group rules](#cni-security-group-rules) - [Optional Configuration](#optional-configuration) - [Log level](#log-level) - [External network](#external-network) @@ -180,6 +181,41 @@ The flavors for control plane and worker node machines must be exposed as enviro The recommmend minimum value of control plane flavor's vCPU is 2 and minimum value of worker node flavor's vCPU is 1. +## CNI security group rules + +Depending on the CNI that will be deployed on the cluster, you may need to add specific security group rules to the control plane and worker nodes. For example, if you are using Calico with BGP, you will need to add the following security group rules to the control plane and worker nodes: + + ```yaml + apiVersion: infrastructure.cluster.x-k8s.io/v1alpha8 + kind: OpenStackCluster + metadata: + name: + namespace: + spec: + ... + allowAllInClusterTraffic: false + managedSecurityGroups: + allNodesSecurityGroupRules: + - remoteManagedGroups: + - controlplane + - worker + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMin: 179 + portRangeMax: 179 + protocol: tcp + description: "Allow BGP between control plane and workers" + - remoteManagedGroups: + - controlplane + - worker + direction: ingress + etherType: IPv4 + name: IP-in-IP (Calico) + protocol: 4 + description: "Allow IP-in-IP between control plane and workers" + ``` + # Optional Configuration ## Log level @@ -503,28 +539,53 @@ spec: Security groups are used to determine which ports of the cluster nodes are accessible from where. -If `spec.managedSecurityGroups` of `OpenStackCluster` is set to `true`, two security groups named +If `spec.managedSecurityGroups` of `OpenStackCluster` is set to a non-nil value (e.g. `{}`), two security groups named `k8s-cluster-${NAMESPACE}-${CLUSTER_NAME}-secgroup-controlplane` and `k8s-cluster-${NAMESPACE}-${CLUSTER_NAME}-secgroup-worker` will be created and added to the control plane and worker nodes respectively. -By default, these groups have rules that allow the following traffic: +Example of `spec.managedSecurityGroups` in `OpenStackCluster` spec when we want to enable the managed security groups: + +```yaml +managedSecurityGroups: {} +``` - Control plane nodes - API server traffic from anywhere - Etcd traffic from other control plane nodes - Kubelet traffic from other cluster nodes - - Calico CNI traffic from other cluster nodes - Worker nodes - Node port traffic from anywhere - Kubelet traffic from other cluster nodes - - Calico CNI traffic from other cluster nodes -To use a CNI other than Calico, the flag `OpenStackCluster.spec.allowAllInClusterTraffic` can be -set to `true`. With this flag set, the rules for the managed security groups permit all traffic +When the flag `OpenStackCluster.spec.allowAllInClusterTraffic` is +set to `true`, the rules for the managed security groups permit all traffic between cluster nodes on all ports and protocols (API server and node port traffic is still permitted from anywhere, as with the default rules). +We can add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`. +It takes a list of security groups rules that should be applied to selected nodes. +The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`. + +Valid values for `remoteManagedGroups` are `controlplane`, `worker` and `bastion`. + +To apply a security group rule that will allow BGP between the control plane and workers, you can follow this example: + +```yaml +managedSecurityGroups: + allNodesSecurityGroupRules: + - remoteManagedGroups: + - controlplane + - worker + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMin: 179 + portRangeMax: 179 + protocol: tcp + description: "Allow BGP between control plane and workers" + ``` + If this is not flexible enough, pre-existing security groups can be added to the spec of an `OpenStackMachineTemplate`, e.g.: @@ -654,7 +715,7 @@ spec: floatingIP: ``` -If `managedSecurityGroups: true`, security group rule opening 22/tcp is added to security groups for bastion, controller, and worker nodes respectively. Otherwise, you have to add `securityGroups` to the `bastion` in `OpenStackCluster` spec and `OpenStackMachineTemplate` spec template respectively. +If `managedSecurityGroups` is set to a non-nil value (e.g. `{}`), security group rule opening 22/tcp is added to security groups for bastion, controller, and worker nodes respectively. Otherwise, you have to add `securityGroups` to the `bastion` in `OpenStackCluster` spec and `OpenStackMachineTemplate` spec template respectively. ### Making changes to the bastion host diff --git a/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md b/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md index 37df012a8c..be1b3b63c8 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md @@ -13,6 +13,9 @@ - [Removal of imageUUID](#removal-of-imageuuid) - [Change to floatingIP](#change-to-floatingip) - [Change to subnet](#change-to-subnet) + - [Change to nodeCidr and dnsNameservers](#change-to-nodecidr-and-dnsnameservers) + - [Change to managedSecurityGroups](#change-to-managedsecuritygroups) + - [Calico CNI](#calico-cni) @@ -195,4 +198,48 @@ In v1alpha8, this will be automatically converted to: dnsNameservers: "10.0.0.123" ``` -Please note that currently `managedSubnets` can only hold one element. \ No newline at end of file +Please note that currently `managedSubnets` can only hold one element. + +#### ⚠️ Change to managedSecurityGroups + +The field `managedSecurityGroups` is now a pointer to a `ManagedSecurityGroups` object rather than a boolean. + +Also, we can now add security group rules that authorize traffic from all nodes via `allNodesSecurityGroupRules`. +It takes a list of security groups rules that should be applied to selected nodes. +The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`. +Valid values for `remoteManagedGroups` are `controlplane`, `worker` and `bastion`. + +```yaml +managedSecurityGroups: true +``` + +becomes + +```yaml +managedSecurityGroups: {} +``` + +To apply a security group rule that will allow BGP between the control plane and workers, you can follow this example: + +```yaml +managedSecurityGroups: + allNodesSecurityGroupRules: + - remoteManagedGroups: + - controlplane + - worker + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMin: 179 + portRangeMax: 179 + protocol: tcp + description: "Allow BGP between control plane and workers" +``` + +#### ⚠️ Calico CNI + +Historically we used to create the necessary security group rules for Calico CNI to work. This is no longer the case. +Now the user needs to request creation of the security group rules by using the `managedSecurityGroups.allNodesSecurityGroupRules` feature. + +Note that when upgrading from a previous version, the Calico CNI security group rules will be added automatically to +allow backwards compatibility if `allowAllInClusterTraffic` is set to false. diff --git a/kustomize/v1alpha8/default/cluster-template.yaml b/kustomize/v1alpha8/default/cluster-template.yaml index 5482f50448..c7554d7d7c 100644 --- a/kustomize/v1alpha8/default/cluster-template.yaml +++ b/kustomize/v1alpha8/default/cluster-template.yaml @@ -28,11 +28,30 @@ spec: kind: Secret apiServerLoadBalancer: enabled: true - managedSecurityGroups: true managedSubnets: - cidr: 10.6.0.0/24 dnsNameservers: - ${OPENSTACK_DNS_NAMESERVERS} + managedSecurityGroups: + allNodesSecurityGroupRules: + - description: Created by cluster-api-provider-openstack - BGP (calico) + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMax: 179 + portRangeMin: 179 + protocol: tcp + remoteManagedGroups: + - controlplane + - worker + - description: Created by cluster-api-provider-openstack - IP-in-IP (calico) + direction: ingress + etherType: IPv4 + name: IP-in-IP (calico) + protocol: "4" + remoteManagedGroups: + - controlplane + - worker externalNetwork: id: ${OPENSTACK_EXTERNAL_NETWORK_ID} --- diff --git a/pkg/cloud/services/compute/instance_test.go b/pkg/cloud/services/compute/instance_test.go index d868e47940..d89849f507 100644 --- a/pkg/cloud/services/compute/instance_test.go +++ b/pkg/cloud/services/compute/instance_test.go @@ -252,8 +252,8 @@ func getDefaultOpenStackCluster() *infrav1.OpenStackCluster { {ID: subnetUUID}, }, }, - ControlPlaneSecurityGroup: &infrav1.SecurityGroup{ID: controlPlaneSecurityGroupUUID}, - WorkerSecurityGroup: &infrav1.SecurityGroup{ID: workerSecurityGroupUUID}, + ControlPlaneSecurityGroup: &infrav1.SecurityGroupStatus{ID: controlPlaneSecurityGroupUUID}, + WorkerSecurityGroup: &infrav1.SecurityGroupStatus{ID: workerSecurityGroupUUID}, }, } } diff --git a/pkg/cloud/services/networking/securitygroups.go b/pkg/cloud/services/networking/securitygroups.go index e2a5617de6..03c63327d3 100644 --- a/pkg/cloud/services/networking/securitygroups.go +++ b/pkg/cloud/services/networking/securitygroups.go @@ -32,13 +32,14 @@ const ( controlPlaneSuffix string = "controlplane" workerSuffix string = "worker" bastionSuffix string = "bastion" + allNodesSuffix string = "allNodes" remoteGroupIDSelf string = "self" ) // ReconcileSecurityGroups reconcile the security groups. func (s *Service) ReconcileSecurityGroups(openStackCluster *infrav1.OpenStackCluster, clusterName string) error { s.scope.Logger().Info("Reconciling security groups") - if !openStackCluster.Spec.ManagedSecurityGroups { + if openStackCluster.Spec.ManagedSecurityGroups == nil { s.scope.Logger().V(4).Info("No need to reconcile security groups") return nil } @@ -67,7 +68,7 @@ func (s *Service) ReconcileSecurityGroups(openStackCluster *infrav1.OpenStackClu return err } - observedSecGroups := make(map[string]*infrav1.SecurityGroup) + observedSecGroups := make(map[string]*infrav1.SecurityGroupStatus) for k, desiredSecGroup := range desiredSecGroups { var err error observedSecGroups[k], err = s.getSecurityGroupByName(desiredSecGroup.Name) @@ -93,12 +94,50 @@ func (s *Service) ReconcileSecurityGroups(openStackCluster *infrav1.OpenStackClu return nil } -func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCluster, secGroupNames map[string]string) (map[string]infrav1.SecurityGroup, error) { - desiredSecGroups := make(map[string]infrav1.SecurityGroup) +type securityGroupSpec struct { + Name string + Rules []resolvedSecurityGroupRuleSpec +} + +type resolvedSecurityGroupRuleSpec struct { + Description string `json:"description,omitempty"` + Direction string `json:"direction,omitempty"` + EtherType string `json:"etherType,omitempty"` + PortRangeMin int `json:"portRangeMin,omitempty"` + PortRangeMax int `json:"portRangeMax,omitempty"` + Protocol string `json:"protocol,omitempty"` + RemoteGroupID string `json:"remoteGroupID,omitempty"` + RemoteIPPrefix string `json:"remoteIPPrefix,omitempty"` +} + +func (r resolvedSecurityGroupRuleSpec) Matches(other infrav1.SecurityGroupRuleStatus) bool { + return r.Description == *other.Description && + r.Direction == other.Direction && + r.EtherType == *other.EtherType && + r.PortRangeMin == *other.PortRangeMin && + r.PortRangeMax == *other.PortRangeMax && + r.Protocol == *other.Protocol && + r.RemoteGroupID == *other.RemoteGroupID && + r.RemoteIPPrefix == *other.RemoteIPPrefix +} + +func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCluster, secGroupNames map[string]string) (map[string]securityGroupSpec, error) { + if openStackCluster.Spec.ManagedSecurityGroups == nil { + return nil, nil + } + + desiredSecGroups := make(map[string]securityGroupSpec) var secControlPlaneGroupID string var secWorkerGroupID string var secBastionGroupID string + + // remoteManagedGroups is a map of suffix to security group ID. + // It will be used to fill in the RemoteGroupID field of the security group rules + // that reference a managed security group. + // For now, we only reference the control plane and worker security groups. + remoteManagedGroups := make(map[string]string) + for i, v := range secGroupNames { secGroup, err := s.getSecurityGroupByName(v) if err != nil { @@ -107,42 +146,54 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl switch i { case controlPlaneSuffix: secControlPlaneGroupID = secGroup.ID + remoteManagedGroups[controlPlaneSuffix] = secControlPlaneGroupID case workerSuffix: secWorkerGroupID = secGroup.ID + remoteManagedGroups[workerSuffix] = secWorkerGroupID case bastionSuffix: secBastionGroupID = secGroup.ID + remoteManagedGroups[bastionSuffix] = secBastionGroupID } } // Start with the default rules - controlPlaneRules := append([]infrav1.SecurityGroupRule{}, defaultRules...) - workerRules := append([]infrav1.SecurityGroupRule{}, defaultRules...) + controlPlaneRules := append([]resolvedSecurityGroupRuleSpec{}, defaultRules...) + workerRules := append([]resolvedSecurityGroupRuleSpec{}, defaultRules...) - controlPlaneRules = append(controlPlaneRules, GetSGControlPlaneHTTPS()...) - workerRules = append(workerRules, GetSGWorkerNodePort()...) + controlPlaneRules = append(controlPlaneRules, getSGControlPlaneHTTPS()...) + workerRules = append(workerRules, getSGWorkerNodePort()...) // If we set additional ports to LB, we need create secgroup rules those ports, this apply to controlPlaneRules only if openStackCluster.Spec.APIServerLoadBalancer.Enabled { - controlPlaneRules = append(controlPlaneRules, GetSGControlPlaneAdditionalPorts(openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts)...) + controlPlaneRules = append(controlPlaneRules, getSGControlPlaneAdditionalPorts(openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts)...) } if openStackCluster.Spec.AllowAllInClusterTraffic { // Permit all ingress from the cluster security groups - controlPlaneRules = append(controlPlaneRules, GetSGControlPlaneAllowAll(remoteGroupIDSelf, secWorkerGroupID)...) - workerRules = append(workerRules, GetSGWorkerAllowAll(remoteGroupIDSelf, secControlPlaneGroupID)...) + controlPlaneRules = append(controlPlaneRules, getSGControlPlaneAllowAll(remoteGroupIDSelf, secWorkerGroupID)...) + workerRules = append(workerRules, getSGWorkerAllowAll(remoteGroupIDSelf, secControlPlaneGroupID)...) } else { - controlPlaneRules = append(controlPlaneRules, GetSGControlPlaneGeneral(remoteGroupIDSelf, secWorkerGroupID)...) - workerRules = append(workerRules, GetSGWorkerGeneral(remoteGroupIDSelf, secControlPlaneGroupID)...) + controlPlaneRules = append(controlPlaneRules, getSGControlPlaneGeneral(remoteGroupIDSelf, secWorkerGroupID)...) + workerRules = append(workerRules, getSGWorkerGeneral(remoteGroupIDSelf, secControlPlaneGroupID)...) } + // For now, we do not create a separate security group for allNodes. + // Instead, we append the rules for allNodes to the control plane and worker security groups. + allNodesRules, err := getAllNodesRules(remoteManagedGroups, openStackCluster.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules) + if err != nil { + return desiredSecGroups, err + } + controlPlaneRules = append(controlPlaneRules, allNodesRules...) + workerRules = append(workerRules, allNodesRules...) + if openStackCluster.Spec.Bastion != nil && openStackCluster.Spec.Bastion.Enabled { - controlPlaneRules = append(controlPlaneRules, GetSGControlPlaneSSH(secBastionGroupID)...) - workerRules = append(workerRules, GetSGWorkerSSH(secBastionGroupID)...) + controlPlaneRules = append(controlPlaneRules, getSGControlPlaneSSH(secBastionGroupID)...) + workerRules = append(workerRules, getSGWorkerSSH(secBastionGroupID)...) - desiredSecGroups[bastionSuffix] = infrav1.SecurityGroup{ + desiredSecGroups[bastionSuffix] = securityGroupSpec{ Name: secGroupNames[bastionSuffix], Rules: append( - []infrav1.SecurityGroupRule{ + []resolvedSecurityGroupRuleSpec{ { Description: "SSH", Direction: "ingress", @@ -157,19 +208,81 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl } } - desiredSecGroups[controlPlaneSuffix] = infrav1.SecurityGroup{ + desiredSecGroups[controlPlaneSuffix] = securityGroupSpec{ Name: secGroupNames[controlPlaneSuffix], Rules: controlPlaneRules, } - desiredSecGroups[workerSuffix] = infrav1.SecurityGroup{ + desiredSecGroups[workerSuffix] = securityGroupSpec{ Name: secGroupNames[workerSuffix], Rules: workerRules, } - return desiredSecGroups, nil } +// getAllNodesRules returns the rules for the allNodes security group that should be created. +func getAllNodesRules(remoteManagedGroups map[string]string, allNodesSecurityGroupRules []infrav1.SecurityGroupRuleSpec) ([]resolvedSecurityGroupRuleSpec, error) { + rules := make([]resolvedSecurityGroupRuleSpec, 0, len(allNodesSecurityGroupRules)) + for _, rule := range allNodesSecurityGroupRules { + if err := validateRemoteManagedGroups(remoteManagedGroups, rule.RemoteManagedGroups); err != nil { + return nil, err + } + r := resolvedSecurityGroupRuleSpec{ + Direction: rule.Direction, + } + if rule.Description != nil { + r.Description = *rule.Description + } + if rule.EtherType != nil { + r.EtherType = *rule.EtherType + } + if rule.PortRangeMin != nil { + r.PortRangeMin = *rule.PortRangeMin + } + if rule.PortRangeMax != nil { + r.PortRangeMax = *rule.PortRangeMax + } + if rule.Protocol != nil { + r.Protocol = *rule.Protocol + } + if rule.RemoteGroupID != nil { + r.RemoteGroupID = *rule.RemoteGroupID + } + if rule.RemoteIPPrefix != nil { + r.RemoteIPPrefix = *rule.RemoteIPPrefix + } + + if len(rule.RemoteManagedGroups) > 0 { + if rule.RemoteGroupID != nil { + return nil, fmt.Errorf("remoteGroupID must not be set when remoteManagedGroups is set") + } + + for _, rg := range rule.RemoteManagedGroups { + rc := r + rc.RemoteGroupID = remoteManagedGroups[rg.String()] + rules = append(rules, rc) + } + } else { + rules = append(rules, r) + } + } + return rules, nil +} + +// validateRemoteManagedGroups validates that the remoteManagedGroups target existing managed security groups. +func validateRemoteManagedGroups(remoteManagedGroups map[string]string, ruleRemoteManagedGroups []infrav1.ManagedSecurityGroupName) error { + if len(ruleRemoteManagedGroups) == 0 { + return fmt.Errorf("remoteManagedGroups is required") + } + + for _, group := range ruleRemoteManagedGroups { + if _, ok := remoteManagedGroups[group.String()]; !ok { + return fmt.Errorf("remoteManagedGroups: %s is not a valid remote managed security group", group) + } + } + return nil +} + func (s *Service) GetSecurityGroups(securityGroupParams []infrav1.SecurityGroupFilter) ([]string, error) { var sgIDs []string for _, sg := range securityGroupParams { @@ -245,8 +358,8 @@ func (s *Service) deleteSecurityGroup(openStackCluster *infrav1.OpenStackCluster // reconcileGroupRules reconciles an already existing observed group by deleting rules not needed anymore and // creating rules that are missing. -func (s *Service) reconcileGroupRules(desired, observed infrav1.SecurityGroup) (infrav1.SecurityGroup, error) { - rulesToDelete := []infrav1.SecurityGroupRule{} +func (s *Service) reconcileGroupRules(desired securityGroupSpec, observed infrav1.SecurityGroupStatus) (infrav1.SecurityGroupStatus, error) { + var rulesToDelete []string // fills rulesToDelete by calculating observed - desired for _, observedRule := range observed.Rules { deleteRule := true @@ -255,20 +368,20 @@ func (s *Service) reconcileGroupRules(desired, observed infrav1.SecurityGroup) ( if r.RemoteGroupID == remoteGroupIDSelf { r.RemoteGroupID = observed.ID } - if r.Equal(observedRule) { + if r.Matches(observedRule) { deleteRule = false break } } if deleteRule { - rulesToDelete = append(rulesToDelete, observedRule) + rulesToDelete = append(rulesToDelete, observedRule.ID) } } - rulesToCreate := []infrav1.SecurityGroupRule{} - reconciledRules := make([]infrav1.SecurityGroupRule, 0, len(desired.Rules)) + rulesToCreate := []resolvedSecurityGroupRuleSpec{} + reconciledRules := make([]infrav1.SecurityGroupRuleStatus, 0, len(desired.Rules)) // fills rulesToCreate by calculating desired - observed - // also adds rules which are in observed and desired to reconciledRules. + // also adds rules which are in observed and desired to reconcileGroupRules. for _, desiredRule := range desired.Rules { r := desiredRule if r.RemoteGroupID == remoteGroupIDSelf { @@ -276,7 +389,7 @@ func (s *Service) reconcileGroupRules(desired, observed infrav1.SecurityGroup) ( } createRule := true for _, observedRule := range observed.Rules { - if r.Equal(observedRule) { + if r.Matches(observedRule) { // add already existing rules to reconciledRules because we won't touch them anymore reconciledRules = append(reconciledRules, observedRule) createRule = false @@ -290,28 +403,31 @@ func (s *Service) reconcileGroupRules(desired, observed infrav1.SecurityGroup) ( s.scope.Logger().V(4).Info("Deleting rules not needed anymore for group", "name", observed.Name, "amount", len(rulesToDelete)) for _, rule := range rulesToDelete { - s.scope.Logger().V(6).Info("Deleting rule", "ID", rule.ID, "name", observed.Name) - err := s.client.DeleteSecGroupRule(rule.ID) + s.scope.Logger().V(6).Info("Deleting rule", "ID", rule, "name", observed.Name) + err := s.client.DeleteSecGroupRule(rule) if err != nil { - return infrav1.SecurityGroup{}, err + return infrav1.SecurityGroupStatus{}, err } } s.scope.Logger().V(4).Info("Creating new rules needed for group", "name", observed.Name, "amount", len(rulesToCreate)) for _, rule := range rulesToCreate { r := rule - r.SecurityGroupID = observed.ID if r.RemoteGroupID == remoteGroupIDSelf { r.RemoteGroupID = observed.ID } - newRule, err := s.createRule(r) + newRule, err := s.createRule(observed.ID, r) if err != nil { - return infrav1.SecurityGroup{}, err + return infrav1.SecurityGroupStatus{}, err } reconciledRules = append(reconciledRules, newRule) } observed.Rules = reconciledRules + if len(reconciledRules) == 0 { + return infrav1.SecurityGroupStatus{}, nil + } + return observed, nil } @@ -354,7 +470,7 @@ func (s *Service) createSecurityGroupIfNotExists(openStackCluster *infrav1.OpenS return nil } -func (s *Service) getSecurityGroupByName(name string) (*infrav1.SecurityGroup, error) { +func (s *Service) getSecurityGroupByName(name string) (*infrav1.SecurityGroupStatus, error) { opts := groups.ListOpts{ Name: name, } @@ -362,20 +478,20 @@ func (s *Service) getSecurityGroupByName(name string) (*infrav1.SecurityGroup, e s.scope.Logger().V(6).Info("Attempting to fetch security group with", "name", name) allGroups, err := s.client.ListSecGroup(opts) if err != nil { - return &infrav1.SecurityGroup{}, err + return &infrav1.SecurityGroupStatus{}, err } switch len(allGroups) { case 0: - return &infrav1.SecurityGroup{}, nil + return &infrav1.SecurityGroupStatus{}, nil case 1: return convertOSSecGroupToConfigSecGroup(allGroups[0]), nil } - return &infrav1.SecurityGroup{}, fmt.Errorf("more than one security group found named: %s", name) + return &infrav1.SecurityGroupStatus{}, fmt.Errorf("more than one security group found named: %s", name) } -func (s *Service) createRule(r infrav1.SecurityGroupRule) (infrav1.SecurityGroupRule, error) { +func (s *Service) createRule(securityGroupID string, r resolvedSecurityGroupRuleSpec) (infrav1.SecurityGroupRuleStatus, error) { dir := rules.RuleDirection(r.Direction) proto := rules.RuleProtocol(r.Protocol) etherType := rules.RuleEtherType(r.EtherType) @@ -389,12 +505,12 @@ func (s *Service) createRule(r infrav1.SecurityGroupRule) (infrav1.SecurityGroup EtherType: etherType, RemoteGroupID: r.RemoteGroupID, RemoteIPPrefix: r.RemoteIPPrefix, - SecGroupID: r.SecurityGroupID, + SecGroupID: securityGroupID, } - s.scope.Logger().V(6).Info("Creating rule", "description", r.Description, "direction", dir, "portRangeMin", r.PortRangeMin, "portRangeMax", r.PortRangeMax, "proto", proto, "etherType", etherType, "remoteGroupID", r.RemoteGroupID, "remoteIPPrefix", r.RemoteIPPrefix, "securityGroupID", r.SecurityGroupID) + s.scope.Logger().V(6).Info("Creating rule", "description", r.Description, "direction", dir, "portRangeMin", r.PortRangeMin, "portRangeMax", r.PortRangeMax, "proto", proto, "etherType", etherType, "remoteGroupID", r.RemoteGroupID, "remoteIPPrefix", r.RemoteIPPrefix, "securityGroupID", securityGroupID) rule, err := s.client.CreateSecGroupRule(createOpts) if err != nil { - return infrav1.SecurityGroupRule{}, err + return infrav1.SecurityGroupRuleStatus{}, err } return convertOSSecGroupRuleToConfigSecGroupRule(*rule), nil } @@ -411,30 +527,29 @@ func getSecBastionGroupName(clusterName string) string { return fmt.Sprintf("%s-cluster-%s-secgroup-%s", secGroupPrefix, clusterName, bastionSuffix) } -func convertOSSecGroupToConfigSecGroup(osSecGroup groups.SecGroup) *infrav1.SecurityGroup { - securityGroupRules := make([]infrav1.SecurityGroupRule, len(osSecGroup.Rules)) +func convertOSSecGroupToConfigSecGroup(osSecGroup groups.SecGroup) *infrav1.SecurityGroupStatus { + securityGroupRules := make([]infrav1.SecurityGroupRuleStatus, len(osSecGroup.Rules)) for i, rule := range osSecGroup.Rules { securityGroupRules[i] = convertOSSecGroupRuleToConfigSecGroupRule(rule) } - return &infrav1.SecurityGroup{ + return &infrav1.SecurityGroupStatus{ ID: osSecGroup.ID, Name: osSecGroup.Name, Rules: securityGroupRules, } } -func convertOSSecGroupRuleToConfigSecGroupRule(osSecGroupRule rules.SecGroupRule) infrav1.SecurityGroupRule { - return infrav1.SecurityGroupRule{ - ID: osSecGroupRule.ID, - Direction: osSecGroupRule.Direction, - Description: osSecGroupRule.Description, - EtherType: osSecGroupRule.EtherType, - SecurityGroupID: osSecGroupRule.SecGroupID, - PortRangeMin: osSecGroupRule.PortRangeMin, - PortRangeMax: osSecGroupRule.PortRangeMax, - Protocol: osSecGroupRule.Protocol, - RemoteGroupID: osSecGroupRule.RemoteGroupID, - RemoteIPPrefix: osSecGroupRule.RemoteIPPrefix, +func convertOSSecGroupRuleToConfigSecGroupRule(osSecGroupRule rules.SecGroupRule) infrav1.SecurityGroupRuleStatus { + return infrav1.SecurityGroupRuleStatus{ + ID: osSecGroupRule.ID, + Direction: osSecGroupRule.Direction, + Description: &osSecGroupRule.Description, + EtherType: &osSecGroupRule.EtherType, + PortRangeMin: &osSecGroupRule.PortRangeMin, + PortRangeMax: &osSecGroupRule.PortRangeMax, + Protocol: &osSecGroupRule.Protocol, + RemoteGroupID: &osSecGroupRule.RemoteGroupID, + RemoteIPPrefix: &osSecGroupRule.RemoteIPPrefix, } } diff --git a/pkg/cloud/services/networking/securitygroups_rules.go b/pkg/cloud/services/networking/securitygroups_rules.go index 5889867127..91effed16e 100644 --- a/pkg/cloud/services/networking/securitygroups_rules.go +++ b/pkg/cloud/services/networking/securitygroups_rules.go @@ -16,11 +16,7 @@ limitations under the License. package networking -import ( - infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8" -) - -var defaultRules = []infrav1.SecurityGroupRule{ +var defaultRules = []resolvedSecurityGroupRuleSpec{ { Direction: "egress", Description: "Full open", @@ -42,8 +38,8 @@ var defaultRules = []infrav1.SecurityGroupRule{ } // Permit traffic for etcd, kubelet. -func getSGControlPlaneCommon(remoteGroupIDSelf, secWorkerGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGControlPlaneCommon(remoteGroupIDSelf, secWorkerGroupID string) []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { Description: "Etcd", Direction: "ingress", @@ -76,47 +72,9 @@ func getSGControlPlaneCommon(remoteGroupIDSelf, secWorkerGroupID string) []infra } } -// Permit traffic for calico. -func getSGControlPlaneCalico(remoteGroupIDSelf, secWorkerGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ - { - Description: "BGP (calico)", - Direction: "ingress", - EtherType: "IPv4", - PortRangeMin: 179, - PortRangeMax: 179, - Protocol: "tcp", - RemoteGroupID: remoteGroupIDSelf, - }, - { - Description: "BGP (calico)", - Direction: "ingress", - EtherType: "IPv4", - PortRangeMin: 179, - PortRangeMax: 179, - Protocol: "tcp", - RemoteGroupID: secWorkerGroupID, - }, - { - Description: "IP-in-IP (calico)", - Direction: "ingress", - EtherType: "IPv4", - Protocol: "4", - RemoteGroupID: remoteGroupIDSelf, - }, - { - Description: "IP-in-IP (calico)", - Direction: "ingress", - EtherType: "IPv4", - Protocol: "4", - RemoteGroupID: secWorkerGroupID, - }, - } -} - // Permit traffic for kubelet. -func getSGWorkerCommon(remoteGroupIDSelf, secControlPlaneGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGWorkerCommon(remoteGroupIDSelf, secControlPlaneGroupID string) []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { // This is needed to support metrics-server deployments Description: "Kubelet API", @@ -139,47 +97,9 @@ func getSGWorkerCommon(remoteGroupIDSelf, secControlPlaneGroupID string) []infra } } -// Permit traffic for calico. -func getSGWorkerCalico(remoteGroupIDSelf, secControlPlaneGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ - { - Description: "BGP (calico)", - Direction: "ingress", - EtherType: "IPv4", - PortRangeMin: 179, - PortRangeMax: 179, - Protocol: "tcp", - RemoteGroupID: remoteGroupIDSelf, - }, - { - Description: "BGP (calico)", - Direction: "ingress", - EtherType: "IPv4", - PortRangeMin: 179, - PortRangeMax: 179, - Protocol: "tcp", - RemoteGroupID: secControlPlaneGroupID, - }, - { - Description: "IP-in-IP (calico)", - Direction: "ingress", - EtherType: "IPv4", - Protocol: "4", - RemoteGroupID: remoteGroupIDSelf, - }, - { - Description: "IP-in-IP (calico)", - Direction: "ingress", - EtherType: "IPv4", - Protocol: "4", - RemoteGroupID: secControlPlaneGroupID, - }, - } -} - // Permit traffic for ssh control plane. -func GetSGControlPlaneSSH(secBastionGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGControlPlaneSSH(secBastionGroupID string) []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { Description: "SSH", Direction: "ingress", @@ -193,8 +113,8 @@ func GetSGControlPlaneSSH(secBastionGroupID string) []infrav1.SecurityGroupRule } // Permit traffic for ssh worker. -func GetSGWorkerSSH(secBastionGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGWorkerSSH(secBastionGroupID string) []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { Description: "SSH", Direction: "ingress", @@ -208,8 +128,8 @@ func GetSGWorkerSSH(secBastionGroupID string) []infrav1.SecurityGroupRule { } // Allow all traffic, including from outside the cluster, to access the API. -func GetSGControlPlaneHTTPS() []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGControlPlaneHTTPS() []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { Description: "Kubernetes API", Direction: "ingress", @@ -222,8 +142,8 @@ func GetSGControlPlaneHTTPS() []infrav1.SecurityGroupRule { } // Allow all traffic, including from outside the cluster, to access node port services. -func GetSGWorkerNodePort() []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGWorkerNodePort() []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { Description: "Node Port Services", Direction: "ingress", @@ -244,8 +164,8 @@ func GetSGWorkerNodePort() []infrav1.SecurityGroupRule { } // Permit all ingress from the cluster security groups. -func GetSGControlPlaneAllowAll(remoteGroupIDSelf, secWorkerGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGControlPlaneAllowAll(remoteGroupIDSelf, secWorkerGroupID string) []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { Description: "In-cluster Ingress", Direction: "ingress", @@ -268,8 +188,8 @@ func GetSGControlPlaneAllowAll(remoteGroupIDSelf, secWorkerGroupID string) []inf } // Permit all ingress from the cluster security groups. -func GetSGWorkerAllowAll(remoteGroupIDSelf, secControlPlaneGroupID string) []infrav1.SecurityGroupRule { - return []infrav1.SecurityGroupRule{ +func getSGWorkerAllowAll(remoteGroupIDSelf, secControlPlaneGroupID string) []resolvedSecurityGroupRuleSpec { + return []resolvedSecurityGroupRuleSpec{ { Description: "In-cluster Ingress", Direction: "ingress", @@ -292,10 +212,10 @@ func GetSGWorkerAllowAll(remoteGroupIDSelf, secControlPlaneGroupID string) []inf } // Permit ports that defined in openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts. -func GetSGControlPlaneAdditionalPorts(ports []int) []infrav1.SecurityGroupRule { - controlPlaneRules := []infrav1.SecurityGroupRule{} +func getSGControlPlaneAdditionalPorts(ports []int) []resolvedSecurityGroupRuleSpec { + controlPlaneRules := []resolvedSecurityGroupRuleSpec{} - r := []infrav1.SecurityGroupRule{ + r := []resolvedSecurityGroupRuleSpec{ { Description: "Additional ports", Direction: "ingress", @@ -319,16 +239,14 @@ func GetSGControlPlaneAdditionalPorts(ports []int) []infrav1.SecurityGroupRule { return controlPlaneRules } -func GetSGControlPlaneGeneral(remoteGroupIDSelf, secWorkerGroupID string) []infrav1.SecurityGroupRule { - controlPlaneRules := []infrav1.SecurityGroupRule{} +func getSGControlPlaneGeneral(remoteGroupIDSelf, secWorkerGroupID string) []resolvedSecurityGroupRuleSpec { + controlPlaneRules := []resolvedSecurityGroupRuleSpec{} controlPlaneRules = append(controlPlaneRules, getSGControlPlaneCommon(remoteGroupIDSelf, secWorkerGroupID)...) - controlPlaneRules = append(controlPlaneRules, getSGControlPlaneCalico(remoteGroupIDSelf, secWorkerGroupID)...) return controlPlaneRules } -func GetSGWorkerGeneral(remoteGroupIDSelf, secControlPlaneGroupID string) []infrav1.SecurityGroupRule { - workerRules := []infrav1.SecurityGroupRule{} +func getSGWorkerGeneral(remoteGroupIDSelf, secControlPlaneGroupID string) []resolvedSecurityGroupRuleSpec { + workerRules := []resolvedSecurityGroupRuleSpec{} workerRules = append(workerRules, getSGWorkerCommon(remoteGroupIDSelf, secControlPlaneGroupID)...) - workerRules = append(workerRules, getSGWorkerCalico(remoteGroupIDSelf, secControlPlaneGroupID)...) return workerRules } diff --git a/pkg/cloud/services/networking/securitygroups_test.go b/pkg/cloud/services/networking/securitygroups_test.go new file mode 100644 index 0000000000..407f964559 --- /dev/null +++ b/pkg/cloud/services/networking/securitygroups_test.go @@ -0,0 +1,531 @@ +/* +Copyright 2024 The Kubernetes Authors. + +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 networking + +import ( + "reflect" + "testing" + + "github.com/go-logr/logr" + "github.com/golang/mock/gomock" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules" + . "github.com/onsi/gomega" + "k8s.io/utils/pointer" + + infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/clients/mock" + "sigs.k8s.io/cluster-api-provider-openstack/pkg/scope" +) + +func TestValidateRemoteManagedGroups(t *testing.T) { + tests := []struct { + name string + rule infrav1.SecurityGroupRuleSpec + remoteManagedGroups map[string]string + wantErr bool + }{ + { + name: "Invalid rule with unknown remoteManagedGroup", + rule: infrav1.SecurityGroupRuleSpec{ + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{"unknownGroup"}, + }, + wantErr: true, + }, + { + name: "Valid rule with missing remoteManagedGroups", + rule: infrav1.SecurityGroupRuleSpec{ + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + Protocol: pointer.String("tcp"), + }, + remoteManagedGroups: map[string]string{ + "self": "self", + "controlplane": "1", + "worker": "2", + "bastion": "3", + }, + wantErr: true, + }, + { + name: "Valid rule with remoteManagedGroups", + rule: infrav1.SecurityGroupRuleSpec{ + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{"controlplane", "worker", "bastion"}, + }, + remoteManagedGroups: map[string]string{ + "self": "self", + "controlplane": "1", + "worker": "2", + "bastion": "3", + }, + wantErr: false, + }, + { + name: "Invalid rule with bastion in remoteManagedGroups", + rule: infrav1.SecurityGroupRuleSpec{ + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{"controlplane", "worker", "bastion"}, + }, + remoteManagedGroups: map[string]string{ + "self": "self", + "controlplane": "1", + "worker": "2", + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateRemoteManagedGroups(tt.remoteManagedGroups, tt.rule.RemoteManagedGroups) + if (err != nil) != tt.wantErr { + t.Errorf("validateAllNodesRule() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestGetAllNodesRules(t *testing.T) { + tests := []struct { + name string + remoteManagedGroups map[string]string + allNodesSecurityGroupRules []infrav1.SecurityGroupRuleSpec + wantRules []resolvedSecurityGroupRuleSpec + wantErr bool + }{ + { + name: "Empty remoteManagedGroups and allNodesSecurityGroupRules", + remoteManagedGroups: map[string]string{}, + allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{}, + wantRules: []resolvedSecurityGroupRuleSpec{}, + wantErr: false, + }, + { + name: "Valid remoteManagedGroups and allNodesSecurityGroupRules", + remoteManagedGroups: map[string]string{ + "controlplane": "1", + "worker": "2", + }, + allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{ + { + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{ + "controlplane", + "worker", + }, + }, + }, + wantRules: []resolvedSecurityGroupRuleSpec{ + { + Protocol: "tcp", + PortRangeMin: 22, + PortRangeMax: 22, + RemoteGroupID: "1", + }, + { + Protocol: "tcp", + PortRangeMin: 22, + PortRangeMax: 22, + RemoteGroupID: "2", + }, + }, + wantErr: false, + }, + { + name: "Valid remoteManagedGroups in a rule", + remoteManagedGroups: map[string]string{ + "controlplane": "1", + "worker": "2", + }, + allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{ + { + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{"controlplane"}, + }, + }, + wantRules: []resolvedSecurityGroupRuleSpec{ + { + Protocol: "tcp", + PortRangeMin: 22, + PortRangeMax: 22, + RemoteGroupID: "1", + }, + }, + }, + { + name: "Invalid allNodesSecurityGroupRules with wrong remoteManagedGroups", + remoteManagedGroups: map[string]string{ + "controlplane": "1", + "worker": "2", + }, + allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{ + { + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{ + "controlplanezzz", + "worker", + }, + }, + }, + wantRules: nil, + wantErr: true, + }, + { + name: "Invalid allNodesSecurityGroupRules with bastion while remoteManagedGroups does not have bastion", + remoteManagedGroups: map[string]string{ + "controlplane": "1", + "worker": "2", + }, + allNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{ + { + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{ + "bastion", + }, + }, + }, + wantRules: nil, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotRules, err := getAllNodesRules(tt.remoteManagedGroups, tt.allNodesSecurityGroupRules) + if (err != nil) != tt.wantErr { + t.Errorf("getAllNodesRules() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(gotRules, tt.wantRules) { + t.Errorf("getAllNodesRules() gotRules = %v, want %v", gotRules, tt.wantRules) + } + }) + } +} + +func TestGenerateDesiredSecGroups(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + secGroupNames := map[string]string{ + "controlplane": "k8s-cluster-mycluster-secgroup-controlplane", + "worker": "k8s-cluster-mycluster-secgroup-worker", + } + + tests := []struct { + name string + openStackCluster *infrav1.OpenStackCluster + mockExpect func(m *mock.MockNetworkClientMockRecorder) + // We could also test the exact rules that are returned, but that'll be a lot data to write out. + // For now we just make sure that the number of rules is correct. + expectedNumberSecurityGroupRules int + wantErr bool + }{ + { + name: "Valid openStackCluster with unmanaged securityGroups", + openStackCluster: &infrav1.OpenStackCluster{}, + mockExpect: func(m *mock.MockNetworkClientMockRecorder) {}, + expectedNumberSecurityGroupRules: 0, + wantErr: false, + }, + { + name: "Valid openStackCluster with securityGroups", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{}, + }, + }, + mockExpect: func(m *mock.MockNetworkClientMockRecorder) { + m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-controlplane"}).Return([]groups.SecGroup{ + { + ID: "0", + Name: "k8s-cluster-mycluster-secgroup-controlplane", + }, + }, nil) + m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-worker"}).Return([]groups.SecGroup{ + { + ID: "1", + Name: "k8s-cluster-mycluster-secgroup-worker", + }, + }, nil) + }, + expectedNumberSecurityGroupRules: 12, + wantErr: false, + }, + { + name: "Valid openStackCluster with securityGroups and allNodesSecurityGroupRules", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{ + { + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{"controlplane", "worker"}, + }, + }, + }, + }, + }, + mockExpect: func(m *mock.MockNetworkClientMockRecorder) { + m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-controlplane"}).Return([]groups.SecGroup{ + { + ID: "0", + Name: "k8s-cluster-mycluster-secgroup-controlplane", + }, + }, nil) + m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-worker"}).Return([]groups.SecGroup{ + { + ID: "1", + Name: "k8s-cluster-mycluster-secgroup-worker", + }, + }, nil) + }, + expectedNumberSecurityGroupRules: 16, + wantErr: false, + }, + { + name: "Valid openStackCluster with securityGroups with invalid allNodesSecurityGroupRules", + openStackCluster: &infrav1.OpenStackCluster{ + Spec: infrav1.OpenStackClusterSpec{ + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllNodesSecurityGroupRules: []infrav1.SecurityGroupRuleSpec{ + { + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteManagedGroups: []infrav1.ManagedSecurityGroupName{"controlplane", "worker", "unknownGroup"}, + }, + }, + }, + }, + }, + mockExpect: func(m *mock.MockNetworkClientMockRecorder) { + m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-controlplane"}).Return([]groups.SecGroup{ + { + ID: "0", + Name: "k8s-cluster-mycluster-secgroup-controlplane", + }, + }, nil) + m.ListSecGroup(groups.ListOpts{Name: "k8s-cluster-mycluster-secgroup-worker"}).Return([]groups.SecGroup{ + { + ID: "1", + Name: "k8s-cluster-mycluster-secgroup-worker", + }, + }, nil) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "", logr.Discard()) + + s, err := NewService(mockScopeFactory) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + tt.mockExpect(mockScopeFactory.NetworkClient.EXPECT()) + + gotSecurityGroups, err := s.generateDesiredSecGroups(tt.openStackCluster, secGroupNames) + if tt.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + var gotNumberSecurityGroupRules int + for _, secGroup := range gotSecurityGroups { + gotNumberSecurityGroupRules += len(secGroup.Rules) + } + g.Expect(gotNumberSecurityGroupRules).To(Equal(tt.expectedNumberSecurityGroupRules)) + }) + } +} + +func TestReconcileGroupRules(t *testing.T) { + mockCtrl := gomock.NewController(t) + defer mockCtrl.Finish() + + tests := []struct { + name string + desiredSGSpecs securityGroupSpec + observedSGStatus infrav1.SecurityGroupStatus + mockExpect func(m *mock.MockNetworkClientMockRecorder) + wantSGStatus infrav1.SecurityGroupStatus + }{ + { + name: "Empty desiredSGSpecs and observedSGStatus", + desiredSGSpecs: securityGroupSpec{}, + observedSGStatus: infrav1.SecurityGroupStatus{}, + mockExpect: func(m *mock.MockNetworkClientMockRecorder) {}, + wantSGStatus: infrav1.SecurityGroupStatus{}, + }, + { + name: "Same desiredSGSpecs and observedSGStatus produces no changes", + desiredSGSpecs: securityGroupSpec{ + Name: "k8s-cluster-mycluster-secgroup-controlplane", + Rules: []resolvedSecurityGroupRuleSpec{ + { + Description: "Allow SSH", + Direction: "ingress", + EtherType: "IPv4", + Protocol: "tcp", + PortRangeMin: 22, + PortRangeMax: 22, + RemoteGroupID: "1", + RemoteIPPrefix: "", + }, + }, + }, + observedSGStatus: infrav1.SecurityGroupStatus{ + ID: "idSG", + Name: "k8s-cluster-mycluster-secgroup-controlplane", + Rules: []infrav1.SecurityGroupRuleStatus{ + { + Description: pointer.String("Allow SSH"), + Direction: "ingress", + EtherType: pointer.String("IPv4"), + ID: "idSGRule", + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteGroupID: pointer.String("1"), + RemoteIPPrefix: pointer.String(""), + }, + }, + }, + mockExpect: func(m *mock.MockNetworkClientMockRecorder) {}, + wantSGStatus: infrav1.SecurityGroupStatus{ + ID: "idSG", + Name: "k8s-cluster-mycluster-secgroup-controlplane", + Rules: []infrav1.SecurityGroupRuleStatus{ + { + Description: pointer.String("Allow SSH"), + Direction: "ingress", + EtherType: pointer.String("IPv4"), + ID: "idSGRule", + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteGroupID: pointer.String("1"), + RemoteIPPrefix: pointer.String(""), + }, + }, + }, + }, + { + name: "Different desiredSGSpecs and observedSGStatus produces changes", + desiredSGSpecs: securityGroupSpec{ + Name: "k8s-cluster-mycluster-secgroup-controlplane", + Rules: []resolvedSecurityGroupRuleSpec{ + { + Description: "Allow SSH", + Direction: "ingress", + EtherType: "IPv4", + Protocol: "tcp", + PortRangeMin: 22, + PortRangeMax: 22, + RemoteGroupID: "1", + RemoteIPPrefix: "", + }, + }, + }, + observedSGStatus: infrav1.SecurityGroupStatus{ + ID: "idSG", + Name: "k8s-cluster-mycluster-secgroup-controlplane", + Rules: []infrav1.SecurityGroupRuleStatus{ + { + Description: pointer.String("Allow SSH legacy"), + Direction: "ingress", + EtherType: pointer.String("IPv4"), + ID: "idSGRuleLegacy", + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(222), + PortRangeMax: pointer.Int(222), + RemoteGroupID: pointer.String("2"), + RemoteIPPrefix: pointer.String(""), + }, + }, + }, + mockExpect: func(m *mock.MockNetworkClientMockRecorder) { + m.DeleteSecGroupRule("idSGRuleLegacy").Return(nil) + m.CreateSecGroupRule(rules.CreateOpts{ + SecGroupID: "idSG", + Description: "Allow SSH", + Direction: "ingress", + EtherType: "IPv4", + Protocol: "tcp", + PortRangeMin: 22, + PortRangeMax: 22, + RemoteGroupID: "1", + }).Return(&rules.SecGroupRule{ + ID: "idSGRule", + Description: "Allow SSH", + Direction: "ingress", + EtherType: "IPv4", + Protocol: "tcp", + PortRangeMin: 22, + PortRangeMax: 22, + RemoteGroupID: "1", + }, nil) + }, + wantSGStatus: infrav1.SecurityGroupStatus{ + ID: "idSG", + Name: "k8s-cluster-mycluster-secgroup-controlplane", + Rules: []infrav1.SecurityGroupRuleStatus{ + { + Description: pointer.String("Allow SSH"), + Direction: "ingress", + EtherType: pointer.String("IPv4"), + ID: "idSGRule", + Protocol: pointer.String("tcp"), + PortRangeMin: pointer.Int(22), + PortRangeMax: pointer.Int(22), + RemoteGroupID: pointer.String("1"), + RemoteIPPrefix: pointer.String(""), + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "", logr.Discard()) + + s, err := NewService(mockScopeFactory) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + tt.mockExpect(mockScopeFactory.NetworkClient.EXPECT()) + + sgStatus, err := s.reconcileGroupRules(tt.desiredSGSpecs, tt.observedSGStatus) + g.Expect(err).To(BeNil()) + g.Expect(sgStatus).To(Equal(tt.wantSGStatus)) + }) + } +} diff --git a/templates/cluster-template-flatcar-sysext.yaml b/templates/cluster-template-flatcar-sysext.yaml index 8e2f89ea2a..ca02b3c639 100644 --- a/templates/cluster-template-flatcar-sysext.yaml +++ b/templates/cluster-template-flatcar-sysext.yaml @@ -230,7 +230,26 @@ spec: identityRef: kind: Secret name: ${CLUSTER_NAME}-cloud-config - managedSecurityGroups: true + managedSecurityGroups: + allNodesSecurityGroupRules: + - description: Created by cluster-api-provider-openstack - BGP (calico) + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMax: 179 + portRangeMin: 179 + protocol: tcp + remoteManagedGroups: + - controlplane + - worker + - description: Created by cluster-api-provider-openstack - IP-in-IP (calico) + direction: ingress + etherType: IPv4 + name: IP-in-IP (calico) + protocol: "4" + remoteManagedGroups: + - controlplane + - worker managedSubnets: - cidr: 10.6.0.0/24 dnsNameservers: diff --git a/templates/cluster-template-flatcar.yaml b/templates/cluster-template-flatcar.yaml index edccdf99ab..c2cfdc6934 100644 --- a/templates/cluster-template-flatcar.yaml +++ b/templates/cluster-template-flatcar.yaml @@ -154,7 +154,26 @@ spec: identityRef: kind: Secret name: ${CLUSTER_NAME}-cloud-config - managedSecurityGroups: true + managedSecurityGroups: + allNodesSecurityGroupRules: + - description: Created by cluster-api-provider-openstack - BGP (calico) + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMax: 179 + portRangeMin: 179 + protocol: tcp + remoteManagedGroups: + - controlplane + - worker + - description: Created by cluster-api-provider-openstack - IP-in-IP (calico) + direction: ingress + etherType: IPv4 + name: IP-in-IP (calico) + protocol: "4" + remoteManagedGroups: + - controlplane + - worker managedSubnets: - cidr: 10.6.0.0/24 dnsNameservers: diff --git a/templates/cluster-template-without-lb.yaml b/templates/cluster-template-without-lb.yaml index 97d39b1d17..2b206b65cd 100644 --- a/templates/cluster-template-without-lb.yaml +++ b/templates/cluster-template-without-lb.yaml @@ -111,7 +111,26 @@ spec: identityRef: kind: Secret name: ${CLUSTER_NAME}-cloud-config - managedSecurityGroups: true + managedSecurityGroups: + allNodesSecurityGroupRules: + - description: Created by cluster-api-provider-openstack - BGP (calico) + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMax: 179 + portRangeMin: 179 + protocol: tcp + remoteManagedGroups: + - controlplane + - worker + - description: Created by cluster-api-provider-openstack - IP-in-IP (calico) + direction: ingress + etherType: IPv4 + name: IP-in-IP (calico) + protocol: "4" + remoteManagedGroups: + - controlplane + - worker managedSubnets: - cidr: 10.6.0.0/24 dnsNameservers: diff --git a/templates/cluster-template.yaml b/templates/cluster-template.yaml index 1384b473b0..e2b1ff52db 100644 --- a/templates/cluster-template.yaml +++ b/templates/cluster-template.yaml @@ -113,7 +113,26 @@ spec: identityRef: kind: Secret name: ${CLUSTER_NAME}-cloud-config - managedSecurityGroups: true + managedSecurityGroups: + allNodesSecurityGroupRules: + - description: Created by cluster-api-provider-openstack - BGP (calico) + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMax: 179 + portRangeMin: 179 + protocol: tcp + remoteManagedGroups: + - controlplane + - worker + - description: Created by cluster-api-provider-openstack - IP-in-IP (calico) + direction: ingress + etherType: IPv4 + name: IP-in-IP (calico) + protocol: "4" + remoteManagedGroups: + - controlplane + - worker managedSubnets: - cidr: 10.6.0.0/24 dnsNameservers: diff --git a/templates/clusterclass-dev-test.yaml b/templates/clusterclass-dev-test.yaml index 44930abc7d..4112633624 100644 --- a/templates/clusterclass-dev-test.yaml +++ b/templates/clusterclass-dev-test.yaml @@ -121,7 +121,26 @@ spec: identityRef: kind: Secret name: dev-test-cloud-config - managedSecurityGroups: true + managedSecurityGroups: + allNodesSecurityGroupRules: + - description: Created by cluster-api-provider-openstack - BGP (calico) + direction: ingress + etherType: IPv4 + name: BGP (Calico) + portRangeMin: 179 + portRangeMax: 179 + protocol: tcp + remoteManagedGroups: + - controlplane + - worker + - description: Created by cluster-api-provider-openstack - IP-in-IP (calico) + direction: ingress + etherType: IPv4 + name: IP-in-IP (calico) + protocol: "4" + remoteManagedGroups: + - controlplane + - worker managedSubnets: - cidr: 10.6.0.0/24 dnsNameservers: diff --git a/test/e2e/shared/openstack.go b/test/e2e/shared/openstack.go index 4d46837775..85661092f3 100644 --- a/test/e2e/shared/openstack.go +++ b/test/e2e/shared/openstack.go @@ -40,6 +40,7 @@ import ( "github.com/gophercloud/gophercloud/openstack/imageservice/v2/images" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/layer3/routers" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/rules" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/trunks" "github.com/gophercloud/gophercloud/openstack/networking/v2/networks" "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" @@ -370,6 +371,58 @@ func DumpOpenStackSecurityGroups(e2eCtx *E2EContext, filter groups.ListOpts) ([] return groupsList, nil } +// DumpCalicoSecurityGroupRules returns the number of Calico security group rules. +func DumpCalicoSecurityGroupRules(e2eCtx *E2EContext, openStackCluster *infrav1.OpenStackCluster) (int, error) { + var count int + + providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx) + if err != nil { + return 0, fmt.Errorf("error creating provider client: %s", err) + } + + networkClient, err := openstack.NewNetworkV2(providerClient, gophercloud.EndpointOpts{ + Region: clientOpts.RegionName, + }) + if err != nil { + return 0, fmt.Errorf("error creating network client: %s", err) + } + + controlPlaneSGID := openStackCluster.Status.ControlPlaneSecurityGroup.ID + controlPlaneSGRules, err := rules.List(networkClient, rules.ListOpts{SecGroupID: controlPlaneSGID}).AllPages() + if err != nil { + return 0, fmt.Errorf("error listing control plane security group rules: %s", err) + } + controlPlaneRulesList, err := rules.ExtractRules(controlPlaneSGRules) + if err != nil { + return 0, fmt.Errorf("error extracting control plane security group rules: %s", err) + } + + workerSGID := openStackCluster.Status.WorkerSecurityGroup.ID + workerSGRules, err := rules.List(networkClient, rules.ListOpts{SecGroupID: workerSGID}).AllPages() + if err != nil { + return 0, fmt.Errorf("error listing worker security group rules: %s", err) + } + workerRulesList, err := rules.ExtractRules(workerSGRules) + if err != nil { + return 0, fmt.Errorf("error extracting worker security group rules: %s", err) + } + allRules := append(controlPlaneRulesList, workerRulesList...) + count += checkCalicoSecurityGroupRules(allRules) + + return count, nil +} + +// checkCalicoSecurityGroupRules returns the number of Calico security group rules. +func checkCalicoSecurityGroupRules(rules []rules.SecGroupRule) int { + var count int + for _, rule := range rules { + if strings.Contains(rule.Description, "calico") { + count++ + } + } + return count +} + // DumpOpenStackVolumes returns all OpenStack volumes to a file in the artifact folder. func DumpOpenStackVolumes(e2eCtx *E2EContext, filter volumes.ListOpts) ([]volumes.Volume, error) { providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx) diff --git a/test/e2e/suites/e2e/e2e_test.go b/test/e2e/suites/e2e/e2e_test.go index 2490d848ac..f4b92c9a42 100644 --- a/test/e2e/suites/e2e/e2e_test.go +++ b/test/e2e/suites/e2e/e2e_test.go @@ -120,6 +120,9 @@ var _ = Describe("e2e tests [PR-Blocking]", func() { waitForNodesReadyWithoutCCMTaint(ctx, workloadCluster.GetClient(), 2) + openStackCluster, err := shared.ClusterForSpec(ctx, e2eCtx, namespace) + Expect(err).NotTo(HaveOccurred()) + // Tag: clusterName is declared on OpenStackCluster and gets propagated to all machines // except the bastion host allServers, err := shared.DumpOpenStackServers(e2eCtx, servers.ListOpts{Tags: clusterName}) @@ -153,6 +156,12 @@ var _ = Describe("e2e tests [PR-Blocking]", func() { securityGroupsList, err := shared.DumpOpenStackSecurityGroups(e2eCtx, groups.ListOpts{Tags: clusterName}) Expect(err).NotTo(HaveOccurred()) Expect(securityGroupsList).To(HaveLen(3)) + + calicoSGRules, err := shared.DumpCalicoSecurityGroupRules(e2eCtx, openStackCluster) + Expect(err).NotTo(HaveOccurred()) + // We expect 4 security group rules that allow Calico traffic on the control plane + // from both the control plane and worker machines and vice versa, that makes 8 rules. + Expect(calicoSGRules).To(Equal(8)) }) })