Repository: kubernetes-sigs/cluster-api-provider-aws
Pull request #4520 (2023-09-26 15:45:11)

[release-2.2] Save Nat Gateways IPs to status on every reconciliation
This is an automated cherry-pick of #4508

/assign richardcase

```release-note
Fix how NAT gateways IPs are saved in the status field
```


Patch:
diff --git a/pkg/cloud/services/network/natgateways.go b/pkg/cloud/services/network/natgateways.go
index 2134e4d5e..194816cb7 100644
--- a/pkg/cloud/services/network/natgateways.go
+++ b/pkg/cloud/services/network/natgateways.go
@@ -69,6 +69,7 @@ func (s *Service) reconcileNatGateways() error {
 		return err
 	}
 
+	natGatewaysIPs := []string{}
 	subnetIDs := []string{}
 
 	for _, sn := range s.scope.Subnets().FilterPublic() {
@@ -77,6 +78,9 @@ func (s *Service) reconcileNatGateways() error {
 		}
 
 		if ngw, ok := existing[sn.ID]; ok {
+			if len(ngw.NatGatewayAddresses) > 0 && ngw.NatGatewayAddresses[0].PublicIp != nil {
+				natGatewaysIPs = append(natGatewaysIPs, *ngw.NatGatewayAddresses[0].PublicIp)
+			}
 			// Make sure tags are up to date.
 			if err := wait.WaitForWithRetryable(wait.NewBackoff(), func() (bool, error) {
 				buildParams := s.getNatGatewayTagParams(*ngw.NatGatewayId)
@@ -96,6 +100,8 @@ func (s *Service) reconcileNatGateways() error {
 		subnetIDs = append(subnetIDs, sn.ID)
 	}
 
+	s.scope.SetNatGatewaysIPs(natGatewaysIPs)
+
 	// Batch the creation of NAT gateways
 	if len(subnetIDs) > 0 {
 		// set NatGatewayCreationStarted if the condition has never been set before
@@ -106,18 +112,12 @@ 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
index 8fd9a897b..19e1ac2b0 100644
--- a/pkg/cloud/services/securitygroup/securitygroups.go
+++ b/pkg/cloud/services/securitygroup/securitygroups.go
@@ -721,68 +721,63 @@ func ingressRuleToSDKType(scope scope.SGScope, i *infrav1.IngressRule) (res *ec2
 }
 
 func ingressRulesFromSDKType(v *ec2.IpPermission) (res infrav1.IngressRules) {
+	for _, ec2range := range v.IpRanges {
+		rule := ingressRuleFromSDKProtocol(v)
+		if ec2range.Description != nil && *ec2range.Description != "" {
+			rule.Description = *ec2range.Description
+		}
+
+		rule.CidrBlocks = []string{*ec2range.CidrIp}
+		res = append(res, rule)
+	}
+
+	for _, ec2range := range v.Ipv6Ranges {
+		rule := ingressRuleFromSDKProtocol(v)
+		if ec2range.Description != nil && *ec2range.Description != "" {
+			rule.Description = *ec2range.Description
+		}
+
+		rule.IPv6CidrBlocks = []string{*ec2range.CidrIpv6}
+		res = append(res, rule)
+	}
+
+	for _, pair := range v.UserIdGroupPairs {
+		rule := ingressRuleFromSDKProtocol(v)
+		if pair.GroupId == nil {
+			continue
+		}
+
+		if pair.Description != nil && *pair.Description != "" {
+			rule.Description = *pair.Description
+		}
+
+		rule.SourceSecurityGroupIDs = []string{*pair.GroupId}
+		res = append(res, rule)
+	}
+
+	return res
+}
+
+func ingressRuleFromSDKProtocol(v *ec2.IpPermission) infrav1.IngressRule {
 	// Ports are only well-defined for TCP and UDP protocols, but EC2 overloads the port range
 	// in the case of ICMP(v6) traffic to indicate which codes are allowed. For all other protocols,
 	// including the custom "-1" All Traffic protocol, FromPort and ToPort are omitted from the response.
 	// See: https://docs.aws.amazon.com/AWSEC2/latest/APIReference/API_IpPermission.html
-	var ir infrav1.IngressRule
 	switch *v.IpProtocol {
 	case IPProtocolTCP,
 		IPProtocolUDP,
 		IPProtocolICMP,
 		IPProtocolICMPv6:
-		ir = infrav1.IngressRule{
+		return infrav1.IngressRule{
 			Protocol: infrav1.SecurityGroupProtocol(*v.IpProtocol),
 			FromPort: *v.FromPort,
 			ToPort:   *v.ToPort,
 		}
 	default:
-		ir = infrav1.IngressRule{
+		return infrav1.IngressRule{
 			Protocol: infrav1.SecurityGroupProtocol(*v.IpProtocol),
 		}
 	}
-
-	if len(v.IpRanges) > 0 {
-		r1 := ir
-		for _, ec2range := range v.IpRanges {
-			if ec2range.Description != nil && *ec2range.Description != "" {
-				r1.Description = *ec2range.Description
-			}
-
-			r1.CidrBlocks = append(r1.CidrBlocks, *ec2range.CidrIp)
-		}
-		res = append(res, r1)
-	}
-
-	if len(v.Ipv6Ranges) > 0 {
-		r1 := ir
-		for _, ec2range := range v.Ipv6Ranges {
-			if ec2range.Description != nil && *ec2range.Description != "" {
-				r1.Description = *ec2range.Description
-			}
-
-			r1.IPv6CidrBlocks = append(r1.IPv6CidrBlocks, *ec2range.CidrIpv6)
-		}
-		res = append(res, r1)
-	}
-
-	if len(v.UserIdGroupPairs) > 0 {
-		r2 := ir
-		for _, pair := range v.UserIdGroupPairs {
-			if pair.GroupId == nil {
-				continue
-			}
-
-			if pair.Description != nil && *pair.Description != "" {
-				r2.Description = *pair.Description
-			}
-
-			r2.SourceSecurityGroupIDs = append(r2.SourceSecurityGroupIDs, *pair.GroupId)
-		}
-		res = append(res, r2)
-	}
-
-	return res
 }
 
 // getIngressRulesToAllowKubeletToAccessTheControlPlaneLB returns ingress rules required in the control plane LB.
@@ -793,7 +788,11 @@ func (s *Service) getIngressRulesToAllowKubeletToAccessTheControlPlaneLB() infra
 		return s.getIngressRuleToAllowVPCCidrInTheAPIServer()
 	}
 
+	natGatewaysCidrs := []string{}
 	natGatewaysIPs := s.scope.GetNatGatewaysIPs()
+	for _, ip := range natGatewaysIPs {
+		natGatewaysCidrs = append(natGatewaysCidrs, fmt.Sprintf("%s/32", ip))
+	}
 	if len(natGatewaysIPs) > 0 {
 		return infrav1.IngressRules{
 			{
@@ -801,7 +800,7 @@ func (s *Service) getIngressRulesToAllowKubeletToAccessTheControlPlaneLB() infra
 				Protocol:    infrav1.SecurityGroupProtocolTCP,
 				FromPort:    int64(s.scope.APIServerPort()),
 				ToPort:      int64(s.scope.APIServerPort()),
-				CidrBlocks:  natGatewaysIPs,
+				CidrBlocks:  natGatewaysCidrs,
 			},
 		}
 	}


Test patch:
diff --git a/pkg/cloud/services/securitygroup/securitygroups_test.go b/pkg/cloud/services/securitygroup/securitygroups_test.go
index 4135cc5b0..efb34b3de 100644
--- a/pkg/cloud/services/securitygroup/securitygroups_test.go
+++ b/pkg/cloud/services/securitygroup/securitygroups_test.go
@@ -852,7 +852,7 @@ func TestControlPlaneLoadBalancerIngressRules(t *testing.T) {
 					Protocol:    infrav1.SecurityGroupProtocolTCP,
 					FromPort:    6443,
 					ToPort:      6443,
-					CidrBlocks:  []string{"1.2.3.4"},
+					CidrBlocks:  []string{"1.2.3.4/32"},
 				},
 				infrav1.IngressRule{
 					Description: "Kubernetes API",
@@ -896,7 +896,7 @@ func TestControlPlaneLoadBalancerIngressRules(t *testing.T) {
 					Protocol:    infrav1.SecurityGroupProtocolTCP,
 					FromPort:    6443,
 					ToPort:      6443,
-					CidrBlocks:  []string{"1.2.3.4"},
+					CidrBlocks:  []string{"1.2.3.4/32"},
 				},
 				infrav1.IngressRule{
 					Description: "My custom ingress rule",
@@ -1231,6 +1231,40 @@ func TestIngressRulesFromSDKType(t *testing.T) {
 		input    *ec2.IpPermission
 		expected infrav1.IngressRules
 	}{
+		{
+			name: "two ingress rules",
+			input: &ec2.IpPermission{
+				IpProtocol: aws.String("tcp"),
+				FromPort:   aws.Int64(6443),
+				ToPort:     aws.Int64(6443),
+				IpRanges: []*ec2.IpRange{
+					{
+						CidrIp:      aws.String("0.0.0.0/0"),
+						Description: aws.String("Kubernetes API"),
+					},
+					{
+						CidrIp:      aws.String("192.168.1.1/32"),
+						Description: aws.String("My VPN"),
+					},
+				},
+			},
+			expected: infrav1.IngressRules{
+				{
+					Description: "Kubernetes API",
+					Protocol:    "tcp",
+					FromPort:    6443,
+					ToPort:      6443,
+					CidrBlocks:  []string{"0.0.0.0/0"},
+				},
+				{
+					Description: "My VPN",
+					Protocol:    "tcp",
+					FromPort:    6443,
+					ToPort:      6443,
+					CidrBlocks:  []string{"192.168.1.1/32"},
+				},
+			},
+		},
 		{
 			name: "Two group pairs",
 			input: &ec2.IpPermission{
@@ -1256,7 +1290,14 @@ func TestIngressRulesFromSDKType(t *testing.T) {
 					Protocol:               "tcp",
 					FromPort:               10250,
 					ToPort:                 10250,
-					SourceSecurityGroupIDs: []string{"sg-source-1", "sg-source-2"},
+					SourceSecurityGroupIDs: []string{"sg-source-1"},
+				},
+				{
+					Description:            "Kubelet API",
+					Protocol:               "tcp",
+					FromPort:               10250,
+					ToPort:                 10250,
+					SourceSecurityGroupIDs: []string{"sg-source-2"},
 				},
 			},
 		},