Merge pull request kubernetes-sigs#2128 from elastx/nodeport-block-internet

🐛 Expose NodePorts on cluster network instead of 0.0.0.0/0

Author: k8s-ci-robot

pkg/cloud/services/networking/securitygroups.go

@@ -24,6 +24,7 @@ import (
 	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/attributestags"
 	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/groups"
 	"github.com/gophercloud/gophercloud/v2/openstack/networking/v2/extensions/security/rules"
+	"k8s.io/utils/net"
 
 	infrav1 "sigs.k8s.io/cluster-api-provider-openstack/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-openstack/pkg/record"
@@ -200,7 +201,19 @@ func (s *Service) generateDesiredSecGroups(openStackCluster *infrav1.OpenStackCl
 	workerRules := append([]resolvedSecurityGroupRuleSpec{}, defaultRules...)
 
 	controlPlaneRules = append(controlPlaneRules, getSGControlPlaneHTTPS()...)
-	workerRules = append(workerRules, getSGWorkerNodePort()...)
+
+	// Fetch subnet to use for worker node port rules
+	// In the future IPv6 support need to be added here
+	if openStackCluster.Status.Network != nil {
+		for _, subnet := range openStackCluster.Status.Network.Subnets {
+			if net.IsIPv4CIDRString(subnet.CIDR) {
+				workerRules = append(workerRules, getSGWorkerNodePortCIDR(subnet.CIDR)...)
+			}
+		}
+	}
+
+	// Add rules allowing nodepors from all cluster nodes, this will take effect even if no subnetCIDR is found to ensure all nodes can commincate over nodeports at all time
+	workerRules = append(workerRules, getSGWorkerNodePort(secWorkerGroupID, secControlPlaneGroupID)...)
 
 	// If we set additional ports to LB, we need create secgroup rules those ports, this apply to controlPlaneRules only
 	if openStackCluster.Spec.APIServerLoadBalancer.IsEnabled() {

pkg/cloud/services/networking/securitygroups_rules.go

@@ -142,23 +142,67 @@ func getSGControlPlaneHTTPS() []resolvedSecurityGroupRuleSpec {
 }
 
 // Allow all traffic, including from outside the cluster, to access node port services.
-func getSGWorkerNodePort() []resolvedSecurityGroupRuleSpec {
+func getSGWorkerNodePort(secWorkerGroupID string, secControlPlaneGroupID string) []resolvedSecurityGroupRuleSpec {
 	return []resolvedSecurityGroupRuleSpec{
 		{
-			Description:  "Node Port Services",
-			Direction:    "ingress",
-			EtherType:    "IPv4",
-			PortRangeMin: 30000,
-			PortRangeMax: 32767,
-			Protocol:     "tcp",
+			Description:   "Node Port Services",
+			Direction:     "ingress",
+			EtherType:     "IPv4",
+			PortRangeMin:  30000,
+			PortRangeMax:  32767,
+			Protocol:      "tcp",
+			RemoteGroupID: secWorkerGroupID,
 		},
 		{
-			Description:  "Node Port Services",
-			Direction:    "ingress",
-			EtherType:    "IPv4",
-			PortRangeMin: 30000,
-			PortRangeMax: 32767,
-			Protocol:     "udp",
+			Description:   "Node Port Services",
+			Direction:     "ingress",
+			EtherType:     "IPv4",
+			PortRangeMin:  30000,
+			PortRangeMax:  32767,
+			Protocol:      "udp",
+			RemoteGroupID: secWorkerGroupID,
+		},
+		{
+			Description:   "Node Port Services",
+			Direction:     "ingress",
+			EtherType:     "IPv4",
+			PortRangeMin:  30000,
+			PortRangeMax:  32767,
+			Protocol:      "tcp",
+			RemoteGroupID: secControlPlaneGroupID,
+		},
+		{
+			Description:   "Node Port Services",
+			Direction:     "ingress",
+			EtherType:     "IPv4",
+			PortRangeMin:  30000,
+			PortRangeMax:  32767,
+			Protocol:      "udp",
+			RemoteGroupID: secControlPlaneGroupID,
+		},
+	}
+}
+
+// Allow all traffic from a specific CIDR to access node port services.
+func getSGWorkerNodePortCIDR(cidr string) []resolvedSecurityGroupRuleSpec {
+	return []resolvedSecurityGroupRuleSpec{
+		{
+			Description:    "Node Port Services",
+			Direction:      "ingress",
+			EtherType:      "IPv4",
+			PortRangeMin:   30000,
+			PortRangeMax:   32767,
+			Protocol:       "tcp",
+			RemoteIPPrefix: cidr,
+		},
+		{
+			Description:    "Node Port Services",
+			Direction:      "ingress",
+			EtherType:      "IPv4",
+			PortRangeMin:   30000,
+			PortRangeMax:   32767,
+			Protocol:       "udp",
+			RemoteIPPrefix: cidr,
 		},
 	}
 }

pkg/cloud/services/networking/securitygroups_test.go

@@ -323,7 +323,7 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
 					ManagedSecurityGroups: &infrav1.ManagedSecurityGroups{},
 				},
 			},
-			expectedNumberSecurityGroupRules: 12,
+			expectedNumberSecurityGroupRules: 14,
 			wantErr:                          false,
 		},
 		{
@@ -342,7 +342,7 @@ func TestGenerateDesiredSecGroups(t *testing.T) {
 					},
 				},
 			},
-			expectedNumberSecurityGroupRules: 16,
+			expectedNumberSecurityGroupRules: 18,
 			wantErr:                          false,
 		},
 		{
@@ -570,13 +570,13 @@ func TestService_ReconcileSecurityGroups(t *testing.T) {
 					Return([]groups.SecGroup{{ID: "1", Name: workerSGName}}, nil)
 				m.ListSecGroup(groups.ListOpts{Name: bastionSGName}).Return(nil, nil)
 
-				// We expect a total of 12 rules to be created.
+				// We expect a total of 14 rules to be created.
 				// Nothing actually looks at the generated
 				// rules, but we give them unique IDs anyway
 				m.CreateSecGroupRule(gomock.Any()).DoAndReturn(func(opts rules.CreateOpts) (*rules.SecGroupRule, error) {
 					log.Info("Created rule", "securityGroup", opts.SecGroupID, "description", opts.Description)
 					return &rules.SecGroupRule{ID: uuid.NewString()}, nil
-				}).Times(12)
+				}).Times(14)
 			},
 			expectedClusterStatus: infrav1.OpenStackClusterStatus{
 				ControlPlaneSecurityGroup: &infrav1.SecurityGroupStatus{
@@ -605,13 +605,13 @@ func TestService_ReconcileSecurityGroups(t *testing.T) {
 				m.ListSecGroup(groups.ListOpts{Name: bastionSGName}).
 					Return([]groups.SecGroup{{ID: "2", Name: bastionSGName}}, nil)
 
-				// We expect a total of 12 rules to be created.
+				// We expect a total of 19 rules to be created.
 				// Nothing actually looks at the generated
 				// rules, but we give them unique IDs anyway
 				m.CreateSecGroupRule(gomock.Any()).DoAndReturn(func(opts rules.CreateOpts) (*rules.SecGroupRule, error) {
 					log.Info("Created rule", "securityGroup", opts.SecGroupID, "description", opts.Description)
 					return &rules.SecGroupRule{ID: uuid.NewString()}, nil
-				}).Times(17)
+				}).Times(19)
 			},
 			expectedClusterStatus: infrav1.OpenStackClusterStatus{
 				ControlPlaneSecurityGroup: &infrav1.SecurityGroupStatus{