diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go index 954fea7a4c..e6d9982cbd 100644 --- a/api/v1beta1/awscluster_conversion.go +++ b/api/v1beta1/awscluster_conversion.go @@ -85,6 +85,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error { } dst.Spec.NetworkSpec.AdditionalControlPlaneIngressRules = restored.Spec.NetworkSpec.AdditionalControlPlaneIngressRules + dst.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks = restored.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks if restored.Spec.NetworkSpec.VPC.IPAMPool != nil { if dst.Spec.NetworkSpec.VPC.IPAMPool == nil { diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index a85eaf08d3..0ce3374db8 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -2082,6 +2082,7 @@ func autoConvert_v1beta2_NetworkSpec_To_v1beta1_NetworkSpec(in *v1beta2.NetworkS out.CNI = (*CNISpec)(unsafe.Pointer(in.CNI)) out.SecurityGroupOverrides = *(*map[SecurityGroupRole]string)(unsafe.Pointer(&in.SecurityGroupOverrides)) // WARNING: in.AdditionalControlPlaneIngressRules requires manual conversion: does not exist in peer-type + // WARNING: in.NodePortIngressRuleCidrBlocks requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta2/awscluster_webhook.go b/api/v1beta2/awscluster_webhook.go index 1cb8012282..e95bdb044e 100644 --- a/api/v1beta2/awscluster_webhook.go +++ b/api/v1beta2/awscluster_webhook.go @@ -18,6 +18,7 @@ package v1beta2 import ( "fmt" + "net" "strings" "github.com/google/go-cmp/cmp" @@ -267,6 +268,12 @@ func (r *AWSCluster) validateNetwork() field.ErrorList { allErrs = append(allErrs, r.validateIngressRule(rule)...) } + for cidrBlockIndex, cidrBlock := range r.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks { + if _, _, err := net.ParseCIDR(cidrBlock); err != nil { + allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "network", fmt.Sprintf("nodePortIngressRuleCidrBlocks[%d]", cidrBlockIndex)), r.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks, "CIDR block is invalid")) + } + } + if r.Spec.NetworkSpec.VPC.ElasticIPPool != nil { eipp := r.Spec.NetworkSpec.VPC.ElasticIPPool if eipp.PublicIpv4Pool != nil { diff --git a/api/v1beta2/awscluster_webhook_test.go b/api/v1beta2/awscluster_webhook_test.go index 5252054f7c..41bc5d3b1c 100644 --- a/api/v1beta2/awscluster_webhook_test.go +++ b/api/v1beta2/awscluster_webhook_test.go @@ -638,6 +638,28 @@ func TestAWSClusterValidateCreate(t *testing.T) { }, wantErr: false, }, + { + name: "accepts cidrBlock for default node port ingress rule", + cluster: &AWSCluster{ + Spec: AWSClusterSpec{ + NetworkSpec: NetworkSpec{ + NodePortIngressRuleCidrBlocks: []string{"10.0.0.0/16"}, + }, + }, + }, + wantErr: false, + }, + { + name: "reject invalid cidrBlock for default node port ingress rule", + cluster: &AWSCluster{ + Spec: AWSClusterSpec{ + NetworkSpec: NetworkSpec{ + NodePortIngressRuleCidrBlocks: []string{"10.0.0.0"}, + }, + }, + }, + wantErr: true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go index 969d2d8b77..05a0d9a929 100644 --- a/api/v1beta2/network_types.go +++ b/api/v1beta2/network_types.go @@ -351,6 +351,11 @@ type NetworkSpec struct { // AdditionalControlPlaneIngressRules is an optional set of ingress rules to add to the control plane // +optional AdditionalControlPlaneIngressRules []IngressRule `json:"additionalControlPlaneIngressRules,omitempty"` + + // NodePortIngressRuleCidrBlocks is an optional set of CIDR blocks to allow traffic to nodes' NodePort services. + // If none are specified here, all IPs are allowed to connect. + // +optional + NodePortIngressRuleCidrBlocks []string `json:"nodePortIngressRuleCidrBlocks,omitempty"` } // IPv6 contains ipv6 specific settings for the network. diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index b3eaa6c08d..c727483f6b 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -1748,6 +1748,11 @@ func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.NodePortIngressRuleCidrBlocks != nil { + in, out := &in.NodePortIngressRuleCidrBlocks, &out.NodePortIngressRuleCidrBlocks + *out = make([]string, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkSpec. diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml index 345b3f4379..e8aae60f5a 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -474,6 +474,13 @@ spec: type: object type: array type: object + nodePortIngressRuleCidrBlocks: + description: |- + NodePortIngressRuleCidrBlocks is an optional set of CIDR blocks to allow traffic to nodes' NodePort services. + If none are specified here, all IPs are allowed to connect. + items: + type: string + type: array securityGroupOverrides: additionalProperties: type: string @@ -2500,6 +2507,13 @@ spec: type: object type: array type: object + nodePortIngressRuleCidrBlocks: + description: |- + NodePortIngressRuleCidrBlocks is an optional set of CIDR blocks to allow traffic to nodes' NodePort services. + If none are specified here, all IPs are allowed to connect. + items: + type: string + type: array securityGroupOverrides: additionalProperties: type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml index f3eb3a2fc7..7d6cc0a025 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml @@ -1414,6 +1414,13 @@ spec: type: object type: array type: object + nodePortIngressRuleCidrBlocks: + description: |- + NodePortIngressRuleCidrBlocks is an optional set of CIDR blocks to allow traffic to nodes' NodePort services. + If none are specified here, all IPs are allowed to connect. + items: + type: string + type: array securityGroupOverrides: additionalProperties: type: string diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml index 94beb2e660..bd35c78474 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml @@ -1011,6 +1011,13 @@ spec: type: object type: array type: object + nodePortIngressRuleCidrBlocks: + description: |- + NodePortIngressRuleCidrBlocks is an optional set of CIDR blocks to allow traffic to nodes' NodePort services. + If none are specified here, all IPs are allowed to connect. + items: + type: string + type: array securityGroupOverrides: additionalProperties: type: string diff --git a/pkg/cloud/scope/cluster.go b/pkg/cloud/scope/cluster.go index 9efaa16d7a..ddea8a4fcc 100644 --- a/pkg/cloud/scope/cluster.go +++ b/pkg/cloud/scope/cluster.go @@ -418,3 +418,8 @@ func (s *ClusterScope) AdditionalControlPlaneIngressRules() []infrav1.IngressRul func (s *ClusterScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) { return getUnstructuredControlPlane(context.TODO(), s.client, s.Cluster) } + +// NodePortIngressRuleCidrBlocks returns the CIDR blocks for the node NodePort ingress rules. +func (s *ClusterScope) NodePortIngressRuleCidrBlocks() []string { + return s.AWSCluster.Spec.NetworkSpec.DeepCopy().NodePortIngressRuleCidrBlocks +} diff --git a/pkg/cloud/scope/managedcontrolplane.go b/pkg/cloud/scope/managedcontrolplane.go index 9c3b6b208d..df64e87f44 100644 --- a/pkg/cloud/scope/managedcontrolplane.go +++ b/pkg/cloud/scope/managedcontrolplane.go @@ -480,3 +480,8 @@ func (s *ManagedControlPlaneScope) AdditionalControlPlaneIngressRules() []infrav func (s *ManagedControlPlaneScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) { return getUnstructuredControlPlane(context.TODO(), s.Client, s.Cluster) } + +// NodePortIngressRuleCidrBlocks returns the CIDR blocks for the node NodePort ingress rules. +func (s *ManagedControlPlaneScope) NodePortIngressRuleCidrBlocks() []string { + return nil +} diff --git a/pkg/cloud/scope/sg.go b/pkg/cloud/scope/sg.go index 5db8282c86..145bf207c0 100644 --- a/pkg/cloud/scope/sg.go +++ b/pkg/cloud/scope/sg.go @@ -59,4 +59,7 @@ type SGScope interface { // ControlPlaneLoadBalancers returns both the ControlPlaneLoadBalancer and SecondaryControlPlaneLoadBalancer AWSLoadBalancerSpecs. // The control plane load balancers should always be returned in the above order. ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec + + // NodePortIngressRuleCidrBlocks returns the CIDR blocks for the node NodePort ingress rules. + NodePortIngressRuleCidrBlocks() []string } diff --git a/pkg/cloud/services/securitygroup/securitygroups.go b/pkg/cloud/services/securitygroup/securitygroups.go index f1f82193cc..bf30f60824 100644 --- a/pkg/cloud/services/securitygroup/securitygroups.go +++ b/pkg/cloud/services/securitygroup/securitygroups.go @@ -591,7 +591,6 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) ( }, } } - cidrBlocks := []string{services.AnyIPv4CidrBlock} switch role { case infrav1.SecurityGroupBastion: return infrav1.IngressRules{ @@ -645,6 +644,10 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) ( return append(cniRules, rules...), nil case infrav1.SecurityGroupNode: + cidrBlocks := []string{services.AnyIPv4CidrBlock} + if scopeCidrBlocks := s.scope.NodePortIngressRuleCidrBlocks(); len(scopeCidrBlocks) > 0 { + cidrBlocks = scopeCidrBlocks + } rules := infrav1.IngressRules{ { Description: "Node Port Services", diff --git a/pkg/cloud/services/securitygroup/securitygroups_test.go b/pkg/cloud/services/securitygroup/securitygroups_test.go index 3bdf795ea8..f522104210 100644 --- a/pkg/cloud/services/securitygroup/securitygroups_test.go +++ b/pkg/cloud/services/securitygroup/securitygroups_test.go @@ -2237,3 +2237,97 @@ func TestExpandIngressRules(t *testing.T) { }) } } + +func TestNodePortServicesIngressRules(t *testing.T) { + scheme := runtime.NewScheme() + _ = infrav1.AddToScheme(scheme) + + testCases := []struct { + name string + cidrBlocks []string + expectedIngresRules infrav1.IngressRules + }{ + { + name: "default node ports services ingress rules, no node port cidr block provided", + cidrBlocks: nil, + expectedIngresRules: infrav1.IngressRules{ + { + Description: "Node Port Services", + Protocol: infrav1.SecurityGroupProtocolTCP, + FromPort: 30000, + ToPort: 32767, + CidrBlocks: []string{services.AnyIPv4CidrBlock}, + }, + { + Description: "Kubelet API", + Protocol: infrav1.SecurityGroupProtocolTCP, + FromPort: 10250, + ToPort: 10250, + SourceSecurityGroupIDs: []string{"Id1", "Id2"}, + }, + }, + }, + { + name: "node port cidr block provided, no default cidr block used for node port services ingress rule", + cidrBlocks: []string{"10.0.0.0/16"}, + expectedIngresRules: infrav1.IngressRules{ + { + Description: "Node Port Services", + Protocol: infrav1.SecurityGroupProtocolTCP, + FromPort: 30000, + ToPort: 32767, + CidrBlocks: []string{"10.0.0.0/16"}, + }, + { + Description: "Kubelet API", + Protocol: infrav1.SecurityGroupProtocolTCP, + FromPort: 10250, + ToPort: 10250, + SourceSecurityGroupIDs: []string{"Id1", "Id2"}, + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + cs, err := scope.NewClusterScope(scope.ClusterScopeParams{ + Client: fake.NewClientBuilder().WithScheme(scheme).Build(), + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test-cluster"}, + }, + AWSCluster: &infrav1.AWSCluster{ + Spec: infrav1.AWSClusterSpec{ + ControlPlaneLoadBalancer: &infrav1.AWSLoadBalancerSpec{}, + NetworkSpec: infrav1.NetworkSpec{ + VPC: infrav1.VPCSpec{ + CidrBlock: "10.0.0.0/16", + }, + NodePortIngressRuleCidrBlocks: tc.cidrBlocks, + }, + }, + Status: infrav1.AWSClusterStatus{ + Network: infrav1.NetworkStatus{ + SecurityGroups: map[infrav1.SecurityGroupRole]infrav1.SecurityGroup{ + infrav1.SecurityGroupControlPlane: {ID: "Id1"}, + infrav1.SecurityGroupNode: {ID: "Id2"}, + }, + }, + }, + }, + }) + if err != nil { + t.Fatalf("Failed to create test context: %v", err) + } + + s := NewService(cs, testSecurityGroupRoles) + rules, err := s.getSecurityGroupIngressRules(infrav1.SecurityGroupNode) + if err != nil { + t.Fatalf("Failed to lookup node security group ingress rules: %v", err) + } + + g := NewGomegaWithT(t) + g.Expect(rules).To(Equal(tc.expectedIngresRules)) + }) + } +}