Skip to content

Commit 9179d2e

Browse files
rosstimothyrosstimothy
authored
Prep for upcoming backend.Key type change (#46675) (#47483)
Adds a (Key) IsZero method to determine if a key is populated. Some Backend implementations today validate keys before operations via a `len(key) == 0` check, however, that will no longer work once the key is migrated. Starts migrating the backend.LockConfiguration away from prepopulating the lock name to passing in a list of components. There were a few locks constructing a portion of the name manually, which will not work when the key type is changed.
1 parent 6ab05bb commit 9179d2e

File tree

14 files changed

+92
-69
lines changed

14 files changed

+92
-69
lines changed

‎lib/auth/init.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,9 @@ func Init(ctx context.Context, cfg InitConfig, opts ...ServerOption) (*Server, e
311311
if err := backend.RunWhileLocked(ctx,
312312
backend.RunWhileLockedConfig{
313313
LockConfiguration: backend.LockConfiguration{
314-
Backend: cfg.Backend,
315-
LockName: domainName,
316-
TTL: 30 * time.Second,
314+
Backend: cfg.Backend,
315+
LockNameComponents: []string{domainName},
316+
TTL: 30 * time.Second,
317317
},
318318
RefreshLockInterval: 20 * time.Second,
319319
}, func(ctx context.Context) error {

‎lib/backend/key.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@ func (k Key) String() string {
5252
return string(k)
5353
}
5454

55+
// IsZero reports whether k represents the zero key.
56+
func (k Key) IsZero() bool {
57+
return len(k) == 0
58+
}
59+
5560
// HasPrefix reports whether the key begins with prefix.
5661
func (k Key) HasPrefix(prefix Key) bool {
5762
return bytes.HasPrefix(k, prefix)

‎lib/backend/key_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,10 @@ func TestKeyCompare(t *testing.T) {
473473
})
474474
}
475475
}
476+
477+
func TestKeyIsZero(t *testing.T) {
478+
assert.True(t, backend.Key{}.IsZero())
479+
assert.True(t, backend.NewKey().IsZero())
480+
assert.False(t, backend.NewKey("a", "b").IsZero())
481+
assert.False(t, backend.ExactKey("a", "b").IsZero())
482+
}

‎lib/backend/lock.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,14 @@ func randomID() ([]byte, error) {
5050
}
5151

5252
type LockConfiguration struct {
53-
Backend Backend
53+
// Backend to create the lock in.
54+
Backend Backend
55+
// LockName the precomputed lock name.
56+
// TODO(tross) DELETE WHEN teleport.e is updated to use LockNameComponents.
5457
LockName string
58+
// LockNameComponents are subcomponents to be used when constructing
59+
// the lock name.
60+
LockNameComponents []string
5561
// TTL defines when lock will be released automatically
5662
TTL time.Duration
5763
// RetryInterval defines interval which is used to retry locking after
@@ -63,9 +69,14 @@ func (l *LockConfiguration) CheckAndSetDefaults() error {
6369
if l.Backend == nil {
6470
return trace.BadParameter("missing Backend")
6571
}
66-
if l.LockName == "" {
67-
return trace.BadParameter("missing LockName")
72+
if l.LockName == "" && len(l.LockNameComponents) == 0 {
73+
return trace.BadParameter("missing LockName/LockNameComponents")
6874
}
75+
76+
if len(l.LockNameComponents) == 0 {
77+
l.LockNameComponents = []string{l.LockName}
78+
}
79+
6980
if l.TTL == 0 {
7081
return trace.BadParameter("missing TTL")
7182
}
@@ -81,7 +92,7 @@ func AcquireLock(ctx context.Context, cfg LockConfiguration) (Lock, error) {
8192
if err != nil {
8293
return Lock{}, trace.Wrap(err)
8394
}
84-
key := lockKey(cfg.LockName)
95+
key := lockKey(cfg.LockNameComponents...)
8596
id, err := randomID()
8697
if err != nil {
8798
return Lock{}, trace.Wrap(err)

‎lib/backend/lock_test.go

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -51,30 +51,30 @@ func TestLockConfiguration_CheckAndSetDefaults(t *testing.T) {
5151
{
5252
name: "minimum valid",
5353
in: LockConfiguration{
54-
Backend: mockBackend{},
55-
LockName: "lock",
56-
TTL: 30 * time.Second,
54+
Backend: mockBackend{},
55+
LockNameComponents: []string{"lock"},
56+
TTL: 30 * time.Second,
5757
},
5858
want: LockConfiguration{
59-
Backend: mockBackend{},
60-
LockName: "lock",
61-
TTL: 30 * time.Second,
62-
RetryInterval: 250 * time.Millisecond,
59+
Backend: mockBackend{},
60+
LockNameComponents: []string{"lock"},
61+
TTL: 30 * time.Second,
62+
RetryInterval: 250 * time.Millisecond,
6363
},
6464
},
6565
{
6666
name: "set RetryAcquireLockTimeout",
6767
in: LockConfiguration{
68-
Backend: mockBackend{},
69-
LockName: "lock",
70-
TTL: 30 * time.Second,
71-
RetryInterval: 10 * time.Second,
68+
Backend: mockBackend{},
69+
LockNameComponents: []string{"lock"},
70+
TTL: 30 * time.Second,
71+
RetryInterval: 10 * time.Second,
7272
},
7373
want: LockConfiguration{
74-
Backend: mockBackend{},
75-
LockName: "lock",
76-
TTL: 30 * time.Second,
77-
RetryInterval: 10 * time.Second,
74+
Backend: mockBackend{},
75+
LockNameComponents: []string{"lock"},
76+
TTL: 30 * time.Second,
77+
RetryInterval: 10 * time.Second,
7878
},
7979
},
8080
{
@@ -95,9 +95,9 @@ func TestLockConfiguration_CheckAndSetDefaults(t *testing.T) {
9595
{
9696
name: "missing TTL",
9797
in: LockConfiguration{
98-
Backend: mockBackend{},
99-
LockName: "lock",
100-
TTL: 0,
98+
Backend: mockBackend{},
99+
LockNameComponents: []string{"lock"},
100+
TTL: 0,
101101
},
102102
wantErr: "missing TTL",
103103
},
@@ -124,9 +124,9 @@ func TestRunWhileLockedConfigCheckAndSetDefaults(t *testing.T) {
124124
ttl := 1 * time.Minute
125125
minimumValidConfig := RunWhileLockedConfig{
126126
LockConfiguration: LockConfiguration{
127-
Backend: mockBackend{},
128-
LockName: lockName,
129-
TTL: ttl,
127+
Backend: mockBackend{},
128+
LockNameComponents: []string{lockName},
129+
TTL: ttl,
130130
},
131131
}
132132
tests := []struct {
@@ -142,10 +142,10 @@ func TestRunWhileLockedConfigCheckAndSetDefaults(t *testing.T) {
142142
},
143143
want: RunWhileLockedConfig{
144144
LockConfiguration: LockConfiguration{
145-
Backend: mockBackend{},
146-
LockName: lockName,
147-
TTL: ttl,
148-
RetryInterval: 250 * time.Millisecond,
145+
Backend: mockBackend{},
146+
LockNameComponents: []string{lockName},
147+
TTL: ttl,
148+
RetryInterval: 250 * time.Millisecond,
149149
},
150150
ReleaseCtxTimeout: time.Second,
151151
// defaults to halft of TTL.
@@ -157,6 +157,7 @@ func TestRunWhileLockedConfigCheckAndSetDefaults(t *testing.T) {
157157
input: func() RunWhileLockedConfig {
158158
cfg := minimumValidConfig
159159
cfg.LockName = ""
160+
cfg.LockNameComponents = nil
160161
return cfg
161162
},
162163
wantErr: "missing LockName",

‎lib/backend/test/suite.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -833,7 +833,7 @@ func testLocking(t *testing.T, newBackend Constructor) {
833833
defer requireNoAsyncErrors()
834834

835835
// Given a lock named `tok1` on the backend...
836-
lock, err := backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockName: tok1, TTL: ttl})
836+
lock, err := backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockNameComponents: []string{tok1}, TTL: ttl})
837837
require.NoError(t, err)
838838

839839
// When I asynchronously release the lock...
@@ -848,7 +848,7 @@ func testLocking(t *testing.T, newBackend Constructor) {
848848
}()
849849

850850
// ...and simultaneously attempt to create a new lock with the same name
851-
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockName: tok1, TTL: ttl})
851+
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockNameComponents: []string{tok1}, TTL: ttl})
852852

853853
// expect that the asynchronous Release() has executed - we're using the
854854
// change in the value of the marker value as a proxy for the Release().
@@ -860,7 +860,7 @@ func testLocking(t *testing.T, newBackend Constructor) {
860860
require.NoError(t, lock.Release(ctx, uut))
861861

862862
// Given a lock with the same name as previously-existing, manually-released lock
863-
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockName: tok1, TTL: ttl})
863+
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockNameComponents: []string{tok1}, TTL: ttl})
864864
require.NoError(t, err)
865865
atomic.StoreInt32(&marker, 7)
866866

@@ -875,7 +875,7 @@ func testLocking(t *testing.T, newBackend Constructor) {
875875
}()
876876

877877
// ...and simultaneously try to acquire another lock with the same name
878-
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockName: tok1, TTL: ttl})
878+
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockNameComponents: []string{tok1}, TTL: ttl})
879879

