Address review feedback - 1

kubernetes · May 18, 2021 · 64acdca · 64acdca
1 parent a784363
commit 64acdca
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 7 deletions.
diff --git a/pkg/apis/kops/validation/validation.go b/pkg/apis/kops/validation/validation.go
@@ -313,7 +313,7 @@ func validateCIDR(cidr string, fieldPath *field.Path) field.ErrorList {
 		if !strings.Contains(cidr, "/") {
 			ip := net.ParseIP(cidr)
 			if ip != nil {
-				if ip.To4() != nil {
+				if ip.To4() != nil && !strings.Contains(cidr, ":") {
 					detail += fmt.Sprintf(" (did you mean \"%s/32\")", cidr)
 				} else {
 					detail += fmt.Sprintf(" (did you mean \"%s/64\")", cidr)

diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go
@@ -345,7 +345,17 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 			}
 
 			// Allow ICMP traffic required for PMTU discovery
-			if utils.IsIPv4CIDR(cidr) {
+			if utils.IsIPv6CIDR(cidr) {
+				c.AddTask(&awstasks.SecurityGroupRule{
+					Name:          fi.String("icmpv6-pmtu-api-elb-" + cidr),
+					Lifecycle:     b.SecurityLifecycle,
+					IPv6CIDR:      fi.String(cidr),
+					FromPort:      fi.Int64(2),
+					Protocol:      fi.String("icmpv6"),
+					SecurityGroup: lbSG,
+					ToPort:        fi.Int64(0),
+				})
+			} else {
 				c.AddTask(&awstasks.SecurityGroupRule{
 					Name:          fi.String("icmp-pmtu-api-elb-" + cidr),
 					Lifecycle:     b.SecurityLifecycle,
@@ -386,7 +396,17 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 				}
 
 				// Allow ICMP traffic required for PMTU discovery
-				if utils.IsIPv4CIDR(cidr) {
+				if utils.IsIPv6CIDR(cidr) {
+					c.AddTask(&awstasks.SecurityGroupRule{
+						Name:          fi.String("icmpv6-pmtu-api-elb-" + cidr),
+						Lifecycle:     b.SecurityLifecycle,
+						IPv6CIDR:      fi.String(cidr),
+						FromPort:      fi.Int64(2),
+						Protocol:      fi.String("icmpv6"),
+						SecurityGroup: masterGroup.Task,
+						ToPort:        fi.Int64(0),
+					})
+				} else {
 					c.AddTask(&awstasks.SecurityGroupRule{
 						Name:          fi.String("icmp-pmtu-api-elb-" + cidr),
 						Lifecycle:     b.SecurityLifecycle,

diff --git a/pkg/model/awsmodel/autoscalinggroup.go b/pkg/model/awsmodel/autoscalinggroup.go
@@ -214,8 +214,11 @@ func (b *AutoscalingGroupModelBuilder) buildLaunchTemplateTask(c *fi.ModelBuilde
 		}
 
 		// @step: add an IPv6 address
-		if subnets[0].IPv6CIDR != "" {
-			lt.IPv6AddressCount = fi.Int64(1)
+		for _, subnet := range subnets {
+			if subnet.IPv6CIDR != "" {
+				lt.IPv6AddressCount = fi.Int64(1)
+				break
+			}
 		}
 	}
 

diff --git a/tests/integration/update_cluster/minimal-ipv6/cloudformation.json b/tests/integration/update_cluster/minimal-ipv6/cloudformation.json
@@ -764,6 +764,18 @@
         "CidrIp": "0.0.0.0/0"
       }
     },
+    "AWSEC2SecurityGroupIngressicmpv6pmtuapielb0": {
+      "Type": "AWS::EC2::SecurityGroupIngress",
+      "Properties": {
+        "GroupId": {
+          "Ref": "AWSEC2SecurityGroupmastersminimalipv6examplecom"
+        },
+        "FromPort": 2,
+        "ToPort": 0,
+        "IpProtocol": "icmpv6",
+        "CidrIpv6": "::/0"
+      }
+    },
     "AWSEC2SecurityGroupapielbminimalipv6examplecom": {
       "Type": "AWS::EC2::SecurityGroup",
       "Properties": {

diff --git a/tests/integration/update_cluster/minimal-ipv6/kubernetes.tf b/tests/integration/update_cluster/minimal-ipv6/kubernetes.tf
@@ -730,6 +730,15 @@ resource "aws_security_group_rule" "icmp-pmtu-api-elb-0-0-0-0--0" {
   type              = "ingress"
 }
 
+resource "aws_security_group_rule" "icmpv6-pmtu-api-elb-__--0" {
+  from_port         = 2
+  ipv6_cidr_blocks  = ["::/0"]
+  protocol          = "icmpv6"
+  security_group_id = aws_security_group.masters-minimal-ipv6-example-com.id
+  to_port           = 0
+  type              = "ingress"
+}
+
 resource "aws_subnet" "us-test-1a-minimal-ipv6-example-com" {
   availability_zone = "us-test-1a"
   cidr_block        = "172.20.32.0/19"

diff --git a/upup/pkg/fi/utils/cidr.go b/upup/pkg/fi/utils/cidr.go
@@ -18,6 +18,7 @@ package utils
 
 import (
 	"net"
+	"strings"
 )
 
 func IsIPv4CIDR(cidr string) bool {
@@ -30,6 +31,10 @@ func IsIPv4CIDR(cidr string) bool {
 	if ip.To4() == nil {
 		return false
 	}
+	// Must NOT contain ":"
+	if strings.Contains(cidr, ":") {
+		return false
+	}
 
 	return true
 }
@@ -44,8 +49,8 @@ func IsIPv6CIDR(cidr string) bool {
 	if ip.To4() != nil {
 		return false
 	}
-	// Must convert to IPv6
-	if ip.To16() == nil {
+	// Must contain ":"
+	if !strings.Contains(cidr, ":") {
 		return false
 	}