TLS Routing behind ALB: tsh kube subcommands UX (#26305) (#27155)

* TLS Routing behind ALB: tsh kube subcommands

* fix lint and ut

* fix ut again

* review comment round 1

* fix an issue where local proxy should be not recreated on top a local proxy kubeconfig

* move maybeStartKubeLocalProxy to runKubectlReexec, remove os.Setenv

* use cf.Stdout

* revert kube util.pointer

* revert kube util.pointer

* Fix fullArgs vs args and remove old KUBECONFIG from env

go.mod

@@ -126,6 +126,7 @@ require (
 	github.com/sethvargo/go-diceware v0.3.0
 	github.com/sirupsen/logrus v1.9.0
 	github.com/snowflakedb/gosnowflake v1.6.19
+	github.com/spf13/cobra v1.6.1
 	github.com/stretchr/testify v1.8.2
 	github.com/ucarion/urlpath v0.0.0-20200424170820-7ccc79b76bbb
 	github.com/vulcand/predicate v1.2.0 // replaced
@@ -345,7 +346,6 @@ require (
 	github.com/shabbyrobe/gocovmerge v0.0.0-20190829150210-3e036491d500 // indirect
 	github.com/siddontang/go v0.0.0-20180604090527-bdc77568d726 // indirect
 	github.com/siddontang/go-log v0.0.0-20180807004314-8d05993dda07 // indirect
-	github.com/spf13/cobra v1.6.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/thales-e-security/pool v0.0.2 // indirect
 	github.com/tidwall/pretty v1.2.1 // indirect

lib/client/client_store.go

@@ -226,33 +226,10 @@ func (s *Store) FullProfileStatus() (*ProfileStatus, []*ProfileStatus, error) {
 // when the store has a lot of keys and when we call the function multiple times in
 // parallel.
 // Although this function speeds up the process since it removes all transversals,
-// it still has to read 3 different files if the proxy is specified, otherwise it also
-// has to read $TSH_HOME/current_profile.
-// - $TSH_HOME/$profile.yaml
+// it still has to read 2 different files:
 // - $TSH_HOME/keys/$PROXY/$USER-kube/$TELEPORT_CLUSTER/$KUBE_CLUSTER-x509.pem
 // - $TSH_HOME/keys/$PROXY/$USER
-func LoadKeysToKubeFromStore(dirPath, proxy, teleportCluster, kubeCluster string) ([]byte, []byte, error) {
-	dirPath = profile.FullProfilePath(dirPath)
-
-	profileStore := NewFSProfileStore(dirPath)
-	// tsh stores the profiles using the proxy host as the profile name.
-	profileName, err := utils.Host(proxy)
-	if err != nil {
-		return nil, nil, trace.Wrap(err)
-	}
-	if profileName == "" {
-		// If no profile name is provided, default to the current profile.
-		profileName, err = profileStore.CurrentProfile()
-		if err != nil {
-			return nil, nil, trace.Wrap(err)
-		}
-	}
-	// Load the desired profile.
-	profile, err := profileStore.GetProfile(profileName)
-	if err != nil {
-		return nil, nil, trace.Wrap(err)
-	}
-
+func LoadKeysToKubeFromStore(profile *profile.Profile, dirPath, teleportCluster, kubeCluster string) ([]byte, []byte, error) {
 	fsKeyStore := NewFSKeyStore(dirPath)
 
 	certPath := fsKeyStore.kubeCertPath(KeyIndex{ProxyHost: profile.SiteName, ClusterName: teleportCluster, Username: profile.Username}, kubeCluster)

lib/client/profile.go

@@ -32,6 +32,7 @@ import (
 	"github.com/gravitational/teleport/api/utils/keypaths"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/tlsca"
+	"github.com/gravitational/teleport/lib/utils"
 )
 
 // ProfileStore is a storage interface for client profile data.
@@ -546,3 +547,15 @@ func (p *ProfileStatus) AppNames() (result []string) {
 	}
 	return result
 }
+
+// ProfileNameFromProxyAddress converts proxy address to profile name or
+// returns the current profile if the proxyAddr is not set.
+func ProfileNameFromProxyAddress(store ProfileStore, proxyAddr string) (string, error) {
+	if proxyAddr == "" {
+		profileName, err := store.CurrentProfile()
+		return profileName, trace.Wrap(err)
+	}
+
+	profileName, err := utils.Host(proxyAddr)
+	return profileName, trace.Wrap(err)
+}

lib/client/profile_test.go

@@ -84,3 +84,29 @@ func TestProfileStore(t *testing.T) {
 		require.ElementsMatch(t, profiles, retProfiles)
 	})
 }
+
+func TestProfileNameFromProxyAddress(t *testing.T) {
+	t.Parallel()
+
+	store := NewMemProfileStore()
+	require.NoError(t, store.SaveProfile(&profile.Profile{
+		WebProxyAddr: "proxy1.example.com:443",
+		Username:     "test-user",
+		SiteName:     "root",
+	}, true))
+
+	t.Run("current profile", func(t *testing.T) {
+		profileName, err := ProfileNameFromProxyAddress(store, "")
+		require.NoError(t, err)
+		require.Equal(t, "proxy1.example.com", profileName)
+	})
+	t.Run("proxy host", func(t *testing.T) {
+		profileName, err := ProfileNameFromProxyAddress(store, "proxy2.example.com:443")
+		require.NoError(t, err)
+		require.Equal(t, "proxy2.example.com", profileName)
+	})
+	t.Run("invalid proxy address", func(t *testing.T) {
+		_, err := ProfileNameFromProxyAddress(store, ":443")
+		require.Error(t, err)
+	})
+}

lib/kube/kubeconfig/localproxy.go

@@ -151,3 +151,36 @@ func LocalProxyClustersFromDefaultConfig(defaultConfig *clientcmdapi.Config, clu
 	}
 	return clusters
 }
+
+// FindTeleportClusterForLocalProxy finds the Teleport kube cluster based on
+// provided cluster address and context name, and prepares a LocalProxyCluster.
+//
+// When the cluster has a ProxyURL set, it means the provided kubeconfig is
+// already pointing to a local proxy through this ProxyURL and thus can be
+// skipped as there is no need to create a new local proxy.
+func FindTeleportClusterForLocalProxy(defaultConfig *clientcmdapi.Config, clusterAddr, contextName string) (LocalProxyCluster, bool) {
+	if contextName == "" {
+		contextName = defaultConfig.CurrentContext
+	}
+
+	context, found := defaultConfig.Contexts[contextName]
+	if !found {
+		return LocalProxyCluster{}, false
+	}
+	cluster, found := defaultConfig.Clusters[context.Cluster]
+	if !found || cluster.Server != clusterAddr || cluster.ProxyURL != "" {
+		return LocalProxyCluster{}, false
+	}
+	auth, found := defaultConfig.AuthInfos[context.AuthInfo]
+	if !found {
+		return LocalProxyCluster{}, false
+	}
+
+	return LocalProxyCluster{
+		TeleportCluster:   context.Cluster,
+		KubeCluster:       KubeClusterFromContext(contextName, context.Cluster),
+		Namespace:         context.Namespace,
+		Impersonate:       auth.Impersonate,
+		ImpersonateGroups: auth.ImpersonateGroups,
+	}, true
+}

lib/kube/kubeconfig/localproxy_test.go

@@ -50,6 +50,7 @@ func TestLocalProxy(t *testing.T) {
 		KubeClusters:        []string{"kube2"},
 		Credentials:         creds,
 		Exec:                exec,
+		SelectCluster:       "kube2",
 	}, false))
 	require.NoError(t, Update(kubeconfigPath, Values{
 		TeleportClusterName: leafClusterName,
@@ -86,6 +87,63 @@ func TestLocalProxy(t *testing.T) {
 		}, clusters)
 	})
 
