Gavinfrazar/v9 fix tctl auth server flag (#16260)

* fix tctl auth server flag (#16130)

* Fix flaky TestHandlerConnectionUpgrade

* Fix tctl --auth-server with logged in profile

* Export tctl applyConfig so we can test it from tsh/tctl tests

* Add test for tctl --auth-server w/ profile

* Update tool/tctl/common/tctl.go

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Update tool/tsh/tctl_test.go

Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Alan's suggestion to not use new()

* Add message to require.NotEmpty

Co-authored-by: STeve Huang <xin.huang@goteleport.com>
Co-authored-by: Alan Parra <alan.parra@goteleport.com>

* Change AuthAddr to AuthSSHAddr

* Fix test

Co-authored-by: STeve Huang <xin.huang@goteleport.com>
Co-authored-by: Alan Parra <alan.parra@goteleport.com>

tool/tctl/common/helpers_test.go

@@ -79,7 +79,7 @@ func runResourceCommand(t *testing.T, fc *config.FileConfig, args []string, opts
 	ccf.ConfigString = mustGetBase64EncFileConfig(t, fc)
 	ccf.Insecure = options.Insecure
 
-	clientConfig, err := applyConfig(&ccf, cfg)
+	clientConfig, err := ApplyConfig(&ccf, cfg)
 	require.NoError(t, err)
 
 	if options.CertPool != nil {
@@ -120,7 +120,7 @@ func runTokensCommand(t *testing.T, fc *config.FileConfig, args []string, opts .
 	ccf.ConfigString = mustGetBase64EncFileConfig(t, fc)
 	ccf.Insecure = options.Insecure
 
-	clientConfig, err := applyConfig(&ccf, cfg)
+	clientConfig, err := ApplyConfig(&ccf, cfg)
 	require.NoError(t, err)
 
 	if options.CertPool != nil {

tool/tctl/common/tctl.go

@@ -159,7 +159,7 @@ func Run(commands []CLICommand) {
 	}
 
 	// configure all commands with Teleport configuration (they share 'cfg')
-	clientConfig, err := applyConfig(&ccf, cfg)
+	clientConfig, err := ApplyConfig(&ccf, cfg)
 	if err != nil {
 		utils.FatalError(err)
 	}
@@ -190,12 +190,12 @@ func Run(commands []CLICommand) {
 	}
 }
 
-// applyConfig takes configuration values from the config file and applies them
+// ApplyConfig takes configuration values from the config file and applies them
 // to 'service.Config' object.
 //
 // The returned authclient.Config has the credentials needed to dial the auth
 // server.
-func applyConfig(ccf *GlobalCLIFlags, cfg *service.Config) (*authclient.Config, error) {
+func ApplyConfig(ccf *GlobalCLIFlags, cfg *service.Config) (*authclient.Config, error) {
 	// --debug flag
 	if ccf.Debug {
 		cfg.Debug = ccf.Debug
@@ -223,6 +223,20 @@ func applyConfig(ccf *GlobalCLIFlags, cfg *service.Config) (*authclient.Config,
 		}
 	}
 
+	if err = config.ApplyFileConfig(fileConf, cfg); err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// --auth-server flag(-s)
+	if len(ccf.AuthServerAddr) != 0 {
+		addrs, err := utils.ParseAddrs(ccf.AuthServerAddr)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+		// Overwrite any existing configuration with flag values.
+		cfg.AuthServers = addrs
+	}
+
 	// Config file should take precedence, if available.
 	if fileConf == nil && ccf.IdentityFilePath == "" {
 		// No config file or identity file.
@@ -236,17 +250,7 @@ func applyConfig(ccf *GlobalCLIFlags, cfg *service.Config) (*authclient.Config,
 			return nil, trace.Wrap(err)
 		}
 	}
-	if err = config.ApplyFileConfig(fileConf, cfg); err != nil {
-		return nil, trace.Wrap(err)
-	}
 
-	// --auth-server flag(-s)
-	if len(ccf.AuthServerAddr) != 0 {
-		cfg.AuthServers, err = utils.ParseAddrs(ccf.AuthServerAddr)
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
-	}
 	// If auth server is not provided on the command line or in file
 	// configuration, use the default.
 	if len(cfg.AuthServers) == 0 {

tool/tsh/tctl_test.go

@@ -18,13 +18,15 @@ package main
 
 import (
 	"context"
+	"path/filepath"
 	"testing"
 
 	"github.com/gravitational/trace"
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/service"
+	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/tool/tctl/common"
 )
 
@@ -94,3 +96,81 @@ func TestLoadConfigFromProfile(t *testing.T) {
 		})
 	}
 }
+
+func TestSetAuthServerFlagWhileLoggedIn(t *testing.T) {
+	tmpHomePath := filepath.Clean(t.TempDir())
+	connector := mockConnector(t)
+
+	alice, err := types.NewUser("alice@example.com")
+	require.NoError(t, err)
+	alice.SetRoles([]string{"access"})
+
+	authProcess, proxyProcess := makeTestServers(t, withBootstrap(connector, alice))
+
+	authServer := authProcess.GetAuthServer()
+	require.NotNil(t, authServer)
+
+	authAddr, err := authProcess.AuthSSHAddr()
+	require.NoError(t, err)
+
+	proxyAddr, err := proxyProcess.ProxyWebAddr()
+	require.NoError(t, err)
+
+	err = Run(context.Background(), []string{
+		"login",
+		"--insecure",
+		"--debug",
+		"--auth", connector.GetName(),
+		"--proxy", proxyAddr.String(),
+	}, setHomePath(tmpHomePath), cliOption(func(cf *CLIConf) error {
+		cf.mockSSOLogin = mockSSOLogin(t, authServer, alice)
+		return nil
+	}))
+	require.NoError(t, err)
+	// we're now logged in with a profile in tmpHomePath.
+
+	tests := []struct {
+		desc           string
+		authServerFlag []string
+		want           []utils.NetAddr
+	}{
+		{
+			desc: "sets default web proxy addr without auth server flag",
+			want: []utils.NetAddr{*proxyAddr},
+		},
+		{
+			desc:           "sets auth addr from auth server flag ignoring profile setting",
+			authServerFlag: []string{authAddr.String()},
+			want:           []utils.NetAddr{*authAddr},
+		},
+		{
+			desc:           "sets auth addr from auth server flag when profile is not found",
+			authServerFlag: []string{"site.not.in.profile.com:3080"},
+			want: []utils.NetAddr{
+				{
+					Addr:        "site.not.in.profile.com:3080",
+					AddrNetwork: "tcp",
+					Path:        "",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			ccf := &common.GlobalCLIFlags{}
+			ccf.AuthServerAddr = tt.authServerFlag
+
+			cfg := &service.Config{}
+			cfg.TeleportHome = tmpHomePath
+			// this is needed for the case where the --auth-server=host is not found in profile.
+			// ApplyConfig will try to read local auth server identity if the profile is not found.
+			cfg.DataDir = authProcess.Config.DataDir
+
+			_, err = common.ApplyConfig(ccf, cfg)
+			require.NoError(t, err)
+			require.NotEmpty(t, cfg.AuthServers, "auth servers should be set to a non-empty default if not specified")
+
+			require.ElementsMatch(t, tt.want, cfg.AuthServers)
+		})
+	}
+}