From 59a6449cd175b24067ecac37e8b2430d508b7f23 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Thu, 31 Jul 2025 14:18:52 -0700 Subject: [PATCH 1/3] Fix printing empty usage and terminate CLI for parsing global flags --- lib/utils/cli.go | 10 ++++++++++ tool/tctl/common/tctl.go | 9 ++++----- tool/tsh/common/tsh.go | 3 +-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/utils/cli.go b/lib/utils/cli.go index 7f64603297988..149f8eba9a28c 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -295,6 +295,16 @@ func InitCLIParser(appName, appHelp string) (app *kingpin.Application) { return app.UsageTemplate(createUsageTemplate()) } +// InitHiddenCLIParser initializes a `kingpin.Application` that does not terminate the application +// or write any usage information to os.Stdout. +func InitHiddenCLIParser() (app *kingpin.Application) { + app = kingpin.New("", "") + app.UsageWriter(io.Discard) + app.Terminate(func(i int) {}) + + return app +} + // createUsageTemplate creates an usage template for kingpin applications. func createUsageTemplate(opts ...func(*usageTemplateOptions)) string { opt := &usageTemplateOptions{ diff --git a/tool/tctl/common/tctl.go b/tool/tctl/common/tctl.go index a8f671c5ad37e..cb3afd2fc9bc8 100644 --- a/tool/tctl/common/tctl.go +++ b/tool/tctl/common/tctl.go @@ -90,12 +90,11 @@ func TryRun(ctx context.Context, commands []CLICommand, args []string) error { utils.InitLogger(utils.LoggingForCLI, slog.LevelWarn) var ccf tctlcfg.GlobalCLIFlags - muApp := utils.InitCLIParser("tctl", GlobalHelpString) + muApp := utils.InitHiddenCLIParser() muApp.Flag("auth-server", fmt.Sprintf("Attempts to connect to specific auth/proxy address(es) instead of local auth [%v]", defaults.AuthConnectAddr().Addr)). Envar(authAddrEnvVar). StringsVar(&ccf.AuthServerAddr) - // We need to parse the arguments before executing managed updates to identify // the profile name and the required version for the current cluster. // All other commands and flags may change between versions, so full parsing @@ -104,9 +103,6 @@ func TryRun(ctx context.Context, commands []CLICommand, args []string) error { slog.WarnContext(ctx, "can't identify current profile", "error", err) } - // app is the command line parser - app := utils.InitCLIParser("tctl", GlobalHelpString) - // cfg (teleport auth server configuration) is going to be shared by all // commands cfg := servicecfg.MakeDefaultConfig() @@ -124,6 +120,9 @@ func TryRun(ctx context.Context, commands []CLICommand, args []string) error { return trace.Wrap(err) } + // app is the command line parser + app := utils.InitCLIParser("tctl", GlobalHelpString) + // Each command will add itself to the CLI parser. for i := range commands { commands[i].Initialize(app, &ccf, cfg) diff --git a/tool/tsh/common/tsh.go b/tool/tsh/common/tsh.go index 280b8193ba513..4ca081920f62f 100644 --- a/tool/tsh/common/tsh.go +++ b/tool/tsh/common/tsh.go @@ -827,10 +827,9 @@ func Run(ctx context.Context, args []string, opts ...CliOption) error { // All other commands and flags may change between versions, so full parsing // should be performed only after managed updates are applied. var proxyArg string - muApp := utils.InitCLIParser("tsh", "") + muApp := utils.InitHiddenCLIParser() muApp.Flag("proxy", "Teleport proxy address").Envar(proxyEnvVar).Hidden().StringVar(&proxyArg) muApp.Flag("check-update", "Check for availability of managed update.").Envar(toolsCheckUpdateEnvVar).Hidden().BoolVar(&cf.checkManagedUpdates) - if _, err := muApp.Parse(utils.FilterArguments(args, muApp.Model())); err != nil { slog.WarnContext(ctx, "can't identify current profile", "error", err) } From f2c48082781e43a24070c64a320ceffd4f7aa78b Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Fri, 1 Aug 2025 03:04:53 -0700 Subject: [PATCH 2/3] Add test with check of both `--help` flag and `help` command that usage print is not empty and both identical. Add godoc clarification --- lib/utils/cli.go | 4 +++- tool/tsh/common/tsh_test.go | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/lib/utils/cli.go b/lib/utils/cli.go index 149f8eba9a28c..70087b93a0573 100644 --- a/lib/utils/cli.go +++ b/lib/utils/cli.go @@ -296,7 +296,9 @@ func InitCLIParser(appName, appHelp string) (app *kingpin.Application) { } // InitHiddenCLIParser initializes a `kingpin.Application` that does not terminate the application -// or write any usage information to os.Stdout. +// or write any usage information to os.Stdout. Can be used in scenarios where multiple `kingpin.Application` +// instances are needed without interfering with subsequent parsing. Usage output is completely suppressed, +// and the default global `--help` flag is ignored to prevent the application from exiting. func InitHiddenCLIParser() (app *kingpin.Application) { app = kingpin.New("", "") app.UsageWriter(io.Discard) diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index 5fdb1b2080862..4995476290d3b 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -464,6 +464,29 @@ func TestNoEnvVars(t *testing.T) { require.NoError(t, trace.NewAggregate(err, ctx.Err())) } +// TestDefaultPrintUsage verifies that the main `kingpin.Application` parser has not been +// previously terminated, and that it correctly prints the usage message when using the +// global `--help` flag or the `help` command, and both are identical. +func TestDefaultPrintUsage(t *testing.T) { + t.Parallel() + testExecutable, err := os.Executable() + require.NoError(t, err) + + ctx := context.Background() + + cmd := exec.CommandContext(ctx, testExecutable, "version", "--help") + cmd.Env = []string{fmt.Sprintf("%s=1", tshBinMainTestEnv)} + flagOutput, err := cmd.CombinedOutput() + require.NoError(t, err) + require.Contains(t, string(flagOutput), "Print the tsh client and Proxy server versions for the current context") + + cmd = exec.CommandContext(ctx, testExecutable, "help", "version") + cmd.Env = []string{fmt.Sprintf("%s=1", tshBinMainTestEnv)} + commandOutput, err := cmd.CombinedOutput() + require.NoError(t, err) + require.Equal(t, string(flagOutput), string(commandOutput)) +} + func TestFailedLogin(t *testing.T) { t.Parallel() tmpHomePath := t.TempDir() From 419bacd69d5beb86955f7b435596a696aee8d7c9 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Fri, 1 Aug 2025 09:11:09 -0700 Subject: [PATCH 3/3] Disable managed update check for version help command test --- tool/tsh/common/tsh_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index 4995476290d3b..9f5779e5f08a5 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -475,13 +475,13 @@ func TestDefaultPrintUsage(t *testing.T) { ctx := context.Background() cmd := exec.CommandContext(ctx, testExecutable, "version", "--help") - cmd.Env = []string{fmt.Sprintf("%s=1", tshBinMainTestEnv)} + cmd.Env = []string{fmt.Sprintf("%s=1", tshBinMainTestEnv), "TELEPORT_TOOLS_VERSION=off"} flagOutput, err := cmd.CombinedOutput() require.NoError(t, err) require.Contains(t, string(flagOutput), "Print the tsh client and Proxy server versions for the current context") cmd = exec.CommandContext(ctx, testExecutable, "help", "version") - cmd.Env = []string{fmt.Sprintf("%s=1", tshBinMainTestEnv)} + cmd.Env = []string{fmt.Sprintf("%s=1", tshBinMainTestEnv), "TELEPORT_TOOLS_VERSION=off"} commandOutput, err := cmd.CombinedOutput() require.NoError(t, err) require.Equal(t, string(flagOutput), string(commandOutput))