Skip to content

Commit f852a0b

Browse files
AndiDogAndiDog
committed
Support additional security group ingress rules for all nodes
1 parent df09e6c commit f852a0b

13 files changed

+444
-22
lines changed

‎api/v1beta1/awscluster_conversion.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {
8585
}
8686

8787
dst.Spec.NetworkSpec.AdditionalControlPlaneIngressRules = restored.Spec.NetworkSpec.AdditionalControlPlaneIngressRules
88+
dst.Spec.NetworkSpec.AdditionalNodeIngressRules = restored.Spec.NetworkSpec.AdditionalNodeIngressRules
8889
dst.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks = restored.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks
8990

9091
if restored.Spec.NetworkSpec.VPC.IPAMPool != nil {

‎api/v1beta1/zz_generated.conversion.go‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎api/v1beta2/awscluster_webhook.go‎

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -264,9 +264,8 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
264264
allErrs = append(allErrs, field.Invalid(field.NewPath("ipamPool"), r.Spec.NetworkSpec.VPC.IPAMPool, "ipamPool must have either id or name"))
265265
}
266266

267-
for _, rule := range r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules {
268-
allErrs = append(allErrs, r.validateIngressRule(rule)...)
269-
}
267+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "network", "additionalControlPlaneIngressRules"), r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules)...)
268+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "network", "additionalNodeIngressRules"), r.Spec.NetworkSpec.AdditionalNodeIngressRules)...)
270269

271270
for cidrBlockIndex, cidrBlock := range r.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks {
272271
if _, _, err := net.ParseCIDR(cidrBlock); err != nil {
@@ -326,18 +325,11 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
326325

327326
// Additional listeners are only supported for NLBs.
328327
// Validate the control plane load balancers.
329-
loadBalancers := []*AWSLoadBalancerSpec{
330-
r.Spec.ControlPlaneLoadBalancer,
331-
r.Spec.SecondaryControlPlaneLoadBalancer,
328+
if r.Spec.ControlPlaneLoadBalancer != nil {
329+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules)...)
332330
}
333-
for _, cp := range loadBalancers {
334-
if cp == nil {
335-
continue
336-
}
337-
338-
for _, rule := range cp.IngressRules {
339-
allErrs = append(allErrs, r.validateIngressRule(rule)...)
340-
}
331+
if r.Spec.SecondaryControlPlaneLoadBalancer != nil {
332+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "ingressRules"), r.Spec.SecondaryControlPlaneLoadBalancer.IngressRules)...)
341333
}
342334

343335
if r.Spec.ControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeDisabled {
@@ -381,15 +373,18 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
381373
return allErrs
382374
}
383375

