Always use load balancer address in kubeconfig

pkg/kubeconfig/create_kubecfg.go

@@ -35,42 +35,20 @@ const DefaultKubecfgAdminLifetime = 18 * time.Hour
 func BuildKubecfg(cluster *kops.Cluster, keyStore fi.Keystore, secretStore fi.SecretStore, cloud fi.Cloud, admin time.Duration, configUser string, internal bool, kopsStateStore string, useKopsAuthenticationPlugin bool) (*KubeconfigBuilder, error) {
 	clusterName := cluster.ObjectMeta.Name
 
-	var master string
+	var server string
 	if internal {
-		master = cluster.APIInternalName()
+		server = "https://" + cluster.APIInternalName()
 	} else {
-		master = cluster.Spec.API.PublicName
+		if cluster.Spec.API.PublicName != "" {
-		if master == "" {
+			server = "https://" + cluster.Spec.API.PublicName
-			master = "api." + clusterName
+		} else {
+			server = "https://api." + clusterName
 		}
 	}
 
-	server := "https://" + master
+	// If a load balancer exists we use it, except for when an SSL certificate is set.
-
+	// This should avoid a lot of pain with DNS pre-creation.
-	// We use the LoadBalancer where we know the master DNS name is otherwise unreachable
+	if cluster.Spec.API.LoadBalancer != nil && (cluster.Spec.API.LoadBalancer.SSLCertificate == "" || admin != 0) {
-	useELBName := false
-
-	// If the master DNS is a gossip DNS name; there's no way that name can resolve outside the cluster
-	if cluster.IsGossip() {
-		useELBName = true
-	}
-
-	// If the cluster has DNS disabled, must use the load balancer name
-	if cluster.UsesNoneDNS() {
-		useELBName = true
-	}
-
-	// If the DNS is set up as a private HostedZone, but here we have to be
-	// careful that we aren't accessing the API over DirectConnect (or a VPN).
-	// We differentiate using the heuristic that if we have an internal ELB
-	// we are likely connected directly to the VPC.
-	privateDNS := cluster.Spec.Networking.Topology != nil && cluster.Spec.Networking.Topology.DNS == kops.DNSTypePrivate
-	internalELB := cluster.Spec.API.LoadBalancer != nil && cluster.Spec.API.LoadBalancer.Type == kops.LoadBalancerTypeInternal
-	if privateDNS && !internalELB {
-		useELBName = true
-	}
-
-	if useELBName {
 		ingresses, err := cloud.GetApiIngressStatus(cluster)
 		if err != nil {
 			return nil, fmt.Errorf("error getting ingress status: %v", err)

pkg/kubeconfig/create_kubecfg_test.go

@@ -121,6 +121,7 @@ func buildMinimalCluster(clusterName string, masterPublicName string, lbCert boo
 	cluster := testutils.BuildMinimalCluster(clusterName)
 	cluster.Spec.API.PublicName = masterPublicName
 	cluster.Spec.KubernetesVersion = "1.24.0"
+	if lbCert || nlb {
 		cluster.Spec.API.LoadBalancer = &kops.LoadBalancerAccessSpec{}
 		if lbCert {
 			cluster.Spec.API.LoadBalancer.SSLCertificate = "cert-arn"
@@ -128,6 +129,7 @@ func buildMinimalCluster(clusterName string, masterPublicName string, lbCert boo
 		if nlb {
 			cluster.Spec.API.LoadBalancer.Class = kops.LoadBalancerClassNetwork
 		}
+	}
 	return cluster
 }
 
@@ -160,11 +162,21 @@ func TestBuildKubecfg(t *testing.T) {
 
 	publicCluster := buildMinimalCluster("testcluster", "testcluster.test.com", false, false)
 	emptyMasterPublicNameCluster := buildMinimalCluster("emptyMasterPublicNameCluster", "", false, false)
-	gossipCluster := buildMinimalCluster("testgossipcluster.k8s.local", "", false, false)
+	gossipCluster := buildMinimalCluster("testgossipcluster.k8s.local", "", false, true)
 	certCluster := buildMinimalCluster("testcluster", "testcluster.test.com", true, false)
 	certNLBCluster := buildMinimalCluster("testcluster", "testcluster.test.com", true, true)
 	certGossipNLBCluster := buildMinimalCluster("testgossipcluster.k8s.local", "", true, true)
 
+	fakeStatus := fakeStatusCloud{
+		GetApiIngressStatusFn: func(cluster *kops.Cluster) ([]fi.ApiIngressStatus, error) {
+			return []fi.ApiIngressStatus{
+				{
+					Hostname: "elbHostName",
+				},
+			}, nil
+		},
+	}
+
 	tests := []struct {
 		name           string
 		args           args
@@ -176,7 +188,7 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Public DNS with admin",
 			args: args{
 				cluster: publicCluster,
-				status:  fakeStatusCloud{},
+				status:  fakeStatus,
 				admin:   DefaultKubecfgAdminLifetime,
 				user:    "",
 			},
@@ -192,12 +204,12 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Public DNS with admin and secondary NLB port",
 			args: args{
 				cluster: certNLBCluster,
-				status:  fakeStatusCloud{},
+				status:  fakeStatus,
 				admin:   DefaultKubecfgAdminLifetime,
 			},
 			want: &KubeconfigBuilder{
 				Context: "testcluster",
-				Server:  "https://testcluster.test.com:8443",
+				Server:  "https://elbHostName:8443",
 				CACerts: []byte(nextCertificate + certData),
 				User:    "testcluster",
 			},
@@ -207,12 +219,12 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Public DNS with admin and CLB ACM Certificate",
 			args: args{
 				cluster: certCluster,
-				status:  fakeStatusCloud{},
+				status:  fakeStatus,
 				admin:   DefaultKubecfgAdminLifetime,
 			},
 			want: &KubeconfigBuilder{
 				Context: "testcluster",
-				Server:  "https://testcluster.test.com",
+				Server:  "https://elbHostName",
 				CACerts: nil,
 				User:    "testcluster",
 			},
@@ -222,7 +234,7 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Public DNS without admin and with ACM certificate",
 			args: args{
 				cluster: certNLBCluster,
-				status:  fakeStatusCloud{},
+				status:  fakeStatus,
 				admin:   0,
 			},
 			want: &KubeconfigBuilder{
@@ -237,7 +249,7 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Public DNS without admin",
 			args: args{
 				cluster: publicCluster,
-				status:  fakeStatusCloud{},
+				status:  fakeStatus,
 				admin:   0,
 				user:    "myuser",
 			},
@@ -253,7 +265,7 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Public DNS with Empty Master Name",
 			args: args{
 				cluster: emptyMasterPublicNameCluster,
-				status:  fakeStatusCloud{},
+				status:  fakeStatus,
 				admin:   0,
 				user:    "",
 			},
@@ -269,15 +281,7 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Gossip cluster",
 			args: args{
 				cluster: gossipCluster,
-				status: fakeStatusCloud{
+				status:  fakeStatus,
-					GetApiIngressStatusFn: func(cluster *kops.Cluster) ([]fi.ApiIngressStatus, error) {
-						return []fi.ApiIngressStatus{
-							{
-								Hostname: "elbHostName",
-							},
-						}, nil
-					},
-				},
 			},
 			want: &KubeconfigBuilder{
 				Context: "testgossipcluster.k8s.local",
@@ -291,7 +295,7 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Public DNS with kops auth plugin",
 			args: args{
 				cluster:                     publicCluster,
-				status:                      fakeStatusCloud{},
+				status:                      fakeStatus,
 				admin:                       0,
 				useKopsAuthenticationPlugin: true,
 			},
@@ -314,7 +318,7 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For internal DNS name with admin",
 			args: args{
 				cluster:  publicCluster,
-				status:   fakeStatusCloud{},
+				status:   fakeStatus,
 				admin:    DefaultKubecfgAdminLifetime,
 				internal: true,
 			},
@@ -330,20 +334,12 @@ func TestBuildKubecfg(t *testing.T) {
 			name: "Test Kube Config Data For Gossip cluster with admin and secondary NLB port",
 			args: args{
 				cluster: certGossipNLBCluster,
-				status: fakeStatusCloud{
+				status:  fakeStatus,
-					GetApiIngressStatusFn: func(cluster *kops.Cluster) ([]fi.ApiIngressStatus, error) {
-						return []fi.ApiIngressStatus{
-							{
-								Hostname: "nlbHostName",
-							},
-						}, nil
-					},
-				},
 				admin:   DefaultKubecfgAdminLifetime,
 			},
 			want: &KubeconfigBuilder{
 				Context: "testgossipcluster.k8s.local",
-				Server:  "https://nlbHostName:8443",
+				Server:  "https://elbHostName:8443",
 				CACerts: []byte(nextCertificate + certData),
 				User:    "testgossipcluster.k8s.local",
 			},