From 66f5b5510c395d369d2dd200a165a4f7edf0b778 Mon Sep 17 00:00:00 2001 From: "STeve (Xin) Huang" Date: Mon, 12 Jun 2023 09:49:09 -0400 Subject: [PATCH] Fix an issue kube local proxy requirement is wrong in separate port mode (#27634) * Fix an issue kube local proxy requirement is wrong in separate port mode * move kube local proxy requirement check to api/profile --- api/profile/profile.go | 6 ++++ api/profile/profile_test.go | 55 +++++++++++++++++++++++++++++++++++++ lib/client/api.go | 15 ++++++---- tool/tsh/common/kube.go | 16 ++++------- tool/tsh/common/kubectl.go | 8 ++---- 5 files changed, 79 insertions(+), 21 deletions(-) diff --git a/api/profile/profile.go b/api/profile/profile.go index a0564ef30dd..abc8f45d16f 100644 --- a/api/profile/profile.go +++ b/api/profile/profile.go @@ -142,6 +142,12 @@ func (p *Profile) TLSConfig() (*tls.Config, error) { }, nil } +// RequireKubeLocalProxy returns true if this profile indicates a local proxy +// is required for kube access. +func (p *Profile) RequireKubeLocalProxy() bool { + return p.KubeProxyAddr == p.WebProxyAddr && p.TLSRoutingConnUpgradeRequired +} + func certPoolFromProfile(p *Profile) (*x509.CertPool, error) { // Check if CAS dir exist if not try to load certs from legacy certs.pem file. if _, err := os.Stat(p.TLSClusterCASDir()); err != nil { diff --git a/api/profile/profile_test.go b/api/profile/profile_test.go index 663e6fd9eaf..5ef6181de23 100644 --- a/api/profile/profile_test.go +++ b/api/profile/profile_test.go @@ -115,3 +115,58 @@ func TestProfilePath(t *testing.T) { require.Equal(t, "/foo/bar", profile.FullProfilePath("/foo/bar")) require.Equal(t, filepath.Join(dir, ".tsh"), profile.FullProfilePath("")) } + +func TestRequireKubeLocalProxy(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputProfile *profile.Profile + checkResult require.BoolAssertionFunc + }{ + { + name: "kube not enabled", + inputProfile: &profile.Profile{ + WebProxyAddr: "example.com:443", + TLSRoutingEnabled: true, + TLSRoutingConnUpgradeRequired: true, + }, + checkResult: require.False, + }, + { + name: "ALPN connection upgrade not required", + inputProfile: &profile.Profile{ + WebProxyAddr: "example.com:443", + KubeProxyAddr: "example.com:443", + TLSRoutingEnabled: true, + }, + checkResult: require.False, + }, + { + name: "kube uses separate listener", + inputProfile: &profile.Profile{ + WebProxyAddr: "example.com:443", + KubeProxyAddr: "example.com:3026", + TLSRoutingEnabled: false, + TLSRoutingConnUpgradeRequired: true, + }, + checkResult: require.False, + }, + { + name: "local proxy required", + inputProfile: &profile.Profile{ + WebProxyAddr: "example.com:443", + KubeProxyAddr: "example.com:443", + TLSRoutingEnabled: true, + TLSRoutingConnUpgradeRequired: true, + }, + checkResult: require.True, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.checkResult(t, test.inputProfile.RequireKubeLocalProxy()) + }) + } +} diff --git a/lib/client/api.go b/lib/client/api.go index 54808d5a1f8..af5e9b0eac8 100644 --- a/lib/client/api.go +++ b/lib/client/api.go @@ -674,7 +674,15 @@ func (c *Config) SaveProfile(makeCurrent bool) error { return nil } - p := &profile.Profile{ + if err := c.ClientStore.SaveProfile(c.Profile(), makeCurrent); err != nil { + return trace.Wrap(err) + } + return nil +} + +// Profile converts Config to *profile.Profile. +func (c *Config) Profile() *profile.Profile { + return &profile.Profile{ Username: c.Username, WebProxyAddr: c.WebProxyAddr, SSHProxyAddr: c.SSHProxyAddr, @@ -690,11 +698,6 @@ func (c *Config) SaveProfile(makeCurrent bool) error { LoadAllCAs: c.LoadAllCAs, PrivateKeyPolicy: c.PrivateKeyPolicy, } - - if err := c.ClientStore.SaveProfile(p, makeCurrent); err != nil { - return trace.Wrap(err) - } - return nil } // ParsedProxyHost holds the hostname and Web & SSH proxy addresses diff --git a/tool/tsh/common/kube.go b/tool/tsh/common/kube.go index f626ae7cb3f..7761984c06f 100644 --- a/tool/tsh/common/kube.go +++ b/tool/tsh/common/kube.go @@ -648,7 +648,7 @@ func (c *kubeCredentialsCommand) run(cf *CLIConf) error { return trace.Wrap(c.issueCert(cf)) } - if err := c.checkLocalProxyRequirement(profile.TLSRoutingConnUpgradeRequired); err != nil { + if err := c.checkLocalProxyRequirement(profile); err != nil { return trace.Wrap(err) } @@ -684,7 +684,7 @@ func (c *kubeCredentialsCommand) issueCert(cf *CLIConf) error { if err != nil { return trace.Wrap(err) } - if err := c.checkLocalProxyRequirement(tc.TLSRoutingConnUpgradeRequired); err != nil { + if err := c.checkLocalProxyRequirement(tc.Profile()); err != nil { return trace.Wrap(err) } @@ -729,7 +729,7 @@ func (c *kubeCredentialsCommand) issueCert(cf *CLIConf) error { 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 { + if err := c.checkLocalProxyRequirement(tc.Profile()); err != nil { return trace.Wrap(err) } @@ -773,8 +773,8 @@ func isNetworkError(err error) bool { return errors.As(err, &opErr) || trace.IsConnectionProblem(err) } -func (c *kubeCredentialsCommand) checkLocalProxyRequirement(connUpgradeRequired bool) error { - if connUpgradeRequired { +func (c *kubeCredentialsCommand) checkLocalProxyRequirement(profile *profile.Profile) error { + if profile.RequireKubeLocalProxy() { return trace.BadParameter("Cannot connect Kubernetes clients to Teleport Proxy directly. Please use `tsh proxy kube` or `tsh kubectl` instead.") } return nil @@ -1183,7 +1183,7 @@ func (c *kubeLoginCommand) run(cf *CLIConf) error { } func (c *kubeLoginCommand) printUserMessage(cf *CLIConf, tc *client.TeleportClient) { - if c.localProxyRequired(tc) { + if tc.Profile().RequireKubeLocalProxy() { c.printLocalProxyUserMessage(cf) return } @@ -1213,10 +1213,6 @@ Use the kubeconfig provided by the local proxy, select a context, and try 'kubec } } -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) { err = client.RetryWithRelogin(ctx, tc, func() error { pc, err := tc.ConnectToProxy(ctx) diff --git a/tool/tsh/common/kubectl.go b/tool/tsh/common/kubectl.go index 6fa2f3e2f61..a3437a7017c 100644 --- a/tool/tsh/common/kubectl.go +++ b/tool/tsh/common/kubectl.go @@ -453,11 +453,11 @@ func makeAndStartKubeLocalProxy(cf *CLIConf, config *clientcmdapi.Config, cluste } // shouldUseKubeLocalProxy checks if a local proxy is required for kube -// access. +// access for `tsh kubectl` or `tsh kube exec`. // // The local proxy is required when all of these conditions are met: // - profile is loadable -// - kube access is enabled +// - kube access is enabled, and is accessed through web proxy address // - ALPN connection upgrade is required (e.g. Proxy behind ALB) // - not `kubectl config` commands // - original/default kubeconfig is loadable @@ -470,9 +470,7 @@ func shouldUseKubeLocalProxy(cf *CLIConf, kubectlArgs []string) (*clientcmdapi.C return nil, nil, false } - // Only use local proxy when Kube is enabled and ALPN connection upgrade is - // required. - if !profile.TLSRoutingConnUpgradeRequired || profile.KubeProxyAddr == "" { + if !profile.RequireKubeLocalProxy() { return nil, nil, false }