Skip to content

Commit e223ac7

Browse files
AndiDogAndiDog
committed
Support additional security group ingress rules for all nodes
1 parent 7b263a4 commit e223ac7

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:
@@ -2465,6 +2542,83 @@ spec:
24652542
- toPort
24662543
type: object
24672544
type: array
2545+
additionalNodeIngressRules:
2546+
description: AdditionalNodeIngressRules is an optional set of
2547+
ingress rules to add to every node
2548+
items:
2549+
description: IngressRule defines an AWS ingress rule for security
2550+
groups.
2551+
properties:
2552+
cidrBlocks:
2553+
description: List of CIDR blocks to allow access from. Cannot
2554+
be specified with SourceSecurityGroupID.
2555+
items:
2556+
type: string
2557+
type: array
2558+
description:
2559+
description: Description provides extended information about
2560+
the ingress rule.
2561+
type: string
2562+
fromPort:
2563+
description: FromPort is the start of port range.
2564+
format: int64
2565+
type: integer
2566+
ipv6CidrBlocks:
2567+
description: List of IPv6 CIDR blocks to allow access from.
2568+
Cannot be specified with SourceSecurityGroupID.
2569+
items:
2570+
type: string
2571+
type: array
2572+
natGatewaysIPsSource:
2573+
description: NatGatewaysIPsSource use the NAT gateways IPs
2574+
as the source for the ingress rule.
2575+
type: boolean
2576+
protocol:
2577+
description: Protocol is the protocol for the ingress rule.
2578+
Accepted values are "-1" (all), "4" (IP in IP),"tcp",
2579+
"udp", "icmp", and "58" (ICMPv6), "50" (ESP).
2580+
enum:
2581+
- "-1"
2582+
- "4"
2583+
- tcp
2584+
- udp
2585+
- icmp
2586+
- "58"
2587+
- "50"
2588+
type: string
2589+
sourceSecurityGroupIds:
2590+
description: The security group id to allow access from.
2591+
Cannot be specified with CidrBlocks.
2592+
items:
2593+
type: string
2594+
type: array
2595+
sourceSecurityGroupRoles:
2596+
description: |-
2597+
The security group role to allow access from. Cannot be specified with CidrBlocks.
2598+
The field will be combined with source security group IDs if specified.
2599+
items:
2600+
description: SecurityGroupRole defines the unique role
2601+
of a security group.
2602+
enum:
2603+
- bastion
2604+
- node
2605+
- controlplane
2606+
- apiserver-lb
2607+
- lb
2608+
- node-eks-additional
2609+
type: string
2610+
type: array
2611+
toPort:
2612+
description: ToPort is the end of port range.
2613+
format: int64
2614+
type: integer
2615+
required:
2616+
- description
2617+
- fromPort
2618+
- protocol
2619+
- toPort
2620+
type: object
2621+
type: array
24682622
cni:
24692623
description: CNI configuration
24702624
properties:

0 commit comments

Comments
 (0)