Merge pull request #4406 from giantswarm/save-nat-ips-status

k8s-ci-robot · web-flow · commit eb15f694fe4d · 2023-09-03T23:13:48.000-07:00
Allow securing api LB, only allowing traffic from required sources
diff --git a/api/v1beta1/awscluster_conversion.go b/api/v1beta1/awscluster_conversion.go
@@ -53,6 +53,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {
 	for role, sg := range restored.Status.Network.SecurityGroups {
 		dst.Status.Network.SecurityGroups[role] = sg
 	}
+	dst.Status.Network.NatGatewaysIPs = restored.Status.Network.NatGatewaysIPs
 
 	return nil
 }
diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go
diff --git a/api/v1beta2/network_types.go b/api/v1beta2/network_types.go
@@ -36,6 +36,9 @@ type NetworkStatus struct {
 
 	// APIServerELB is the Kubernetes api server load balancer.
 	APIServerELB LoadBalancer `json:"apiServerElb,omitempty"`
+
+	// NatGatewaysIPs contains the public IPs of the NAT Gateways
+	NatGatewaysIPs []string `json:"natGatewaysIPs,omitempty"`
 }
 
 // ELBScheme defines the scheme of a load balancer.
@@ -55,6 +58,15 @@ func (e ELBScheme) String() string {
 	return string(e)
 }
 
+// Equals returns true if two ELBScheme are equal.
+func (e ELBScheme) Equals(other *ELBScheme) bool {
+	if other == nil {
+		return false
+	}
+
+	return e == *other
+}
+
 // ELBProtocol defines listener protocols for a load balancer.
 type ELBProtocol string
 
diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go
diff --git a/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml b/config/crd/bases/controlplane.cluster.x-k8s.io_awsmanagedcontrolplanes.yaml
@@ -1354,6 +1354,12 @@ spec:
                           balancer.
                         type: object
                     type: object
+                  natGatewaysIPs:
+                    description: NatGatewaysIPs contains the public IPs of the NAT
+                      Gateways
+                    items:
+                      type: string
+                    type: array
                   securityGroups:
                     additionalProperties:
                       description: SecurityGroup defines an AWS security group.
@@ -2815,6 +2821,12 @@ spec:
                           balancer.
                         type: object
                     type: object
+                  natGatewaysIPs:
+                    description: NatGatewaysIPs contains the public IPs of the NAT
+                      Gateways
+                    items:
+                      type: string
+                    type: array
                   securityGroups:
                     additionalProperties:
                       description: SecurityGroup defines an AWS security group.
diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_awsclusters.yaml
@@ -1887,6 +1887,12 @@ spec:
                           balancer.
                         type: object
                     type: object
+                  natGatewaysIPs:
+                    description: NatGatewaysIPs contains the public IPs of the NAT
+                      Gateways
+                    items:
+                      type: string
+                    type: array
                   securityGroups:
                     additionalProperties:
                       description: SecurityGroup defines an AWS security group.