880880
// expect that the asynchronous Release() has executed - we're using the
881881
// change in the value of the marker value as a proxy for the call to
@@ -889,9 +889,9 @@ func testLocking(t *testing.T, newBackend Constructor) {
889889

890890
// Given a pair of locks named `tok1` and `tok2`
891891
y := int32(0)
892-
lock1, err := backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockName: tok1, TTL: ttl})
892+
lock1, err := backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockNameComponents: []string{tok1}, TTL: ttl})
893893
require.NoError(t, err)
894-
lock2, err := backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockName: tok2, TTL: ttl})
894+
lock2, err := backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockNameComponents: []string{tok2}, TTL: ttl})
895895
require.NoError(t, err)
896896

897897
// When I asynchronously release the locks...
@@ -908,7 +908,7 @@ func testLocking(t *testing.T, newBackend Constructor) {
908908
}
909909
}()
910910

911-
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockName: tok1, TTL: ttl})
911+
lock, err = backend.AcquireLock(ctx, backend.LockConfiguration{Backend: uut, LockNameComponents: []string{tok1}, TTL: ttl})
912912
require.NoError(t, err)
913913
require.Equal(t, int32(15), atomic.LoadInt32(&y))
914914
require.NoError(t, lock.Release(ctx, uut))

