Login error handling (#1093)

* Improved handling unknown API in kgs login * Reordered imports * Improved parsing int32 in tests * Updated changelog * Reordered imports
giantswarm · Aug 1, 2023 · 5e8ab9e · 5e8ab9e
1 parent fb7cef3
commit 5e8ab9e
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ and this project's packages adheres to [Semantic Versioning](http://semver.org/s
 
 ## [Unreleased]
 
+### Changed
+
+- Graceful failure of the `login` command in case workload cluster API is not known
+
 ## [2.39.0] - 2023-06-22
 
 ### Breaking changes

diff --git a/cmd/login/error.go b/cmd/login/error.go
@@ -226,3 +226,21 @@ var insufficientPermissionsError = &microerror.Error{
 func IsInsufficientPermissions(err error) bool {
 	return microerror.Cause(err) == insufficientPermissionsError
 }
+
+var clusterAPINotReadyError = &microerror.Error{
+	Kind: "clusterAPINotReadyError",
+}
+
+// IsClusterAPINotReady asserts clusterAPINotReadyError.
+func IsClusterAPINotReady(err error) bool {
+	return microerror.Cause(err) == clusterAPINotReadyError
+}
+
+var clusterAPINotKnownError = &microerror.Error{
+	Kind: "clusterAPINotKnownError",
+}
+
+// IsClusterAPINotKnown asserts clusterAPINotKnownError.
+func IsClusterAPINotKnown(err error) bool {
+	return microerror.Cause(err) == clusterAPINotKnownError
+}
diff --git a/cmd/login/runner_config_modify_test.go b/cmd/login/runner_config_modify_test.go
@@ -593,8 +593,13 @@ func createCluster(name string, namespace string) v1beta1.Cluster {
 	return v1beta1.Cluster{
 		TypeMeta:   v1.TypeMeta{Kind: "Cluster", APIVersion: "cluster.x-k8s.io/v1beta1"},
 		ObjectMeta: v1.ObjectMeta{Name: name, Namespace: namespace},
-		Spec:       v1beta1.ClusterSpec{},
-		Status:     v1beta1.ClusterStatus{},
+		Spec: v1beta1.ClusterSpec{
+			ControlPlaneEndpoint: v1beta1.APIEndpoint{
+				Host: "localhost",
+				Port: 6443,
+			},
+		},
+		Status: v1beta1.ClusterStatus{},
 	}
 }
 

diff --git a/cmd/login/wc.go b/cmd/login/wc.go
@@ -6,12 +6,14 @@ import (
 	"net"
 	"regexp"
 	"strings"
+	"time"
+
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/tools/clientcmd"
 
 	"github.com/fatih/color"
 	"github.com/giantswarm/k8sclient/v7/pkg/k8sclient"
 	"github.com/giantswarm/microerror"
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/client-go/tools/clientcmd"
 
 	"github.com/giantswarm/kubectl-gs/v2/internal/key"
 	"github.com/giantswarm/kubectl-gs/v2/pkg/data/domain/clientcert"
@@ -21,6 +23,10 @@ import (
 	"github.com/giantswarm/kubectl-gs/v2/pkg/kubeconfig"
 )
 
+const (
+	newClusterMaxAge = 30 * time.Minute
+)
+
 func (r *runner) getServiceSet(client k8sclient.Interface) (serviceSet, error) {
 	var err error
 
@@ -129,6 +135,13 @@ func (r *runner) handleWCClientCert(ctx context.Context) error {
 	// At the moment, the only available login option for WC is client cert
 	contextName, contextExists, err := r.createClusterClientCert(ctx, client, provider)
 	if err != nil {
+		if IsClusterAPINotReady(err) {
+			fmt.Fprintf(r.stdout, "\nCould not create a context for workload cluster %s, as the cluster's API server endpoint is not known yet.\n", r.flag.WCName)
+			fmt.Fprintf(r.stdout, "If the cluster has been created recently, please wait for a few minutes and try again.\n")
+		} else if IsClusterAPINotKnown(err) {
+			fmt.Fprintf(r.stdout, "\nCould not create a context for workload cluster %s, as the cluster's API server endpoint is not known.\n", r.flag.WCName)
+			fmt.Fprintf(r.stdout, "Since the cluster has been created a while ago, this appears to be a problem with cluster creation. Please contact Giant Swarm's support.\n")
+		}
 		return microerror.Mask(err)
 	}
 
@@ -193,6 +206,13 @@ func (r *runner) createClusterClientCert(ctx context.Context, client k8sclient.I
 		return "", false, microerror.Mask(err)
 	}
 
+	if c.Cluster.Spec.ControlPlaneEndpoint.Host == "" || c.Cluster.Spec.ControlPlaneEndpoint.Port == 0 {
+		if c.Cluster.ObjectMeta.CreationTimestamp.Time.Before(time.Now().Add(-newClusterMaxAge)) {
+			return "", false, microerror.Maskf(clusterAPINotKnownError, "API for cluster '%s' is not known", r.flag.WCName)
+		}
+		return "", false, microerror.Maskf(clusterAPINotReadyError, "API for cluster '%s' is not ready yet", r.flag.WCName)
+	}
+
 	clusterServer := fmt.Sprintf("https://%s:%d", c.Cluster.Spec.ControlPlaneEndpoint.Host, c.Cluster.Spec.ControlPlaneEndpoint.Port)
 
 	// If the control plane host is an IP address then it is a CAPI cluster and

diff --git a/cmd/login/wc_test.go b/cmd/login/wc_test.go
@@ -7,11 +7,13 @@ import (
 	"crypto/x509"
 	"fmt"
 	"math/big"
+	"net/url"
 	"os"
 	"reflect"
 	"strconv"
 	"strings"
 	"testing"
+	"time"
 
 	v1 "k8s.io/api/authorization/v1"
 
@@ -43,6 +45,8 @@ func TestWCClientCert(t *testing.T) {
 		name                 string
 		flags                *flag
 		provider             string
+		controlPlaneEndpoint string
+		creationTimestamp    time.Time
 		capi                 bool
 		clustersInNamespaces map[string]string
 		isAdmin              bool
@@ -53,6 +57,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 0",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:    "cluster",
 				WCCertTTL: "8h",
@@ -64,6 +69,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 1",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:    "anothercluster",
 				WCCertTTL: "8h",
@@ -76,6 +82,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 2",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:        "cluster",
 				WCCertTTL:     "8h",
@@ -95,6 +102,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 3",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:      "cluster",
 				WCCertTTL:   "8h",
@@ -114,6 +122,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 4",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:         "cluster",
 				WCCertTTL:      "8h",
@@ -126,6 +135,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 5",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization", "anothercluster": "default"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:    "cluster",
 				WCCertTTL: "8h",
@@ -137,6 +147,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 6",
 			clustersInNamespaces: map[string]string{"cluster": "default"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:    "cluster",
 				WCCertTTL: "8h",
@@ -156,6 +167,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 7",
 			clustersInNamespaces: map[string]string{"cluster": "default"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:              "cluster",
 				WCCertTTL:           "8h",
@@ -175,6 +187,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 8",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:    "cluster",
 				WCCertTTL: "8h",
@@ -187,6 +200,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 9",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:    "cluster",
 				WCCertTTL: "8h",
@@ -198,6 +212,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 10",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:    "cluster",
 				WCCertTTL: "8h",
@@ -217,6 +232,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 11",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:         "cluster",
 				WCCertTTL:      "8h",
@@ -229,6 +245,7 @@ func TestWCClientCert(t *testing.T) {
 		{
 			name:                 "case 12",
 			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			controlPlaneEndpoint: "https://localhost:6443",
 			flags: &flag{
 				WCName:         "cluster",
 				WCCertTTL:      "8h",
@@ -238,10 +255,36 @@ func TestWCClientCert(t *testing.T) {
 			capi:     true,
 			isAdmin:  true,
 		},
+		// Logging into WC with empty control plane endpoint
+		{
+			name:                 "case 13",
+			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			flags: &flag{
+				WCName:    "cluster",
+				WCCertTTL: "8h",
+			},
+			provider:    "aws",
+			isAdmin:     true,
+			expectError: clusterAPINotKnownError,
+		},
+		// Logging into newly created WC with empty control plane endpoint
+		{
+			name:                 "case 14",
+			clustersInNamespaces: map[string]string{"cluster": "org-organization"},
+			creationTimestamp:    time.Now().Add(-15 * time.Minute),
+			flags: &flag{
+				WCName:    "cluster",
+				WCCertTTL: "8h",
+			},
+			provider:    "capa",
+			capi:        true,
+			isAdmin:     true,
+			expectError: clusterAPINotReadyError,
+		},
 	}
 
-	for i, tc := range testCases {
-		t.Run(strconv.Itoa(i), func(t *testing.T) {
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
 			configDir, err := os.MkdirTemp("", "loginTest")
 			if err != nil {
 				t.Fatal(err)
@@ -285,7 +328,7 @@ func TestWCClientCert(t *testing.T) {
 					}
 				}
 				for wcName, wcNamespace := range tc.clustersInNamespaces {
-					err = client.CtrlClient().Create(ctx, getCluster(wcName, wcNamespace))
+					err = client.CtrlClient().Create(ctx, getCluster(wcName, wcNamespace, tc.controlPlaneEndpoint, tc.creationTimestamp))
 					if err != nil {
 						t.Fatal(err)
 					}
@@ -498,7 +541,8 @@ func getOrganization(orgnamespace string) *securityv1alpha1.Organization {
 	return organization
 }
 
-func getCluster(name string, namespace string) *capi.Cluster {
+func getCluster(name, namespace, controlPlaneEndpoint string, creationTimestamp time.Time) *capi.Cluster {
+	controlPlaneURL, _ := url.Parse(controlPlaneEndpoint)
 	cluster := &capi.Cluster{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "Cluster",
@@ -517,6 +561,20 @@ func getCluster(name string, namespace string) *capi.Cluster {
 		Spec: capi.ClusterSpec{},
 	}
 
+	if controlPlaneURL != nil {
+		port, err := strconv.ParseInt(controlPlaneURL.Port(), 10, 32)
+		if err == nil {
+			cluster.Spec.ControlPlaneEndpoint = capi.APIEndpoint{
+				Host: controlPlaneURL.Host,
+				Port: int32(port),
+			}
+		}
+	}
+
+	if !creationTimestamp.IsZero() {
+		cluster.ObjectMeta.CreationTimestamp = metav1.NewTime(creationTimestamp)
+	}
+
 	return cluster
 }