Fix backends with prefixes (#47238)

#46701 incorrectly changed how the prefixes for the etcd and dynamo
backends were applied and removed. In addition, the backend lock
key was not being constructed correctly and caused acquisition of
locks to fail on a cluster that had been upgraded.

Tests have been added to ensure that keys are now being constructed
correctly in all cases. Additionally, backend/helpers.go was moved
to backed/lock.go to better described what functionality the file
contains.

Backends with prefixes (etcd, dynamo) were incorrectly constructing
the key for locks. In addition the prefixes were not being trimmed
correctly which broke CAS operations.

lib/backend/dynamo/dynamodbbk.go

@@ -215,10 +215,10 @@ const (
 	hashKeyKey = "HashKey"
 )
 
-var (
+const (
 	// keyPrefix is a prefix that is added to every dynamodb key
 	// for backwards compatibility
-	keyPrefix = backend.KeyFromString("teleport")
+	keyPrefix = "teleport"
 )
 
 // GetName is a part of backend API and it returns DynamoDB backend type
@@ -1000,12 +1000,12 @@ const (
 // prependPrefix adds leading 'teleport/' to the key for backwards compatibility
 // with previous implementation of DynamoDB backend
 func prependPrefix(key backend.Key) string {
-	return keyPrefix.AppendKey(key).String()
+	return keyPrefix + key.String()
 }
 
 // trimPrefix removes leading 'teleport' from the key
 func trimPrefix(key string) backend.Key {
-	return backend.KeyFromString(key).TrimPrefix(keyPrefix)
+	return backend.KeyFromString(key).TrimPrefix(backend.KeyFromString(keyPrefix))
 }
 
 // create is a helper that writes a key/value pair in Dynamo with a given expiration.

lib/backend/dynamo/dynamodbbk_test.go

@@ -404,3 +404,21 @@ const (
 	readScalingPolicySuffix  = "read-target-tracking-scaling-policy"
 	writeScalingPolicySuffix = "write-target-tracking-scaling-policy"
 )
+
+func TestKeyPrefix(t *testing.T) {
+	t.Run("leading separator in key", func(t *testing.T) {
+		prefixed := prependPrefix(backend.NewKey("test", "llama"))
+		assert.Equal(t, "teleport/test/llama", prefixed)
+
+		key := trimPrefix(prefixed)
+		assert.Equal(t, "/test/llama", key.String())
+	})
+
+	t.Run("no leading separator in key", func(t *testing.T) {
+		prefixed := prependPrefix(backend.KeyFromString(".locks/test/llama"))
+		assert.Equal(t, "teleport.locks/test/llama", prefixed)
+
+		key := trimPrefix(prefixed)
+		assert.Equal(t, ".locks/test/llama", key.String())
+	})
+}

lib/backend/etcdbk/etcd.go

@@ -162,7 +162,6 @@ type EtcdBackend struct {
 	ctx         context.Context
 	cancel      context.CancelFunc
 	watchDone   chan struct{}
-	prefix      backend.Key
 }
 
 // Config represents JSON config for etcd backend
@@ -291,7 +290,6 @@ func New(ctx context.Context, params backend.Params, opts ...Option) (*EtcdBacke
 		buf:         buf,
 		leaseBucket: utils.SeventhJitter(options.leaseBucket),
 		leaseCache:  leaseCache,
-		prefix:      backend.KeyFromString(cfg.Key),
 	}
 
 	// Check that the etcd nodes are at least the minimum version supported
@@ -1109,9 +1107,9 @@ func fromType(eventType mvccpb.Event_EventType) types.OpType {
 }
 
 func (b *EtcdBackend) trimPrefix(in []byte) backend.Key {
-	return backend.KeyFromString(string(in)).TrimPrefix(b.prefix)
+	return backend.KeyFromString(string(in)).TrimPrefix(backend.KeyFromString(b.cfg.Key))
 }
 
 func (b *EtcdBackend) prependPrefix(in backend.Key) string {
-	return in.PrependKey(b.prefix).String()
+	return b.cfg.Key + in.String()
 }

lib/backend/etcdbk/etcd_test.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/lib/backend"
@@ -258,3 +259,29 @@ func etcdTestEndpoint() string {
 	}
 	return "https://127.0.0.1:2379"
 }
+
+func TestKeyPrefix(t *testing.T) {
+	prefixes := []string{"teleport", "/teleport", "/teleport/"}
+
+	for _, prefix := range prefixes {
+		t.Run("prefix="+prefix, func(t *testing.T) {
+			bk := EtcdBackend{cfg: &Config{Key: prefix}}
+
+			t.Run("leading separator in key", func(t *testing.T) {
+				prefixed := bk.prependPrefix(backend.NewKey("test", "llama"))
+				assert.Equal(t, prefix+"/test/llama", prefixed)
+
+				key := bk.trimPrefix([]byte(prefixed))
+				assert.Equal(t, "/test/llama", key.String())
+			})
+
+			t.Run("no leading separator in key", func(t *testing.T) {
+				prefixed := bk.prependPrefix(backend.KeyFromString(".locks/test/llama"))
+				assert.Equal(t, prefix+".locks/test/llama", prefixed)
+
+				key := bk.trimPrefix([]byte(prefixed))
+				assert.Equal(t, ".locks/test/llama", key.String())
+			})
+		})
+	}
+}

lib/backend/helpers.go → lib/backend/lock.go

@@ -22,6 +22,7 @@ import (
 	"bytes"
 	"context"
 	"log/slog"
+	"strings"
 	"time"
 
 	"github.com/google/uuid"
@@ -35,7 +36,12 @@ const (
 )
 
 func lockKey(parts ...string) Key {
-	return internalKey(locksPrefix, parts...)
+	key := Key{
+		components: append([]string{locksPrefix}, parts...),
+	}
+	key.s = strings.Join(key.components, SeparatorString)
+
+	return key
 }
 
 type Lock struct {

lib/backend/helpers_test.go → lib/backend/lock_test.go

@@ -23,9 +23,24 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestLockKey(t *testing.T) {
+	t.Run("empty parts", func(t *testing.T) {
+		key := lockKey()
+		assert.Equal(t, ".locks", key.String())
+		assert.Equal(t, []string{".locks"}, key.Components())
+	})
+
+	t.Run("with parts", func(t *testing.T) {
+		key := lockKey("test", "llama")
+		assert.Equal(t, ".locks/test/llama", key.String())
+		assert.Equal(t, []string{".locks", "test", "llama"}, key.Components())
+	})
+}
+
 func TestLockConfiguration_CheckAndSetDefaults(t *testing.T) {
 	type mockBackend struct {
 		Backend