Merge pull request #1785 from shiftstack/issues/1784

🐛 Don't apply worker SG to control plane machines
kubernetes-sigs · Dec 11, 2023 · a873934 · a873934
2 parents 70494b6 + 7e082f1
commit a873934
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 13 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
@@ -89,8 +89,9 @@ func getDefaultOpenStackMachine() *infrav1.OpenStackMachine {
 			ServerMetadata: map[string]string{
 				"test-metadata": "test-value",
 			},
-			ConfigDrive:   pointer.Bool(true),
-			ServerGroupID: serverGroupUUID,
+			ConfigDrive:    pointer.Bool(true),
+			SecurityGroups: []infrav1.SecurityGroupFilter{},
+			ServerGroupID:  serverGroupUUID,
 		},
 	}
 }
@@ -105,10 +106,11 @@ func getDefaultInstanceSpec() *compute.InstanceSpec {
 		Metadata: map[string]string{
 			"test-metadata": "test-value",
 		},
-		ConfigDrive:   *pointer.Bool(true),
-		FailureDomain: *pointer.String(failureDomain),
-		ServerGroupID: serverGroupUUID,
-		Tags:          []string{"test-tag"},
+		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 {