Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 fix: create ingress rules from all load balancers #4866

Merged
merged 3 commits into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions pkg/cloud/scope/managedcontrolplane.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,11 @@ func (s *ManagedControlPlaneScope) ControlPlaneLoadBalancer() *infrav1.AWSLoadBa
return nil
}

// ControlPlaneLoadBalancers returns the AWSLoadBalancerSpecs.
func (s *ManagedControlPlaneScope) ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec {
return nil
}

// Partition returns the cluster partition.
func (s *ManagedControlPlaneScope) Partition() string {
if s.ControlPlane.Spec.Partition == "" {
Expand Down
5 changes: 5 additions & 0 deletions pkg/cloud/scope/sg.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type SGScope interface {
Bastion() *infrav1.Bastion

// ControlPlaneLoadBalancer returns the load balancer settings that are requested.
// Deprecated: Use ControlPlaneLoadBalancers()
ControlPlaneLoadBalancer() *infrav1.AWSLoadBalancerSpec

// SetNatGatewaysIPs sets the Nat Gateways Public IPs.
Expand All @@ -54,4 +55,8 @@ type SGScope interface {

// AdditionalControlPlaneIngressRules returns the additional ingress rules for the control plane security group.
AdditionalControlPlaneIngressRules() []infrav1.IngressRule

// ControlPlaneLoadBalancers returns both the ControlPlaneLoadBalancer and SecondaryControlPlaneLoadBalancer AWSLoadBalancerSpecs.
// The control plane load balancers should always be returned in the above order.
ControlPlaneLoadBalancers() []*infrav1.AWSLoadBalancerSpec
}
32 changes: 21 additions & 11 deletions pkg/cloud/services/securitygroup/securitygroups.go
Original file line number Diff line number Diff line change
Expand Up @@ -660,10 +660,15 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
rulesToApply := customIngressRules.Difference(kubeletRules)
return append(kubeletRules, rulesToApply...), nil
case infrav1.SecurityGroupLB:
rules := infrav1.IngressRules{}
allowedNLBTraffic := false
// We hand this group off to the in-cluster cloud provider, so these rules aren't used
// Except if the load balancer type is NLB, and we have an AWS Cluster in which case we
// need to open port 6443 to the NLB traffic and health check inside the VPC.
if s.scope.ControlPlaneLoadBalancer() != nil && s.scope.ControlPlaneLoadBalancer().LoadBalancerType == infrav1.LoadBalancerTypeNLB {
for _, lb := range s.scope.ControlPlaneLoadBalancers() {
if lb == nil || lb.LoadBalancerType != infrav1.LoadBalancerTypeNLB {
continue
}
var (
ipv4CidrBlocks []string
ipv6CidrBlocks []string
Expand All @@ -673,25 +678,26 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
if s.scope.VPC().IsIPv6Enabled() {
ipv6CidrBlocks = []string{s.scope.VPC().IPv6.CidrBlock}
}
if s.scope.ControlPlaneLoadBalancer().PreserveClientIP {
if lb.PreserveClientIP {
ipv4CidrBlocks = []string{services.AnyIPv4CidrBlock}
if s.scope.VPC().IsIPv6Enabled() {
ipv6CidrBlocks = []string{services.AnyIPv6CidrBlock}
}
}

rules := infrav1.IngressRules{
{
if !allowedNLBTraffic {
rules = append(rules, infrav1.IngressRule{
Description: "Allow NLB traffic to the control plane instances.",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: int64(s.scope.APIServerPort()),
ToPort: int64(s.scope.APIServerPort()),
CidrBlocks: ipv4CidrBlocks,
IPv6CidrBlocks: ipv6CidrBlocks,
},
})
allowedNLBTraffic = true
}

for _, ln := range s.scope.ControlPlaneLoadBalancer().AdditionalListeners {
for _, ln := range lb.AdditionalListeners {
rules = append(rules, infrav1.IngressRule{
Description: fmt.Sprintf("Allow NLB traffic to the control plane instances on port %d.", ln.Port),
Protocol: infrav1.SecurityGroupProtocolTCP,
Expand All @@ -701,10 +707,8 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
IPv6CidrBlocks: ipv6CidrBlocks,
})
}

return rules, nil
}
return infrav1.IngressRules{}, nil
return rules, nil
}

return nil, errors.Errorf("Cannot determine ingress rules for unknown security group role %q", role)
Expand Down Expand Up @@ -915,8 +919,14 @@ func (s *Service) getIngressRulesToAllowKubeletToAccessTheControlPlaneLB() infra
// getControlPlaneLBIngressRules returns the ingress rules for the control plane LB.
// We allow all traffic when no other rules are defined.
func (s *Service) getControlPlaneLBIngressRules() infrav1.IngressRules {
if s.scope.ControlPlaneLoadBalancer() != nil && len(s.scope.ControlPlaneLoadBalancer().IngressRules) > 0 {
return s.scope.ControlPlaneLoadBalancer().IngressRules
ingressRules := infrav1.IngressRules{}
for _, lb := range s.scope.ControlPlaneLoadBalancers() {
if lb != nil && len(lb.IngressRules) > 0 {
ingressRules = append(ingressRules, lb.IngressRules...)
}
}
if len(ingressRules) > 0 {
return ingressRules
}

// If no custom ingress rules have been defined we allow all traffic so that the MC can access the WC API
Expand Down
58 changes: 58 additions & 0 deletions pkg/cloud/services/securitygroup/securitygroups_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1317,6 +1317,64 @@ func TestControlPlaneLoadBalancerIngressRules(t *testing.T) {
},
},
},
{
name: "defined rules are used when using internal and external LB",
awsCluster: &infrav1.AWSCluster{
Spec: infrav1.AWSClusterSpec{
ControlPlaneLoadBalancer: &infrav1.AWSLoadBalancerSpec{
IngressRules: []infrav1.IngressRule{
{
Description: "My custom ingress rule",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 1234,
ToPort: 1234,
CidrBlocks: []string{"172.126.1.1/0"},
},
},
Scheme: &infrav1.ELBSchemeInternal,
},
SecondaryControlPlaneLoadBalancer: &infrav1.AWSLoadBalancerSpec{
IngressRules: []infrav1.IngressRule{
{
Description: "Another custom ingress rule",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 2345,
ToPort: 2345,
CidrBlocks: []string{"0.0.0.0/0"},
},
},
},
NetworkSpec: infrav1.NetworkSpec{
VPC: infrav1.VPCSpec{
CidrBlock: "10.0.0.0/16",
},
},
},
},
expectedIngresRules: infrav1.IngressRules{
infrav1.IngressRule{
Description: "Kubernetes API",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 6443,
ToPort: 6443,
CidrBlocks: []string{"10.0.0.0/16"},
},
infrav1.IngressRule{
Description: "My custom ingress rule",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 1234,
ToPort: 1234,
CidrBlocks: []string{"172.126.1.1/0"},
},
infrav1.IngressRule{
Description: "Another custom ingress rule",
Protocol: infrav1.SecurityGroupProtocolTCP,
FromPort: 2345,
ToPort: 2345,
CidrBlocks: []string{"0.0.0.0/0"},
},
},
},
}

for _, tc := range testCases {
Expand Down
Loading