diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go index e6d9982cbd..17855592b9 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.AdditionalNodeIngressRules = restored.Spec.NetworkSpec.AdditionalNodeIngressRules dst.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks = restored.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks if restored.Spec.NetworkSpec.VPC.IPAMPool != nil { diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 2d1641f424..1c8a27b61c 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.AdditionalNodeIngressRules 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 88d5965d3e..bc917fe439 100644 --- a/api/v1beta2/awscluster_webhook.go +++ b/api/v1beta2/awscluster_webhook.go @@ -264,9 +264,8 @@ func (r *AWSCluster) validateNetwork() field.ErrorList { allErrs = append(allErrs, field.Invalid(field.NewPath("ipamPool"), r.Spec.NetworkSpec.VPC.IPAMPool, "ipamPool must have either id or name")) } - for _, rule := range r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules { - allErrs = append(allErrs, r.validateIngressRule(rule)...) - } + allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "network", "additionalControlPlaneIngressRules"), r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules)...) + allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "network", "additionalNodeIngressRules"), r.Spec.NetworkSpec.AdditionalNodeIngressRules)...) for cidrBlockIndex, cidrBlock := range r.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks { if _, _, err := net.ParseCIDR(cidrBlock); err != nil { @@ -326,18 +325,11 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList { // Additional listeners are only supported for NLBs. // Validate the control plane load balancers. - loadBalancers := []*AWSLoadBalancerSpec{ - r.Spec.ControlPlaneLoadBalancer, - r.Spec.SecondaryControlPlaneLoadBalancer, + if r.Spec.ControlPlaneLoadBalancer != nil { + allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules)...) } - for _, cp := range loadBalancers { - if cp == nil { - continue - } - - for _, rule := range cp.IngressRules { - allErrs = append(allErrs, r.validateIngressRule(rule)...) - } + if r.Spec.SecondaryControlPlaneLoadBalancer != nil { + allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "ingressRules"), r.Spec.SecondaryControlPlaneLoadBalancer.IngressRules)...) } if r.Spec.ControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeDisabled { @@ -381,15 +373,18 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList { return allErrs } -func (r *AWSCluster) validateIngressRule(rule IngressRule) field.ErrorList { +func (r *AWSCluster) validateIngressRules(path *field.Path, rules []IngressRule) field.ErrorList { var allErrs field.ErrorList - if rule.NatGatewaysIPsSource { - if rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil || rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil { - allErrs = append(allErrs, field.Invalid(field.NewPath("additionalControlPlaneIngressRules"), r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules, "CIDR blocks and security group IDs or security group roles cannot be used together")) - } - } else { - if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) { - allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules, "CIDR blocks and security group IDs or security group roles cannot be used together")) + for ruleIndex, rule := range rules { + rulePath := path.Index(ruleIndex) + if rule.NatGatewaysIPsSource { + if rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil || rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil { + allErrs = append(allErrs, field.Invalid(rulePath, rules, "natGatewaysIPsSource cannot be used together with CIDR blocks, security group IDs or security group roles")) + } + } else { + if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) { + allErrs = append(allErrs, field.Invalid(rulePath, rules, "CIDR blocks and security group IDs or security group roles cannot be used together")) + } } } return allErrs diff --git a/api/v1beta2/awscluster_webhook_test.go b/api/v1beta2/awscluster_webhook_test.go index 13260b0341..6b8dc10489 100644 --- a/api/v1beta2/awscluster_webhook_test.go +++ b/api/v1beta2/awscluster_webhook_test.go @@ -638,6 +638,91 @@ func TestAWSClusterValidateCreate(t *testing.T) { }, wantErr: false, }, + { + name: "accepts node ingress rules with source security group id and role", + cluster: &AWSCluster{ + Spec: AWSClusterSpec{ + NetworkSpec: NetworkSpec{ + AdditionalNodeIngressRules: []IngressRule{ + { + Protocol: SecurityGroupProtocolTCP, + SourceSecurityGroupIDs: []string{"test"}, + SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion}, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "rejects node ingress rules with cidr block and source security group id", + cluster: &AWSCluster{ + Spec: AWSClusterSpec{ + NetworkSpec: NetworkSpec{ + AdditionalNodeIngressRules: []IngressRule{ + { + Protocol: SecurityGroupProtocolTCP, + CidrBlocks: []string{"test"}, + SourceSecurityGroupIDs: []string{"test"}, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "rejects node ingress rules with cidr block and source security group id and role", + cluster: &AWSCluster{ + Spec: AWSClusterSpec{ + NetworkSpec: NetworkSpec{ + AdditionalNodeIngressRules: []IngressRule{ + { + Protocol: SecurityGroupProtocolTCP, + IPv6CidrBlocks: []string{"test"}, + SourceSecurityGroupIDs: []string{"test"}, + SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion}, + }, + }, + }, + }, + }, + wantErr: true, + }, + { + name: "accepts node ingress rules with cidr block", + cluster: &AWSCluster{ + Spec: AWSClusterSpec{ + NetworkSpec: NetworkSpec{ + AdditionalNodeIngressRules: []IngressRule{ + { + Protocol: SecurityGroupProtocolTCP, + CidrBlocks: []string{"test"}, + }, + }, + }, + }, + }, + wantErr: false, + }, + { + name: "accepts node ingress rules with source security group id and role", + cluster: &AWSCluster{ + Spec: AWSClusterSpec{ + NetworkSpec: NetworkSpec{ + AdditionalNodeIngressRules: []IngressRule{ + { + Protocol: SecurityGroupProtocolTCP, + SourceSecurityGroupIDs: []string{"test"}, + SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion}, + }, + }, + }, + }, + }, + wantErr: false, + }, { name: "accepts cidrBlock for default node port ingress rule", cluster: &AWSCluster{ diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go index 05a0d9a929..9974cbddb1 100644 --- a/api/v1beta2/network_types.go +++ b/api/v1beta2/network_types.go @@ -352,6 +352,10 @@ type NetworkSpec struct { // +optional AdditionalControlPlaneIngressRules []IngressRule `json:"additionalControlPlaneIngressRules,omitempty"` + // AdditionalNodeIngressRules is an optional set of ingress rules to add to every node + // +optional + AdditionalNodeIngressRules []IngressRule `json:"additionalNodeIngressRules,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 diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index c727483f6b..e365f9be37 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -1748,6 +1748,13 @@ func (in *NetworkSpec) DeepCopyInto(out *NetworkSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.AdditionalNodeIngressRules != nil { + in, out := &in.AdditionalNodeIngressRules, &out.AdditionalNodeIngressRules + *out = make([]IngressRule, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } if in.NodePortIngressRuleCidrBlocks != nil { in, out := &in.NodePortIngressRuleCidrBlocks, &out.NodePortIngressRuleCidrBlocks *out = make([]string, len(*in)) 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 21be0fc920..9b929ce6e9 100644 --- a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml +++ b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml @@ -443,6 +443,83 @@ spec: - toPort type: object type: array + additionalNodeIngressRules: + description: AdditionalNodeIngressRules is an optional set of + ingress rules to add to every node + items: + description: IngressRule defines an AWS ingress rule for security + groups. + properties: + cidrBlocks: + description: List of CIDR blocks to allow access from. Cannot + be specified with SourceSecurityGroupID. + items: + type: string + type: array + description: + description: Description provides extended information about + the ingress rule. + type: string + fromPort: + description: FromPort is the start of port range. + format: int64 + type: integer + ipv6CidrBlocks: + description: List of IPv6 CIDR blocks to allow access from. + Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + natGatewaysIPsSource: + description: NatGatewaysIPsSource use the NAT gateways IPs + as the source for the ingress rule. + type: boolean + protocol: + description: Protocol is the protocol for the ingress rule. + Accepted values are "-1" (all), "4" (IP in IP),"tcp", + "udp", "icmp", and "58" (ICMPv6), "50" (ESP). + enum: + - "-1" + - "4" + - tcp + - udp + - icmp + - "58" + - "50" + type: string + sourceSecurityGroupIds: + description: The security group id to allow access from. + Cannot be specified with CidrBlocks. + items: + type: string + type: array + sourceSecurityGroupRoles: + description: |- + The security group role to allow access from. Cannot be specified with CidrBlocks. + The field will be combined with source security group IDs if specified. + items: + description: SecurityGroupRole defines the unique role + of a security group. + enum: + - bastion + - node + - controlplane + - apiserver-lb + - lb + - node-eks-additional + type: string + type: array + toPort: + description: ToPort is the end of port range. + format: int64 + type: integer + required: + - description + - fromPort + - protocol + - toPort + type: object + type: array cni: description: CNI configuration properties: @@ -2465,6 +2542,83 @@ spec: - toPort type: object type: array + additionalNodeIngressRules: + description: AdditionalNodeIngressRules is an optional set of + ingress rules to add to every node + items: + description: IngressRule defines an AWS ingress rule for security + groups. + properties: + cidrBlocks: + description: List of CIDR blocks to allow access from. Cannot + be specified with SourceSecurityGroupID. + items: + type: string + type: array + description: + description: Description provides extended information about + the ingress rule. + type: string + fromPort: + description: FromPort is the start of port range. + format: int64 + type: integer + ipv6CidrBlocks: + description: List of IPv6 CIDR blocks to allow access from. + Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + natGatewaysIPsSource: + description: NatGatewaysIPsSource use the NAT gateways IPs + as the source for the ingress rule. + type: boolean + protocol: + description: Protocol is the protocol for the ingress rule. + Accepted values are "-1" (all), "4" (IP in IP),"tcp", + "udp", "icmp", and "58" (ICMPv6), "50" (ESP). + enum: + - "-1" + - "4" + - tcp + - udp + - icmp + - "58" + - "50" + type: string + sourceSecurityGroupIds: + description: The security group id to allow access from. + Cannot be specified with CidrBlocks. + items: + type: string + type: array + sourceSecurityGroupRoles: + description: |- + The security group role to allow access from. Cannot be specified with CidrBlocks. + The field will be combined with source security group IDs if specified. + items: + description: SecurityGroupRole defines the unique role + of a security group. + enum: + - bastion + - node + - controlplane + - apiserver-lb + - lb + - node-eks-additional + type: string + type: array + toPort: + description: ToPort is the end of port range. + format: int64 + type: integer + required: + - description + - fromPort + - protocol + - toPort + type: object + type: array cni: description: CNI configuration properties: 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 3ccb164ec1..6e9553ff60 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml @@ -1379,6 +1379,83 @@ spec: - toPort type: object type: array + additionalNodeIngressRules: + description: AdditionalNodeIngressRules is an optional set of + ingress rules to add to every node + items: + description: IngressRule defines an AWS ingress rule for security + groups. + properties: + cidrBlocks: + description: List of CIDR blocks to allow access from. Cannot + be specified with SourceSecurityGroupID. + items: + type: string + type: array + description: + description: Description provides extended information about + the ingress rule. + type: string + fromPort: + description: FromPort is the start of port range. + format: int64 + type: integer + ipv6CidrBlocks: + description: List of IPv6 CIDR blocks to allow access from. + Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + natGatewaysIPsSource: + description: NatGatewaysIPsSource use the NAT gateways IPs + as the source for the ingress rule. + type: boolean + protocol: + description: Protocol is the protocol for the ingress rule. + Accepted values are "-1" (all), "4" (IP in IP),"tcp", + "udp", "icmp", and "58" (ICMPv6), "50" (ESP). + enum: + - "-1" + - "4" + - tcp + - udp + - icmp + - "58" + - "50" + type: string + sourceSecurityGroupIds: + description: The security group id to allow access from. + Cannot be specified with CidrBlocks. + items: + type: string + type: array + sourceSecurityGroupRoles: + description: |- + The security group role to allow access from. Cannot be specified with CidrBlocks. + The field will be combined with source security group IDs if specified. + items: + description: SecurityGroupRole defines the unique role + of a security group. + enum: + - bastion + - node + - controlplane + - apiserver-lb + - lb + - node-eks-additional + type: string + type: array + toPort: + description: ToPort is the end of port range. + format: int64 + type: integer + required: + - description + - fromPort + - protocol + - toPort + type: object + type: array cni: description: CNI configuration properties: 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 feb6ec2ef2..52ec03e1d3 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml @@ -976,6 +976,84 @@ spec: - toPort type: object type: array + additionalNodeIngressRules: + description: AdditionalNodeIngressRules is an optional + set of ingress rules to add to every node + items: + description: IngressRule defines an AWS ingress rule + for security groups. + properties: + cidrBlocks: + description: List of CIDR blocks to allow access + from. Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + description: + description: Description provides extended information + about the ingress rule. + type: string + fromPort: + description: FromPort is the start of port range. + format: int64 + type: integer + ipv6CidrBlocks: + description: List of IPv6 CIDR blocks to allow access + from. Cannot be specified with SourceSecurityGroupID. + items: + type: string + type: array + natGatewaysIPsSource: + description: NatGatewaysIPsSource use the NAT gateways + IPs as the source for the ingress rule. + type: boolean + protocol: + description: Protocol is the protocol for the ingress + rule. Accepted values are "-1" (all), "4" (IP + in IP),"tcp", "udp", "icmp", and "58" (ICMPv6), + "50" (ESP). + enum: + - "-1" + - "4" + - tcp + - udp + - icmp + - "58" + - "50" + type: string + sourceSecurityGroupIds: + description: The security group id to allow access + from. Cannot be specified with CidrBlocks. + items: + type: string + type: array + sourceSecurityGroupRoles: + description: |- + The security group role to allow access from. Cannot be specified with CidrBlocks. + The field will be combined with source security group IDs if specified. + items: + description: SecurityGroupRole defines the unique + role of a security group. + enum: + - bastion + - node + - controlplane + - apiserver-lb + - lb + - node-eks-additional + type: string + type: array + toPort: + description: ToPort is the end of port range. + format: int64 + type: integer + required: + - description + - fromPort + - protocol + - toPort + type: object + type: array cni: description: CNI configuration properties: diff --git a/pkg/cloud/scope/cluster.go b/pkg/cloud/scope/cluster.go index ddea8a4fcc..d220a44eec 100644 --- a/pkg/cloud/scope/cluster.go +++ b/pkg/cloud/scope/cluster.go @@ -413,6 +413,11 @@ func (s *ClusterScope) AdditionalControlPlaneIngressRules() []infrav1.IngressRul return s.AWSCluster.Spec.NetworkSpec.DeepCopy().AdditionalControlPlaneIngressRules } +// AdditionalNodeIngressRules returns the additional ingress rules for the node security group. +func (s *ClusterScope) AdditionalNodeIngressRules() []infrav1.IngressRule { + return s.AWSCluster.Spec.NetworkSpec.DeepCopy().AdditionalNodeIngressRules +} + // UnstructuredControlPlane returns the unstructured object for the control plane, if any. // When the reference is not set, it returns an empty object. func (s *ClusterScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) { diff --git a/pkg/cloud/scope/managedcontrolplane.go b/pkg/cloud/scope/managedcontrolplane.go index 14ded92263..c931e59c51 100644 --- a/pkg/cloud/scope/managedcontrolplane.go +++ b/pkg/cloud/scope/managedcontrolplane.go @@ -475,6 +475,11 @@ func (s *ManagedControlPlaneScope) AdditionalControlPlaneIngressRules() []infrav return s.ControlPlane.Spec.NetworkSpec.DeepCopy().AdditionalControlPlaneIngressRules } +// AdditionalNodeIngressRules returns the additional ingress rules for the node security group. +func (s *ManagedControlPlaneScope) AdditionalNodeIngressRules() []infrav1.IngressRule { + return s.ControlPlane.Spec.NetworkSpec.DeepCopy().AdditionalNodeIngressRules +} + // UnstructuredControlPlane returns the unstructured object for the control plane, if any. // When the reference is not set, it returns an empty object. func (s *ManagedControlPlaneScope) UnstructuredControlPlane() (*unstructured.Unstructured, error) { diff --git a/pkg/cloud/scope/sg.go b/pkg/cloud/scope/sg.go index 145bf207c0..05409d835c 100644 --- a/pkg/cloud/scope/sg.go +++ b/pkg/cloud/scope/sg.go @@ -56,6 +56,9 @@ type SGScope interface { // AdditionalControlPlaneIngressRules returns the additional ingress rules for the control plane security group. AdditionalControlPlaneIngressRules() []infrav1.IngressRule + // AdditionalNodeIngressRules returns the additional ingress rules for the node security group. + AdditionalNodeIngressRules() []infrav1.IngressRule + // ControlPlaneLoadBalancers returns both the ControlPlaneLoadBalancer and SecondaryControlPlaneLoadBalancer AWSLoadBalancerSpecs. // The control plane load balancers should always be returned in the above order. ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec diff --git a/pkg/cloud/services/securitygroup/securitygroups.go b/pkg/cloud/services/securitygroup/securitygroups.go index a989c0e108..a7fcaa1b19 100644 --- a/pkg/cloud/services/securitygroup/securitygroups.go +++ b/pkg/cloud/services/securitygroup/securitygroups.go @@ -680,6 +680,13 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) ( IPv6CidrBlocks: []string{services.AnyIPv6CidrBlock}, }) } + + additionalIngressRules, err := s.processIngressRulesSGs(s.scope.AdditionalNodeIngressRules()) + if err != nil { + return nil, err + } + rules = append(rules, additionalIngressRules...) + return append(cniRules, rules...), nil case infrav1.SecurityGroupEKSNodeAdditional: ingressRules := s.scope.AdditionalControlPlaneIngressRules()