From 63b0037e51634e0441a435b50dc0b63d6a21dcad Mon Sep 17 00:00:00 2001 From: Tim Buckley Date: Tue, 5 Nov 2024 17:00:09 -0700 Subject: [PATCH] Machine ID: Fix usage of "starts" and "configures" terminology (#48415) This fixes an issue in tbot's CLI help text where all "configure" commands claimed to start a bot when they were actually generating a configuration. This was caused by commands being reused for both start and configure cases and the help text was statically defined. This fixes the issue by introducing a CommandMode enum to indicate to a command at construction time whether it will be used for the "start" or "configure" case, and implements a stringer to return the appropriate term for each case. --- lib/tbot/cli/cli_test.go | 4 +-- lib/tbot/cli/start_application.go | 5 ++-- lib/tbot/cli/start_application_tunnel.go | 5 ++-- lib/tbot/cli/start_database.go | 5 ++-- lib/tbot/cli/start_database_tunnel.go | 5 ++-- lib/tbot/cli/start_identity.go | 5 ++-- lib/tbot/cli/start_kubernetes.go | 5 ++-- lib/tbot/cli/start_legacy.go | 4 +-- lib/tbot/cli/start_shared.go | 24 ++++++++++++++++++ lib/tbot/cli/start_spiffe_svid.go | 5 ++-- tool/tbot/main.go | 32 ++++++++++++------------ 11 files changed, 65 insertions(+), 34 deletions(-) diff --git a/lib/tbot/cli/cli_test.go b/lib/tbot/cli/cli_test.go index 64ce04a6705fd..6e60e8c7b18f0 100644 --- a/lib/tbot/cli/cli_test.go +++ b/lib/tbot/cli/cli_test.go @@ -138,7 +138,7 @@ type startConfigureTestCase struct { func testStartConfigureCommand[T startConfigureCommand]( t *testing.T, - newCommand func(parentCmd *kingpin.CmdClause, action MutatorAction) T, + newCommand func(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) T, testCases []startConfigureTestCase, ) { for _, tt := range testCases { @@ -148,7 +148,7 @@ func testStartConfigureCommand[T startConfigureCommand]( cmd := newCommand(subcommand, func(mut ConfigMutator) error { actionCalled = true return nil - }) + }, CommandModeStart) command, err := app.Parse(tt.args) require.NoError(t, err) diff --git a/lib/tbot/cli/start_application.go b/lib/tbot/cli/start_application.go index 8f4eb84cb76db..eb3c2a4507655 100644 --- a/lib/tbot/cli/start_application.go +++ b/lib/tbot/cli/start_application.go @@ -19,6 +19,7 @@ package cli import ( + "fmt" "log/slog" "github.com/alecthomas/kingpin/v2" @@ -40,8 +41,8 @@ type ApplicationCommand struct { // NewApplicationCommand initializes a command and flag for application outputs // and returns a struct that will contain the parse result. -func NewApplicationCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *ApplicationCommand { - cmd := parentCmd.Command("application", "Starts with an application output.").Alias("app") +func NewApplicationCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *ApplicationCommand { + cmd := parentCmd.Command("application", fmt.Sprintf("%s tbot with an application output.", mode)).Alias("app") c := &ApplicationCommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/lib/tbot/cli/start_application_tunnel.go b/lib/tbot/cli/start_application_tunnel.go index 47e3c73988e34..3c005f4ca1cea 100644 --- a/lib/tbot/cli/start_application_tunnel.go +++ b/lib/tbot/cli/start_application_tunnel.go @@ -19,6 +19,7 @@ package cli import ( + "fmt" "log/slog" "github.com/alecthomas/kingpin/v2" @@ -39,8 +40,8 @@ type ApplicationTunnelCommand struct { // NewApplicationTunnelCommand initializes flags for an app tunnel command and // returns a struct to contain the parse result. -func NewApplicationTunnelCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *ApplicationTunnelCommand { - cmd := parentCmd.Command("application-tunnel", "Starts an application tunnel.").Alias("app-tunnel") +func NewApplicationTunnelCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *ApplicationTunnelCommand { + cmd := parentCmd.Command("application-tunnel", fmt.Sprintf("%s tbot with an application tunnel.", mode)).Alias("app-tunnel") c := &ApplicationTunnelCommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/lib/tbot/cli/start_database.go b/lib/tbot/cli/start_database.go index bfa076077856a..794f72a18f243 100644 --- a/lib/tbot/cli/start_database.go +++ b/lib/tbot/cli/start_database.go @@ -19,6 +19,7 @@ package cli import ( + "fmt" "log/slog" "github.com/alecthomas/kingpin/v2" @@ -42,8 +43,8 @@ type DatabaseCommand struct { // NewDatabaseCommand initializes a command and flags for database outputs and // returns a struct that will contain the parse result. -func NewDatabaseCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *DatabaseCommand { - cmd := parentCmd.Command("database", "Starts with a database output.").Alias("db") +func NewDatabaseCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *DatabaseCommand { + cmd := parentCmd.Command("database", fmt.Sprintf("%s tbot with a database output.", mode)).Alias("db") c := &DatabaseCommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/lib/tbot/cli/start_database_tunnel.go b/lib/tbot/cli/start_database_tunnel.go index 378ebaa8919e3..44f119d9381ce 100644 --- a/lib/tbot/cli/start_database_tunnel.go +++ b/lib/tbot/cli/start_database_tunnel.go @@ -19,6 +19,7 @@ package cli import ( + "fmt" "log/slog" "github.com/alecthomas/kingpin/v2" @@ -40,8 +41,8 @@ type DatabaseTunnelCommand struct { } // NewDatabaseTunnelCommand creates a command supporting `tbot start database-tunnel` -func NewDatabaseTunnelCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *DatabaseTunnelCommand { - cmd := parentCmd.Command("database-tunnel", "Start a database tunnel listener.").Alias("db-tunnel") +func NewDatabaseTunnelCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *DatabaseTunnelCommand { + cmd := parentCmd.Command("database-tunnel", fmt.Sprintf("%s tbot with a database tunnel listener.", mode)).Alias("db-tunnel") c := &DatabaseTunnelCommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/lib/tbot/cli/start_identity.go b/lib/tbot/cli/start_identity.go index 087fbc7262482..60985489104e8 100644 --- a/lib/tbot/cli/start_identity.go +++ b/lib/tbot/cli/start_identity.go @@ -19,6 +19,7 @@ package cli import ( + "fmt" "log/slog" "github.com/alecthomas/kingpin/v2" @@ -39,8 +40,8 @@ type IdentityCommand struct { // NewIdentityCommand initializes the command and flags for identity outputs // and returns a struct that will contain the parse result. -func NewIdentityCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *IdentityCommand { - cmd := parentCmd.Command("identity", "Start with an identity output for SSH and Teleport API access.").Alias("ssh").Alias("id") +func NewIdentityCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *IdentityCommand { + cmd := parentCmd.Command("identity", fmt.Sprintf("%s tbot with an identity output for SSH and Teleport API access.", mode)).Alias("ssh").Alias("id") c := &IdentityCommand{} c.sharedDestinationArgs = newSharedDestinationArgs(cmd) diff --git a/lib/tbot/cli/start_kubernetes.go b/lib/tbot/cli/start_kubernetes.go index e03bddc57d972..5c5c9e41c2d27 100644 --- a/lib/tbot/cli/start_kubernetes.go +++ b/lib/tbot/cli/start_kubernetes.go @@ -19,6 +19,7 @@ package cli import ( + "fmt" "log/slog" "github.com/alecthomas/kingpin/v2" @@ -40,8 +41,8 @@ type KubernetesCommand struct { // NewKubernetesCommand initializes the command and flags for kubernetes outputs // and returns a struct to contain the parse result. -func NewKubernetesCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *KubernetesCommand { - cmd := parentCmd.Command("kubernetes", "Starts with a kubernetes output.").Alias("k8s") +func NewKubernetesCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *KubernetesCommand { + cmd := parentCmd.Command("kubernetes", fmt.Sprintf("%s tbot with a kubernetes output.", mode)).Alias("k8s") c := &KubernetesCommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/lib/tbot/cli/start_legacy.go b/lib/tbot/cli/start_legacy.go index def7d0b8ed765..0d406a2062fa4 100644 --- a/lib/tbot/cli/start_legacy.go +++ b/lib/tbot/cli/start_legacy.go @@ -132,7 +132,7 @@ type LegacyCommand struct { // NewLegacyCommand initializes and returns a command supporting // `tbot start legacy` and `tbot configure legacy`. -func NewLegacyCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *LegacyCommand { +func NewLegacyCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *LegacyCommand { joinMethodList := fmt.Sprintf( "(%s)", strings.Join(config.SupportedJoinMethods, ", "), @@ -140,7 +140,7 @@ func NewLegacyCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *Legac c := &LegacyCommand{ action: action, - cmd: parentCmd.Command("legacy", "Start with either a config file or a legacy output.").Default(), + cmd: parentCmd.Command("legacy", fmt.Sprintf("%s tbot with either a config file or a legacy output.", mode)).Default(), } c.AuthProxyArgs = newAuthProxyArgs(c.cmd) c.LegacyDestinationDirArgs = newLegacyDestinationDirArgs(c.cmd) diff --git a/lib/tbot/cli/start_shared.go b/lib/tbot/cli/start_shared.go index ba3501f8b6f85..2e2a07a94020c 100644 --- a/lib/tbot/cli/start_shared.go +++ b/lib/tbot/cli/start_shared.go @@ -287,3 +287,27 @@ func (s *sharedDestinationArgs) BuildDestination() (bot.Destination, error) { return dest, nil } + +// CommandMode is a simple enum to help shared start/configure command +// substitute the correct verb based on whether they are being used for "start" +// or "configure" actions. +type CommandMode int + +const ( + // CommandModeStart indicates a command instance will be used for + // `tbot start ...` + CommandModeStart CommandMode = iota + + // CommandModeConfigure indicates a command instance will be used for + // `tbot configure ...` + CommandModeConfigure +) + +func (c CommandMode) String() string { + switch c { + case CommandModeConfigure: + return "Configures" + default: + return "Starts" + } +} diff --git a/lib/tbot/cli/start_spiffe_svid.go b/lib/tbot/cli/start_spiffe_svid.go index b6a7d2029f409..dd179d16279aa 100644 --- a/lib/tbot/cli/start_spiffe_svid.go +++ b/lib/tbot/cli/start_spiffe_svid.go @@ -19,6 +19,7 @@ package cli import ( + "fmt" "log/slog" "github.com/alecthomas/kingpin/v2" @@ -45,8 +46,8 @@ type SPIFFESVIDCommand struct { // NewSPIFFESVIDCommand initializes the command and flags for the // `spiffe-svid` output and returns a struct that will contain the parse // result. -func NewSPIFFESVIDCommand(parentCmd *kingpin.CmdClause, action MutatorAction) *SPIFFESVIDCommand { - cmd := parentCmd.Command("spiffe-svid", "Starts with a SPIFFE-compatible SVID output.") +func NewSPIFFESVIDCommand(parentCmd *kingpin.CmdClause, action MutatorAction, mode CommandMode) *SPIFFESVIDCommand { + cmd := parentCmd.Command("spiffe-svid", fmt.Sprintf("%s tbot with a SPIFFE-compatible SVID output.", mode)) c := &SPIFFESVIDCommand{} c.sharedStartArgs = newSharedStartArgs(cmd) diff --git a/tool/tbot/main.go b/tool/tbot/main.go index b73fc6c2c2b0e..666eb0d37ac13 100644 --- a/tool/tbot/main.go +++ b/tool/tbot/main.go @@ -117,29 +117,29 @@ func Run(args []string, stdout io.Writer) error { }), // `start` and `configure` commands - cli.NewLegacyCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewLegacyCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewLegacyCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewLegacyCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), - cli.NewIdentityCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewIdentityCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewIdentityCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewIdentityCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), - cli.NewDatabaseCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewDatabaseCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewDatabaseCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewDatabaseCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), - cli.NewKubernetesCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewKubernetesCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewKubernetesCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewKubernetesCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), - cli.NewApplicationCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewApplicationCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewApplicationCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewApplicationCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), - cli.NewApplicationTunnelCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewApplicationTunnelCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewApplicationTunnelCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewApplicationTunnelCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), - cli.NewDatabaseTunnelCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewDatabaseTunnelCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewDatabaseTunnelCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewDatabaseTunnelCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), - cli.NewSPIFFESVIDCommand(startCmd, buildConfigAndStart(ctx, globalCfg)), - cli.NewSPIFFESVIDCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout)), + cli.NewSPIFFESVIDCommand(startCmd, buildConfigAndStart(ctx, globalCfg), cli.CommandModeStart), + cli.NewSPIFFESVIDCommand(configureCmd, buildConfigAndConfigure(ctx, globalCfg, &configureOutPath, stdout), cli.CommandModeConfigure), ) // Initialize legacy-style commands. These are simple enough to not really