Connect: Clear only stale clients in cache (#65832)

* Clear only stale clients in cache

* Match test on specific error

* Improve comment
This commit is contained in:
Grzegorz Zdunek
2026-04-20 18:18:32 +02:00
committed by GitHub
parent 7691ff5a1c
commit 36dc67e68d
7 changed files with 50 additions and 58 deletions
+11 -4
View File
@@ -558,10 +558,17 @@ func testClientCache(t *testing.T, pack *dbhelpers.DatabasePack, creds *helpers.
require.NoError(t, err)
require.Equal(t, concurrentCallsForClient[0], secondCallForClient)
// Let's remove the client from the cache.
// The call to GetCachedClient will
// connect to proxy and return a new client.
err = daemonService.ClearCachedClientsForRoot(cluster.URI)
// Reissue user certs by assuming a role with a bogus ID in DropAccessRequests.
// This makes the cached client stale.
accessRequest := &api.AssumeRoleRequest{
RootClusterUri: cluster.URI.String(),
DropRequestIds: []string{"does-not-matter"},
}
err = cluster.AssumeRole(ctx, secondCallForClient, accessRequest)
require.NoError(t, err)
// Clearing stale clients should delete the stale client and force a new one.
err = daemonService.ClearStaleCachedClientsForRoot(cluster.URI)
require.NoError(t, err)
thirdCallForClient, err := daemonService.GetCachedClient(ctx, cluster.URI)
require.NoError(t, err)
+13 -30
View File
@@ -186,25 +186,9 @@ func (c *Cache) Get(ctx context.Context, profileName, leafClusterName string) (*
return clt, nil
}
// ClearOption configures ClearForRoot behavior.
type ClearOption func(*clearConfig)
type clearConfig struct {
onlyClearClientsWithStaleCert bool
}
// WithClearingOnlyClientsWithStaleCert closes only clients that use outdated certs.
func WithClearingOnlyClientsWithStaleCert() ClearOption {
return func(c *clearConfig) { c.onlyClearClientsWithStaleCert = true }
}
// ClearForRoot closes and removes clients from the cache for the root cluster and its leaf clusters.
func (c *Cache) ClearForRoot(profileName string, opts ...ClearOption) error {
cfg := &clearConfig{}
for _, o := range opts {
o(cfg)
}
// ClearStaleClientsForRoot closes and removes clients from the cache
// for the root cluster and its leaf clusters, if their cert is outdated.
func (c *Cache) ClearStaleClientsForRoot(profileName string) error {
c.mu.Lock()
defer c.mu.Unlock()
@@ -217,16 +201,14 @@ func (c *Cache) ClearForRoot(profileName string, opts ...ClearOption) error {
if k.profile != profileName {
continue
}
if cfg.onlyClearClientsWithStaleCert {
stale, err := clt.isTLSCertStale()
// If an error occurs, close the client as well.
if err != nil {
errors = append(errors, err)
} else if !stale {
continue
}
stale, err := clt.isTLSCertStale()
// If an error occurs, close the client as well.
if err != nil {
errors = append(errors, err)
} else if !stale {
continue
}
if err := clt.client.Close(); err != nil {
if err = clt.client.Close(); err != nil {
errors = append(errors, err)
}
deleted = append(deleted, k.String())
@@ -275,7 +257,8 @@ func (c *Cache) getFromCache(k key) *clientWithMetadata {
// NoCache is a client cache implementation that returns a new client
// on each call to Get.
//
// ClearForRoot and Clear still work as expected.
// Clear works as expected.
// ClearStaleClientsForRoot always clears clients for the root cluster.
type NoCache struct {
mu sync.Mutex
newClientFunc NewClientFunc
@@ -314,7 +297,7 @@ func (c *NoCache) Get(ctx context.Context, profileName, leafClusterName string)
return newClient, nil
}
func (c *NoCache) ClearForRoot(profileName string, _ ...ClearOption) error {
func (c *NoCache) ClearStaleClientsForRoot(profileName string) error {
c.mu.Lock()
defer c.mu.Unlock()
+17 -7
View File
@@ -67,12 +67,13 @@ func TestClearingClientsWithStaleCert(t *testing.T) {
cache, err := New(Config{
NewClientFunc: func(ctx context.Context, profileName, leafClusterName string) (*client.TeleportClient, error) {
config := &client.Config{
ClientStore: clientStore,
SSHProxyAddr: "localhost:3080",
WebProxyAddr: "localhost:3080",
Username: "testuser",
Tracer: tracing.NoopProvider().Tracer("test"),
SiteName: "root",
ClientStore: clientStore,
SSHProxyAddr: "localhost:3080",
WebProxyAddr: "localhost:3080",
Username: "testuser",
Tracer: tracing.NoopProvider().Tracer("test"),
SiteName: "root",
AddKeysToAgent: client.AddKeysToAgentNo,
}
if leafClusterName != "" {
config.SiteName = leafClusterName
@@ -106,7 +107,7 @@ func TestClearingClientsWithStaleCert(t *testing.T) {
require.NoError(t, err)
// Clear stale clients.
err = cache.ClearForRoot("root", WithClearingOnlyClientsWithStaleCert())
err = cache.ClearStaleClientsForRoot("root")
require.NoError(t, err)
newRootClient, err := cache.Get(t.Context(), "root", "")
@@ -120,6 +121,15 @@ func TestClearingClientsWithStaleCert(t *testing.T) {
require.NotEqual(t, newLeaf1Client, leaf1Client)
// The client opened after updating the cert should be untouched.
require.Equal(t, newLeaf2Client, leaf2Client)
// Verify that the client is considered "stale" after logging out.
err = clientStore.DeleteKeyRing(keyRing.KeyRingIndex)
require.NoError(t, err)
err = cache.ClearStaleClientsForRoot("root")
require.NoError(t, err)
_, err = cache.Get(t.Context(), "root", "")
// Getting the client should return an error because we are logged out.
require.ErrorContains(t, err, "are you logged in?")
}
// makeCerts makes TSL and SSH certs.
@@ -61,7 +61,7 @@ func (s *Handler) Login(ctx context.Context, req *api.LoginRequest) (*api.EmptyR
// Clear the cache after login, not before.
// During a re-login, another thread might try to retrieve a client from the cache.
// Because the cache is empty, it could initialize a new client using the previous certificate.
if err = s.DaemonService.ClearCachedClientsForRoot(cluster.URI); err != nil {
if err = s.DaemonService.ClearStaleCachedClientsForRoot(cluster.URI); err != nil {
return nil, trace.Wrap(err)
}
@@ -103,7 +103,7 @@ func (s *Handler) LoginPasswordless(stream api.TerminalService_LoginPasswordless
// Clear the cache after login, not before.
// During a re-login, another thread might try to retrieve a client from the cache.
// Because the cache is empty, it could initialize a new client using the previous certificate.
err = s.DaemonService.ClearCachedClientsForRoot(cluster.URI)
err = s.DaemonService.ClearStaleCachedClientsForRoot(cluster.URI)
return trace.Wrap(err)
}
+3 -3
View File
@@ -93,9 +93,9 @@ type ClientCache interface {
// otherwise it dials the remote server.
// The caller should not close the returned client.
Get(ctx context.Context, profileName, leafClusterName string) (*client.ClusterClient, error)
// ClearForRoot closes and removes clients from the cache
// for the root cluster and its leaf clusters.
ClearForRoot(profileName string, opts ...clientcache.ClearOption) error
// ClearStaleClientsForRoot closes and removes clients from the cache
// for the root cluster and its leaf clusters, if their cert is outdated.
ClearStaleClientsForRoot(profileName string) error
// Clear closes and removes all clients.
Clear() error
}
+3 -11
View File
@@ -40,7 +40,6 @@ import (
api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1"
"github.com/gravitational/teleport/lib/auth/authclient"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/client/clientcache"
"github.com/gravitational/teleport/lib/client/sso"
dtauthn "github.com/gravitational/teleport/lib/devicetrust/authn"
"github.com/gravitational/teleport/lib/teleterm/api/uri"
@@ -353,7 +352,7 @@ func (s *Service) ClusterLogout(ctx context.Context, uri uri.ResourceURI, remove
return trace.Wrap(err)
}
return trace.Wrap(s.ClearCachedClientsForRoot(uri))
return trace.Wrap(s.ClearStaleCachedClientsForRoot(uri))
}
// CreateGateway creates a gateway to given targetURI
@@ -875,7 +874,7 @@ func (s *Service) AssumeRole(ctx context.Context, req *api.AssumeRoleRequest) er
}
// We have to reconnect using the updated cert.
return trace.Wrap(s.ClearCachedClientsForRoot(cluster.URI))
return trace.Wrap(s.ClearStaleCachedClientsForRoot(cluster.URI))
}
// ListKubernetesResourcesRequest defines a request to retrieve kube resources paginated.
@@ -1277,18 +1276,11 @@ func (s *Service) GetCachedClient(ctx context.Context, resourceURI uri.ResourceU
return clt, trace.Wrap(err)
}
// ClearCachedClientsForRoot closes and removes clients from the cache
// for the root cluster and its leaf clusters.
func (s *Service) ClearCachedClientsForRoot(clusterURI uri.ResourceURI) error {
profileName := clusterURI.GetProfileName()
return trace.Wrap(s.clientCache.ClearForRoot(profileName))
}
// ClearStaleCachedClientsForRoot closes and removes clients from the cache
// for the root cluster and its leaf clusters, if their cert is outdated.
func (s *Service) ClearStaleCachedClientsForRoot(clusterURI uri.ResourceURI) error {
profileName := clusterURI.GetProfileName()
err := s.clientCache.ClearForRoot(profileName, clientcache.WithClearingOnlyClientsWithStaleCert())
err := s.clientCache.ClearStaleClientsForRoot(profileName)
return trace.Wrap(err)
}
+1 -1
View File
@@ -201,7 +201,7 @@ func (p *vnetClientApplication) retryWithRelogin(ctx context.Context, tc *client
return nil
}),
client.WithAfterLoginHook(func() error {
return trace.Wrap(p.clientCache.ClearForRoot(profileName), "clearing client cache after relogin")
return trace.Wrap(p.clientCache.ClearStaleClientsForRoot(profileName), "clearing client cache after relogin")
}),
client.WithMakeCurrentProfile(false),
)