Don't apply worker SG to control plane machines

Currently, if a worker machine security group is specified but a control plane machine security group is not, the worker machine SG will be be applied to both worker *and* control plane machines. Correct this mistake. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
kubernetes-sigs · Dec 8, 2023 · bc7ec1c · bc7ec1c
1 parent f323b4f
commit bc7ec1c
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 7 deletions.
diff --git a/controllers/openstackmachine_controller.go b/controllers/openstackmachine_controller.go
@@ -499,15 +499,21 @@ func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *
 	instanceSpec.SecurityGroups = openStackMachine.Spec.SecurityGroups
 	if openStackCluster.Spec.ManagedSecurityGroups {
 		var managedSecurityGroup string
-		if util.IsControlPlaneMachine(machine) && openStackCluster.Status.ControlPlaneSecurityGroup != nil {
-			managedSecurityGroup = openStackCluster.Status.ControlPlaneSecurityGroup.ID
-		} else if openStackCluster.Status.WorkerSecurityGroup != nil {
-			managedSecurityGroup = openStackCluster.Status.WorkerSecurityGroup.ID
+		if util.IsControlPlaneMachine(machine) {
+			if openStackCluster.Status.ControlPlaneSecurityGroup != nil {
+				managedSecurityGroup = openStackCluster.Status.ControlPlaneSecurityGroup.ID
+			}
+		} else {
+			if openStackCluster.Status.WorkerSecurityGroup != nil {
+				managedSecurityGroup = openStackCluster.Status.WorkerSecurityGroup.ID
+			}
 		}
 
-		instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupFilter{
-			ID: managedSecurityGroup,
-		})
+		if managedSecurityGroup != "" {
+			instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupFilter{
+				ID: managedSecurityGroup,
+			})
+		}
 	}
 
 	instanceSpec.Ports = openStackMachine.Spec.Ports

diff --git a/controllers/openstackmachine_controller_test.go b/controllers/openstackmachine_controller_test.go
@@ -90,6 +90,7 @@ func getDefaultOpenStackMachine() *infrav1.OpenStackMachine {
 				"test-metadata": "test-value",
 			},
 			ConfigDrive:   pointer.Bool(true),
+			SecurityGroups: []infrav1.SecurityGroupFilter{},
 			ServerGroupID: serverGroupUUID,
 		},
 	}
@@ -108,6 +109,7 @@ func getDefaultInstanceSpec() *compute.InstanceSpec {
 		ConfigDrive:   *pointer.Bool(true),
 		FailureDomain: *pointer.String(failureDomain),
 		ServerGroupID: serverGroupUUID,
+		SecurityGroups: []infrav1.SecurityGroupFilter{},
 		Tags:          []string{"test-tag"},
 	}
 }
@@ -165,6 +167,44 @@ func Test_machineToInstanceSpec(t *testing.T) {
 				return i
 			},
 		},
+		{
+			name: "Control plane security group not applied to worker",
+			openStackCluster: func() *infrav1.OpenStackCluster {
+				c := getDefaultOpenStackCluster()
+				c.Spec.ManagedSecurityGroups = true
+				c.Status.WorkerSecurityGroup = nil
+				return c
+			},
+			machine:          getDefaultMachine,
+			openStackMachine: getDefaultOpenStackMachine,
+			wantInstanceSpec: func() *compute.InstanceSpec {
+				i := getDefaultInstanceSpec()
+				i.SecurityGroups = []infrav1.SecurityGroupFilter{}
+				return i
+			},
+		},
+		{
+			name: "Worker security group not applied to control plane",
+			openStackCluster: func() *infrav1.OpenStackCluster {
+				c := getDefaultOpenStackCluster()
+				c.Spec.ManagedSecurityGroups = true
+				c.Status.ControlPlaneSecurityGroup = nil
+				return c
+			},
+			machine: func() *clusterv1.Machine {
+				m := getDefaultMachine()
+				m.Labels = map[string]string{
+					clusterv1.MachineControlPlaneLabel: "true",
+				}
+				return m
+			},
+			openStackMachine: getDefaultOpenStackMachine,
+			wantInstanceSpec: func() *compute.InstanceSpec {
+				i := getDefaultInstanceSpec()
+				i.SecurityGroups = []infrav1.SecurityGroupFilter{}
+				return i
+			},
+		},
 		{
 			name: "Extra security group",
 			openStackCluster: func() *infrav1.OpenStackCluster {