Setup a second NLB listener when an AWS ACM certificate is used

pkg/apis/kops/validation/validation.go

@@ -217,6 +217,12 @@ func validateClusterSpec(spec *kops.ClusterSpec, c *kops.Cluster, fieldPath *fie
 		if featureflag.Spotinst.Enabled() && spec.API.LoadBalancer.Class == kops.LoadBalancerClassNetwork {
 			allErrs = append(allErrs, field.Forbidden(fieldPath, "cannot use NLB together with spotinst"))
 		}
+		if spec.API.LoadBalancer.SSLCertificate != "" && spec.API.LoadBalancer.Class != kops.LoadBalancerClassNetwork && c.IsKubernetesGTE("1.19") {
+			allErrs = append(allErrs, field.Forbidden(fieldPath, "sslCertificate requires network loadbalancer for K8s 1.19+ see https://github.com/kubernetes/kops/blob/master/permalinks/acm_nlb.md"))
+		}
+		if spec.API.LoadBalancer.Class == kops.LoadBalancerClassNetwork && spec.API.LoadBalancer.UseForInternalApi && spec.API.LoadBalancer.Type == kops.LoadBalancerTypeInternal {
+			allErrs = append(allErrs, field.Forbidden(fieldPath, "useForInternalApi cannot be used with internal NLB due lack of hairpinning support"))
+		}
 	}
 
 	return allErrs

pkg/kubeconfig/create_kubecfg.go

@@ -99,11 +99,17 @@ func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.Se
 
 	b := NewKubeconfigBuilder()
 
+	// Use the secondary load balancer port if a certificate is on the primary listener
+	if admin != 0 && cluster.Spec.API != nil && cluster.Spec.API.LoadBalancer != nil && cluster.Spec.API.LoadBalancer.SSLCertificate != "" && cluster.Spec.API.LoadBalancer.Class == kops.LoadBalancerClassNetwork {
+		server = server + ":8443"
+	}
+
 	b.Context = clusterName
 	b.Server = server
 
