[v15] improve tctl config loading ux

docs/pages/reference/cli/tctl.mdx

@@ -34,7 +34,10 @@ If there is a Teleport configuration file on the host where `tctl` is run,
 `tctl` attempts to authenticate to the Auth Service named in the configuration
 file using an identity stored in its local backend.
 
-`tctl` only authenticates using this method if an identity file is not provided.
+`tctl` authenticates using this method if a configuration file exists at
+`/etc/teleport.yaml` or `TELEPORT_CONFIG_FILE` points to a configuration file
+in another location. If the `auth_service` is disabled in the configuration
+file, then the configuration file is ignored.
 
 <Admonition type="note">
 
@@ -58,13 +61,6 @@ the cluster. The `tsh` profile is created when a user runs `tsh login`.
 a Teleport configuration file is present. If you are using your `tsh` profile to
 authenticate `tctl`, you must ensure that one of these conditions is true:
 
-- `TELEPORT_CONFIG_FILE` is blank
-- No file exists at `/etc/teleport.yaml`
-
-Otherwise `tctl` will attempt to connect to a Teleport cluster on the machine,
-which could result in the error,
-`ERROR: open /var/lib/teleport/host_uuid: permission denied`.
-
   </TabItem>
   <TabItem scope={["cloud", "team"]} label="Cloud-Hosted">
 

tool/tctl/common/tctl.go

@@ -148,11 +148,11 @@ func TryRun(commands []CLICommand, args []string) error {
 	app.Flag("debug", "Enable verbose logging to stderr").
 		Short('d').
 		BoolVar(&ccf.Debug)
-	app.Flag("config", fmt.Sprintf("Path to a configuration file [%v]. Can also be set via the %v environment variable.", defaults.ConfigFilePath, defaults.ConfigFileEnvar)).
+	app.Flag("config", fmt.Sprintf("Path to a configuration file [%v] for an Auth Service instance. Can also be set via the %v environment variable. Ignored if the auth_service is disabled.", defaults.ConfigFilePath, defaults.ConfigFileEnvar)).
 		Short('c').
 		ExistingFileVar(&ccf.ConfigFile)
 	app.Flag("config-string",
-		"Base64 encoded configuration string").Hidden().Envar(defaults.ConfigEnvar).StringVar(&ccf.ConfigString)
+		"Base64 encoded configuration string. Ignored if the config auth_service is disabled.").Hidden().Envar(defaults.ConfigEnvar).StringVar(&ccf.ConfigString)
 	app.Flag("auth-server",
 		fmt.Sprintf("Attempts to connect to specific auth/proxy address(es) instead of local auth [%v]", defaults.AuthConnectAddr().Addr)).
 		Envar(authAddrEnvVar).
@@ -311,8 +311,17 @@ func ApplyConfig(ccf *GlobalCLIFlags, cfg *servicecfg.Config) (*authclient.Confi
 		}
 	}
 
-	if err = config.ApplyFileConfig(fileConf, cfg); err != nil {
-		return nil, trace.Wrap(err)
+	// It only makes sense to use file config when tctl is run on the same
+	// host as the auth server.
+	// If this is any other host, then it's remote tctl usage.
+	// Remote tctl usage will require ~/.tsh or an identity file.
+	// ~/.tsh which will provide credentials AND config to reach auth server.
+	// Identity file requires --auth-server flag.
+	localAuthSvcConf := fileConf != nil && fileConf.Auth.Enabled()
+	if localAuthSvcConf {
+		if err = config.ApplyFileConfig(fileConf, cfg); err != nil {
+			return nil, trace.Wrap(err)
+		}
 	}
 
 	// --auth-server flag(-s)
@@ -327,10 +336,14 @@ func ApplyConfig(ccf *GlobalCLIFlags, cfg *servicecfg.Config) (*authclient.Confi
 		}
 	}
 
