From 22ecd0fa061ad06c13de7bea703e71b959580cb4 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Fri, 31 Jan 2025 14:08:32 -0800 Subject: [PATCH 1/5] Add requirement to set base URL for OSS builds Fix progress bar for darwin platform --- .../autoupdate/tools/updater/modules.go | 16 ++++-- integration/autoupdate/tools/updater_test.go | 49 +++++++++++++++++++ lib/autoupdate/tools/helper.go | 22 ++++++++- lib/autoupdate/tools/updater.go | 7 +-- 4 files changed, 83 insertions(+), 11 deletions(-) diff --git a/integration/autoupdate/tools/updater/modules.go b/integration/autoupdate/tools/updater/modules.go index 28d03f42333df..017e5adb3cb70 100644 --- a/integration/autoupdate/tools/updater/modules.go +++ b/integration/autoupdate/tools/updater/modules.go @@ -22,6 +22,7 @@ import ( "context" "crypto" "fmt" + "os" "time" "github.com/gravitational/trace" @@ -35,8 +36,12 @@ import ( "github.com/gravitational/teleport/lib/tlsca" ) -// TestPassword is password generated during the test to login in test cluster. -const TestPassword = "UPDATER_TEST_PASSWORD" +const ( + // TestPassword is env var used for setting password generated during the test to login in test cluster. + TestPassword = "UPDATER_TEST_PASSWORD" + // TestBuild is env var for setting test build type during the test. + TestBuild = "UPDATER_TEST_BUILD" +) var ( version = teleport.Version @@ -54,17 +59,20 @@ func (p *TestModules) GetSuggestedAccessLists(context.Context, *tlsca.Identity, // BuildType returns build type (OSS or Enterprise) func (p *TestModules) BuildType() string { + if build := os.Getenv(TestBuild); build != "" { + return build + } return "CLI" } // IsEnterpriseBuild returns false for [TestModules]. func (p *TestModules) IsEnterpriseBuild() bool { - return false + return os.Getenv(TestBuild) == modules.BuildEnterprise } // IsOSSBuild returns false for [TestModules]. func (p *TestModules) IsOSSBuild() bool { - return false + return os.Getenv(TestBuild) == modules.BuildOSS } // LicenseExpiry returns the expiry date of the enterprise license, if applicable. diff --git a/integration/autoupdate/tools/updater_test.go b/integration/autoupdate/tools/updater_test.go index 0c05f2524caf0..21f2962d75768 100644 --- a/integration/autoupdate/tools/updater_test.go +++ b/integration/autoupdate/tools/updater_test.go @@ -34,7 +34,10 @@ import ( "github.com/stretchr/testify/require" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/integration/autoupdate/tools/updater" + "github.com/gravitational/teleport/lib/autoupdate" "github.com/gravitational/teleport/lib/autoupdate/tools" + "github.com/gravitational/teleport/lib/modules" ) var ( @@ -216,3 +219,49 @@ func TestUpdateInterruptSignal(t *testing.T) { } assert.Contains(t, output.String(), "Update progress:") } + +// TestUpdateForOSSBuild verifies the update logic for AGPL editions of Teleport requires +// base URL environment variable. +func TestUpdateForOSSBuild(t *testing.T) { + t.Setenv(types.HomeEnvVar, t.TempDir()) + ctx := context.Background() + + // Enable OSS build. + t.Setenv(updater.TestBuild, modules.BuildOSS) + + // Fetch compiled test binary with updater logic and install to $TELEPORT_HOME. + updater := tools.NewUpdater( + toolsDir, + testVersions[0], + tools.WithBaseURL(baseURL), + ) + err := updater.Update(ctx, testVersions[0]) + require.NoError(t, err) + + // Verify that requested update is ignored by OSS build and version wasn't updated. + cmd := exec.CommandContext(ctx, filepath.Join(toolsDir, "tsh"), "version") + cmd.Env = append( + os.Environ(), + fmt.Sprintf("%s=%s", teleportToolsVersion, testVersions[1]), + ) + out, err := cmd.Output() + require.NoError(t, err) + + matches := pattern.FindStringSubmatch(string(out)) + require.Len(t, matches, 2) + require.Equal(t, testVersions[0], matches[1]) + + // Next update is set with the base URL env variable, must download new version. + t.Setenv(autoupdate.BaseURLEnvVar, baseURL) + cmd = exec.CommandContext(ctx, filepath.Join(toolsDir, "tsh"), "version") + cmd.Env = append( + os.Environ(), + fmt.Sprintf("%s=%s", teleportToolsVersion, testVersions[1]), + ) + out, err = cmd.Output() + require.NoError(t, err) + + matches = pattern.FindStringSubmatch(string(out)) + require.Len(t, matches, 2) + require.Equal(t, testVersions[1], matches[1]) +} diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index f7d3e691b2ef4..6b329079f19f7 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -28,9 +28,15 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/lib/autoupdate" + "github.com/gravitational/teleport/lib/modules" stacksignal "github.com/gravitational/teleport/lib/utils/signal" ) +const ( + // warningOSSBuild is warning exposed to the user that build type without base url is disabled. + warningOSSBuild = "Client tools update is disabled. Use 'TELEPORT_CDN_BASE_URL' environment variable to set CDN base URL" +) + // Variables might to be overridden during compilation time for integration tests. var ( // version is the current version of the Teleport. @@ -53,8 +59,14 @@ func CheckAndUpdateLocal(ctx context.Context, reExecArgs []string) error { return nil } + bType := modules.GetModules().BuildType() + envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) + if bType == modules.BuildOSS && envBaseURL == "" { + slog.WarnContext(ctx, warningOSSBuild) + return nil + } // Overrides default base URL for custom CDN for downloading updates. - if envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar); envBaseURL != "" { + if envBaseURL != "" { baseURL = envBaseURL } @@ -86,8 +98,14 @@ func CheckAndUpdateRemote(ctx context.Context, proxy string, insecure bool, reEx slog.WarnContext(ctx, "Client tools update is disabled", "error", err) return nil } + bType := modules.GetModules().BuildType() + envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) + if bType == modules.BuildOSS && envBaseURL == "" { + slog.WarnContext(ctx, warningOSSBuild) + return nil + } // Overrides default base URL for custom CDN for downloading updates. - if envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar); envBaseURL != "" { + if envBaseURL != "" { baseURL = envBaseURL } diff --git a/lib/autoupdate/tools/updater.go b/lib/autoupdate/tools/updater.go index b148be735aedb..d18689b61bb93 100644 --- a/lib/autoupdate/tools/updater.go +++ b/lib/autoupdate/tools/updater.go @@ -409,11 +409,6 @@ func (u *Updater) downloadHash(ctx context.Context, url string) ([]byte, error) // downloadArchive downloads the archive package by `url` and writes content to the writer interface, // return calculated sha256 hash sum of the content. func (u *Updater) downloadArchive(ctx context.Context, url string, f io.Writer) ([]byte, error) { - // Display a progress bar before initiating the update request to inform the user that - // an update is in progress, allowing them the option to cancel before actual response - // which might be delayed with slow internet connection or complete isolation to CDN. - pw, finish := newProgressWriter(10) - defer finish() req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) if err != nil { return nil, trace.Wrap(err) @@ -429,6 +424,8 @@ func (u *Updater) downloadArchive(ctx context.Context, url string, f io.Writer) if resp.StatusCode != http.StatusOK { return nil, trace.BadParameter("bad status when downloading archive: %v", resp.StatusCode) } + pw, finish := newProgressWriter(10) + defer finish() if resp.ContentLength != -1 { if err := checkFreeSpace(u.toolsDir, uint64(resp.ContentLength)); err != nil { From 4600602ab9f571c4ce865ce598032ab273a6956e Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Fri, 31 Jan 2025 17:36:17 -0800 Subject: [PATCH 2/5] Show warning only before update --- lib/autoupdate/tools/helper.go | 23 +++-------------------- lib/autoupdate/tools/updater.go | 10 ++++++++++ 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index 6b329079f19f7..b8e4f818c5d2b 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -28,15 +28,9 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/lib/autoupdate" - "github.com/gravitational/teleport/lib/modules" stacksignal "github.com/gravitational/teleport/lib/utils/signal" ) -const ( - // warningOSSBuild is warning exposed to the user that build type without base url is disabled. - warningOSSBuild = "Client tools update is disabled. Use 'TELEPORT_CDN_BASE_URL' environment variable to set CDN base URL" -) - // Variables might to be overridden during compilation time for integration tests. var ( // version is the current version of the Teleport. @@ -59,14 +53,8 @@ func CheckAndUpdateLocal(ctx context.Context, reExecArgs []string) error { return nil } - bType := modules.GetModules().BuildType() - envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) - if bType == modules.BuildOSS && envBaseURL == "" { - slog.WarnContext(ctx, warningOSSBuild) - return nil - } // Overrides default base URL for custom CDN for downloading updates. - if envBaseURL != "" { + if envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar); envBaseURL != "" { baseURL = envBaseURL } @@ -98,14 +86,9 @@ func CheckAndUpdateRemote(ctx context.Context, proxy string, insecure bool, reEx slog.WarnContext(ctx, "Client tools update is disabled", "error", err) return nil } - bType := modules.GetModules().BuildType() - envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) - if bType == modules.BuildOSS && envBaseURL == "" { - slog.WarnContext(ctx, warningOSSBuild) - return nil - } + // Overrides default base URL for custom CDN for downloading updates. - if envBaseURL != "" { + if envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar); envBaseURL != "" { baseURL = envBaseURL } diff --git a/lib/autoupdate/tools/updater.go b/lib/autoupdate/tools/updater.go index d18689b61bb93..088ee4f28fb63 100644 --- a/lib/autoupdate/tools/updater.go +++ b/lib/autoupdate/tools/updater.go @@ -43,6 +43,7 @@ import ( "github.com/gravitational/teleport/api/client/webclient" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/autoupdate" + "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/packaging" ) @@ -57,6 +58,8 @@ const ( lockFileName = ".lock" // updatePackageSuffix is directory suffix used for package extraction in tools directory. updatePackageSuffix = "-update-pkg" + // warnMessageOSSBuild is warning exposed to the user that build type without base url is disabled. + warnMessageOSSBuild = "Client tools update is disabled. Use 'TELEPORT_CDN_BASE_URL' environment variable to set CDN base URL" ) var ( @@ -262,6 +265,13 @@ func (u *Updater) UpdateWithLock(ctx context.Context, updateToolsVersion string) // Update downloads requested version and replace it with existing one and cleanups the previous downloads // with defined updater directory suffix. func (u *Updater) Update(ctx context.Context, toolsVersion string) error { + // Disable update for the OSS build if custom base URL wasn't set. + envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) + if modules.GetModules().BuildType() == modules.BuildOSS && envBaseURL == "" { + slog.WarnContext(ctx, warnMessageOSSBuild) + return nil + } + // Get platform specific download URLs. packages, err := teleportPackageURLs(u.uriTemplate, u.baseURL, toolsVersion) if err != nil { From ea475fe575cb32061a1b7d47259b72d79b4a0b98 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Wed, 5 Feb 2025 11:51:46 -0800 Subject: [PATCH 3/5] Move error to teleportPackageURLs --- lib/autoupdate/tools/helper.go | 7 +++++++ lib/autoupdate/tools/updater.go | 10 ---------- lib/autoupdate/tools/utils.go | 7 +++++++ 3 files changed, 14 insertions(+), 10 deletions(-) diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index b8e4f818c5d2b..a40e5e659c530 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -111,6 +111,13 @@ func updateAndReExec(ctx context.Context, updater *Updater, toolsVersion string, // is required if the user passed in the TELEPORT_TOOLS_VERSION // explicitly. err := updater.UpdateWithLock(ctxUpdate, toolsVersion) + if err != nil && errors.Is(err, errNoBaseURL) { + // If base URL wasn't defined we have to cancel update and re-execution with warning. + slog.WarnContext(ctx, "Client tools updates are disabled because the server is licensed under AGPL "+ + "but Teleport-distributed binaries are licensed under Community Edition. To use Community Edition "+ + "builds or custom binaries, set the 'TELEPORT_CDN_BASE_URL' environment variable.") + return nil + } if err != nil && !errors.Is(err, context.Canceled) { return trace.Wrap(err) } diff --git a/lib/autoupdate/tools/updater.go b/lib/autoupdate/tools/updater.go index 088ee4f28fb63..d18689b61bb93 100644 --- a/lib/autoupdate/tools/updater.go +++ b/lib/autoupdate/tools/updater.go @@ -43,7 +43,6 @@ import ( "github.com/gravitational/teleport/api/client/webclient" "github.com/gravitational/teleport/api/constants" "github.com/gravitational/teleport/lib/autoupdate" - "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/packaging" ) @@ -58,8 +57,6 @@ const ( lockFileName = ".lock" // updatePackageSuffix is directory suffix used for package extraction in tools directory. updatePackageSuffix = "-update-pkg" - // warnMessageOSSBuild is warning exposed to the user that build type without base url is disabled. - warnMessageOSSBuild = "Client tools update is disabled. Use 'TELEPORT_CDN_BASE_URL' environment variable to set CDN base URL" ) var ( @@ -265,13 +262,6 @@ func (u *Updater) UpdateWithLock(ctx context.Context, updateToolsVersion string) // Update downloads requested version and replace it with existing one and cleanups the previous downloads // with defined updater directory suffix. func (u *Updater) Update(ctx context.Context, toolsVersion string) error { - // Disable update for the OSS build if custom base URL wasn't set. - envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) - if modules.GetModules().BuildType() == modules.BuildOSS && envBaseURL == "" { - slog.WarnContext(ctx, warnMessageOSSBuild) - return nil - } - // Get platform specific download URLs. packages, err := teleportPackageURLs(u.uriTemplate, u.baseURL, toolsVersion) if err != nil { diff --git a/lib/autoupdate/tools/utils.go b/lib/autoupdate/tools/utils.go index eb9e5f7ad8a9f..66cfd7633f09a 100644 --- a/lib/autoupdate/tools/utils.go +++ b/lib/autoupdate/tools/utils.go @@ -40,6 +40,8 @@ import ( "github.com/gravitational/teleport/lib/utils" ) +var errNoBaseURL = errors.New("baseURL is not defined") + // Dir returns the path to client tools in $TELEPORT_HOME/bin. func Dir() (string, error) { home := os.Getenv(types.HomeEnvVar) @@ -128,6 +130,11 @@ type packageURL struct { // teleportPackageURLs returns the URL for the Teleport archive to download. func teleportPackageURLs(uriTmpl string, baseURL, version string) ([]packageURL, error) { + envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) + if modules.GetModules().BuildType() == modules.BuildOSS && envBaseURL == "" { + return nil, errNoBaseURL + } + var flags autoupdate.InstallFlags m := modules.GetModules() if m.IsBoringBinary() { From 73cb99b6a044a33932c5520393d89d6460c67f3c Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Wed, 5 Feb 2025 13:26:08 -0800 Subject: [PATCH 4/5] Move error to teleportPackageURLs --- lib/autoupdate/tools/helper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index a40e5e659c530..4d5f13bb24496 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -113,7 +113,7 @@ func updateAndReExec(ctx context.Context, updater *Updater, toolsVersion string, err := updater.UpdateWithLock(ctxUpdate, toolsVersion) if err != nil && errors.Is(err, errNoBaseURL) { // If base URL wasn't defined we have to cancel update and re-execution with warning. - slog.WarnContext(ctx, "Client tools updates are disabled because the server is licensed under AGPL "+ + slog.WarnContext(ctx, "client tools updates are disabled because the server is licensed under AGPL "+ "but Teleport-distributed binaries are licensed under Community Edition. To use Community Edition "+ "builds or custom binaries, set the 'TELEPORT_CDN_BASE_URL' environment variable.") return nil From f27d61ed294a33eb68eadbf6e1448e405414ceae Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Wed, 5 Feb 2025 13:52:29 -0800 Subject: [PATCH 5/5] Fix linter --- lib/autoupdate/tools/helper.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index 4d5f13bb24496..c05618e430a92 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -31,6 +31,11 @@ import ( stacksignal "github.com/gravitational/teleport/lib/utils/signal" ) +// warnMessageOSSBuild is warning exposed to the user that build type without base url is disabled. +const warnMessageOSSBuild = "Client tools updates are disabled because the server is licensed under AGPL " + + "but Teleport-distributed binaries are licensed under Community Edition. To use Community Edition " + + "builds or custom binaries, set the 'TELEPORT_CDN_BASE_URL' environment variable." + // Variables might to be overridden during compilation time for integration tests. var ( // version is the current version of the Teleport. @@ -113,9 +118,7 @@ func updateAndReExec(ctx context.Context, updater *Updater, toolsVersion string, err := updater.UpdateWithLock(ctxUpdate, toolsVersion) if err != nil && errors.Is(err, errNoBaseURL) { // If base URL wasn't defined we have to cancel update and re-execution with warning. - slog.WarnContext(ctx, "client tools updates are disabled because the server is licensed under AGPL "+ - "but Teleport-distributed binaries are licensed under Community Edition. To use Community Edition "+ - "builds or custom binaries, set the 'TELEPORT_CDN_BASE_URL' environment variable.") + slog.WarnContext(ctx, warnMessageOSSBuild) return nil } if err != nil && !errors.Is(err, context.Canceled) {