Skip to content

Commit

Permalink
feat: change primary key on identity_verifiable_addresses and identit…
Browse files Browse the repository at this point in the history
…y_recovery_addresses
  • Loading branch information
alnr committed Oct 7, 2024
1 parent bafd32a commit 792cee3
Show file tree
Hide file tree
Showing 19 changed files with 343 additions and 58 deletions.
62 changes: 33 additions & 29 deletions driver/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,45 +7,39 @@ import (
"context"
"io/fs"

"github.com/ory/kratos/selfservice/sessiontokenexchange"
"github.com/ory/x/contextx"
"github.com/ory/x/jsonnetsecure"
"github.com/ory/x/otelx"
prometheus "github.com/ory/x/prometheusx"

"github.com/gorilla/sessions"
"github.com/pkg/errors"

"github.com/ory/nosurf"

"github.com/ory/x/logrusx"

"github.com/ory/kratos/cipher"
"github.com/ory/kratos/continuity"
"github.com/ory/kratos/courier"
"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/hash"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/persistence"
"github.com/ory/kratos/schema"
"github.com/ory/kratos/selfservice/errorx"
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/selfservice/flow/logout"
"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/registration"
"github.com/ory/kratos/selfservice/flow/settings"
"github.com/ory/kratos/selfservice/flow/verification"
"github.com/ory/kratos/selfservice/sessiontokenexchange"
"github.com/ory/kratos/selfservice/strategy/code"
"github.com/ory/kratos/selfservice/strategy/link"

"github.com/ory/x/healthx"

"github.com/ory/kratos/persistence"
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/selfservice/flow/logout"
"github.com/ory/kratos/selfservice/flow/registration"

"github.com/ory/kratos/x"

"github.com/ory/x/dbal"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/selfservice/errorx"
password2 "github.com/ory/kratos/selfservice/strategy/password"
"github.com/ory/kratos/session"
"github.com/ory/kratos/x"
"github.com/ory/nosurf"
"github.com/ory/x/contextx"
"github.com/ory/x/dbal"
"github.com/ory/x/healthx"
"github.com/ory/x/jsonnetsecure"
"github.com/ory/x/logrusx"
"github.com/ory/x/otelx"
"github.com/ory/x/popx"
prometheus "github.com/ory/x/prometheusx"
)