-	// Config file should take precedence, if available.
-	if fileConf == nil {
-		// No config file. Try profile or identity file.
-		log.Debug("No config file or identity file, loading auth config via extension.")
+	// Config file (for an auth_service) should take precedence.
+	if !localAuthSvcConf {
+		// Try profile or identity file.
+		if fileConf == nil {
+			log.Debug("no config file, loading auth config via extension")
+		} else {
+			log.Debug("auth_service disabled in config file, loading auth config via extension")
+		}
 		authConfig, err := LoadConfigFromProfile(ccf, cfg)
 		if err == nil {
 			return authConfig, nil
@@ -366,11 +379,11 @@ func ApplyConfig(ccf *GlobalCLIFlags, cfg *servicecfg.Config) (*authclient.Confi
 	if err != nil {
 		if errors.Is(err, fs.ErrNotExist) {
 			return nil, trace.Wrap(err, "Could not load Teleport host UUID file at %s. "+
-				"Please make sure that Teleport is up and running prior to using tctl.",
+				"Please make sure that a Teleport Auth Service instance is running on this host prior to using tctl or provide credentials by logging in with tsh first.",
 				filepath.Join(cfg.DataDir, utils.HostUUIDFile))
 		} else if errors.Is(err, fs.ErrPermission) {
 			return nil, trace.Wrap(err, "Teleport does not have permission to read Teleport host UUID file at %s. "+
-				"Ensure that you are running as a user with appropriate permissions.",
+				"Ensure that you are running as a user with appropriate permissions or provide credentials by logging in with tsh first.",
 				filepath.Join(cfg.DataDir, utils.HostUUIDFile))
 		}
 		return nil, trace.Wrap(err)
@@ -380,7 +393,7 @@ func ApplyConfig(ccf *GlobalCLIFlags, cfg *servicecfg.Config) (*authclient.Confi
 		// The "admin" identity is not present? This means the tctl is running
 		// NOT on the auth server
 		if trace.IsNotFound(err) {
-			return nil, trace.AccessDenied("tctl must be either used on the auth server or provided with the identity file via --identity flag")
+			return nil, trace.AccessDenied("tctl must be used on an Auth Service host or provided with credentials by logging in with tsh first.")
 		}
 		return nil, trace.Wrap(err)
 	}

tool/tctl/common/tctl_test.go

@@ -50,14 +50,32 @@ func TestConnect(t *testing.T) {
 	}
 	process := makeAndRunTestAuthServer(t, withFileConfig(fileConfig), withFileDescriptors(dynAddr.Descriptors))
 	clt := testenv.MakeDefaultAuthClient(t, process)
+	fileConfigAgent := &config.FileConfig{
+		Global: config.Global{
+			DataDir: t.TempDir(),
+		},
+		Auth: config.Auth{
+			Service: config.Service{
+				EnabledFlag:   "false",
+				ListenAddress: dynAddr.AuthAddr,
+			},
+		},
+		SSH: config.SSH{
+			Service: config.Service{
+				EnabledFlag:   "true",
+				ListenAddress: dynAddr.NodeSSHAddr,
+			},
+		},
+	}
 
 	username := "admin"
 	mustAddUser(t, clt, "admin", "access")
 
 	for _, tc := range []struct {
-		name         string
-		cliFlags     GlobalCLIFlags
-		modifyConfig func(*servicecfg.Config)
+		name            string
+		cliFlags        GlobalCLIFlags
+		modifyConfig    func(*servicecfg.Config)
+		wantErrContains string
 	}{
 		{
 			name: "default to data dir",
@@ -69,17 +87,47 @@ func TestConnect(t *testing.T) {
 				cfg.DataDir = fileConfig.DataDir
 			},
 		}, {
-			name: "config file",
+			name: "auth config file",
 			cliFlags: GlobalCLIFlags{
 				ConfigFile: mustWriteFileConfig(t, fileConfig),
 				Insecure:   true,
 			},
 		}, {
-			name: "config file string",
+			name: "auth config file string",
 			cliFlags: GlobalCLIFlags{
 				ConfigString: mustGetBase64EncFileConfig(t, fileConfig),
 				Insecure:     true,
 			},
+		}, {
+			name: "ignores agent config file",
+			cliFlags: GlobalCLIFlags{
+				ConfigFile: mustWriteFileConfig(t, fileConfigAgent),
+				Insecure:   true,
+			},
+			wantErrContains: "make sure that a Teleport Auth Service instance is running",
+		}, {
+			name: "ignores agent config file string",
+			cliFlags: GlobalCLIFlags{
+				ConfigString: mustGetBase64EncFileConfig(t, fileConfigAgent),
+				Insecure:     true,
+			},
+			wantErrContains: "make sure that a Teleport Auth Service instance is running",
+		}, {
+			name: "ignores agent config file and loads identity file",
+			cliFlags: GlobalCLIFlags{
+				AuthServerAddr:   []string{fileConfig.Auth.ListenAddress},
+				IdentityFilePath: mustWriteIdentityFile(t, clt, username),
+				ConfigFile:       mustWriteFileConfig(t, fileConfigAgent),
+				Insecure:         true,
+			},
+		}, {
+			name: "ignores agent config file string and loads identity file",
+			cliFlags: GlobalCLIFlags{
+				AuthServerAddr:   []string{fileConfig.Auth.ListenAddress},
+				IdentityFilePath: mustWriteIdentityFile(t, clt, username),
+				ConfigString:     mustGetBase64EncFileConfig(t, fileConfigAgent),
+				Insecure:         true,
+			},
 		}, {
 			name: "identity file",
 			cliFlags: GlobalCLIFlags{
@@ -92,11 +140,18 @@ func TestConnect(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			cfg := servicecfg.MakeDefaultConfig()
 			cfg.CircuitBreakerConfig = breaker.NoopBreakerConfig()
+			// set tsh home to a fake path so that the existence of a real
+			// ~/.tsh does not interfere with the test result.
+			cfg.TeleportHome = t.TempDir()
 			if tc.modifyConfig != nil {
 				tc.modifyConfig(cfg)
 			}
 
 			clientConfig, err := ApplyConfig(&tc.cliFlags, cfg)
+			if tc.wantErrContains != "" {
+				require.ErrorContains(t, err, tc.wantErrContains)
+				return
+			}
 			require.NoError(t, err)
 
 			_, err = authclient.Connect(ctx, clientConfig)

tool/tsh/common/tctl_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/api/types"
+	"github.com/gravitational/teleport/lib/config"
 	"github.com/gravitational/teleport/lib/service/servicecfg"
 	"github.com/gravitational/teleport/lib/utils"
 	toolcommon "github.com/gravitational/teleport/tool/common"
@@ -169,6 +170,24 @@ func TestSetAuthServerFlagWhileLoggedIn(t *testing.T) {
 	proxyAddr, err := proxyProcess.ProxyWebAddr()
 	require.NoError(t, err)
 
+	// we wont actually run this agent, just make a config file to test with.
+	fileConfigAgent := &config.FileConfig{
+		Global: config.Global{
+			DataDir: t.TempDir(),
+		},
+		Auth: config.Auth{
+			Service: config.Service{
+				EnabledFlag:   "false",
+				ListenAddress: authProcess.Config.Auth.ListenAddr.String(),
+			},
+		},
+		SSH: config.SSH{
+			Service: config.Service{
+				EnabledFlag: "true",
+			},
+		},
+	}
+
 	err = Run(context.Background(), []string{
 		"login",
 		"--insecure",
@@ -181,8 +200,14 @@ func TestSetAuthServerFlagWhileLoggedIn(t *testing.T) {
 	tests := []struct {
 		desc           string
 		authServerFlag []string
+		configFileFlag string
 		want           []utils.NetAddr
 	}{
+		{
+			desc:           "ignores agent config file and loads profile setting",
+			configFileFlag: mustWriteFileConfig(t, fileConfigAgent),
+			want:           []utils.NetAddr{*proxyAddr},
+		},
 		{
 			desc: "sets default web proxy addr without auth server flag",
 			want: []utils.NetAddr{*proxyAddr},
@@ -208,6 +233,7 @@ func TestSetAuthServerFlagWhileLoggedIn(t *testing.T) {
 		t.Run(tt.desc, func(t *testing.T) {
 			ccf := &common.GlobalCLIFlags{}
 			ccf.AuthServerAddr = tt.authServerFlag
+			ccf.ConfigFile = tt.configFileFlag
 
 			cfg := &servicecfg.Config{}
 			cfg.TeleportHome = tmpHomePath

tool/tsh/common/tsh_helper_test.go

@@ -33,6 +33,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sync/errgroup"
+	"gopkg.in/yaml.v2"
 
 	"github.com/gravitational/teleport/api/breaker"
 	apiclient "github.com/gravitational/teleport/api/client"
@@ -474,3 +475,12 @@ func mustRegisterKubeClusters(t *testing.T, ctx context.Context, authSrv *auth.S
 		}
 	}, time.Second*10, time.Millisecond*500, "dynamically created kube clusters failed to register")
 }
+
+func mustWriteFileConfig(t *testing.T, fc *config.FileConfig) string {
+	fileConfPath := filepath.Join(t.TempDir(), "teleport.yaml")
+	fileConfYAML, err := yaml.Marshal(fc)
+	require.NoError(t, err)
+	err = os.WriteFile(fileConfPath, fileConfYAML, 0o600)
+	require.NoError(t, err)
+	return fileConfPath
+}