Skip to content

Commit fd82e3a

Browse files
AndiDogtamalsaha
AndiDog
authored and
tamalsaha
committed
Support additional security group ingress rules for all nodes
1 parent e9f2823 commit fd82e3a

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
@@ -87,6 +87,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {
8787
}
8888

8989
dst.Spec.NetworkSpec.AdditionalControlPlaneIngressRules = restored.Spec.NetworkSpec.AdditionalControlPlaneIngressRules
90+
dst.Spec.NetworkSpec.AdditionalNodeIngressRules = restored.Spec.NetworkSpec.AdditionalNodeIngressRules
9091
dst.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks = restored.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks
9192

9293
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
@@ -296,9 +296,8 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
296296
allErrs = append(allErrs, field.Invalid(field.NewPath("ipamPool"), r.Spec.NetworkSpec.VPC.IPAMPool, "ipamPool must have either id or name"))
297297
}
298298

299-
for _, rule := range r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules {
300-
allErrs = append(allErrs, r.validateIngressRule(rule)...)
301-
}
299+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "network", "additionalControlPlaneIngressRules"), r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules)...)
300+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "network", "additionalNodeIngressRules"), r.Spec.NetworkSpec.AdditionalNodeIngressRules)...)
302301

303302
for cidrBlockIndex, cidrBlock := range r.Spec.NetworkSpec.NodePortIngressRuleCidrBlocks {
304303
if _, _, err := net.ParseCIDR(cidrBlock); err != nil {
@@ -374,18 +373,11 @@ func (r *AWSCluster) validateControlPlaneLBs() (admission.Warnings, field.ErrorL
374373

375374
// Additional listeners are only supported for NLBs.
376375
// Validate the control plane load balancers.
377-
loadBalancers := []*AWSLoadBalancerSpec{
378-
r.Spec.ControlPlaneLoadBalancer,
379-
r.Spec.SecondaryControlPlaneLoadBalancer,
376+
if r.Spec.ControlPlaneLoadBalancer != nil {
377+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "controlPlaneLoadBalancer", "ingressRules"), r.Spec.ControlPlaneLoadBalancer.IngressRules)...)
380378
}
381-
for _, cp := range loadBalancers {
382-
if cp == nil {
383-
continue
384-
}
385-
386-
for _, rule := range cp.IngressRules {
387-
allErrs = append(allErrs, r.validateIngressRule(rule)...)
388-
}
379+
if r.Spec.SecondaryControlPlaneLoadBalancer != nil {
380+
allErrs = append(allErrs, r.validateIngressRules(field.NewPath("spec", "secondaryControlPlaneLoadBalancer", "ingressRules"), r.Spec.SecondaryControlPlaneLoadBalancer.IngressRules)...)
389381
}
390382

391383
if r.Spec.ControlPlaneLoadBalancer.LoadBalancerType == LoadBalancerTypeDisabled {
@@ -429,15 +421,18 @@ func (r *AWSCluster) validateControlPlaneLBs() (admission.Warnings, field.ErrorL
429421
return allWarnings, allErrs
430422
}
431423

432-
func (r *AWSCluster) validateIngressRule(rule IngressRule) field.ErrorList {
424+
func (r *AWSCluster) validateIngressRules(path *field.Path, rules []IngressRule) field.ErrorList {
433425
var allErrs field.ErrorList
434-
if rule.NatGatewaysIPsSource {
435-
if rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil || rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil {
436-
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"))
437-
}
438-
} else {
439-
if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) {
440-
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"))
426+
for ruleIndex, rule := range rules {
427+
rulePath := path.Index(ruleIndex)
428+
if rule.NatGatewaysIPsSource {
429+
if rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil || rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil {
430+
allErrs = append(allErrs, field.Invalid(rulePath, rules, "natGatewaysIPsSource cannot be used together with CIDR blocks, security group IDs or security group roles"))
431+
}
432+
} else {
433+
if (rule.CidrBlocks != nil || rule.IPv6CidrBlocks != nil) && (rule.SourceSecurityGroupIDs != nil || rule.SourceSecurityGroupRoles != nil) {
434+
allErrs = append(allErrs, field.Invalid(rulePath, rules, "CIDR blocks and security group IDs or security group roles cannot be used together"))
435+
}
441436
}
442437
}
443438
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:
@@ -2489,6 +2566,83 @@ spec:
24892566
- toPort
24902567
type: object
24912568
type: array
2569+
additionalNodeIngressRules:
2570+
description: AdditionalNodeIngressRules is an optional set of
2571+
ingress rules to add to every node
2572+
items:
2573+
description: IngressRule defines an AWS ingress rule for security
2574+
groups.
2575+
properties:
2576+
cidrBlocks:
2577+
description: List of CIDR blocks to allow access from. Cannot
2578+
be specified with SourceSecurityGroupID.
2579+
items:
2580+
type: string
2581+
type: array
2582+
description:
2583+
description: Description provides extended information about
2584+
the ingress rule.
2585+
type: string
2586+
fromPort:
2587+
description: FromPort is the start of port range.
2588+
format: int64
2589+
type: integer
2590+
ipv6CidrBlocks:
2591+
description: List of IPv6 CIDR blocks to allow access from.
2592+
Cannot be specified with SourceSecurityGroupID.
2593+
items:
2594+
type: string
2595+
type: array
2596+
natGatewaysIPsSource:
2597+
description: NatGatewaysIPsSource use the NAT gateways IPs
2598+
as the source for the ingress rule.
2599+
type: boolean
2600+
protocol:
2601+
description: Protocol is the protocol for the ingress rule.
2602+
Accepted values are "-1" (all), "4" (IP in IP),"tcp",
2603+
"udp", "icmp", and "58" (ICMPv6), "50" (ESP).
2604+
enum:
2605+
- "-1"
2606+
- "4"
2607+
- tcp
2608+
- udp
2609+
- icmp
2610+
- "58"
2611+
- "50"
2612+
type: string
2613+
sourceSecurityGroupIds:
2614+
description: The security group id to allow access from.
2615+
Cannot be specified with CidrBlocks.
2616+
items:
2617+
type: string
2618+
type: array
2619+
sourceSecurityGroupRoles:
2620+
description: |-
2621+
The security group role to allow access from. Cannot be specified with CidrBlocks.
2622+
The field will be combined with source security group IDs if specified.
2623+
items:
2624+
description: SecurityGroupRole defines the unique role
2625+
of a security group.
2626+
enum:
2627+
- bastion
2628+
- node
2629+
- controlplane
2630+
- apiserver-lb
2631+
- lb
2632+
- node-eks-additional
2633+
type: string
2634+
type: array
2635+
toPort:
2636+
description: ToPort is the end of port range.
2637+
format: int64
2638+
type: integer
2639+
required:
2640+
- description
2641+
- fromPort
2642+
- protocol
2643+
- toPort
2644+
type: object
2645+
type: array
24922646
cni:
24932647
description: CNI configuration
24942648
properties:

0 commit comments

Comments
 (0)