Move spec.allowAllInClusterTraffic to spec.managedSecurityGroups

api/v1alpha5/conversion.go

@@ -212,6 +212,7 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha5_OpenStackClusterSpec(in *
 
 	if in.ManagedSecurityGroups != nil {
 		out.ManagedSecurityGroups = true
+		out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic
 	}
 
 	return nil
@@ -252,6 +253,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
 		}
 	}
 

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,
+				},
 			},
 		},
 	}

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,7 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha6_OpenStackClusterSpec(in *
 
 	if in.ManagedSecurityGroups != nil {
 		out.ManagedSecurityGroups = true
+		out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic
 	}
 
 	return nil
@@ -632,6 +639,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
 		}
 	}
 

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,
+				},
 			},
 		},
 	}

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,7 @@ func Convert_v1alpha8_OpenStackClusterSpec_To_v1alpha7_OpenStackClusterSpec(in *
 
 	if in.ManagedSecurityGroups != nil {
 		out.ManagedSecurityGroups = true
+		out.AllowAllInClusterTraffic = in.ManagedSecurityGroups.AllowAllInClusterTraffic
 	}
 
 	return nil

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,
+				},
 			},
 		},
 	}

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,11 @@ 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.
+	// +kubebuilder:default=false
+	// +kubebuilder:validation:Required
+	AllowAllInClusterTraffic bool `json:"allowAllInClusterTraffic"`
 }
 
 func init() {

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 = ""

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,13 @@ spec:
                     x-kubernetes-list-map-keys:
                     - name
                     x-kubernetes-list-type: map
+                  allowAllInClusterTraffic:
+                    default: false
+                    description: AllowAllInClusterTraffic allows all ingress and egress
+                      traffic between cluster nodes when set to true.
+                    type: boolean
+                required:
+                - allowAllInClusterTraffic
                 type: object
               managedSubnets:
                 description: |-

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,14 @@ spec:
                             x-kubernetes-list-map-keys:
                             - name
                             x-kubernetes-list-type: map
+                          allowAllInClusterTraffic:
+                            default: false
+                            description: AllowAllInClusterTraffic allows all ingress
+                              and egress traffic between cluster nodes when set to
+                              true.
+                            type: boolean
+                        required:
+                        - allowAllInClusterTraffic
                         type: object
                       managedSubnets:
                         description: |-

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: <cluster-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: <cluster-name>
   namespace: <cluster-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).

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

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)...)