Skip to content

Commit

Permalink
Machine ID: Fix usage of "starts" and "configures" terminology (#48494)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
timothyb89 authored Nov 8, 2024
1 parent 3687b24 commit 50df51f
Show file tree
Hide file tree
Showing 11 changed files with 65 additions and 34 deletions.
4 changes: 2 additions & 2 deletions lib/tbot/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions lib/tbot/cli/start_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package cli

import (
"fmt"
"log/slog"

"github.com/alecthomas/kingpin/v2"
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions lib/tbot/cli/start_application_tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package cli

import (
"fmt"
"log/slog"

"github.com/alecthomas/kingpin/v2"
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions lib/tbot/cli/start_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package cli

import (
"fmt"
"log/slog"

"github.com/alecthomas/kingpin/v2"
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions lib/tbot/cli/start_database_tunnel.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package cli

import (
"fmt"
"log/slog"

"github.com/alecthomas/kingpin/v2"
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions lib/tbot/cli/start_identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package cli

import (
"fmt"
"log/slog"

"github.com/alecthomas/kingpin/v2"
Expand All @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions lib/tbot/cli/start_kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package cli

import (
"fmt"
"log/slog"

"github.com/alecthomas/kingpin/v2"
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions lib/tbot/cli/start_legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,15 @@ 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, ", "),
)

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)
Expand Down
24 changes: 24 additions & 0 deletions lib/tbot/cli/start_shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
5 changes: 3 additions & 2 deletions lib/tbot/cli/start_spiffe_svid.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package cli

import (
"fmt"
"log/slog"

"github.com/alecthomas/kingpin/v2"
Expand All @@ -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)
Expand Down
32 changes: 16 additions & 16 deletions tool/tbot/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 50df51f

Please sign in to comment.