From 0f30ce65a0de3a2a2f41b5d21b096bdcbd8fd7be Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Wed, 5 Feb 2025 14:14:43 -0800 Subject: [PATCH 1/2] Default BaseURL only for community / enterprise editions (#51732) * Add requirement to set base URL for OSS builds Fix progress bar for darwin platform * Show warning only before update * Move error to teleportPackageURLs * Move error to teleportPackageURLs * Fix linter --- .../autoupdate/tools/updater/modules.go | 16 ++++-- integration/autoupdate/tools/updater_test.go | 49 +++++++++++++++++++ lib/autoupdate/tools/helper.go | 11 +++++ lib/autoupdate/tools/updater.go | 7 +-- lib/autoupdate/tools/utils.go | 7 +++ 5 files changed, 81 insertions(+), 9 deletions(-) diff --git a/integration/autoupdate/tools/updater/modules.go b/integration/autoupdate/tools/updater/modules.go index a01736489abd3..3b80318d82fcb 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(ctx context.Context, identity *tls // 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..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. @@ -86,6 +91,7 @@ func CheckAndUpdateRemote(ctx context.Context, proxy string, insecure bool, reEx slog.WarnContext(ctx, "Client tools update is disabled", "error", err) return nil } + // Overrides default base URL for custom CDN for downloading updates. if envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar); envBaseURL != "" { baseURL = envBaseURL @@ -110,6 +116,11 @@ 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, warnMessageOSSBuild) + 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 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 { 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 27e04c74ae0bac7313ed858d9b2d14e37dc70e95 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 10 Feb 2025 18:42:14 -0800 Subject: [PATCH 2/2] Fix setting modules build type for tsh and tctl (#51986) * Fix setting modules for tsh and tctl * Restore IsOSSBuild() * Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable. * Check only the build type of client tools --- Makefile | 22 ++++++++++++---------- lib/autoupdate/tools/helper.go | 12 +----------- lib/autoupdate/tools/updater.go | 2 +- lib/autoupdate/tools/utils.go | 10 ++++++---- 4 files changed, 20 insertions(+), 26 deletions(-) diff --git a/Makefile b/Makefile index 372bfa5bc39b2..21fd53b17ec8c 100644 --- a/Makefile +++ b/Makefile @@ -330,6 +330,14 @@ all: version binaries: $(MAKE) $(BINARIES) +# Appending new conditional settings for community build type for tools. +ifeq ("$(GITHUB_REPOSITORY_OWNER)","gravitational") +# TELEPORT_LDFLAGS and TOOLS_LDFLAGS if appended will overwrite the previous LDFLAGS set in the BUILDFLAGS. +# This is done here to prevent any changes to the (BUI)LDFLAGS passed to the other binaries +TELEPORT_LDFLAGS ?= -ldflags '$(GO_LDFLAGS) -X github.com/gravitational/teleport/lib/modules.teleportBuildType=community' +TOOLS_LDFLAGS ?= -ldflags '$(GO_LDFLAGS) -X github.com/gravitational/teleport/lib/modules.teleportBuildType=community' +endif + # By making these 3 targets below (tsh, tctl and teleport) PHONY we are solving # several problems: # * Build will rely on go build internal caching https://golang.org/doc/go1.10 at all times @@ -343,15 +351,9 @@ $(BUILDDIR)/tctl: @if [[ -z "$(LIBFIDO2_BUILD_TAG)" ]]; then \ echo 'Warning: Building tctl without libfido2. Install libfido2 to have access to MFA.' >&2; \ fi - GOOS=$(OS) GOARCH=$(ARCH) $(CGOFLAG) go build -tags "$(PAM_TAG) $(FIPS_TAG) $(LIBFIDO2_BUILD_TAG) $(PIV_BUILD_TAG) $(KUSTOMIZE_NO_DYNAMIC_PLUGIN)" -o $(BUILDDIR)/tctl $(BUILDFLAGS) ./tool/tctl + GOOS=$(OS) GOARCH=$(ARCH) $(CGOFLAG) go build -tags "$(PAM_TAG) $(FIPS_TAG) $(LIBFIDO2_BUILD_TAG) $(PIV_BUILD_TAG) $(KUSTOMIZE_NO_DYNAMIC_PLUGIN)" -o $(BUILDDIR)/tctl $(BUILDFLAGS) $(TOOLS_LDFLAGS) ./tool/tctl .PHONY: $(BUILDDIR)/teleport -# Appending new conditional settings for community build type -ifeq ("$(GITHUB_REPOSITORY_OWNER)","gravitational") -# TELEPORT_LDFLAGS if appended will overwrite the previous LDFLAGS set in the BUILDFLAGS. -# This is done here to prevent any changes to the (BUI)LDFLAGS passed to the other binaries -TELEPORT_LDFLAGS ?= -ldflags '$(GO_LDFLAGS) -X github.com/gravitational/teleport/lib/modules.teleportBuildType=community' -endif $(BUILDDIR)/teleport: ensure-webassets bpf-bytecode rdpclient GOOS=$(OS) GOARCH=$(ARCH) $(CGOFLAG) go build -tags "webassets_embed $(PAM_TAG) $(FIPS_TAG) $(BPF_TAG) $(WEBASSETS_TAG) $(RDPCLIENT_TAG) $(PIV_BUILD_TAG) $(KUSTOMIZE_NO_DYNAMIC_PLUGIN)" -o $(BUILDDIR)/teleport $(BUILDFLAGS) $(TELEPORT_LDFLAGS) ./tool/teleport @@ -364,7 +366,7 @@ $(BUILDDIR)/tsh: @if [[ -z "$(LIBFIDO2_BUILD_TAG)" ]]; then \ echo 'Warning: Building tsh without libfido2. Install libfido2 to have access to MFA.' >&2; \ fi - GOOS=$(OS) GOARCH=$(ARCH) $(CGOFLAG_TSH) go build -tags "$(FIPS_TAG) $(LIBFIDO2_BUILD_TAG) $(TOUCHID_TAG) $(PIV_BUILD_TAG) $(VNETDAEMON_TAG) $(KUSTOMIZE_NO_DYNAMIC_PLUGIN)" -o $(BUILDDIR)/tsh $(BUILDFLAGS) ./tool/tsh + GOOS=$(OS) GOARCH=$(ARCH) $(CGOFLAG_TSH) go build -tags "$(FIPS_TAG) $(LIBFIDO2_BUILD_TAG) $(TOUCHID_TAG) $(PIV_BUILD_TAG) $(VNETDAEMON_TAG) $(KUSTOMIZE_NO_DYNAMIC_PLUGIN)" -o $(BUILDDIR)/tsh $(BUILDFLAGS) $(TOOLS_LDFLAGS) ./tool/tsh .PHONY: $(BUILDDIR)/tbot # tbot is CGO-less by default except on Windows because lib/client/terminal/ wants CGO on this OS @@ -372,7 +374,7 @@ $(BUILDDIR)/tbot: TBOT_CGO_FLAGS ?= $(if $(filter windows,$(OS)),$(CGOFLAG)) # Build mode pie requires CGO $(BUILDDIR)/tbot: BUILDFLAGS_TBOT += $(if $(TBOT_CGO_FLAGS), -buildmode=pie) $(BUILDDIR)/tbot: - GOOS=$(OS) GOARCH=$(ARCH) $(TBOT_CGO_FLAGS) go build -tags "$(FIPS_TAG) $(KUSTOMIZE_NO_DYNAMIC_PLUGIN)" -o $(BUILDDIR)/tbot $(BUILDFLAGS_TBOT) ./tool/tbot + GOOS=$(OS) GOARCH=$(ARCH) $(TBOT_CGO_FLAGS) go build -tags "$(FIPS_TAG) $(KUSTOMIZE_NO_DYNAMIC_PLUGIN)" -o $(BUILDDIR)/tbot $(BUILDFLAGS_TBOT) $(TOOLS_LDFLAGS) ./tool/tbot TELEPORT_ARGS ?= start .PHONY: teleport-hot-reload @@ -1741,7 +1743,7 @@ changelog: # does not match version set it will fail to create a release. If tag doesn't exist it # will also fail to create a release. # -# For more information on release notes generation see: +# For more information on release notes generation see: # https://github.com/gravitational/shared-workflows/tree/gus/release-notes/tools/release-notes#readme .PHONY: create-github-release create-github-release: LATEST = false diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index c05618e430a92..499a7c5d377c5 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -31,11 +31,6 @@ 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. @@ -116,12 +111,7 @@ 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, warnMessageOSSBuild) - return nil - } - if err != nil && !errors.Is(err, context.Canceled) { + if err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, errNoBaseURL) { return trace.Wrap(err) } diff --git a/lib/autoupdate/tools/updater.go b/lib/autoupdate/tools/updater.go index d18689b61bb93..eded34049e473 100644 --- a/lib/autoupdate/tools/updater.go +++ b/lib/autoupdate/tools/updater.go @@ -263,7 +263,7 @@ func (u *Updater) UpdateWithLock(ctx context.Context, updateToolsVersion string) // with defined updater directory suffix. func (u *Updater) Update(ctx context.Context, toolsVersion string) error { // Get platform specific download URLs. - packages, err := teleportPackageURLs(u.uriTemplate, u.baseURL, toolsVersion) + packages, err := teleportPackageURLs(ctx, u.uriTemplate, u.baseURL, toolsVersion) if err != nil { return trace.Wrap(err) } diff --git a/lib/autoupdate/tools/utils.go b/lib/autoupdate/tools/utils.go index 66cfd7633f09a..6da7974bd5793 100644 --- a/lib/autoupdate/tools/utils.go +++ b/lib/autoupdate/tools/utils.go @@ -23,6 +23,7 @@ import ( "bytes" "context" "errors" + "log/slog" "os" "os/exec" "path/filepath" @@ -128,15 +129,16 @@ type packageURL struct { Optional bool } -// teleportPackageURLs returns the URL for the Teleport archive to download. -func teleportPackageURLs(uriTmpl string, baseURL, version string) ([]packageURL, error) { +// teleportPackageURLs returns URLs for the Teleport archives to download. +func teleportPackageURLs(ctx context.Context, uriTmpl string, baseURL, version string) ([]packageURL, error) { + m := modules.GetModules() envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) - if modules.GetModules().BuildType() == modules.BuildOSS && envBaseURL == "" { + if m.BuildType() == modules.BuildOSS && envBaseURL == "" { + slog.WarnContext(ctx, "Client tools updates are disabled as they are licensed under AGPL. To use Community Edition builds or custom binaries, set the 'TELEPORT_CDN_BASE_URL' environment variable.") return nil, errNoBaseURL } var flags autoupdate.InstallFlags - m := modules.GetModules() if m.IsBoringBinary() { flags |= autoupdate.FlagFIPS }