diff --git a/pkg/openstack/loadbalancer.go b/pkg/openstack/loadbalancer.go index fab2ad5008..6c829e33b7 100644 --- a/pkg/openstack/loadbalancer.go +++ b/pkg/openstack/loadbalancer.go @@ -728,25 +728,52 @@ func getSubnetIDForLB(network *gophercloud.ServiceClient, node corev1.Node, pref return "", cpoerrors.ErrNotFound } -// applyNodeSecurityGroupIDForLB associates the security group with all the ports on the nodes. -func applyNodeSecurityGroupIDForLB(network *gophercloud.ServiceClient, nodes []*corev1.Node, sg string) error { +// isPortMember returns true if IP and subnetID are one of the FixedIPs on the port +func isPortMember(port PortWithPortSecurity, IP string, subnetID string) bool { + for _, fixedIP := range port.FixedIPs { + if (subnetID == "" || subnetID == fixedIP.SubnetID) && IP == fixedIP.IPAddress { + return true + } + } + return false +} + +// applyNodeSecurityGroupIDForLB associates the security group with the ports being members of the LB on the nodes. +func applyNodeSecurityGroupIDForLB(network *gophercloud.ServiceClient, svcConf *serviceConfig, nodes []*corev1.Node, sg string) error { for _, node := range nodes { serverID, _, err := instanceIDFromProviderID(node.Spec.ProviderID) if err != nil { return fmt.Errorf("error getting server ID from the node: %w", err) } + + addr, _ := nodeAddressForLB(node, svcConf.preferredIPFamily) + if addr == "" { + // If node has no viable address let's ignore it. + continue + } + listOpts := neutronports.ListOpts{DeviceID: serverID} - allPorts, err := openstackutil.GetPorts(network, listOpts) + allPorts, err := openstackutil.GetPorts[PortWithPortSecurity](network, listOpts) if err != nil { return err } for _, port := range allPorts { + // You can't assign an SG to a port with port_security_enabled=false, skip them. + if !port.PortSecurityEnabled { + continue + } + // If the Security Group is already present on the port, skip it. if slices.Contains(port.SecurityGroups, sg) { continue } + // Only add SGs to the port actually attached to the LB + if !isPortMember(port, addr, svcConf.lbMemberSubnetID) { + continue + } + // Add the SG to the port // TODO(dulek): This isn't an atomic operation. In order to protect from lost update issues we should use // `revision_number` handling to make sure our update to `security_groups` field wasn't preceded @@ -768,7 +795,7 @@ func applyNodeSecurityGroupIDForLB(network *gophercloud.ServiceClient, nodes []* func disassociateSecurityGroupForLB(network *gophercloud.ServiceClient, sg string) error { // Find all the ports that have the security group associated. listOpts := neutronports.ListOpts{SecurityGroups: []string{sg}} - allPorts, err := openstackutil.GetPorts(network, listOpts) + allPorts, err := openstackutil.GetPorts[neutronports.Port](network, listOpts) if err != nil { return err } @@ -2382,7 +2409,7 @@ func (lbaas *LbaasV2) ensureAndUpdateOctaviaSecurityGroup(clusterName string, ap } } - if err := applyNodeSecurityGroupIDForLB(lbaas.network, nodes, lbSecGroupID); err != nil { + if err := applyNodeSecurityGroupIDForLB(lbaas.network, svcConf, nodes, lbSecGroupID); err != nil { return err } return nil diff --git a/pkg/openstack/openstack.go b/pkg/openstack/openstack.go index c0c8bcd16a..b67a58caf3 100644 --- a/pkg/openstack/openstack.go +++ b/pkg/openstack/openstack.go @@ -27,6 +27,7 @@ import ( "github.com/gophercloud/gophercloud" "github.com/gophercloud/gophercloud/openstack/compute/v2/extensions/availabilityzones" "github.com/gophercloud/gophercloud/openstack/compute/v2/servers" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/portsecurity" "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/trunk_details" neutronports "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" "github.com/spf13/pflag" @@ -78,6 +79,11 @@ type PortWithTrunkDetails struct { trunk_details.TrunkDetailsExt } +type PortWithPortSecurity struct { + neutronports.Port + portsecurity.PortSecurityExt +} + // LoadBalancer is used for creating and maintaining load balancers type LoadBalancer struct { secret *gophercloud.ServiceClient diff --git a/pkg/util/openstack/network.go b/pkg/util/openstack/network.go index 0a811a2ae4..889086f480 100644 --- a/pkg/util/openstack/network.go +++ b/pkg/util/openstack/network.go @@ -140,15 +140,16 @@ func getSubnet(networkSubnet string, subnetList []subnets.Subnet) *subnets.Subne } // GetPorts gets all the filtered ports. -func GetPorts(client *gophercloud.ServiceClient, listOpts neutronports.ListOpts) ([]neutronports.Port, error) { +func GetPorts[PortType interface{}](client *gophercloud.ServiceClient, listOpts neutronports.ListOpts) ([]PortType, error) { mc := metrics.NewMetricContext("port", "list") allPages, err := neutronports.List(client, listOpts).AllPages() if mc.ObserveRequest(err) != nil { - return []neutronports.Port{}, err + return []PortType{}, err } - allPorts, err := neutronports.ExtractPorts(allPages) + var allPorts []PortType + err = neutronports.ExtractPortsInto(allPages, &allPorts) if err != nil { - return []neutronports.Port{}, err + return []PortType{}, err } return allPorts, nil