Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add mapping table for encoding subject into UUIDs #809

Merged
merged 8 commits into from
Feb 17, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions cmd/migrate/migrate_uuid_mapping.go
Original file line number Diff line number Diff line change
@@ -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
}
1 change: 1 addition & 0 deletions cmd/migrate/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func newMigrateCmd() *cobra.Command {
newStatusCmd(),
newUpCmd(),
newDownCmd(),
newMigrateUUIDMappingCmd(),
)
return cmd
}
Expand Down
2 changes: 2 additions & 0 deletions internal/driver/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/gobuffalo/pop/v6"

"github.com/ory/keto/internal/driver/config"
"github.com/ory/keto/internal/uuidmapping"

"github.com/spf13/cobra"

Expand Down Expand Up @@ -34,6 +35,7 @@ type (
x.WriterProvider

relationtuple.ManagerProvider
uuidmapping.ManagerProvider
expand.EngineProvider
check.EngineProvider
persistence.Migrator
Expand Down
8 changes: 8 additions & 0 deletions internal/driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
grpcHealthV1 "google.golang.org/grpc/health/grpc_health_v1"

"github.com/ory/keto/internal/driver/config"
"github.com/ory/keto/internal/uuidmapping"

"github.com/ory/herodot"
"github.com/ory/x/healthx"
Expand Down Expand Up @@ -149,6 +150,13 @@ func (r *RegistryDefault) RelationTupleManager() relationtuple.Manager {
return r.p
}

func (r *RegistryDefault) UUIDMappingManager() uuidmapping.Manager {
if r.p == nil {
panic("no relation tuple manager, but expected to have one")
}
return r.p
}

func (r *RegistryDefault) Persister() persistence.Persister {
if r.p == nil {
panic("no persister, but expected to have one")
Expand Down
2 changes: 2 additions & 0 deletions internal/persistence/definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import (
"github.com/gobuffalo/pop/v6"

"github.com/ory/keto/internal/relationtuple"
"github.com/ory/keto/internal/uuidmapping"
)

type (
Persister interface {
relationtuple.Manager
uuidmapping.Manager

Connection(ctx context.Context) *pop.Connection
}
Expand Down
7 changes: 7 additions & 0 deletions internal/persistence/sql/full_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"testing"

"github.com/ory/keto/internal/uuidmapping"

"github.com/gofrs/uuid"

"github.com/ory/keto/internal/x/dbx"
Expand Down Expand Up @@ -66,6 +68,11 @@ func TestPersister(t *testing.T) {
// same registry, but different persisters only differing in the network ID
relationtuple.IsolationTest(t, p0, p1, addNamespace(r, nspaces))
})

t.Run("uuidmapping.ManagerTest", func(t *testing.T) {
p, _, _ := setup(t, dsn)
uuidmapping.ManagerTest(t, p)
})
})
}
}
2 changes: 1 addition & 1 deletion internal/persistence/sql/migrations/single_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func TestToSingleTableMigrator_HasLegacyTable(t *testing.T) {

ln, err := m.LegacyNamespaces(ctx)
require.NoError(t, err)
assert.Equal(t, nspaces, ln)
assert.ElementsMatch(t, nspaces, ln)
hperl marked this conversation as resolved.
Show resolved Hide resolved
})
})
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE keto_uuid_mappings
(
id UUID NOT NULL,
string_representation TEXT NOT NULL CHECK (string_representation <> ''),

PRIMARY KEY (id)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE keto_uuid_mappings
(
id VARCHAR(64) NOT NULL,
string_representation TEXT NOT NULL CHECK (string_representation <> ''),

PRIMARY KEY (id)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE keto_uuid_mappings
(
id UUID NOT NULL,
string_representation TEXT NOT NULL CHECK (string_representation <> ''),

PRIMARY KEY (id)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE keto_uuid_mappings
(
id UUID NOT NULL,
string_representation TEXT NOT NULL CHECK (string_representation <> ''),

PRIMARY KEY (id)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP TABLE keto_uuid_mappings;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP TABLE keto_uuid_mappings;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP TABLE keto_uuid_mappings;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP TABLE keto_uuid_mappings;
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP TABLE keto_uuid_mappings;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE keto_uuid_mappings
(
id VARCHAR(64) NOT NULL,
string_representation TEXT NOT NULL CHECK (string_representation <> ''),

PRIMARY KEY (id)
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
CREATE TABLE keto_uuid_mappings
(
id UUID NOT NULL,
string_representation TEXT NOT NULL CHECK (string_representation <> ''),

PRIMARY KEY (id)
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realized that we of course have to change the column types in the relation tuple table to UUID as well. I am afraid that we have to create a new table and insert into that instead of the old one. Or do you have other ideas?
The migrator we use can theoretically be extended to add go migrations btw, I think that would definitely make sense. Then we also don't need the command 🤔
What do you think about that @aeneasr?

https://github.com/ory/x/blob/fdc336251a395d29c7a242b611dc3fe3740620f8/popx/migration_box.go

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a new table might not be necessary. Assuming that object and subject_id already hold UUIDs, we can, in Postgres, do:

ALTER TABLE keto_relation_tuples ALTER object TYPE UUID using object::uuid;

(and similar for subject_id). MySQL does not have a UUID type, so we will stick with VARCHAR(64). The same statement works in Cockroachdb when using SET enable_experimental_alter_column_type_general = true.

111 changes: 111 additions & 0 deletions internal/persistence/sql/migrations/uuid_mapping.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package migrations

import (
"context"

"github.com/gobuffalo/pop/v6"
"github.com/gofrs/uuid"
"github.com/ory/x/sqlcon"

"github.com/ory/keto/internal/persistence/sql"
)

type (
toUUIDMappingMigrator struct {
d dependencies
perPage int
}
)

// NewToUUIDMappingMigrator creates a new UUID mapping migrator.
func NewToUUIDMappingMigrator(d dependencies) *toUUIDMappingMigrator {
return &toUUIDMappingMigrator{d: d, perPage: 100}
}

// MigrateUUIDMappings migrates to UUID-mapped subject IDs for all relation
// tuples in the database.
func (m *toUUIDMappingMigrator) MigrateUUIDMappings(ctx context.Context) error {
p, ok := m.d.Persister().(*sql.Persister)
if !ok {
panic("got unexpected persister")
}

return p.Transaction(ctx, func(ctx context.Context, c *pop.Connection) error {
for page := 1; ; page++ {
relationTuples, hasNext, err := m.getRelationTuples(ctx, page)
if err != nil {
return err
}

if err := p.Connection(ctx).All(&relationTuples); err != nil {
return err
}

for _, rt := range relationTuples {
if err := m.migrateSubjectID(ctx, rt); err != nil {
return err
}
if err := m.migrateSubjectSetObject(ctx, rt); err != nil {
return err
}
if err := m.migrateObject(ctx, rt); err != nil {
return err
}
if err := c.Update(rt); err != nil {
return err
}
}

if !hasNext {
break
}
}
return nil
})
}

func (m *toUUIDMappingMigrator) migrateSubjectID(ctx context.Context, rt *sql.RelationTuple) error {
_, err := uuid.FromString(rt.SubjectID.String)
if err == nil || !rt.SubjectID.Valid || rt.SubjectID.String == "" {
return nil
}

rt.SubjectID.String, err = m.addUUIDMapping(ctx, rt.SubjectID.String)
return err
}

func (m *toUUIDMappingMigrator) migrateSubjectSetObject(ctx context.Context, rt *sql.RelationTuple) error {
_, err := uuid.FromString(rt.SubjectSetObject.String)
if err == nil || !rt.SubjectSetObject.Valid || rt.SubjectSetObject.String == "" {
return nil
}

rt.SubjectSetObject.String, err = m.addUUIDMapping(ctx, rt.SubjectSetObject.String)
return err
}

func (m *toUUIDMappingMigrator) migrateObject(ctx context.Context, rt *sql.RelationTuple) error {
_, err := uuid.FromString(rt.Object)
if err == nil || rt.Object == "" {
return nil
}

rt.Object, err = m.addUUIDMapping(ctx, rt.Object)
return err
}

func (m *toUUIDMappingMigrator) addUUIDMapping(ctx context.Context, value string) (id string, err error) {
uid, err := m.d.Persister().ToUUID(ctx, value)
return uid.String(), err
}

func (m *toUUIDMappingMigrator) getRelationTuples(ctx context.Context, page int) (res []*sql.RelationTuple, hasNext bool, err error) {
q := m.d.Persister().Connection(ctx).
Order("nid, shard_id").
Paginate(page, m.perPage)

if err := q.All(&res); err != nil {
return nil, false, sqlcon.HandleError(err)
}
return res, q.Paginator.Page < q.Paginator.TotalPages, nil
}
Loading