diff --git a/pkg/cloud/interfaces.go b/pkg/cloud/interfaces.go
@@ -79,7 +79,6 @@ type ClusterScoper interface {
 	AdditionalTags() infrav1.Tags
 	// SetFailureDomain sets the infrastructure provider failure domain key to the spec given as input.
 	SetFailureDomain(id string, spec clusterv1.FailureDomainSpec)
-
 	// PatchObject persists the cluster configuration and status.
 	PatchObject() error
 	// Close closes the current scope persisting the cluster configuration and status.
diff --git a/pkg/cloud/scope/cluster.go b/pkg/cloud/scope/cluster.go
@@ -299,6 +299,16 @@ func (s *ClusterScope) SetFailureDomain(id string, spec clusterv1.FailureDomainS
 	s.AWSCluster.Status.FailureDomains[id] = spec
 }
 
+// SetNatGatewaysIPs sets the Nat Gateways Public IPs.
+func (s *ClusterScope) SetNatGatewaysIPs(ips []string) {
+	s.AWSCluster.Status.Network.NatGatewaysIPs = ips
+}
+
+// GetNatGatewaysIPs gets the Nat Gateways Public IPs.
+func (s *ClusterScope) GetNatGatewaysIPs() []string {
+	return s.AWSCluster.Status.Network.NatGatewaysIPs
+}
+
 // InfraCluster returns the AWS infrastructure cluster or control plane object.
 func (s *ClusterScope) InfraCluster() cloud.ClusterObject {
 	return s.AWSCluster
diff --git a/pkg/cloud/scope/managedcontrolplane.go b/pkg/cloud/scope/managedcontrolplane.go
@@ -169,6 +169,16 @@ func (s *ManagedControlPlaneScope) Subnets() infrav1.Subnets {
 	return s.ControlPlane.Spec.NetworkSpec.Subnets
 }
 
+// SetNatGatewaysIPs sets the Nat Gateways Public IPs.
+func (s *ManagedControlPlaneScope) SetNatGatewaysIPs(ips []string) {
+	s.ControlPlane.Status.Network.NatGatewaysIPs = ips
+}
+
+// GetNatGatewaysIPs gets the Nat Gateways Public IPs.
+func (s *ManagedControlPlaneScope) GetNatGatewaysIPs() []string {
+	return s.ControlPlane.Status.Network.NatGatewaysIPs
+}
+
 // IdentityRef returns the cluster identityRef.
 func (s *ManagedControlPlaneScope) IdentityRef() *infrav1.AWSIdentityReference {
 	return s.ControlPlane.Spec.IdentityRef
diff --git a/pkg/cloud/scope/network.go b/pkg/cloud/scope/network.go
@@ -45,4 +45,9 @@ type NetworkScope interface {
 
 	// TagUnmanagedNetworkResources returns is tagging unmanaged network resources is set.
 	TagUnmanagedNetworkResources() bool
+
+	// SetNatGatewaysIPs sets the Nat Gateways Public IPs.
+	SetNatGatewaysIPs(ips []string)
+	// GetNatGatewaysIPs gets the Nat Gateways Public IPs.
+	GetNatGatewaysIPs() []string
 }
diff --git a/pkg/cloud/scope/sg.go b/pkg/cloud/scope/sg.go
@@ -45,4 +45,10 @@ type SGScope interface {
 
 	// ControlPlaneLoadBalancer returns the load balancer settings that are requested.
 	ControlPlaneLoadBalancer() *infrav1.AWSLoadBalancerSpec
+
+	// SetNatGatewaysIPs sets the Nat Gateways Public IPs.
+	SetNatGatewaysIPs(ips []string)
+
+	// GetNatGatewaysIPs gets the Nat Gateways Public IPs.
+	GetNatGatewaysIPs() []string
 }
diff --git a/pkg/cloud/services/network/natgateways.go b/pkg/cloud/services/network/natgateways.go
@@ -106,12 +106,18 @@ func (s *Service) reconcileNatGateways() error {
 			}
 		}
 		ngws, err := s.createNatGateways(subnetIDs)
+		var natGatewaysIPs []string
 
 		for _, ng := range ngws {
 			subnet := s.scope.Subnets().FindByID(*ng.SubnetId)
 			subnet.NatGatewayID = ng.NatGatewayId
+			if len(ng.NatGatewayAddresses) > 0 && ng.NatGatewayAddresses[0].PublicIp != nil {
+				natGatewaysIPs = append(natGatewaysIPs, *ng.NatGatewayAddresses[0].PublicIp)
+			}
 		}
 
+		s.scope.SetNatGatewaysIPs(natGatewaysIPs)
+
 		if err != nil {
 			return err
 		}
diff --git a/pkg/cloud/services/securitygroup/securitygroups.go b/pkg/cloud/services/securitygroup/securitygroups.go
@@ -577,11 +577,10 @@ func (s *Service) getSecurityGroupIngressRules(role infrav1.SecurityGroupRole) (
 		}
 		return infrav1.IngressRules{}, nil
 	case infrav1.SecurityGroupAPIServerLB:
-		rules := s.getDefaultIngressRulesForControlPlaneLB()
-		if s.scope.ControlPlaneLoadBalancer() != nil && len(s.scope.ControlPlaneLoadBalancer().IngressRules) > 0 {
-			rules = s.scope.ControlPlaneLoadBalancer().IngressRules
-		}
-		return rules, nil
+		kubeletRules := s.getIngressRulesToAllowKubeletToAccessTheControlPlaneLB()
+		customIngressRules := s.getControlPlaneLBIngressRules()
+		rulesToApply := customIngressRules.Difference(kubeletRules)
+		return append(kubeletRules, rulesToApply...), nil
 	case infrav1.SecurityGroupLB:
 		// 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
@@ -786,7 +785,43 @@ func ingressRulesFromSDKType(v *ec2.IpPermission) (res infrav1.IngressRules) {
 	return res
 }
 
-func (s *Service) getDefaultIngressRulesForControlPlaneLB() infrav1.IngressRules {
+// getIngressRulesToAllowKubeletToAccessTheControlPlaneLB returns ingress rules required in the control plane LB.
+// The control plane LB will be accessed by in-cluster components like the kubelet, that means allowing the NatGateway IPs
+// when using an internet-facing LB, or the VPC CIDR when using an internal LB.
+func (s *Service) getIngressRulesToAllowKubeletToAccessTheControlPlaneLB() infrav1.IngressRules {
+	if s.scope.ControlPlaneLoadBalancer() != nil && infrav1.ELBSchemeInternal.Equals(s.scope.ControlPlaneLoadBalancer().Scheme) {
+		return s.getIngressRuleToAllowVPCCidrInTheAPIServer()
+	}
+
+	natGatewaysIPs := s.scope.GetNatGatewaysIPs()
+	if len(natGatewaysIPs) > 0 {
+		return infrav1.IngressRules{
+			{
+				Description: "Kubernetes API",
+				Protocol:    infrav1.SecurityGroupProtocolTCP,
+				FromPort:    int64(s.scope.APIServerPort()),
+				ToPort:      int64(s.scope.APIServerPort()),
+				CidrBlocks:  natGatewaysIPs,
+			},
+		}
+	}
+
+	// If Nat Gateway IPs are not available yet, we allow all traffic for now so that the MC can access the WC API
+	return s.getIngressRuleToAllowAnyIPInTheAPIServer()
+}
+
+// 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
+	}
+
+	// If no custom ingress rules have been defined we allow all traffic so that the MC can access the WC API
+	return s.getIngressRuleToAllowAnyIPInTheAPIServer()
+}
+
+func (s *Service) getIngressRuleToAllowAnyIPInTheAPIServer() infrav1.IngressRules {
 	if s.scope.VPC().IsIPv6Enabled() {
 		return infrav1.IngressRules{
 			{
@@ -809,3 +844,27 @@ func (s *Service) getDefaultIngressRulesForControlPlaneLB() infrav1.IngressRules
 		},
 	}
 }
+
+func (s *Service) getIngressRuleToAllowVPCCidrInTheAPIServer() infrav1.IngressRules {
+	if s.scope.VPC().IsIPv6Enabled() {
+		return infrav1.IngressRules{
+			{
+				Description:    "Kubernetes API IPv6",
+				Protocol:       infrav1.SecurityGroupProtocolTCP,
+				FromPort:       int64(s.scope.APIServerPort()),
+				ToPort:         int64(s.scope.APIServerPort()),
+				IPv6CidrBlocks: []string{s.scope.VPC().IPv6.CidrBlock},
+			},
+		}
+	}
+
+	return infrav1.IngressRules{
+		{
+			Description: "Kubernetes API",
+			Protocol:    infrav1.SecurityGroupProtocolTCP,
+			FromPort:    int64(s.scope.APIServerPort()),
+			ToPort:      int64(s.scope.APIServerPort()),
+			CidrBlocks:  []string{s.scope.VPC().CidrBlock},
+		},
+	}
+}
diff --git a/pkg/cloud/services/securitygroup/securitygroups_test.go b/pkg/cloud/services/securitygroup/securitygroups_test.go

Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,7 @@ func (src *AWSCluster) ConvertTo(dstRaw conversion.Hub) error {`
`53`	`53`	`for role, sg := range restored.Status.Network.SecurityGroups {`
`54`	`54`	`dst.Status.Network.SecurityGroups[role] = sg`
`55`	`55`	`}`
	`56`	`+ dst.Status.Network.NatGatewaysIPs = restored.Status.Network.NatGatewaysIPs`
`56`	`57`
`57`	`58`	`return nil`
`58`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,4 +45,9 @@ type NetworkScope interface {`
`45`	`45`
`46`	`46`	`// TagUnmanagedNetworkResources returns is tagging unmanaged network resources is set.`
`47`	`47`	`TagUnmanagedNetworkResources() bool`
	`48`	`+`
	`49`	`+ // SetNatGatewaysIPs sets the Nat Gateways Public IPs.`
	`50`	`+ SetNatGatewaysIPs(ips []string)`
	`51`	`+ // GetNatGatewaysIPs gets the Nat Gateways Public IPs.`
	`52`	`+ GetNatGatewaysIPs() []string`
`48`	`53`	`}`