chore: remove kv Session service and sessionsv1 bucket (#18823)

* chore: remove sessionsv1 bucket

* chore: add extra configuration to sessions.Service

cmd/influxd/launcher/launcher.go

@@ -925,7 +925,13 @@ func (m *Launcher) run(ctx context.Context) (err error) {
 
 	var sessionSvc platform.SessionService
 	{
-		sessionSvc = session.NewService(session.NewStorage(inmem.NewSessionStore()), userSvc, userResourceSvc, authSvc, time.Duration(m.sessionLength)*time.Minute)
+		sessionSvc = session.NewService(
+			session.NewStorage(inmem.NewSessionStore()),
+			userSvc,
+			userResourceSvc,
+			authSvc,
+			session.WithSessionLength(time.Duration(m.sessionLength)*time.Minute),
+		)
 		sessionSvc = session.NewSessionMetrics(m.reg, sessionSvc)
 		sessionSvc = session.NewSessionLogger(m.log.With(zap.String("service", "session")), sessionSvc)
 	}

kv/initial_migration.go

@@ -43,7 +43,6 @@ func (m InitialMigration) Up(ctx context.Context, store SchemaStore) error {
 		userpasswordBucket,
 		scrapersBucket,
 		secretBucket,
-		sessionBucket,
 		telegrafBucket,
 		telegrafPluginsBucket,
 		urmBucket,
@@ -62,6 +61,8 @@ func (m InitialMigration) Up(ctx context.Context, store SchemaStore) error {
 		variableBucket,
 		variableIndexBucket,
 		variableOrgsIndex,
+		// deprecated: removed in later migration
+		[]byte("sessionsv1"),
 	} {
 		if err := store.CreateBucket(ctx, bucket); err != nil {
 			return err

kv/migration/all/0006_delete-bucket-sessionsv1.go

@@ -0,0 +1,7 @@
+package all
+
+import "github.com/influxdata/influxdb/v2/kv/migration"
+
+// Migration0006_DeleteBucketSessionsv1 removes the sessionsv1 bucket
+// from the backing kv store.
+var Migration0006_DeleteBucketSessionsv1 = migration.DeleteBuckets("delete sessionsv1 bucket", []byte("sessionsv1"))

kv/migration/all/all.go

@@ -17,5 +17,7 @@ var Migrations = [...]migration.Spec{
 	Migration0004_AddDbrpBuckets,
 	// add pkger buckets
 	Migration0005_AddPkgerBuckets,
+	// delete bucket sessionsv1
+	Migration0006_DeleteBucketSessionsv1,
 	// {{ do_not_edit . }}
 }

kv/migration/buckets.go

@@ -6,41 +6,78 @@ import (
 	"github.com/influxdata/influxdb/v2/kv"
 )
 
-// CreateBucketsMigration is a migration Spec which creates
+type bucketMigrationType string
+
+const (
+	createBucketMigration = bucketMigrationType("create")
+	deleteBucketMigration = bucketMigrationType("delete")
+)
+
+// BucketsMigration is a migration Spec which creates
 // the provided list of buckets on a store when Up is called
 // and deletes them on Down.
-type CreateBucketsMigration struct {
+type BucketsMigration struct {
+	typ     bucketMigrationType
 	name    string
 	buckets [][]byte
 }
 
-// CreateBuckets returns a new CreateBucketsMigration Spec.
-func CreateBuckets(name string, buckets ...[]byte) Spec {
-	return CreateBucketsMigration{name, buckets}
+// CreateBuckets returns a new BucketsMigration Spec.
+func CreateBuckets(name string, bucket []byte, extraBuckets ...[]byte) Spec {
+	buckets := append([][]byte{bucket}, extraBuckets...)
+	return BucketsMigration{createBucketMigration, name, buckets}
+}
+
+// DeleteBuckets returns a new BucketsMigration Spec.
+func DeleteBuckets(name string, bucket []byte, extraBuckets ...[]byte) Spec {
+	buckets := append([][]byte{bucket}, extraBuckets...)
+	return BucketsMigration{deleteBucketMigration, name, buckets}
 }
 
 // MigrationName returns the name of the migration.
-func (c CreateBucketsMigration) MigrationName() string {
-	return c.name
+func (m BucketsMigration) MigrationName() string {
+	return m.name
 }
 
 // Up creates the buckets on the store.
-func (c CreateBucketsMigration) Up(ctx context.Context, store kv.SchemaStore) error {
-	for _, bucket := range c.buckets {
-		if err := store.CreateBucket(ctx, bucket); err != nil {
+func (m BucketsMigration) Up(ctx context.Context, store kv.SchemaStore) error {
+	var fn func(context.Context, []byte) error
+	switch m.typ {
+	case createBucketMigration:
+		fn = store.CreateBucket
+	case deleteBucketMigration:
+		fn = store.DeleteBucket
+	default:
+		panic("unrecognized buckets migration type")
+	}
+
+	for _, bucket := range m.buckets {
+		if err := fn(ctx, bucket); err != nil {
 			return err
 		}
+
 	}
 
 	return nil
 }
 
 // Down delets the buckets on the store.
-func (c CreateBucketsMigration) Down(ctx context.Context, store kv.SchemaStore) error {
-	for _, bucket := range c.buckets {
-		if err := store.DeleteBucket(ctx, bucket); err != nil {
+func (m BucketsMigration) Down(ctx context.Context, store kv.SchemaStore) error {
+	var fn func(context.Context, []byte) error
+	switch m.typ {
+	case createBucketMigration:
+		fn = store.DeleteBucket
+	case deleteBucketMigration:
+		fn = store.CreateBucket
+	default:
+		panic("unrecognized buckets migration type")
+	}
+
+	for _, bucket := range m.buckets {
+		if err := fn(ctx, bucket); err != nil {
 			return err
 		}
+
 	}
 
 	return nil

kv/migration/buckets_test.go

@@ -0,0 +1,97 @@
+package migration
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/influxdata/influxdb/v2/inmem"
+	"github.com/influxdata/influxdb/v2/kv"
+)
+
+func Test_BucketsMigration_CreateBuckets(t *testing.T) {
+	var (
+		ctx    = context.Background()
+		bucket = []byte("some_bucket")
+		store  = inmem.NewKVStore()
+	)
+
+	// bucket should not exist
+	bucketShouldNotExist(t, store, bucket)
+
+	// build new create buckets migration
+	migration := CreateBuckets("create bucket some_bucket", bucket)
+
+	// apply migration up
+	if err := migration.Up(ctx, store); err != nil {
+		t.Fatal("unexpected error", err)
+	}
+
+	// bucket should now exist
+	bucketShouldExist(t, store, bucket)
+
+	// apply migration down
+	if err := migration.Down(ctx, store); err != nil {
+		t.Fatal("unexpected error", err)
+	}
+
+	// bucket should no longer exist
+	bucketShouldNotExist(t, store, bucket)
+}
+
+func Test_BucketsMigration_DeleteBuckets(t *testing.T) {
+	var (
+		ctx    = context.Background()
+		bucket = []byte("some_bucket")
+		store  = inmem.NewKVStore()
+	)
+
+	// initially create bucket
+	if err := store.CreateBucket(ctx, bucket); err != nil {
+		t.Fatal("unexpected error", err)
+	}
+
+	// ensure bucket is there to start with
+	bucketShouldExist(t, store, bucket)
+
+	// build new delete buckets migration
+	migration := DeleteBuckets("delete bucket some_bucket", bucket)
+
+	// apply migration up
+	if err := migration.Up(ctx, store); err != nil {
+		t.Fatal("unexpected error", err)
+	}
+
+	// bucket should have been removed
+	bucketShouldNotExist(t, store, bucket)
+
+	// apply migration down
+	if err := migration.Down(ctx, store); err != nil {
+		t.Fatal("unexpected error", err)
+	}
+
+	// bucket should exist again
+	bucketShouldExist(t, store, bucket)
+}
+
+func bucketShouldExist(t *testing.T, store kv.Store, bucket []byte) {
+	t.Helper()
+
+	if err := store.View(context.Background(), func(tx kv.Tx) error {
+		_, err := tx.Bucket(bucket)
+		return err
+	}); err != nil {
+		t.Fatal("unexpected error", err)
+	}
+}
+
+func bucketShouldNotExist(t *testing.T, store kv.Store, bucket []byte) {
+	t.Helper()
+
+	if err := store.View(context.Background(), func(tx kv.Tx) error {
+		_, err := tx.Bucket(bucket)
+		return err
+	}); !errors.Is(err, kv.ErrBucketNotFound) {
+		t.Fatalf("expected bucket not found, got %q", err)
+	}
+}