Skip to content

Commit

Permalink
Machine ID: Fix tshwrap destination dir handling (#48250)
Browse files Browse the repository at this point in the history
Recent CLI handling changes broke destination dir resolution for
commands that use `tshwrap.GetDestinationDir()` to resolve
destinations from either the CLI or config file.

The old behavior depends on implicit creation of an identity service
if `--destination-dir` was provided on the CLI and no config file is
loaded. We now no longer make this assumption and prefer to only
resolve explicit configurations, so this tweaks
`tshwrap.GetDestinationDir()` to explicitly handle the CLI parameter
rather than hoping it appears in the generated config.

While this is legacy functionality, it's not slated for removal in
v17. Most users should still prefer to use the new app and database
tunnel services wherever possible.

Fixes #48107
  • Loading branch information
timothyb89 authored Nov 1, 2024
1 parent c1ce01d commit dab886c
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 15 deletions.
26 changes: 16 additions & 10 deletions lib/tbot/tshwrap/wrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,28 @@ type destinationHolder interface {
GetDestination() bot.Destination
}

// GetDestinationDirectory attempts to select an unambiguous destination, either from
// CLI or YAML config. It returns an error if the selected destination is
// invalid.
func GetDestinationDirectory(botConfig *config.BotConfig) (*config.DestinationDirectory, error) {
// GetDestinationDirectory attempts to select an unambiguous destination, either
// from CLI or YAML config. It returns an error if the selected destination is
// invalid. Note that CLI destinations will not be validated.
func GetDestinationDirectory(cliDestinationPath string, botConfig *config.BotConfig) (*config.DestinationDirectory, error) {
if cliDestinationPath != "" {
d := &config.DestinationDirectory{
Path: cliDestinationPath,
}
if err := d.CheckAndSetDefaults(); err != nil {
return nil, trace.Wrap(err)
}

return d, nil
}

var destinationHolders []destinationHolder
for _, svc := range botConfig.Services {
if v, ok := svc.(destinationHolder); ok {
destinationHolders = append(destinationHolders, v)
}
}
// WARNING:
// This code is dependent on some unexpected "behavior" in
// config.FromCLIConf() - when users provide --destination-dir then all
// outputs configured in the YAML file are overwritten by an identity
// output with a directory destination with a path of --destination-dir.
// See: https://github.com/gravitational/teleport/issues/27206

if len(destinationHolders) == 0 {
return nil, trace.BadParameter("either --destination-dir or a config file containing an output or service must be specified")
} else if len(destinationHolders) > 1 {
Expand Down
6 changes: 3 additions & 3 deletions lib/tbot/tshwrap/wrap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,16 @@ func TestGetDestinationDirectory(t *testing.T) {
return cfg
}
t.Run("one output configured", func(t *testing.T) {
dest, err := GetDestinationDirectory(config(1))
dest, err := GetDestinationDirectory("", config(1))
require.NoError(t, err)
require.Equal(t, "/from-bot-config0", dest.Path)
})
t.Run("no outputs specified", func(t *testing.T) {
_, err := GetDestinationDirectory(config(0))
_, err := GetDestinationDirectory("", config(0))
require.ErrorContains(t, err, "either --destination-dir or a config file containing an output or service must be specified")
})
t.Run("multiple outputs specified", func(t *testing.T) {
_, err := GetDestinationDirectory(config(2))
_, err := GetDestinationDirectory("", config(2))
require.ErrorContains(t, err, "the config file contains multiple outputs and services; a --destination-dir must be specified")
})
}
2 changes: 1 addition & 1 deletion tool/tbot/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func onDBCommand(globalCfg *cli.GlobalArgs, dbCmd *cli.DBCommand) error {
return trace.Wrap(err)
}

destination, err := tshwrap.GetDestinationDirectory(botConfig)
destination, err := tshwrap.GetDestinationDirectory(dbCmd.DestinationDir, botConfig)
if err != nil {
return trace.Wrap(err)
}
Expand Down
2 changes: 1 addition & 1 deletion tool/tbot/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func onProxyCommand(
return trace.Wrap(err)
}

destination, err := tshwrap.GetDestinationDirectory(botConfig)
destination, err := tshwrap.GetDestinationDirectory(proxyCmd.DestinationDir, botConfig)
if err != nil {
return trace.Wrap(err)
}
Expand Down

0 comments on commit dab886c

Please sign in to comment.