From 21c7b1d856f5463a4252bde6463a852bf527bf18 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Thu, 17 Oct 2024 22:53:35 -0400 Subject: [PATCH 01/17] Add client tools auto update tctl commands --- tool/tctl/common/autoupdate_command.go | 217 ++++++++++++++++++++ tool/tctl/common/autoupdate_command_test.go | 93 +++++++++ tool/tctl/common/cmds.go | 1 + tool/tctl/common/helpers_test.go | 5 +- 4 files changed, 315 insertions(+), 1 deletion(-) create mode 100644 tool/tctl/common/autoupdate_command.go create mode 100644 tool/tctl/common/autoupdate_command_test.go diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go new file mode 100644 index 0000000000000..e8552190d2e48 --- /dev/null +++ b/tool/tctl/common/autoupdate_command.go @@ -0,0 +1,217 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package common + +import ( + "context" + "fmt" + "io" + "os" + "time" + + "github.com/alecthomas/kingpin/v2" + "github.com/coreos/go-semver/semver" + "github.com/gravitational/trace" + + "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/client/webclient" + autoupdatev1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" + "github.com/gravitational/teleport/api/types/autoupdate" + "github.com/gravitational/teleport/lib/auth/authclient" + "github.com/gravitational/teleport/lib/service/servicecfg" + "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/lib/utils/interval" +) + +// versionResponse is structure for formatting the autoupdate version response. +type versionResponse struct { + TargetVersion string `json:"target_version"` +} + +// AutoUpdateCommand implements the `tctl autoupdate` command for managing +// autoupdate process for tools and agents. +type AutoUpdateCommand struct { + app *kingpin.Application + config *servicecfg.Config + + updateCmd *kingpin.CmdClause + getCmd *kingpin.CmdClause + watchCmd *kingpin.CmdClause + + mode string + toolsTargetVersion string + proxy string + + // stdout allows to switch standard output source for resource command. Used in tests. + stdout io.Writer +} + +// Initialize allows AutoUpdateCommand to plug itself into the CLI parser. +func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *servicecfg.Config) { + c.app = app + c.config = config + autoUpdateCmd := app.Command("autoupdate", "Teleport auto update commands.") + + clientToolsCmd := autoUpdateCmd.Command("client-tools", "Client tools auto update commands.") + + c.updateCmd = clientToolsCmd.Command("update", "Edit client tools auto update configuration.") + c.updateCmd.Flag("set-mode", `Sets the mode to enable or disable tools auto update in cluster.`).EnumVar(&c.mode, "enabled", "disabled") + c.updateCmd.Flag("set-target-version", `Defines client tools target version required to be updated.`).StringVar(&c.toolsTargetVersion) + + c.getCmd = clientToolsCmd.Command("get", "Receive tools auto update target version.") + c.getCmd.Flag("proxy", `URL of the proxy`).StringVar(&c.proxy) + + c.watchCmd = clientToolsCmd.Command("watch", "Start monitoring auto update target version updates.") + c.watchCmd.Flag("proxy", `URL of the proxy`).StringVar(&c.proxy) + + if c.stdout == nil { + c.stdout = os.Stdout + } +} + +// TryRun takes the CLI command as an argument and executes it. +func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { + switch cmd { + case c.updateCmd.FullCommand(): + err = c.Upsert(ctx, client) + case c.getCmd.FullCommand(): + err = c.Get(ctx, client) + case c.watchCmd.FullCommand(): + err = c.Watch(ctx, client) + default: + return false, nil + } + return true, trace.Wrap(err) +} + +// Upsert works with AutoUpdateConfig and AutoUpdateVersion resources to create or update. +func (c *AutoUpdateCommand) Upsert(ctx context.Context, client *authclient.Client) error { + if c.mode != "" { + config, err := client.GetAutoUpdateConfig(ctx) + if trace.IsNotFound(err) { + if config, err = autoupdate.NewAutoUpdateConfig(&autoupdatev1pb.AutoUpdateConfigSpec{}); err != nil { + return trace.Wrap(err) + } + } else if err != nil { + return trace.Wrap(err) + } + if config.Spec.Tools == nil { + config.Spec.Tools = &autoupdatev1pb.AutoUpdateConfigSpecTools{} + } + if config.Spec.Tools.Mode != c.mode { + config.Spec.Tools.Mode = c.mode + if _, err := client.UpsertAutoUpdateConfig(ctx, config); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "autoupdate_config has been upserted\n") + } + } + + version, err := client.GetAutoUpdateVersion(ctx) + if trace.IsNotFound(err) { + if version, err = autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{}); err != nil { + return trace.Wrap(err) + } + } else if err != nil { + return trace.Wrap(err) + } + if version.Spec.Tools == nil { + version.Spec.Tools = &autoupdatev1pb.AutoUpdateVersionSpecTools{} + } + if version.Spec.Tools.TargetVersion != c.toolsTargetVersion { + version.Spec.Tools.TargetVersion = c.toolsTargetVersion + if _, err := client.UpsertAutoUpdateVersion(ctx, version); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "autoupdate_version has been upserted\n") + } + + return nil +} + +// Get makes request to fetch tools autoupdate version, if proxy flag is not set +// authorized handler should be used. +func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) error { + response, err := c.get(ctx, client) + if err != nil { + return trace.Wrap(err) + } + + if err := utils.WriteJSON(c.stdout, response); err != nil { + return trace.Wrap(err) + } + + return nil +} + +// Watch launch the watcher of the tools auto update target version updates to pull the version +// every minute. +func (c *AutoUpdateCommand) Watch(ctx context.Context, client *authclient.Client) error { + current := teleport.SemVersion + ticker := interval.New(interval.Config{ + Duration: time.Minute, + }) + defer ticker.Stop() + + for { + response, err := c.get(ctx, client) + if err != nil { + return trace.Wrap(err) + } + if response.TargetVersion != "" { + semVersion, err := semver.NewVersion(response.TargetVersion) + if err != nil { + return trace.Wrap(err) + } + if !semVersion.Equal(*current) { + if err := utils.WriteJSON(c.stdout, response); err != nil { + return trace.Wrap(err) + } + current = semVersion + } + } + + select { + case <-ctx.Done(): + return nil + case <-ticker.Next(): + } + } +} + +func (c *AutoUpdateCommand) get(ctx context.Context, client *authclient.Client) (*versionResponse, error) { + var response versionResponse + if c.proxy != "" { + find, err := webclient.Find(&webclient.Config{Context: ctx, ProxyAddr: c.proxy, Insecure: true}) + if err != nil { + return nil, trace.Wrap(err) + } + response.TargetVersion = find.AutoUpdate.ToolsVersion + } else { + version, err := client.GetAutoUpdateVersion(ctx) + if err != nil { + return nil, trace.Wrap(err) + } + if version.Spec.Tools != nil { + response.TargetVersion = version.Spec.Tools.TargetVersion + } + } + + return &response, nil +} diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go new file mode 100644 index 0000000000000..82a25f4a7f1e0 --- /dev/null +++ b/tool/tctl/common/autoupdate_command_test.go @@ -0,0 +1,93 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package common + +import ( + "bytes" + "context" + "testing" + "time" + + "github.com/gravitational/trace" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport/lib/auth/authclient" + "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/teleport/tool/teleport/testenv" +) + +// TestClientToolsAutoUpdateCommands verifies all commands related to client auto updates, by +// enabling/disabling auto update, setting the target version and retrieve it. +func TestClientToolsAutoUpdateCommands(t *testing.T) { + ctx := context.Background() + log := utils.NewSlogLoggerForTests() + process := testenv.MakeTestServer(t, testenv.WithLogger(log)) + authClient := testenv.MakeDefaultAuthClient(t, process) + + // Enable mode to check that resources were modified. + _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "update", "--set-mode=enabled"}) + require.NoError(t, err) + + config, err := authClient.GetAutoUpdateConfig(ctx) + require.NoError(t, err) + assert.Equal(t, "enabled", config.Spec.Tools.Mode) + + // Disable mode to check that resources were modified. + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "update", "--set-mode=disabled"}) + require.NoError(t, err) + + config, err = authClient.GetAutoUpdateConfig(ctx) + require.NoError(t, err) + assert.Equal(t, "disabled", config.Spec.Tools.Mode) + + // Set target version for auto update. + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "get"}) + require.Error(t, err) + assert.True(t, trace.IsNotFound(err)) + + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "update", "--set-target-version=1.2.3"}) + require.NoError(t, err) + + version, err := authClient.GetAutoUpdateVersion(ctx) + require.NoError(t, err) + assert.Equal(t, "1.2.3", version.Spec.Tools.TargetVersion) + + getBuf, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "get"}) + require.NoError(t, err) + response := mustDecodeJSON[versionResponse](t, getBuf) + assert.Equal(t, "1.2.3", response.TargetVersion) + + // Test the watch command that returns the target version. + timeoutCtx, cancel := context.WithTimeout(ctx, time.Second) + defer cancel() + watchBuf, err := runAutoUpdateCommand(t, timeoutCtx, authClient, []string{"client-tools", "watch"}) + require.NotErrorIs(t, err, context.Canceled) + response = mustDecodeJSON[versionResponse](t, watchBuf) + assert.Equal(t, "1.2.3", response.TargetVersion) +} + +func runAutoUpdateCommand(t *testing.T, ctx context.Context, client *authclient.Client, args []string) (*bytes.Buffer, error) { + var stdoutBuff bytes.Buffer + command := &AutoUpdateCommand{ + stdout: &stdoutBuff, + } + args = append([]string{"autoupdate"}, args...) + return &stdoutBuff, runCommandWithContext(t, ctx, client, command, args) +} diff --git a/tool/tctl/common/cmds.go b/tool/tctl/common/cmds.go index 1c727eba01e25..68f1ad43b74d7 100644 --- a/tool/tctl/common/cmds.go +++ b/tool/tctl/common/cmds.go @@ -64,5 +64,6 @@ func Commands() []CLICommand { &webauthnwinCommand{}, &touchIDCommand{}, &TerraformCommand{}, + &AutoUpdateCommand{}, } } diff --git a/tool/tctl/common/helpers_test.go b/tool/tctl/common/helpers_test.go index a9c17e4cef4fd..536e43451f0cf 100644 --- a/tool/tctl/common/helpers_test.go +++ b/tool/tctl/common/helpers_test.go @@ -64,6 +64,10 @@ type cliCommand interface { } func runCommand(t *testing.T, client *authclient.Client, cmd cliCommand, args []string) error { + return runCommandWithContext(t, context.Background(), client, cmd, args) +} + +func runCommandWithContext(t *testing.T, ctx context.Context, client *authclient.Client, cmd cliCommand, args []string) error { cfg := servicecfg.MakeDefaultConfig() cfg.CircuitBreakerConfig = breaker.NoopBreakerConfig() @@ -73,7 +77,6 @@ func runCommand(t *testing.T, client *authclient.Client, cmd cliCommand, args [] selectedCmd, err := app.Parse(args) require.NoError(t, err) - ctx := context.Background() _, err = cmd.TryRun(ctx, selectedCmd, client) return err } From 4490b9de3bf60addf422e5e547b04ccb85a5f7af Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 21 Oct 2024 14:44:17 -0400 Subject: [PATCH 02/17] Always print version for watch command Restrict update empty target version Rename command to upsert --- tool/tctl/common/autoupdate_command.go | 51 +++++++++++++------------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index e8552190d2e48..8564878034ae2 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -29,7 +29,6 @@ import ( "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" - "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/webclient" autoupdatev1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" "github.com/gravitational/teleport/api/types/autoupdate" @@ -50,7 +49,7 @@ type AutoUpdateCommand struct { app *kingpin.Application config *servicecfg.Config - updateCmd *kingpin.CmdClause + upsertCmd *kingpin.CmdClause getCmd *kingpin.CmdClause watchCmd *kingpin.CmdClause @@ -70,15 +69,15 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service clientToolsCmd := autoUpdateCmd.Command("client-tools", "Client tools auto update commands.") - c.updateCmd = clientToolsCmd.Command("update", "Edit client tools auto update configuration.") - c.updateCmd.Flag("set-mode", `Sets the mode to enable or disable tools auto update in cluster.`).EnumVar(&c.mode, "enabled", "disabled") - c.updateCmd.Flag("set-target-version", `Defines client tools target version required to be updated.`).StringVar(&c.toolsTargetVersion) + c.upsertCmd = clientToolsCmd.Command("update", "Edit client tools auto update configuration.") + c.upsertCmd.Flag("set-mode", "Sets the mode to enable or disable tools auto update in cluster.").EnumVar(&c.mode, "enabled", "disabled") + c.upsertCmd.Flag("set-target-version", "Defines client tools target version required to be updated.").StringVar(&c.toolsTargetVersion) c.getCmd = clientToolsCmd.Command("get", "Receive tools auto update target version.") - c.getCmd.Flag("proxy", `URL of the proxy`).StringVar(&c.proxy) + c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) c.watchCmd = clientToolsCmd.Command("watch", "Start monitoring auto update target version updates.") - c.watchCmd.Flag("proxy", `URL of the proxy`).StringVar(&c.proxy) + c.watchCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) if c.stdout == nil { c.stdout = os.Stdout @@ -88,7 +87,7 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service // TryRun takes the CLI command as an argument and executes it. func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { switch cmd { - case c.updateCmd.FullCommand(): + case c.upsertCmd.FullCommand(): err = c.Upsert(ctx, client) case c.getCmd.FullCommand(): err = c.Get(ctx, client) @@ -123,23 +122,25 @@ func (c *AutoUpdateCommand) Upsert(ctx context.Context, client *authclient.Clien } } - version, err := client.GetAutoUpdateVersion(ctx) - if trace.IsNotFound(err) { - if version, err = autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{}); err != nil { + if c.toolsTargetVersion != "" { + version, err := client.GetAutoUpdateVersion(ctx) + if trace.IsNotFound(err) { + if version, err = autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{}); err != nil { + return trace.Wrap(err) + } + } else if err != nil { return trace.Wrap(err) } - } else if err != nil { - return trace.Wrap(err) - } - if version.Spec.Tools == nil { - version.Spec.Tools = &autoupdatev1pb.AutoUpdateVersionSpecTools{} - } - if version.Spec.Tools.TargetVersion != c.toolsTargetVersion { - version.Spec.Tools.TargetVersion = c.toolsTargetVersion - if _, err := client.UpsertAutoUpdateVersion(ctx, version); err != nil { - return trace.Wrap(err) + if version.Spec.Tools == nil { + version.Spec.Tools = &autoupdatev1pb.AutoUpdateVersionSpecTools{} + } + if version.Spec.Tools.TargetVersion != c.toolsTargetVersion { + version.Spec.Tools.TargetVersion = c.toolsTargetVersion + if _, err := client.UpsertAutoUpdateVersion(ctx, version); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "autoupdate_version has been upserted\n") } - fmt.Fprint(c.stdout, "autoupdate_version has been upserted\n") } return nil @@ -163,7 +164,7 @@ func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) // Watch launch the watcher of the tools auto update target version updates to pull the version // every minute. func (c *AutoUpdateCommand) Watch(ctx context.Context, client *authclient.Client) error { - current := teleport.SemVersion + var current semver.Version ticker := interval.New(interval.Config{ Duration: time.Minute, }) @@ -179,11 +180,11 @@ func (c *AutoUpdateCommand) Watch(ctx context.Context, client *authclient.Client if err != nil { return trace.Wrap(err) } - if !semVersion.Equal(*current) { + if !semVersion.Equal(current) { if err := utils.WriteJSON(c.stdout, response); err != nil { return trace.Wrap(err) } - current = semVersion + current = *semVersion } } From f639ac47f27c22852f3ed89399735fef34fc92d1 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Thu, 24 Oct 2024 13:53:22 -0400 Subject: [PATCH 03/17] Add alias on/off for tools mode Rename update command to configure --- tool/tctl/common/autoupdate_command.go | 20 +++++++++++++------- tool/tctl/common/autoupdate_command_test.go | 6 +++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 8564878034ae2..5ffaa9218614e 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -49,9 +49,9 @@ type AutoUpdateCommand struct { app *kingpin.Application config *servicecfg.Config - upsertCmd *kingpin.CmdClause - getCmd *kingpin.CmdClause - watchCmd *kingpin.CmdClause + configureCmd *kingpin.CmdClause + getCmd *kingpin.CmdClause + watchCmd *kingpin.CmdClause mode string toolsTargetVersion string @@ -69,9 +69,9 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service clientToolsCmd := autoUpdateCmd.Command("client-tools", "Client tools auto update commands.") - c.upsertCmd = clientToolsCmd.Command("update", "Edit client tools auto update configuration.") - c.upsertCmd.Flag("set-mode", "Sets the mode to enable or disable tools auto update in cluster.").EnumVar(&c.mode, "enabled", "disabled") - c.upsertCmd.Flag("set-target-version", "Defines client tools target version required to be updated.").StringVar(&c.toolsTargetVersion) + c.configureCmd = clientToolsCmd.Command("configure", "Edit client tools auto update configuration.") + c.configureCmd.Flag("set-mode", "Sets the mode to enable or disable tools auto update in cluster.").EnumVar(&c.mode, "enabled", "disabled", "on", "off") + c.configureCmd.Flag("set-target-version", "Defines client tools target version required to be updated.").StringVar(&c.toolsTargetVersion) c.getCmd = clientToolsCmd.Command("get", "Receive tools auto update target version.") c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) @@ -87,7 +87,7 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service // TryRun takes the CLI command as an argument and executes it. func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { switch cmd { - case c.upsertCmd.FullCommand(): + case c.configureCmd.FullCommand(): err = c.Upsert(ctx, client) case c.getCmd.FullCommand(): err = c.Get(ctx, client) @@ -110,6 +110,12 @@ func (c *AutoUpdateCommand) Upsert(ctx context.Context, client *authclient.Clien } else if err != nil { return trace.Wrap(err) } + switch c.mode { + case "on": + c.mode = autoupdate.ToolsUpdateModeEnabled + case "off": + c.mode = autoupdate.ToolsUpdateModeDisabled + } if config.Spec.Tools == nil { config.Spec.Tools = &autoupdatev1pb.AutoUpdateConfigSpecTools{} } diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index 82a25f4a7f1e0..7da8b6b36d941 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -42,7 +42,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { authClient := testenv.MakeDefaultAuthClient(t, process) // Enable mode to check that resources were modified. - _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "update", "--set-mode=enabled"}) + _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "configure", "--set-mode=enabled"}) require.NoError(t, err) config, err := authClient.GetAutoUpdateConfig(ctx) @@ -50,7 +50,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { assert.Equal(t, "enabled", config.Spec.Tools.Mode) // Disable mode to check that resources were modified. - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "update", "--set-mode=disabled"}) + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "configure", "--set-mode=disabled"}) require.NoError(t, err) config, err = authClient.GetAutoUpdateConfig(ctx) @@ -62,7 +62,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { require.Error(t, err) assert.True(t, trace.IsNotFound(err)) - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "update", "--set-target-version=1.2.3"}) + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "configure", "--set-target-version=1.2.3"}) require.NoError(t, err) version, err := authClient.GetAutoUpdateVersion(ctx) From fade3c5189807b07fb6494583cfcaddae6cf0b48 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Thu, 24 Oct 2024 23:35:46 -0400 Subject: [PATCH 04/17] Add semantic version validation --- tool/tctl/common/autoupdate_command.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 5ffaa9218614e..74eb9787a2562 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -129,6 +129,9 @@ func (c *AutoUpdateCommand) Upsert(ctx context.Context, client *authclient.Clien } if c.toolsTargetVersion != "" { + if _, err := semver.NewVersion(c.toolsTargetVersion); err != nil { + return trace.WrapWithMessage(err, "not semantic version") + } version, err := client.GetAutoUpdateVersion(ctx) if trace.IsNotFound(err) { if version, err = autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{}); err != nil { From 2b4309fa8b9d315d19041e7c7fdadcd30fa5ad8e Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Fri, 1 Nov 2024 15:02:20 -0400 Subject: [PATCH 05/17] Drop watch command for autoupdate --- tool/tctl/common/autoupdate_command.go | 43 --------------------- tool/tctl/common/autoupdate_command_test.go | 12 +----- 2 files changed, 1 insertion(+), 54 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 74eb9787a2562..1e66d7589a88c 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -23,7 +23,6 @@ import ( "fmt" "io" "os" - "time" "github.com/alecthomas/kingpin/v2" "github.com/coreos/go-semver/semver" @@ -35,7 +34,6 @@ import ( "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/utils" - "github.com/gravitational/teleport/lib/utils/interval" ) // versionResponse is structure for formatting the autoupdate version response. @@ -51,7 +49,6 @@ type AutoUpdateCommand struct { configureCmd *kingpin.CmdClause getCmd *kingpin.CmdClause - watchCmd *kingpin.CmdClause mode string toolsTargetVersion string @@ -76,9 +73,6 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service c.getCmd = clientToolsCmd.Command("get", "Receive tools auto update target version.") c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) - c.watchCmd = clientToolsCmd.Command("watch", "Start monitoring auto update target version updates.") - c.watchCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) - if c.stdout == nil { c.stdout = os.Stdout } @@ -91,8 +85,6 @@ func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *auth err = c.Upsert(ctx, client) case c.getCmd.FullCommand(): err = c.Get(ctx, client) - case c.watchCmd.FullCommand(): - err = c.Watch(ctx, client) default: return false, nil } @@ -170,41 +162,6 @@ func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) return nil } -// Watch launch the watcher of the tools auto update target version updates to pull the version -// every minute. -func (c *AutoUpdateCommand) Watch(ctx context.Context, client *authclient.Client) error { - var current semver.Version - ticker := interval.New(interval.Config{ - Duration: time.Minute, - }) - defer ticker.Stop() - - for { - response, err := c.get(ctx, client) - if err != nil { - return trace.Wrap(err) - } - if response.TargetVersion != "" { - semVersion, err := semver.NewVersion(response.TargetVersion) - if err != nil { - return trace.Wrap(err) - } - if !semVersion.Equal(current) { - if err := utils.WriteJSON(c.stdout, response); err != nil { - return trace.Wrap(err) - } - current = *semVersion - } - } - - select { - case <-ctx.Done(): - return nil - case <-ticker.Next(): - } - } -} - func (c *AutoUpdateCommand) get(ctx context.Context, client *authclient.Client) (*versionResponse, error) { var response versionResponse if c.proxy != "" { diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index 7da8b6b36d941..64c2c6e059062 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -21,12 +21,10 @@ package common import ( "bytes" "context" - "testing" - "time" - "github.com/gravitational/trace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "testing" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/utils" @@ -73,14 +71,6 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { require.NoError(t, err) response := mustDecodeJSON[versionResponse](t, getBuf) assert.Equal(t, "1.2.3", response.TargetVersion) - - // Test the watch command that returns the target version. - timeoutCtx, cancel := context.WithTimeout(ctx, time.Second) - defer cancel() - watchBuf, err := runAutoUpdateCommand(t, timeoutCtx, authClient, []string{"client-tools", "watch"}) - require.NotErrorIs(t, err, context.Canceled) - response = mustDecodeJSON[versionResponse](t, watchBuf) - assert.Equal(t, "1.2.3", response.TargetVersion) } func runAutoUpdateCommand(t *testing.T, ctx context.Context, client *authclient.Client, args []string) (*bytes.Buffer, error) { From 55feb65c15f056a020894d8298d771449104e6a8 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 4 Nov 2024 12:50:51 -0500 Subject: [PATCH 06/17] Replace Upsert with Update/Create Add format option for output json/yaml --- tool/tctl/common/autoupdate_command.go | 134 +++++++++++++------- tool/tctl/common/autoupdate_command_test.go | 5 +- 2 files changed, 92 insertions(+), 47 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 1e66d7589a88c..e2b3a6cde63b4 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -28,6 +28,7 @@ import ( "github.com/coreos/go-semver/semver" "github.com/gravitational/trace" + "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/webclient" autoupdatev1pb "github.com/gravitational/teleport/api/gen/proto/go/teleport/autoupdate/v1" "github.com/gravitational/teleport/api/types/autoupdate" @@ -53,6 +54,7 @@ type AutoUpdateCommand struct { mode string toolsTargetVersion string proxy string + format string // stdout allows to switch standard output source for resource command. Used in tests. stdout io.Writer @@ -72,6 +74,7 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service c.getCmd = clientToolsCmd.Command("get", "Receive tools auto update target version.") c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) + c.getCmd.Flag("format", "Output format: 'yaml' or 'json'").Default(teleport.YAML).StringVar(&c.format) if c.stdout == nil { c.stdout = os.Stdout @@ -82,7 +85,7 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { switch cmd { case c.configureCmd.FullCommand(): - err = c.Upsert(ctx, client) + err = c.Configure(ctx, client) case c.getCmd.FullCommand(): err = c.Get(ctx, client) default: @@ -91,57 +94,18 @@ func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *auth return true, trace.Wrap(err) } -// Upsert works with AutoUpdateConfig and AutoUpdateVersion resources to create or update. -func (c *AutoUpdateCommand) Upsert(ctx context.Context, client *authclient.Client) error { +// Configure works with AutoUpdateConfig and AutoUpdateVersion resources to create or update. +func (c *AutoUpdateCommand) Configure(ctx context.Context, client *authclient.Client) error { if c.mode != "" { - config, err := client.GetAutoUpdateConfig(ctx) - if trace.IsNotFound(err) { - if config, err = autoupdate.NewAutoUpdateConfig(&autoupdatev1pb.AutoUpdateConfigSpec{}); err != nil { - return trace.Wrap(err) - } - } else if err != nil { + if err := c.configureAutoUpdateConfig(ctx, client); err != nil { return trace.Wrap(err) } - switch c.mode { - case "on": - c.mode = autoupdate.ToolsUpdateModeEnabled - case "off": - c.mode = autoupdate.ToolsUpdateModeDisabled - } - if config.Spec.Tools == nil { - config.Spec.Tools = &autoupdatev1pb.AutoUpdateConfigSpecTools{} - } - if config.Spec.Tools.Mode != c.mode { - config.Spec.Tools.Mode = c.mode - if _, err := client.UpsertAutoUpdateConfig(ctx, config); err != nil { - return trace.Wrap(err) - } - fmt.Fprint(c.stdout, "autoupdate_config has been upserted\n") - } } if c.toolsTargetVersion != "" { - if _, err := semver.NewVersion(c.toolsTargetVersion); err != nil { - return trace.WrapWithMessage(err, "not semantic version") - } - version, err := client.GetAutoUpdateVersion(ctx) - if trace.IsNotFound(err) { - if version, err = autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{}); err != nil { - return trace.Wrap(err) - } - } else if err != nil { + if err := c.configureAutoUpdateVersion(ctx, client); err != nil { return trace.Wrap(err) } - if version.Spec.Tools == nil { - version.Spec.Tools = &autoupdatev1pb.AutoUpdateVersionSpecTools{} - } - if version.Spec.Tools.TargetVersion != c.toolsTargetVersion { - version.Spec.Tools.TargetVersion = c.toolsTargetVersion - if _, err := client.UpsertAutoUpdateVersion(ctx, version); err != nil { - return trace.Wrap(err) - } - fmt.Fprint(c.stdout, "autoupdate_version has been upserted\n") - } } return nil @@ -155,10 +119,90 @@ func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) return trace.Wrap(err) } - if err := utils.WriteJSON(c.stdout, response); err != nil { + switch c.format { + case teleport.JSON: + if err := utils.WriteJSON(c.stdout, response); err != nil { + return trace.Wrap(err) + } + case teleport.YAML: + if err := utils.WriteYAML(c.stdout, response); err != nil { + return trace.Wrap(err) + } + default: + return trace.BadParameter("unsupported output format %s, supported values are %s and %s", c.format, teleport.JSON, teleport.YAML) + } + + return nil +} + +func (c *AutoUpdateCommand) configureAutoUpdateConfig(ctx context.Context, client *authclient.Client) error { + configExists := true + config, err := client.GetAutoUpdateConfig(ctx) + if trace.IsNotFound(err) { + configExists = false + if config, err = autoupdate.NewAutoUpdateConfig(&autoupdatev1pb.AutoUpdateConfigSpec{}); err != nil { + return trace.Wrap(err) + } + } else if err != nil { return trace.Wrap(err) } + switch c.mode { + case "on": + c.mode = autoupdate.ToolsUpdateModeEnabled + case "off": + c.mode = autoupdate.ToolsUpdateModeDisabled + } + if config.Spec.Tools == nil { + config.Spec.Tools = &autoupdatev1pb.AutoUpdateConfigSpecTools{} + } + if config.Spec.Tools.Mode != c.mode { + config.Spec.Tools.Mode = c.mode + if configExists { + if _, err := client.UpdateAutoUpdateConfig(ctx, config); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "autoupdate_config has been updated\n") + } else { + if _, err := client.CreateAutoUpdateConfig(ctx, config); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "autoupdate_config has been created\n") + } + } + return nil +} +func (c *AutoUpdateCommand) configureAutoUpdateVersion(ctx context.Context, client *authclient.Client) error { + if _, err := semver.NewVersion(c.toolsTargetVersion); err != nil { + return trace.WrapWithMessage(err, "not semantic version") + } + versionExists := true + version, err := client.GetAutoUpdateVersion(ctx) + if trace.IsNotFound(err) { + versionExists = false + if version, err = autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{}); err != nil { + return trace.Wrap(err) + } + } else if err != nil { + return trace.Wrap(err) + } + if version.Spec.Tools == nil { + version.Spec.Tools = &autoupdatev1pb.AutoUpdateVersionSpecTools{} + } + if version.Spec.Tools.TargetVersion != c.toolsTargetVersion { + version.Spec.Tools.TargetVersion = c.toolsTargetVersion + if versionExists { + if _, err := client.UpsertAutoUpdateVersion(ctx, version); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "autoupdate_version has been updated\n") + } else { + if _, err := client.CreateAutoUpdateVersion(ctx, version); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "autoupdate_version has been created\n") + } + } return nil } diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index 64c2c6e059062..f9a4ca04a12fb 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -21,10 +21,11 @@ package common import ( "bytes" "context" + "testing" + "github.com/gravitational/trace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "testing" "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/utils" @@ -67,7 +68,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { require.NoError(t, err) assert.Equal(t, "1.2.3", version.Spec.Tools.TargetVersion) - getBuf, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "get"}) + getBuf, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "get", "--format=json"}) require.NoError(t, err) response := mustDecodeJSON[versionResponse](t, getBuf) assert.Equal(t, "1.2.3", response.TargetVersion) From ea0fc11ae877b7bf51fbd67beecd4a4843dc4531 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 5 Nov 2024 13:02:35 -0500 Subject: [PATCH 07/17] Change update message --- tool/tctl/common/autoupdate_command.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index e2b3a6cde63b4..8ab79f91dc75b 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -161,13 +161,12 @@ func (c *AutoUpdateCommand) configureAutoUpdateConfig(ctx context.Context, clien if _, err := client.UpdateAutoUpdateConfig(ctx, config); err != nil { return trace.Wrap(err) } - fmt.Fprint(c.stdout, "autoupdate_config has been updated\n") } else { if _, err := client.CreateAutoUpdateConfig(ctx, config); err != nil { return trace.Wrap(err) } - fmt.Fprint(c.stdout, "autoupdate_config has been created\n") } + fmt.Fprint(c.stdout, "client tools auto update mode has been updated\n") } return nil } @@ -195,13 +194,12 @@ func (c *AutoUpdateCommand) configureAutoUpdateVersion(ctx context.Context, clie if _, err := client.UpsertAutoUpdateVersion(ctx, version); err != nil { return trace.Wrap(err) } - fmt.Fprint(c.stdout, "autoupdate_version has been updated\n") } else { if _, err := client.CreateAutoUpdateVersion(ctx, version); err != nil { return trace.Wrap(err) } - fmt.Fprint(c.stdout, "autoupdate_version has been created\n") } + fmt.Fprint(c.stdout, "client tools auto update target version has been updated\n") } return nil } From 6c51b7ef02e6fc0117641d1432907e661195741c Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Thu, 7 Nov 2024 11:38:06 -0500 Subject: [PATCH 08/17] Use get/set naming for client-tools --- tool/tctl/common/autoupdate_command.go | 32 ++++++++++----------- tool/tctl/common/autoupdate_command_test.go | 6 ++-- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 8ab79f91dc75b..c40692a220c4c 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -48,8 +48,8 @@ type AutoUpdateCommand struct { app *kingpin.Application config *servicecfg.Config - configureCmd *kingpin.CmdClause - getCmd *kingpin.CmdClause + setCmd *kingpin.CmdClause + getCmd *kingpin.CmdClause mode string toolsTargetVersion string @@ -68,9 +68,9 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service clientToolsCmd := autoUpdateCmd.Command("client-tools", "Client tools auto update commands.") - c.configureCmd = clientToolsCmd.Command("configure", "Edit client tools auto update configuration.") - c.configureCmd.Flag("set-mode", "Sets the mode to enable or disable tools auto update in cluster.").EnumVar(&c.mode, "enabled", "disabled", "on", "off") - c.configureCmd.Flag("set-target-version", "Defines client tools target version required to be updated.").StringVar(&c.toolsTargetVersion) + c.setCmd = clientToolsCmd.Command("set", "Sets client tools auto update configuration.") + c.setCmd.Flag("mode", "Defines the mode to enable or disable tools auto update in cluster.").EnumVar(&c.mode, "enabled", "disabled", "on", "off") + c.setCmd.Flag("target-version", "Defines client tools target version required to be updated.").StringVar(&c.toolsTargetVersion) c.getCmd = clientToolsCmd.Command("get", "Receive tools auto update target version.") c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) @@ -84,8 +84,8 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service // TryRun takes the CLI command as an argument and executes it. func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { switch cmd { - case c.configureCmd.FullCommand(): - err = c.Configure(ctx, client) + case c.setCmd.FullCommand(): + err = c.Set(ctx, client) case c.getCmd.FullCommand(): err = c.Get(ctx, client) default: @@ -94,16 +94,16 @@ func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *auth return true, trace.Wrap(err) } -// Configure works with AutoUpdateConfig and AutoUpdateVersion resources to create or update. -func (c *AutoUpdateCommand) Configure(ctx context.Context, client *authclient.Client) error { +// Set creates or updates AutoUpdateConfig and AutoUpdateVersion resources with specified parameters. +func (c *AutoUpdateCommand) Set(ctx context.Context, client *authclient.Client) error { if c.mode != "" { - if err := c.configureAutoUpdateConfig(ctx, client); err != nil { + if err := c.setAutoUpdateConfig(ctx, client); err != nil { return trace.Wrap(err) } } if c.toolsTargetVersion != "" { - if err := c.configureAutoUpdateVersion(ctx, client); err != nil { + if err := c.setAutoUpdateVersion(ctx, client); err != nil { return trace.Wrap(err) } } @@ -135,7 +135,7 @@ func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) return nil } -func (c *AutoUpdateCommand) configureAutoUpdateConfig(ctx context.Context, client *authclient.Client) error { +func (c *AutoUpdateCommand) setAutoUpdateConfig(ctx context.Context, client *authclient.Client) error { configExists := true config, err := client.GetAutoUpdateConfig(ctx) if trace.IsNotFound(err) { @@ -166,12 +166,12 @@ func (c *AutoUpdateCommand) configureAutoUpdateConfig(ctx context.Context, clien return trace.Wrap(err) } } - fmt.Fprint(c.stdout, "client tools auto update mode has been updated\n") + fmt.Fprint(c.stdout, "client tools auto update mode has been set\n") } return nil } -func (c *AutoUpdateCommand) configureAutoUpdateVersion(ctx context.Context, client *authclient.Client) error { +func (c *AutoUpdateCommand) setAutoUpdateVersion(ctx context.Context, client *authclient.Client) error { if _, err := semver.NewVersion(c.toolsTargetVersion); err != nil { return trace.WrapWithMessage(err, "not semantic version") } @@ -191,7 +191,7 @@ func (c *AutoUpdateCommand) configureAutoUpdateVersion(ctx context.Context, clie if version.Spec.Tools.TargetVersion != c.toolsTargetVersion { version.Spec.Tools.TargetVersion = c.toolsTargetVersion if versionExists { - if _, err := client.UpsertAutoUpdateVersion(ctx, version); err != nil { + if _, err := client.UpdateAutoUpdateVersion(ctx, version); err != nil { return trace.Wrap(err) } } else { @@ -199,7 +199,7 @@ func (c *AutoUpdateCommand) configureAutoUpdateVersion(ctx context.Context, clie return trace.Wrap(err) } } - fmt.Fprint(c.stdout, "client tools auto update target version has been updated\n") + fmt.Fprint(c.stdout, "client tools auto update target version has been set\n") } return nil } diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index f9a4ca04a12fb..cabe4965732c8 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -41,7 +41,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { authClient := testenv.MakeDefaultAuthClient(t, process) // Enable mode to check that resources were modified. - _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "configure", "--set-mode=enabled"}) + _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=enabled"}) require.NoError(t, err) config, err := authClient.GetAutoUpdateConfig(ctx) @@ -49,7 +49,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { assert.Equal(t, "enabled", config.Spec.Tools.Mode) // Disable mode to check that resources were modified. - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "configure", "--set-mode=disabled"}) + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=disabled"}) require.NoError(t, err) config, err = authClient.GetAutoUpdateConfig(ctx) @@ -61,7 +61,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { require.Error(t, err) assert.True(t, trace.IsNotFound(err)) - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "configure", "--set-target-version=1.2.3"}) + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--target-version=1.2.3"}) require.NoError(t, err) version, err := authClient.GetAutoUpdateVersion(ctx) From 9954ce4270fd58760903f044dce05b0e03f8c808 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Thu, 7 Nov 2024 12:06:36 -0500 Subject: [PATCH 09/17] Add mode to response --- tool/tctl/common/autoupdate_command.go | 25 ++++++++++++++++----- tool/tctl/common/autoupdate_command_test.go | 8 ++----- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index c40692a220c4c..070a69ef09c2d 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -37,8 +37,9 @@ import ( "github.com/gravitational/teleport/lib/utils" ) -// versionResponse is structure for formatting the autoupdate version response. -type versionResponse struct { +// getResponse is structure for formatting the client tools auto update response. +type getResponse struct { + Mode string `json:"mode"` TargetVersion string `json:"target_version"` } @@ -204,20 +205,32 @@ func (c *AutoUpdateCommand) setAutoUpdateVersion(ctx context.Context, client *au return nil } -func (c *AutoUpdateCommand) get(ctx context.Context, client *authclient.Client) (*versionResponse, error) { - var response versionResponse +func (c *AutoUpdateCommand) get(ctx context.Context, client *authclient.Client) (*getResponse, error) { + var response getResponse if c.proxy != "" { find, err := webclient.Find(&webclient.Config{Context: ctx, ProxyAddr: c.proxy, Insecure: true}) if err != nil { return nil, trace.Wrap(err) } response.TargetVersion = find.AutoUpdate.ToolsVersion + response.Mode = autoupdate.ToolsUpdateModeDisabled + if find.AutoUpdate.ToolsAutoUpdate { + response.Mode = autoupdate.ToolsUpdateModeEnabled + } } else { + config, err := client.GetAutoUpdateConfig(ctx) + if err != nil && !trace.IsNotFound(err) { + return nil, trace.Wrap(err) + } + if config != nil && config.Spec.Tools != nil { + response.Mode = config.Spec.Tools.Mode + } + version, err := client.GetAutoUpdateVersion(ctx) - if err != nil { + if err != nil && !trace.IsNotFound(err) { return nil, trace.Wrap(err) } - if version.Spec.Tools != nil { + if version != nil && version.Spec.Tools != nil { response.TargetVersion = version.Spec.Tools.TargetVersion } } diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index cabe4965732c8..fd85faf52d178 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -23,7 +23,6 @@ import ( "context" "testing" - "github.com/gravitational/trace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -57,10 +56,6 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { assert.Equal(t, "disabled", config.Spec.Tools.Mode) // Set target version for auto update. - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "get"}) - require.Error(t, err) - assert.True(t, trace.IsNotFound(err)) - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--target-version=1.2.3"}) require.NoError(t, err) @@ -70,8 +65,9 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { getBuf, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "get", "--format=json"}) require.NoError(t, err) - response := mustDecodeJSON[versionResponse](t, getBuf) + response := mustDecodeJSON[getResponse](t, getBuf) assert.Equal(t, "1.2.3", response.TargetVersion) + assert.Equal(t, "disabled", response.Mode) } func runAutoUpdateCommand(t *testing.T, ctx context.Context, client *authclient.Client, args []string) (*bytes.Buffer, error) { From 96620efc5babde1613b5085391f4979ed8bf8819 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 11 Nov 2024 16:51:28 -0500 Subject: [PATCH 10/17] Change sub-command help messages Leave only aliases for enabled/disabled --- tool/tctl/common/autoupdate_command.go | 66 ++++++++++----------- tool/tctl/common/autoupdate_command_test.go | 4 +- 2 files changed, 35 insertions(+), 35 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 070a69ef09c2d..fe3f2c0b91184 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -65,16 +65,16 @@ type AutoUpdateCommand struct { func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *servicecfg.Config) { c.app = app c.config = config - autoUpdateCmd := app.Command("autoupdate", "Teleport auto update commands.") + autoUpdateCmd := app.Command("autoupdate", "Manage auto update configuration.") - clientToolsCmd := autoUpdateCmd.Command("client-tools", "Client tools auto update commands.") + clientToolsCmd := autoUpdateCmd.Command("client-tools", "Manage client tools auto update configuration.") - c.setCmd = clientToolsCmd.Command("set", "Sets client tools auto update configuration.") - c.setCmd.Flag("mode", "Defines the mode to enable or disable tools auto update in cluster.").EnumVar(&c.mode, "enabled", "disabled", "on", "off") + c.setCmd = clientToolsCmd.Command("set", "Modifies client tools auto update configuration.") + c.setCmd.Flag("mode", "Specifies whether client tools auto updates are enabled for the cluster.").EnumVar(&c.mode, "on", "off") c.setCmd.Flag("target-version", "Defines client tools target version required to be updated.").StringVar(&c.toolsTargetVersion) - c.getCmd = clientToolsCmd.Command("get", "Receive tools auto update target version.") - c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address going to be used for requesting target version for auto update.").StringVar(&c.proxy) + c.getCmd = clientToolsCmd.Command("get", "Retrieve client tools auto update configuration.") + c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address will be used to retrieve client tools auto update configuration.").StringVar(&c.proxy) c.getCmd.Flag("format", "Output format: 'yaml' or 'json'").Default(teleport.YAML).StringVar(&c.format) if c.stdout == nil { @@ -147,17 +147,15 @@ func (c *AutoUpdateCommand) setAutoUpdateConfig(ctx context.Context, client *aut } else if err != nil { return trace.Wrap(err) } - switch c.mode { - case "on": - c.mode = autoupdate.ToolsUpdateModeEnabled - case "off": - c.mode = autoupdate.ToolsUpdateModeDisabled + mode := autoupdate.ToolsUpdateModeDisabled + if c.mode == "on" { + mode = autoupdate.ToolsUpdateModeEnabled } if config.Spec.Tools == nil { config.Spec.Tools = &autoupdatev1pb.AutoUpdateConfigSpecTools{} } - if config.Spec.Tools.Mode != c.mode { - config.Spec.Tools.Mode = c.mode + if config.Spec.Tools.Mode != mode { + config.Spec.Tools.Mode = mode if configExists { if _, err := client.UpdateAutoUpdateConfig(ctx, config); err != nil { return trace.Wrap(err) @@ -206,33 +204,35 @@ func (c *AutoUpdateCommand) setAutoUpdateVersion(ctx context.Context, client *au } func (c *AutoUpdateCommand) get(ctx context.Context, client *authclient.Client) (*getResponse, error) { - var response getResponse if c.proxy != "" { - find, err := webclient.Find(&webclient.Config{Context: ctx, ProxyAddr: c.proxy, Insecure: true}) + find, err := webclient.Find(&webclient.Config{ + Context: ctx, + ProxyAddr: c.proxy, + }) if err != nil { return nil, trace.Wrap(err) } - response.TargetVersion = find.AutoUpdate.ToolsVersion - response.Mode = autoupdate.ToolsUpdateModeDisabled + mode := autoupdate.ToolsUpdateModeDisabled if find.AutoUpdate.ToolsAutoUpdate { - response.Mode = autoupdate.ToolsUpdateModeEnabled - } - } else { - config, err := client.GetAutoUpdateConfig(ctx) - if err != nil && !trace.IsNotFound(err) { - return nil, trace.Wrap(err) - } - if config != nil && config.Spec.Tools != nil { - response.Mode = config.Spec.Tools.Mode + mode = autoupdate.ToolsUpdateModeEnabled } + return &getResponse{ + TargetVersion: find.AutoUpdate.ToolsVersion, + Mode: mode, + }, nil + } - version, err := client.GetAutoUpdateVersion(ctx) - if err != nil && !trace.IsNotFound(err) { - return nil, trace.Wrap(err) - } - if version != nil && version.Spec.Tools != nil { - response.TargetVersion = version.Spec.Tools.TargetVersion - } + var response getResponse + config, err := client.GetAutoUpdateConfig(ctx) + if config != nil && config.Spec.Tools != nil { + response.Mode = config.Spec.Tools.Mode + } + version, err := client.GetAutoUpdateVersion(ctx) + if err != nil && !trace.IsNotFound(err) { + return nil, trace.Wrap(err) + } + if version != nil && version.Spec.Tools != nil { + response.TargetVersion = version.Spec.Tools.TargetVersion } return &response, nil diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index fd85faf52d178..34ea41595464b 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -40,7 +40,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { authClient := testenv.MakeDefaultAuthClient(t, process) // Enable mode to check that resources were modified. - _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=enabled"}) + _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=on"}) require.NoError(t, err) config, err := authClient.GetAutoUpdateConfig(ctx) @@ -48,7 +48,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { assert.Equal(t, "enabled", config.Spec.Tools.Mode) // Disable mode to check that resources were modified. - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=disabled"}) + _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=off"}) require.NoError(t, err) config, err = authClient.GetAutoUpdateConfig(ctx) From 3d9b1f780d38e22b3f7be8ae57981b847010aceb Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 11 Nov 2024 21:03:12 -0500 Subject: [PATCH 11/17] Reorganize tctl commands to have commands not required auth client --- tool/tctl/common/autoupdate_command.go | 103 +++++++++++++------------ tool/tctl/common/cmds.go | 8 ++ tool/tctl/common/tctl.go | 37 +++++---- tool/tctl/common/version_command.go | 55 +++++++++++++ tool/tctl/main.go | 2 +- tool/tsh/common/tctl_test.go | 2 +- 6 files changed, 135 insertions(+), 72 deletions(-) create mode 100644 tool/tctl/common/version_command.go diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index fe3f2c0b91184..a88705e9a5493 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -46,8 +46,9 @@ type getResponse struct { // AutoUpdateCommand implements the `tctl autoupdate` command for managing // autoupdate process for tools and agents. type AutoUpdateCommand struct { - app *kingpin.Application - config *servicecfg.Config + app *kingpin.Application + config *servicecfg.Config + readOnly bool setCmd *kingpin.CmdClause getCmd *kingpin.CmdClause @@ -84,11 +85,13 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service // TryRun takes the CLI command as an argument and executes it. func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { - switch cmd { - case c.setCmd.FullCommand(): + switch { + case !c.readOnly && cmd == c.setCmd.FullCommand(): err = c.Set(ctx, client) - case c.getCmd.FullCommand(): + case !c.readOnly && c.proxy == "" && cmd == c.getCmd.FullCommand(): err = c.Get(ctx, client) + case c.readOnly && c.proxy != "" && cmd == c.getCmd.FullCommand(): + err = c.GetByProxy(ctx) default: return false, nil } @@ -112,28 +115,45 @@ func (c *AutoUpdateCommand) Set(ctx context.Context, client *authclient.Client) return nil } -// Get makes request to fetch tools autoupdate version, if proxy flag is not set -// authorized handler should be used. +// Get makes request to auth service to fetch tools autoupdate version. func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) error { - response, err := c.get(ctx, client) - if err != nil { + var response getResponse + config, err := client.GetAutoUpdateConfig(ctx) + if err != nil && !trace.IsNotFound(err) { return trace.Wrap(err) } + if config != nil && config.Spec.Tools != nil { + response.Mode = config.Spec.Tools.Mode + } - switch c.format { - case teleport.JSON: - if err := utils.WriteJSON(c.stdout, response); err != nil { - return trace.Wrap(err) - } - case teleport.YAML: - if err := utils.WriteYAML(c.stdout, response); err != nil { - return trace.Wrap(err) - } - default: - return trace.BadParameter("unsupported output format %s, supported values are %s and %s", c.format, teleport.JSON, teleport.YAML) + version, err := client.GetAutoUpdateVersion(ctx) + if err != nil && !trace.IsNotFound(err) { + return trace.Wrap(err) + } + if version != nil && version.Spec.Tools != nil { + response.TargetVersion = version.Spec.Tools.TargetVersion } - return nil + return c.printResponse(response) +} + +// GetByProxy makes request to find endpoint without auth to fetch tools autoupdate version. +func (c *AutoUpdateCommand) GetByProxy(ctx context.Context) error { + find, err := webclient.Find(&webclient.Config{ + Context: ctx, + ProxyAddr: c.proxy, + }) + if err != nil { + return trace.Wrap(err) + } + mode := autoupdate.ToolsUpdateModeDisabled + if find.AutoUpdate.ToolsAutoUpdate { + mode = autoupdate.ToolsUpdateModeEnabled + } + return c.printResponse(getResponse{ + TargetVersion: find.AutoUpdate.ToolsVersion, + Mode: mode, + }) } func (c *AutoUpdateCommand) setAutoUpdateConfig(ctx context.Context, client *authclient.Client) error { @@ -203,37 +223,18 @@ func (c *AutoUpdateCommand) setAutoUpdateVersion(ctx context.Context, client *au return nil } -func (c *AutoUpdateCommand) get(ctx context.Context, client *authclient.Client) (*getResponse, error) { - if c.proxy != "" { - find, err := webclient.Find(&webclient.Config{ - Context: ctx, - ProxyAddr: c.proxy, - }) - if err != nil { - return nil, trace.Wrap(err) +func (c *AutoUpdateCommand) printResponse(response getResponse) error { + switch c.format { + case teleport.JSON: + if err := utils.WriteJSON(c.stdout, response); err != nil { + return trace.Wrap(err) } - mode := autoupdate.ToolsUpdateModeDisabled - if find.AutoUpdate.ToolsAutoUpdate { - mode = autoupdate.ToolsUpdateModeEnabled + case teleport.YAML: + if err := utils.WriteYAML(c.stdout, response); err != nil { + return trace.Wrap(err) } - return &getResponse{ - TargetVersion: find.AutoUpdate.ToolsVersion, - Mode: mode, - }, nil - } - - var response getResponse - config, err := client.GetAutoUpdateConfig(ctx) - if config != nil && config.Spec.Tools != nil { - response.Mode = config.Spec.Tools.Mode - } - version, err := client.GetAutoUpdateVersion(ctx) - if err != nil && !trace.IsNotFound(err) { - return nil, trace.Wrap(err) - } - if version != nil && version.Spec.Tools != nil { - response.TargetVersion = version.Spec.Tools.TargetVersion + default: + return trace.BadParameter("unsupported output format %s, supported values are %s and %s", c.format, teleport.JSON, teleport.YAML) } - - return &response, nil + return nil } diff --git a/tool/tctl/common/cmds.go b/tool/tctl/common/cmds.go index 68f1ad43b74d7..5f611c85d69f9 100644 --- a/tool/tctl/common/cmds.go +++ b/tool/tctl/common/cmds.go @@ -67,3 +67,11 @@ func Commands() []CLICommand { &AutoUpdateCommand{}, } } + +// CommandsWithoutAuth returns the set of available subcommands for tctl not require auth client. +func CommandsWithoutAuth() []CLICommand { + return []CLICommand{ + &VersionCommand{}, + &AutoUpdateCommand{readOnly: true}, + } +} diff --git a/tool/tctl/common/tctl.go b/tool/tctl/common/tctl.go index 5af22702f8b17..cb677cc3a3ba7 100644 --- a/tool/tctl/common/tctl.go +++ b/tool/tctl/common/tctl.go @@ -51,7 +51,6 @@ import ( "github.com/gravitational/teleport/lib/client/sso" "github.com/gravitational/teleport/lib/config" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/utils" @@ -107,7 +106,7 @@ type CLICommand interface { // "distributions" like OSS or Enterprise // // distribution: name of the Teleport distribution -func Run(ctx context.Context, commands []CLICommand) { +func Run(ctx context.Context, commands []CLICommand, commandsWithoutAuth []CLICommand) { // The user has typed a command like `tsh ssh ...` without being logged in, // if the running binary needs to be updated, update and re-exec. // @@ -143,7 +142,7 @@ func Run(ctx context.Context, commands []CLICommand) { } } - err = TryRun(commands, os.Args[1:]) + err = TryRun(commands, commandsWithoutAuth, os.Args[1:]) if err != nil { var exitError *common.ExitCodeError if errors.As(err, &exitError) { @@ -155,7 +154,7 @@ func Run(ctx context.Context, commands []CLICommand) { // TryRun is a helper function for Run to call - it runs a tctl command and returns an error. // This is useful for testing tctl, because we can capture the returned error in tests. -func TryRun(commands []CLICommand, args []string) error { +func TryRun(commands []CLICommand, commandsWithoutAuth []CLICommand, args []string) error { utils.InitLogger(utils.LoggingForCLI, slog.LevelWarn) // app is the command line parser @@ -167,8 +166,9 @@ func TryRun(commands []CLICommand, args []string) error { cfg.CircuitBreakerConfig = breaker.NoopBreakerConfig() // each command will add itself to the CLI parser: - for i := range commands { - commands[i].Initialize(app, cfg) + commandsAgg := append(commands, commandsWithoutAuth...) + for i := range commandsAgg { + commandsAgg[i].Initialize(app, cfg) } var ccf GlobalCLIFlags @@ -206,10 +206,6 @@ func TryRun(commands []CLICommand, args []string) error { app.Flag("insecure", "When specifying a proxy address in --auth-server, do not verify its TLS certificate. Danger: any data you send can be intercepted or modified by an attacker."). BoolVar(&ccf.Insecure) - // "version" command is always available: - ver := app.Command("version", "Print the version of your tctl binary.") - app.HelpFlag.Short('h') - // parse CLI commands+flags: utils.UpdateAppUsageTemplate(app, args) selectedCmd, err := app.Parse(args) @@ -225,12 +221,6 @@ func TryRun(commands []CLICommand, args []string) error { return trace.BadParameter("tctl --identity also requires --auth-server") } - // "version" command? - if selectedCmd == ver.FullCommand() { - modules.GetModules().PrintVersion() - return nil - } - cfg.TeleportHome = os.Getenv(types.HomeEnvVar) if cfg.TeleportHome != "" { cfg.TeleportHome = filepath.Clean(cfg.TeleportHome) @@ -238,14 +228,24 @@ func TryRun(commands []CLICommand, args []string) error { cfg.Debug = ccf.Debug + ctx := context.Background() + var match bool + for _, c := range commandsWithoutAuth { + match, err = c.TryRun(ctx, selectedCmd, nil) + if err != nil { + return trace.Wrap(err) + } + if match { + return nil + } + } + // configure all commands with Teleport configuration (they share 'cfg') clientConfig, err := ApplyConfig(&ccf, cfg) if err != nil { return trace.Wrap(err) } - ctx := context.Background() - resolver, err := reversetunnelclient.CachingResolver( ctx, reversetunnelclient.WebClientResolver(&webclient.Config{ @@ -310,7 +310,6 @@ func TryRun(commands []CLICommand, args []string) error { }) // execute whatever is selected: - var match bool for _, c := range commands { match, err = c.TryRun(ctx, selectedCmd, client) if err != nil { diff --git a/tool/tctl/common/version_command.go b/tool/tctl/common/version_command.go new file mode 100644 index 0000000000000..e4b97036cc682 --- /dev/null +++ b/tool/tctl/common/version_command.go @@ -0,0 +1,55 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package common + +import ( + "context" + + "github.com/alecthomas/kingpin/v2" + "github.com/gravitational/trace" + + "github.com/gravitational/teleport/lib/auth/authclient" + "github.com/gravitational/teleport/lib/modules" + "github.com/gravitational/teleport/lib/service/servicecfg" +) + +// VersionCommand implements the `tctl version` +type VersionCommand struct { + app *kingpin.Application + + verCmd *kingpin.CmdClause +} + +// Initialize allows VersionCommand to plug itself into the CLI parser. +func (c *VersionCommand) Initialize(app *kingpin.Application, _ *servicecfg.Config) { + c.app = app + c.verCmd = app.Command("version", "Print the version of your tctl binary.") + app.HelpFlag.Short('h') +} + +// TryRun takes the CLI command as an argument and executes it. +func (c *VersionCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { + switch cmd { + case c.verCmd.FullCommand(): + modules.GetModules().PrintVersion() + default: + return false, nil + } + return true, trace.Wrap(err) +} diff --git a/tool/tctl/main.go b/tool/tctl/main.go index 6dfae87ffdef2..d4db4d7c01888 100644 --- a/tool/tctl/main.go +++ b/tool/tctl/main.go @@ -29,5 +29,5 @@ func main() { ctx, cancel := signal.GetSignalHandler().NotifyContext(context.Background()) defer cancel() - common.Run(ctx, common.Commands()) + common.Run(ctx, common.Commands(), common.CommandsWithoutAuth()) } diff --git a/tool/tsh/common/tctl_test.go b/tool/tsh/common/tctl_test.go index d0cf25df9ef13..be526afc57621 100644 --- a/tool/tsh/common/tctl_test.go +++ b/tool/tsh/common/tctl_test.go @@ -139,7 +139,7 @@ func TestRemoteTctlWithProfile(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - err := common.TryRun(tt.commands, tt.args) + err := common.TryRun(tt.commands, nil, tt.args) if tt.wantErrContains != "" { var exitError *toolcommon.ExitCodeError require.ErrorAs(t, err, &exitError) From 177b3f46a4b1cf6872e9f2f75b8fdd645abc81d4 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 12 Nov 2024 00:25:02 -0500 Subject: [PATCH 12/17] Propagate insecure flag with global config to commands by context --- tool/tctl/common/autoupdate_command.go | 6 ++++++ tool/tctl/common/autoupdate_command_test.go | 22 ++++++++++++++++++++- tool/tctl/common/tctl.go | 5 ++++- 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index a88705e9a5493..a4f454c8ee86d 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -139,9 +139,15 @@ func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) // GetByProxy makes request to find endpoint without auth to fetch tools autoupdate version. func (c *AutoUpdateCommand) GetByProxy(ctx context.Context) error { + var insecure bool + if ccf, ok := ctx.Value(globalConfigKey{}).(GlobalCLIFlags); ok { + insecure = ccf.Insecure + } + find, err := webclient.Find(&webclient.Config{ Context: ctx, ProxyAddr: c.proxy, + Insecure: insecure, }) if err != nil { return trace.Wrap(err) diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index 34ea41595464b..74ec414f104e7 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -34,7 +34,7 @@ import ( // TestClientToolsAutoUpdateCommands verifies all commands related to client auto updates, by // enabling/disabling auto update, setting the target version and retrieve it. func TestClientToolsAutoUpdateCommands(t *testing.T) { - ctx := context.Background() + ctx := context.WithValue(context.Background(), globalConfigKey{}, GlobalCLIFlags{Insecure: true}) log := utils.NewSlogLoggerForTests() process := testenv.MakeTestServer(t, testenv.WithLogger(log)) authClient := testenv.MakeDefaultAuthClient(t, process) @@ -68,6 +68,16 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { response := mustDecodeJSON[getResponse](t, getBuf) assert.Equal(t, "1.2.3", response.TargetVersion) assert.Equal(t, "disabled", response.Mode) + + // Make same request with proxy flag to read command expecting the same + // response from `webapi/find` endpoint. + proxy, err := process.ProxyWebAddr() + require.NoError(t, err) + getProxyBuf, err := runAutoUpdateReadCommand(t, ctx, authClient, []string{"client-tools", "get", "--proxy=" + proxy.Addr, "--format=json"}) + require.NoError(t, err) + response = mustDecodeJSON[getResponse](t, getProxyBuf) + assert.Equal(t, "1.2.3", response.TargetVersion) + assert.Equal(t, "disabled", response.Mode) } func runAutoUpdateCommand(t *testing.T, ctx context.Context, client *authclient.Client, args []string) (*bytes.Buffer, error) { @@ -78,3 +88,13 @@ func runAutoUpdateCommand(t *testing.T, ctx context.Context, client *authclient. args = append([]string{"autoupdate"}, args...) return &stdoutBuff, runCommandWithContext(t, ctx, client, command, args) } + +func runAutoUpdateReadCommand(t *testing.T, ctx context.Context, client *authclient.Client, args []string) (*bytes.Buffer, error) { + var stdoutBuff bytes.Buffer + command := &AutoUpdateCommand{ + stdout: &stdoutBuff, + readOnly: true, + } + args = append([]string{"autoupdate"}, args...) + return &stdoutBuff, runCommandWithContext(t, ctx, client, command, args) +} diff --git a/tool/tctl/common/tctl.go b/tool/tctl/common/tctl.go index cb677cc3a3ba7..82a61ec07d315 100644 --- a/tool/tctl/common/tctl.go +++ b/tool/tctl/common/tctl.go @@ -70,6 +70,9 @@ const ( authAddrEnvVar = "TELEPORT_AUTH_SERVER" ) +// globalConfigKey context value key for propagating the global config for the commands. +type globalConfigKey struct{} + // GlobalCLIFlags keeps the CLI flags that apply to all tctl commands type GlobalCLIFlags struct { // Debug enables verbose logging mode to the console @@ -228,7 +231,7 @@ func TryRun(commands []CLICommand, commandsWithoutAuth []CLICommand, args []stri cfg.Debug = ccf.Debug - ctx := context.Background() + ctx := context.WithValue(context.Background(), globalConfigKey{}, ccf) var match bool for _, c := range commandsWithoutAuth { match, err = c.TryRun(ctx, selectedCmd, nil) From f6697c9526a13ea61442249f3ca366e85ddc6d98 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 17 Dec 2024 14:08:51 -0800 Subject: [PATCH 13/17] Fix autoupdate command without auth client --- tool/tctl/common/autoupdate_command.go | 41 ++++++++++++--------- tool/tctl/common/autoupdate_command_test.go | 40 +++++++++++--------- tool/tctl/common/cmds.go | 8 ---- tool/tctl/common/helpers_test.go | 7 +--- tool/tctl/main.go | 2 +- tool/tsh/common/tctl_test.go | 2 +- 6 files changed, 49 insertions(+), 51 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index a4f454c8ee86d..0480a907552b2 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -35,6 +35,8 @@ import ( "github.com/gravitational/teleport/lib/auth/authclient" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/utils" + commonclient "github.com/gravitational/teleport/tool/tctl/common/client" + tctlcfg "github.com/gravitational/teleport/tool/tctl/common/config" ) // getResponse is structure for formatting the client tools auto update response. @@ -46,9 +48,8 @@ type getResponse struct { // AutoUpdateCommand implements the `tctl autoupdate` command for managing // autoupdate process for tools and agents. type AutoUpdateCommand struct { - app *kingpin.Application - config *servicecfg.Config - readOnly bool + app *kingpin.Application + ccf *tctlcfg.GlobalCLIFlags setCmd *kingpin.CmdClause getCmd *kingpin.CmdClause @@ -63,9 +64,9 @@ type AutoUpdateCommand struct { } // Initialize allows AutoUpdateCommand to plug itself into the CLI parser. -func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *servicecfg.Config) { +func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, ccf *tctlcfg.GlobalCLIFlags, _ *servicecfg.Config) { c.app = app - c.config = config + c.ccf = ccf autoUpdateCmd := app.Command("autoupdate", "Manage auto update configuration.") clientToolsCmd := autoUpdateCmd.Command("client-tools", "Manage client tools auto update configuration.") @@ -84,17 +85,28 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, config *service } // TryRun takes the CLI command as an argument and executes it. -func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, client *authclient.Client) (match bool, err error) { +func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, clientFunc commonclient.InitFunc) (match bool, err error) { + var commandFunc func(ctx context.Context, client *authclient.Client) error switch { - case !c.readOnly && cmd == c.setCmd.FullCommand(): - err = c.Set(ctx, client) - case !c.readOnly && c.proxy == "" && cmd == c.getCmd.FullCommand(): - err = c.Get(ctx, client) - case c.readOnly && c.proxy != "" && cmd == c.getCmd.FullCommand(): + case cmd == c.setCmd.FullCommand(): + commandFunc = c.Set + case + c.proxy == "" && cmd == c.getCmd.FullCommand(): + commandFunc = c.Get + case c.proxy != "" && cmd == c.getCmd.FullCommand(): err = c.GetByProxy(ctx) + return true, trace.Wrap(err) default: return false, nil } + + client, closeFn, err := clientFunc(ctx) + if err != nil { + return false, trace.Wrap(err) + } + err = commandFunc(ctx, client) + closeFn(ctx) + return true, trace.Wrap(err) } @@ -139,15 +151,10 @@ func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) // GetByProxy makes request to find endpoint without auth to fetch tools autoupdate version. func (c *AutoUpdateCommand) GetByProxy(ctx context.Context) error { - var insecure bool - if ccf, ok := ctx.Value(globalConfigKey{}).(GlobalCLIFlags); ok { - insecure = ccf.Insecure - } - find, err := webclient.Find(&webclient.Config{ Context: ctx, ProxyAddr: c.proxy, - Insecure: insecure, + Insecure: c.ccf.Insecure, }) if err != nil { return trace.Wrap(err) diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index 74ec414f104e7..0c78c284fc44f 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -26,21 +26,24 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/gravitational/teleport/api/breaker" "github.com/gravitational/teleport/lib/auth/authclient" + "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/utils" + tctlcfg "github.com/gravitational/teleport/tool/tctl/common/config" "github.com/gravitational/teleport/tool/teleport/testenv" ) // TestClientToolsAutoUpdateCommands verifies all commands related to client auto updates, by // enabling/disabling auto update, setting the target version and retrieve it. func TestClientToolsAutoUpdateCommands(t *testing.T) { - ctx := context.WithValue(context.Background(), globalConfigKey{}, GlobalCLIFlags{Insecure: true}) + ctx := context.Background() log := utils.NewSlogLoggerForTests() process := testenv.MakeTestServer(t, testenv.WithLogger(log)) authClient := testenv.MakeDefaultAuthClient(t, process) // Enable mode to check that resources were modified. - _, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=on"}) + _, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "set", "--mode=on"}) require.NoError(t, err) config, err := authClient.GetAutoUpdateConfig(ctx) @@ -48,7 +51,7 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { assert.Equal(t, "enabled", config.Spec.Tools.Mode) // Disable mode to check that resources were modified. - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--mode=off"}) + _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "set", "--mode=off"}) require.NoError(t, err) config, err = authClient.GetAutoUpdateConfig(ctx) @@ -56,14 +59,14 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { assert.Equal(t, "disabled", config.Spec.Tools.Mode) // Set target version for auto update. - _, err = runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "set", "--target-version=1.2.3"}) + _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "set", "--target-version=1.2.3"}) require.NoError(t, err) version, err := authClient.GetAutoUpdateVersion(ctx) require.NoError(t, err) assert.Equal(t, "1.2.3", version.Spec.Tools.TargetVersion) - getBuf, err := runAutoUpdateCommand(t, ctx, authClient, []string{"client-tools", "get", "--format=json"}) + getBuf, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "get", "--format=json"}) require.NoError(t, err) response := mustDecodeJSON[getResponse](t, getBuf) assert.Equal(t, "1.2.3", response.TargetVersion) @@ -73,28 +76,29 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { // response from `webapi/find` endpoint. proxy, err := process.ProxyWebAddr() require.NoError(t, err) - getProxyBuf, err := runAutoUpdateReadCommand(t, ctx, authClient, []string{"client-tools", "get", "--proxy=" + proxy.Addr, "--format=json"}) + getProxyBuf, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "get", "--proxy=" + proxy.Addr, "--format=json"}) require.NoError(t, err) response = mustDecodeJSON[getResponse](t, getProxyBuf) assert.Equal(t, "1.2.3", response.TargetVersion) assert.Equal(t, "disabled", response.Mode) } -func runAutoUpdateCommand(t *testing.T, ctx context.Context, client *authclient.Client, args []string) (*bytes.Buffer, error) { +func runAutoUpdateCommand(t *testing.T, client *authclient.Client, args []string) (*bytes.Buffer, error) { var stdoutBuff bytes.Buffer command := &AutoUpdateCommand{ stdout: &stdoutBuff, } - args = append([]string{"autoupdate"}, args...) - return &stdoutBuff, runCommandWithContext(t, ctx, client, command, args) -} -func runAutoUpdateReadCommand(t *testing.T, ctx context.Context, client *authclient.Client, args []string) (*bytes.Buffer, error) { - var stdoutBuff bytes.Buffer - command := &AutoUpdateCommand{ - stdout: &stdoutBuff, - readOnly: true, - } - args = append([]string{"autoupdate"}, args...) - return &stdoutBuff, runCommandWithContext(t, ctx, client, command, args) + cfg := servicecfg.MakeDefaultConfig() + cfg.CircuitBreakerConfig = breaker.NoopBreakerConfig() + app := utils.InitCLIParser("tctl", GlobalHelpString) + command.Initialize(app, &tctlcfg.GlobalCLIFlags{Insecure: true}, cfg) + + selectedCmd, err := app.Parse(append([]string{"autoupdate"}, args...)) + require.NoError(t, err) + + _, err = command.TryRun(context.Background(), selectedCmd, func(ctx context.Context) (*authclient.Client, func(context.Context), error) { + return client, func(context.Context) {}, nil + }) + return &stdoutBuff, err } diff --git a/tool/tctl/common/cmds.go b/tool/tctl/common/cmds.go index 63f92a177db49..2cd7b7a579802 100644 --- a/tool/tctl/common/cmds.go +++ b/tool/tctl/common/cmds.go @@ -69,11 +69,3 @@ func Commands() []CLICommand { &AutoUpdateCommand{}, } } - -// CommandsWithoutAuth returns the set of available subcommands for tctl not require auth client. -func CommandsWithoutAuth() []CLICommand { - return []CLICommand{ - &VersionCommand{}, - &AutoUpdateCommand{readOnly: true}, - } -} diff --git a/tool/tctl/common/helpers_test.go b/tool/tctl/common/helpers_test.go index e2f27351c8b48..0cf773852c96f 100644 --- a/tool/tctl/common/helpers_test.go +++ b/tool/tctl/common/helpers_test.go @@ -66,10 +66,6 @@ type cliCommand interface { } func runCommand(t *testing.T, client *authclient.Client, cmd cliCommand, args []string) error { - return runCommandWithContext(t, context.Background(), client, cmd, args) -} - -func runCommandWithContext(t *testing.T, ctx context.Context, client *authclient.Client, cmd cliCommand, args []string) error { cfg := servicecfg.MakeDefaultConfig() cfg.CircuitBreakerConfig = breaker.NoopBreakerConfig() @@ -79,8 +75,7 @@ func runCommandWithContext(t *testing.T, ctx context.Context, client *authclient selectedCmd, err := app.Parse(args) require.NoError(t, err) - ctx := context.Background() - _, err = cmd.TryRun(ctx, selectedCmd, func(ctx context.Context) (*authclient.Client, func(context.Context), error) { + _, err = cmd.TryRun(context.Background(), selectedCmd, func(ctx context.Context) (*authclient.Client, func(context.Context), error) { return client, func(context.Context) {}, nil }) return err diff --git a/tool/tctl/main.go b/tool/tctl/main.go index d4db4d7c01888..6dfae87ffdef2 100644 --- a/tool/tctl/main.go +++ b/tool/tctl/main.go @@ -29,5 +29,5 @@ func main() { ctx, cancel := signal.GetSignalHandler().NotifyContext(context.Background()) defer cancel() - common.Run(ctx, common.Commands(), common.CommandsWithoutAuth()) + common.Run(ctx, common.Commands()) } diff --git a/tool/tsh/common/tctl_test.go b/tool/tsh/common/tctl_test.go index 4ac233b8eb8e2..94958414f93fb 100644 --- a/tool/tsh/common/tctl_test.go +++ b/tool/tsh/common/tctl_test.go @@ -140,7 +140,7 @@ func TestRemoteTctlWithProfile(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - err := common.TryRun(tt.commands, nil, tt.args) + err := common.TryRun(tt.commands, tt.args) if tt.wantErrContains != "" { var exitError *toolcommon.ExitCodeError require.ErrorAs(t, err, &exitError) From bdabbbad3551d60a42d2395141ea723555bd4601 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Thu, 19 Dec 2024 15:11:01 -0800 Subject: [PATCH 14/17] Change commands to enable/disable/target --- tool/tctl/common/autoupdate_command.go | 141 ++++++++++++-------- tool/tctl/common/autoupdate_command_test.go | 30 +++-- 2 files changed, 104 insertions(+), 67 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 0480a907552b2..0c91d3e036bd6 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -51,14 +51,17 @@ type AutoUpdateCommand struct { app *kingpin.Application ccf *tctlcfg.GlobalCLIFlags - setCmd *kingpin.CmdClause - getCmd *kingpin.CmdClause + targetCmd *kingpin.CmdClause + enableCmd *kingpin.CmdClause + disableCmd *kingpin.CmdClause + statusCmd *kingpin.CmdClause - mode string toolsTargetVersion string proxy string format string + clear bool + // stdout allows to switch standard output source for resource command. Used in tests. stdout io.Writer } @@ -71,13 +74,16 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, ccf *tctlcfg.Gl clientToolsCmd := autoUpdateCmd.Command("client-tools", "Manage client tools auto update configuration.") - c.setCmd = clientToolsCmd.Command("set", "Modifies client tools auto update configuration.") - c.setCmd.Flag("mode", "Specifies whether client tools auto updates are enabled for the cluster.").EnumVar(&c.mode, "on", "off") - c.setCmd.Flag("target-version", "Defines client tools target version required to be updated.").StringVar(&c.toolsTargetVersion) + c.statusCmd = clientToolsCmd.Command("status", "Prints if the client tools updates are enabled/disabled, and the target version in specified format.") + c.statusCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address will be used to retrieve client tools auto update configuration.").StringVar(&c.proxy) + c.statusCmd.Flag("format", "Output format: 'yaml' or 'json'").Default(teleport.YAML).StringVar(&c.format) + + c.enableCmd = clientToolsCmd.Command("enable", "Enables client tools auto updates. Clients will be told to update to the target version.") + c.disableCmd = clientToolsCmd.Command("disable", "Disables client tools auto updates. Clients will not be told to update to the target version.") - c.getCmd = clientToolsCmd.Command("get", "Retrieve client tools auto update configuration.") - c.getCmd.Flag("proxy", "Address of the Teleport proxy. When defined this address will be used to retrieve client tools auto update configuration.").StringVar(&c.proxy) - c.getCmd.Flag("format", "Output format: 'yaml' or 'json'").Default(teleport.YAML).StringVar(&c.format) + c.targetCmd = clientToolsCmd.Command("target", "Sets the client tools target version. This command is not supported on Teleport Cloud.") + c.targetCmd.Arg("version", "Client tools target version. Clients will be told to update to this version.").StringVar(&c.toolsTargetVersion) + c.targetCmd.Flag("clear", "removes the target version, Teleport will default to its current proxy version.").BoolVar(&c.clear) if c.stdout == nil { c.stdout = os.Stdout @@ -88,13 +94,16 @@ func (c *AutoUpdateCommand) Initialize(app *kingpin.Application, ccf *tctlcfg.Gl func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, clientFunc commonclient.InitFunc) (match bool, err error) { var commandFunc func(ctx context.Context, client *authclient.Client) error switch { - case cmd == c.setCmd.FullCommand(): - commandFunc = c.Set - case - c.proxy == "" && cmd == c.getCmd.FullCommand(): - commandFunc = c.Get - case c.proxy != "" && cmd == c.getCmd.FullCommand(): - err = c.GetByProxy(ctx) + case cmd == c.targetCmd.FullCommand(): + commandFunc = c.TargetVersion + case cmd == c.enableCmd.FullCommand(): + commandFunc = c.Enable + case cmd == c.disableCmd.FullCommand(): + commandFunc = c.Disable + case c.proxy == "" && cmd == c.statusCmd.FullCommand(): + commandFunc = c.Status + case c.proxy != "" && cmd == c.statusCmd.FullCommand(): + err = c.StatusByProxy(ctx) return true, trace.Wrap(err) default: return false, nil @@ -110,25 +119,33 @@ func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, clientFunc c return true, trace.Wrap(err) } -// Set creates or updates AutoUpdateConfig and AutoUpdateVersion resources with specified parameters. -func (c *AutoUpdateCommand) Set(ctx context.Context, client *authclient.Client) error { - if c.mode != "" { - if err := c.setAutoUpdateConfig(ctx, client); err != nil { +// TargetVersion creates or updates AutoUpdateVersion resource with client tools target version. +func (c *AutoUpdateCommand) TargetVersion(ctx context.Context, client *authclient.Client) error { + switch { + case c.clear: + if err := c.clearTargetVersion(ctx, client); err != nil { return trace.Wrap(err) } - } - - if c.toolsTargetVersion != "" { - if err := c.setAutoUpdateVersion(ctx, client); err != nil { + case c.toolsTargetVersion != "": + if err := c.setTargetVersion(ctx, client); err != nil { return trace.Wrap(err) } } - return nil } -// Get makes request to auth service to fetch tools autoupdate version. -func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) error { +// Enable enables client tools auto updates in cluster. +func (c *AutoUpdateCommand) Enable(ctx context.Context, client *authclient.Client) error { + return c.setMode(ctx, client, true) +} + +// Disable disables client tools auto updates in cluster. +func (c *AutoUpdateCommand) Disable(ctx context.Context, client *authclient.Client) error { + return c.setMode(ctx, client, false) +} + +// Status makes request to auth service to fetch client tools auto update version and mode. +func (c *AutoUpdateCommand) Status(ctx context.Context, client *authclient.Client) error { var response getResponse config, err := client.GetAutoUpdateConfig(ctx) if err != nil && !trace.IsNotFound(err) { @@ -149,8 +166,9 @@ func (c *AutoUpdateCommand) Get(ctx context.Context, client *authclient.Client) return c.printResponse(response) } -// GetByProxy makes request to find endpoint without auth to fetch tools autoupdate version. -func (c *AutoUpdateCommand) GetByProxy(ctx context.Context) error { +// StatusByProxy makes request to `webapi/find` endpoint to fetch tools auto update version and mode +// without authentication. +func (c *AutoUpdateCommand) StatusByProxy(ctx context.Context) error { find, err := webclient.Find(&webclient.Config{ Context: ctx, ProxyAddr: c.proxy, @@ -169,51 +187,45 @@ func (c *AutoUpdateCommand) GetByProxy(ctx context.Context) error { }) } -func (c *AutoUpdateCommand) setAutoUpdateConfig(ctx context.Context, client *authclient.Client) error { - configExists := true +func (c *AutoUpdateCommand) setMode(ctx context.Context, client *authclient.Client, enabled bool) error { + setMode := client.UpdateAutoUpdateConfig config, err := client.GetAutoUpdateConfig(ctx) if trace.IsNotFound(err) { - configExists = false if config, err = autoupdate.NewAutoUpdateConfig(&autoupdatev1pb.AutoUpdateConfigSpec{}); err != nil { return trace.Wrap(err) } + setMode = client.CreateAutoUpdateConfig } else if err != nil { return trace.Wrap(err) } - mode := autoupdate.ToolsUpdateModeDisabled - if c.mode == "on" { - mode = autoupdate.ToolsUpdateModeEnabled - } + if config.Spec.Tools == nil { config.Spec.Tools = &autoupdatev1pb.AutoUpdateConfigSpecTools{} } - if config.Spec.Tools.Mode != mode { - config.Spec.Tools.Mode = mode - if configExists { - if _, err := client.UpdateAutoUpdateConfig(ctx, config); err != nil { - return trace.Wrap(err) - } - } else { - if _, err := client.CreateAutoUpdateConfig(ctx, config); err != nil { - return trace.Wrap(err) - } - } - fmt.Fprint(c.stdout, "client tools auto update mode has been set\n") + + config.Spec.Tools.Mode = autoupdate.ToolsUpdateModeDisabled + if enabled { + config.Spec.Tools.Mode = autoupdate.ToolsUpdateModeEnabled } + if _, err := setMode(ctx, config); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "client tools auto update mode has been changed\n") + return nil } -func (c *AutoUpdateCommand) setAutoUpdateVersion(ctx context.Context, client *authclient.Client) error { +func (c *AutoUpdateCommand) setTargetVersion(ctx context.Context, client *authclient.Client) error { if _, err := semver.NewVersion(c.toolsTargetVersion); err != nil { return trace.WrapWithMessage(err, "not semantic version") } - versionExists := true + setTargetVersion := client.UpdateAutoUpdateVersion version, err := client.GetAutoUpdateVersion(ctx) if trace.IsNotFound(err) { - versionExists = false if version, err = autoupdate.NewAutoUpdateVersion(&autoupdatev1pb.AutoUpdateVersionSpec{}); err != nil { return trace.Wrap(err) } + setTargetVersion = client.CreateAutoUpdateVersion } else if err != nil { return trace.Wrap(err) } @@ -222,20 +234,31 @@ func (c *AutoUpdateCommand) setAutoUpdateVersion(ctx context.Context, client *au } if version.Spec.Tools.TargetVersion != c.toolsTargetVersion { version.Spec.Tools.TargetVersion = c.toolsTargetVersion - if versionExists { - if _, err := client.UpdateAutoUpdateVersion(ctx, version); err != nil { - return trace.Wrap(err) - } - } else { - if _, err := client.CreateAutoUpdateVersion(ctx, version); err != nil { - return trace.Wrap(err) - } + if _, err := setTargetVersion(ctx, version); err != nil { + return trace.Wrap(err) } fmt.Fprint(c.stdout, "client tools auto update target version has been set\n") } return nil } +func (c *AutoUpdateCommand) clearTargetVersion(ctx context.Context, client *authclient.Client) error { + version, err := client.GetAutoUpdateVersion(ctx) + if trace.IsNotFound(err) { + return nil + } else if err != nil { + return trace.Wrap(err) + } + if version.Spec.Tools != nil { + version.Spec.Tools = nil + if _, err := client.UpdateAutoUpdateVersion(ctx, version); err != nil { + return trace.Wrap(err) + } + fmt.Fprint(c.stdout, "client tools auto update target version has been cleared\n") + } + return nil +} + func (c *AutoUpdateCommand) printResponse(response getResponse) error { switch c.format { case teleport.JSON: diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index 0c78c284fc44f..902a41c1a1460 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -23,6 +23,7 @@ import ( "context" "testing" + "github.com/gravitational/trace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -42,31 +43,37 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { process := testenv.MakeTestServer(t, testenv.WithLogger(log)) authClient := testenv.MakeDefaultAuthClient(t, process) - // Enable mode to check that resources were modified. - _, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "set", "--mode=on"}) + // Check that AutoUpdateConfig and AutoUpdateVersion are not created. + _, err := authClient.GetAutoUpdateConfig(ctx) + require.True(t, trace.IsNotFound(err)) + _, err = authClient.GetAutoUpdateVersion(ctx) + require.True(t, trace.IsNotFound(err)) + + // Enable client tools auto updates to check that AutoUpdateConfig resource is modified. + _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "enable"}) require.NoError(t, err) config, err := authClient.GetAutoUpdateConfig(ctx) require.NoError(t, err) assert.Equal(t, "enabled", config.Spec.Tools.Mode) - // Disable mode to check that resources were modified. - _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "set", "--mode=off"}) + // Disable client tools auto updates to check that AutoUpdateConfig resource is modified. + _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "disable"}) require.NoError(t, err) config, err = authClient.GetAutoUpdateConfig(ctx) require.NoError(t, err) assert.Equal(t, "disabled", config.Spec.Tools.Mode) - // Set target version for auto update. - _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "set", "--target-version=1.2.3"}) + // Set target version for client tools auto updates. + _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "target", "1.2.3"}) require.NoError(t, err) version, err := authClient.GetAutoUpdateVersion(ctx) require.NoError(t, err) assert.Equal(t, "1.2.3", version.Spec.Tools.TargetVersion) - getBuf, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "get", "--format=json"}) + getBuf, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "status", "--format=json"}) require.NoError(t, err) response := mustDecodeJSON[getResponse](t, getBuf) assert.Equal(t, "1.2.3", response.TargetVersion) @@ -76,11 +83,18 @@ func TestClientToolsAutoUpdateCommands(t *testing.T) { // response from `webapi/find` endpoint. proxy, err := process.ProxyWebAddr() require.NoError(t, err) - getProxyBuf, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "get", "--proxy=" + proxy.Addr, "--format=json"}) + getProxyBuf, err := runAutoUpdateCommand(t, authClient, []string{"client-tools", "status", "--proxy=" + proxy.Addr, "--format=json"}) require.NoError(t, err) response = mustDecodeJSON[getResponse](t, getProxyBuf) assert.Equal(t, "1.2.3", response.TargetVersion) assert.Equal(t, "disabled", response.Mode) + + // Set clear flag for the target version update to check that it is going to be reset. + _, err = runAutoUpdateCommand(t, authClient, []string{"client-tools", "target", "--clear"}) + require.NoError(t, err) + version, err = authClient.GetAutoUpdateVersion(ctx) + require.NoError(t, err) + assert.Nil(t, version.Spec.Tools) } func runAutoUpdateCommand(t *testing.T, client *authclient.Client, args []string) (*bytes.Buffer, error) { From 00b28ed6194c03edbd7126a6b496ac1d34b09b66 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 7 Jan 2025 11:34:22 -0800 Subject: [PATCH 15/17] Add retry in case of the parallel request --- tool/tctl/common/autoupdate_command.go | 35 ++++++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 0c91d3e036bd6..039cdfe0ec2c3 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -121,27 +121,46 @@ func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, clientFunc c // TargetVersion creates or updates AutoUpdateVersion resource with client tools target version. func (c *AutoUpdateCommand) TargetVersion(ctx context.Context, client *authclient.Client) error { + var err error switch { case c.clear: - if err := c.clearTargetVersion(ctx, client); err != nil { - return trace.Wrap(err) - } + err = c.clearTargetVersion(ctx, client) case c.toolsTargetVersion != "": - if err := c.setTargetVersion(ctx, client); err != nil { - return trace.Wrap(err) + // For parallel requests where we attempt to create a resource simultaneously, retries should be implemented. + // The same approach applies to updates if the resource has been deleted during the process. + // Second create request must return `AlreadyExists` error, update for deleted resource `NotFound` error. + err = c.setTargetVersion(ctx, client) + if trace.IsNotFound(err) || trace.IsAlreadyExists(err) { + err = c.setTargetVersion(ctx, client) } + } - return nil + return trace.Wrap(err) } // Enable enables client tools auto updates in cluster. func (c *AutoUpdateCommand) Enable(ctx context.Context, client *authclient.Client) error { - return c.setMode(ctx, client, true) + // For parallel requests where we attempt to create a resource simultaneously, retries should be implemented. + // The same approach applies to updates if the resource has been deleted during the process. + // Second create request must return `AlreadyExists` error, update for deleted resource `NotFound` error. + err := c.setMode(ctx, client, true) + if trace.IsNotFound(err) || trace.IsAlreadyExists(err) { + err = c.setMode(ctx, client, true) + } + + return err } // Disable disables client tools auto updates in cluster. func (c *AutoUpdateCommand) Disable(ctx context.Context, client *authclient.Client) error { - return c.setMode(ctx, client, false) + // For parallel requests where we attempt to create a resource simultaneously, retries should be implemented. + // The same approach applies to updates if the resource has been deleted during the process. + // Second create request must return `AlreadyExists` error, update for deleted resource `NotFound` error. + err := c.setMode(ctx, client, false) + if trace.IsNotFound(err) || trace.IsAlreadyExists(err) { + err = c.setMode(ctx, client, false) + } + return err } // Status makes request to auth service to fetch client tools auto update version and mode. From 59ae5b7d6e8a0c7c59b38007cf3e14149635beab Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 7 Jan 2025 14:23:13 -0800 Subject: [PATCH 16/17] Add more than one retry Code review changes --- tool/tctl/common/autoupdate_command.go | 71 +++++++++++---------- tool/tctl/common/autoupdate_command_test.go | 2 +- 2 files changed, 38 insertions(+), 35 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index 039cdfe0ec2c3..c7b19e0616d0f 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -1,6 +1,6 @@ /* * Teleport - * Copyright (C) 2024 Gravitational, Inc. + * Copyright (C) 2025 Gravitational, Inc. * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published by @@ -39,11 +39,10 @@ import ( tctlcfg "github.com/gravitational/teleport/tool/tctl/common/config" ) -// getResponse is structure for formatting the client tools auto update response. -type getResponse struct { - Mode string `json:"mode"` - TargetVersion string `json:"target_version"` -} +const ( + // maxRetries is the default number of RPC call retries to prevent parallel create/update errors. + maxRetries = 3 +) // AutoUpdateCommand implements the `tctl autoupdate` command for managing // autoupdate process for tools and agents. @@ -97,9 +96,9 @@ func (c *AutoUpdateCommand) TryRun(ctx context.Context, cmd string, clientFunc c case cmd == c.targetCmd.FullCommand(): commandFunc = c.TargetVersion case cmd == c.enableCmd.FullCommand(): - commandFunc = c.Enable + commandFunc = c.SetModeCommand(true) case cmd == c.disableCmd.FullCommand(): - commandFunc = c.Disable + commandFunc = c.SetModeCommand(false) case c.proxy == "" && cmd == c.statusCmd.FullCommand(): commandFunc = c.Status case c.proxy != "" && cmd == c.statusCmd.FullCommand(): @@ -129,38 +128,42 @@ func (c *AutoUpdateCommand) TargetVersion(ctx context.Context, client *authclien // For parallel requests where we attempt to create a resource simultaneously, retries should be implemented. // The same approach applies to updates if the resource has been deleted during the process. // Second create request must return `AlreadyExists` error, update for deleted resource `NotFound` error. - err = c.setTargetVersion(ctx, client) - if trace.IsNotFound(err) || trace.IsAlreadyExists(err) { + for i := 0; i < maxRetries; i++ { err = c.setTargetVersion(ctx, client) + if err == nil { + break + } + if !trace.IsNotFound(err) && !trace.IsAlreadyExists(err) { + return trace.Wrap(err) + } } - } return trace.Wrap(err) } -// Enable enables client tools auto updates in cluster. -func (c *AutoUpdateCommand) Enable(ctx context.Context, client *authclient.Client) error { - // For parallel requests where we attempt to create a resource simultaneously, retries should be implemented. - // The same approach applies to updates if the resource has been deleted during the process. - // Second create request must return `AlreadyExists` error, update for deleted resource `NotFound` error. - err := c.setMode(ctx, client, true) - if trace.IsNotFound(err) || trace.IsAlreadyExists(err) { - err = c.setMode(ctx, client, true) +// SetModeCommand returns a command to enable or disable client tools auto-updates in the cluster. +func (c *AutoUpdateCommand) SetModeCommand(enabled bool) func(ctx context.Context, client *authclient.Client) error { + return func(ctx context.Context, client *authclient.Client) error { + // For parallel requests where we attempt to create a resource simultaneously, retries should be implemented. + // The same approach applies to updates if the resource has been deleted during the process. + // Second create request must return `AlreadyExists` error, update for deleted resource `NotFound` error. + for i := 0; i < maxRetries; i++ { + err := c.setMode(ctx, client, enabled) + if err == nil { + break + } + if !trace.IsNotFound(err) && !trace.IsAlreadyExists(err) { + return trace.Wrap(err) + } + } + return nil } - - return err } -// Disable disables client tools auto updates in cluster. -func (c *AutoUpdateCommand) Disable(ctx context.Context, client *authclient.Client) error { - // For parallel requests where we attempt to create a resource simultaneously, retries should be implemented. - // The same approach applies to updates if the resource has been deleted during the process. - // Second create request must return `AlreadyExists` error, update for deleted resource `NotFound` error. - err := c.setMode(ctx, client, false) - if trace.IsNotFound(err) || trace.IsAlreadyExists(err) { - err = c.setMode(ctx, client, false) - } - return err +// getResponse is structure for formatting the client tools auto update response. +type getResponse struct { + Mode string `json:"mode"` + TargetVersion string `json:"target_version"` } // Status makes request to auth service to fetch client tools auto update version and mode. @@ -229,7 +232,7 @@ func (c *AutoUpdateCommand) setMode(ctx context.Context, client *authclient.Clie if _, err := setMode(ctx, config); err != nil { return trace.Wrap(err) } - fmt.Fprint(c.stdout, "client tools auto update mode has been changed\n") + fmt.Fprintln(c.stdout, "client tools auto update mode has been changed") return nil } @@ -256,7 +259,7 @@ func (c *AutoUpdateCommand) setTargetVersion(ctx context.Context, client *authcl if _, err := setTargetVersion(ctx, version); err != nil { return trace.Wrap(err) } - fmt.Fprint(c.stdout, "client tools auto update target version has been set\n") + fmt.Fprintln(c.stdout, "client tools auto update target version has been set") } return nil } @@ -273,7 +276,7 @@ func (c *AutoUpdateCommand) clearTargetVersion(ctx context.Context, client *auth if _, err := client.UpdateAutoUpdateVersion(ctx, version); err != nil { return trace.Wrap(err) } - fmt.Fprint(c.stdout, "client tools auto update target version has been cleared\n") + fmt.Fprintln(c.stdout, "client tools auto update target version has been cleared") } return nil } diff --git a/tool/tctl/common/autoupdate_command_test.go b/tool/tctl/common/autoupdate_command_test.go index 902a41c1a1460..31d2782fbc335 100644 --- a/tool/tctl/common/autoupdate_command_test.go +++ b/tool/tctl/common/autoupdate_command_test.go @@ -1,6 +1,6 @@ /* * Teleport - * Copyright (C) 2024 Gravitational, Inc. + * Copyright (C) 2025 Gravitational, Inc. * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Affero General Public License as published by From 776a0e2aaf1f39ddfdda2cbaf6378469c0038b64 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Wed, 8 Jan 2025 08:14:14 -0800 Subject: [PATCH 17/17] Update tool/tctl/common/autoupdate_command.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --- tool/tctl/common/autoupdate_command.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tool/tctl/common/autoupdate_command.go b/tool/tctl/common/autoupdate_command.go index c7b19e0616d0f..c089010c091f4 100644 --- a/tool/tctl/common/autoupdate_command.go +++ b/tool/tctl/common/autoupdate_command.go @@ -39,10 +39,8 @@ import ( tctlcfg "github.com/gravitational/teleport/tool/tctl/common/config" ) -const ( - // maxRetries is the default number of RPC call retries to prevent parallel create/update errors. - maxRetries = 3 -) +// maxRetries is the default number of RPC call retries to prevent parallel create/update errors. +const maxRetries = 3 // AutoUpdateCommand implements the `tctl autoupdate` command for managing // autoupdate process for tools and agents.