Move NLB's VPC CIDR security group rule logic into model

This way the security group rule task doesn't need to be aware of VPCs, since we know the VPC CIDR ahead of time via cluster spec. This also fixes the terraform and cloudformation rendering of this rule (see the added cidr block in the integration test outputs) These rules are for NLB's health checks. The AWS docs recommend allowing access from the entire VPC CIDRs Also add rules for additionalNetworkCIDRs, supporting VPCs with multiple CIDR blocks.
kubernetes · Nov 3, 2020 · 9e38d09 · 9e38d09
1 parent be5c344
commit 9e38d09
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 21 deletions.
diff --git a/pkg/model/awsmodel/api_loadbalancer.go b/pkg/model/awsmodel/api_loadbalancer.go
@@ -357,8 +357,19 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 				Protocol:      fi.String("tcp"),
 				SecurityGroup: masterGroup.Task,
 				ToPort:        fi.Int64(443),
-				VPC:           b.LinkToVPC(),
+				CIDR:          &b.Cluster.Spec.NetworkCIDR,
 			})
+			for _, cidr := range b.Cluster.Spec.AdditionalNetworkCIDRs {
+				c.AddTask(&awstasks.SecurityGroupRule{
+					Name:          fi.String(fmt.Sprintf("https-lb-to-master%s-%s", suffix, cidr)),
+					Lifecycle:     b.SecurityLifecycle,
+					FromPort:      fi.Int64(443),
+					Protocol:      fi.String("tcp"),
+					SecurityGroup: masterGroup.Task,
+					ToPort:        fi.Int64(443),
+					CIDR:          &cidr,
+				})
+			}
 		}
 	}
 

diff --git a/tests/integration/update_cluster/complex/cloudformation.json b/tests/integration/update_cluster/complex/cloudformation.json
@@ -643,7 +643,32 @@
         },
         "FromPort": 443,
         "ToPort": 443,
-        "IpProtocol": "tcp"
+        "IpProtocol": "tcp",
+        "CidrIp": "172.20.0.0/16"
+      }
+    },
+    "AWSEC2SecurityGroupIngresshttpslbtomaster1010016": {
+      "Type": "AWS::EC2::SecurityGroupIngress",
+      "Properties": {
+        "GroupId": {
+          "Ref": "AWSEC2SecurityGroupmasterscomplexexamplecom"
+        },
+        "FromPort": 443,
+        "ToPort": 443,
+        "IpProtocol": "tcp",
+        "CidrIp": "10.2.0.0/16"
+      }
+    },
+    "AWSEC2SecurityGroupIngresshttpslbtomaster1020016": {
+      "Type": "AWS::EC2::SecurityGroupIngress",
+      "Properties": {
+        "GroupId": {
+          "Ref": "AWSEC2SecurityGroupmasterscomplexexamplecom"
+        },
+        "FromPort": 443,
+        "ToPort": 443,
+        "IpProtocol": "tcp",
+        "CidrIp": "10.2.0.0/16"
       }
     },
     "AWSEC2SecurityGroupIngressicmppmtuapielb111024": {

diff --git a/tests/integration/update_cluster/complex/kubernetes.tf b/tests/integration/update_cluster/complex/kubernetes.tf
@@ -543,6 +543,25 @@ resource "aws_security_group_rule" "https-api-elb-2001_0_8500__--40" {
 }
 
 resource "aws_security_group_rule" "https-elb-to-master" {
+  cidr_blocks       = ["172.20.0.0/16"]
+  from_port         = 443
+  protocol          = "tcp"
+  security_group_id = aws_security_group.masters-complex-example-com.id
+  to_port           = 443
+  type              = "ingress"
+}
+
+resource "aws_security_group_rule" "https-lb-to-master-10-1-0-0--16" {
+  cidr_blocks       = ["10.2.0.0/16"]
+  from_port         = 443
+  protocol          = "tcp"
+  security_group_id = aws_security_group.masters-complex-example-com.id
+  to_port           = 443
+  type              = "ingress"
+}
+
+resource "aws_security_group_rule" "https-lb-to-master-10-2-0-0--16" {
+  cidr_blocks       = ["10.2.0.0/16"]
   from_port         = 443
   protocol          = "tcp"
   security_group_id = aws_security_group.masters-complex-example-com.id

diff --git a/upup/pkg/fi/cloudup/awstasks/securitygrouprule.go b/upup/pkg/fi/cloudup/awstasks/securitygrouprule.go
@@ -47,7 +47,6 @@ type SecurityGroupRule struct {
 	SourceGroup *SecurityGroup
 
 	Egress *bool
-	VPC    *VPC
 }
 
 func (e *SecurityGroupRule) Find(c *fi.Context) (*SecurityGroupRule, error) {
@@ -105,7 +104,6 @@ func (e *SecurityGroupRule) Find(c *fi.Context) (*SecurityGroupRule, error) {
 			ToPort:        foundRule.ToPort,
 			Protocol:      foundRule.IpProtocol,
 			Egress:        e.Egress,
-			VPC:           e.VPC,
 		}
 
 		if aws.StringValue(actual.Protocol) == "-1" {
@@ -157,19 +155,6 @@ func (e *SecurityGroupRule) matches(rule *ec2.IpPermission) bool {
 		}
 	}
 
-	if e.VPC != nil && e.VPC.CIDR != nil {
-		match := false
-		for _, ipRange := range rule.IpRanges {
-			if aws.StringValue(ipRange.CidrIp) == *e.VPC.CIDR {
-				match = true
-				break
-			}
-		}
-		if !match {
-			return false
-		}
-	}
-
 	if e.SourceGroup != nil {
 		// TODO: Only if len 1?
 		match := false
@@ -266,10 +251,6 @@ func (_ *SecurityGroupRule) RenderAWS(t *awsup.AWSAPITarget, a, e, changes *Secu
 			}
 		} else {
 			CIDR := e.CIDR
-			//TODO: Verify NLB is setting vpc CIDR
-			if e.VPC != nil { //ALLOW security group to use vpc cidr for network load balancer.
-				CIDR = e.VPC.CIDR
-			}
 			// Default to 0.0.0.0/0 ?
 			ipPermission.IpRanges = []*ec2.IpRange{
 				{CidrIp: CIDR},