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

[v15] Update Proxy Features #48224

Merged
merged 6 commits into from
Nov 4, 2024

Conversation

mcbattirola
Copy link
Contributor

@mcbattirola mcbattirola commented Oct 31, 2024

Backport #45979 to branch/v15

This was manually backported and required some changes:

  • There's no entitlements package on v15, so all calls to its methods and types were removed.
  • Some tests were updated to include more feature tests (since in master and v16 we tested mostly the Entitlements)
  • I didn't update getUserContext to use h.GetFeatures(). This is because in v15 this method has some additional logic for compatibility that it uses the Ping call to determine. In the original PR, the Ping was only for fetching features, so it was replaced, but in this PR I didn't touch that code to preserve the current behavior.
  • I added an additional commit 72b7e75. This commit exports h.ClusterFeatures, that the original PR made private. This is to prevent the build from breaking when we merge this PR, since Teleport Enterprise uses this method.

Teleport.E backport: https://github.com/gravitational/teleport.e/pull/5295

Note that Entitlements is a new version of Features. They currently coexist in master for compatibility, so even though a package used in the original PR doesn't exist, it is fine to backport this because the original PR also updated the Features.

mcbattirola and others added 2 commits October 31, 2024 13:15
* Add feature watcher

* Add test

* Update godocs, fix typos, rename SupportEntitlementsCompatibility to BackfillFeatures

* Godocs; rename start and stop functions

* Use `Feature` prefix in var names instead of `License`

* Fix lint

* Fix TestGetWebConfig_LegacyFeatureLimits

* Fix TestGetWebConfig_WithEntitlements

* Fix tests and lint

* Remove sync.Once

* Add jitter

* Remove Ping call from getUserContext

* Move entitlements test to entitlements package

* Apply suggestions from code review: godoc and tests improvement.

Co-authored-by: Zac Bergquist <[email protected]>

* Improve tests

* Shadow `t` in EventuallyWithT closure to avoid mistakes

* Improve startFeatureWatcher godoc

* Log features on update

* Log features on update

* Avoid race condition on test

* Improve TODO comment

Co-authored-by: rosstimothy <[email protected]>

* Use handler config context

* Start feature watcher in NewHandler

* Improve startFeatureWatcher godocs

* Add TODO to unexport SetClusterFeatures

* Remove featureWatcherReady

* Use require in require.EventuallyWithT in cases where an error is not expected

* Use return of assert.NoError` to return early on require.EventuallyWithT

---------

Co-authored-by: Zac Bergquist <[email protected]>
Co-authored-by: rosstimothy <[email protected]>
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-48224.d1v2yqnl3ruxch.amplifyapp.com

@mcbattirola mcbattirola force-pushed the mcbattirola/v15/update-proxy-features branch 2 times, most recently from 40a2506 to 2cad841 Compare October 31, 2024 16:27
@mcbattirola mcbattirola force-pushed the mcbattirola/v15/update-proxy-features branch from 2cad841 to cf91623 Compare October 31, 2024 16:28
@mcbattirola mcbattirola added backport no-changelog Indicates that a PR does not require a changelog entry labels Oct 31, 2024
@mcbattirola mcbattirola changed the title [wip][v15] Update Proxy Features [v15] Update Proxy Features Oct 31, 2024
@mcbattirola mcbattirola marked this pull request as ready for review October 31, 2024 17:03
@michellescripts
Copy link
Contributor

@mcbattirola mcbattirola added this pull request to the merge queue Nov 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 4, 2024
@mcbattirola mcbattirola enabled auto-merge November 4, 2024 16:28
@mcbattirola mcbattirola added this pull request to the merge queue Nov 4, 2024
Merged via the queue into branch/v15 with commit e8721d8 Nov 4, 2024
32 of 33 checks passed
@mcbattirola mcbattirola deleted the mcbattirola/v15/update-proxy-features branch November 4, 2024 18:19
@@ -339,6 +343,8 @@ func (c *Config) SetDefaults() {
if c.PresenceChecker == nil {
c.PresenceChecker = client.RunPresenceTask
}

c.FeatureWatchInterval = cmp.Or(c.FeatureWatchInterval, DefaultFeatureWatchInterval)
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, cmp.Or requires Go 1.22 or later.

We build with Go 1.22, so that's why the build hasn't failed.

We do however set the language level to Go 1.21 in branch/v15, which generates a warning.

module github.com/gravitational/teleport

go 1.21

toolchain go1.22.10

At this point, we might as well just update the go directive to 1.22 here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants