From 5dd08bd2e27db5783d3a03901c5949858a5c8ab1 Mon Sep 17 00:00:00 2001 From: rosstimothy <39066650+rosstimothy@users.noreply.github.com> Date: Wed, 12 Aug 2026 02:12:37 +0000 Subject: [PATCH] Stop honoring `teleport.cache.enabled` (#69439) Caching has become an integral part in functionality provided by teleport. Allowing users to disable the cache was never something that was a recomended configuration - it was predominately used as a stopgap to address an outage. However, over time the cache has become more stable and the need to bypass it should never arise. Additionally, this toggle causes several architectural difficulties that increase complexity and maintainability of teleport. By always enforcing that the cache is on, we can eventually separate the gRPC, backend, and caching layers - a frequent developer experience nightmare. Moving to a world where the cache is always guaranteed to be present and available we will be able to remove layers of abstraction and increase performance. We do still honor `teleport.cache.max_backoff`. While it was tempting to remove all teleport.cache knobs, it seemed that still allowing users to control the backoff interval could alleviate any caching related issues that lead to or are contributing to an outage. This seemed rather important to still offer now that the ability to disable caching no longer exists. --- .../config-reference/instance-wide.yaml | 4 - integration/integrations/integration_test.go | 75 ++++++++++--------- lib/config/configuration_test.go | 4 +- lib/config/fileconf.go | 36 ++++----- lib/service/servicecfg/config.go | 14 +--- tool/tsh/common/db_test.go | 14 ++-- tool/tsh/common/tsh_test.go | 40 ++++++---- 7 files changed, 96 insertions(+), 91 deletions(-) diff --git a/docs/pages/includes/config-reference/instance-wide.yaml b/docs/pages/includes/config-reference/instance-wide.yaml index 5b5d6a67370..b07f6007eb1 100644 --- a/docs/pages/includes/config-reference/instance-wide.yaml +++ b/docs/pages/includes/config-reference/instance-wide.yaml @@ -144,10 +144,6 @@ teleport: # Used in Teleport v18.3.0 and later for the SSH service. relay_server: teleport-relay.example.com:3042 - # cache: - # # The cache is enabled by default, it can be disabled with this flag - # enabled: true - # The duration (in string form) of the delay between receiving a termination # signal and the beginning of the shutdown procedures. It can be used to # give time to load balancers to stop routing connections to the Teleport diff --git a/integration/integrations/integration_test.go b/integration/integrations/integration_test.go index b4e4c4bb1d3..b5de567e828 100644 --- a/integration/integrations/integration_test.go +++ b/integration/integrations/integration_test.go @@ -19,15 +19,16 @@ package integrations import ( - "context" "encoding/json" "fmt" "net/http" "net/url" "os" "testing" + "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apidefaults "github.com/gravitational/teleport/api/defaults" @@ -46,7 +47,7 @@ func TestMain(m *testing.M) { // TestIntegrationCRUD starts a Teleport cluster and using its Proxy Web server, // tests the CRUD operations over the Integration resource. func TestIntegrationCRUD(t *testing.T) { - ctx := context.Background() + ctx := t.Context() // Start Teleport Auth and Proxy services authProcess, proxyProcess, _ := helpers.MakeTestServers(t) @@ -118,22 +119,22 @@ func TestIntegrationCRUD(t *testing.T) { respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodPost, integrationsEndpoint, createIntegrationWithoutS3LocationReq) require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) - // Get One Integration by name - respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"/my-aws-account", nil) - require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) + require.EventuallyWithT(t, func(tc *assert.CollectT) { + respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"/my-aws-account", nil) + require.Equal(tc, http.StatusOK, respStatusCode, string(respBody)) + var integrationResp ui.Integration + require.NoError(tc, json.Unmarshal(respBody, &integrationResp)) - integrationResp := ui.Integration{} - require.NoError(t, json.Unmarshal(respBody, &integrationResp)) - - require.Equal(t, ui.Integration{ - Name: "my-aws-account", - SubKind: types.IntegrationSubKindAWSOIDC, - AWSOIDC: &ui.IntegrationAWSOIDCSpec{ - RoleARN: "arn:aws:iam::123456789012:role/DevTeam", - IssuerS3Bucket: "my-bucket", - IssuerS3Prefix: "prefix", - }, - }, integrationResp, string(respBody)) + require.Equal(tc, ui.Integration{ + Name: "my-aws-account", + SubKind: types.IntegrationSubKindAWSOIDC, + AWSOIDC: &ui.IntegrationAWSOIDCSpec{ + RoleARN: "arn:aws:iam::123456789012:role/DevTeam", + IssuerS3Bucket: "my-bucket", + IssuerS3Prefix: "prefix", + }, + }, integrationResp, string(respBody)) + }, 15*time.Second, 100*time.Millisecond) // Update the integration to another RoleARN respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodPut, integrationsEndpoint+"/my-aws-account", ui.UpdateIntegrationRequest{ @@ -145,7 +146,7 @@ func TestIntegrationCRUD(t *testing.T) { }) require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) - integrationResp = ui.Integration{} + var integrationResp ui.Integration require.NoError(t, json.Unmarshal(respBody, &integrationResp)) require.Equal(t, ui.Integration{ @@ -201,30 +202,32 @@ func TestIntegrationCRUD(t *testing.T) { } // List integrations should return a full page - respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10", nil) - require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) + require.EventuallyWithT(t, func(tc *assert.CollectT) { + respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10", nil) + require.Equal(tc, http.StatusOK, respStatusCode, string(respBody)) - listResp = ui.IntegrationsListResponse{} - require.NoError(t, json.Unmarshal(respBody, &listResp)) + listResp = ui.IntegrationsListResponse{} + require.NoError(tc, json.Unmarshal(respBody, &listResp)) - require.Len(t, listResp.Items, pageSize) + require.Len(tc, listResp.Items, pageSize) - // Requesting the 2nd page should return a full page - respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) + // Requesting the 2nd page should return a full page + respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) + require.Equal(tc, http.StatusOK, respStatusCode, string(respBody)) - listResp = ui.IntegrationsListResponse{} - require.NoError(t, json.Unmarshal(respBody, &listResp)) + listResp = ui.IntegrationsListResponse{} + require.NoError(tc, json.Unmarshal(respBody, &listResp)) - require.Len(t, listResp.Items, pageSize) + require.Len(tc, listResp.Items, pageSize) - // Requesting the 3rd page should return two items and empty StartKey - respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) - require.Equal(t, http.StatusOK, respStatusCode, string(respBody)) + // Requesting the 3rd page should return two items and empty StartKey + respStatusCode, respBody = webPack.DoWebAPIRequest(t, http.MethodGet, integrationsEndpoint+"?limit=10&startKey="+listResp.NextKey, nil) + require.Equal(tc, http.StatusOK, respStatusCode, string(respBody)) - listResp = ui.IntegrationsListResponse{} - require.NoError(t, json.Unmarshal(respBody, &listResp)) + listResp = ui.IntegrationsListResponse{} + require.NoError(tc, json.Unmarshal(respBody, &listResp)) - require.Len(t, listResp.Items, 2) - require.Empty(t, listResp.NextKey) + require.Len(tc, listResp.Items, 2) + require.Empty(tc, listResp.NextKey) + }, 15*time.Second, 100*time.Millisecond) } diff --git a/lib/config/configuration_test.go b/lib/config/configuration_test.go index 4217fa6ad71..1cf80df7e06 100644 --- a/lib/config/configuration_test.go +++ b/lib/config/configuration_test.go @@ -1461,9 +1461,9 @@ func TestParseCachePolicy(t *testing.T) { }{ {in: &CachePolicy{EnabledFlag: "yes", TTL: "never"}, out: &servicecfg.CachePolicy{Enabled: true}}, {in: &CachePolicy{EnabledFlag: "true", TTL: "10h"}, out: &servicecfg.CachePolicy{Enabled: true}}, - {in: &CachePolicy{Type: "whatever", EnabledFlag: "false", TTL: "10h"}, out: &servicecfg.CachePolicy{Enabled: false}}, + {in: &CachePolicy{Type: "whatever", EnabledFlag: "false", TTL: "10h"}, out: &servicecfg.CachePolicy{Enabled: true}}, {in: &CachePolicy{Type: "name", EnabledFlag: "yes", TTL: "never"}, out: &servicecfg.CachePolicy{Enabled: true}}, - {in: &CachePolicy{EnabledFlag: "no"}, out: &servicecfg.CachePolicy{Enabled: false}}, + {in: &CachePolicy{EnabledFlag: "no"}, out: &servicecfg.CachePolicy{Enabled: true}}, } for i, tc := range tcs { comment := fmt.Sprintf("test case #%v", i) diff --git a/lib/config/fileconf.go b/lib/config/fileconf.go index 9a3f5348bad..996d09dc371 100644 --- a/lib/config/fileconf.go +++ b/lib/config/fileconf.go @@ -774,34 +774,34 @@ func (a *AuditQueueConfig) Parse() (servicecfg.AuditQueueConfig, error) { // CachePolicy is used to control local cache type CachePolicy struct { - // Type is for cache type `sqlite` or `in-memory` - Type string `yaml:"type,omitempty"` - // EnabledFlag enables or disables cache - EnabledFlag string `yaml:"enabled,omitempty"` - // TTL sets maximum TTL for the cached values - TTL string `yaml:"ttl,omitempty"` // MaxBackoff sets the maximum backoff on error. MaxBackoff time.Duration `yaml:"max_backoff,omitempty"` -} -// Enabled determines if a given "_service" section has been set to 'true' -func (c *CachePolicy) Enabled() bool { - if c.EnabledFlag == "" { - return true - } - enabled, _ := apiutils.ParseBool(c.EnabledFlag) - return enabled + // ---- Deprecated Fields ------ + // Type is for cache type `sqlite` or `in-memory` + // Deprecated: No longer supported. All caches are `in-memory`. This + // field still exists to prevent breaking any teleport.yaml files which + // had specified a type prior to deprecation. + Type string `yaml:"type,omitempty"` + // EnabledFlag enables or disables cache + // Deprecated: No longer supported. Caching is always enabled. This + // field still exists to prevent breaking any teleport.yaml files which + // had specified a value prior to deprecation. + EnabledFlag string `yaml:"enabled,omitempty"` + // TTL sets maximum TTL for the cached values + // Deprecated: No longer supported. Caches no longer evict stale data. This + // field still exists to prevent breaking any teleport.yaml files which + // had specified a value prior to deprecation. + TTL string `yaml:"ttl,omitempty"` + // ---- Deprecated Fields ------ } // Parse parses cache policy from Teleport config func (c *CachePolicy) Parse() (*servicecfg.CachePolicy, error) { out := servicecfg.CachePolicy{ - Enabled: c.Enabled(), + Enabled: true, MaxRetryPeriod: c.MaxBackoff, } - if err := out.CheckAndSetDefaults(); err != nil { - return nil, trace.Wrap(err) - } return &out, nil } diff --git a/lib/service/servicecfg/config.go b/lib/service/servicecfg/config.go index 522efb94a7d..85bfab67235 100644 --- a/lib/service/servicecfg/config.go +++ b/lib/service/servicecfg/config.go @@ -567,19 +567,6 @@ type CachePolicy struct { MaxRetryPeriod time.Duration } -// CheckAndSetDefaults checks and sets default values -func (c *CachePolicy) CheckAndSetDefaults() error { - return nil -} - -// String returns human-friendly representation of the policy -func (c CachePolicy) String() string { - if !c.Enabled { - return "no cache" - } - return "in-memory cache" -} - // CheckServicesForSELinux returns false if any services that don't // support SELinux enforcement are enabled. func (cfg *Config) CheckServicesForSELinux() bool { @@ -787,6 +774,7 @@ func ApplyDefaults(cfg *Config) { cfg.Ciphers = sc.Ciphers cfg.KEXAlgorithms = kex cfg.MACAlgorithms = macs + cfg.CachePolicy.Enabled = true // Auth service defaults. cfg.Auth.Enabled = true diff --git a/tool/tsh/common/db_test.go b/tool/tsh/common/db_test.go index a514d6d02c2..3fd707f282b 100644 --- a/tool/tsh/common/db_test.go +++ b/tool/tsh/common/db_test.go @@ -570,7 +570,7 @@ func TestLocalProxyRequirement(t *testing.T) { require.NoError(t, err) // Log into Teleport cluster. - err = Run(context.Background(), []string{ + err = Run(t.Context(), []string{ "login", "--insecure", "--debug", "--proxy", proxyAddr.String(), }, setHomePath(tmpHomePath), setMockSSOLogin(authServer, alice, connector.GetName())) require.NoError(t, err) @@ -645,14 +645,18 @@ func TestLocalProxyRequirement(t *testing.T) { wantTunnelReason: dbConnectRequireReasonTunnelFlag, }, } + for name, tt := range tests { t.Run(name, func(t *testing.T) { - _, err = authServer.UpsertAuthPreference(ctx, tt.clusterAuthPref) + created, err := authServer.UpsertAuthPreference(ctx, tt.clusterAuthPref) require.NoError(t, err) - t.Cleanup(func() { - _, err = authServer.UpsertAuthPreference(ctx, defaultAuthPref) + + require.EventuallyWithT(t, func(t *assert.CollectT) { + got, err := authServer.GetAuthPreference(ctx) require.NoError(t, err) - }) + require.Empty(t, cmp.Diff(created, got)) + }, 15*time.Second, 100*time.Millisecond) + cf := &CLIConf{ Context: ctx, TracingProvider: tracing.NoopProvider(), diff --git a/tool/tsh/common/tsh_test.go b/tool/tsh/common/tsh_test.go index a08fdd76468..8e434ccf42b 100644 --- a/tool/tsh/common/tsh_test.go +++ b/tool/tsh/common/tsh_test.go @@ -61,6 +61,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/breaker" "github.com/gravitational/teleport/api/client/proto" + "github.com/gravitational/teleport/api/client/webclient" "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/profile" @@ -753,9 +754,15 @@ func TestLoginIdentityOut(t *testing.T) { kubeServer, err := types.NewKubernetesServerV3FromCluster(cluster, kubeClusterName, kubeClusterName) require.NoError(t, err) - _, err = authServer.UpsertKubernetesServer(context.Background(), kubeServer) + _, err = authServer.UpsertKubernetesServer(t.Context(), kubeServer) require.NoError(t, err) + require.EventuallyWithT(t, func(tc *assert.CollectT) { + servers, err := authServer.UnifiedResourceCache.GetKubernetesServers(t.Context()) + require.NoError(tc, err) + require.Len(tc, servers, 1) + }, 30*time.Second, 100*time.Millisecond) + cases := []struct { name string extraArgs []string @@ -797,13 +804,16 @@ func TestLoginIdentityOut(t *testing.T) { }, }, } + for _, tt := range cases { t.Run(tt.name, func(t *testing.T) { identPath := filepath.Join(t.TempDir(), "ident") if tt.requiresTLSRouting { - switchProxyListenerMode(t, authServer, types.ProxyListenerMode_Multiplex) + switchProxyListenerMode(t, authServer, proxyAddr.String(), types.ProxyListenerMode_Multiplex) + } else { + switchProxyListenerMode(t, authServer, proxyAddr.String(), types.ProxyListenerMode_Separate) } - err = Run(context.Background(), append([]string{ + err = Run(t.Context(), append([]string{ "login", "--insecure", "--debug", @@ -817,20 +827,24 @@ func TestLoginIdentityOut(t *testing.T) { } // switchProxyListenerMode switches the proxy listener mode to the specified mode -// and schedules a reversion to the previous value once the sub-test completes. -func switchProxyListenerMode(t *testing.T, authServer *auth.Server, mode types.ProxyListenerMode) { - networkCfg, err := authServer.GetClusterNetworkingConfig(context.Background()) +// and waits for the proxy to observe the change. +func switchProxyListenerMode(t *testing.T, authServer *auth.Server, proxyAddr string, mode types.ProxyListenerMode) { + networkCfg, err := authServer.GetClusterNetworkingConfig(t.Context()) require.NoError(t, err) - prevValue := networkCfg.GetProxyListenerMode() networkCfg.SetProxyListenerMode(mode) - _, err = authServer.UpsertClusterNetworkingConfig(context.Background(), networkCfg) + _, err = authServer.UpsertClusterNetworkingConfig(t.Context(), networkCfg) require.NoError(t, err) - t.Cleanup(func() { - networkCfg.SetProxyListenerMode(prevValue) - _, err = authServer.UpsertClusterNetworkingConfig(context.Background(), networkCfg) - require.NoError(t, err) - }) + wantTLSRouting := mode == types.ProxyListenerMode_Multiplex + require.EventuallyWithT(t, func(tc *assert.CollectT) { + resp, err := webclient.Ping(&webclient.Config{ + Context: t.Context(), + ProxyAddr: proxyAddr, + Insecure: true, + }) + require.NoError(tc, err) + require.Equal(tc, wantTLSRouting, resp.Proxy.TLSRoutingEnabled) + }, 15*time.Second, 100*time.Millisecond) } // TestLoginScopeChangeClearsAgentKeys verifies that when the login scope changes