Fix backends with prefixes (#47238) (#47485)

#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

@@ -916,7 +916,7 @@ 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 + string(key)
+	return keyPrefix + key.String()
 }
 
 // trimPrefix removes leading 'teleport' from the key

lib/backend/dynamo/dynamodbbk_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
 	log "github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/lib/backend"
@@ -222,3 +223,21 @@ func TestCreateTable(t *testing.T) {
 		})
 	}
 }
+
+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.Key(".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

@@ -1106,10 +1106,10 @@ func fromType(eventType mvccpb.Event_EventType) types.OpType {
 	}
 }
 
-func (b *EtcdBackend) trimPrefix(in backend.Key) backend.Key {
-	return in.TrimPrefix(backend.Key(b.cfg.Key))
+func (b *EtcdBackend) trimPrefix(in []byte) backend.Key {
+	return backend.Key(in).TrimPrefix(backend.Key(b.cfg.Key))
 }
 
 func (b *EtcdBackend) prependPrefix(in backend.Key) string {
-	return b.cfg.Key + string(in)
+	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.Key(".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

@@ -21,11 +21,13 @@ package backend
 import (
 	"bytes"
 	"context"
+	"log/slog"
 	"time"
 
 	"github.com/google/uuid"
 	"github.com/gravitational/trace"
-	log "github.com/sirupsen/logrus"
+
+	logutils "github.com/gravitational/teleport/lib/utils/log"
 )
 
 const (
@@ -205,7 +207,7 @@ func RunWhileLocked(ctx context.Context, cfg RunWhileLockedConfig, fn func(conte
 			case <-cfg.Backend.Clock().After(refreshAfter):
 				if err := lock.resetTTL(ctx, cfg.Backend); err != nil {
 					cancelFunction()
-					log.Errorf("%v", err)
+					slog.ErrorContext(ctx, "failed to reset lock ttl", "error", err, "lock", logutils.StringerAttr(lock.key))
 					return
 				}
 			case <-stopRefresh:

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, [][]byte{[]byte(".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, [][]byte{[]byte(".locks"), []byte("test"), []byte("llama")}, key.Components())
+	})
+}
+
 func TestLockConfiguration_CheckAndSetDefaults(t *testing.T) {
 	type mockBackend struct {
 		Backend