Skip to content

Commit

Permalink
Restore priority of env check over remote check
Browse files Browse the repository at this point in the history
In case of double re-execution case we should stop second one to prevent loop re-execution
Drop localDir set during compilation
  • Loading branch information
vapopov committed Jan 17, 2025
1 parent 5961a09 commit 5156850
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 22 deletions.
1 change: 0 additions & 1 deletion integration/autoupdate/tools/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ func buildBinary(output string, toolsDir string, version string, baseURL string,
"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.toolsDir=%s'", toolsDir),
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),
}, " "),
Expand Down
16 changes: 11 additions & 5 deletions integration/autoupdate/tools/updater_tsh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ import (
// and after auto update this not leads to recursive alias re-execution.
func TestAliasLoginWithUpdater(t *testing.T) {
ctx := context.Background()
t.Setenv(types.HomeEnvVar, toolsDir)

homeDir := filepath.Join(t.TempDir(), "home")
require.NoError(t, os.MkdirAll(homeDir, 0700))
installDir := filepath.Join(t.TempDir(), "local")
require.NoError(t, os.MkdirAll(installDir, 0700))

t.Setenv(types.HomeEnvVar, homeDir)

alice, err := types.NewUser("alice")
require.NoError(t, err)
Expand Down Expand Up @@ -94,9 +100,9 @@ func TestAliasLoginWithUpdater(t *testing.T) {
// Assign alias to the login command for test cluster.
proxyAddr, err := rootServer.ProxyWebAddr()
require.NoError(t, err)
configPath := filepath.Join(toolsDir, client.TSHConfigPath)
configPath := filepath.Join(homeDir, client.TSHConfigPath)
require.NoError(t, os.MkdirAll(filepath.Dir(configPath), 0700))
executable := filepath.Join(toolsDir, "tsh")
executable := filepath.Join(installDir, "tsh")
out, err := yaml.Marshal(client.TSHConfig{
Aliases: map[string]string{
"loginalice": fmt.Sprintf(
Expand All @@ -109,7 +115,7 @@ func TestAliasLoginWithUpdater(t *testing.T) {
require.NoError(t, os.WriteFile(configPath, out, 0600))

// Fetch compiled test binary and install to tools dir [v1.2.3].
err = tools.NewUpdater(toolsDir, testVersions[0], tools.WithBaseURL(baseURL)).Update(ctx, testVersions[0])
err = tools.NewUpdater(installDir, testVersions[0], tools.WithBaseURL(baseURL)).Update(ctx, testVersions[0])
require.NoError(t, err)

// Execute alias command which must be transformed to the login command.
Expand All @@ -122,7 +128,7 @@ func TestAliasLoginWithUpdater(t *testing.T) {
require.NoError(t, cmd.Run())

// Verify tctl status after login.
cmd = exec.CommandContext(ctx, filepath.Join(toolsDir, "tctl"), "status", "--insecure")
cmd = exec.CommandContext(ctx, filepath.Join(installDir, "tctl"), "status", "--insecure")
cmd.Env = os.Environ()
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
Expand Down
22 changes: 8 additions & 14 deletions lib/autoupdate/tools/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@ var (
version = teleport.Version
// baseURL is CDN URL for downloading official Teleport packages.
baseURL = defaultBaseURL
// toolsDir is client tools directory for package download and installation.
toolsDir = ""
)

// CheckAndUpdateLocal verifies if the TELEPORT_TOOLS_VERSION environment variable
Expand All @@ -48,12 +46,10 @@ var (
// If $TELEPORT_HOME/bin contains downloaded client tools, it always re-executes
// using the version from the home directory.
func CheckAndUpdateLocal(ctx context.Context, reExecArgs []string) error {
var err error
if toolsDir == "" {
if toolsDir, err = Dir(); err != nil {
slog.WarnContext(ctx, "Client tools update is disabled", "error", err)
return nil
}
toolsDir, err := Dir()
if err != nil {
slog.WarnContext(ctx, "Client tools update is disabled", "error", err)
return nil
}

updater := NewUpdater(toolsDir, version, WithBaseURL(baseURL))
Expand All @@ -79,12 +75,10 @@ func CheckAndUpdateLocal(ctx context.Context, reExecArgs []string) error {
// If $TELEPORT_HOME/bin contains downloaded client tools, it always re-executes
// using the version from the home directory.
func CheckAndUpdateRemote(ctx context.Context, proxy string, insecure bool, reExecArgs []string) error {
var err error
if toolsDir == "" {
if toolsDir, err = Dir(); err != nil {
slog.WarnContext(ctx, "Client tools update is disabled", "error", err)
return nil
}
toolsDir, err := Dir()
if err != nil {
slog.WarnContext(ctx, "Client tools update is disabled", "error", err)
return nil
}
updater := NewUpdater(toolsDir, version, WithBaseURL(baseURL))
// The user has typed a command like `tsh ssh ...` without being logged in,
Expand Down
32 changes: 30 additions & 2 deletions lib/autoupdate/tools/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,11 +155,31 @@ func (u *Updater) CheckLocal() (version string, reExec bool, err error) {
return toolsVersion, true, nil
}

// CheckRemote checks against the Proxy Service to determine if client tools need updating by requesting
// CheckRemote first checks the version set by the environment variable. If not set or disabled,
// it checks against the Proxy Service to determine if client tools need updating by requesting
// the `webapi/find` handler, which stores information about the required client tools version to
// operate with this cluster. It returns the semantic version that needs updating and whether
// re-execution is necessary, by re-execution flag we understand that update and re-execute is required.
func (u *Updater) CheckRemote(ctx context.Context, proxyAddr string, insecure bool) (version string, reExec bool, err error) {
// Check if the user has requested a specific version of client tools.
requestedVersion := os.Getenv(teleportToolsVersionEnv)
switch requestedVersion {
// The user has turned off any form of automatic updates.
case "off":
return "", false, nil
// Requested version already the same as client version.
case u.localVersion:
return u.localVersion, false, nil
// No requested version, we continue.
case "":
// Requested version that is not the local one.
default:
if _, err := semver.NewVersion(requestedVersion); err != nil {
return "", false, trace.Wrap(err, "checking that request version is semantic")
}
return requestedVersion, true, nil
}

certPool, err := x509.SystemCertPool()
if err != nil {
return "", false, trace.Wrap(err)
Expand Down Expand Up @@ -317,7 +337,15 @@ func (u *Updater) Exec(args []string) (int, error) {
if err := os.Unsetenv(teleportToolsVersionEnv); err != nil {
return 0, trace.Wrap(err)
}
env := append(os.Environ(), teleportToolsVersionEnv+"=off")

env := os.Environ()
executablePath, err := os.Executable()
if err != nil {
return 0, trace.Wrap(err)
}
if path == executablePath {
env = append(env, teleportToolsVersionEnv+"=off")
}

if runtime.GOOS == constants.WindowsOS {
cmd := exec.Command(path, args...)
Expand Down

0 comments on commit 5156850

Please sign in to comment.