From a6649fc82ca20c2cd3878cc909bf10ae8207f16e Mon Sep 17 00:00:00 2001
From: James Sharpe <james.sharpe@zenotech.com>
Date: Thu, 25 Apr 2019 22:34:47 +0100
Subject: [PATCH] fix: Handle shortening cluster names for clusters named by
 their FQDN.

Closes #3755

Signed-off-by: James Sharpe <james.sharpe@zenotech.com>
---
 pkg/kube/cluster/cluster.go      | 22 +++-------------------
 pkg/kube/cluster/cluster_test.go | 13 ++++++++++---
 pkg/kube/vault/vault.go          | 13 +++++++------
 3 files changed, 20 insertions(+), 28 deletions(-)

diff --git a/pkg/kube/cluster/cluster.go b/pkg/kube/cluster/cluster.go
index 2e8e0d1dd3..ab54d0543a 100644
--- a/pkg/kube/cluster/cluster.go
+++ b/pkg/kube/cluster/cluster.go
@@ -3,9 +3,10 @@ package cluster
 import (
 	"strings"
 
-	"github.com/jenkins-x/jx/pkg/kube"
 	"github.com/pkg/errors"
 	"k8s.io/client-go/tools/clientcmd/api"
+
+	"github.com/jenkins-x/jx/pkg/kube"
 )
 
 // Name gets the cluster name from the current context
@@ -43,24 +44,7 @@ func ShortName(kuber kube.Kuber) (string, error) {
 	if err != nil {
 		return "", errors.Wrap(err, "retrieving the cluster name")
 	}
-	return ShortClusterName(clusterName), nil
-}
-
-// ShortClusterName shrinks the cluster name
-func ShortClusterName(clusterName string) string {
-	return ShortNameN(clusterName, 16)
-}
-
-// ShortNameN shrinks the name to a max length
-func ShortNameN(clusterName string, maxLen int) string {
-	shortClusterName := clusterName
-	if len(clusterName) > maxLen {
-		shortClusterName = clusterName[0:maxLen]
-	}
-	if strings.HasSuffix(shortClusterName, "_") || strings.HasSuffix(shortClusterName, "-") {
-		shortClusterName = shortClusterName[0 : len(shortClusterName)-1]
-	}
-	return shortClusterName
+	return kube.ToValidNameTruncated(clusterName, 16), nil
 }
 
 // SimplifiedClusterName get the simplified cluster name from the long-winded context cluster name that gets generated
diff --git a/pkg/kube/cluster/cluster_test.go b/pkg/kube/cluster/cluster_test.go
index 82ac6821d0..90ccd402fe 100644
--- a/pkg/kube/cluster/cluster_test.go
+++ b/pkg/kube/cluster/cluster_test.go
@@ -1,14 +1,15 @@
 package cluster_test
 
 import (
-	"github.com/jenkins-x/jx/pkg/kube/vault"
 	"testing"
 
-	"github.com/jenkins-x/jx/pkg/kube/cluster"
-	kube_test "github.com/jenkins-x/jx/pkg/kube/mocks"
 	. "github.com/petergtz/pegomock"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/client-go/tools/clientcmd/api"
+
+	"github.com/jenkins-x/jx/pkg/kube/cluster"
+	kube_test "github.com/jenkins-x/jx/pkg/kube/mocks"
+	"github.com/jenkins-x/jx/pkg/kube/vault"
 )
 
 func TestGetSimplifiedClusterName(t *testing.T) {
@@ -41,6 +42,9 @@ func TestShortClusterName(t *testing.T) {
 		},
 	}
 	When(kuber.LoadConfig()).ThenReturn(&config, nil, nil)
+
+	clusterName, err = cluster.ShortName(kuber)
+
 	assert.NoError(t, err)
 	assert.Equal(t, "short-cluster-na", clusterName)
 
@@ -51,6 +55,9 @@ func TestShortClusterName(t *testing.T) {
 		},
 	}
 	When(kuber.LoadConfig()).ThenReturn(&config, nil, nil)
+
+	clusterName, err = cluster.ShortName(kuber)
+
 	assert.NoError(t, err)
 	assert.Equal(t, "short-cluster-na", clusterName)
 }
diff --git a/pkg/kube/vault/vault.go b/pkg/kube/vault/vault.go
index 14d8798547..e65081071b 100644
--- a/pkg/kube/vault/vault.go
+++ b/pkg/kube/vault/vault.go
@@ -5,15 +5,16 @@ import (
 
 	"github.com/banzaicloud/bank-vaults/operator/pkg/apis/vault/v1alpha1"
 	"github.com/banzaicloud/bank-vaults/operator/pkg/client/clientset/versioned"
+	"github.com/pkg/errors"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
+
 	"github.com/jenkins-x/jx/pkg/kube"
 	"github.com/jenkins-x/jx/pkg/kube/cluster"
 	"github.com/jenkins-x/jx/pkg/kube/serviceaccount"
 	"github.com/jenkins-x/jx/pkg/kube/services"
 	"github.com/jenkins-x/jx/pkg/vault"
-	"github.com/pkg/errors"
-	v1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes"
 )
 
 const (
@@ -137,9 +138,9 @@ func SystemVaultName(kuber kube.Kuber) (string, error) {
 
 // SystemVaultNameForCluster returns the system vault name from a given cluster name
 func SystemVaultNameForCluster(clusterName string) string {
-	shortClusterName := cluster.ShortClusterName(clusterName)
+	shortClusterName := kube.ToValidNameTruncated(clusterName, 16)
 	fullName := fmt.Sprintf("%s-%s", vault.SystemVaultNamePrefix, shortClusterName)
-	return cluster.ShortNameN(fullName, 22)
+	return kube.ToValidNameTruncated(fullName, 22)
 }
 
 // CreateGKEVault creates a new vault backed by GCP KMS and storage