From a9e7360775b4d4eb0b8a9d954219007fa761fe87 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Wed, 17 Aug 2022 16:55:46 -0400 Subject: [PATCH] daemon/config: remove AuthzMiddleware field The authorization.Middleware contains a sync.Mutex field, making it non-copyable. Remove one of the barriers to allowing deep copies of config.Config values. Inject the middleware into Daemon as a constructor argument instead. Signed-off-by: Cory Snider --- cmd/dockerd/daemon.go | 22 +++++++++------------- daemon/config/config.go | 34 ++++++++++++++++------------------ daemon/daemon.go | 5 +++-- 3 files changed, 28 insertions(+), 33 deletions(-) diff --git a/cmd/dockerd/daemon.go b/cmd/dockerd/daemon.go index 90348f1217..1e30b5a585 100644 --- a/cmd/dockerd/daemon.go +++ b/cmd/dockerd/daemon.go @@ -190,11 +190,9 @@ func (cli *DaemonCli) start(opts *daemonOptions) (err error) { pluginStore := plugin.NewStore() - if err := cli.initMiddlewares(&cli.api, pluginStore); err != nil { - logrus.Fatalf("Error creating middlewares: %v", err) - } + cli.authzMiddleware = initMiddlewares(&cli.api, cli.Config, pluginStore) - d, err := daemon.NewDaemon(ctx, cli.Config, pluginStore) + d, err := daemon.NewDaemon(ctx, cli.Config, pluginStore, cli.authzMiddleware) if err != nil { return errors.Wrap(err, "failed to start daemon") } @@ -543,25 +541,23 @@ func initRouter(opts routerOptions) { opts.api.InitRouter(routers...) } -// TODO: remove this from cli and return the authzMiddleware -func (cli *DaemonCli) initMiddlewares(s *apiserver.Server, pluginStore plugingetter.PluginGetter) error { +func initMiddlewares(s *apiserver.Server, cfg *config.Config, pluginStore plugingetter.PluginGetter) *authorization.Middleware { v := dockerversion.Version - exp := middleware.NewExperimentalMiddleware(cli.Config.Experimental) + exp := middleware.NewExperimentalMiddleware(cfg.Experimental) s.UseMiddleware(exp) vm := middleware.NewVersionMiddleware(v, api.DefaultVersion, api.MinVersion) s.UseMiddleware(vm) - if cli.Config.CorsHeaders != "" { - c := middleware.NewCORSMiddleware(cli.Config.CorsHeaders) + if cfg.CorsHeaders != "" { + c := middleware.NewCORSMiddleware(cfg.CorsHeaders) s.UseMiddleware(c) } - cli.authzMiddleware = authorization.NewMiddleware(cli.Config.AuthorizationPlugins, pluginStore) - cli.Config.AuthzMiddleware = cli.authzMiddleware - s.UseMiddleware(cli.authzMiddleware) - return nil + authzMiddleware := authorization.NewMiddleware(cfg.AuthorizationPlugins, pluginStore) + s.UseMiddleware(authzMiddleware) + return authzMiddleware } func (cli *DaemonCli) getContainerdDaemonOpts() ([]supervisor.DaemonOpt, error) { diff --git a/daemon/config/config.go b/daemon/config/config.go index 42f7add76d..6016d6ced0 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -17,7 +17,6 @@ import ( "github.com/containerd/containerd/runtime/v2/shim" "github.com/docker/docker/opts" - "github.com/docker/docker/pkg/authorization" "github.com/docker/docker/registry" "github.com/imdario/mergo" "github.com/pkg/errors" @@ -150,23 +149,22 @@ type DNSConfig struct { // It includes json tags to deserialize configuration from a file // using the same names that the flags in the command line use. type CommonConfig struct { - AuthzMiddleware *authorization.Middleware `json:"-"` - AuthorizationPlugins []string `json:"authorization-plugins,omitempty"` // AuthorizationPlugins holds list of authorization plugins - AutoRestart bool `json:"-"` - Context map[string][]string `json:"-"` - DisableBridge bool `json:"-"` - ExecOptions []string `json:"exec-opts,omitempty"` - GraphDriver string `json:"storage-driver,omitempty"` - GraphOptions []string `json:"storage-opts,omitempty"` - Labels []string `json:"labels,omitempty"` - Mtu int `json:"mtu,omitempty"` - NetworkDiagnosticPort int `json:"network-diagnostic-port,omitempty"` - Pidfile string `json:"pidfile,omitempty"` - RawLogs bool `json:"raw-logs,omitempty"` - Root string `json:"data-root,omitempty"` - ExecRoot string `json:"exec-root,omitempty"` - SocketGroup string `json:"group,omitempty"` - CorsHeaders string `json:"api-cors-header,omitempty"` + AuthorizationPlugins []string `json:"authorization-plugins,omitempty"` // AuthorizationPlugins holds list of authorization plugins + AutoRestart bool `json:"-"` + Context map[string][]string `json:"-"` + DisableBridge bool `json:"-"` + ExecOptions []string `json:"exec-opts,omitempty"` + GraphDriver string `json:"storage-driver,omitempty"` + GraphOptions []string `json:"storage-opts,omitempty"` + Labels []string `json:"labels,omitempty"` + Mtu int `json:"mtu,omitempty"` + NetworkDiagnosticPort int `json:"network-diagnostic-port,omitempty"` + Pidfile string `json:"pidfile,omitempty"` + RawLogs bool `json:"raw-logs,omitempty"` + Root string `json:"data-root,omitempty"` + ExecRoot string `json:"exec-root,omitempty"` + SocketGroup string `json:"group,omitempty"` + CorsHeaders string `json:"api-cors-header,omitempty"` // Proxies holds the proxies that are configured for the daemon. Proxies `json:"proxies"` diff --git a/daemon/daemon.go b/daemon/daemon.go index eecdd85f33..f285788366 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -46,6 +46,7 @@ import ( "github.com/docker/docker/libnetwork" "github.com/docker/docker/libnetwork/cluster" nwconfig "github.com/docker/docker/libnetwork/config" + "github.com/docker/docker/pkg/authorization" "github.com/docker/docker/pkg/fileutils" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/plugingetter" @@ -721,7 +722,7 @@ func (daemon *Daemon) IsSwarmCompatible() error { // NewDaemon sets up everything for the daemon to be able to service // requests from the webserver. -func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.Store) (daemon *Daemon, err error) { +func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.Store, authzMiddleware *authorization.Middleware) (daemon *Daemon, err error) { // Verify platform-specific requirements. // TODO(thaJeztah): this should be called before we try to create the daemon; perhaps together with the config validation. if err := checkSystem(); err != nil { @@ -928,7 +929,7 @@ func NewDaemon(ctx context.Context, config *config.Config, pluginStore *plugin.S RegistryService: registryService, LiveRestoreEnabled: config.LiveRestoreEnabled, LogPluginEvent: d.LogPluginEvent, // todo: make private - AuthzMiddleware: config.AuthzMiddleware, + AuthzMiddleware: authzMiddleware, }) if err != nil { return nil, errors.Wrap(err, "couldn't create plugin manager")