diff --git a/api/v1alpha5/conversion.go b/api/v1alpha5/conversion.go index 4dfc78a140..29624bbf0b 100644 --- a/api/v1alpha5/conversion.go +++ b/api/v1alpha5/conversion.go @@ -212,6 +212,10 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in * if in.ManagedSecurityGroups != nil { out.ManagedSecurityGroups = true + + if in.ManagedSecurityGroups.AllowAllInClusterTraffic { + out.AllowAllInClusterTraffic = true + } } return nil @@ -252,6 +256,8 @@ func Convert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in * out.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} if !in.AllowAllInClusterTraffic { out.ManagedSecurityGroups.AllNodesSecurityGroupRules = infrav1.LegacyCalicoSecurityGroupRules() + } else { + out.ManagedSecurityGroups.AllowAllInClusterTraffic = true } } diff --git a/api/v1alpha5/conversion_test.go b/api/v1alpha5/conversion_test.go index b51adaff7c..480471b3c6 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\":null,\"network\":{}},\"status\":{\"ready\":false}}", + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"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\":null,\"network\":{}}}}}", + "cluster.x-k8s.io/conversion-data": "{\"spec\":{\"template\":{\"spec\":{\"apiServerLoadBalancer\":{},\"cloudName\":\"\",\"controlPlaneEndpoint\":{\"host\":\"\",\"port\":0},\"disableAPIServerFloatingIP\":false,\"disableExternalNetwork\":false,\"externalNetwork\":{},\"managedSecurityGroups\":null,\"network\":{}}}}}", }, }, }, @@ -140,8 +140,9 @@ func TestConvert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( AllowAllInClusterTraffic: true, }, expectedOut: &infrav1.OpenStackClusterSpec{ - ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{}, - AllowAllInClusterTraffic: true, + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllowAllInClusterTraffic: true, + }, }, }, } diff --git a/api/v1alpha5/zz_generated.conversion.go b/api/v1alpha5/zz_generated.conversion.go index 0d4ef4e7b5..8ec8034142 100644 --- a/api/v1alpha5/zz_generated.conversion.go +++ b/api/v1alpha5/zz_generated.conversion.go @@ -671,7 +671,7 @@ func autoConvert_v1alpha5_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups) - out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic + // WARNING: in.AllowAllInClusterTraffic requires manual conversion: does not exist in peer-type out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.ControlPlaneEndpoint = in.ControlPlaneEndpoint @@ -719,7 +719,6 @@ func autoConvert_v1alpha8_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec( out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort // 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)) out.ControlPlaneEndpoint = in.ControlPlaneEndpoint diff --git a/api/v1alpha6/conversion.go b/api/v1alpha6/conversion.go index 417d161237..b8b49e9b62 100644 --- a/api/v1alpha6/conversion.go +++ b/api/v1alpha6/conversion.go @@ -156,6 +156,12 @@ func restorev1alpha6ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackCl if prevBastion != nil && dstBastion != nil { restorev1alpha6MachineSpec(&prevBastion.Instance, &dstBastion.Instance) } + + // To avoid lossy conversion, we need to restore AllowAllInClusterTraffic + // even if ManagedSecurityGroups is set to false + if previous.AllowAllInClusterTraffic && !previous.ManagedSecurityGroups { + dst.AllowAllInClusterTraffic = true + } } var _ ctrlconversion.Convertible = &OpenStackCluster{} @@ -592,6 +598,10 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in * if in.ManagedSecurityGroups != nil { out.ManagedSecurityGroups = true + + if in.ManagedSecurityGroups.AllowAllInClusterTraffic { + out.AllowAllInClusterTraffic = true + } } return nil @@ -632,6 +642,8 @@ func Convert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in * out.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} if !in.AllowAllInClusterTraffic { out.ManagedSecurityGroups.AllNodesSecurityGroupRules = infrav1.LegacyCalicoSecurityGroupRules() + } else { + out.ManagedSecurityGroups.AllowAllInClusterTraffic = true } } diff --git a/api/v1alpha6/conversion_test.go b/api/v1alpha6/conversion_test.go index 2e684b8e35..77f5c4de76 100644 --- a/api/v1alpha6/conversion_test.go +++ b/api/v1alpha6/conversion_test.go @@ -685,8 +685,9 @@ func TestConvert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( AllowAllInClusterTraffic: true, }, expectedOut: &infrav1.OpenStackClusterSpec{ - ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{}, - AllowAllInClusterTraffic: true, + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllowAllInClusterTraffic: true, + }, }, }, } diff --git a/api/v1alpha6/zz_generated.conversion.go b/api/v1alpha6/zz_generated.conversion.go index 7df2ce7482..2bbcd48d10 100644 --- a/api/v1alpha6/zz_generated.conversion.go +++ b/api/v1alpha6/zz_generated.conversion.go @@ -693,7 +693,7 @@ func autoConvert_v1alpha6_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups) - out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic + // WARNING: in.AllowAllInClusterTraffic requires manual conversion: does not exist in peer-type out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.ControlPlaneEndpoint = in.ControlPlaneEndpoint @@ -742,7 +742,6 @@ func autoConvert_v1alpha8_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec( out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort // 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)) out.ControlPlaneEndpoint = in.ControlPlaneEndpoint diff --git a/api/v1alpha7/conversion.go b/api/v1alpha7/conversion.go index 1e84d4f0b9..2f0cba5d92 100644 --- a/api/v1alpha7/conversion.go +++ b/api/v1alpha7/conversion.go @@ -204,6 +204,12 @@ func restorev1alpha7ClusterSpec(previous *OpenStackClusterSpec, dst *OpenStackCl if len(previous.DNSNameservers) > 0 && dst.NodeCIDR == "" { dst.DNSNameservers = previous.DNSNameservers } + + // To avoid lossy conversion, we need to restore AllowAllInClusterTraffic + // even if ManagedSecurityGroups is set to false + if previous.AllowAllInClusterTraffic && !previous.ManagedSecurityGroups { + dst.AllowAllInClusterTraffic = true + } } func restorev1alpha8ClusterSpec(previous *infrav1.OpenStackClusterSpec, dst *infrav1.OpenStackClusterSpec) { @@ -581,6 +587,8 @@ func Convert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec(in * out.ManagedSecurityGroups = &infrav1.ManagedSecurityGroups{} if !in.AllowAllInClusterTraffic { out.ManagedSecurityGroups.AllNodesSecurityGroupRules = infrav1.LegacyCalicoSecurityGroupRules() + } else { + out.ManagedSecurityGroups.AllowAllInClusterTraffic = true } } @@ -610,6 +618,10 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in * if in.ManagedSecurityGroups != nil { out.ManagedSecurityGroups = true + + if in.ManagedSecurityGroups.AllowAllInClusterTraffic { + out.AllowAllInClusterTraffic = true + } } return nil diff --git a/api/v1alpha7/conversion_test.go b/api/v1alpha7/conversion_test.go index 90b391f037..f3c2338e70 100644 --- a/api/v1alpha7/conversion_test.go +++ b/api/v1alpha7/conversion_test.go @@ -300,8 +300,9 @@ func TestConvert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( AllowAllInClusterTraffic: true, }, expectedOut: &infrav1.OpenStackClusterSpec{ - ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{}, - AllowAllInClusterTraffic: true, + ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{ + AllowAllInClusterTraffic: true, + }, }, }, } diff --git a/api/v1alpha7/zz_generated.conversion.go b/api/v1alpha7/zz_generated.conversion.go index 5ff683e8fb..c29aaf334f 100644 --- a/api/v1alpha7/zz_generated.conversion.go +++ b/api/v1alpha7/zz_generated.conversion.go @@ -885,7 +885,7 @@ func autoConvert_v1alpha7_OpenStackClusterSpec_To_v1alpha8_OpenStackClusterSpec( out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort // WARNING: in.ManagedSecurityGroups requires manual conversion: inconvertible types (bool vs *sigs.k8s.io/cluster-api-provider-openstack/api/v1alpha8.ManagedSecurityGroups) - out.AllowAllInClusterTraffic = in.AllowAllInClusterTraffic + // WARNING: in.AllowAllInClusterTraffic requires manual conversion: does not exist in peer-type out.DisablePortSecurity = in.DisablePortSecurity out.Tags = *(*[]string)(unsafe.Pointer(&in.Tags)) out.ControlPlaneEndpoint = in.ControlPlaneEndpoint @@ -924,7 +924,6 @@ func autoConvert_v1alpha8_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec( out.APIServerFixedIP = in.APIServerFixedIP out.APIServerPort = in.APIServerPort // 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)) out.ControlPlaneEndpoint = in.ControlPlaneEndpoint diff --git a/api/v1alpha8/openstackcluster_types.go b/api/v1alpha8/openstackcluster_types.go index cabb2a55ca..2b3d50e57e 100644 --- a/api/v1alpha8/openstackcluster_types.go +++ b/api/v1alpha8/openstackcluster_types.go @@ -129,13 +129,6 @@ type OpenStackClusterSpec struct { // +optional 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 - // ingress and egress between cluster nodes is permitted, allowing CNIs other than - // Calico to be used. - // +optional - AllowAllInClusterTraffic bool `json:"allowAllInClusterTraffic"` - // DisablePortSecurity disables the port security of the network created for the // Kubernetes cluster, which also disables SecurityGroups DisablePortSecurity bool `json:"disablePortSecurity,omitempty"` @@ -280,6 +273,10 @@ type ManagedSecurityGroups struct { // +listMapKey=name // +optional AllNodesSecurityGroupRules []SecurityGroupRuleSpec `json:"allNodesSecurityGroupRules" patchStrategy:"merge" patchMergeKey:"name"` + + // AllowAllInClusterTraffic allows all ingress and egress traffic between cluster nodes when set to true. + // +optional + AllowAllInClusterTraffic bool `json:"allowAllInClusterTraffic"` } func init() { diff --git a/api/v1alpha8/openstackcluster_webhook.go b/api/v1alpha8/openstackcluster_webhook.go index 086a0e3c25..3df15413d9 100644 --- a/api/v1alpha8/openstackcluster_webhook.go +++ b/api/v1alpha8/openstackcluster_webhook.go @@ -145,6 +145,10 @@ func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warn if r.Spec.ManagedSecurityGroups != nil { old.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules = []SecurityGroupRuleSpec{} r.Spec.ManagedSecurityGroups.AllNodesSecurityGroupRules = []SecurityGroupRuleSpec{} + + // Allow change to the allowAllInClusterTraffic. + old.Spec.ManagedSecurityGroups.AllowAllInClusterTraffic = false + r.Spec.ManagedSecurityGroups.AllowAllInClusterTraffic = false } // Allow changes on AllowedCIDRs @@ -162,10 +166,6 @@ func (r *OpenStackCluster) ValidateUpdate(oldRaw runtime.Object) (admission.Warn old.Spec.ControlPlaneOmitAvailabilityZone = false r.Spec.ControlPlaneOmitAvailabilityZone = false - // Allow change to the allowAllInClusterTraffic. - old.Spec.AllowAllInClusterTraffic = false - r.Spec.AllowAllInClusterTraffic = false - // Allow change on the spec.APIServerFloatingIP only if it matches the current api server loadbalancer IP. if old.Status.APIServerLoadBalancer != nil && r.Spec.APIServerFloatingIP == old.Status.APIServerLoadBalancer.IP { r.Spec.APIServerFloatingIP = "" 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 a9d2d44d11..9de3df2684 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclusters.yaml @@ -4838,13 +4838,6 @@ spec: spec: description: OpenStackClusterSpec defines the desired state of OpenStackCluster. properties: - allowAllInClusterTraffic: - description: |- - 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 - ingress and egress between cluster nodes is permitted, allowing CNIs other than - Calico to be used. - type: boolean apiServerFixedIP: description: |- APIServerFixedIP is the fixed IP which will be associated with the API server. @@ -5536,6 +5529,10 @@ spec: x-kubernetes-list-map-keys: - name x-kubernetes-list-type: map + allowAllInClusterTraffic: + description: AllowAllInClusterTraffic allows all ingress and egress + traffic between cluster nodes when set to true. + type: boolean type: object managedSubnets: description: |- 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 10a79fa903..cbf5fae31a 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_openstackclustertemplates.yaml @@ -2263,13 +2263,6 @@ spec: description: OpenStackClusterSpec defines the desired state of OpenStackCluster. properties: - allowAllInClusterTraffic: - description: |- - 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 - ingress and egress between cluster nodes is permitted, allowing CNIs other than - Calico to be used. - type: boolean apiServerFixedIP: description: |- APIServerFixedIP is the fixed IP which will be associated with the API server. @@ -2971,6 +2964,11 @@ spec: x-kubernetes-list-map-keys: - name x-kubernetes-list-type: map + allowAllInClusterTraffic: + description: AllowAllInClusterTraffic allows all ingress + and egress traffic between cluster nodes when set to + true. + type: boolean type: object managedSubnets: description: |- diff --git a/docs/book/src/clusteropenstack/configuration.md b/docs/book/src/clusteropenstack/configuration.md index 18f1269f66..0a87afcee1 100644 --- a/docs/book/src/clusteropenstack/configuration.md +++ b/docs/book/src/clusteropenstack/configuration.md @@ -193,7 +193,6 @@ Depending on the CNI that will be deployed on the cluster, you may need to add s namespace: spec: ... - allowAllInClusterTraffic: false managedSecurityGroups: allNodesSecurityGroupRules: - remoteManagedGroups: @@ -214,6 +213,7 @@ Depending on the CNI that will be deployed on the cluster, you may need to add s name: IP-in-IP (Calico) protocol: 4 description: "Allow IP-in-IP between control plane and workers" + allowAllInClusterTraffic: false ``` # Optional Configuration @@ -312,7 +312,6 @@ metadata: name: namespace: spec: - allowAllInClusterTraffic: true apiServerLoadBalancer: allowedCidrs: - 192.168.10/24 @@ -558,7 +557,7 @@ managedSecurityGroups: {} - Node port traffic from anywhere - Kubelet traffic from other cluster nodes -When the flag `OpenStackCluster.spec.allowAllInClusterTraffic` is +When the flag `OpenStackCluster.spec.managedSecurityGroups.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). 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 be1b3b63c8..44124f8b8f 100644 --- a/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md +++ b/docs/book/src/topics/crd-changes/v1alpha7-to-v1alpha8.md @@ -209,6 +209,8 @@ It takes a list of security groups rules that should be applied to selected node The following rule fields are mutually exclusive: `remoteManagedGroups`, `remoteGroupID` and `remoteIPPrefix`. Valid values for `remoteManagedGroups` are `controlplane`, `worker` and `bastion`. +Also, `OpenStackCluster.Spec.AllowAllInClusterTraffic` moved under `ManagedSecurityGroups`. + ```yaml managedSecurityGroups: true ``` @@ -219,6 +221,20 @@ becomes managedSecurityGroups: {} ``` +and + +```yaml +allowAllInClusterTraffic: true +managedSecurityGroups: true +``` + +becomes + +```yaml +managedSecurityGroups: + allowAllInClusterTraffic: true +``` + To apply a security group rule that will allow BGP between the control plane and workers, you can follow this example: ```yaml diff --git a/pkg/cloud/services/networking/securitygroups.go b/pkg/cloud/services/networking/securitygroups.go index 51f6083a56..e977a229b9 100644 --- a/pkg/cloud/services/networking/securitygroups.go +++ b/pkg/cloud/services/networking/securitygroups.go @@ -168,7 +168,7 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl controlPlaneRules = append(controlPlaneRules, getSGControlPlaneAdditionalPorts(openStackCluster.Spec.APIServerLoadBalancer.AdditionalPorts)...) } - if openStackCluster.Spec.AllowAllInClusterTraffic { + if openStackCluster.Spec.ManagedSecurityGroups != nil && openStackCluster.Spec.ManagedSecurityGroups.AllowAllInClusterTraffic { // Permit all ingress from the cluster security groups controlPlaneRules = append(controlPlaneRules, getSGControlPlaneAllowAll(remoteGroupIDSelf, secWorkerGroupID)...) workerRules = append(workerRules, getSGWorkerAllowAll(remoteGroupIDSelf, secControlPlaneGroupID)...)