Skip to content

Commit

Permalink
Merge pull request #5005 from alexander-demicev/natgatwaysipsingress
Browse files Browse the repository at this point in the history 
✨ Add natgatewayips as source for ingress rules
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Jun 20, 2024
2 parents 888c659 + ce23840 commit 7ab0780
Show file tree
Hide file tree
Showing 9 changed files with 261 additions and 23 deletions.
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.

20 changes: 12 additions & 8 deletions api/v1beta2/awscluster_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,9 +264,7 @@ func (r *AWSCluster) validateNetwork() field.ErrorList {
}

for _, rule := range r.Spec.NetworkSpec.AdditionalControlPlaneIngressRules {
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"))
}
allErrs = append(allErrs, r.validateIngressRule(rule)...)
}

if r.Spec.NetworkSpec.VPC.ElasticIPPool != nil {
Expand Down Expand Up @@ -323,9 +321,7 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
}

for _, rule := range cp.IngressRules {
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"))
}
allErrs = append(allErrs, r.validateIngressRule(rule)...)
}
}

Expand Down Expand Up @@ -367,11 +363,19 @@ func (r *AWSCluster) validateControlPlaneLBs() field.ErrorList {
}
}

for _, rule := range r.Spec.ControlPlaneLoadBalancer.IngressRules {
return allErrs
}

func (r *AWSCluster) validateIngressRule(rule 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"))
}
}

return allErrs
}
69 changes: 69 additions & 0 deletions api/v1beta2/awscluster_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,59 @@ func TestAWSClusterValidateCreate(t *testing.T) {
},
wantErr: true,
},
{
name: "rejects ingress rules with cidr block, source security group id, role and nat gateway IP source",
cluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
IngressRules: []IngressRule{
{
Protocol: SecurityGroupProtocolTCP,
IPv6CidrBlocks: []string{"test"},
SourceSecurityGroupIDs: []string{"test"},
SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion},
NatGatewaysIPsSource: true,
},
},
},
},
},
wantErr: true,
},
{
name: "rejects ingress rules with source security role and nat gateway IP source",
cluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
IngressRules: []IngressRule{
{
Protocol: SecurityGroupProtocolTCP,
SourceSecurityGroupRoles: []SecurityGroupRole{SecurityGroupBastion},
NatGatewaysIPsSource: true,
},
},
},
},
},
wantErr: true,
},
{
name: "rejects ingress rules with cidr block and nat gateway IP source",
cluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
IngressRules: []IngressRule{
{
Protocol: SecurityGroupProtocolTCP,
IPv6CidrBlocks: []string{"test"},
NatGatewaysIPsSource: true,
},
},
},
},
},
wantErr: true,
},
{
name: "accepts ingress rules with cidr block",
cluster: &AWSCluster{
Expand All @@ -424,6 +477,22 @@ func TestAWSClusterValidateCreate(t *testing.T) {
},
wantErr: false,
},
{
name: "accepts ingress rules with nat gateway IPs source",
cluster: &AWSCluster{
Spec: AWSClusterSpec{
ControlPlaneLoadBalancer: &AWSLoadBalancerSpec{
IngressRules: []IngressRule{
{
Protocol: SecurityGroupProtocolTCP,
NatGatewaysIPsSource: true,
},
},
},
},
},
wantErr: false,
},
{
name: "accepts ingress rules with source security group role",
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 @@ -930,6 +930,10 @@ type IngressRule struct {
// The field will be combined with source security group IDs if specified.
// +optional
SourceSecurityGroupRoles []SecurityGroupRole `json:"sourceSecurityGroupRoles,omitempty"`

// NatGatewaysIPsSource use the NAT gateways IPs as the source for the ingress rule.
// +optional
NatGatewaysIPsSource bool `json:"natGatewaysIPsSource,omitempty"`
}

// String returns a string representation of the ingress rule.
Expand Down
16 changes: 16 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 @@ -393,6 +393,10 @@ spec:
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",
Expand Down Expand Up @@ -1920,6 +1924,10 @@ spec:
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
Expand Down Expand Up @@ -2376,6 +2384,10 @@ spec:
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",
Expand Down Expand Up @@ -3916,6 +3928,10 @@ spec:
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
Expand Down
16 changes: 16 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1164,6 +1164,10 @@ spec:
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",
Expand Down Expand Up @@ -1329,6 +1333,10 @@ spec:
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",
Expand Down Expand Up @@ -1943,6 +1951,10 @@ spec:
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",
Expand Down Expand Up @@ -2866,6 +2878,10 @@ spec:
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
Expand Down
12 changes: 12 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_awsclustertemplates.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -756,6 +756,10 @@ spec:
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
Expand Down Expand Up @@ -925,6 +929,10 @@ spec:
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
Expand Down Expand Up @@ -1544,6 +1552,10 @@ spec:
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
Expand Down
31 changes: 27 additions & 4 deletions pkg/cloud/services/securitygroup/securitygroups.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,12 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
rules = append(rules, s.defaultSSHIngressRule(s.scope.SecurityGroups()[infrav1.SecurityGroupBastion].ID))
}

rules = append(rules, s.processIngressRulesSGs(s.scope.AdditionalControlPlaneIngressRules())...)
additionalIngressRules, err := s.processIngressRulesSGs(s.scope.AdditionalControlPlaneIngressRules())
if err != nil {
return nil, err
}

rules = append(rules, additionalIngressRules...)

return append(cniRules, rules...), nil

Expand Down Expand Up @@ -639,7 +644,10 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
return infrav1.IngressRules{}, nil
case infrav1.SecurityGroupAPIServerLB:
kubeletRules := s.getIngressRulesToAllowKubeletToAccessTheControlPlaneLB()
customIngressRules := s.processIngressRulesSGs(s.getControlPlaneLBIngressRules())
customIngressRules, err := s.processIngressRulesSGs(s.getControlPlaneLBIngressRules())
if err != nil {
return nil, err
}
rulesToApply := customIngressRules.Difference(kubeletRules)
return append(kubeletRules, rulesToApply...), nil
case infrav1.SecurityGroupLB:
Expand Down Expand Up @@ -964,10 +972,25 @@ func (s *Service) getIngressRuleToAllowVPCCidrInTheAPIServer() infrav1.IngressRu
}
}

func (s *Service) processIngressRulesSGs(ingressRules []infrav1.IngressRule) infrav1.IngressRules {
func (s *Service) processIngressRulesSGs(ingressRules []infrav1.IngressRule) (infrav1.IngressRules, error) {
output := []infrav1.IngressRule{}

for _, rule := range ingressRules {
if rule.NatGatewaysIPsSource { // if the rule has NatGatewaysIPsSource set to true, use the NAT Gateway IPs as the source
natGatewaysCidrs := []string{}
natGatewaysIPs := s.scope.GetNatGatewaysIPs()
for _, ip := range natGatewaysIPs {
natGatewaysCidrs = append(natGatewaysCidrs, fmt.Sprintf("%s/32", ip))
}
if len(natGatewaysIPs) > 0 {
rule.CidrBlocks = natGatewaysCidrs
output = append(output, rule)
continue
}

return nil, errors.New("NAT Gateway IPs are not available yet")
}

if len(rule.CidrBlocks) != 0 || len(rule.IPv6CidrBlocks) != 0 { // don't set source security group if cidr blocks are set
output = append(output, rule)
continue
Expand All @@ -988,5 +1011,5 @@ func (s *Service) processIngressRulesSGs(ingressRules []infrav1.IngressRule) inf
output = append(output, rule)
}

return output
return output, nil
}

0 comments on commit 7ab0780

Please sign in to comment.