+	t.Run("FindTeleportClusterForLocalProxy", func(t *testing.T) {
+		inputConfig := configAfterLogins.DeepCopy()
+
+		// Simulate a scenario that kube3 is already pointing to a local proxy
+		// through ProxyURL.
+		inputConfig.Clusters[leafClusterName].ProxyURL = "https://localhost:8443"
+
+		tests := []struct {
+			name          string
+			selectContext string
+			checkResult   require.BoolAssertionFunc
+			wantCluster   LocalProxyCluster
+		}{
+			{
+				name:          "not Teleport cluster",
+				selectContext: "dev",
+				checkResult:   require.False,
+			},
+			{
+				name:          "context not found",
+				selectContext: "not-found",
+				checkResult:   require.False,
+			},
+			{
+				name:          "find Teleport cluster by context name",
+				selectContext: rootClusterName + "-kube1",
+				checkResult:   require.True,
+				wantCluster: LocalProxyCluster{
+					TeleportCluster: rootClusterName,
+					KubeCluster:     "kube1",
+				},
+			},
+			{
+				name:          "find Teleport cluster by current context",
+				selectContext: "",
+				checkResult:   require.True,
+				wantCluster: LocalProxyCluster{
+					TeleportCluster: rootClusterName,
+					KubeCluster:     "kube2",
+				},
+			},
+			{
+				name:          "skip local proxy config",
+				selectContext: leafClusterName + "-kube3",
+				checkResult:   require.False,
+			},
+		}
+
+		for _, test := range tests {
+			t.Run(test.name, func(t *testing.T) {
+				cluster, found := FindTeleportClusterForLocalProxy(inputConfig, rootKubeClusterAddr, test.selectContext)
+				test.checkResult(t, found)
+				require.Equal(t, test.wantCluster, cluster)
+			})
+		}
+	})
+
 	t.Run("CreateLocalProxyConfig", func(t *testing.T) {
 		caData := []byte("CAData")
 		clientKeyData := []byte("clientKeyData")

tool/tsh/kube.go

@@ -437,17 +437,19 @@ func newKubeExecCommand(parent *kingpin.CmdClause) *kubeExecCommand {
 }
 
 func (c *kubeExecCommand) run(cf *CLIConf) error {
-	var p ExecOptions
-	var err error
+	closeFn, newKubeConfigLocation, err := maybeStartKubeLocalProxy(cf)
+	if err != nil {
+		return trace.Wrap(err)
+	}
+	defer closeFn()
 
+	f := c.kubeCmdFactory(newKubeConfigLocation)
+	var p ExecOptions
 	p.IOStreams = genericclioptions.IOStreams{
 		In:     os.Stdin,
 		Out:    os.Stdout,
 		ErrOut: os.Stderr,
 	}
-	kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag()
-	matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags)
-	f := cmdutil.NewFactory(matchVersionKubeConfigFlags)
 	p.ResourceName = c.target
 	p.ContainerName = c.container
 	p.Quiet = c.quiet
@@ -481,6 +483,17 @@ func (c *kubeExecCommand) run(cf *CLIConf) error {
 	return trace.Wrap(p.Run(cf.Context))
 }
 
+func (c *kubeExecCommand) kubeCmdFactory(overwriteKubeConfigLocation string) cmdutil.Factory {
+	kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag()
+
+	if overwriteKubeConfigLocation != "" {
+		kubeConfigFlags.KubeConfig = &overwriteKubeConfigLocation
+	}
+
+	matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags)
+	return cmdutil.NewFactory(matchVersionKubeConfigFlags)
+}
+
 type kubeSessionsCommand struct {
 	*kingpin.CmdClause
 	format   string
@@ -582,18 +595,30 @@ func newKubeCredentialsCommand(parent *kingpin.CmdClause) *kubeCredentialsComman
 }
 
 func (c *kubeCredentialsCommand) run(cf *CLIConf) error {
+	profile, err := cf.GetProfile()
+	if err != nil {
+		// Cannot find the profile, continue to c.issueCert for a login.
+		return trace.Wrap(c.issueCert(cf))
+	}
+
+	if err := c.checkLocalProxyRequirement(profile.TLSRoutingConnUpgradeRequired); err != nil {
+		return trace.Wrap(err)
+	}
+
 	// client.LoadKeysToKubeFromStore function is used to speed up the credentials
 	// loading process since Teleport Store transverses the entire store to find the keys.
 	// This operation takes a long time when the store has a lot of keys and when
 	// we call the function multiple times in parallel.
 	// Although client.LoadKeysToKubeFromStore function speeds up the process since
-	// it removes all transversals, it still has to read 3 different files from the disk:
-	// - $TSH_HOME/$profile.yaml
+	// it removes all transversals, it still has to read 2 different files from the disk:
 	// - $TSH_HOME/keys/$PROXY/$USER-kube/$TELEPORT_CLUSTER/$KUBE_CLUSTER-x509.pem
 	// - $TSH_HOME/keys/$PROXY/$USER
+	//
+	// In addition to these files, $TSH_HOME/$profile.yaml is also read from
+	// cf.GetProfile call above.
 	if kubeCert, privKey, err := client.LoadKeysToKubeFromStore(
+		profile,
 		cf.HomePath,
-		cf.Proxy,
 		c.teleportCluster,
 		c.kubeCluster,
 	); err == nil {
@@ -604,10 +629,17 @@ func (c *kubeCredentialsCommand) run(cf *CLIConf) error {
 		}
 	}
 
+	return trace.Wrap(c.issueCert(cf))
+}
+
+func (c *kubeCredentialsCommand) issueCert(cf *CLIConf) error {
 	tc, err := makeClient(cf)
 	if err != nil {
 		return trace.Wrap(err)
 	}
+	if err := c.checkLocalProxyRequirement(tc.TLSRoutingConnUpgradeRequired); err != nil {
+		return trace.Wrap(err)
+	}
 
 	_, span := tc.Tracer.Start(cf.Context, "tsh.kubeCredentials/GetKey")
 	// Try loading existing keys.
@@ -638,6 +670,12 @@ func (c *kubeCredentialsCommand) run(cf *CLIConf) error {
 
 	ctx, span := tc.Tracer.Start(cf.Context, "tsh.kubeCredentials/RetryWithRelogin")
 	err = client.RetryWithRelogin(ctx, tc, func() error {
+		// The requirement may change after a new login so check again just in
+		// case.
+		if err := c.checkLocalProxyRequirement(tc.TLSRoutingConnUpgradeRequired); err != nil {
+			return trace.Wrap(err)
+		}
+
 		var err error
 		k, err = tc.IssueUserCertsWithMFA(ctx, client.ReissueParams{
 			RouteToCluster:    c.teleportCluster,
@@ -665,6 +703,13 @@ func (c *kubeCredentialsCommand) run(cf *CLIConf) error {
 	return c.writeKeyResponse(cf.Stdout(), k, c.kubeCluster)
 }
 
+func (c *kubeCredentialsCommand) checkLocalProxyRequirement(connUpgradeRequired bool) error {
+	if connUpgradeRequired {
+		return trace.BadParameter("Cannot connect Kubernetes clients to Teleport Proxy directly. Please use `tsh proxy kube` or `tsh kubectl` instead.")
+	}
+	return nil
+}
+
 // checkIfCertsAreAllowedToAccessCluster evaluates if the new cert created by the user
 // to access kubeCluster has at least one kubernetes_user or kubernetes_group
 // defined. If not, it returns an error.
@@ -1062,12 +1107,44 @@ func (c *kubeLoginCommand) run(cf *CLIConf) error {
 	if err := updateKubeConfig(cf, tc, profileKubeconfigPath, c.overrideContextName); err != nil {
 		return trace.Wrap(err)
 	}
+
+	c.printUserMessage(cf, tc)
+	return nil
+}
+
+func (c *kubeLoginCommand) printUserMessage(cf *CLIConf, tc *client.TeleportClient) {
+	if c.localProxyRequired(tc) {
+		c.printLocalProxyUserMessage(cf)
+		return
+	}
+
 	if c.kubeCluster != "" {
-		fmt.Printf("Logged into Kubernetes cluster %q. Try 'kubectl version' to test the connection.\n", c.kubeCluster)
+		fmt.Fprintf(cf.Stdout(), "Logged into Kubernetes cluster %q. Try 'kubectl version' to test the connection.\n", c.kubeCluster)
 	} else {
-		fmt.Printf("Created kubeconfig with every Kubernetes cluster available. Select a context and try 'kubectl version' to test the connection.\n")
+		fmt.Fprintf(cf.Stdout(), "Created kubeconfig with every Kubernetes cluster available. Select a context and try 'kubectl version' to test the connection.\n")
 	}
-	return nil
+}
+
+func (c *kubeLoginCommand) printLocalProxyUserMessage(cf *CLIConf) {
+	switch {
+	case c.kubeCluster != "":
+		fmt.Fprintf(cf.Stdout(), `Logged into Kubernetes cluster %q. Start the local proxy:
+  tsh proxy kube -p 8443
+
+Use the kubeconfig provided by the local proxy, and try 'kubectl version' to test the connection.
+`, c.kubeCluster)
+
+	default:
+		fmt.Fprintf(cf.Stdout(), `Logged into all Kubernetes clusters available. Start the local proxy:
+  tsh proxy kube -p 8443
+
+Use the kubeconfig provided by the local proxy, select a context, and try 'kubectl version' to test the connection.
+`)
+	}
+}
+
+func (c *kubeLoginCommand) localProxyRequired(tc *client.TeleportClient) bool {
+	return tc.TLSRoutingConnUpgradeRequired
 }
 
 func fetchKubeClusters(ctx context.Context, tc *client.TeleportClient) (teleportCluster string, kubeClusters []types.KubeCluster, err error) {