✨ WIP - Reconcile Machines & Managed Security Groups

api/v1alpha5/openstackmachine_types.go

@@ -110,6 +110,9 @@ type OpenStackMachineStatus struct {
 	// +optional
 	InstanceState *InstanceState `json:"instanceState,omitempty"`
 
+	// SecurityGroups is the list of security groups IDs associated with this machine.
+	SecurityGroups []string `json:"securityGroups,omitempty"`
+
 	FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`
 
 	// FailureMessage will be set in the event that there is a terminal problem

api/v1alpha6/openstackmachine_types.go

@@ -114,6 +114,9 @@ type OpenStackMachineStatus struct {
 
 	FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`
 
+	// SecurityGroups is the list of security groups IDs associated with this machine.
+	SecurityGroups []string `json:"securityGroups,omitempty"`
+
 	// FailureMessage will be set in the event that there is a terminal problem
 	// reconciling the Machine and will contain a more verbose string suitable
 	// for logging and human consumption.

api/v1alpha7/openstackmachine_types.go

@@ -109,6 +109,9 @@ type OpenStackMachineStatus struct {
 
 	FailureReason *errors.MachineStatusError `json:"failureReason,omitempty"`
 
+	// SecurityGroups is the list of security groups IDs associated with this machine.
+	SecurityGroups []string `json:"securityGroups,omitempty"`
+
 	// FailureMessage will be set in the event that there is a terminal problem
 	// reconciling the Machine and will contain a more verbose string suitable
 	// for logging and human consumption.

config/crd/bases/infrastructure.cluster.x-k8s.io_openstackmachines.yaml

@@ -546,6 +546,12 @@ spec:
               ready:
                 description: Ready is true when the provider resource is ready.
                 type: boolean
+              securityGroups:
+                description: SecurityGroups is the list of security groups IDs associated
+                  with this machine.
+                items:
+                  type: string
+                type: array
             type: object
         type: object
     served: true
@@ -1108,6 +1114,12 @@ spec:
               ready:
                 description: Ready is true when the provider resource is ready.
                 type: boolean
+              securityGroups:
+                description: SecurityGroups is the list of security groups IDs associated
+                  with this machine.
+                items:
+                  type: string
+                type: array
             type: object
         type: object
     served: true
@@ -1601,6 +1613,12 @@ spec:
               ready:
                 description: Ready is true when the provider resource is ready.
                 type: boolean
+              securityGroups:
+                description: SecurityGroups is the list of security groups IDs associated
+                  with this machine.
+                items:
+                  type: string
+                type: array
             type: object
         type: object
     served: true

controllers/openstackcluster_controller.go

@@ -124,7 +124,7 @@ func (r *OpenStackClusterReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 
 	// Handle non-deleted clusters
-	return reconcileNormal(scope, cluster, openStackCluster)
+	return r.reconcileNormal(ctx, scope, cluster, openStackCluster)
 }
 
 func (r *OpenStackClusterReconciler) reconcileDelete(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
@@ -258,7 +258,111 @@ func deleteBastion(scope scope.Scope, cluster *clusterv1.Cluster, openStackClust
 	return nil
 }
 
-func reconcileNormal(scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) { //nolint:unparam
+func getDesiredSecurityGroupsFromMachineLabels(machine *clusterv1.Machine, openStackCluster *infrav1.OpenStackCluster) []string {
+	desiredSecurityGroups := []string{}
+	if openStackCluster.Spec.ManagedSecurityGroups {
+		// For now we only support the control plane & workers
+		if util.IsControlPlaneMachine(machine) {
+			desiredSecurityGroups = append(desiredSecurityGroups, openStackCluster.Status.ControlPlaneSecurityGroup.ID)
+		} else {
+			desiredSecurityGroups = append(desiredSecurityGroups, openStackCluster.Status.WorkerSecurityGroup.ID)
+		}
+	}
+
+	return desiredSecurityGroups
+}
+
+func getAppliedSecurityGroups(openStackMachine *infrav1.OpenStackMachine) []string {
+	appliedSecurityGroups := []string{}
+	appliedSecurityGroups = append(appliedSecurityGroups, openStackMachine.Status.SecurityGroups...)
+
+	return appliedSecurityGroups
+}
+
+func getMachineFromOpenStackMachine(openStackMachine infrav1.OpenStackMachine, machineList *clusterv1.MachineList) *clusterv1.Machine {
+	for i := range machineList.Items {
+		machine := &machineList.Items[i]
+		if machine.Name == openStackMachine.Name {
+			return machine
+		}
+	}
+	return nil
+}
+
+func setMissingSecurityGroups(openStackMachine *infrav1.OpenStackMachine, securityGroups []string, computeService *compute.Service) error {
+	for _, securityGroup := range securityGroups {
+		err := computeService.AddSecurityGroupToInstance(*openStackMachine.Spec.InstanceID, securityGroup)
+		if err != nil {
+			return fmt.Errorf("failed to add security group %s to instance %s: %w", securityGroup, *openStackMachine.Spec.InstanceID, err)
+		}
+		statusSecurityGroups := openStackMachine.Status.SecurityGroups
+		if !contains(statusSecurityGroups, securityGroup) {
+			statusSecurityGroups = append(statusSecurityGroups, securityGroup)
+			openStackMachine.Status.SecurityGroups = statusSecurityGroups
+		}
+	}
+	return nil
+}
+
+func (r *OpenStackClusterReconciler) reconcileManagedSecurityGroups(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) error {
+	if openStackCluster.Spec.DisablePortSecurity {
+		scope.Logger().Info("Skipping managed security groups because port security is disabled")
+		return nil
+	}
+
+	networkingService, err := networking.NewService(scope)
+	if err != nil {
+		return err
+	}
+	computeService, err := compute.NewService(scope)
+	if err != nil {
+		return err
+	}
+	clusterName := fmt.Sprintf("%s-%s", cluster.Namespace, cluster.Name)
+	err = networkingService.ReconcileSecurityGroups(openStackCluster, clusterName)
+	if err != nil {
+		handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile security groups: %w", err))
+		return fmt.Errorf("failed to reconcile security groups: %w", err)
+	}
+
+	labels := map[string]string{
+		clusterv1.ClusterNameLabel: cluster.Name,
+	}
+	machineList := &clusterv1.MachineList{}
+	if err := r.Client.List(ctx, machineList, client.InNamespace(cluster.Namespace), client.MatchingLabels(labels)); err != nil {
+		return fmt.Errorf("failed to list machines: %w", err)
+	}
+
+	openStackMachines := &infrav1.OpenStackMachineList{}
+	err = r.Client.List(ctx, openStackMachines, client.InNamespace(cluster.Namespace), client.MatchingLabels(labels))
+	if err != nil {
+		return fmt.Errorf("failed to list openstack machines: %w", err)
+	}
+
+	for _, openStackMachine := range openStackMachines.Items {
+		machine := getMachineFromOpenStackMachine(openStackMachine, machineList)
+		if machine == nil {
+			return fmt.Errorf("failed to find machine for openstack machine %s", openStackMachine.Name)
+		}
+
+		desiredSecurityGroups := getDesiredSecurityGroupsFromMachineLabels(machine, openStackCluster)
+		observedSecurityGroups := getAppliedSecurityGroups(&openStackMachine)
+		missingSecurityGroups := []string{}
+		for _, desiredSecurityGroup := range desiredSecurityGroups {
+			if !contains(observedSecurityGroups, desiredSecurityGroup) {
+				missingSecurityGroups = append(missingSecurityGroups, desiredSecurityGroup)
+			}
+		}
+		err := setMissingSecurityGroups(&openStackMachine, missingSecurityGroups, computeService)
+		if err != nil {
+			return fmt.Errorf("failed to set missing security groups: %w", err)
+		}
+	}
+
+	return nil
+}
+
+func (r *OpenStackClusterReconciler) reconcileNormal(ctx context.Context, scope scope.Scope, cluster *clusterv1.Cluster, openStackCluster *infrav1.OpenStackCluster) (ctrl.Result, error) {
 	scope.Logger().Info("Reconciling Cluster")
 
 	// If the OpenStackCluster doesn't have our finalizer, add it.
@@ -277,6 +381,11 @@ func reconcileNormal(scope scope.Scope, cluster *clusterv1.Cluster, openStackClu
 		return reconcile.Result{}, err
 	}
 
+	err = r.reconcileManagedSecurityGroups(ctx, scope, cluster, openStackCluster)
+	if err != nil {
+		return reconcile.Result{}, err
+	}
+
 	if err = reconcileBastion(scope, cluster, openStackCluster); err != nil {
 		return reconcile.Result{}, err
 	}
@@ -499,12 +608,6 @@ func reconcileNetworkComponents(scope scope.Scope, cluster *clusterv1.Cluster, o
 		}
 	}
 
-	err = networkingService.ReconcileSecurityGroups(openStackCluster, clusterName)
-	if err != nil {
-		handleUpdateOSCError(openStackCluster, fmt.Errorf("failed to reconcile security groups: %w", err))
-		return fmt.Errorf("failed to reconcile security groups: %w", err)
-	}
-
 	// Calculate the port that we will use for the API server
 	var apiServerPort int
 	switch {

controllers/openstackmachine_controller.go

@@ -497,18 +497,6 @@ func machineToInstanceSpec(openStackCluster *infrav1.OpenStackCluster, machine *
 	instanceSpec.Tags = machineTags
 
 	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
-		}
-
-		instanceSpec.SecurityGroups = append(instanceSpec.SecurityGroups, infrav1.SecurityGroupFilter{
-			ID: managedSecurityGroup,
-		})
-	}
 
 	instanceSpec.Ports = openStackMachine.Spec.Ports
 

pkg/cloud/services/compute/instance.go

@@ -225,6 +225,44 @@ func (s *Service) CreateInstance(eventObject runtime.Object, openStackCluster *i
 	return s.createInstanceImpl(eventObject, openStackCluster, instanceSpec, clusterName, retryIntervalInstanceStatus)
 }
 
+func contains(arr []string, target string) bool {
+	for _, a := range arr {
+		if a == target {
+			return true
+		}
+	}
+	return false
+}
+
+func (s *Service) AddSecurityGroupToInstance(instanceID string, securityGroup string) error {
+	attachedPorts, err := s.getComputeClient().ListAttachedInterfaces(instanceID)
+	if err != nil {
+		return fmt.Errorf("error getting ports for server %s: %v", instanceID, err)
+	}
+
+	if err != nil {
+		return err
+	}
+	for _, port := range attachedPorts {
+		portSpecs, err := s._networkingService.GetPort(port.PortID)
+		if err != nil {
+			return err
+		}
+		portSecurityGroups := portSpecs.SecurityGroups
+		if !contains(portSecurityGroups, securityGroup) {
+			portSecurityGroups = append(portSecurityGroups, securityGroup)
+		}
+		portUpdateOpts := ports.UpdateOpts{
+			SecurityGroups: &portSecurityGroups,
+		}
+		if _, err := s._networkingService.UpdatePort(port.PortID, portUpdateOpts); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func (s *Service) getAndValidateFlavor(flavorName string) (*flavors.Flavor, error) {
 	f, err := s.getComputeClient().GetFlavorFromName(flavorName)
 	if err != nil {

pkg/cloud/services/networking/port.go

@@ -53,6 +53,13 @@ func (s *Service) GetPortFromInstanceIP(instanceID string, ip string) ([]ports.P
 	}
 	return s.client.ListPort(portOpts)
 }
+func (s *Service) GetPort(id string) (*ports.Port, error) {
+	return s.client.GetPort(id)
+}
+
+func (s *Service) UpdatePort(id string, opts ports.UpdateOpts) (*ports.Port, error) {
+	return s.client.UpdatePort(id, opts)
+}
 
 func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string, portName string, portOpts *infrav1.PortOpts, instanceSecurityGroups []string, instanceTags []string) (*ports.Port, error) {
 	networkID := portOpts.Network.ID
@@ -191,6 +198,19 @@ func (s *Service) GetOrCreatePort(eventObject runtime.Object, clusterName string
 	return port, nil
 }
 
+func (s *Service) UpdatePortSecurityGroups(eventObject runtime.Object, portID string, securityGroupIDs []string) error {
+	_, err := s.client.UpdatePort(portID, ports.UpdateOpts{
+		SecurityGroups: &securityGroupIDs,
+	})
+	if err != nil {
+		record.Warnf(eventObject, "FailedUpdatePortSecurityGroups", "Failed to update port %s: %v", portID, err)
+		return err
+	}
+
+	record.Eventf(eventObject, "SuccessfulUpdatePortSecurityGroups", "Updated port %s", portID)
+	return nil
+}
+
 func (s *Service) getSubnetIDForFixedIP(subnet *infrav1.SubnetFilter, networkID string) (string, error) {
 	if subnet == nil {
 		return "", nil