Add client tools auto update tctl commands by vapopov · Pull Request #47692 · gravitational/teleport

vapopov · 2024-10-18T02:57:58Z

In this PR implemented tctl client auto update commands described in RDF https://github.com/gravitational/teleport/blob/5b84564a3db3e17aa84926e3edc70a6158af6b3f/rfd/0144-client-tools-updates.md

tigrato · 2024-10-21T08:13:19Z

+	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")


Suggested change

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-mode", `Sets the mode to enable or disable tools auto update in cluster.`).EnumVar(&c.mode, "enabled", "disabled","on", "off")

It's simpler with on/off?

mode was proposed by agent auto update RFD to be compliant, since this is constant state (and replaced in both rfds) not sure if we should use on/off in addition
cc @hugoShaka @sclevine

Should be enabled/disabled to match enabled/disabled/suspended modes for agent upgrades.

@sclevine @vapopov

The on, off is just to be consistent with teleport codebase where:

enabled, on, true means enabled

disabled, off, false means disabled

the idea is to accept them in the cli but always store the enabled/disabled in the resource but it's not a strong opinion.

Restrict update empty target version Rename command to upsert

rosstimothy · 2024-10-22T13:17:31Z

+
+	clientToolsCmd := autoUpdateCmd.Command("client-tools", "Client tools auto update commands.")
+
+	c.upsertCmd = clientToolsCmd.Command("update", "Edit client tools auto update configuration.")


Should we update the name of the command as well? Or should we actually be performing a conditional update instead of an upsert?

For the user experience it looks like the update, when resource wasn't created from api we return empty value. Originally in RFD it was defined as update, if you think that upsert looks better for understanding I can change it here and in RFD

I think update here will very likely understood as "update the clients" rather than "update the AU configuration resource". The help message is "Edit client tools auto update configuration.", so maybe configure would be a better fit for a subcommand that sets the AU client tools configuration.

Rename update command to configure

vapopov · 2024-10-29T11:10:29Z

@tigrato @rosstimothy @sclevine @hugoShaka Could you please review recent changes

sclevine · 2024-10-29T14:40:13Z

@russjones It looks like there is some uncertainty above about use cases for tctl autoupdate watch. I'm having a hard time coming up with a use case for this command myself, since I'd imagine customer automation would run tctl autoupdate get (or curl | jq) at scheduled intervals, instead of using a long-running command. Could you provide an example use case?

Add format option for output json/yaml

…date-command

rosstimothy · 2025-01-07T20:57:12Z

