Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default BaseURL only for community / enterprise editions #51732

Merged
merged 5 commits into from
Feb 5, 2025

Conversation

vapopov
Copy link
Contributor

@vapopov vapopov commented Jan 31, 2025

This PR prevents AGPL editions of Teleport agents from auto-updating to non-AGPL editions of Teleport via {tsh, tctl}.

Also added fix for progress bar for Darwin platform where we download two packages for version <v17, otherwise we always show empty progress bar for second package.

Changelog: Client tools managed updates requires base URL for open source build type.
Changelog: Fixed progress bar for client tools managed updates in darwin platform for not required packages.

Fix progress bar for darwin platform
@vapopov vapopov force-pushed the vapopov/client-tools-managed-updates-agpl branch from bc20d6e to 22ecd0f Compare January 31, 2025 22:17
@@ -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"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
warnMessageOSSBuild = "Client tools update is disabled. Use 'TELEPORT_CDN_BASE_URL' environment variable to set CDN base URL"
warnMessageOSSBuild = "Client tools updates are disabled. Use the 'TELEPORT_CDN_BASE_URL' environment variable to set the CDN base URL."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we explain why in the error message? e.g.

Suggested change
warnMessageOSSBuild = "Client tools update is disabled. Use 'TELEPORT_CDN_BASE_URL' environment variable to set CDN base URL"
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."

@@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: you could push this logic loser to the envBaseURL parsing in teleportPackageURLs, and return a sentinel error (errNoBaseURL).

That would prevent leaking the default base URL to other consumers of teleportPackageURLs in the future.

@@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we explain why in the error message? e.g.

Suggested change
warnMessageOSSBuild = "Client tools update is disabled. Use 'TELEPORT_CDN_BASE_URL' environment variable to set CDN base URL"
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."

@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from bernardjkim February 3, 2025 14:10
@vapopov vapopov force-pushed the vapopov/client-tools-managed-updates-agpl branch from 4825395 to ea475fe Compare February 5, 2025 19:55
@vapopov vapopov added this pull request to the merge queue Feb 5, 2025
Merged via the queue into master with commit a5391f9 Feb 5, 2025
41 checks passed
@vapopov vapopov deleted the vapopov/client-tools-managed-updates-agpl branch February 5, 2025 22:35
@public-teleport-github-review-bot

@vapopov See the table below for backport results.

Branch Result
branch/v15 Failed
branch/v16 Failed
branch/v17 Create PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants