From 74c178a977dd483a6f065379608b397eba4965f3 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Fri, 12 Sep 2025 10:46:18 -0700 Subject: [PATCH] Fix client tools managed updates sequential update (#58954) * Fix CTMU sequential update * Preserve the archive directory structure * Disable compression for integration tests launched with the `-race` flag, since this creates a heavy CPU load due to the design of the `compress/gzip` library. Filter out managed updates integration tests from the Kubernetes integration test suite, because even when executed with the `-run` filter, `TestMain` is still executed and introduces additional load during test initialization. * Check re-exec env variable to ensure that remote check is not ignored * Apply suggestions from code review Co-authored-by: Hugo Shaka * Disable deprecated `~/.tsh/bin/tsh` path --------- Co-authored-by: Hugo Shaka --- Makefile | 2 +- integration/autoupdate/tools/main_test.go | 79 ++++++++++-------- .../autoupdate/tools/updater/modules.go | 22 +++-- .../autoupdate/tools/updater/tctl/main.go | 4 +- .../autoupdate/tools/updater/tsh/main.go | 4 +- integration/autoupdate/tools/updater_test.go | 1 + .../autoupdate/tools/updater_tsh_test.go | 80 +++++++++++++++---- integration/helpers/archive/packaging.go | 48 ++++++++++- lib/autoupdate/tools/helper.go | 16 ++-- lib/autoupdate/tools/updater.go | 35 ++++---- lib/autoupdate/tools/utils.go | 9 +++ lib/autoupdate/tools/utils_test.go | 52 ++++++++++++ lib/service/service.go | 2 +- lib/utils/packaging/unarchive.go | 15 +++- lib/utils/packaging/unarchive_test.go | 16 ++-- lib/utils/packaging/unarchive_unix.go | 10 ++- 16 files changed, 292 insertions(+), 103 deletions(-) create mode 100644 lib/autoupdate/tools/utils_test.go diff --git a/Makefile b/Makefile index bba4b3e663664..4ea0a7aaa8993 100644 --- a/Makefile +++ b/Makefile @@ -1072,7 +1072,7 @@ integration: $(TEST_LOG_DIR) ensure-gotestsum INTEGRATION_KUBE_REGEX := TestKube.* .PHONY: integration-kube integration-kube: FLAGS ?= -v -race -integration-kube: PACKAGES = $(shell go list ./... | grep 'integration\([^s]\|$$\)') +integration-kube: PACKAGES = $(shell go list ./... | grep 'integration\([^s]\|$$\)' | grep -v 'integration/autoupdate') integration-kube: $(TEST_LOG_DIR) ensure-gotestsum @echo KUBECONFIG is: $(KUBECONFIG), TEST_KUBE: $(TEST_KUBE) $(CGOFLAG) go test -json -run "$(INTEGRATION_KUBE_REGEX)" $(PACKAGES) $(FLAGS) \ diff --git a/integration/autoupdate/tools/main_test.go b/integration/autoupdate/tools/main_test.go index ec69eeeb90082..7d2427ff13d89 100644 --- a/integration/autoupdate/tools/main_test.go +++ b/integration/autoupdate/tools/main_test.go @@ -29,7 +29,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" "runtime" "strings" @@ -38,9 +37,13 @@ import ( "github.com/gravitational/trace" "github.com/gravitational/teleport/api/constants" + "github.com/gravitational/teleport/integration/autoupdate/tools/updater/tctl" + "github.com/gravitational/teleport/integration/autoupdate/tools/updater/tsh" "github.com/gravitational/teleport/integration/helpers/archive" + "github.com/gravitational/teleport/lib/autoupdate" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/modules/modulestest" + "github.com/gravitational/teleport/lib/utils" ) const ( @@ -51,8 +54,10 @@ const ( var ( // testVersions list of the pre-compiled binaries with encoded versions to check. testVersions = []string{ - "1.2.3", - "3.2.1", + "1.0.0", + "2.0.0", + "3.0.0", + "4.0.0", } limitedWriter = newLimitedResponseWriter() @@ -61,6 +66,19 @@ var ( ) func TestMain(m *testing.M) { + executable, err := os.Executable() + if err != nil { + log.Fatal("failed to get executable name", err) + } + switch filepath.Base(executable) { + case "tsh", "tsh.exe": + tsh.Main() + return + case "tctl", "tctl.exe": + tctl.Main() + return + } + modules.SetInsecureTestMode(true) modules.SetModules(&modulestest.Modules{TestBuildType: modules.BuildCommunity}) ctx := context.Background() @@ -74,6 +92,12 @@ func TestMain(m *testing.M) { log.Fatalf("failed to create temporary directory: %v", err) } + for _, version := range testVersions { + if err := buildAndArchiveApps(ctx, tmp, version); err != nil { + log.Fatalf("failed to build testing app binary archive: %v", err) + } + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { filePath := filepath.Join(tmp, r.URL.Path) switch { @@ -84,10 +108,8 @@ func TestMain(m *testing.M) { } })) baseURL = server.URL - for _, version := range testVersions { - if err := buildAndArchiveApps(ctx, tmp, version, server.URL); err != nil { - log.Fatalf("failed to build testing app binary archive: %v", err) - } + if err := os.Setenv(autoupdate.BaseURLEnvVar, server.URL); err != nil { + log.Fatalf("failed to set base URL environment variable: %v", err) } // Run tests after binary is built. @@ -100,6 +122,9 @@ func TestMain(m *testing.M) { if err := os.RemoveAll(toolsDir); err != nil { log.Fatalf("failed to remove tools directory: %v", err) } + if err := os.Unsetenv(autoupdate.BaseURLEnvVar); err != nil { + log.Fatalf("failed to unset %q environment variable: %v", autoupdate.BaseURLEnvVar, err) + } os.Exit(code) } @@ -133,18 +158,25 @@ func serve256File(w http.ResponseWriter, _ *http.Request, filePath string) { } // buildAndArchiveApps compiles the updater integration and pack it depends on platform is used. -func buildAndArchiveApps(ctx context.Context, path string, version string, baseURL string) error { +func buildAndArchiveApps(ctx context.Context, path string, version string) error { versionPath := filepath.Join(path, version) for _, app := range []string{"tsh", "tctl"} { - output := filepath.Join(versionPath, app) + output := filepath.Join(versionPath, version, app) switch runtime.GOOS { case constants.WindowsOS: - output = filepath.Join(versionPath, app+".exe") + output = filepath.Join(versionPath, version, app+".exe") case constants.DarwinOS: - output = filepath.Join(versionPath, app+".app", "Contents", "MacOS", app) + output = filepath.Join(versionPath, app+".app", "Contents", "MacOS", version, app) } - if err := buildBinary(output, version, baseURL, app); err != nil { - return trace.Wrap(err) + if err := os.MkdirAll(filepath.Dir(output), 0755); err != nil { + return err + } + testBinary, err := os.Executable() + if err != nil { + return err + } + if err := utils.CopyFile(testBinary, output, 0o755); err != nil { + return err } } switch runtime.GOOS { @@ -153,26 +185,9 @@ func buildAndArchiveApps(ctx context.Context, path string, version string, baseU return trace.Wrap(archive.CompressDirToPkgFile(ctx, versionPath, archivePath, "com.example.pkgtest")) case constants.WindowsOS: archivePath := filepath.Join(path, fmt.Sprintf("teleport-v%s-windows-amd64-bin.zip", version)) - return trace.Wrap(archive.CompressDirToZipFile(ctx, versionPath, archivePath)) + return trace.Wrap(archive.CompressDirToZipFile(ctx, versionPath, archivePath, archive.WithNoCompress())) default: archivePath := filepath.Join(path, fmt.Sprintf("teleport-v%s-linux-%s-bin.tar.gz", version, runtime.GOARCH)) - return trace.Wrap(archive.CompressDirToTarGzFile(ctx, versionPath, archivePath)) + return trace.Wrap(archive.CompressDirToTarGzFile(ctx, versionPath, archivePath, archive.WithNoCompress())) } } - -// buildBinary executes command to build client tool binary with updater logic for testing. -func buildBinary(output string, version string, baseURL string, app string) error { - cmd := exec.Command( - "go", "build", "-o", output, - "-ldflags", strings.Join([]string{ - fmt.Sprintf("-X 'github.com/gravitational/teleport/integration/autoupdate/tools/updater.version=%s'", version), - fmt.Sprintf("-X 'github.com/gravitational/teleport/lib/autoupdate/tools.version=%s'", version), - fmt.Sprintf("-X 'github.com/gravitational/teleport/lib/autoupdate/tools.baseURL=%s'", baseURL), - }, " "), - fmt.Sprintf("./updater/%s", app), - ) - cmd.Stdout = os.Stdout - cmd.Stderr = os.Stderr - - return trace.Wrap(cmd.Run()) -} diff --git a/integration/autoupdate/tools/updater/modules.go b/integration/autoupdate/tools/updater/modules.go index 5597a111daccc..de196c8592abd 100644 --- a/integration/autoupdate/tools/updater/modules.go +++ b/integration/autoupdate/tools/updater/modules.go @@ -23,16 +23,18 @@ import ( "crypto" "fmt" "os" + "path/filepath" + "strings" "time" "github.com/gravitational/trace" - "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/types/accesslist" "github.com/gravitational/teleport/api/utils/keys" "github.com/gravitational/teleport/api/utils/keys/hardwarekey" "github.com/gravitational/teleport/entitlements" + "github.com/gravitational/teleport/lib/autoupdate/tools" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/tlsca" ) @@ -44,9 +46,19 @@ const ( TestBuild = "UPDATER_TEST_BUILD" ) -var ( - version = teleport.Version -) +func init() { + path, err := os.Executable() + if err != nil { + return + } + // For the integration test we use a pattern where the version is encoded + // in the directory name to simplify usage and avoid recompiling each + // individual binary. + parts := strings.Split(path, string(filepath.Separator)) + if len(parts) > 2 { + tools.Version = parts[len(parts)-2] + } +} type TestModules struct{} @@ -83,7 +95,7 @@ func (p *TestModules) LicenseExpiry() time.Time { // PrintVersion prints the Teleport version. func (p *TestModules) PrintVersion() { - fmt.Printf("Teleport v%v git\n", version) + fmt.Printf("Teleport v%v git\n", tools.Version) } // Features returns supported features diff --git a/integration/autoupdate/tools/updater/tctl/main.go b/integration/autoupdate/tools/updater/tctl/main.go index bbf894618e3f6..40338dc19a926 100644 --- a/integration/autoupdate/tools/updater/tctl/main.go +++ b/integration/autoupdate/tools/updater/tctl/main.go @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -package main +package tctl import ( "context" @@ -27,7 +27,7 @@ import ( tctl "github.com/gravitational/teleport/tool/tctl/common" ) -func main() { +func Main() { ctx, cancel := stacksignal.GetSignalHandler().NotifyContext(context.Background()) defer cancel() diff --git a/integration/autoupdate/tools/updater/tsh/main.go b/integration/autoupdate/tools/updater/tsh/main.go index 73abe237f6436..15680856c86ee 100644 --- a/integration/autoupdate/tools/updater/tsh/main.go +++ b/integration/autoupdate/tools/updater/tsh/main.go @@ -16,7 +16,7 @@ * along with this program. If not, see . */ -package main +package tsh import ( "context" @@ -30,7 +30,7 @@ import ( tsh "github.com/gravitational/teleport/tool/tsh/common" ) -func main() { +func Main() { ctx, cancel := stacksignal.GetSignalHandler().NotifyContext(context.Background()) defer cancel() diff --git a/integration/autoupdate/tools/updater_test.go b/integration/autoupdate/tools/updater_test.go index fa258f4cc244e..d4fd382228592 100644 --- a/integration/autoupdate/tools/updater_test.go +++ b/integration/autoupdate/tools/updater_test.go @@ -219,6 +219,7 @@ func TestUpdateForOSSBuild(t *testing.T) { // Enable OSS build. t.Setenv(updater.TestBuild, modules.BuildOSS) + t.Setenv(autoupdate.BaseURLEnvVar, "") // Fetch compiled test binary with updater logic and install to $TELEPORT_HOME. updater := tools.NewUpdater( diff --git a/integration/autoupdate/tools/updater_tsh_test.go b/integration/autoupdate/tools/updater_tsh_test.go index 217c588dd148b..4d95faea5b817 100644 --- a/integration/autoupdate/tools/updater_tsh_test.go +++ b/integration/autoupdate/tools/updater_tsh_test.go @@ -27,7 +27,9 @@ import ( "os/exec" "path/filepath" "testing" + "time" + "github.com/jonboulle/clockwork" "github.com/stretchr/testify/require" "gopkg.in/yaml.v3" @@ -40,6 +42,7 @@ import ( "github.com/gravitational/teleport/lib/autoupdate/tools" "github.com/gravitational/teleport/lib/client" "github.com/gravitational/teleport/lib/service" + "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/utils" testserver "github.com/gravitational/teleport/tool/teleport/testenv" ) @@ -52,7 +55,7 @@ import ( // $ tsh loginByAlias // $ tctl status // $ tsh version -// Teleport v3.2.1 +// Teleport v2.0.0 func TestAliasLoginWithUpdater(t *testing.T) { ctx := context.Background() @@ -63,7 +66,7 @@ func TestAliasLoginWithUpdater(t *testing.T) { proxyAddr, err := rootServer.ProxyWebAddr() require.NoError(t, err) - // Fetch compiled test binary and install to tools dir [v1.2.3]. + // Fetch compiled test binary and install to tools dir [v1.0.0]. updater := tools.NewUpdater(installDir, testVersions[0], tools.WithBaseURL(baseURL)) require.NoError(t, updater.Update(ctx, testVersions[0])) tshPath, err := updater.ToolPath("tsh", testVersions[0]) @@ -101,7 +104,7 @@ func TestAliasLoginWithUpdater(t *testing.T) { require.NoError(t, cmd.Run()) // Run version command to verify that login command executed auto update and - // tsh was upgraded to [v3.2.1]. + // tsh was upgraded to [v2.0.0]. cmd = exec.CommandContext(ctx, tshPath, "version") out, err = cmd.Output() require.NoError(t, err) @@ -111,14 +114,51 @@ func TestAliasLoginWithUpdater(t *testing.T) { require.Contains(t, string(out), fmt.Sprintf("Re-executed from version: %s", testVersions[0])) } +// TestSequentialUpdate runs test cluster with sequential changing version required for +// client tools for managed updates. After each new login we should receive updated version. +func TestSequentialUpdate(t *testing.T) { + ctx := context.Background() + + rootServer, _, installDir := bootstrapTestServer(t) + + // Assign alias to the login command for test cluster. + proxyAddr, err := rootServer.ProxyWebAddr() + require.NoError(t, err) + + // Fetch compiled test binary and install to tools dir [v1.0.0]. + updater := tools.NewUpdater(installDir, testVersions[0], tools.WithBaseURL(baseURL)) + require.NoError(t, updater.Update(ctx, testVersions[0])) + tshPath, err := updater.ToolPath("tsh", testVersions[0]) + require.NoError(t, err) + + for _, testVersion := range testVersions[1:] { + // Set cluster version to be upgraded. + setupManagedUpdates(t, rootServer.GetAuthServer(), autoupdate.ToolsUpdateModeEnabled, testVersion) + + cmd := exec.CommandContext(ctx, tshPath, + "login", "--proxy", proxyAddr.String(), "--insecure", "--user", "alice", "--auth", constants.LocalConnector) + cmd.Env = os.Environ() + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + require.NoError(t, cmd.Run()) + + // Run version command to verify that login command executed auto update and + // tsh was upgraded to [testVersion]. + cmd = exec.CommandContext(ctx, tshPath, "version") + out, err := cmd.Output() + require.NoError(t, err) + matchVersion(t, string(out), testVersion) + } +} + // TestLoginWithUpdaterAndProfile runs test cluster with disabled managed updates for client tools, // verifies that if we set env variable during login we keep using updated version. // // # Managed updates: disabled. -// $ TELEPORT_TOOLS_VERSION=3.2.1 tsh login --proxy proxy.example.com +// $ TELEPORT_TOOLS_VERSION=2.0.0 tsh login --proxy proxy.example.com // # Check that created profile after login has enabled autoupdates flag. // $ tsh version -// Teleport v3.2.1 +// Teleport v2.0.0 func TestLoginWithUpdaterAndProfile(t *testing.T) { ctx := context.Background() @@ -128,7 +168,7 @@ func TestLoginWithUpdaterAndProfile(t *testing.T) { proxyAddr, err := rootServer.ProxyWebAddr() require.NoError(t, err) - // Fetch compiled test binary and install to tools dir [v1.2.3]. + // Fetch compiled test binary and install to tools dir [v1.0.0]. updater := tools.NewUpdater(installDir, testVersions[0], tools.WithBaseURL(baseURL)) require.NoError(t, updater.Update(ctx, testVersions[0])) tshPath, err := updater.ToolPath("tsh", testVersions[0]) @@ -146,7 +186,7 @@ func TestLoginWithUpdaterAndProfile(t *testing.T) { require.NoError(t, os.Unsetenv("TELEPORT_TOOLS_VERSION")) // Run version command to verify that login command executed auto update and - // tsh was upgraded to [v3.2.1]. + // tsh was upgraded to [v2.0.0]. cmd = exec.CommandContext(ctx, tshPath, "version") out, err := cmd.Output() require.NoError(t, err) @@ -157,11 +197,11 @@ func TestLoginWithUpdaterAndProfile(t *testing.T) { // verifies that after first update and disabling. // // # Managed updates: disabled. -// $ TELEPORT_TOOLS_VERSION=3.2.1 tsh version -// Teleport v3.2.1 +// $ TELEPORT_TOOLS_VERSION=2.0.0 tsh version +// Teleport v2.0.0 // $ tsh login --proxy proxy.example.com // $ tsh version -// Teleport v1.2.3 +// Teleport v1.0.0 func TestLoginWithDisabledUpdateInProfile(t *testing.T) { ctx := context.Background() @@ -171,7 +211,7 @@ func TestLoginWithDisabledUpdateInProfile(t *testing.T) { proxyAddr, err := rootServer.ProxyWebAddr() require.NoError(t, err) - // Fetch compiled test binary and install to tools dir [v1.2.3]. + // Fetch compiled test binary and install to tools dir [v1.0.0]. updater := tools.NewUpdater(installDir, testVersions[0], tools.WithBaseURL(baseURL)) require.NoError(t, updater.Update(ctx, testVersions[0])) tshPath, err := updater.ToolPath("tsh", testVersions[0]) @@ -197,7 +237,7 @@ func TestLoginWithDisabledUpdateInProfile(t *testing.T) { require.NoError(t, cmd.Run()) // Run version command to verify that login command executed auto update and - // tsh was upgraded to [v3.2.1]. + // tsh was upgraded to [v2.0.0]. cmd = exec.CommandContext(ctx, tshPath, "version") out, err = cmd.Output() require.NoError(t, err) @@ -210,10 +250,10 @@ func TestLoginWithDisabledUpdateInProfile(t *testing.T) { // // # Managed updates: disabled. // $ tsh login --proxy proxy.example.com -// $ TELEPORT_TOOLS_VERSION=3.2.1 tsh version -// Teleport v3.2.1 +// $ TELEPORT_TOOLS_VERSION=2.0.0 tsh version +// Teleport v2.0.0 // $ tsh version -// Teleport v1.2.3 +// Teleport v1.0.0 func TestLoginWithDisabledUpdateForcedByEnv(t *testing.T) { ctx := context.Background() @@ -223,7 +263,7 @@ func TestLoginWithDisabledUpdateForcedByEnv(t *testing.T) { proxyAddr, err := rootServer.ProxyWebAddr() require.NoError(t, err) - // Fetch compiled test binary and install to tools dir [v1.2.3]. + // Fetch compiled test binary and install to tools dir [v1.0.0]. updater := tools.NewUpdater(installDir, testVersions[0], tools.WithBaseURL(baseURL)) require.NoError(t, updater.Update(ctx, testVersions[0])) tshPath, err := updater.ToolPath("tsh", testVersions[0]) @@ -249,7 +289,7 @@ func TestLoginWithDisabledUpdateForcedByEnv(t *testing.T) { require.NoError(t, os.Unsetenv("TELEPORT_TOOLS_VERSION")) // Run version command to verify that login command executed auto update and - // tsh is version [v1.2.3] since it was requested not during login and cluster + // tsh is version [v1.0.0] since it was requested not during login and cluster // has disabled managed updates. cmd = exec.CommandContext(ctx, tshPath, "version") out, err = cmd.Output() @@ -316,6 +356,9 @@ func bootstrapTestServer(t *testing.T) (*service.TeleportProcess, string, string testserver.WithBootstrap(alice), testserver.WithClusterName(t, "root"), testserver.WithAuthPreference(ap), + testserver.WithConfig(func(cfg *servicecfg.Config) { + cfg.Clock = clockwork.NewFakeClock() + }), ) authService := rootServer.GetAuthServer() @@ -348,4 +391,7 @@ func setupManagedUpdates(t *testing.T, server *auth.Server, muMode string, muVer require.NoError(t, err) _, err = server.UpsertAutoUpdateVersion(ctx, version) require.NoError(t, err) + + // Expire the fn cache to force the next answer to be fresh. + server.GetClock().(clockwork.FakeClock).Advance(20 * time.Second) } diff --git a/integration/helpers/archive/packaging.go b/integration/helpers/archive/packaging.go index ee237749115a3..e14b385960b83 100644 --- a/integration/helpers/archive/packaging.go +++ b/integration/helpers/archive/packaging.go @@ -35,9 +35,28 @@ import ( "github.com/gravitational/teleport" ) +// Option configures compression behavior. +type Option func(*options) + +type options struct { + noCompress bool +} + +// WithNoCompress disables compression. +func WithNoCompress() Option { + return func(o *options) { + o.noCompress = true + } +} + // CompressDirToZipFile compresses a source directory into `.zip` format and stores at `archivePath`, // preserving the relative file path structure of the source directory. -func CompressDirToZipFile(ctx context.Context, sourceDir, archivePath string) (err error) { +func CompressDirToZipFile(ctx context.Context, sourceDir, archivePath string, opts ...Option) (err error) { + o := &options{} + for _, opt := range opts { + opt(o) + } + archive, err := os.Create(archivePath) if err != nil { return trace.Wrap(err) @@ -71,7 +90,16 @@ func CompressDirToZipFile(ctx context.Context, sourceDir, archivePath string) (e if err != nil { return trace.Wrap(err) } - zipFileWriter, err := zipWriter.Create(relPath) + header, err := zip.FileInfoHeader(info) + if err != nil { + return trace.Wrap(err) + } + header.Name = relPath + header.Method = zip.Deflate + if o.noCompress { + header.Method = zip.Store + } + zipFileWriter, err := zipWriter.CreateHeader(header) if err != nil { return trace.Wrap(err) } @@ -92,7 +120,12 @@ func CompressDirToZipFile(ctx context.Context, sourceDir, archivePath string) (e // CompressDirToTarGzFile compresses a source directory into .tar.gz format and stores at `archivePath`, // preserving the relative file path structure of the source directory. -func CompressDirToTarGzFile(ctx context.Context, sourceDir, archivePath string) (err error) { +func CompressDirToTarGzFile(ctx context.Context, sourceDir, archivePath string, opts ...Option) (err error) { + o := &options{} + for _, opt := range opts { + opt(o) + } + archive, err := os.Create(archivePath) if err != nil { return trace.Wrap(err) @@ -108,7 +141,14 @@ func CompressDirToTarGzFile(ctx context.Context, sourceDir, archivePath string) } } }() - gzipWriter := gzip.NewWriter(archive) + level := gzip.DefaultCompression + if o.noCompress { + level = gzip.NoCompression + } + gzipWriter, err := gzip.NewWriterLevel(archive, level) + if err != nil { + return trace.Wrap(err) + } tarWriter := tar.NewWriter(gzipWriter) err = filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error { if err != nil { diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index bad8752e5123f..3348927deb2d8 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -36,15 +36,16 @@ import ( // Variables might to be overridden during compilation time for integration tests. var ( - // version is the current version of the Teleport. - version = teleport.Version - // baseURL is CDN URL for downloading official Teleport packages. - baseURL = autoupdate.DefaultBaseURL + // Version is the current version of the Teleport. + // The variable is overloaded during integration tests to emulate different + // Teleport versions. See `integration/autoupdate/tools/updater/modules.go`. + Version = teleport.Version ) // newUpdater inits the updater with default base URL and creates directory // if it doesn't exist. func newUpdater(toolsDir string) (*Updater, error) { + baseURL := autoupdate.DefaultBaseURL // Overrides default base URL for custom CDN for downloading updates. if envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar); envBaseURL != "" { baseURL = envBaseURL @@ -55,7 +56,7 @@ func newUpdater(toolsDir string) (*Updater, error) { return nil, trace.Wrap(err) } - return NewUpdater(toolsDir, version, WithBaseURL(baseURL)), nil + return NewUpdater(toolsDir, Version, WithBaseURL(baseURL)), nil } // CheckAndUpdateLocal verifies if the TELEPORT_TOOLS_VERSION environment variable @@ -127,7 +128,10 @@ func CheckAndUpdateLocal(ctx context.Context, currentProfileName string, reExecA func CheckAndUpdateRemote(ctx context.Context, currentProfileName string, insecure bool, reExecArgs []string) error { // If client tools updates are explicitly disabled, we want to catch this as soon as possible // so we don't try to read te user home directory, fail, and log warnings. - if os.Getenv(teleportToolsVersionEnv) == teleportToolsVersionEnvDisabled { + // If we are re-executed, we ignore the "off" version because some previous Teleport versions + // are disabling execution too aggressively and this causes stuck updates. + // If "off" was set by the user, we would not be re-executed. + if os.Getenv(teleportToolsVersionEnv) == teleportToolsVersionEnvDisabled && os.Getenv(teleportToolsVersionReExecEnv) == "" { return nil } diff --git a/lib/autoupdate/tools/updater.go b/lib/autoupdate/tools/updater.go index 348d6f581c7bc..1e2cb437791e1 100644 --- a/lib/autoupdate/tools/updater.go +++ b/lib/autoupdate/tools/updater.go @@ -34,7 +34,6 @@ import ( "path/filepath" "regexp" "runtime" - "strings" "syscall" "time" @@ -214,6 +213,12 @@ func (u *Updater) CheckRemote(ctx context.Context, proxyAddr string, insecure bo proxyHost := utils.TryHost(proxyAddr) // Check if the user has requested a specific version of client tools. requestedVersion := os.Getenv(teleportToolsVersionEnv) + // If we are re-executed, we ignore the "off" version because some previous Teleport versions + // are disabling execution too aggressively and this causes stuck updates. + // If "off" was set by the user, we would not be re-executed. + if requestedVersion == teleportToolsVersionEnvDisabled && os.Getenv(teleportToolsVersionReExecEnv) != "" { + requestedVersion = "" + } switch requestedVersion { // The user has turned off any form of automatic updates. case teleportToolsVersionEnvDisabled: @@ -412,28 +417,22 @@ func (u *Updater) Exec(ctx context.Context, toolsVersion string, args []string) return 0, trace.Wrap(err) } - for _, unset := range []string{ + env := filterEnvs(os.Environ(), []string{ teleportToolsVersionReExecEnv, teleportToolsDirsEnv, - } { - if err := os.Unsetenv(unset); err != nil { - return 0, trace.Wrap(err) - } - } - env := append(os.Environ(), fmt.Sprintf("%s=%s", teleportToolsDirsEnv, u.toolsDir)) + }) + env = append(env, fmt.Sprintf("%s=%s", teleportToolsVersionReExecEnv, u.localVersion)) + env = append(env, fmt.Sprintf("%s=%s", teleportToolsDirsEnv, u.toolsDir)) // To prevent re-execution loop we have to disable update logic for re-execution, - // by unsetting current tools version env variable and setting it to "off". - // The re-execution path and tools directory are absolute. Since the v2 logic - // no longer uses a static path, any re-execution from the tools directory - // must disable further re-execution. - if path == executablePath || strings.HasPrefix(path, u.toolsDir) { - if err := os.Unsetenv(teleportToolsVersionEnv); err != nil { - return 0, trace.Wrap(err) - } + // by unsetting current tools version env variable and setting it to "off" + // if same version is requested to be re-executed. + // We should also prevent further re-execution if the current version is run from + // the deprecated `~/.tsh/bin/tsh` path; otherwise, a downgrade could result in a loop. + if path == executablePath || executablePath == filepath.Join(u.toolsDir, filepath.Base(executablePath)) { + env = filterEnvs(env, []string{teleportToolsVersionEnv}) env = append(env, teleportToolsVersionEnv+"="+teleportToolsVersionEnvDisabled) - slog.DebugContext(ctx, "Disable next re-execution") + slog.DebugContext(ctx, "Disable re-execution") } - env = append(env, fmt.Sprintf("%s=%s", teleportToolsVersionReExecEnv, u.localVersion)) slog.DebugContext(ctx, "Re-execute updated version", "execute", path, "from", executablePath) if runtime.GOOS == constants.WindowsOS { diff --git a/lib/autoupdate/tools/utils.go b/lib/autoupdate/tools/utils.go index 0a32aee531ddd..0a8eb7d7c58a3 100644 --- a/lib/autoupdate/tools/utils.go +++ b/lib/autoupdate/tools/utils.go @@ -28,6 +28,7 @@ import ( "os/exec" "path/filepath" "runtime" + "slices" "strings" "time" @@ -254,3 +255,11 @@ func checkFreeSpace(path string, requested uint64) error { return nil } + +// filterEnvs excludes environment variables by the list of the keys. +func filterEnvs(envs []string, excludeKeys []string) []string { + return slices.DeleteFunc(envs, func(e string) bool { + parts := strings.SplitN(e, "=", 2) + return slices.Contains(excludeKeys, parts[0]) + }) +} diff --git a/lib/autoupdate/tools/utils_test.go b/lib/autoupdate/tools/utils_test.go new file mode 100644 index 0000000000000..0d5904bb526ad --- /dev/null +++ b/lib/autoupdate/tools/utils_test.go @@ -0,0 +1,52 @@ +/* + * Teleport + * 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 + * 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 tools + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestFilterEnv verifies excluding environment variables by the list of the keys. +func TestFilterEnv(t *testing.T) { + env := "TEST_ENV_WITHOUT_FILTER" + env1 := "TEST_ENV_WITHOUT_FILTER1" + env2 := "TEST_ENV_WITHOUT_FILTER2" + env3 := "TEST_ENV_WITHOUT_FILTER3" + + source := []string{ + env3, + env, + fmt.Sprint(env1, "=", "test"), + fmt.Sprint(teleportToolsVersionEnv, "=", teleportToolsVersionEnvDisabled), + fmt.Sprint(env2, "=", "test"), + env3, + env, + env3, + } + + assert.Equal(t, []string{ + env, + fmt.Sprint(env1, "=", "test"), + fmt.Sprint(env2, "=", "test"), + env, + }, filterEnvs(source, []string{teleportToolsVersionEnv, env3})) +} diff --git a/lib/service/service.go b/lib/service/service.go index 301bd64faa316..e96796810a3de 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -4608,7 +4608,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { IntegrationAppHandler: connectionsHandler, FeatureWatchInterval: utils.HalfJitter(web.DefaultFeatureWatchInterval * 2), } - webHandler, err := web.NewHandler(webConfig) + webHandler, err := web.NewHandler(webConfig, web.SetClock(process.Clock)) if err != nil { return trace.Wrap(err) } diff --git a/lib/utils/packaging/unarchive.go b/lib/utils/packaging/unarchive.go index ddcab0da5c5c2..2ca1a5ff00708 100644 --- a/lib/utils/packaging/unarchive.go +++ b/lib/utils/packaging/unarchive.go @@ -35,6 +35,11 @@ const ( // reservedFreeDisk is the predefined amount of free disk space (in bytes) required // to remain available after extracting Teleport binaries. reservedFreeDisk = 10 * 1024 * 1024 + // directoryPerm defines the permissions used when creating the directory + // structure, matching those from the archive. + directoryPerm = 0o755 + // binaryPerm defines the permissions applied to extracted binaries. + binaryPerm = 0o755 ) // RemoveWithSuffix removes all that matches the provided suffix, except for file or directory with `skipName`. @@ -118,8 +123,12 @@ func replaceZip(archivePath string, extractDir string, execNames []string) (map[ } defer file.Close() - dest := filepath.Join(extractDir, baseName) - destFile, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o755) + dest := filepath.Join(extractDir, zipFile.Name) + // Preserve the archive directory structure. + if err := os.MkdirAll(filepath.Dir(dest), directoryPerm); err != nil { + return trace.Wrap(err) + } + destFile, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, binaryPerm) if err != nil { return trace.Wrap(err) } @@ -130,7 +139,7 @@ func replaceZip(archivePath string, extractDir string, execNames []string) (map[ }(zipFile); err != nil { return nil, trace.Wrap(err) } - execPaths[baseName] = baseName + execPaths[baseName] = zipFile.Name } return execPaths, nil diff --git a/lib/utils/packaging/unarchive_test.go b/lib/utils/packaging/unarchive_test.go index 4b64083aec364..f4de77b598fce 100644 --- a/lib/utils/packaging/unarchive_test.go +++ b/lib/utils/packaging/unarchive_test.go @@ -54,7 +54,7 @@ func TestPackaging(t *testing.T) { require.NoError(t, err) archivePath := filepath.Join(toolsDir, "tsh.tar.gz") - err = archive.CompressDirToTarGzFile(ctx, sourceDir, archivePath) + err = archive.CompressDirToTarGzFile(ctx, sourceDir, archivePath, archive.WithNoCompress()) require.NoError(t, err) require.FileExists(t, archivePath, "archive not created") @@ -63,11 +63,10 @@ func TestPackaging(t *testing.T) { require.NoError(t, err) for tool, path := range toolsMap { assert.FileExists(t, filepath.Join(extractDir, path), fmt.Sprintf("script: %q not found", tool)) + data, err := os.ReadFile(filepath.Join(extractDir, path)) + require.NoError(t, err) + assert.Equal(t, script, string(data)) } - - data, err := os.ReadFile(filepath.Join(extractDir, "tsh")) - require.NoError(t, err) - assert.Equal(t, script, string(data)) }) t.Run("pkg", func(t *testing.T) { @@ -107,11 +106,10 @@ func TestPackaging(t *testing.T) { require.NoError(t, err) for tool, path := range toolsMap { assert.FileExists(t, filepath.Join(extractDir, path), fmt.Sprintf("script: %q not found", tool)) + data, err := os.ReadFile(filepath.Join(extractDir, path)) + require.NoError(t, err) + assert.Equal(t, script, string(data)) } - - data, err := os.ReadFile(filepath.Join(extractDir, "tsh")) - require.NoError(t, err) - assert.Equal(t, script, string(data)) }) } diff --git a/lib/utils/packaging/unarchive_unix.go b/lib/utils/packaging/unarchive_unix.go index a525fb21c0499..551aad22547e4 100644 --- a/lib/utils/packaging/unarchive_unix.go +++ b/lib/utils/packaging/unarchive_unix.go @@ -86,8 +86,12 @@ func replaceTarGz(archivePath string, extractDir string, execNames []string) (ma } if err = func(header *tar.Header) error { - dest := filepath.Join(extractDir, baseName) - destFile, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o755) + dest := filepath.Join(extractDir, header.Name) + // Preserve the archive directory structure. + if err := os.MkdirAll(filepath.Dir(dest), directoryPerm); err != nil { + return trace.Wrap(err) + } + destFile, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, binaryPerm) if err != nil { return trace.Wrap(err) } @@ -98,7 +102,7 @@ func replaceTarGz(archivePath string, extractDir string, execNames []string) (ma }(header); err != nil { return nil, trace.Wrap(err) } - execPaths[baseName] = baseName + execPaths[baseName] = header.Name } return execPaths, trace.Wrap(gzipReader.Close())