Migrations: Revert remove ACL migration (#5942)

* Migrations: Revert remove ACL migration * Gofmt * CR Fixes * CR Fixes * CR Fixes 2
treeverse · May 31, 2023 · acbc0dc · acbc0dc
1 parent 9bbeb4e
commit acbc0dc
Show file tree

Hide file tree

Showing 8 changed files with 870 additions and 42 deletions.
diff --git a/cmd/lakefs/cmd/migrate.go b/cmd/lakefs/cmd/migrate.go
@@ -10,6 +10,7 @@ import (
 	"github.com/treeverse/lakefs/pkg/config"
 	"github.com/treeverse/lakefs/pkg/kv"
 	"github.com/treeverse/lakefs/pkg/kv/migrations"
+	"github.com/treeverse/lakefs/pkg/logging"
 )
 
 // migrateCmd represents the migrate command
@@ -77,6 +78,8 @@ var upCmd = &cobra.Command{
 		}
 		defer kvStore.Close()
 
+		force, _ := cmd.Flags().GetBool("force")
+
 		_, err = kv.ValidateSchemaVersion(ctx, kvStore)
 		switch {
 		case err == nil:
@@ -85,7 +88,7 @@ var upCmd = &cobra.Command{
 			_, _ = fmt.Fprintf(os.Stderr, "%s\n", err)
 			os.Exit(1)
 		case errors.Is(err, kv.ErrMigrationRequired):
-			err = doMigration(ctx, kvStore, cfg)
+			err = DoMigration(ctx, kvStore, cfg, force)
 			if err != nil {
 				_, _ = fmt.Fprintf(os.Stderr, "Migration failed: %s\n", err)
 				os.Exit(1)
@@ -98,7 +101,7 @@ var upCmd = &cobra.Command{
 	},
 }
 
-func doMigration(ctx context.Context, kvStore kv.Store, cfg *config.Config) error {
+func DoMigration(ctx context.Context, kvStore kv.Store, cfg *config.Config, force bool) error {
 	var (
 		version int
 		err     error
@@ -109,22 +112,15 @@ func doMigration(ctx context.Context, kvStore kv.Store, cfg *config.Config) erro
 			return err
 		}
 		switch {
-		case version < kv.ACLNoReposMigrateVersion || version >= kv.NextSchemaVersion:
+		case version >= kv.NextSchemaVersion || version < kv.ACLMigrateVersion:
 			return fmt.Errorf("wrong starting version %d: %w", version, kv.ErrMigrationVersion)
-
+		case version < kv.ACLNoReposMigrateVersion:
+			err = migrations.MigrateToACL(ctx, kvStore, cfg, logging.Default(), version, force)
 		case version < kv.ACLImportMigrateVersion:
-			// skip migrate to ACL for users with External authorizations
-			if !cfg.IsAuthUISimplified() {
-				fmt.Println("skipping ACL migration - external Authorization")
-				err = kv.SetDBSchemaVersion(ctx, kvStore, kv.ACLImportMigrateVersion)
-				if err != nil {
-					return fmt.Errorf("failed to upgrade version, to fix this re-run migration: %w", err)
-				}
-			} else {
-				if err = migrations.MigrateImportPermissions(ctx, kvStore); err != nil {
-					return err
-				}
-			}
+			err = migrations.MigrateImportPermissions(ctx, kvStore, cfg)
+		}
+		if err != nil {
+			return err
 		}
 	}
 	return nil

diff --git a/cmd/lakefs/cmd/migrate_test.go b/cmd/lakefs/cmd/migrate_test.go
@@ -0,0 +1,110 @@
+package cmd_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/treeverse/lakefs/cmd/lakefs/cmd"
+	"github.com/treeverse/lakefs/pkg/config"
+	"github.com/treeverse/lakefs/pkg/kv"
+	"github.com/treeverse/lakefs/pkg/kv/kvtest"
+)
+
+func TestDoMigrate(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("not_initialized", func(t *testing.T) {
+		kvStore := kvtest.GetStore(ctx, t)
+		err := cmd.DoMigration(ctx, kvStore, nil, false)
+		require.ErrorIs(t, err, kv.ErrNotFound)
+		_, err = kv.GetDBSchemaVersion(ctx, kvStore)
+		require.ErrorIs(t, err, kv.ErrNotFound)
+	})
+
+	t.Run("not_meets_required_version", func(t *testing.T) {
+		kvStore := kvtest.GetStore(ctx, t)
+		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.InitialMigrateVersion))
+		err := cmd.DoMigration(ctx, kvStore, nil, false)
+		require.ErrorIs(t, err, kv.ErrMigrationVersion)
+		version, err := kv.GetDBSchemaVersion(ctx, kvStore)
+		require.NoError(t, err)
+		require.Equal(t, kv.InitialMigrateVersion, version)
+	})
+
+	t.Run("from_acl_v1_no_force", func(t *testing.T) {
+		cfg := config.Config{}
+		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
+		kvStore := kvtest.GetStore(ctx, t)
+		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.ACLMigrateVersion))
+		err := cmd.DoMigration(ctx, kvStore, &cfg, false)
+		require.ErrorIs(t, err, kv.ErrMigrationVersion)
+		version, err := kv.GetDBSchemaVersion(ctx, kvStore)
+		require.NoError(t, err)
+		require.Equal(t, kv.ACLMigrateVersion, version)
+	})
+
+	t.Run("from_acl_v1_force", func(t *testing.T) {
+		cfg := config.Config{}
+		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
+		kvStore := kvtest.GetStore(ctx, t)
+		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.ACLNoReposMigrateVersion))
+		err := cmd.DoMigration(ctx, kvStore, &cfg, true)
+		require.NoError(t, err)
+		version, err := kv.GetDBSchemaVersion(ctx, kvStore)
+		require.NoError(t, err)
+		require.True(t, kv.IsLatestSchemaVersion(version))
+	})
+
+	t.Run("from_acl_v2", func(t *testing.T) {
+		cfg := config.Config{}
+		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
+		startVer := kv.ACLNoReposMigrateVersion
+		for !kv.IsLatestSchemaVersion(startVer) {
+			kvStore := kvtest.GetStore(ctx, t)
+			require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, uint(startVer)))
+			err := cmd.DoMigration(ctx, kvStore, &cfg, false)
+			require.NoError(t, err)
+			version, err := kv.GetDBSchemaVersion(ctx, kvStore)
+			require.NoError(t, err)
+			require.True(t, kv.IsLatestSchemaVersion(version))
+			startVer += 1
+		}
+	})
+
+	t.Run("latest_version", func(t *testing.T) {
+		cfg := config.Config{}
+		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
+		kvStore := kvtest.GetStore(ctx, t)
+		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.NextSchemaVersion-1))
+		err := cmd.DoMigration(ctx, kvStore, &cfg, false)
+		require.NoError(t, err)
+		version, err := kv.GetDBSchemaVersion(ctx, kvStore)
+		require.NoError(t, err)
+		require.True(t, kv.IsLatestSchemaVersion(version))
+	})
+
+	t.Run("next_version", func(t *testing.T) {
+		cfg := config.Config{}
+		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
+		kvStore := kvtest.GetStore(ctx, t)
+		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, kv.NextSchemaVersion))
+		err := cmd.DoMigration(ctx, kvStore, &cfg, false)
+		require.ErrorIs(t, err, kv.ErrMigrationVersion)
+		version, err := kv.GetDBSchemaVersion(ctx, kvStore)
+		require.NoError(t, err)
+		require.Equal(t, kv.NextSchemaVersion, version)
+	})
+
+	t.Run("invalid_version", func(t *testing.T) {
+		cfg := config.Config{}
+		cfg.Auth.UIConfig.RBAC = config.AuthRBACSimplified
+		kvStore := kvtest.GetStore(ctx, t)
+		require.NoError(t, kv.SetDBSchemaVersion(ctx, kvStore, 0))
+		err := cmd.DoMigration(ctx, kvStore, &cfg, false)
+		require.ErrorIs(t, err, kv.ErrMigrationVersion)
+		version, err := kv.GetDBSchemaVersion(ctx, kvStore)
+		require.NoError(t, err)
+		require.Equal(t, 0, version)
+	})
+}
diff --git a/pkg/auth/base.go b/pkg/auth/base.go
@@ -76,6 +76,25 @@ var statementByName = map[string]model.Statement{
 	},
 }
 
