From dcec5af66d136cdcf47262aaf4636bdeb82e903c Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Fri, 7 Feb 2025 19:38:38 -0800 Subject: [PATCH 1/4] Fix setting modules for tsh and tctl --- Makefile | 22 ++++++++++++---------- lib/autoupdate/tools/helper.go | 6 ++---- lib/modules/modules.go | 4 +++- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/Makefile b/Makefile index 48786b9691e2b..7b006314c477b 100644 --- a/Makefile +++ b/Makefile @@ -357,6 +357,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 @@ -370,15 +378,9 @@ $(BUILDDIR)/tctl: @if [[ "$(OS)" != "windows" && -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) $(TOUCHID_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) $(TOUCHID_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 @@ -391,7 +393,7 @@ $(BUILDDIR)/tsh: @if [[ "$(OS)" != "windows" && -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 @@ -399,11 +401,11 @@ $(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 .PHONY: $(BUILDDIR)/teleport-update $(BUILDDIR)/teleport-update: - GOOS=$(OS) GOARCH=$(ARCH) CGO_ENABLED=0 go build -o $(BUILDDIR)/teleport-update $(BUILDFLAGS_TELEPORT_UPDATE) ./tool/teleport-update + GOOS=$(OS) GOARCH=$(ARCH) CGO_ENABLED=0 go build -o $(BUILDDIR)/teleport-update $(BUILDFLAGS_TELEPORT_UPDATE) $(TOOLS_LDFLAGS) ./tool/teleport-update TELEPORT_ARGS ?= start .PHONY: teleport-hot-reload diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index c05618e430a92..026c887607d08 100644 --- a/lib/autoupdate/tools/helper.go +++ b/lib/autoupdate/tools/helper.go @@ -116,12 +116,10 @@ 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 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) { + } else if err != nil && !errors.Is(err, context.Canceled) { return trace.Wrap(err) } diff --git a/lib/modules/modules.go b/lib/modules/modules.go index da15b036bd78c..b9a7e4b37131a 100644 --- a/lib/modules/modules.go +++ b/lib/modules/modules.go @@ -383,7 +383,9 @@ func (p *defaultModules) IsEnterpriseBuild() bool { // IsOSSBuild returns true for [defaultModules]. func (p *defaultModules) IsOSSBuild() bool { - return true + // teleportBuildType variable might be overridden during the build process, + // we have to always check the type. + return teleportBuildType == BuildOSS } // PrintVersion prints the Teleport version. From 9f2f01c7893a42a305dbb8186dec1b85dea95f55 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 10 Feb 2025 12:34:29 -0800 Subject: [PATCH 2/4] Restore IsOSSBuild() --- lib/modules/modules.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/modules/modules.go b/lib/modules/modules.go index b9a7e4b37131a..da15b036bd78c 100644 --- a/lib/modules/modules.go +++ b/lib/modules/modules.go @@ -383,9 +383,7 @@ func (p *defaultModules) IsEnterpriseBuild() bool { // IsOSSBuild returns true for [defaultModules]. func (p *defaultModules) IsOSSBuild() bool { - // teleportBuildType variable might be overridden during the build process, - // we have to always check the type. - return teleportBuildType == BuildOSS + return true } // PrintVersion prints the Teleport version. From a2867fb276dee93c3bb4ce2418ad6ca933f79b57 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 10 Feb 2025 15:27:15 -0800 Subject: [PATCH 3/4] Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable. --- lib/autoupdate/tools/helper.go | 10 +--------- lib/autoupdate/tools/updater.go | 12 +++++++++++- lib/autoupdate/tools/utils.go | 11 +++++++---- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/lib/autoupdate/tools/helper.go b/lib/autoupdate/tools/helper.go index 026c887607d08..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,10 +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 errors.Is(err, errNoBaseURL) { - // If base URL wasn't defined we have to cancel update and re-execution with warning. - slog.WarnContext(ctx, warnMessageOSSBuild) - } else 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..7e266c8d89369 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" ) @@ -203,6 +204,15 @@ func (u *Updater) CheckRemote(ctx context.Context, proxyAddr string, insecure bo return "", false, trace.Wrap(err) } + m := modules.GetModules() + envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) + if resp.Edition == modules.BuildOSS && m.BuildType() == modules.BuildCommunity && envBaseURL == "" { + 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 "", false, errNoBaseURL + } + // If a version of client tools has already been downloaded to // tools directory, return that. toolsVersion, err := CheckToolVersion(u.toolsDir) @@ -263,7 +273,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..82953d70d37af 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,17 @@ 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 } From 595f58dc67939d8ff14a56c0ec7235da104e4e33 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Mon, 10 Feb 2025 16:22:56 -0800 Subject: [PATCH 4/4] Check only the build type of client tools --- lib/autoupdate/tools/updater.go | 10 ---------- lib/autoupdate/tools/utils.go | 3 +-- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/lib/autoupdate/tools/updater.go b/lib/autoupdate/tools/updater.go index 7e266c8d89369..eded34049e473 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" ) @@ -204,15 +203,6 @@ func (u *Updater) CheckRemote(ctx context.Context, proxyAddr string, insecure bo return "", false, trace.Wrap(err) } - m := modules.GetModules() - envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) - if resp.Edition == modules.BuildOSS && m.BuildType() == modules.BuildCommunity && envBaseURL == "" { - 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 "", false, errNoBaseURL - } - // If a version of client tools has already been downloaded to // tools directory, return that. toolsVersion, err := CheckToolVersion(u.toolsDir) diff --git a/lib/autoupdate/tools/utils.go b/lib/autoupdate/tools/utils.go index 82953d70d37af..6da7974bd5793 100644 --- a/lib/autoupdate/tools/utils.go +++ b/lib/autoupdate/tools/utils.go @@ -134,8 +134,7 @@ func teleportPackageURLs(ctx context.Context, uriTmpl string, baseURL, version s m := modules.GetModules() envBaseURL := os.Getenv(autoupdate.BaseURLEnvVar) 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.") + 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 }