Skip to content

Commit 31f6b85

Browse files
rosstimothyrosstimothy
authored
Move trusted cluster storage out of presence (#43803)
This groups the trusted cluster and cert authority storage into the same service. While on its own this doesn't change much, it unlocks the ability to fix racy behavior described in #36400.
1 parent 268109d commit 31f6b85

File tree

15 files changed

+904
-874
lines changed

15 files changed

+904
-874
lines changed

‎lib/auth/migration/0001_db_ca.go

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ package migration
2020

2121
import (
2222
"context"
23+
"log/slog"
2324

2425
"github.com/gravitational/trace"
2526

@@ -33,9 +34,8 @@ import (
3334
// Database CA for all existing clusters that do not already
3435
// have one. Introduced in v10.
3536
type createDBAuthority struct {
36-
trustServiceFn func(b backend.Backend) services.Trust
37-
configServiceFn func(b backend.Backend) (services.ClusterConfiguration, error)
38-
presenceServiceFn func(b backend.Backend) services.Presence
37+
trustServiceFn func(b backend.Backend) services.Trust
38+
configServiceFn func(b backend.Backend) (services.ClusterConfiguration, error)
3939
}
4040

4141
func (d createDBAuthority) Version() int64 {
@@ -66,25 +66,18 @@ func (d createDBAuthority) Up(ctx context.Context, b backend.Backend) error {
6666
}
6767
}
6868

69-
if d.presenceServiceFn == nil {
70-
d.presenceServiceFn = func(b backend.Backend) services.Presence {
71-
return local.NewPresenceService(b)
72-
}
73-
}
74-
7569
trustSvc := d.trustServiceFn(b)
7670
configSvc, err := d.configServiceFn(b)
7771
if err != nil {
7872
return trace.Wrap(err)
7973
}
80-
presenceSvc := d.presenceServiceFn(b)
8174

8275
localClusterName, err := configSvc.GetClusterName()
8376
if err != nil {
8477
return trace.Wrap(err)
8578
}
8679

87-
trustedClusters, err := presenceSvc.GetTrustedClusters(ctx)
80+
trustedClusters, err := trustSvc.GetTrustedClusters(ctx)
8881
if err != nil {
8982
return trace.Wrap(err)
9083
}
@@ -133,7 +126,7 @@ func migrateDBAuthority(ctx context.Context, trustSvc services.Trust, cluster st
133126
// The migration for this cluster can be skipped since
134127
// the new CA already exists.
135128
if err == nil {
136-
log.Debugf("Migrations: cert authority %q already exists.", toType)
129+
slog.DebugContext(ctx, "Migrations: cert authority already exists.", "authority", toType)
137130
return nil
138131
}
139132
if !trace.IsNotFound(err) {
@@ -157,7 +150,7 @@ func migrateDBAuthority(ctx context.Context, trustSvc services.Trust, cluster st
157150
return trace.Wrap(err)
158151
}
159152

160-
log.Infof("Migrating %s CA for cluster: %s", toType, cluster)
153+
slog.InfoContext(ctx, "Migrating CA", "authority", toType, "cluster", cluster)
161154

162155
existingCAV2, ok := existingCA.(*types.CertAuthorityV2)
163156
if !ok {
@@ -175,7 +168,7 @@ func migrateDBAuthority(ctx context.Context, trustSvc services.Trust, cluster st
175168

176169
err = trustSvc.CreateCertAuthority(ctx, newCA)
177170
if trace.IsAlreadyExists(err) {
178-
log.Warnf("%s CA has already been created by a different Auth instance", toType)
171+
slog.WarnContext(ctx, "CA has already been created by a different Auth instance", "authority", toType)
179172
return nil
180173
}
181174
return trace.Wrap(err)

‎lib/auth/migration/0001_db_ca_test.go

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ func TestDBAuthorityUp(t *testing.T) {
181181
cases := []struct {
182182
name string
183183
fakeTrust *fakeTrust
184-
fakePresence fakePresence
185184
assertion require.ErrorAssertionFunc
186185
validateFunc func(t *testing.T, created []types.CertAuthority)
187186
}{
@@ -194,8 +193,6 @@ func TestDBAuthorityUp(t *testing.T) {
194193
leaf1DB: fakeCA,
195194
leaf2Host: fakeCA,
196195
},
197-
},
198-
fakePresence: fakePresence{
199196
clusters: []types.TrustedCluster{
200197
&types.TrustedClusterV2{
201198
Kind: types.KindTrustedCluster,
@@ -227,8 +224,6 @@ func TestDBAuthorityUp(t *testing.T) {
227224
leaf2DB: fakeCA,
228225
leaf1DB: fakeCA,
229226
},
230-
},
231-
fakePresence: fakePresence{
232227
clusters: []types.TrustedCluster{
233228
&types.TrustedClusterV2{
234229
Kind: types.KindTrustedCluster,
@@ -255,7 +250,6 @@ func TestDBAuthorityUp(t *testing.T) {
255250
b, err := memory.New(memory.Config{EventsOff: true})
256251
require.NoError(t, err)
257252

258-
test.fakePresence.Presence = local.NewPresenceService(b)
259253
test.fakeTrust.Trust = local.NewCAService(b)
260254

261255
migration := createDBAuthority{
@@ -272,9 +266,6 @@ func TestDBAuthorityUp(t *testing.T) {
272266
clusterName: clusterName("root"),
273267
}, nil
274268
},
275-
presenceServiceFn: func(b backend.Backend) services.Presence {
276-
return test.fakePresence
277-
},
278269
}
279270

280271
test.assertion(t, migration.Up(context.Background(), b))
@@ -292,22 +283,14 @@ func (f fakeConfig) GetClusterName(opts ...services.MarshalOption) (types.Cluste
292283
return f.clusterName, nil
293284
}
294285

295-
type fakePresence struct {
296-
services.Presence
297-
clusters []types.TrustedCluster
298-
}
299-
300-
func (f fakePresence) GetTrustedClusters(ctx context.Context) ([]types.TrustedCluster, error) {
301-
return f.clusters, nil
302-
}
303-
304286
type fakeTrust struct {
305287
services.Trust
306288

307289
authorities map[types.CertAuthID]types.CertAuthority
308290

309-
mu sync.Mutex
310-
created []types.CertAuthority
291+
clusters []types.TrustedCluster
292+
mu sync.Mutex
293+
created []types.CertAuthority
311294
}
312295

313296
func (f *fakeTrust) casCreated() []types.CertAuthority {
@@ -336,3 +319,7 @@ func (f *fakeTrust) GetCertAuthority(ctx context.Context, id types.CertAuthID, l
336319

337320
return ca, nil
338321
}
322+
323+
func (f *fakeTrust) GetTrustedClusters(ctx context.Context) ([]types.TrustedCluster, error) {
324+
return f.clusters, nil
325+
}

‎lib/auth/tls_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,8 +1403,8 @@ func TestTunnelConnectionsCRUD(t *testing.T) {
14031403
require.NoError(t, err)
14041404

14051405
suite := &suite.ServicesTestSuite{
1406-
PresenceS: clt,
1407-
Clock: clockwork.NewFakeClock(),
1406+
TrustS: clt,
1407+
Clock: clockwork.NewFakeClock(),
14081408
}
14091409
suite.TunnelConnectionsCRUD(t)
14101410
}
@@ -4102,7 +4102,7 @@ func TestEvents(t *testing.T) {
41024102
LocalConfigS: testSrv.Auth(),
41034103
EventsS: clt,
41044104
PresenceS: testSrv.Auth(),
4105-
CAS: testSrv.Auth(),
4105+
TrustS: testSrv.Auth(),
41064106
ProvisioningS: clt,
41074107
Access: clt,
41084108
UsersS: clt,

‎lib/cache/cache.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2110,7 +2110,7 @@ func (c *Cache) GetRemoteCluster(ctx context.Context, clusterName string) (types
21102110
rg.Release()
21112111
// fallback is sane because this method is never used
21122112
// in construction of derivative caches.
2113-
if rc, err := c.Config.Presence.GetRemoteCluster(ctx, clusterName); err == nil {
2113+
if rc, err := c.Config.Trust.GetRemoteCluster(ctx, clusterName); err == nil {
21142114
return rc, nil
21152115
}
21162116
}

‎lib/cache/cache_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,16 +1626,16 @@ func TestTunnelConnections(t *testing.T) {
16261626
LastHeartbeat: time.Now().UTC(),
16271627
})
16281628
},
1629-
create: modifyNoContext(p.presenceS.UpsertTunnelConnection),
1629+
create: modifyNoContext(p.trustS.UpsertTunnelConnection),
16301630
list: func(ctx context.Context) ([]types.TunnelConnection, error) {
1631-
return p.presenceS.GetTunnelConnections(clusterName)
1631+
return p.trustS.GetTunnelConnections(clusterName)
16321632
},
16331633
cacheList: func(ctx context.Context) ([]types.TunnelConnection, error) {
16341634
return p.cache.GetTunnelConnections(clusterName)
16351635
},
1636-
update: modifyNoContext(p.presenceS.UpsertTunnelConnection),
1636+
update: modifyNoContext(p.trustS.UpsertTunnelConnection),
16371637
deleteAll: func(ctx context.Context) error {
1638-
return p.presenceS.DeleteAllTunnelConnections()
1638+
return p.trustS.DeleteAllTunnelConnections()
16391639
},
16401640
})
16411641
}
@@ -1730,11 +1730,11 @@ func TestRemoteClusters(t *testing.T) {
17301730
return types.NewRemoteCluster(name)
17311731
},
17321732
create: func(ctx context.Context, rc types.RemoteCluster) error {
1733-
_, err := p.presenceS.CreateRemoteCluster(ctx, rc)
1733+
_, err := p.trustS.CreateRemoteCluster(ctx, rc)
17341734
return err
17351735
},
17361736
list: func(ctx context.Context) ([]types.RemoteCluster, error) {
1737-
return p.presenceS.GetRemoteClusters(ctx)
1737+
return p.trustS.GetRemoteClusters(ctx)
17381738
},
17391739
cacheGet: func(ctx context.Context, name string) (types.RemoteCluster, error) {
17401740
return p.cache.GetRemoteCluster(ctx, name)
@@ -1743,11 +1743,11 @@ func TestRemoteClusters(t *testing.T) {
17431743
return p.cache.GetRemoteClusters(ctx)
17441744
},
17451745
update: func(ctx context.Context, rc types.RemoteCluster) error {
1746-
_, err := p.presenceS.UpdateRemoteCluster(ctx, rc)
1746+
_, err := p.trustS.UpdateRemoteCluster(ctx, rc)
17471747
return err
17481748
},
17491749
deleteAll: func(ctx context.Context) error {
1750-
return p.presenceS.DeleteAllRemoteClusters(ctx)
1750+
return p.trustS.DeleteAllRemoteClusters(ctx)
17511751
},
17521752
})
17531753
}

‎lib/cache/collections.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -809,28 +809,28 @@ var _ executor[types.AccessRequest, noReader] = accessRequestExecutor{}
809809
type tunnelConnectionExecutor struct{}
810810

811811
func (tunnelConnectionExecutor) getAll(ctx context.Context, cache *Cache, loadSecrets bool) ([]types.TunnelConnection, error) {
812-
return cache.Presence.GetAllTunnelConnections()
812+
return cache.Trust.GetAllTunnelConnections()
813813
}
814814

815815
func (tunnelConnectionExecutor) upsert(ctx context.Context, cache *Cache, resource types.TunnelConnection) error {
816-
return cache.presenceCache.UpsertTunnelConnection(resource)
816+
return cache.trustCache.UpsertTunnelConnection(resource)
817817
}
818818

819819
func (tunnelConnectionExecutor) deleteAll(ctx context.Context, cache *Cache) error {
820-
return cache.presenceCache.DeleteAllTunnelConnections()
820+
return cache.trustCache.DeleteAllTunnelConnections()
821821
}
822822

823823
func (tunnelConnectionExecutor) delete(ctx context.Context, cache *Cache, resource types.Resource) error {
824-
return cache.presenceCache.DeleteTunnelConnection(resource.GetSubKind(), resource.GetName())
824+
return cache.trustCache.DeleteTunnelConnection(resource.GetSubKind(), resource.GetName())
825825
}
826826

827827
func (tunnelConnectionExecutor) isSingleton() bool { return false }
828828

829829
func (tunnelConnectionExecutor) getReader(cache *Cache, cacheOK bool) tunnelConnectionGetter {
830830
if cacheOK {
831-
return cache.presenceCache
831+
return cache.trustCache
832832
}
833-
return cache.Config.Presence
833+
return cache.Config.Trust
834834
}
835835

836836
type tunnelConnectionGetter interface {
@@ -843,36 +843,36 @@ var _ executor[types.TunnelConnection, tunnelConnectionGetter] = tunnelConnectio
843843
type remoteClusterExecutor struct{}
844844

845845
func (remoteClusterExecutor) getAll(ctx context.Context, cache *Cache, loadSecrets bool) ([]types.RemoteCluster, error) {
846-
return cache.Presence.GetRemoteClusters(ctx)
846+
return cache.Trust.GetRemoteClusters(ctx)
847847
}
848848

849849
func (remoteClusterExecutor) upsert(ctx context.Context, cache *Cache, resource types.RemoteCluster) error {
850-
err := cache.presenceCache.DeleteRemoteCluster(ctx, resource.GetName())
850+
err := cache.trustCache.DeleteRemoteCluster(ctx, resource.GetName())
851851
if err != nil {
852852
if !trace.IsNotFound(err) {
853853
cache.Logger.WithError(err).Warnf("Failed to delete remote cluster %v.", resource.GetName())
854854
return trace.Wrap(err)
855855
}
856856
}
857-
_, err = cache.presenceCache.CreateRemoteCluster(ctx, resource)
857+
_, err = cache.trustCache.CreateRemoteCluster(ctx, resource)
858858
return trace.Wrap(err)
859859
}
860860

861861
func (remoteClusterExecutor) deleteAll(ctx context.Context, cache *Cache) error {
862-
return cache.presenceCache.DeleteAllRemoteClusters(ctx)
862+
return cache.trustCache.DeleteAllRemoteClusters(ctx)
863863
}
864864

865865
func (remoteClusterExecutor) delete(ctx context.Context, cache *Cache, resource types.Resource) error {
866-
return cache.presenceCache.DeleteRemoteCluster(ctx, resource.GetName())
866+
return cache.trustCache.DeleteRemoteCluster(ctx, resource.GetName())
867867
}
868868

869869
func (remoteClusterExecutor) isSingleton() bool { return false }
870870

871871
func (remoteClusterExecutor) getReader(cache *Cache, cacheOK bool) remoteClusterGetter {
872872
if cacheOK {
873-
return cache.presenceCache
873+
return cache.trustCache
874874
}
875-
return cache.Config.Presence
875+
return cache.Config.Trust
876876
}
877877

878878
type remoteClusterGetter interface {

0 commit comments

Comments
 (0)