+// GetActionsForPolicyType returns the actions for police type typ.
+func GetActionsForPolicyType(typ string) ([]string, error) {
+	statement, ok := statementByName[typ]
+	if !ok {
+		return nil, fmt.Errorf("%w: %s", ErrStatementNotFound, typ)
+	}
+	actions := make([]string, len(statement.Action))
+	copy(actions, statement.Action)
+	return actions, nil
+}
+
+func GetActionsForPolicyTypeOrDie(typ string) []string {
+	ret, err := GetActionsForPolicyType(typ)
+	if err != nil {
+		panic(err)
+	}
+	return ret
+}
+
 // MakeStatementForPolicyType returns statements for policy type typ,
 // limited to resources.
 func MakeStatementForPolicyType(typ string, resources []string) (model.Statements, error) {

diff --git a/pkg/kv/migration.go b/pkg/kv/migration.go
@@ -58,8 +58,9 @@ func ValidateSchemaVersion(ctx context.Context, store Store) (int, error) {
 		return kvVersion, fmt.Errorf("incompatible schema version. Latest: %d: %w", NextSchemaVersion-1, ErrMigrationVersion)
 	case kvVersion < InitialMigrateVersion:
 		return kvVersion, fmt.Errorf("migration to KV required. Did you migrate using version v0.80.x? https://docs.lakefs.io/reference/upgrade.html#lakefs-0800-or-greater-kv-migration: %w", ErrMigrationVersion)
-	case kvVersion < ACLNoReposMigrateVersion:
-		return kvVersion, fmt.Errorf("migration to ACL required. Did you migrate using version v0.99.x? https://docs.lakefs.io/reference/access-control-list.html#migrating-from-the-previous-version-of-acls: %w", ErrMigrationVersion)
+	case kvVersion < ACLMigrateVersion,
+		kvVersion < ACLNoReposMigrateVersion:
+		return kvVersion, fmt.Errorf("migration to ACL required. Please run 'lakefs migrate up': %w", ErrMigrationRequired)
 	case kvVersion < ACLImportMigrateVersion:
 		return kvVersion, fmt.Errorf("ACL migration required. Please run 'lakefs migrate up': %w", ErrMigrationRequired)
 	}

diff --git a/pkg/kv/migrations/import_permissions.go b/pkg/kv/migrations/import_permissions.go
@@ -5,13 +5,20 @@ import (
 	"fmt"
 
 	"github.com/treeverse/lakefs/pkg/auth/model"
+	"github.com/treeverse/lakefs/pkg/config"
 	"github.com/treeverse/lakefs/pkg/kv"
 	"github.com/treeverse/lakefs/pkg/permissions"
 	"golang.org/x/exp/slices"
 	"google.golang.org/protobuf/types/known/timestamppb"
 )
 
-func MigrateImportPermissions(ctx context.Context, kvStore kv.Store) error {
+func MigrateImportPermissions(ctx context.Context, kvStore kv.Store, cfg *config.Config) error {
+	// skip migrate for users with External authorizations
+	if !cfg.IsAuthUISimplified() {
+		fmt.Println("skipping ACL migration - external Authorization")
+		return updateKVSchemaVersion(ctx, kvStore, kv.ACLImportMigrateVersion)
+	}
+
 	const action = "fs:Import*"
 	it, err := kv.NewPrimaryIterator(ctx, kvStore, (&model.PolicyData{}).ProtoReflect().Type(), model.PartitionKey, model.PolicyPath(""), kv.IteratorOptionsFrom([]byte("")))
 	if err != nil {
@@ -42,9 +49,5 @@ func MigrateImportPermissions(ctx context.Context, kvStore kv.Store) error {
 		}
 	}
 
-	err = kv.SetDBSchemaVersion(ctx, kvStore, kv.ACLImportMigrateVersion)
-	if err != nil {
-		return fmt.Errorf("migration succeeded but failed to upgrade version, to fix this re-run migration: %w", err)
-	}
-	return nil
+	return updateKVSchemaVersion(ctx, kvStore, kv.ACLImportMigrateVersion)
 }