@@ -0,0 +1,295 @@
+/*
+ * Teleport
+ * Copyright (C) 2024  Gravitational, Inc.


Suggested change

* Copyright (C) 2024 Gravitational, Inc.

* Copyright (C) 2025 Gravitational, Inc.

rosstimothy · 2025-01-07T21:09:05Z

+// getResponse is structure for formatting the client tools auto update response.
+type getResponse struct {
+	Mode          string `json:"mode"`
+	TargetVersion string `json:"target_version"`
+}


Suggestion: move this closer to where it is being used

rosstimothy · 2025-01-07T21:12:14Z

+		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")


Suggested change

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")

rosstimothy · 2025-01-07T21:17:29Z

+		err = c.setTargetVersion(ctx, client)
+		if trace.IsNotFound(err) || trace.IsAlreadyExists(err) {
+			err = c.setTargetVersion(ctx, client)
+		}


Suggestion: I believe most of our existing code that retries in similar scenarios does so at least 3 times

Suggested change

err = c.setTargetVersion(ctx, client)

if trace.IsNotFound(err) || trace.IsAlreadyExists(err) {

err = c.setTargetVersion(ctx, client)

}

const maxRetries = 3

...

func (c *AutoUpdateCommand) TargetVersion(ctx context.Context, client *authclient.Client) error {

...

for i := 0; i < maxRetries; i++ {

err = c.setTargetVersion(ctx, client)

if trace.IsNotFound(err) || trace.IsAlreadyExists(err) {

continue

}

}

rosstimothy · 2025-01-07T21:19:39Z

+	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 {
+	// 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


Suggestion: apply the same retry loop as above, but move it within setMode so that it only needs to be implemented once

rosstimothy · 2025-01-07T21:20:17Z

+	if _, err := setMode(ctx, config); err != nil {
+		return trace.Wrap(err)
+	}
+	fmt.Fprint(c.stdout, "client tools auto update mode has been changed\n")


Suggested change

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")

rosstimothy · 2025-01-07T21:20:30Z

+		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")


Suggested change

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")

rosstimothy · 2025-01-07T21:20:46Z

@@ -0,0 +1,118 @@
+/*
+ * Teleport
+ * Copyright (C) 2024  Gravitational, Inc.


Suggested change

* Copyright (C) 2024 Gravitational, Inc.

* Copyright (C) 2025 Gravitational, Inc.

Code review changes

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

public-teleport-github-review-bot · 2025-01-08T17:09:41Z

@vapopov See the table below for backport results.

Branch	Result
branch/v15	Failed
branch/v16	Failed
branch/v17	Failed

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Update tool/tctl/common/autoupdate_command.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --------- Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Update tool/tctl/common/autoupdate_command.go --------- Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Update tool/tctl/common/autoupdate_command.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --------- Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Update tool/tctl/common/autoupdate_command.go --------- Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Update tool/tctl/common/autoupdate_command.go

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Update tool/tctl/common/autoupdate_command.go

* Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Update tool/tctl/common/autoupdate_command.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --------- Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

* [v15] Client tools autoupdates (#48648) * Expose client tools auto update for find endpoint (#46785) * Expose client tools auto update for find endpoint * Group auto update settings in find response Log error instead returning error Add tests auto update settings in find endpoint Add check for not implemented error * Add more test cases * Client AutoUpdate proto structure changes (#47532) * Update client autoupdate proto structure * Replace with reserved * Fix unit tests * Add more info in proto * Rename proto to be aligned RFD namings * Replace enum type for ToolsMode to string * Add packaging utility for client tools auto updates (#47060) * Add packaging utility for client tools auto updates * Add error handling for close functions * Move archive to existing utils package * Move archive helpers to integration/helper CR changes * CR changes * CR changes * CR changes Replace creating directory with extract path as argument * CR changes * Validate full size before un-archive Extract files to extractDir with ignore dir structure * Change compressing with relative paths Add test for cleanup and fix skip logic * CR changes * CR changes * Fix linter * Client tools auto update (#47466) * Add client tools auto update * Replace fork for posix platform for re-exec Move integration tests to client tools specific dir Use context cancellation with SIGTERM, SIGINT Remove cancelable tee reader with context replacement Renaming * Fix syscall path execution Fix archive cleanup if hash is not valid Limit the archive write bytes * Cover the case with single package for darwin platform after v17 * Move updater logic to tools package * Move context out from the library Base URL renaming * Add more context in comments * Changes in find endpoint * Replace test http server with `httptest` Replace hash for bytes matching Proper temp file close for archive download * Add more context to comments * Move feature flag to main package to be reused * Constant rename * Replace build tag with lib/modules to identify enterprise build * Replace fips tag with modules flag * Client auto updates integration for {tctl,tsh} (#47815) * Client auto updates integration for tctl/tsh * Add version validation Fix recursive version check for darwin platform Fix cleanup for multi-package support * Fix identifying tools removal from home directory * Replace ToolsMode with ToolsAutoUpdate * Reuse insecure flag for tests * Fix CheckRemote with login * Fix windows administrative access requirement Update must be able to be canceled, re-execute with latest version or last updated Show progress bar before request is made * Fix update cancellation for login action Address review comments * Add signal handler with stack context cancellation * Use copy instead of hard link for windows Fix progress bar if we can't receive size of package * Replace with list in order to support manual cancel * Download archive package to temp directory * Decrease timeout for client tools proxy call * Add audit logs for auto update resources (#48218) * Connect: Make sure tsh auto-updates are turned off * Add dir for code shared between Node.js processes * Connect: Make sure tsh auto-updates are turned off * Pass TELEPORT_TOOLS_VERSION=off to tsh vnet-daemon * Disable client tools auto update disabled if there are no home dir (#49159) Move updater to general tools package * Move client auto update helper to lib package (#49247) * Restrict AutoUpdateVersion to be created/updated for cloud (#49008) (#50244) * Restrict AutoUpdateVersion to be created/updated for cloud * Check builtin Admin role and Cloud feature * More informative error message * Remove KindAutoUpdateAgentRollout from editor role preset * Add remove tctl command for AutoUpdateConfig and AutoUpdateVersion (#49532) (#49676) * Fix auto-update re-exec arguments modified by aliases (#50228) (#51183) * Fix auto-update re-exec arguments modified by aliases * Make arguments to be required to set * Restore progress bar show before request * Improve integration tests to execute with `tsh` and `tctl` Added a full-cycle integration test to verify client tools auto-updates within a test cluster by modifying AutoUpdateConfig and AutoUpdateVersion resources. The test executes the login command using alias configurations to ensure no recursive re-execution occurs. The updater binary used in integration tests has been replaced with the `Run` logic of tctl and tsh. * Set generated test password by env variable instead of constant value * Restore priority of env check over remote check In case of double re-execution case we should stop second one to prevent loop re-execution Drop localDir set during compilation * Add client tools auto update tctl commands (#47692) * Add client tools auto update tctl commands * Always print version for watch command Restrict update empty target version Rename command to upsert * Add alias on/off for tools mode Rename update command to configure * Add semantic version validation * Drop watch command for autoupdate * Replace Upsert with Update/Create Add format option for output json/yaml * Change update message * Use get/set naming for client-tools * Add mode to response * Change sub-command help messages Leave only aliases for enabled/disabled * Reorganize tctl commands to have commands not required auth client * Propagate insecure flag with global config to commands by context * Fix autoupdate command without auth client * Change commands to enable/disable/target * Add retry in case of the parallel request * Add more than one retry Code review changes * Add template for client tools auto-update download url (#51210) * Add templates for client tools auto-update download url * Change to base url setting by env MakeURL moved to common function to be general for both, agent and client tools * Add godoc for common function and constant for default package * Use flags and version arguments instead of revision * Move base url env to shared constant * Fix tests after backporting * Pass TELEPORT_TOOLS_VERSION=off when starting tshd * Prevent keystore cleanup to remove bin directory (#52331) * Don't show the progress bar until the request to the CDN is made * Fix Windows permission error with self remove (#52316) * Fix windows permission error with self replace * Aggregate errors * Update comments --------- Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>

vapopov added the no-changelog Indicates that a PR does not require a changelog entry label Oct 18, 2024

vapopov requested review from hugoShaka, rosstimothy and sclevine October 18, 2024 02:57

github-actions Bot added size/md tctl tctl - Teleport admin tool labels Oct 18, 2024

github-actions Bot requested review from fheinecke and tigrato October 18, 2024 02:58

vapopov force-pushed the vapopov/tctl-autoupdate-command branch from 6277da8 to 5627b11 Compare October 18, 2024 03:00

Add client tools auto update tctl commands

21c7b1d

vapopov force-pushed the vapopov/tctl-autoupdate-command branch from 5627b11 to 21c7b1d Compare October 18, 2024 17:17

tigrato reviewed Oct 21, 2024

View reviewed changes

rosstimothy reviewed Oct 21, 2024

View reviewed changes

Always print version for watch command

4490b9d

Restrict update empty target version Rename command to upsert

rosstimothy reviewed Oct 22, 2024

View reviewed changes

vapopov added 2 commits October 24, 2024 13:53

Add alias on/off for tools mode

f639ac4

Rename update command to configure

Add semantic version validation

fade3c5

vapopov requested review from rosstimothy and tigrato October 29, 2024 11:08

Drop watch command for autoupdate

2b4309f

vapopov force-pushed the vapopov/tctl-autoupdate-command branch from 43300ae to 2b4309f Compare November 1, 2024 23:15

rosstimothy reviewed Nov 4, 2024

View reviewed changes

Comment thread tool/tctl/common/autoupdate_command.go Outdated

Comment thread tool/tctl/common/autoupdate_command.go Outdated

Comment thread tool/tctl/common/autoupdate_command.go Outdated

sclevine approved these changes Nov 4, 2024

View reviewed changes

vapopov force-pushed the vapopov/tctl-autoupdate-command branch from 1605224 to 2e5654e Compare November 4, 2024 18:55

Replace Upsert with Update/Create

55feb65

Add format option for output json/yaml

vapopov force-pushed the vapopov/tctl-autoupdate-command branch from 2e5654e to 55feb65 Compare November 4, 2024 19:09

vapopov requested a review from rosstimothy November 4, 2024 20:03

Merge remote-tracking branch 'origin/master' into vapopov/tctl-autoup…

3afad02

…date-command

Add retry in case of the parallel request

00b28ed

rosstimothy reviewed Jan 7, 2025

View reviewed changes

Add more than one retry

59ae5b7

Code review changes

rosstimothy approved these changes Jan 8, 2025

View reviewed changes

Comment thread tool/tctl/common/autoupdate_command.go Outdated

public-teleport-github-review-bot Bot removed the request for review from fheinecke January 8, 2025 13:22

Update tool/tctl/common/autoupdate_command.go

776a0e2

Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com>

vapopov enabled auto-merge January 8, 2025 16:21

vapopov added backport/branch/v15 labels Jan 8, 2025

vapopov added this pull request to the merge queue Jan 8, 2025

Merged via the queue into master with commit cf01dc4 Jan 8, 2025

vapopov deleted the vapopov/tctl-autoupdate-command branch January 8, 2025 17:07

vapopov mentioned this pull request Jan 11, 2025

[v17] Add client tools auto update tctl commands #50971

Merged

vapopov mentioned this pull request Jan 11, 2025

[v16] Add client tools auto update tctl commands #50972

Merged

vapopov mentioned this pull request Feb 11, 2025

[v14] Client tools managed updates #52025

Merged

vapopov mentioned this pull request Feb 11, 2025

[v15] Add client tools auto update tctl commands #52045

Merged

	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-mode", `Sets the mode to enable or disable tools auto update in cluster.`).EnumVar(&c.mode, "enabled", "disabled","on", "off")


		clientToolsCmd := autoUpdateCmd.Command("client-tools", "Client tools auto update commands.")

		c.upsertCmd = clientToolsCmd.Command("update", "Edit client tools auto update configuration.")

	* Copyright (C) 2024 Gravitational, Inc.
	* Copyright (C) 2025 Gravitational, Inc.

	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")

Conversation

vapopov commented Oct 18, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hugoShaka Oct 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vapopov commented Oct 29, 2024

Uh oh!

sclevine commented Oct 29, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

public-teleport-github-review-bot Bot commented Jan 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hugoShaka Oct 23, 2024 •

edited

Loading