diff --git a/cmd/cli/handler_migrate.go b/cmd/cli/handler_migrate.go index a4c2dd4885d..38701eda28e 100644 --- a/cmd/cli/handler_migrate.go +++ b/cmd/cli/handler_migrate.go @@ -5,7 +5,6 @@ package cli import ( "bytes" - "context" "fmt" "io" "io/fs" @@ -317,12 +316,12 @@ func (h *MigrateHandler) makePersister(cmd *cobra.Command, args []string) (p per return d.Persister(), nil } -func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) (err error) { +func (h *MigrateHandler) MigrateSQLUp(cmd *cobra.Command, args []string) (err error) { p, err := h.makePersister(cmd, args) if err != nil { return err } - conn := p.Connection(context.Background()) + conn := p.Connection(cmd.Context()) if conn == nil { _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Migrations can only be executed against a SQL-compatible driver but DSN is not a SQL source.") return cmdx.FailSilently(cmd) @@ -334,7 +333,7 @@ func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) (err erro } // convert migration tables - if err := p.PrepareMigration(context.Background()); err != nil { + if err := p.PrepareMigration(cmd.Context()); err != nil { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not convert the migration table:\n%+v\n", err) return cmdx.FailSilently(cmd) } @@ -342,13 +341,118 @@ func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) (err erro // print migration status _, _ = fmt.Fprintln(cmd.OutOrStdout(), "The following migration is planned:") - status, err := p.MigrationStatus(context.Background()) + // print migration status + status, err := p.MigrationStatus(cmd.Context()) + if err != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not get the migration status:\n%+v\n", errorsx.WithStack(err)) + return cmdx.FailSilently(cmd) + } + _ = status.Write(os.Stdout) + + if !flagx.MustGetBool(cmd, "yes") { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "To skip the next question use flag --yes (at your own risk).") + if !cmdx.AskForConfirmation("Do you wish to execute this migration plan?", nil, nil) { + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Migration aborted.") + return nil + } + } + + // apply migrations + if err := p.MigrateUp(cmd.Context()); err != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not apply migrations:\n%+v\n", errorsx.WithStack(err)) + return cmdx.FailSilently(cmd) + } + + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "Successfully applied migrations!") + return nil +} + +func (h *MigrateHandler) MigrateSQLDown(cmd *cobra.Command, args []string) (err error) { + p, err := h.makePersister(cmd, args) + if err != nil { + return err + } + + steps := flagx.MustGetInt(cmd, "steps") + if steps < 0 { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Flag --steps must be a positive integer.") + return cmdx.FailSilently(cmd) + } + + versions := flagx.MustGetStringSlice(cmd, "version") + if len(versions) > 0 && steps > 0 { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Flags --steps and --version are mutually exclusive.") + return cmdx.FailSilently(cmd) + } + + conn := p.Connection(cmd.Context()) + if conn == nil { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Migrations can only be executed against a SQL-compatible driver but DSN is not a SQL source.") + return cmdx.FailSilently(cmd) + } + + if err := conn.Open(); err != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not open the database connection:\n%+v\n", err) + return cmdx.FailSilently(cmd) + } + + // convert migration tables + if err := p.PrepareMigration(cmd.Context()); err != nil { + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not convert the migration table:\n%+v\n", err) + return cmdx.FailSilently(cmd) + } + + status, err := p.MigrationStatus(cmd.Context()) if err != nil { - fmt.Fprintf(cmd.ErrOrStderr(), "Could not get the migration status:\n%+v\n", errorsx.WithStack(err)) + _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not get the migration status:\n%+v\n", errorsx.WithStack(err)) return cmdx.FailSilently(cmd) } + + // Now we need to rollback the last `steps` migrations that have a status of "Applied": + var count int + var rollingBack int + var contents []string + for i := len(status) - 1; i >= 0; i-- { + if status[i].State == popx.Applied { + count++ + if steps > 0 && count <= steps { + status[i].State = "Rollback" + rollingBack++ + contents = append(contents, status[i].Content) + } + if len(versions) > 0 { + for _, v := range versions { + if status[i].Version == v { + status[i].State = "Rollback" + rollingBack++ + contents = append(contents, status[i].Content) + } + } + } + } + } + + // print migration status + _, _ = fmt.Fprintln(cmd.OutOrStdout(), "The migration plan is as follows:") _ = status.Write(os.Stdout) + if rollingBack < 1 { + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "") + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "There are apparently no migrations to roll back.") + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Please provide the --steps argument with a value larger than 0.") + _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "") + return cmdx.FailSilently(cmd) + } + + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "\nThe SQL statements to be executed from top to bottom are:\n\n") + + for i := len(status) - 1; i >= 0; i-- { + if status[i].State == "Rollback" { + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "------------ %s - %s ------------\n", status[i].Version, status[i].Name) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n\n", status[i].Content) + } + } + if !flagx.MustGetBool(cmd, "yes") { _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "To skip the next question use flag --yes (at your own risk).") if !cmdx.AskForConfirmation("Do you wish to execute this migration plan?", nil, nil) { @@ -358,7 +462,7 @@ func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) (err erro } // apply migrations - if err := p.MigrateUp(context.Background()); err != nil { + if err := p.MigrateDown(cmd.Context(), steps); err != nil { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not apply migrations:\n%+v\n", errorsx.WithStack(err)) return cmdx.FailSilently(cmd) } @@ -372,7 +476,7 @@ func (h *MigrateHandler) MigrateStatus(cmd *cobra.Command, args []string) error if err != nil { return err } - conn := p.Connection(context.Background()) + conn := p.Connection(cmd.Context()) if conn == nil { _, _ = fmt.Fprintln(cmd.ErrOrStderr(), "Migrations can only be checked against a SQL-compatible driver but DSN is not a SQL source.") return cmdx.FailSilently(cmd) @@ -408,5 +512,4 @@ func (h *MigrateHandler) MigrateStatus(cmd *cobra.Command, args []string) error cmdx.PrintTable(cmd, s) return nil - } diff --git a/cmd/migrate_sql.go b/cmd/migrate_sql.go index 1ab3bde3717..d78eee4e4fa 100644 --- a/cmd/migrate_sql.go +++ b/cmd/migrate_sql.go @@ -15,8 +15,9 @@ import ( func NewMigrateSqlCmd(slOpts []servicelocatorx.Option, dOpts []driver.OptionsModifier, cOpts []configx.OptionModifier) *cobra.Command { cmd := &cobra.Command{ - Use: "sql ", - Short: "Create SQL schemas and apply migration plans", + Use: "sql ", + Deprecated: "Please use `migrate sql up` instead.", + Short: "Perform SQL migrations", Long: `Run this command on a fresh SQL installation and when you upgrade Hydra to a new minor version. For example, upgrading Hydra 0.7.0 to 0.8.0 requires running this command. @@ -30,11 +31,13 @@ You can read in the database URL using the -e flag, for example: ### WARNING ### Before running this command on an existing database, create a back up!`, - RunE: cli.NewHandler(slOpts, dOpts, cOpts).Migration.MigrateSQL, + RunE: cli.NewHandler(slOpts, dOpts, cOpts).Migration.MigrateSQLUp, } cmd.Flags().BoolP("read-from-env", "e", false, "If set, reads the database connection string from the environment variable DSN or config file key dsn.") cmd.Flags().BoolP("yes", "y", false, "If set all confirmation requests are accepted without user interaction.") + cmd.AddCommand(NewMigrateSqlDownCmd(slOpts, dOpts, cOpts)) + cmd.AddCommand(NewMigrateSqlUpCmd(slOpts, dOpts, cOpts)) return cmd } diff --git a/cmd/migrate_sql_down.go b/cmd/migrate_sql_down.go new file mode 100644 index 00000000000..9e81d44284a --- /dev/null +++ b/cmd/migrate_sql_down.go @@ -0,0 +1,59 @@ +// Copyright © 2022 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "github.com/spf13/cobra" + + "github.com/ory/hydra/v2/driver" + "github.com/ory/x/configx" + "github.com/ory/x/servicelocatorx" + + "github.com/ory/hydra/v2/cmd/cli" +) + +func NewMigrateSqlDownCmd(slOpts []servicelocatorx.Option, dOpts []driver.OptionsModifier, cOpts []configx.OptionModifier) *cobra.Command { + cmd := &cobra.Command{ + Use: "down ", + Short: "Roll back SQL migrations", + Args: cobra.RangeArgs(0, 1), + Example: `Revert the most recent migration: + DSN=... hydra migrate sql down -e --steps 1 + +Review migrations to decide which one to roll back: + DSN=... hydra migrate sql down -e --steps 0 + +Revert a specific migration: + DSN=... hydra migrate sql down -e --version 20230606112801000001 +`, + Long: `Run this command to roll back SQL migrations. This command is useful when you want to revert to a previous version of Ory Hydra. + +:::warning + +Before running this command on an existing database, create a back up. This command can be destructive as it may drop +indices, columns, or whole tables. Run this command close to the SQL instance (same VPC / same machine). + +::: + +This command will not execute anything unless you provide a --steps flag with a value greater than 0. Per default, this +command will roll back one migration at a time. You can specify the number of migrations to roll back using the --steps +flag. + +Choosing how many migrations to roll back depends on the current state of the database. Please first execute the command +without the --steps flag to review the migrations and decide which one to roll back. + +Once you have decided which migration to roll back, you can use the --steps flag to specify the number of migrations to +roll back. For example, to roll back the most recent migration, you can run: + + DSN=... hydra migrate sql down -e --steps 1`, + RunE: cli.NewHandler(slOpts, dOpts, cOpts).Migration.MigrateSQLDown, + } + + cmd.Flags().BoolP("read-from-env", "e", false, "If set, reads the database connection string from the environment variable DSN or config file key dsn.") + cmd.Flags().BoolP("yes", "y", false, "If set all confirmation requests are accepted without user interaction.") + cmd.Flags().Int("steps", 0, "The number of migrations to roll back.") + cmd.Flags().StringSlice("version", []string{}, "One or more migration versions.") + + return cmd +} diff --git a/cmd/migrate_sql_up.go b/cmd/migrate_sql_up.go new file mode 100644 index 00000000000..6a77a56c1bf --- /dev/null +++ b/cmd/migrate_sql_up.go @@ -0,0 +1,41 @@ +// Copyright © 2022 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package cmd + +import ( + "github.com/spf13/cobra" + + "github.com/ory/hydra/v2/driver" + "github.com/ory/x/configx" + "github.com/ory/x/servicelocatorx" + + "github.com/ory/hydra/v2/cmd/cli" +) + +func NewMigrateSqlUpCmd(slOpts []servicelocatorx.Option, dOpts []driver.OptionsModifier, cOpts []configx.OptionModifier) *cobra.Command { + cmd := &cobra.Command{ + Use: "up ", + Args: cobra.RangeArgs(0, 1), + Short: "Create and upgrade the Ory Hydra SQL schema", + Long: `Run this command on a fresh SQL installation and when you upgrade Ory Hydra to a newer version. + +:::warning + +Before running this command on an existing database, create a back up. This command can be destructive as it may drop +indices, columns, or whole tables. Run this command close to the SQL instance (same VPC / same machine). + +::: + +It is recommended to review the migrations before running them. You can do this by running the command without the --yes +flag: + + DSN=... hydra migrate sql up -e`, + RunE: cli.NewHandler(slOpts, dOpts, cOpts).Migration.MigrateSQLUp, + } + + cmd.Flags().BoolP("read-from-env", "e", false, "If set, reads the database connection string from the environment variable DSN or config file key dsn.") + cmd.Flags().BoolP("yes", "y", false, "If set all confirmation requests are accepted without user interaction.") + + return cmd +} diff --git a/cmd/root.go b/cmd/root.go index 6feabdb8103..d32bf78099a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -7,6 +7,8 @@ import ( "fmt" "os" + "github.com/pkg/errors" + "github.com/ory/x/cmdx" "github.com/ory/hydra/v2/driver" @@ -99,8 +101,11 @@ func RegisterCommandRecursive(parent *cobra.Command, slOpts []servicelocatorx.Op // Execute adds all child commands to the root command sets flags appropriately. func Execute() { - if err := NewRootCmd(nil, nil, nil).Execute(); err != nil { - fmt.Println(err) - os.Exit(-1) + c := NewRootCmd(nil, nil, nil) + if err := c.Execute(); err != nil { + if !errors.Is(err, cmdx.ErrNoPrintButFail) { + _, _ = fmt.Fprintln(c.ErrOrStderr(), err) + } + os.Exit(1) } } diff --git a/driver/registry_sql.go b/driver/registry_sql.go index 76159e4c805..c29573e81da 100644 --- a/driver/registry_sql.go +++ b/driver/registry_sql.go @@ -250,6 +250,9 @@ func (m *RegistrySQL) alwaysCanHandle(dsn string) bool { func (m *RegistrySQL) Ping() error { return m.Persister().Ping() } +func (m *RegistrySQL) PingContext(ctx context.Context) error { + return m.Persister().PingContext(ctx) +} func (m *RegistrySQL) ClientManager() client.Manager { return m.Persister() diff --git a/go.mod b/go.mod index 26adbfc96ce..cabeade5360 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,8 @@ replace github.com/gobuffalo/pop/v6 => github.com/ory/pop/v6 v6.2.0 // may be breaking for some users. replace github.com/ory/fosite => github.com/ory/fosite v0.47.1-0.20241101073333-eab241e153a4 +replace github.com/ory/x => ../x + require ( github.com/ThalesIgnite/crypto11 v1.2.5 github.com/bradleyjkemp/cupaloy/v2 v2.8.0 diff --git a/go.sum b/go.sum index 5f7a2ba7603..298bfdd3eef 100644 --- a/go.sum +++ b/go.sum @@ -394,8 +394,6 @@ github.com/ory/kratos-client-go v1.2.1 h1:Q3T/adfAfAkHFcV1LGLnwz4QkY6ghBdX9zde5T github.com/ory/kratos-client-go v1.2.1/go.mod h1:WiQYlrqW4Atj6Js7oDN5ArbZxo0nTO2u/e1XaDv2yMI= github.com/ory/pop/v6 v6.2.0 h1:hRFOGAOEHw91kUHQ32k5NHqCkcHrRou/romvrJP1w0E= github.com/ory/pop/v6 v6.2.0/go.mod h1:okVAYKGtgunD/wbW3NGhZTndJCS+6FqO+cA89rQ4doc= -github.com/ory/x v0.0.668 h1:HfJgq+vRwC6ptzc3+Y1VFpo9zc8eXHEtX24qxAPqr5s= -github.com/ory/x v0.0.668/go.mod h1:0Av1u/Gh7WXCrEDJJnySAJrDzluaWllOfl5zqf9Dky8= github.com/pborman/uuid v1.2.1 h1:+ZZIw58t/ozdjRaXh/3awHfmWRbzYxJoAdNJxe/3pvw= github.com/pborman/uuid v1.2.1/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= github.com/pelletier/go-toml v1.9.5 h1:4yBQzkHv+7BHq2PQUZF3Mx0IYxG7LsP222s7Agd3ve8= diff --git a/jwk/jwt_strategy.go b/jwk/jwt_strategy.go index 6154066459b..f8ca396a1df 100644 --- a/jwk/jwt_strategy.go +++ b/jwk/jwt_strategy.go @@ -53,7 +53,7 @@ func (j *DefaultJWTSigner) getKeys(ctx context.Context) (private *jose.JSONWebKe return nil, errors.WithStack(fosite.ErrServerError. WithWrap(err). - WithHintf(`Could not ensure that signing keys for "%s" exists. If you are running against a persistent SQL database this is most likely because your "secrets.system" ("SECRETS_SYSTEM" environment variable) is not set or changed. When running with an SQL database backend you need to make sure that the secret is set and stays the same, unless when doing key rotation. This may also happen when you forget to run "hydra migrate sql..`, j.setID)) + WithHintf(`Could not ensure that signing keys for "%s" exists. If you are running against a persistent SQL database this is most likely because your "secrets.system" ("SECRETS_SYSTEM" environment variable) is not set or changed. When running with an SQL database backend you need to make sure that the secret is set and stays the same, unless when doing key rotation. This may also happen when you forget to run "hydra migrate sql up -e".`, j.setID)) } func (j *DefaultJWTSigner) GetPublicKeyID(ctx context.Context) (string, error) { diff --git a/persistence/definitions.go b/persistence/definitions.go index 5cd9c9a3f15..e9cdd98bba1 100644 --- a/persistence/definitions.go +++ b/persistence/definitions.go @@ -35,6 +35,7 @@ type ( Connection(context.Context) *pop.Connection Transaction(context.Context, func(ctx context.Context, c *pop.Connection) error) error Ping() error + PingContext(ctx context.Context) error Networker } Provider interface { diff --git a/persistence/sql/persister.go b/persistence/sql/persister.go index 98161c55cf6..d8beeb728be 100644 --- a/persistence/sql/persister.go +++ b/persistence/sql/persister.go @@ -182,6 +182,10 @@ func (p *Persister) Ping() error { type pinger interface{ Ping() error } return p.conn.Store.(pinger).Ping() } +func (p *Persister) PingContext(ctx context.Context) error { + type pinger interface{ PingContext(context.Context) error } + return p.conn.Store.(pinger).PingContext(ctx) +} func (p *Persister) mustSetNetwork(nid uuid.UUID, v interface{}) interface{} { rv := reflect.ValueOf(v) diff --git a/scripts/db-diff.sh b/scripts/db-diff.sh index 61ce4993edc..52e77387ff7 100755 --- a/scripts/db-diff.sh +++ b/scripts/db-diff.sh @@ -81,7 +81,7 @@ function dump_pg { make test-resetdb >/dev/null 2>&1 sleep 4 - go run . migrate sql "$TEST_DATABASE_POSTGRESQL" --yes >&2 || true + go run . migrate sql up "$TEST_DATABASE_POSTGRESQL" --yes >&2 || true sleep 1 pg_dump -s "$TEST_DATABASE_POSTGRESQL" | sed '/^--/d' } @@ -94,7 +94,7 @@ function dump_cockroach { make test-resetdb >/dev/null 2>&1 sleep 4 - go run . migrate sql "$TEST_DATABASE_COCKROACHDB" --yes > /dev/null || true + go run . migrate sql up "$TEST_DATABASE_COCKROACHDB" --yes > /dev/null || true hydra::util::parse-connection-url "${TEST_DATABASE_COCKROACHDB}" docker run --rm --net=host -it cockroachdb/cockroach:latest-v24.1 dump --dump-all --dump-mode=schema --insecure --user="${DB_USER}" --host="${DB_HOST}" --port="${DB_PORT}" } @@ -107,7 +107,7 @@ function dump_sqlite { hydra::util::ensure-sqlite rm "$SQLITE_PATH" > /dev/null 2>&1 || true - go run -tags sqlite,sqlite_omit_load_extension . migrate sql "sqlite://$SQLITE_PATH?_fk=true" --yes > /dev/null 2>&1 || true + go run -tags sqlite,sqlite_omit_load_extension . migrate sql up "sqlite://$SQLITE_PATH?_fk=true" --yes > /dev/null 2>&1 || true echo '.dump' | sqlite3 "$SQLITE_PATH" } @@ -120,7 +120,7 @@ function dump_mysql { hydra::util::ensure-mysqldump make test-resetdb >/dev/null 2>&1 sleep 10 - go run . migrate sql "$TEST_DATABASE_MYSQL" --yes > /dev/null || true + go run . migrate sql up "$TEST_DATABASE_MYSQL" --yes > /dev/null || true hydra::util::parse-connection-url "${TEST_DATABASE_MYSQL}" mysqldump --user="$DB_USER" --password="$DB_PASSWORD" --host="$DB_HOST" --port="$DB_PORT" "$DB_DB" --no-data } diff --git a/test/e2e/circle-ci.bash b/test/e2e/circle-ci.bash index 1f93dee3b25..03b8f70ac79 100755 --- a/test/e2e/circle-ci.bash +++ b/test/e2e/circle-ci.bash @@ -92,7 +92,7 @@ case $i in esac done -./hydra migrate sql --yes $TEST_DATABASE > ./hydra-migrate.e2e.log 2>&1 +./hydra migrate sql up --yes $TEST_DATABASE > ./hydra-migrate.e2e.log 2>&1 DSN=$TEST_DATABASE \ ./hydra serve all --dev --sqa-opt-out > ./hydra.e2e.log 2>&1 & diff --git a/test/e2e/docker-compose.cockroach.yml b/test/e2e/docker-compose.cockroach.yml index 780e8ffdf2a..de0eee0384c 100644 --- a/test/e2e/docker-compose.cockroach.yml +++ b/test/e2e/docker-compose.cockroach.yml @@ -5,7 +5,7 @@ services: image: oryd/hydra:e2e environment: - DSN=cockroach://root@cockroachd:26257/defaultdb?sslmode=disable&max_conns=20&max_idle_conns=4 - command: migrate sql -e --yes + command: migrate sql up -e --yes restart: on-failure hydra: diff --git a/test/e2e/docker-compose.mysql.yml b/test/e2e/docker-compose.mysql.yml index 5750d3ec81e..e8a03d4eb67 100644 --- a/test/e2e/docker-compose.mysql.yml +++ b/test/e2e/docker-compose.mysql.yml @@ -5,7 +5,7 @@ services: image: oryd/hydra:e2e environment: - DSN=mysql://root:secret@tcp(mysqld:3306)/mysql?max_conns=20&max_idle_conns=4 - command: migrate sql -e --yes + command: migrate sql up -e --yes restart: on-failure hydra: diff --git a/test/e2e/docker-compose.postgres.yml b/test/e2e/docker-compose.postgres.yml index 7c1a4f6bee2..72e3ed7443c 100644 --- a/test/e2e/docker-compose.postgres.yml +++ b/test/e2e/docker-compose.postgres.yml @@ -5,7 +5,7 @@ services: image: oryd/hydra:e2e environment: - DSN=postgres://hydra:secret@postgresd:5432/hydra?sslmode=disable&max_conns=20&max_idle_conns=4 - command: migrate sql -e --yes + command: migrate sql up -e --yes restart: on-failure hydra: