From 4f841dae5423acf3514d50add9e99d28bc339fbb Mon Sep 17 00:00:00 2001 From: aeneasr <3372410+aeneasr@users.noreply.github.com> Date: Thu, 18 Aug 2022 18:14:48 +0200 Subject: [PATCH] fix: make servicelocator explicit --- cmd/cliclient/cleanup.go | 6 +++- cmd/cliclient/migrate.go | 20 ++++++++--- cmd/courier/root.go | 7 ++-- cmd/courier/watch.go | 6 ++-- cmd/daemon/serve.go | 37 ++++++--------------- cmd/root.go | 4 +-- cmd/serve/root.go | 13 ++++---- driver/factory.go | 19 ++++++----- driver/factory_test.go | 10 ++++-- driver/registry.go | 7 ++++ go.mod | 2 +- go.sum | 4 +-- persistence/sql/migratest/migration_test.go | 20 +++++++---- 13 files changed, 90 insertions(+), 65 deletions(-) diff --git a/cmd/cliclient/cleanup.go b/cmd/cliclient/cleanup.go index 3627a6e7c339..8a9e24b48d9a 100644 --- a/cmd/cliclient/cleanup.go +++ b/cmd/cliclient/cleanup.go @@ -3,6 +3,8 @@ package cliclient import ( "github.com/pkg/errors" + "github.com/ory/x/servicelocatorx" + "github.com/ory/x/contextx" "github.com/ory/x/configx" @@ -36,7 +38,9 @@ func (h *CleanupHandler) CleanupSQL(cmd *cobra.Command, args []string) error { d, err := driver.NewWithoutInit( cmd.Context(), cmd.ErrOrStderr(), - opts..., + servicelocatorx.NewOptions(), + nil, + opts, ) if len(d.Config().DSN(cmd.Context())) == 0 { return errors.New(`required config value "dsn" was not set`) diff --git a/cmd/cliclient/migrate.go b/cmd/cliclient/migrate.go index d95bf6d5b283..4b43f1851461 100644 --- a/cmd/cliclient/migrate.go +++ b/cmd/cliclient/migrate.go @@ -7,6 +7,8 @@ import ( "os" "strings" + "github.com/ory/x/servicelocatorx" + "github.com/pkg/errors" "github.com/ory/x/contextx" @@ -35,8 +37,12 @@ func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) error { d, err = driver.NewWithoutInit( cmd.Context(), cmd.ErrOrStderr(), - configx.WithFlags(cmd.Flags()), - configx.SkipValidation()) + servicelocatorx.NewOptions(), + nil, + []configx.OptionModifier{ + configx.WithFlags(cmd.Flags()), + configx.SkipValidation(), + }) if err != nil { return err } @@ -57,9 +63,13 @@ func (h *MigrateHandler) MigrateSQL(cmd *cobra.Command, args []string) error { d, err = driver.NewWithoutInit( cmd.Context(), cmd.ErrOrStderr(), - configx.WithFlags(cmd.Flags()), - configx.SkipValidation(), - configx.WithValue(config.ViperKeyDSN, args[0])) + servicelocatorx.NewOptions(), + nil, + []configx.OptionModifier{ + configx.WithFlags(cmd.Flags()), + configx.SkipValidation(), + configx.WithValue(config.ViperKeyDSN, args[0]), + }) if err != nil { return err } diff --git a/cmd/courier/root.go b/cmd/courier/root.go index 815dfea85370..338dd84f485d 100644 --- a/cmd/courier/root.go +++ b/cmd/courier/root.go @@ -3,6 +3,9 @@ package courier import ( "github.com/spf13/cobra" + "github.com/ory/kratos/driver" + "github.com/ory/x/servicelocatorx" + "github.com/ory/x/configx" ) @@ -16,8 +19,8 @@ func NewCourierCmd() *cobra.Command { return c } -func RegisterCommandRecursive(parent *cobra.Command) { +func RegisterCommandRecursive(parent *cobra.Command, slOpts []servicelocatorx.Option, dOpts []driver.RegistryOption) { c := NewCourierCmd() parent.AddCommand(c) - c.AddCommand(NewWatchCmd()) + c.AddCommand(NewWatchCmd(slOpts, dOpts)) } diff --git a/cmd/courier/watch.go b/cmd/courier/watch.go index b966e4c422f7..faf8ebff826b 100644 --- a/cmd/courier/watch.go +++ b/cmd/courier/watch.go @@ -4,6 +4,8 @@ import ( cx "context" "net/http" + "github.com/ory/x/servicelocatorx" + "golang.org/x/sync/errgroup" "github.com/spf13/cobra" @@ -17,12 +19,12 @@ import ( "github.com/ory/x/reqlog" ) -func NewWatchCmd() *cobra.Command { +func NewWatchCmd(slOpts []servicelocatorx.Option, dOpts []driver.RegistryOption) *cobra.Command { var c = &cobra.Command{ Use: "watch", Short: "Starts the Ory Kratos message courier", RunE: func(cmd *cobra.Command, args []string) error { - r, err := driver.New(cmd.Context(), cmd.ErrOrStderr(), configx.WithFlags(cmd.Flags())) + r, err := driver.New(cmd.Context(), cmd.ErrOrStderr(), servicelocatorx.NewOptions(slOpts...), dOpts, []configx.OptionModifier{configx.WithFlags(cmd.Flags())}) if err != nil { return err } diff --git a/cmd/daemon/serve.go b/cmd/daemon/serve.go index 91969c61a4b4..52513e1fb53c 100644 --- a/cmd/daemon/serve.go +++ b/cmd/daemon/serve.go @@ -4,11 +4,11 @@ import ( "crypto/tls" "net/http" + "github.com/ory/x/servicelocatorx" + "github.com/pkg/errors" "golang.org/x/net/context" - "github.com/ory/x/servicelocator" - "golang.org/x/sync/errgroup" "github.com/ory/kratos/schema" @@ -52,7 +52,6 @@ import ( ) type options struct { - mwf []func(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc) ctx stdctx.Context } @@ -67,19 +66,13 @@ func NewOptions(ctx stdctx.Context, opts []Option) *options { type Option func(*options) -func WithRootMiddleware(m func(rw http.ResponseWriter, r *http.Request, next http.HandlerFunc)) Option { - return func(o *options) { - o.mwf = append(o.mwf, m) - } -} - func WithContext(ctx stdctx.Context) Option { return func(o *options) { o.ctx = ctx } } -func ServePublic(r driver.Registry, cmd *cobra.Command, args []string, opts ...Option) error { +func ServePublic(r driver.Registry, cmd *cobra.Command, args []string, slOpts *servicelocatorx.Options, opts []Option) error { modifiers := NewOptions(cmd.Context(), opts) ctx := modifiers.ctx @@ -87,11 +80,7 @@ func ServePublic(r driver.Registry, cmd *cobra.Command, args []string, opts ...O l := r.Logger() n := negroni.New() - for _, mw := range servicelocator.HTTPMiddlewares(ctx) { - n.Use(mw) - } - - for _, mw := range modifiers.mwf { + for _, mw := range slOpts.HTTPMiddlewares() { n.UseFunc(mw) } @@ -166,7 +155,7 @@ func ServePublic(r driver.Registry, cmd *cobra.Command, args []string, opts ...O return nil } -func ServeAdmin(r driver.Registry, cmd *cobra.Command, args []string, opts ...Option) error { +func ServeAdmin(r driver.Registry, cmd *cobra.Command, args []string, slOpts *servicelocatorx.Options, opts []Option) error { modifiers := NewOptions(cmd.Context(), opts) ctx := modifiers.ctx @@ -174,11 +163,7 @@ func ServeAdmin(r driver.Registry, cmd *cobra.Command, args []string, opts ...Op l := r.Logger() n := negroni.New() - for _, mw := range servicelocator.HTTPMiddlewares(ctx) { - n.Use(mw) - } - - for _, mw := range modifiers.mwf { + for _, mw := range slOpts.HTTPMiddlewares() { n.UseFunc(mw) } @@ -306,7 +291,7 @@ func sqa(ctx stdctx.Context, cmd *cobra.Command, d driver.Registry) *metricsx.Se ) } -func bgTasks(d driver.Registry, cmd *cobra.Command, args []string, opts ...Option) error { +func bgTasks(d driver.Registry, cmd *cobra.Command, args []string, slOpts *servicelocatorx.Options, opts []Option) error { modifiers := NewOptions(cmd.Context(), opts) ctx := modifiers.ctx @@ -316,7 +301,7 @@ func bgTasks(d driver.Registry, cmd *cobra.Command, args []string, opts ...Optio return nil } -func ServeAll(d driver.Registry, opts ...Option) func(cmd *cobra.Command, args []string) error { +func ServeAll(d driver.Registry, slOpts *servicelocatorx.Options, opts []Option) func(cmd *cobra.Command, args []string) error { return func(cmd *cobra.Command, args []string) error { mods := NewOptions(cmd.Context(), opts) ctx := mods.ctx @@ -326,13 +311,13 @@ func ServeAll(d driver.Registry, opts ...Option) func(cmd *cobra.Command, args [ opts = append(opts, WithContext(ctx)) g.Go(func() error { - return ServePublic(d, cmd, args, opts...) + return ServePublic(d, cmd, args, slOpts, opts) }) g.Go(func() error { - return ServeAdmin(d, cmd, args, opts...) + return ServeAdmin(d, cmd, args, slOpts, opts) }) g.Go(func() error { - return bgTasks(d, cmd, args, opts...) + return bgTasks(d, cmd, args, slOpts, opts) }) return g.Wait() } diff --git a/cmd/root.go b/cmd/root.go index f3808a1402c8..d2cb518320af 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -28,7 +28,7 @@ func NewRootCmd() (cmd *cobra.Command) { cmd = &cobra.Command{ Use: "kratos", } - courier.RegisterCommandRecursive(cmd) + courier.RegisterCommandRecursive(cmd, nil, nil) cmd.AddCommand(identities.NewGetCmd(cmd)) cmd.AddCommand(identities.NewDeleteCmd(cmd)) cmd.AddCommand(jsonnet.NewFormatCmd()) @@ -37,7 +37,7 @@ func NewRootCmd() (cmd *cobra.Command) { cmd.AddCommand(jsonnet.NewLintCmd()) cmd.AddCommand(identities.NewListCmd(cmd)) migrate.RegisterCommandRecursive(cmd) - serve.RegisterCommandRecursive(cmd) + serve.RegisterCommandRecursive(cmd, nil, nil) cleanup.RegisterCommandRecursive(cmd) remote.RegisterCommandRecursive(cmd) cmd.AddCommand(identities.NewValidateCmd()) diff --git a/cmd/serve/root.go b/cmd/serve/root.go index 5764e6cc2ea3..3aadce77c93a 100644 --- a/cmd/serve/root.go +++ b/cmd/serve/root.go @@ -17,6 +17,7 @@ package serve import ( "github.com/ory/kratos/driver/config" "github.com/ory/x/configx" + "github.com/ory/x/servicelocatorx" "github.com/spf13/cobra" @@ -25,15 +26,15 @@ import ( ) // serveCmd represents the serve command -func NewServeCmd() (serveCmd *cobra.Command) { +func NewServeCmd(slOpts []servicelocatorx.Option, dOpts []driver.RegistryOption) (serveCmd *cobra.Command) { serveCmd = &cobra.Command{ Use: "serve", Short: "Run the Ory Kratos server", RunE: func(cmd *cobra.Command, args []string) error { ctx := cmd.Context() opts := configx.ConfigOptionsFromContext(ctx) - - d, err := driver.New(ctx, cmd.ErrOrStderr(), append(opts, configx.WithFlags(cmd.Flags()))...) + sl := servicelocatorx.NewOptions(slOpts...) + d, err := driver.New(ctx, cmd.ErrOrStderr(), sl, dOpts, append(opts, configx.WithFlags(cmd.Flags()))) if err != nil { return err } @@ -56,7 +57,7 @@ DON'T DO THIS IN PRODUCTION! d.Logger().Warnf("Config version is '%s' but kratos runs on version '%s'", configVersion, config.Version) } - return daemon.ServeAll(d)(cmd, args) + return daemon.ServeAll(d, sl, nil)(cmd, args) }, } configx.RegisterFlags(serveCmd.PersistentFlags()) @@ -67,6 +68,6 @@ DON'T DO THIS IN PRODUCTION! return serveCmd } -func RegisterCommandRecursive(parent *cobra.Command) { - parent.AddCommand(NewServeCmd()) +func RegisterCommandRecursive(parent *cobra.Command, slOpts []servicelocatorx.Option, dOpts []driver.RegistryOption) { + parent.AddCommand(NewServeCmd(slOpts, dOpts)) } diff --git a/driver/factory.go b/driver/factory.go index af0e0160317f..fc79e91b3edf 100644 --- a/driver/factory.go +++ b/driver/factory.go @@ -4,22 +4,20 @@ import ( "context" "io" - "github.com/ory/kratos/x/servicelocatorx" - "github.com/ory/x/contextx" - "github.com/ory/x/servicelocator" + "github.com/ory/x/servicelocatorx" "github.com/ory/kratos/driver/config" "github.com/ory/x/configx" "github.com/ory/x/logrusx" ) -func New(ctx context.Context, stdOutOrErr io.Writer, opts ...configx.OptionModifier) (Registry, error) { - r, err := NewWithoutInit(ctx, stdOutOrErr, opts...) +func New(ctx context.Context, stdOutOrErr io.Writer, sl *servicelocatorx.Options, dOpts []RegistryOption, opts []configx.OptionModifier) (Registry, error) { + r, err := NewWithoutInit(ctx, stdOutOrErr, sl, dOpts, opts) if err != nil { return nil, err } - ctxter := servicelocator.Contextualizer(ctx, &contextx.Default{}) + ctxter := sl.Contextualizer() if err := r.Init(ctx, ctxter); err != nil { r.Logger().WithError(err).Error("Unable to initialize service registry.") return nil, err @@ -28,10 +26,13 @@ func New(ctx context.Context, stdOutOrErr io.Writer, opts ...configx.OptionModif return r, nil } -func NewWithoutInit(ctx context.Context, stdOutOrErr io.Writer, opts ...configx.OptionModifier) (Registry, error) { - l := logrusx.New("Ory Kratos", config.Version) +func NewWithoutInit(ctx context.Context, stdOutOrErr io.Writer, sl *servicelocatorx.Options, dOpts []RegistryOption, opts []configx.OptionModifier) (Registry, error) { + l := sl.Logger() + if l == nil { + l = logrusx.New("Ory Kratos", config.Version) + } - c := servicelocatorx.ConfigFromContext(ctx, nil) + c := newOptions(dOpts).config if c == nil { var err error c, err = config.New(ctx, l, stdOutOrErr, opts...) diff --git a/driver/factory_test.go b/driver/factory_test.go index b609680ba86d..40565e32b7ba 100644 --- a/driver/factory_test.go +++ b/driver/factory_test.go @@ -5,6 +5,8 @@ import ( "os" "testing" + "github.com/ory/x/servicelocatorx" + "github.com/gofrs/uuid" "github.com/ory/x/configx" @@ -21,8 +23,12 @@ func TestDriverNew(t *testing.T) { r, err := driver.New( context.Background(), os.Stderr, - configx.WithValue(config.ViperKeyDSN, config.DefaultSQLiteMemoryDSN), - configx.SkipValidation()) + servicelocatorx.NewOptions(), + nil, + []configx.OptionModifier{ + configx.WithValue(config.ViperKeyDSN, config.DefaultSQLiteMemoryDSN), + configx.SkipValidation(), + }) require.NoError(t, err) assert.EqualValues(t, config.DefaultSQLiteMemoryDSN, r.Config().DSN(ctx)) diff --git a/driver/registry.go b/driver/registry.go index cab077590f15..dde98c0e9983 100644 --- a/driver/registry.go +++ b/driver/registry.go @@ -154,6 +154,7 @@ func NewRegistryFromDSN(ctx context.Context, c *config.Config, l *logrusx.Logger type options struct { skipNetworkInit bool + config *config.Config } type RegistryOption func(*options) @@ -162,6 +163,12 @@ func SkipNetworkInit(o *options) { o.skipNetworkInit = true } +func WithConfig(config *config.Config) func(o *options) { + return func(o *options) { + o.config = config + } +} + func newOptions(os []RegistryOption) *options { o := new(options) for _, f := range os { diff --git a/go.mod b/go.mod index e5581b1c1bbc..c2517839d518 100644 --- a/go.mod +++ b/go.mod @@ -76,7 +76,7 @@ require ( github.com/ory/kratos-client-go v0.6.3-alpha.1 github.com/ory/mail/v3 v3.0.0 github.com/ory/nosurf v1.2.7 - github.com/ory/x v0.0.453 + github.com/ory/x v0.0.454 github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2 github.com/pkg/errors v0.9.1 github.com/pquerna/otp v1.3.0 diff --git a/go.sum b/go.sum index 784fa0e272ac..9739f4f0dc30 100644 --- a/go.sum +++ b/go.sum @@ -1488,8 +1488,8 @@ github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2 h1:zm6sDvHy/U9XrGpi github.com/ory/sessions v1.2.2-0.20220110165800-b09c17334dc2/go.mod h1:dk2InVEVJ0sfLlnXv9EAgkf6ecYs/i80K/zI+bUmuGM= github.com/ory/viper v1.7.5 h1:+xVdq7SU3e1vNaCsk/ixsfxE4zylk1TJUiJrY647jUE= github.com/ory/viper v1.7.5/go.mod h1:ypOuyJmEUb3oENywQZRgeAMwqgOyDqwboO1tj3DjTaM= -github.com/ory/x v0.0.453 h1:57/UpP55cON7w+L/qloIn1kKKqyjWbBK6KaCvT4LqTA= -github.com/ory/x v0.0.453/go.mod h1:i3TlzVVChaun6sfVscSqGyPr7IuzC3C0aSgS+ODSbNQ= +github.com/ory/x v0.0.454 h1:hDKNrFFMBkBrSHlTY1w+IhZ9CnxMpMz8RRaiaRtxQgA= +github.com/ory/x v0.0.454/go.mod h1:i3TlzVVChaun6sfVscSqGyPr7IuzC3C0aSgS+ODSbNQ= github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw= github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE= github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs= diff --git a/persistence/sql/migratest/migration_test.go b/persistence/sql/migratest/migration_test.go index 56720540f112..772ba39c80a8 100644 --- a/persistence/sql/migratest/migration_test.go +++ b/persistence/sql/migratest/migration_test.go @@ -10,6 +10,8 @@ import ( "testing" "time" + "github.com/ory/x/servicelocatorx" + "github.com/ory/x/fsx" "github.com/ory/kratos/identity" @@ -128,13 +130,17 @@ func TestMigrations(t *testing.T) { d, err := driver.New( context.Background(), os.Stderr, - configx.WithValues(map[string]interface{}{ - config.ViperKeyDSN: url, - config.ViperKeyPublicBaseURL: "https://www.ory.sh/", - config.ViperKeyIdentitySchemas: config.Schemas{{ID: "default", URL: "file://stub/default.schema.json"}}, - config.ViperKeySecretsDefault: []string{"secret"}, - }), - configx.SkipValidation(), + servicelocatorx.NewOptions(), + nil, + []configx.OptionModifier{ + configx.WithValues(map[string]interface{}{ + config.ViperKeyDSN: url, + config.ViperKeyPublicBaseURL: "https://www.ory.sh/", + config.ViperKeyIdentitySchemas: config.Schemas{{ID: "default", URL: "file://stub/default.schema.json"}}, + config.ViperKeySecretsDefault: []string{"secret"}, + }), + configx.SkipValidation(), + }, ) require.NoError(t, err)