type Registry interface {
Expand Down Expand Up @@ -85,6 +79,8 @@ type Registry interface {
continuity.ManagementProvider
continuity.PersistenceProvider

cipher.Provider

courier.Provider

persistence.Provider
Expand Down Expand Up @@ -186,10 +182,12 @@ type options struct {
replaceIdentitySchemaProvider func(Registry) schema.IdentitySchemaProvider
inspect func(Registry) error
extraMigrations []fs.FS
replacementStrategies []NewStrategy
extraHooks map[string]func(config.SelfServiceHook) any
disableMigrationLogging bool
jsonnetPool jsonnetsecure.Pool
extraGoMigrations popx.Migrations

replacementStrategies []NewStrategy
extraHooks map[string]func(config.SelfServiceHook) any
disableMigrationLogging bool
jsonnetPool jsonnetsecure.Pool
}

type RegistryOption func(*options)
Expand Down Expand Up @@ -251,6 +249,12 @@ func WithExtraMigrations(m ...fs.FS) RegistryOption {
}
}

func WithExtraGoMigrations(m ...popx.Migration) RegistryOption {
return func(o *options) {
o.extraGoMigrations = append(o.extraGoMigrations, m...)

Check warning on line 254 in driver/registry.go

View check run for this annotation

Codecov / codecov/patch

driver/registry.go#L252-L254

Added lines #L252 - L254 were not covered by tests
}
}

func WithDisabledMigrationLogging() RegistryOption {
return func(o *options) {
o.disableMigrationLogging = true
Expand Down
5 changes: 4 additions & 1 deletion driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,10 @@ func (m *RegistryDefault) Init(ctx context.Context, ctxer contextx.Contextualize
m.Logger().WithError(err).Warnf("Unable to open database, retrying.")
return errors.WithStack(err)
}
p, err := sql.NewPersister(ctx, m, c, sql.WithExtraMigrations(o.extraMigrations...), sql.WithDisabledLogging(o.disableMigrationLogging))
p, err := sql.NewPersister(ctx, m, c,
sql.WithExtraMigrations(o.extraMigrations...),
sql.WithExtraGoMigrations(o.extraGoMigrations...),
sql.WithDisabledLogging(o.disableMigrationLogging))
if err != nil {
m.Logger().WithError(err).Warnf("Unable to initialize persister, retrying.")
return err
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ require (
github.com/jmoiron/sqlx v1.4.0
github.com/julienschmidt/httprouter v1.3.0
github.com/knadh/koanf/parsers/json v0.1.0
github.com/laher/mergefs v0.1.2-0.20230223191438-d16611b2f4e7
github.com/laher/mergefs v0.1.2-0.20230223191438-d16611b2f4e7 // indirect
github.com/lestrrat-go/jwx/v2 v2.1.1
github.com/luna-duclos/instrumentedsql v1.1.3
github.com/mailhog/MailHog v1.0.1
Expand Down
1 change: 0 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,6 @@ github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
github.com/markbates/pkger v0.17.1 h1:/MKEtWqtc0mZvu9OinB9UzVN9iYCwLWuyUv4Bw+PCno=
github.com/markbates/pkger v0.17.1/go.mod h1:0JoVlrol20BSywW79rN3kdFFsE5xYM+rSCQDXbLhiuI=
github.com/matryer/is v1.4.0 h1:sosSmIWwkYITGrxZ25ULNDeKiMNzFSr4V/eqBQP0PeE=
github.com/matryer/is v1.4.0/go.mod h1:8I/i5uYgLzgsgEloJE1U6xx5HkBQpAZvepWuujKwMRU=
github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
Expand Down
29 changes: 14 additions & 15 deletions persistence/sql/migratest/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,21 @@ import (
"encoding/json"
"os"
"path/filepath"
"slices"
"sync"
"testing"
"time"

"github.com/ory/x/pagination/keysetpagination"
"github.com/ory/x/servicelocatorx"

"github.com/ory/kratos/identity"

"github.com/bradleyjkemp/cupaloy/v2"
"github.com/stretchr/testify/assert"

"github.com/ory/x/dbal"

"github.com/ory/kratos/x/xsql"

"github.com/ory/x/migratest"

"github.com/gobuffalo/pop/v6"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ory/kratos/driver"
"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/identity"
"github.com/ory/kratos/persistence/sql/migrations/gomigrations"
"github.com/ory/kratos/selfservice/flow/login"
"github.com/ory/kratos/selfservice/flow/recovery"
"github.com/ory/kratos/selfservice/flow/registration"
Expand All @@ -41,9 +32,14 @@ import (
"github.com/ory/kratos/selfservice/strategy/link"
"github.com/ory/kratos/session"
"github.com/ory/kratos/x"
"github.com/ory/kratos/x/xsql"
"github.com/ory/x/configx"
"github.com/ory/x/dbal"
"github.com/ory/x/logrusx"
"github.com/ory/x/migratest"
"github.com/ory/x/pagination/keysetpagination"
"github.com/ory/x/popx"
"github.com/ory/x/servicelocatorx"
"github.com/ory/x/sqlcon"
"github.com/ory/x/sqlcon/dockertest"
)
Expand Down Expand Up @@ -87,15 +83,15 @@ func TestMigrations_Postgres(t *testing.T) {
t.Skip("skipping testing in short mode")
}
t.Parallel()
testDatabase(t, "postgres", dockertest.ConnectPop(t, dockertest.RunTestPostgreSQLWithVersion(t, "11.8")))
testDatabase(t, "postgres", dockertest.ConnectPop(t, dockertest.RunTestPostgreSQLWithVersion(t, "14")))
}

func TestMigrations_Mysql(t *testing.T) {
if testing.Short() {
t.Skip("skipping testing in short mode")
}
t.Parallel()
testDatabase(t, "mysql", dockertest.ConnectPop(t, dockertest.RunTestMySQLWithVersion(t, "8.0.34")))
testDatabase(t, "mysql", dockertest.ConnectPop(t, dockertest.RunTestMySQLWithVersion(t, "8.0")))
}

func TestMigrations_Cockroach(t *testing.T) {
Expand Down Expand Up @@ -134,6 +130,9 @@ func testDatabase(t *testing.T, db string, c *pop.Connection) {
os.DirFS("../migrations/sql"),
popx.NewMigrator(c, l, nil, 1*time.Minute),
popx.WithTestdata(t, os.DirFS("./testdata")),
popx.WithGoMigrations(slices.Concat(
gomigrations.ChangeAddressesPK,
)),
)
require.NoError(t, err)
tm.DumpMigrations = true
Expand Down
71 changes: 71 additions & 0 deletions persistence/sql/migrations/gomigrations/identity_pk.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// Copyright © 2024 Ory Corp
// SPDX-License-Identifier: Apache-2.0

package gomigrations

import (
"fmt"
"path/filepath"
"runtime"

"github.com/gobuffalo/pop/v6"
"github.com/pkg/errors"

"github.com/ory/x/popx"
)

func path() string {
_, file, line, _ := runtime.Caller(1)
return fmt.Sprintf("%s:%d", filepath.Base(file), line)
}

var ChangeAddressesPK = []popx.Migration{
{
Version: "20241001000000000000",
Path: path(),
Name: "Change primary key for identity_verifiable_addresses",
Direction: "up",
Type: "go",
DBType: "cockroach",
RunnerNoTx: func(m popx.Migration, c *pop.Connection) error {
_, err := c.Store.Exec("ALTER TABLE identity_verifiable_addresses ALTER PRIMARY KEY USING COLUMNS (identity_id,id)")
return errors.WithStack(err)
},
},
{
Version: "20241001000000000000",
Path: path(),
Name: "Revert primary key for identity_verifiable_addresses",
Direction: "down",
Type: "go",
DBType: "cockroach",
RunnerNoTx: func(m popx.Migration, c *pop.Connection) error {
_, err := c.Store.Exec("ALTER TABLE identity_verifiable_addresses ALTER PRIMARY KEY USING COLUMNS (id)")
return errors.WithStack(err)
},
},
{
Version: "20241001000000000001",
Path: path(),
Name: "Change primary key for identity_recovery_addresses",
Direction: "up",
Type: "go",
DBType: "cockroach",
RunnerNoTx: func(m popx.Migration, c *pop.Connection) error {
_, err := c.Store.Exec("ALTER TABLE identity_recovery_addresses ALTER PRIMARY KEY USING COLUMNS (identity_id,id)")
return errors.WithStack(err)
},
},
{
Version: "20241001000000000001",
Path: path(),
Name: "Revert primary key for identity_recovery_addresses",
Direction: "down",
Type: "go",
DBType: "cockroach",
RunnerNoTx: func(m popx.Migration, c *pop.Connection) error {
_, err := c.Store.Exec("ALTER TABLE identity_recovery_addresses ALTER PRIMARY KEY USING COLUMNS (id)")
return errors.WithStack(err)
},
},
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
ALTER TABLE identity_verifiable_addresses
DROP FOREIGN KEY identity_verifiable_addresses_ibfk_1;

ALTER TABLE identity_verifiable_addresses
DROP PRIMARY KEY,
ADD PRIMARY KEY (id),
ADD CONSTRAINT identity_verifiable_addresses_ibfk_1 FOREIGN KEY (identity_id) REFERENCES identities(id) ON DELETE CASCADE;
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
ALTER TABLE identity_verifiable_addresses
DROP PRIMARY KEY,
ADD PRIMARY KEY (identity_id, id),
ADD UNIQUE KEY identity_verifiable_addresses_id_uq_idx (id);
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ALTER TABLE identity_verifiable_addresses
DROP CONSTRAINT identity_verifiable_addresses_pkey,
ADD PRIMARY KEY (id);
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
CREATE UNIQUE INDEX identity_verifiable_addresses_id_uq_idx ON identity_verifiable_addresses (id);

ALTER TABLE identity_verification_codes
DROP CONSTRAINT identity_verification_codes_identity_verifiable_addresses_id_fk;

ALTER TABLE identity_verification_tokens
DROP CONSTRAINT identity_verification_tokens_identity_verifiable_address_i_fkey;

ALTER TABLE identity_verifiable_addresses
DROP CONSTRAINT identity_verifiable_addresses_pkey,
ADD PRIMARY KEY (identity_id, id);

ALTER TABLE identity_verification_codes
ADD CONSTRAINT identity_verification_codes_identity_verifiable_addresses_id_fk FOREIGN KEY (identity_verifiable_address_id) REFERENCES identity_verifiable_addresses(id) ON DELETE CASCADE;

ALTER TABLE identity_verification_tokens
ADD CONSTRAINT identity_verification_tokens_identity_verifiable_address_i_fkey FOREIGN KEY (identity_verifiable_address_id) REFERENCES identity_verifiable_addresses(id) ON DELETE CASCADE;
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
CREATE TABLE IF NOT EXISTS "_identity_verifiable_addresses_tmp" (
"id" TEXT PRIMARY KEY,
"status" TEXT NOT NULL,
"via" TEXT NOT NULL,
"verified" bool NOT NULL,
"value" TEXT NOT NULL,
"verified_at" DATETIME,
"identity_id" TEXT NOT NULL,
"created_at" DATETIME NOT NULL,
"updated_at" DATETIME NOT NULL,
"nid" TEXT NOT NULL,
FOREIGN KEY ("identity_id") REFERENCES "identities" ("id") ON UPDATE RESTRICT ON DELETE CASCADE,
FOREIGN KEY ("nid") REFERENCES "networks" ("id") ON UPDATE RESTRICT ON DELETE CASCADE
);

INSERT INTO "_identity_verifiable_addresses_tmp"
("id", "status", "via", "verified", "value", "verified_at", "identity_id", "created_at", "updated_at", "nid")
SELECT
"id", "status", "via", "verified", "value", "verified_at", "identity_id", "created_at", "updated_at", "nid"
FROM "identity_verifiable_addresses";

DROP TABLE "identity_verifiable_addresses";
ALTER TABLE "_identity_verifiable_addresses_tmp" RENAME TO "identity_verifiable_addresses";

CREATE UNIQUE INDEX IF NOT EXISTS "identity_verifiable_addresses_status_via_uq_idx" ON "identity_verifiable_addresses" (nid, via, value);
CREATE INDEX IF NOT EXISTS "identity_verifiable_addresses_status_via_idx" ON "identity_verifiable_addresses" (nid, via, value);
CREATE INDEX IF NOT EXISTS identity_recovery_addresses_nid_id_idx ON identity_recovery_addresses (nid, id);
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
CREATE TABLE IF NOT EXISTS "_identity_verifiable_addresses_tmp" (
"id" TEXT NOT NULL,
"status" TEXT NOT NULL,
"via" TEXT NOT NULL,
"verified" bool NOT NULL,
"value" TEXT NOT NULL,
"verified_at" DATETIME,
"identity_id" TEXT NOT NULL,
"created_at" DATETIME NOT NULL,
"updated_at" DATETIME NOT NULL,
"nid" TEXT NOT NULL,
PRIMARY KEY ("identity_id","id"),
FOREIGN KEY ("identity_id") REFERENCES "identities" ("id") ON UPDATE RESTRICT ON DELETE CASCADE,
FOREIGN KEY ("nid") REFERENCES "networks" ("id") ON UPDATE RESTRICT ON DELETE CASCADE
);

INSERT INTO "_identity_verifiable_addresses_tmp"
("id", "status", "via", "verified", "value", "verified_at", "identity_id", "created_at", "updated_at", "nid")
SELECT
"id", "status", "via", "verified", "value", "verified_at", "identity_id", "created_at", "updated_at", "nid"
FROM "identity_verifiable_addresses";

DROP TABLE "identity_verifiable_addresses";
ALTER TABLE "_identity_verifiable_addresses_tmp" RENAME TO "identity_verifiable_addresses";

CREATE UNIQUE INDEX "identity_verifiable_addresses_status_via_uq_idx" ON "identity_verifiable_addresses" (nid, via, value);
CREATE UNIQUE INDEX "identity_verifiable_addresses_id_uq_idx" ON "identity_verifiable_addresses" (id);
CREATE INDEX "identity_verifiable_addresses_status_via_idx" ON "identity_verifiable_addresses" (nid, via, value);
CREATE INDEX identity_verifiable_addresses_nid_id_idx ON identity_recovery_addresses (nid, id);
CREATE INDEX identity_verifiable_addresses_id_nid_idx ON identity_recovery_addresses (id, nid);
Loading

0 comments on commit 792cee3

Please sign in to comment.