384-
func (r *AWSCluster) validateIngressRule(rule IngressRule) field.ErrorList {
376+
func (r *AWSCluster) validateIngressRules(path *field.Path, rules []IngressRule) field.ErrorList {
385377
var allErrs field.ErrorList
386-
if rule.NatGatewaysIPsSource {
387-
if rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil || rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil {
388-
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"))
389-
}
390-
} else {
391-
if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) {
392-
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"))
378+
for ruleIndex, rule := range rules {
379+
rulePath := path.Index(ruleIndex)
380+
if rule.NatGatewaysIPsSource {
381+
if rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil || rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil {
382+
allErrs = append(allErrs, field.Invalid(rulePath, rules, "natGatewaysIPsSource cannot be used together with CIDR blocks, security group IDs or security group roles"))
383+
}
384+
} else {
385+
if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) {
386+
allErrs = append(allErrs, field.Invalid(rulePath, rules, "CIDR blocks and security group IDs or security group roles cannot be used together"))
387+
}
393388
}
394389
}
395390
return allErrs

‎api/v1beta2/awscluster_webhook_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,91 @@ func TestAWSClusterValidateCreate(t *testing.T) {
638638
},
639639
wantErr: false,
640640
},
641+
{
642+
name: "accepts node ingress rules with source security group id and role",
643+
cluster: &AWSCluster{
644+
Spec: AWSClusterSpec{
645+
NetworkSpec: NetworkSpec{
646+
AdditionalNodeIngressRules: []IngressRule{
647+
{
648+
Protocol: SecurityGroupProtocolTCP,
649+
SourceSecurityGroupIDs: []string{"test"},
650+
SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion},
651+
},
652+
},
653+
},
654+
},
655+
},
656+
wantErr: false,
657+
},
658+
{
659+
name: "rejects node ingress rules with cidr block and source security group id",
660+
cluster: &AWSCluster{
661+
Spec: AWSClusterSpec{
662+
NetworkSpec: NetworkSpec{
663+
AdditionalNodeIngressRules: []IngressRule{
664+
{
665+
Protocol: SecurityGroupProtocolTCP,
666+
CidrBlocks: []string{"test"},
667+
SourceSecurityGroupIDs: []string{"test"},
668+
},
669+
},
670+
},
671+
},
672+
},
673+
wantErr: true,
674+
},
675+
{
676+
name: "rejects node ingress rules with cidr block and source security group id and role",
677+
cluster: &AWSCluster{
678+
Spec: AWSClusterSpec{
679+
NetworkSpec: NetworkSpec{
680+
AdditionalNodeIngressRules: []IngressRule{
681+
{
682+
Protocol: SecurityGroupProtocolTCP,
683+
IPv6CidrBlocks: []string{"test"},
684+
SourceSecurityGroupIDs: []string{"test"},
685+
SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion},
686+
},
687+
},
688+
},
689+
},
690+
},
691+
wantErr: true,
692+
},
693+
{
694+
name: "accepts node ingress rules with cidr block",
695+
cluster: &AWSCluster{
696+
Spec: AWSClusterSpec{
697+
NetworkSpec: NetworkSpec{
698+
AdditionalNodeIngressRules: []IngressRule{
699+
{
700+
Protocol: SecurityGroupProtocolTCP,
701+
CidrBlocks: []string{"test"},
702+
},
703+
},
704+
},
705+
},
706+
},
707+
wantErr: false,
708+
},
709+
{
710+
name: "accepts node ingress rules with source security group id and role",
711+
cluster: &AWSCluster{
712+
Spec: AWSClusterSpec{
713+
NetworkSpec: NetworkSpec{
714+
AdditionalNodeIngressRules: []IngressRule{
715+
{
716+
Protocol: SecurityGroupProtocolTCP,
717+
SourceSecurityGroupIDs: []string{"test"},
718+
SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion},
719+
},
720+
},
721+
},
722+
},
723+
},
724+
wantErr: false,
725+
},
641726
{
642727
name: "accepts cidrBlock for default node port ingress rule",
643728
cluster: &AWSCluster{

‎api/v1beta2/network_types.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,10 @@ type NetworkSpec struct {
352352
// +optional
353353
AdditionalControlPlaneIngressRules []IngressRule `json:"additionalControlPlaneIngressRules,omitempty"`
354354

355+
// AdditionalNodeIngressRules is an optional set of ingress rules to add to every node
356+
// +optional
357+
AdditionalNodeIngressRules []IngressRule `json:"additionalNodeIngressRules,omitempty"`
358+
355359
// NodePortIngressRuleCidrBlocks is an optional set of CIDR blocks to allow traffic to nodes' NodePort services.
356360
// If none are specified here, all IPs are allowed to connect.
357361
// +optional

‎api/v1beta2/zz_generated.deepcopy.go‎

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,83 @@ spec:
443443
- toPort
444444
type: object
445445
type: array
446+
additionalNodeIngressRules:
447+
description: AdditionalNodeIngressRules is an optional set of
448+
ingress rules to add to every node
449+
items:
450+
description: IngressRule defines an AWS ingress rule for security
451+
groups.
452+
properties:
453+
cidrBlocks:
454+
description: List of CIDR blocks to allow access from. Cannot
455+
be specified with SourceSecurityGroupID.
456+
items:
457+
type: string
458+
type: array
459+
description:
460+
description: Description provides extended information about
461+
the ingress rule.
462+
type: string
463+
fromPort:
464+
description: FromPort is the start of port range.
465+
format: int64
466+
type: integer
467+
ipv6CidrBlocks:
468+
description: List of IPv6 CIDR blocks to allow access from.
469+
Cannot be specified with SourceSecurityGroupID.
470+
items:
471+
type: string
472+
type: array
473+
natGatewaysIPsSource:
474+
description: NatGatewaysIPsSource use the NAT gateways IPs
475+
as the source for the ingress rule.
476+
type: boolean
477+
protocol:
478+
description: Protocol is the protocol for the ingress rule.
479+
Accepted values are "-1" (all), "4" (IP in IP),"tcp",
480+
"udp", "icmp", and "58" (ICMPv6), "50" (ESP).
481+
enum:
482+
- "-1"
483+
- "4"
484+
- tcp
485+
- udp
486+
- icmp
487+
- "58"
488+
- "50"
489+
type: string
490+
sourceSecurityGroupIds:
491+
description: The security group id to allow access from.
492+
Cannot be specified with CidrBlocks.
493+
items:
494+
type: string
495+
type: array
496+
sourceSecurityGroupRoles:
497+
description: |-
498+
The security group role to allow access from. Cannot be specified with CidrBlocks.
499+
The field will be combined with source security group IDs if specified.
500+
items:
501+
description: SecurityGroupRole defines the unique role
502+
of a security group.
503+
enum:
504+
- bastion
505+
- node
506+
- controlplane
507+
- apiserver-lb
508+
- lb
509+
- node-eks-additional
510+
type: string
511+
type: array
512+
toPort:
513+
description: ToPort is the end of port range.
514+
format: int64
515+
type: integer
516+
required:
517+
- description
518+
- fromPort
519+
- protocol
520+
- toPort
521+
type: object
522+
type: array
446523
cni:
447524
description: CNI configuration
448525
properties:
@@ -2486,6 +2563,83 @@ spec:
24862563
- toPort
24872564
type: object
24882565
type: array
2566+
additionalNodeIngressRules:
2567+
description: AdditionalNodeIngressRules is an optional set of
2568+
ingress rules to add to every node
2569+
items:
2570+
description: IngressRule defines an AWS ingress rule for security
2571+
groups.
2572+
properties:
2573+
cidrBlocks:
2574+
description: List of CIDR blocks to allow access from. Cannot
2575+
be specified with SourceSecurityGroupID.
2576+
items:
2577+
type: string
2578+
type: array
2579+
description:
2580+
description: Description provides extended information about
2581+
the ingress rule.
2582+
type: string
2583+
fromPort:
2584+
description: FromPort is the start of port range.
2585+
format: int64
2586+
type: integer
2587+
ipv6CidrBlocks:
2588+
description: List of IPv6 CIDR blocks to allow access from.
2589+
Cannot be specified with SourceSecurityGroupID.
2590+
items:
2591+
type: string
2592+
type: array
2593+
natGatewaysIPsSource:
2594+
description: NatGatewaysIPsSource use the NAT gateways IPs
2595+
as the source for the ingress rule.
2596+
type: boolean
2597+
protocol:
2598+
description: Protocol is the protocol for the ingress rule.
2599+
Accepted values are "-1" (all), "4" (IP in IP),"tcp",
2600+
"udp", "icmp", and "58" (ICMPv6), "50" (ESP).
2601+
enum:
2602+
- "-1"
2603+
- "4"
2604+
- tcp
2605+
- udp
2606+
- icmp
2607+
- "58"
2608+
- "50"
2609+
type: string
2610+
sourceSecurityGroupIds:
2611+
description: The security group id to allow access from.
2612+
Cannot be specified with CidrBlocks.
2613+
items:
2614+
type: string
2615+
type: array
2616+
sourceSecurityGroupRoles:
2617+
description: |-
2618+
The security group role to allow access from. Cannot be specified with CidrBlocks.
2619+
The field will be combined with source security group IDs if specified.
2620+
items:
2621+
description: SecurityGroupRole defines the unique role
2622+
of a security group.
2623+
enum:
2624+
- bastion
2625+
- node
2626+
- controlplane
2627+
- apiserver-lb
2628+
- lb
2629+
- node-eks-additional
2630+
type: string
2631+
type: array
2632+
toPort:
2633+
description: ToPort is the end of port range.
2634+
format: int64
2635+
type: integer
2636+
required:
2637+
- description
2638+
- fromPort
2639+
- protocol
2640+
- toPort
2641+
type: object
2642+
type: array
24892643
cni:
24902644
description: CNI configuration
24912645
properties:

0 commit comments

Comments
 (0)