From 4dcdfda8ba966bc175945d555933b1f8101f8717 Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Wed, 2 Feb 2022 19:25:01 +0100 Subject: [PATCH] feat: map subjects/objects using UUIDv5 --- cmd/migrate/migrate_uuid_mapping.go | 49 +++++++++++++++++++ cmd/migrate/root.go | 1 + ...create-uuid-mapping-table.cockroach.up.sql | 9 ++-- ...000_create-uuid-mapping-table.mysql.up.sql | 9 ++-- ..._create-uuid-mapping-table.postgres.up.sql | 9 ++-- ...0_create-uuid-mapping-table.sqlite3.up.sql | 9 ++-- ...create-uuid-mapping-table.cockroach.up.sql | 2 - ...001_create-uuid-mapping-table.mysql.up.sql | 2 - ..._create-uuid-mapping-table.postgres.up.sql | 2 - ...1_create-uuid-mapping-table.sqlite3.up.sql | 2 - ...400_create-uuid-mapping-table.mysql.up.sql | 13 ++--- ...110200400_create-uuid-mapping-table.up.sql | 13 ++--- .../sql/migrations/uuid_mapping.go | 3 +- internal/persistence/sql/uuid_mapping.go | 34 ++++++------- internal/uuidmapping/definitions.go | 33 ++++++------- 15 files changed, 100 insertions(+), 90 deletions(-) create mode 100644 cmd/migrate/migrate_uuid_mapping.go diff --git a/cmd/migrate/migrate_uuid_mapping.go b/cmd/migrate/migrate_uuid_mapping.go new file mode 100644 index 000000000..8c15c60c4 --- /dev/null +++ b/cmd/migrate/migrate_uuid_mapping.go @@ -0,0 +1,49 @@ +package migrate + +import ( + "fmt" + + "github.com/ory/x/cmdx" + "github.com/ory/x/flagx" + "github.com/pkg/errors" + "github.com/spf13/cobra" + + "github.com/ory/keto/internal/driver" + "github.com/ory/keto/internal/persistence" + "github.com/ory/keto/internal/persistence/sql/migrations" +) + +func newMigrateUUIDMappingCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "uuid-mapping", + Short: "Migrate the non-UUID subject and object names to UUIDs.", + Long: `Migrate the non-UUID subject and object names to UUIDs. +This step only has to be executed once. +Please ensure that you have a backup in case something goes wrong!"`, + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + reg, err := driver.NewDefaultRegistry(cmd.Context(), cmd.Flags(), false) + if errors.Is(err, persistence.ErrNetworkMigrationsMissing) { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), + "Migrations were not applied yet, please apply them first using `keto migrate up`.") + return cmdx.FailSilently(cmd) + } else if err != nil { + return err + } + + if !flagx.MustGetBool(cmd, FlagYes) && + !cmdx.AskForConfirmation( + "Are you sure you want to migrate the subject and object names to UUIDs?", + cmd.InOrStdin(), cmd.OutOrStdout()) { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "OK, aborting.") + return nil + } + + migrator := migrations.NewToUUIDMappingMigrator(reg) + return migrator.MigrateUUIDMappings(cmd.Context()) + }, + } + RegisterYesFlag(cmd.Flags()) + + return cmd +} diff --git a/cmd/migrate/root.go b/cmd/migrate/root.go index fc92a1e7c..a4388b6f1 100644 --- a/cmd/migrate/root.go +++ b/cmd/migrate/root.go @@ -13,6 +13,7 @@ func newMigrateCmd() *cobra.Command { newStatusCmd(), newUpCmd(), newDownCmd(), + newMigrateUUIDMappingCmd(), ) return cmd } diff --git a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.cockroach.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.cockroach.up.sql index 72e1a5f85..9aad41258 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.cockroach.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.cockroach.up.sql @@ -1,10 +1,7 @@ CREATE TABLE keto_uuid_mappings ( - id UUID NOT NULL, - string_representation VARCHAR(255) NOT NULL CHECK (string_representation <> ''), + id UUID NOT NULL, + string_representation TEXT NOT NULL CHECK (string_representation <> ''), - PRIMARY KEY (id), - - -- enforce uniqueness - CONSTRAINT chk_keto_uuid_map_uniq UNIQUE (id, string_representation) + PRIMARY KEY (id) ); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.mysql.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.mysql.up.sql index 1a1055c47..5db883292 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.mysql.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.mysql.up.sql @@ -1,10 +1,7 @@ CREATE TABLE keto_uuid_mappings ( - id VARCHAR(64) NOT NULL, - string_representation VARCHAR(255) NOT NULL CHECK (string_representation <> ''), + id VARCHAR(64) NOT NULL, + string_representation TEXT NOT NULL CHECK (string_representation <> ''), - PRIMARY KEY (id), - - -- enforce uniqueness - CONSTRAINT chk_keto_uuid_map_uniq UNIQUE (id, string_representation) + PRIMARY KEY (id) ); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.postgres.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.postgres.up.sql index 72e1a5f85..9aad41258 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.postgres.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.postgres.up.sql @@ -1,10 +1,7 @@ CREATE TABLE keto_uuid_mappings ( - id UUID NOT NULL, - string_representation VARCHAR(255) NOT NULL CHECK (string_representation <> ''), + id UUID NOT NULL, + string_representation TEXT NOT NULL CHECK (string_representation <> ''), - PRIMARY KEY (id), - - -- enforce uniqueness - CONSTRAINT chk_keto_uuid_map_uniq UNIQUE (id, string_representation) + PRIMARY KEY (id) ); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.sqlite3.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.sqlite3.up.sql index 72e1a5f85..9aad41258 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.sqlite3.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000000_create-uuid-mapping-table.sqlite3.up.sql @@ -1,10 +1,7 @@ CREATE TABLE keto_uuid_mappings ( - id UUID NOT NULL, - string_representation VARCHAR(255) NOT NULL CHECK (string_representation <> ''), + id UUID NOT NULL, + string_representation TEXT NOT NULL CHECK (string_representation <> ''), - PRIMARY KEY (id), - - -- enforce uniqueness - CONSTRAINT chk_keto_uuid_map_uniq UNIQUE (id, string_representation) + PRIMARY KEY (id) ); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.cockroach.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.cockroach.up.sql index 41af24927..e69de29bb 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.cockroach.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.cockroach.up.sql @@ -1,2 +0,0 @@ - -CREATE INDEX keto_uuid_mappings_subject_ids_idx ON keto_uuid_mappings (string_representation); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.mysql.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.mysql.up.sql index 41af24927..e69de29bb 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.mysql.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.mysql.up.sql @@ -1,2 +0,0 @@ - -CREATE INDEX keto_uuid_mappings_subject_ids_idx ON keto_uuid_mappings (string_representation); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.postgres.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.postgres.up.sql index 41af24927..e69de29bb 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.postgres.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.postgres.up.sql @@ -1,2 +0,0 @@ - -CREATE INDEX keto_uuid_mappings_subject_ids_idx ON keto_uuid_mappings (string_representation); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.sqlite3.up.sql b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.sqlite3.up.sql index 41af24927..e69de29bb 100644 --- a/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.sqlite3.up.sql +++ b/internal/persistence/sql/migrations/sql/20220110200400000001_create-uuid-mapping-table.sqlite3.up.sql @@ -1,2 +0,0 @@ - -CREATE INDEX keto_uuid_mappings_subject_ids_idx ON keto_uuid_mappings (string_representation); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.mysql.up.sql b/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.mysql.up.sql index cf8fb4f82..5db883292 100644 --- a/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.mysql.up.sql +++ b/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.mysql.up.sql @@ -1,12 +1,7 @@ CREATE TABLE keto_uuid_mappings ( - id VARCHAR(64) NOT NULL, - string_representation VARCHAR(255) NOT NULL CHECK (string_representation <> ''), + id VARCHAR(64) NOT NULL, + string_representation TEXT NOT NULL CHECK (string_representation <> ''), - PRIMARY KEY (id), - - -- enforce uniqueness - CONSTRAINT chk_keto_uuid_map_uniq UNIQUE (id, string_representation) -); - -CREATE INDEX keto_uuid_mappings_subject_ids_idx ON keto_uuid_mappings (string_representation); \ No newline at end of file + PRIMARY KEY (id) +); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.up.sql b/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.up.sql index 0304e7729..9aad41258 100644 --- a/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.up.sql +++ b/internal/persistence/sql/migrations/templates/20220110200400_create-uuid-mapping-table.up.sql @@ -1,12 +1,7 @@ CREATE TABLE keto_uuid_mappings ( - id UUID NOT NULL, - string_representation VARCHAR(255) NOT NULL CHECK (string_representation <> ''), + id UUID NOT NULL, + string_representation TEXT NOT NULL CHECK (string_representation <> ''), - PRIMARY KEY (id), - - -- enforce uniqueness - CONSTRAINT chk_keto_uuid_map_uniq UNIQUE (id, string_representation) -); - -CREATE INDEX keto_uuid_mappings_subject_ids_idx ON keto_uuid_mappings (string_representation); \ No newline at end of file + PRIMARY KEY (id) +); \ No newline at end of file diff --git a/internal/persistence/sql/migrations/uuid_mapping.go b/internal/persistence/sql/migrations/uuid_mapping.go index aba324de3..0b2e7b28b 100644 --- a/internal/persistence/sql/migrations/uuid_mapping.go +++ b/internal/persistence/sql/migrations/uuid_mapping.go @@ -5,7 +5,6 @@ import ( "github.com/gobuffalo/pop/v6" "github.com/gofrs/uuid" - "github.com/ory/x/sqlcon" "github.com/ory/keto/internal/persistence/sql" @@ -96,7 +95,7 @@ func (m *toUUIDMappingMigrator) migrateObject(ctx context.Context, rt *sql.Relat } func (m *toUUIDMappingMigrator) addUUIDMapping(ctx context.Context, value string) (id string, err error) { - uid, err := m.d.Persister().MappedUUID(ctx, value) + uid, err := m.d.Persister().ToUUID(ctx, value) return uid.String(), err } diff --git a/internal/persistence/sql/uuid_mapping.go b/internal/persistence/sql/uuid_mapping.go index f4b695ae7..5758a7dcc 100644 --- a/internal/persistence/sql/uuid_mapping.go +++ b/internal/persistence/sql/uuid_mapping.go @@ -24,17 +24,26 @@ func (UUIDMapping) TableName() string { return "keto_uuid_mappings" } -func (p *Persister) AddUUIDMapping(ctx context.Context, id uuid.UUID, representation string) error { +func (p *Persister) ToUUID(ctx context.Context, text string) (uuid.UUID, error) { + id := uuid.NewV5(p.NetworkID(ctx), text) + p.d.Logger().Trace("adding UUID mapping") + m := &UUIDMapping{ ID: id, - StringRepresentation: representation, + StringRepresentation: text, + } + + err := sqlcon.HandleError(p.Connection(ctx).Create(m)) + + // Ignore errors if the UUID already exists. + if errors.Is(err, sqlcon.ErrUniqueViolation) { + return id, nil } - p.d.Logger().Trace("adding UUID mapping") - return sqlcon.HandleError(p.Connection(ctx).Create(m)) + return id, err } -func (p *Persister) LookupUUID(ctx context.Context, id uuid.UUID) (rep string, err error) { +func (p *Persister) FromUUID(ctx context.Context, id uuid.UUID) (rep string, err error) { p.d.Logger().Trace("looking up UUID") m := &UUIDMapping{} @@ -44,18 +53,3 @@ func (p *Persister) LookupUUID(ctx context.Context, id uuid.UUID) (rep string, e return m.StringRepresentation, nil } - -func (p *Persister) MappedUUID(ctx context.Context, representation string) (uuid.UUID, error) { - p.d.Logger().Trace("looking up mapped UUID") - - m := &UUIDMapping{} - if err := sqlcon.HandleError(p.Connection(ctx).Where("string_representation = ?", representation).First(m)); err != nil { - if errors.Is(err, sqlcon.ErrNoRows) { - id := uuid.Must(uuid.NewV4()) - return id, p.AddUUIDMapping(ctx, id, representation) - } - return uuid.Nil, err - } - - return m.ID, nil -} diff --git a/internal/uuidmapping/definitions.go b/internal/uuidmapping/definitions.go index ea96018ce..9ddeac751 100644 --- a/internal/uuidmapping/definitions.go +++ b/internal/uuidmapping/definitions.go @@ -15,37 +15,34 @@ type ( UUIDMappingManager() Manager } Manager interface { - // MappedUUID returns the mapped UUID for the given string - // representation. If the string representation is not mapped, a new - // UUID will be created automatically. - MappedUUID(ctx context.Context, representation string) (uuid.UUID, error) + // ToUUID returns the mapped UUID for the given string representation. + // If the string representation is not mapped, a new UUID will be + // created automatically. + ToUUID(ctx context.Context, representation string) (uuid.UUID, error) - // AddUUIDMapping adds a new mapping between a UUID and a string. - AddUUIDMapping(ctx context.Context, id uuid.UUID, representation string) error - - // LookupUUID returns the string representation for the given UUID. - LookupUUID(ctx context.Context, id uuid.UUID) (rep string, err error) + // FromUUID returns the text representation for the given UUID. + FromUUID(ctx context.Context, id uuid.UUID) (text string, err error) } ) func ManagerTest(t *testing.T, m Manager) { ctx := context.Background() - t.Run("case=add_lookup", func(t *testing.T) { - id := uuid.Must(uuid.NewV4()) + t.Run("case=ToUUID_FromUUID", func(t *testing.T) { rep1 := "foo" - require.NoError(t, m.AddUUIDMapping(ctx, id, rep1)) + id, err := m.ToUUID(ctx, rep1) + require.NoError(t, err) - rep2, err := m.LookupUUID(ctx, id) + rep2, err := m.FromUUID(ctx, id) assert.NoError(t, err) assert.Equal(t, rep1, rep2) }) - t.Run("case=MappedUUID", func(t *testing.T) { - id1, err := m.MappedUUID(ctx, "string") - require.NoError(t, err) - id2, err := m.MappedUUID(ctx, "string") - require.NoError(t, err) + t.Run("case=FromUUID", func(t *testing.T) { + id1, err := m.ToUUID(ctx, "string") + assert.NoError(t, err) + id2, err := m.ToUUID(ctx, "string") + assert.NoError(t, err) assert.Equal(t, id1, id2) }) }