From 9fdd9b0445291b0606d15b6652fb6c51ee87bbb6 Mon Sep 17 00:00:00 2001 From: matheus Date: Tue, 1 Oct 2024 12:01:31 -0300 Subject: [PATCH 1/4] Update proxy features (#45979) * 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 * 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 <39066650+rosstimothy@users.noreply.github.com> * 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 Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --- lib/service/service.go | 1 + lib/web/apiserver.go | 53 +++++----- lib/web/apiserver_test.go | 83 +++++++++------ lib/web/features.go | 71 +++++++++++++ lib/web/features_test.go | 176 ++++++++++++++++++++++++++++++++ lib/web/integrations_awsoidc.go | 6 +- lib/web/join_tokens.go | 2 +- 7 files changed, 331 insertions(+), 61 deletions(-) create mode 100644 lib/web/features.go create mode 100644 lib/web/features_test.go diff --git a/lib/service/service.go b/lib/service/service.go index d38d5d18cc681..bc365744b3cf0 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -4583,6 +4583,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { TracerProvider: process.TracingProvider, AutomaticUpgradesChannels: cfg.Proxy.AutomaticUpgradesChannels, IntegrationAppHandler: connectionsHandler, + FeatureWatchInterval: utils.HalfJitter(web.DefaultFeatureWatchInterval * 2), } webHandler, err := web.NewHandler(webConfig) if err != nil { diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 771adabe1f3f3..5cdc861a34349 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -21,6 +21,7 @@ package web import ( + "cmp" "compress/gzip" "context" "encoding/base64" @@ -126,6 +127,13 @@ const ( // Example values: // - github-actions-ssh: indicates that the resource was added via the Bot GitHub Actions SSH flow webUIFlowLabelKey = "teleport.internal/ui-flow" + // IncludedResourceModeAll describes that only requestable resources should be returned. + IncludedResourceModeRequestable = "requestable" + // IncludedResourceModeAll describes that all resources, requestable and available, should be returned. + IncludedResourceModeAll = "all" + // DefaultFeatureWatchInterval is the default time in which the feature watcher + // should ping the auth server to check for updated features + DefaultFeatureWatchInterval = time.Minute * 5 ) // healthCheckAppServerFunc defines a function used to perform a health check @@ -161,12 +169,8 @@ type Handler struct { // userConns tracks amount of current active connections with user certificates. userConns atomic.Int32 - // ClusterFeatures contain flags for supported and unsupported features. - // Note: This field can become stale since it's only set on initial proxy - // startup. To get the latest feature flags you'll need to ping from the - // auth server. - // https://github.com/gravitational/teleport/issues/39161 - ClusterFeatures proto.Features + // clusterFeatures contain flags for supported and unsupported features. + clusterFeatures proto.Features // nodeWatcher is a services.NodeWatcher used by Assist to lookup nodes from // the proxy's cache and get nodes in real time. @@ -325,6 +329,10 @@ type Config struct { // IntegrationAppHandler handles App Access requests which use an Integration. IntegrationAppHandler app.ServerHandler + + // FeatureWatchInterval is the interval between pings to the auth server + // to fetch new cluster features + FeatureWatchInterval time.Duration } // SetDefaults ensures proper default values are set if @@ -339,6 +347,8 @@ func (c *Config) SetDefaults() { if c.PresenceChecker == nil { c.PresenceChecker = client.RunPresenceTask } + + c.FeatureWatchInterval = cmp.Or(c.FeatureWatchInterval, DefaultFeatureWatchInterval) } type APIHandler struct { @@ -395,7 +405,7 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) { log: newPackageLogger(), logger: slog.Default().With(teleport.ComponentKey, teleport.ComponentWeb), clock: clockwork.NewRealClock(), - ClusterFeatures: cfg.ClusterFeatures, + clusterFeatures: cfg.ClusterFeatures, healthCheckAppServer: cfg.HealthCheckAppServer, tracer: cfg.TracerProvider.Tracer(teleport.ComponentWeb), wsIODeadline: wsIODeadline, @@ -669,6 +679,8 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) { } } + go h.startFeatureWatcher() + return &APIHandler{ handler: h, appHandler: appHandler, @@ -1160,20 +1172,12 @@ func (h *Handler) getUserContext(w http.ResponseWriter, r *http.Request, p httpr } desktopRecordingEnabled := recConfig.GetMode() != types.RecordOff - pingResp, err := clt.Ping(r.Context()) - if err != nil { - return nil, trace.Wrap(err) - } - accessMonitoringEnabled := pingResp.GetServerFeatures().GetAccessMonitoring().GetEnabled() - - // DELETE IN 16.0 - // If ServerFeatures.AccessMonitoring is nil, then that means the response came from a older auth - // where ServerFeatures.AccessMonitoring field does not exist. - if pingResp.GetServerFeatures().GetAccessMonitoring() == nil { - accessMonitoringEnabled = pingResp.ServerFeatures != nil && pingResp.ServerFeatures.GetIdentityGovernance() - } + features := h.GetClusterFeatures() + entitlement := modules.GetProtoEntitlement(&features, entitlements.AccessMonitoring) + // ensure entitlement is set & feature is configured + accessMonitoringEnabled := entitlement.Enabled && features.GetAccessMonitoringConfigured() - userContext, err := ui.NewUserContext(user, accessChecker.Roles(), *pingResp.ServerFeatures, desktopRecordingEnabled, accessMonitoringEnabled) + userContext, err := ui.NewUserContext(user, accessChecker.Roles(), features, desktopRecordingEnabled, accessMonitoringEnabled) if err != nil { return nil, trace.Wrap(err) } @@ -1692,14 +1696,7 @@ func (h *Handler) getWebConfig(w http.ResponseWriter, r *http.Request, p httprou } } - clusterFeatures := h.ClusterFeatures - // ping server to get cluster features since h.ClusterFeatures may be stale - pingResponse, err := h.GetProxyClient().Ping(r.Context()) - if err != nil { - h.log.WithError(err).Warn("Cannot retrieve cluster features, client may receive stale features") - } else { - clusterFeatures = *pingResponse.ServerFeatures - } + clusterFeatures := h.GetClusterFeatures() // get tunnel address to display on cloud instances tunnelPublicAddr := "" diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 854695f648866..dc7aedbe97d25 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -4592,6 +4592,7 @@ func TestGetWebConfig(t *testing.T) { env := newWebPack(t, 1, func(cfg *proxyConfig) { cfg.serviceConfig = svcConfig }) + handler := env.proxies[0].handler.handler // Set auth preference with passwordless. const MOTD = "Welcome to cluster, your activity will be recorded." @@ -4622,6 +4623,9 @@ func TestGetWebConfig(t *testing.T) { _, err = env.server.Auth().UpsertGithubConnector(ctx, github) require.NoError(t, err) + // start the feature watcher so the web config gets new features + env.clock.Advance(DefaultFeatureWatchInterval * 2) + expectedCfg := webclient.WebConfig{ Auth: webclient.WebConfigAuthSettings{ SecondFactor: constants.SecondFactorOptional, @@ -4669,6 +4673,7 @@ func TestGetWebConfig(t *testing.T) { AutomaticUpgrades: true, }, }) + env.clock.Advance(DefaultFeatureWatchInterval * 2) svcConfig.Proxy.AssistAPIKey = "test" require.NoError(t, err) @@ -4680,7 +4685,7 @@ func TestGetWebConfig(t *testing.T) { }, } require.NoError(t, channels.CheckAndSetDefaults()) - env.proxies[0].handler.handler.cfg.AutomaticUpgradesChannels = channels + handler.cfg.AutomaticUpgradesChannels = channels expectedCfg.IsCloud = true expectedCfg.IsUsageBasedBilling = true @@ -4689,14 +4694,20 @@ func TestGetWebConfig(t *testing.T) { expectedCfg.AssistEnabled = false expectedCfg.JoinActiveSessions = false - // request and verify enabled features are enabled. - re, err = clt.Get(ctx, endpoint, nil) - require.NoError(t, err) - require.True(t, strings.HasPrefix(string(re.Bytes()), "var GRV_CONFIG")) - str = strings.ReplaceAll(string(re.Bytes()), "var GRV_CONFIG = ", "") - err = json.Unmarshal([]byte(str[:len(str)-1]), &cfg) - require.NoError(t, err) - require.Equal(t, expectedCfg, cfg) + // request and verify enabled features are eventually enabled. + require.EventuallyWithT(t, func(t *assert.CollectT) { + re, err := clt.Get(ctx, endpoint, nil) + if !assert.NoError(t, err) { + return + } + assert.True(t, bytes.HasPrefix(re.Bytes(), []byte("var GRV_CONFIG"))) + res := bytes.ReplaceAll(re.Bytes(), []byte("var GRV_CONFIG = "), []byte{}) + err = json.Unmarshal(res[:len(res)-1], &cfg) + assert.NoError(t, err) + diff := cmp.Diff(expectedCfg, cfg) + assert.Empty(t, diff) + + }, time.Second*5, time.Millisecond*50) // use mock client to assert that if ping returns an error, we'll default to // cluster config @@ -4715,15 +4726,22 @@ func TestGetWebConfig(t *testing.T) { IsUsageBasedBilling: false, }, }) + env.clock.Advance(DefaultFeatureWatchInterval * 2) // request and verify again - re, err = clt.Get(ctx, endpoint, nil) - require.NoError(t, err) - require.True(t, strings.HasPrefix(string(re.Bytes()), "var GRV_CONFIG")) - str = strings.ReplaceAll(string(re.Bytes()), "var GRV_CONFIG = ", "") - err = json.Unmarshal([]byte(str[:len(str)-1]), &cfg) - require.NoError(t, err) - require.Equal(t, expectedCfg, cfg) + require.EventuallyWithT(t, func(t *assert.CollectT) { + re, err := clt.Get(ctx, endpoint, nil) + if !assert.NoError(t, err) { + return + } + assert.True(t, bytes.HasPrefix(re.Bytes(), []byte("var GRV_CONFIG"))) + res := bytes.ReplaceAll(re.Bytes(), []byte("var GRV_CONFIG = "), []byte{}) + err = json.Unmarshal(res[:len(res)-1], &cfg) + assert.NoError(t, err) + diff := cmp.Diff(expectedCfg, cfg) + assert.Empty(t, diff) + + }, time.Second*5, time.Millisecond*50) } func TestGetWebConfig_IGSFeatureLimits(t *testing.T) { @@ -4745,6 +4763,8 @@ func TestGetWebConfig_IGSFeatureLimits(t *testing.T) { Questionnaire: true, }, }) + // start the feature watcher so the web config gets new features + env.clock.Advance(DefaultFeatureWatchInterval * 2) expectedCfg := webclient.WebConfig{ Auth: webclient.WebConfigAuthSettings{ @@ -4766,20 +4786,25 @@ func TestGetWebConfig_IGSFeatureLimits(t *testing.T) { IsUsageBasedBilling: true, } - // Make a request. clt := env.proxies[0].newClient(t) - endpoint := clt.Endpoint("web", "config.js") - re, err := clt.Get(ctx, endpoint, nil) - require.NoError(t, err) - require.True(t, strings.HasPrefix(string(re.Bytes()), "var GRV_CONFIG")) - - // Response is type application/javascript, we need to strip off the variable name - // and the semicolon at the end, then we are left with json like object. - var cfg webclient.WebConfig - str := strings.ReplaceAll(string(re.Bytes()), "var GRV_CONFIG = ", "") - err = json.Unmarshal([]byte(str[:len(str)-1]), &cfg) - require.NoError(t, err) - require.Equal(t, expectedCfg, cfg) + require.EventuallyWithT(t, func(t *assert.CollectT) { + // Make a request. + endpoint := clt.Endpoint("web", "config.js") + re, err := clt.Get(ctx, endpoint, nil) + if !assert.NoError(t, err) { + return + } + assert.True(t, bytes.HasPrefix(re.Bytes(), []byte("var GRV_CONFIG"))) + + // Response is type application/javascript, we need to strip off the variable name + // and the semicolon at the end, then we are left with json like object. + var cfg webclient.WebConfig + res := bytes.ReplaceAll(re.Bytes(), []byte("var GRV_CONFIG = "), []byte{}) + err = json.Unmarshal(res[:len(res)-1], &cfg) + assert.NoError(t, err) + diff := cmp.Diff(expectedCfg, cfg) + assert.Empty(t, diff) + }, time.Second*5, time.Millisecond*50) } func TestCreatePrivilegeToken(t *testing.T) { diff --git a/lib/web/features.go b/lib/web/features.go new file mode 100644 index 0000000000000..0625cff63c6a5 --- /dev/null +++ b/lib/web/features.go @@ -0,0 +1,71 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package web + +import ( + "github.com/gravitational/teleport/api/client/proto" +) + +// SetClusterFeatures sets the flags for supported and unsupported features. +// TODO(mcbattirola): make method unexported, fix tests using it to set +// test modules instead. +func (h *Handler) SetClusterFeatures(features proto.Features) { + h.Mutex.Lock() + defer h.Mutex.Unlock() + + h.clusterFeatures = features +} + +// GetClusterFeatures returns flags for supported and unsupported features. +func (h *Handler) GetClusterFeatures() proto.Features { + h.Mutex.Lock() + defer h.Mutex.Unlock() + + return h.clusterFeatures +} + +// startFeatureWatcher periodically pings the auth server and updates `clusterFeatures`. +// Must be called only once per `handler`, otherwise it may close an already closed channel +// which will cause a panic. +// The watcher doesn't ping the auth server immediately upon start because features are +// already set by the config object in `NewHandler`. +func (h *Handler) startFeatureWatcher() { + ticker := h.clock.NewTicker(h.cfg.FeatureWatchInterval) + h.log.WithField("interval", h.cfg.FeatureWatchInterval).Info("Proxy handler features watcher has started") + ctx := h.cfg.Context + + defer ticker.Stop() + for { + select { + case <-ticker.Chan(): + h.log.Info("Pinging auth server for features") + pingResponse, err := h.GetProxyClient().Ping(ctx) + if err != nil { + h.log.WithError(err).Error("Auth server ping failed") + continue + } + + h.SetClusterFeatures(*pingResponse.ServerFeatures) + h.log.WithField("features", pingResponse.ServerFeatures).Info("Done updating proxy features") + case <-ctx.Done(): + h.log.Info("Feature service has stopped") + return + } + } +} diff --git a/lib/web/features_test.go b/lib/web/features_test.go new file mode 100644 index 0000000000000..3798e819b46eb --- /dev/null +++ b/lib/web/features_test.go @@ -0,0 +1,176 @@ +/* + * Teleport + * Copyright (C) 2024 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package web + +import ( + "context" + "log/slog" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/jonboulle/clockwork" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/gravitational/teleport" + "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/utils" + "github.com/gravitational/teleport/entitlements" + "github.com/gravitational/teleport/lib/auth/authclient" +) + +// mockedPingTestProxy is a test proxy with a mocked Ping method +// that returns the internal features +type mockedFeatureGetter struct { + authclient.ClientI + features proto.Features +} + +func (m *mockedFeatureGetter) Ping(ctx context.Context) (proto.PingResponse, error) { + return proto.PingResponse{ + ServerFeatures: utils.CloneProtoMsg(&m.features), + }, nil +} + +func (m *mockedFeatureGetter) setFeatures(f proto.Features) { + m.features = f +} + +func TestFeaturesWatcher(t *testing.T) { + clock := clockwork.NewFakeClock() + + mockClient := &mockedFeatureGetter{features: proto.Features{ + Kubernetes: true, + Entitlements: map[string]*proto.EntitlementInfo{}, + AccessRequests: &proto.AccessRequestsFeature{}, + }} + + ctx, cancel := context.WithCancel(context.Background()) + handler := &Handler{ + cfg: Config{ + FeatureWatchInterval: 100 * time.Millisecond, + ProxyClient: mockClient, + Context: ctx, + }, + clock: clock, + clusterFeatures: proto.Features{}, + log: newPackageLogger(), + logger: slog.Default().With(teleport.ComponentKey, teleport.ComponentWeb), + } + + // before running the watcher, features should match the value passed to the handler + requireFeatures(t, clock, proto.Features{}, handler.GetClusterFeatures) + + go handler.startFeatureWatcher() + clock.BlockUntil(1) + + // after starting the watcher, handler.GetClusterFeatures should return + // values matching the client's response + features := proto.Features{ + Kubernetes: true, + Entitlements: map[string]*proto.EntitlementInfo{}, + AccessRequests: &proto.AccessRequestsFeature{}, + } + entitlements.BackfillFeatures(&features) + expected := utils.CloneProtoMsg(&features) + requireFeatures(t, clock, *expected, handler.GetClusterFeatures) + + // update values once again and check if the features are properly updated + features = proto.Features{ + Kubernetes: false, + Entitlements: map[string]*proto.EntitlementInfo{}, + AccessRequests: &proto.AccessRequestsFeature{}, + } + entitlements.BackfillFeatures(&features) + mockClient.setFeatures(features) + expected = utils.CloneProtoMsg(&features) + requireFeatures(t, clock, *expected, handler.GetClusterFeatures) + + // test updating entitlements + features = proto.Features{ + Kubernetes: true, + Entitlements: map[string]*proto.EntitlementInfo{ + string(entitlements.ExternalAuditStorage): {Enabled: true}, + string(entitlements.AccessLists): {Enabled: true}, + string(entitlements.AccessMonitoring): {Enabled: true}, + string(entitlements.App): {Enabled: true}, + string(entitlements.CloudAuditLogRetention): {Enabled: true}, + }, + AccessRequests: &proto.AccessRequestsFeature{}, + } + entitlements.BackfillFeatures(&features) + mockClient.setFeatures(features) + + expected = &proto.Features{ + Kubernetes: true, + Entitlements: map[string]*proto.EntitlementInfo{ + string(entitlements.ExternalAuditStorage): {Enabled: true}, + string(entitlements.AccessLists): {Enabled: true}, + string(entitlements.AccessMonitoring): {Enabled: true}, + string(entitlements.App): {Enabled: true}, + string(entitlements.CloudAuditLogRetention): {Enabled: true}, + }, + AccessRequests: &proto.AccessRequestsFeature{}, + } + entitlements.BackfillFeatures(expected) + requireFeatures(t, clock, *expected, handler.GetClusterFeatures) + + // stop watcher and ensure it stops updating features + cancel() + features = proto.Features{ + Kubernetes: !features.Kubernetes, + App: !features.App, + DB: true, + Entitlements: map[string]*proto.EntitlementInfo{}, + AccessRequests: &proto.AccessRequestsFeature{}, + } + entitlements.BackfillFeatures(&features) + mockClient.setFeatures(features) + notExpected := utils.CloneProtoMsg(&features) + // assert the handler never get these last features as the watcher is stopped + neverFeatures(t, clock, *notExpected, handler.GetClusterFeatures) +} + +// requireFeatures is a helper function that advances the clock, then +// calls `getFeatures` every 100ms for up to 1 second, until it +// returns the expected result (`want`). +func requireFeatures(t *testing.T, fakeClock clockwork.FakeClock, want proto.Features, getFeatures func() proto.Features) { + t.Helper() + + // Advance the clock so the service fetch and stores features + fakeClock.Advance(1 * time.Second) + + require.EventuallyWithT(t, func(t *assert.CollectT) { + diff := cmp.Diff(want, getFeatures()) + assert.Empty(t, diff) + }, 5*time.Second, time.Millisecond*100) +} + +// neverFeatures is a helper function that advances the clock, then +// calls `getFeatures` every 100ms for up to 1 second. If at some point `getFeatures` +// returns `doNotWant`, the test fails. +func neverFeatures(t *testing.T, fakeClock clockwork.FakeClock, doNotWant proto.Features, getFeatures func() proto.Features) { + t.Helper() + + fakeClock.Advance(1 * time.Second) + require.Never(t, func() bool { + return cmp.Diff(doNotWant, getFeatures()) == "" + }, 1*time.Second, time.Millisecond*100) +} diff --git a/lib/web/integrations_awsoidc.go b/lib/web/integrations_awsoidc.go index 1819a66059ece..631643f1fd7d4 100644 --- a/lib/web/integrations_awsoidc.go +++ b/lib/web/integrations_awsoidc.go @@ -125,7 +125,7 @@ func (h *Handler) awsOIDCDeployService(w http.ResponseWriter, r *http.Request, p } teleportVersionTag := teleport.Version - if automaticUpgrades(h.ClusterFeatures) { + if automaticUpgrades(h.GetClusterFeatures()) { cloudStableVersion, err := h.cfg.AutomaticUpgradesChannels.DefaultVersion(ctx) if err != nil { return "", trace.Wrap(err) @@ -178,7 +178,7 @@ func (h *Handler) awsOIDCDeployDatabaseServices(w http.ResponseWriter, r *http.R } teleportVersionTag := teleport.Version - if automaticUpgrades(h.ClusterFeatures) { + if automaticUpgrades(h.GetClusterFeatures()) { cloudStableVersion, err := h.cfg.AutomaticUpgradesChannels.DefaultVersion(ctx) if err != nil { return "", trace.Wrap(err) @@ -480,7 +480,7 @@ func (h *Handler) awsOIDCEnrollEKSClusters(w http.ResponseWriter, r *http.Reques return nil, trace.BadParameter("an integration name is required") } - agentVersion, err := kubeutils.GetKubeAgentVersion(ctx, h.cfg.ProxyClient, h.ClusterFeatures, h.cfg.AutomaticUpgradesChannels) + agentVersion, err := kubeutils.GetKubeAgentVersion(ctx, h.cfg.ProxyClient, h.GetClusterFeatures(), h.cfg.AutomaticUpgradesChannels) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/join_tokens.go b/lib/web/join_tokens.go index 9b5bfd459aaf3..31338c34837e3 100644 --- a/lib/web/join_tokens.go +++ b/lib/web/join_tokens.go @@ -216,7 +216,7 @@ func (h *Handler) createTokenHandle(w http.ResponseWriter, r *http.Request, para func (h *Handler) getAutoUpgrades(ctx context.Context) (bool, string, error) { var autoUpgradesVersion string var err error - autoUpgrades := automaticUpgrades(h.ClusterFeatures) + autoUpgrades := automaticUpgrades(h.GetClusterFeatures()) if autoUpgrades { autoUpgradesVersion, err = h.cfg.AutomaticUpgradesChannels.DefaultVersion(ctx) if err != nil { From 72b7e756ea25b992e4e69acb8f0f960d5988f57c Mon Sep 17 00:00:00 2001 From: mcbattirola Date: Thu, 31 Oct 2024 13:16:40 -0300 Subject: [PATCH 2/4] Export ClusterFeatures --- lib/web/apiserver.go | 6 +++--- lib/web/features.go | 4 ++-- lib/web/features_test.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 5cdc861a34349..146b040e7bad2 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -169,8 +169,8 @@ type Handler struct { // userConns tracks amount of current active connections with user certificates. userConns atomic.Int32 - // clusterFeatures contain flags for supported and unsupported features. - clusterFeatures proto.Features + // ClusterFeatures contain flags for supported and unsupported features. + ClusterFeatures proto.Features // nodeWatcher is a services.NodeWatcher used by Assist to lookup nodes from // the proxy's cache and get nodes in real time. @@ -405,7 +405,7 @@ func NewHandler(cfg Config, opts ...HandlerOption) (*APIHandler, error) { log: newPackageLogger(), logger: slog.Default().With(teleport.ComponentKey, teleport.ComponentWeb), clock: clockwork.NewRealClock(), - clusterFeatures: cfg.ClusterFeatures, + ClusterFeatures: cfg.ClusterFeatures, healthCheckAppServer: cfg.HealthCheckAppServer, tracer: cfg.TracerProvider.Tracer(teleport.ComponentWeb), wsIODeadline: wsIODeadline, diff --git a/lib/web/features.go b/lib/web/features.go index 0625cff63c6a5..b387d0f176958 100644 --- a/lib/web/features.go +++ b/lib/web/features.go @@ -29,7 +29,7 @@ func (h *Handler) SetClusterFeatures(features proto.Features) { h.Mutex.Lock() defer h.Mutex.Unlock() - h.clusterFeatures = features + h.ClusterFeatures = features } // GetClusterFeatures returns flags for supported and unsupported features. @@ -37,7 +37,7 @@ func (h *Handler) GetClusterFeatures() proto.Features { h.Mutex.Lock() defer h.Mutex.Unlock() - return h.clusterFeatures + return h.ClusterFeatures } // startFeatureWatcher periodically pings the auth server and updates `clusterFeatures`. diff --git a/lib/web/features_test.go b/lib/web/features_test.go index 3798e819b46eb..e031ed4becd2b 100644 --- a/lib/web/features_test.go +++ b/lib/web/features_test.go @@ -70,7 +70,7 @@ func TestFeaturesWatcher(t *testing.T) { Context: ctx, }, clock: clock, - clusterFeatures: proto.Features{}, + ClusterFeatures: proto.Features{}, log: newPackageLogger(), logger: slog.Default().With(teleport.ComponentKey, teleport.ComponentWeb), } From cf916233314a2976b4777a41eb53b9d46a33db06 Mon Sep 17 00:00:00 2001 From: mcbattirola Date: Thu, 31 Oct 2024 13:26:21 -0300 Subject: [PATCH 3/4] Merge fix --- lib/web/apiserver.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 146b040e7bad2..76a440143b977 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -127,10 +127,6 @@ const ( // Example values: // - github-actions-ssh: indicates that the resource was added via the Bot GitHub Actions SSH flow webUIFlowLabelKey = "teleport.internal/ui-flow" - // IncludedResourceModeAll describes that only requestable resources should be returned. - IncludedResourceModeRequestable = "requestable" - // IncludedResourceModeAll describes that all resources, requestable and available, should be returned. - IncludedResourceModeAll = "all" // DefaultFeatureWatchInterval is the default time in which the feature watcher // should ping the auth server to check for updated features DefaultFeatureWatchInterval = time.Minute * 5 @@ -1172,12 +1168,20 @@ func (h *Handler) getUserContext(w http.ResponseWriter, r *http.Request, p httpr } desktopRecordingEnabled := recConfig.GetMode() != types.RecordOff - features := h.GetClusterFeatures() - entitlement := modules.GetProtoEntitlement(&features, entitlements.AccessMonitoring) - // ensure entitlement is set & feature is configured - accessMonitoringEnabled := entitlement.Enabled && features.GetAccessMonitoringConfigured() + pingResp, err := clt.Ping(r.Context()) + if err != nil { + return nil, trace.Wrap(err) + } + accessMonitoringEnabled := pingResp.GetServerFeatures().GetAccessMonitoring().GetEnabled() + + // DELETE IN 16.0 + // If ServerFeatures.AccessMonitoring is nil, then that means the response came from a older auth + // where ServerFeatures.AccessMonitoring field does not exist. + if pingResp.GetServerFeatures().GetAccessMonitoring() == nil { + accessMonitoringEnabled = pingResp.ServerFeatures != nil && pingResp.ServerFeatures.GetIdentityGovernance() + } - userContext, err := ui.NewUserContext(user, accessChecker.Roles(), features, desktopRecordingEnabled, accessMonitoringEnabled) + userContext, err := ui.NewUserContext(user, accessChecker.Roles(), *pingResp.ServerFeatures, desktopRecordingEnabled, accessMonitoringEnabled) if err != nil { return nil, trace.Wrap(err) } From e416820417cec3f2ba56bfcaaf33f4d835797a23 Mon Sep 17 00:00:00 2001 From: mcbattirola Date: Thu, 31 Oct 2024 13:32:02 -0300 Subject: [PATCH 4/4] Fix tests --- lib/web/features_test.go | 42 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/lib/web/features_test.go b/lib/web/features_test.go index e031ed4becd2b..9e167f58324ca 100644 --- a/lib/web/features_test.go +++ b/lib/web/features_test.go @@ -32,7 +32,6 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/client/proto" "github.com/gravitational/teleport/api/utils" - "github.com/gravitational/teleport/entitlements" "github.com/gravitational/teleport/lib/auth/authclient" ) @@ -58,7 +57,6 @@ func TestFeaturesWatcher(t *testing.T) { mockClient := &mockedFeatureGetter{features: proto.Features{ Kubernetes: true, - Entitlements: map[string]*proto.EntitlementInfo{}, AccessRequests: &proto.AccessRequestsFeature{}, }} @@ -85,51 +83,39 @@ func TestFeaturesWatcher(t *testing.T) { // values matching the client's response features := proto.Features{ Kubernetes: true, - Entitlements: map[string]*proto.EntitlementInfo{}, AccessRequests: &proto.AccessRequestsFeature{}, } - entitlements.BackfillFeatures(&features) expected := utils.CloneProtoMsg(&features) requireFeatures(t, clock, *expected, handler.GetClusterFeatures) // update values once again and check if the features are properly updated features = proto.Features{ Kubernetes: false, - Entitlements: map[string]*proto.EntitlementInfo{}, AccessRequests: &proto.AccessRequestsFeature{}, } - entitlements.BackfillFeatures(&features) mockClient.setFeatures(features) expected = utils.CloneProtoMsg(&features) requireFeatures(t, clock, *expected, handler.GetClusterFeatures) - // test updating entitlements + // test updating features features = proto.Features{ - Kubernetes: true, - Entitlements: map[string]*proto.EntitlementInfo{ - string(entitlements.ExternalAuditStorage): {Enabled: true}, - string(entitlements.AccessLists): {Enabled: true}, - string(entitlements.AccessMonitoring): {Enabled: true}, - string(entitlements.App): {Enabled: true}, - string(entitlements.CloudAuditLogRetention): {Enabled: true}, - }, - AccessRequests: &proto.AccessRequestsFeature{}, + Kubernetes: true, + ExternalAuditStorage: true, + AccessList: &proto.AccessListFeature{CreateLimit: 10}, + AccessMonitoring: &proto.AccessMonitoringFeature{}, + App: true, + AccessRequests: &proto.AccessRequestsFeature{}, } - entitlements.BackfillFeatures(&features) mockClient.setFeatures(features) expected = &proto.Features{ - Kubernetes: true, - Entitlements: map[string]*proto.EntitlementInfo{ - string(entitlements.ExternalAuditStorage): {Enabled: true}, - string(entitlements.AccessLists): {Enabled: true}, - string(entitlements.AccessMonitoring): {Enabled: true}, - string(entitlements.App): {Enabled: true}, - string(entitlements.CloudAuditLogRetention): {Enabled: true}, - }, - AccessRequests: &proto.AccessRequestsFeature{}, + Kubernetes: true, + ExternalAuditStorage: true, + AccessList: &proto.AccessListFeature{CreateLimit: 10}, + AccessMonitoring: &proto.AccessMonitoringFeature{}, + App: true, + AccessRequests: &proto.AccessRequestsFeature{}, } - entitlements.BackfillFeatures(expected) requireFeatures(t, clock, *expected, handler.GetClusterFeatures) // stop watcher and ensure it stops updating features @@ -138,10 +124,8 @@ func TestFeaturesWatcher(t *testing.T) { Kubernetes: !features.Kubernetes, App: !features.App, DB: true, - Entitlements: map[string]*proto.EntitlementInfo{}, AccessRequests: &proto.AccessRequestsFeature{}, } - entitlements.BackfillFeatures(&features) mockClient.setFeatures(features) notExpected := utils.CloneProtoMsg(&features) // assert the handler never get these last features as the watcher is stopped