Skip to content

Commit

Permalink
Support additional security group ingress rules for all nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
@AndiDog
AndiDog committed Jan 22, 2025
1 parent 7b263a4 commit e223ac7
Show file tree
Hide file tree
Showing 13 changed files with 444 additions and 22 deletions.
1 change: 1 addition & 0 deletions api/v1beta1/awscluster_conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions api/v1beta1/zz_generated.conversion.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 17 additions & 22 deletions api/v1beta2/awscluster_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
85 changes: 85 additions & 0 deletions api/v1beta2/awscluster_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
4 changes: 4 additions & 0 deletions api/v1beta2/network_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

154 changes: 154 additions & 0 deletions config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
Loading

0 comments on commit e223ac7

Please sign in to comment.