-	// add the CA Cert to the kubeconfig only if we didn't specify a SSL cert for the LB or are targeting the internal DNS name
-	if cluster.Spec.API == nil || cluster.Spec.API.LoadBalancer == nil || cluster.Spec.API.LoadBalancer.SSLCertificate == "" || internal {
+	// add the CA Cert to the kubeconfig only if we didn't specify a certificate for the LB
+	//  or if we're using admin credentials and the secondary port
+	if cluster.Spec.API == nil || cluster.Spec.API.LoadBalancer == nil || cluster.Spec.API.LoadBalancer.SSLCertificate == "" || cluster.Spec.API.LoadBalancer.Class == kops.LoadBalancerClassNetwork || internal {
 		cert, _, _, err := keyStore.FindKeypair(fi.CertificateIDCA)
 		if err != nil {
 			return nil, fmt.Errorf("error fetching CA keypair: %v", err)

pkg/kubeconfig/create_kubecfg_test.go

@@ -71,10 +71,20 @@ func (f fakeKeyStore) MirrorTo(basedir vfs.Path) error {
 }
 
 // build a generic minimal cluster
-func buildMinimalCluster(clusterName string, masterPublicName string) *kops.Cluster {
+func buildMinimalCluster(clusterName string, masterPublicName string, lbCert bool, nlb bool) *kops.Cluster {
 	cluster := testutils.BuildMinimalCluster(clusterName)
 	cluster.Spec.MasterPublicName = masterPublicName
 	cluster.Spec.MasterInternalName = fmt.Sprintf("internal.%v", masterPublicName)
+	cluster.Spec.KubernetesVersion = "1.19.3"
+	cluster.Spec.API = &kops.AccessSpec{
+		LoadBalancer: &kops.LoadBalancerAccessSpec{},
+	}
+	if lbCert {
+		cluster.Spec.API.LoadBalancer.SSLCertificate = "cert-arn"
+	}
+	if nlb {
+		cluster.Spec.API.LoadBalancer.Class = kops.LoadBalancerClassNetwork
+	}
 	return cluster
 }
 
@@ -107,9 +117,12 @@ func TestBuildKubecfg(t *testing.T) {
 		useKopsAuthenticationPlugin bool
 	}
 
-	publiccluster := buildMinimalCluster("testcluster", "testcluster.test.com")
-	emptyMasterPublicNameCluster := buildMinimalCluster("emptyMasterPublicNameCluster", "")
-	gossipCluster := buildMinimalCluster("testgossipcluster.k8s.local", "")
+	publicCluster := buildMinimalCluster("testcluster", "testcluster.test.com", false, false)
+	emptyMasterPublicNameCluster := buildMinimalCluster("emptyMasterPublicNameCluster", "", false, false)
+	gossipCluster := buildMinimalCluster("testgossipcluster.k8s.local", "", false, false)
+	certCluster := buildMinimalCluster("testcluster", "testcluster.test.com", true, false)
+	certNLBCluster := buildMinimalCluster("testcluster", "testcluster.test.com", true, true)
+	certGossipNLBCluster := buildMinimalCluster("testgossipcluster.k8s.local", "", true, true)
 
 	tests := []struct {
 		name           string
@@ -121,7 +134,7 @@ func TestBuildKubecfg(t *testing.T) {
 		{
 			name: "Test Kube Config Data For Public DNS with admin",
 			args: args{
-				cluster: publiccluster,
+				cluster: publicCluster,
 				status:  fakeStatusStore{},
 				admin:   DefaultKubecfgAdminLifetime,
 				user:    "",
@@ -134,10 +147,55 @@ func TestBuildKubecfg(t *testing.T) {
 			},
 			wantClientCert: true,
 		},
+		{
+			name: "Test Kube Config Data For Public DNS with admin and secondary NLB port",
+			args: args{
+				cluster: certNLBCluster,
+				status:  fakeStatusStore{},
+				admin:   DefaultKubecfgAdminLifetime,
+			},
+			want: &KubeconfigBuilder{
+				Context: "testcluster",
+				Server:  "https://testcluster.test.com:8443",
+				CACert:  []byte(certData),
+				User:    "testcluster",
+			},
+			wantClientCert: true,
+		},
+		{
+			name: "Test Kube Config Data For Public DNS with admin and CLB ACM Certificate",
+			args: args{
+				cluster: certCluster,
+				status:  fakeStatusStore{},
+				admin:   DefaultKubecfgAdminLifetime,
+			},
+			want: &KubeconfigBuilder{
+				Context: "testcluster",
+				Server:  "https://testcluster.test.com",
+				CACert:  nil,
+				User:    "testcluster",
+			},
+			wantClientCert: true,
+		},
+		{
+			name: "Test Kube Config Data For Public DNS without admin and with ACM certificate",
+			args: args{
+				cluster: certNLBCluster,
+				status:  fakeStatusStore{},
+				admin:   0,
+			},
+			want: &KubeconfigBuilder{
+				Context: "testcluster",
+				Server:  "https://testcluster.test.com",
+				CACert:  []byte(certData),
+				User:    "testcluster",
+			},
+			wantClientCert: false,
+		},
 		{
 			name: "Test Kube Config Data For Public DNS without admin",
 			args: args{
-				cluster: publiccluster,
+				cluster: publicCluster,
 				status:  fakeStatusStore{},
 				admin:   0,
 				user:    "myuser",
@@ -191,7 +249,7 @@ func TestBuildKubecfg(t *testing.T) {
 		{
 			name: "Public DNS with kops auth plugin",
 			args: args{
-				cluster:                     publiccluster,
+				cluster:                     publicCluster,
 				status:                      fakeStatusStore{},
 				admin:                       0,
 				useKopsAuthenticationPlugin: true,
@@ -214,7 +272,7 @@ func TestBuildKubecfg(t *testing.T) {
 		{
 			name: "Test Kube Config Data For internal DNS name with admin",
 			args: args{
-				cluster:  publiccluster,
+				cluster:  publicCluster,
 				status:   fakeStatusStore{},
 				admin:    DefaultKubecfgAdminLifetime,
 				internal: true,
@@ -227,6 +285,29 @@ func TestBuildKubecfg(t *testing.T) {
 			},
 			wantClientCert: true,
 		},
+		{
+			name: "Test Kube Config Data For Gossip cluster with admin and secondary NLB port",
+			args: args{
+				cluster: certGossipNLBCluster,
+				status: fakeStatusStore{
+					GetApiIngressStatusFn: func(cluster *kops.Cluster) ([]kops.ApiIngressStatus, error) {
+						return []kops.ApiIngressStatus{
+							{
+								Hostname: "nlbHostName",
+							},
+						}, nil
+					},
+				},
+				admin: DefaultKubecfgAdminLifetime,
+			},
+			want: &KubeconfigBuilder{
+				Context: "testgossipcluster.k8s.local",
+				Server:  "https://nlbHostName:8443",
+				CACert:  []byte(certData),
+				User:    "testgossipcluster.k8s.local",
+			},
+			wantClientCert: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/model/awsmodel/api_loadbalancer.go

@@ -109,14 +109,21 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 			"443": {InstancePort: 443},
 		}
 
-		nlbListenerPort := "443"
-		nlbListeners := map[string]*awstasks.NetworkLoadBalancerListener{
-			nlbListenerPort: {Port: 443},
+		nlbListeners := []*awstasks.NetworkLoadBalancerListener{
+			{
+				Port:            443,
+				TargetGroupName: b.NLBTargetGroupName("tcp"),
+			},
 		}
 
 		if lbSpec.SSLCertificate != "" {
 			listeners["443"].SSLCertificateID = lbSpec.SSLCertificate
-			nlbListeners["443"].SSLCertificateID = lbSpec.SSLCertificate
+			nlbListeners[0].Port = 8443
+			nlbListeners = append(nlbListeners, &awstasks.NetworkLoadBalancerListener{
+				Port:             443,
+				TargetGroupName:  b.NLBTargetGroupName("tls"),
+				SSLCertificateID: lbSpec.SSLCertificate,
+			})
 		}
 
 		if lbSpec.SecurityGroupOverride != nil {
@@ -138,6 +145,7 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 			LoadBalancerName: fi.String(loadBalancerName),
 			Subnets:          elbSubnets,
 			Listeners:        nlbListeners,
+			TargetGroups:     make([]*awstasks.TargetGroup, 0),
 
 			Tags: tags,
 			VPC:  b.LinkToVPC(),
@@ -196,28 +204,47 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 			c.AddTask(clb)
 		} else if b.APILoadBalancerClass() == kops.LoadBalancerClassNetwork {
 
-			targetGroupPort := fi.Int64(443)
-			targetGroupName := b.NLBTargetGroupName("api")
-			tags := b.CloudTags(targetGroupName, false)
+			tcpGroupName := b.NLBTargetGroupName("tcp")
+			tcpGroupTags := b.CloudTags(tcpGroupName, false)
 
 			// Override the returned name to be the expected NLB TG name
-			tags["Name"] = targetGroupName
+			tcpGroupTags["Name"] = tcpGroupName
 
 			tg := &awstasks.TargetGroup{
-				Name:               fi.String(targetGroupName),
+				Name:               fi.String(tcpGroupName),
 				VPC:                b.LinkToVPC(),
-				Tags:               tags,
+				Tags:               tcpGroupTags,
 				Protocol:           fi.String("TCP"),
-				Port:               targetGroupPort,
+				Port:               fi.Int64(443),
 				HealthyThreshold:   fi.Int64(2),
 				UnhealthyThreshold: fi.Int64(2),
 				Shared:             fi.Bool(false),
 			}
 
 			c.AddTask(tg)
 
-			nlb.TargetGroup = tg
-
+			nlb.TargetGroups = append(nlb.TargetGroups, tg)
+
+			if lbSpec.SSLCertificate != "" {
+				tlsGroupName := b.NLBTargetGroupName("tls")
+				tlsGroupTags := b.CloudTags(tlsGroupName, false)
+
+				// Override the returned name to be the expected NLB TG name
+				tlsGroupTags["Name"] = tlsGroupName
+				secondaryTG := &awstasks.TargetGroup{
+					Name:               fi.String(tlsGroupName),
+					VPC:                b.LinkToVPC(),
+					Tags:               tlsGroupTags,
+					Protocol:           fi.String("TLS"),
+					Port:               fi.Int64(443),
+					HealthyThreshold:   fi.Int64(2),
+					UnhealthyThreshold: fi.Int64(2),
+					Shared:             fi.Bool(false),
+				}
+				c.AddTask(secondaryTG)
+				nlb.TargetGroups = append(nlb.TargetGroups, secondaryTG)
+			}
+			sort.Stable(awstasks.OrderTargetGroupsByName(nlb.TargetGroups))
 			c.AddTask(nlb)
 		}
 
@@ -291,7 +318,6 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 
 			for _, masterGroup := range masterGroups {
 				t := &awstasks.SecurityGroupRule{
-					// TODO: figure out how to only add this to one SG (GetSecurityGroups above returns multiple)
 					Name:          fi.String(fmt.Sprintf("https-api-elb-%s", cidr)),
 					Lifecycle:     b.SecurityLifecycle,
 					CIDR:          fi.String(cidr),
@@ -304,7 +330,6 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 
 				// Allow ICMP traffic required for PMTU discovery
 				c.AddTask(&awstasks.SecurityGroupRule{
-					// TODO: figure out how to only add this to one SG (GetSecurityGroups above returns multiple)
 					Name:          fi.String("icmp-pmtu-api-elb-" + cidr),
 					Lifecycle:     b.SecurityLifecycle,
 					CIDR:          fi.String(cidr),
@@ -313,6 +338,19 @@ func (b *APILoadBalancerBuilder) Build(c *fi.ModelBuilderContext) error {
 					SecurityGroup: masterGroup.Task,
 					ToPort:        fi.Int64(4),
 				})
+
+				if b.Cluster.Spec.API != nil && b.Cluster.Spec.API.LoadBalancer != nil && b.Cluster.Spec.API.LoadBalancer.SSLCertificate != "" {
+					// Allow access to masters on secondary port through NLB
+					c.AddTask(&awstasks.SecurityGroupRule{
+						Name:          fi.String(fmt.Sprintf("tcp-api-%s", cidr)),
+						Lifecycle:     b.SecurityLifecycle,
+						CIDR:          fi.String(cidr),
+						FromPort:      fi.Int64(8443),
+						Protocol:      fi.String("tcp"),
+						SecurityGroup: masterGroup.Task,
+						ToPort:        fi.Int64(8443),
+					})
+				}
 			}
 		}
 	}

pkg/model/awsmodel/autoscalinggroup.go

@@ -370,7 +370,10 @@ func (b *AutoscalingGroupModelBuilder) buildAutoScalingGroupTask(c *fi.ModelBuil
 	if !featureflag.Spotinst.Enabled() {
 		if b.UseLoadBalancerForAPI() && ig.Spec.Role == kops.InstanceGroupRoleMaster {
 			if b.UseNetworkLoadBalancer() {
-				t.TargetGroups = append(t.TargetGroups, b.LinkToTargetGroup("api"))
+				t.TargetGroups = append(t.TargetGroups, b.LinkToTargetGroup("tcp"))
+				if b.Cluster.Spec.API.LoadBalancer.SSLCertificate != "" {
+					t.TargetGroups = append(t.TargetGroups, b.LinkToTargetGroup("tls"))
+				}
 			} else {
 				t.LoadBalancers = append(t.LoadBalancers, b.LinkToCLB("api"))
 			}

pkg/model/firewall.go

@@ -418,6 +418,7 @@ func (b *KopsModelContext) GetSecurityGroups(role kops.InstanceGroupRole) ([]Sec
 				"port=4002", // etcd events
 				"port=4789", // VXLAN
 				"port=179",  // Calico
+				"port=8443", // k8s api secondary listener
 
 				// TODO: UDP vs TCP
 				// TODO: Protocol 4 for calico

pkg/model/names.go

@@ -105,7 +105,7 @@ func (b *KopsModelContext) LinkToNLB(prefix string) *awstasks.NetworkLoadBalance
 }
 
 func (b *KopsModelContext) LinkToTargetGroup(prefix string) *awstasks.TargetGroup {
-	name := b.NLBTargetGroupName(prefix) // TODO: this will need to change for the ACM cert bugfix since we'll have multiple TGs
+	name := b.NLBTargetGroupName(prefix)
 	return &awstasks.TargetGroup{Name: &name}
 }