‎lib/events/athena/consumer.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,8 +253,8 @@ func (c *consumer) runContinuouslyOnSingleAuth(ctx context.Context, eventsProces
253253
default:
254254
err := backend.RunWhileLocked(ctx, backend.RunWhileLockedConfig{
255255
LockConfiguration: backend.LockConfiguration{
256-
Backend: c.backend,
257-
LockName: "athena_lock",
256+
Backend: c.backend,
257+
LockNameComponents: []string{"athena_lock"},
258258
// TTL is higher then batchMaxInterval because we want to optimize
259259
// for low backend writes.
260260
TTL: 5 * c.batchMaxInterval,

‎lib/services/local/access.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,9 @@ func (s *AccessService) DeleteAllLocks(ctx context.Context) error {
327327
func (s *AccessService) ReplaceRemoteLocks(ctx context.Context, clusterName string, newRemoteLocks []types.Lock) error {
328328
return backend.RunWhileLocked(ctx, backend.RunWhileLockedConfig{
329329
LockConfiguration: backend.LockConfiguration{
330-
Backend: s.Backend,
331-
LockName: "ReplaceRemoteLocks/" + clusterName,
332-
TTL: time.Minute,
330+
Backend: s.Backend,
331+
LockNameComponents: []string{"ReplaceRemoteLocks", clusterName},
332+
TTL: time.Minute,
333333
},
334334
}, func(ctx context.Context) error {
335335
remoteLocksKey := backend.ExactKey(locksPrefix, clusterName)

‎lib/services/local/access_list.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ package local
1818

1919
import (
2020
"context"
21-
"strings"
2221
"time"
2322

2423
"github.com/google/go-cmp/cmp"
@@ -196,7 +195,7 @@ func (a *AccessListService) runOpWithLock(ctx context.Context, accessList *acces
196195

197196
var err error
198197
if feature := modules.GetModules().Features(); !feature.IGSEnabled() {
199-
err = a.service.RunWhileLocked(ctx, "createAccessListLimitLock", accessListLockTTL, func(ctx context.Context, _ backend.Backend) error {
198+
err = a.service.RunWhileLocked(ctx, []string{"createAccessListLimitLock"}, accessListLockTTL, func(ctx context.Context, _ backend.Backend) error {
200199
if err := a.VerifyAccessListCreateLimit(ctx, accessList.GetName()); err != nil {
201200
return trace.Wrap(err)
202201
}
@@ -453,7 +452,7 @@ func (a *AccessListService) UpsertAccessListWithMembers(ctx context.Context, acc
453452

454453
var err error
455454
if feature := modules.GetModules().Features(); !feature.IGSEnabled() {
456-
err = a.service.RunWhileLocked(ctx, "createAccessListWithMembersLimitLock", accessListLockTTL, func(ctx context.Context, _ backend.Backend) error {
455+
err = a.service.RunWhileLocked(ctx, []string{"createAccessListWithMembersLimitLock"}, accessListLockTTL, func(ctx context.Context, _ backend.Backend) error {
457456
if err := a.VerifyAccessListCreateLimit(ctx, accessList.GetName()); err != nil {
458457
return trace.Wrap(err)
459458
}
@@ -638,8 +637,8 @@ func (a *AccessListService) DeleteAllAccessListReviews(ctx context.Context) erro
638637
return trace.Wrap(a.reviewService.DeleteAllResources(ctx))
639638
}
640639

641-
func lockName(accessListName string) string {
642-
return strings.Join([]string{"access_list", accessListName}, string(backend.Separator))
640+
func lockName(accessListName string) []string {
641+
return []string{"access_list", accessListName}
643642
}
644643

645644
// VerifyAccessListCreateLimit ensures creating access list is limited to no more than 1 (updating is allowed).

‎lib/services/local/externalauditstorage.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ func (s *ExternalAuditStorageService) CreateDraftExternalAuditStorage(ctx contex
8383
var lease *backend.Lease
8484
err = backend.RunWhileLocked(ctx, backend.RunWhileLockedConfig{
8585
LockConfiguration: backend.LockConfiguration{
86-
Backend: s.backend,
87-
LockName: externalAuditStorageLockName,
88-
TTL: externalAuditStorageLockTTL,
86+
Backend: s.backend,
87+
LockNameComponents: []string{externalAuditStorageLockName},
88+
TTL: externalAuditStorageLockTTL,
8989
},
9090
}, func(ctx context.Context) error {
9191
// Check that the referenced AWS OIDC integration actually exists.
@@ -122,9 +122,9 @@ func (s *ExternalAuditStorageService) UpsertDraftExternalAuditStorage(ctx contex
122122
var lease *backend.Lease
123123
err = backend.RunWhileLocked(ctx, backend.RunWhileLockedConfig{
124124
LockConfiguration: backend.LockConfiguration{
125-
Backend: s.backend,
126-
LockName: externalAuditStorageLockName,
127-
TTL: externalAuditStorageLockTTL,
125+
Backend: s.backend,
126+
LockNameComponents: []string{externalAuditStorageLockName},
127+
TTL: externalAuditStorageLockTTL,
128128
},
129129
}, func(ctx context.Context) error {
130130
// Check that the referenced AWS OIDC integration actually exists.
@@ -185,9 +185,9 @@ func (s *ExternalAuditStorageService) GetClusterExternalAuditStorage(ctx context
185185
func (s *ExternalAuditStorageService) PromoteToClusterExternalAuditStorage(ctx context.Context) error {
186186
err := backend.RunWhileLocked(ctx, backend.RunWhileLockedConfig{
187187
LockConfiguration: backend.LockConfiguration{
188-
Backend: s.backend,
189-
LockName: externalAuditStorageLockName,
190-
TTL: externalAuditStorageLockTTL,
188+
Backend: s.backend,
189+
LockNameComponents: []string{externalAuditStorageLockName},
190+
TTL: externalAuditStorageLockTTL,
191191
},
192192
}, func(ctx context.Context) error {
193193
draft, err := s.GetDraftExternalAuditStorage(ctx)

‎lib/services/local/generic/generic.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -421,14 +421,14 @@ func (s *Service[T]) MakeKey(name string) backend.Key {
421421
}
422422

423423
// RunWhileLocked will run the given function in a backend lock. This is a wrapper around the backend.RunWhileLocked function.
424-
func (s *Service[T]) RunWhileLocked(ctx context.Context, lockName string, ttl time.Duration, fn func(context.Context, backend.Backend) error) error {
424+
func (s *Service[T]) RunWhileLocked(ctx context.Context, lockNameComponents []string, ttl time.Duration, fn func(context.Context, backend.Backend) error) error {
425425
return trace.Wrap(backend.RunWhileLocked(ctx,
426426
backend.RunWhileLockedConfig{
427427
LockConfiguration: backend.LockConfiguration{
428-
Backend: s.backend,
429-
LockName: lockName,
430-
TTL: ttl,
431-
RetryInterval: s.runWhileLockedRetryInterval,
428+
Backend: s.backend,
429+
LockNameComponents: lockNameComponents,
430+
TTL: ttl,
431+
RetryInterval: s.runWhileLockedRetryInterval,
432432
},
433433
}, func(ctx context.Context) error {
434434
return fn(ctx, s.backend)

‎lib/services/local/generic/generic_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ func TestGenericCRUD(t *testing.T) {
256256
require.ErrorIs(t, err, trace.NotFound(`generic resource "doesnotexist" doesn't exist`))
257257

258258
// Test running while locked.
259-
err = service.RunWhileLocked(ctx, "test-lock", time.Second*5, func(ctx context.Context, backend backend.Backend) error {
259+
err = service.RunWhileLocked(ctx, []string{"test-lock"}, time.Second*5, func(ctx context.Context, backend backend.Backend) error {
260260
item, err := backend.Get(ctx, service.MakeKey(r1.GetName()))
261261
require.NoError(t, err)
262262

‎lib/services/local/integrations.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ func (s *IntegrationsService) DeleteIntegration(ctx context.Context, name string
124124
// so that no new EAS integrations can be concurrently created.
125125
err := backend.RunWhileLocked(ctx, backend.RunWhileLockedConfig{
126126
LockConfiguration: backend.LockConfiguration{
127-
Backend: s.backend,
128-
LockName: externalAuditStorageLockName,
129-
TTL: externalAuditStorageLockTTL,
127+
Backend: s.backend,
128+
LockNameComponents: []string{externalAuditStorageLockName},
129+
TTL: externalAuditStorageLockTTL,
130130
},
131131
}, func(ctx context.Context) error {
132132
if err := notReferencedByEAS(ctx, s.backend, name); err != nil {

‎lib/services/local/saml_idp_service_provider.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ func (s *SAMLIdPServiceProviderService) CreateSAMLIdPServiceProvider(ctx context
143143
return trace.Wrap(err)
144144
}
145145

146-
return trace.Wrap(s.svc.RunWhileLocked(ctx, samlIDPServiceProviderModifyLock, samlIDPServiceProviderModifyLockTTL,
146+
return trace.Wrap(s.svc.RunWhileLocked(ctx, []string{samlIDPServiceProviderModifyLock}, samlIDPServiceProviderModifyLockTTL,
147147
func(ctx context.Context, backend backend.Backend) error {
148148
if err := s.ensureEntityIDIsUnique(ctx, sp); err != nil {
149149
return trace.Wrap(err)
@@ -181,7 +181,7 @@ func (s *SAMLIdPServiceProviderService) UpdateSAMLIdPServiceProvider(ctx context
181181
return trace.Wrap(err)
182182
}
183183

184-
return trace.Wrap(s.svc.RunWhileLocked(ctx, samlIDPServiceProviderModifyLock, samlIDPServiceProviderModifyLockTTL,
184+
return trace.Wrap(s.svc.RunWhileLocked(ctx, []string{samlIDPServiceProviderModifyLock}, samlIDPServiceProviderModifyLockTTL,
185185
func(ctx context.Context, backend backend.Backend) error {
186186
if err := s.ensureEntityIDIsUnique(ctx, sp); err != nil {
187187
return trace.Wrap(err)

0 commit comments

Comments
 (0)