diff --git a/HACKING.md b/HACKING.md index 5a84cf8..dc2f49b 100644 --- a/HACKING.md +++ b/HACKING.md @@ -67,19 +67,14 @@ A single module instance can have one or more names. The first name is called called "aliases" and only used by module.GetInstance (e.g. module instance can be fetched by any name). -Some modules attach additional meaning to names. This is generally accepted -since it is better to have only a single instance managing one resource. For -example, module instance implementing forwarding to the downstream server can not -reasonably enforce any limitations unless it is only one instance "controlling" -that downstream. Unique names requirement helps a lot here. - -"Semantical names" idea explained above is not applied when modules instances -are defined "inline" (in place they are used in). These instances have no -instance names and are not added to the global map so they can not be accessed -by modules other than one that used ConfigFromNode on the corresponding config -block. All arguments after the module name in an inline definition represent -"inline arguments". They are passed to the module instance directly and not -used anyhow by other code (i.e. they are not guaranteed to be unique). +Some module instances are defined "inline" in user configuration. That is. +their configuration block is placed right where they are used. These instances +have no instance names and are not added to the global map so they can not be +accessed by modules other than one that used ConfigFromNode on the +corresponding config block. All arguments after the module name in an inline +definition represent "inline arguments". They are passed to the module instance +directly and not used anyhow by other code (i.e. they are not guaranteed to be +unique). ### A word on error logging @@ -99,4 +94,4 @@ together, remote module will provide logs about concrete errors happened and queue module will provide information about tries made and scheduled to be made in the future. -[1]: https://github.com/foxcpp/maddy/wiki/Dev:-Comments-on-design \ No newline at end of file +[1]: https://github.com/foxcpp/maddy/wiki/Dev:-Comments-on-design diff --git a/config/module/modconfig.go b/config/module/modconfig.go index 55d6f7d..a1e887a 100644 --- a/config/module/modconfig.go +++ b/config/module/modconfig.go @@ -14,7 +14,6 @@ import ( "github.com/foxcpp/maddy/config" "github.com/foxcpp/maddy/config/parser" - "github.com/foxcpp/maddy/log" "github.com/foxcpp/maddy/module" ) @@ -25,8 +24,6 @@ func createInlineModule(modName string, args []string) (module.Module, error) { return nil, fmt.Errorf("unknown module: %s", modName) } - log.Debugln("module create", modName, args, "(inline)") - return newMod(modName, "", nil, args) } @@ -35,7 +32,6 @@ func createInlineModule(modName string, args []string) (module.Module, error) { // // args must contain at least one argument, otherwise initInlineModule panics. func initInlineModule(modObj module.Module, globals map[string]interface{}, block *config.Node) error { - log.Debugln("module init", modObj.Name(), modObj.InstanceName(), "(inline)") return modObj.Init(config.NewMap(globals, block)) } diff --git a/endpoint/imap/imap.go b/endpoint/imap/imap.go index e66dce3..65abd0f 100644 --- a/endpoint/imap/imap.go +++ b/endpoint/imap/imap.go @@ -27,8 +27,7 @@ import ( ) type Endpoint struct { - name string - aliases []string + addrs []string serv *imapserver.Server listeners []net.Listener Auth module.AuthProvider @@ -41,16 +40,11 @@ type Endpoint struct { Log log.Logger } -func New(_, instName string, aliases, inlineArgs []string) (module.Module, error) { - if len(inlineArgs) != 0 { - return nil, errors.New("imap: inline arguments are not used") - } +func New(modName string, addrs []string) (module.Module, error) { endp := &Endpoint{ - name: instName, - aliases: aliases, - Log: log.Logger{Name: "imap"}, + addrs: addrs, + Log: log.Logger{Name: "imap"}, } - endp.name = instName return endp, nil } @@ -83,12 +77,11 @@ func (endp *Endpoint) Init(cfg *config.Map) error { return fmt.Errorf("imap: failed to init backend: nil update channel") } - args := append([]string{endp.name}, endp.aliases...) - addresses := make([]config.Endpoint, 0, len(args)) - for _, addr := range args { + addresses := make([]config.Endpoint, 0, len(endp.addrs)) + for _, addr := range endp.addrs { saddr, err := config.ParseEndpoint(addr) if err != nil { - return fmt.Errorf("imap: invalid address: %s", endp.name) + return fmt.Errorf("imap: invalid address: %s", addr) } addresses = append(addresses, saddr) } @@ -154,7 +147,7 @@ func (endp *Endpoint) Name() string { } func (endp *Endpoint) InstanceName() string { - return endp.name + return "imap" } func (endp *Endpoint) Close() error { @@ -204,7 +197,7 @@ func (endp *Endpoint) enableExtensions() error { } func init() { - module.Register("imap", New) + module.RegisterEndpoint("imap", New) imap.CharsetReader = message.CharsetReader } diff --git a/endpoint/smtp/smtp.go b/endpoint/smtp/smtp.go index 7e8e3e5..1307026 100644 --- a/endpoint/smtp/smtp.go +++ b/endpoint/smtp/smtp.go @@ -216,7 +216,7 @@ type Endpoint struct { Auth module.AuthProvider serv *smtp.Server name string - aliases []string + addrs []string listeners []net.Listener pipeline *msgpipeline.MsgPipeline resolver dns.Resolver @@ -232,20 +232,17 @@ type Endpoint struct { } func (endp *Endpoint) Name() string { - return "smtp" + return endp.name } func (endp *Endpoint) InstanceName() string { return endp.name } -func New(modName, instName string, aliases, inlineArgs []string) (module.Module, error) { - if len(inlineArgs) != 0 { - return nil, fmt.Errorf("%s: inline arguments are not used", modName) - } +func New(modName string, addrs []string) (module.Module, error) { endp := &Endpoint{ - name: instName, - aliases: aliases, + name: modName, + addrs: addrs, submission: modName == "submission", lmtp: modName == "lmtp", resolver: net.DefaultResolver, @@ -264,9 +261,8 @@ func (endp *Endpoint) Init(cfg *config.Map) error { endp.Log.Debugf("authentication provider: %s %s", endp.Auth.(module.Module).Name(), endp.Auth.(module.Module).InstanceName()) } - args := append([]string{endp.name}, endp.aliases...) - addresses := make([]config.Endpoint, 0, len(args)) - for _, addr := range args { + addresses := make([]config.Endpoint, 0, len(endp.addrs)) + for _, addr := range endp.addrs { saddr, err := config.ParseEndpoint(addr) if err != nil { return fmt.Errorf("smtp: invalid address: %s", addr) @@ -441,9 +437,9 @@ func (endp *Endpoint) Close() error { } func init() { - module.Register("smtp", New) - module.Register("submission", New) - module.Register("lmtp", New) + module.RegisterEndpoint("smtp", New) + module.RegisterEndpoint("submission", New) + module.RegisterEndpoint("lmtp", New) rand.Seed(time.Now().UnixNano()) } diff --git a/maddy.go b/maddy.go index b5b9c60..4746320 100644 --- a/maddy.go +++ b/maddy.go @@ -5,7 +5,6 @@ import ( "github.com/foxcpp/maddy/config" "github.com/foxcpp/maddy/log" - "github.com/foxcpp/maddy/module" // Import packages for side-effect of module registration. _ "github.com/foxcpp/maddy/auth/external" @@ -22,13 +21,7 @@ import ( _ "github.com/foxcpp/maddy/target/smtp_downstream" ) -type modInfo struct { - instance module.Module - cfg config.Node -} - func Start(cfg []config.Node) error { - instances := make(map[string]modInfo) globals := config.NewMap(nil, &config.Node{Children: cfg}) globals.String("hostname", false, false, "", nil) globals.String("autogenerated_msg_domain", false, false, "", nil) @@ -45,65 +38,17 @@ func Start(cfg []config.Node) error { defer log.DefaultLogger.Out.Close() - for _, block := range unmatched { - var instName string - var modAliases []string - if len(block.Args) == 0 { - instName = block.Name - } else { - instName = block.Args[0] - modAliases = block.Args[1:] - } - - modName := block.Name - - factory := module.Get(modName) - if factory == nil { - return config.NodeErr(&block, "unknown module: %s", modName) - } - - if module.HasInstance(instName) { - return config.NodeErr(&block, "config block named %s already exists", instName) - } - - log.Debugln("module create", modName, instName) - inst, err := factory(modName, instName, modAliases, nil) - if err != nil { - return err - } - - block := block - module.RegisterInstance(inst, config.NewMap(globals.Values, &block)) - for _, alias := range modAliases { - if module.HasInstance(alias) { - return config.NodeErr(&block, "config block named %s already exists", alias) - } - module.RegisterAlias(alias, instName) - log.Debugln("module alias", alias, "->", instName) - } - instances[instName] = modInfo{instance: inst, cfg: block} + insts, err := instancesFromConfig(globals.Values, unmatched) + if err != nil { + return err } - for _, inst := range instances { - if module.Initialized[inst.instance.InstanceName()] { - log.Debugln("module init", inst.instance.Name(), inst.instance.InstanceName(), "skipped because it was lazily initialized before") - continue - } + handleSignals() - module.Initialized[inst.instance.InstanceName()] = true - log.Debugln("module init", inst.instance.Name(), inst.instance.InstanceName()) - if err := inst.instance.Init(config.NewMap(globals.Values, &inst.cfg)); err != nil { - return err - } - } - - waitForSignal() - - for _, inst := range instances { - if closer, ok := inst.instance.(io.Closer); ok { - log.Debugln("clean-up for module", inst.instance.Name(), inst.instance.InstanceName()) + for _, inst := range insts { + if closer, ok := inst.(io.Closer); ok { if err := closer.Close(); err != nil { - log.Printf("module %s (%s) close failed: %v", inst.instance.Name(), inst.instance.InstanceName(), err) + log.Printf("module %s (%s) close failed: %v", inst.Name(), inst.InstanceName(), err) } } } diff --git a/module.go b/module.go new file mode 100644 index 0000000..9b770cb --- /dev/null +++ b/module.go @@ -0,0 +1,100 @@ +package maddy + +import ( + "github.com/foxcpp/maddy/config" + "github.com/foxcpp/maddy/log" + "github.com/foxcpp/maddy/module" +) + +type modInfo struct { + instance module.Module + cfg config.Node +} + +func instancesFromConfig(globals map[string]interface{}, nodes []config.Node) ([]module.Module, error) { + var ( + endpoints []modInfo + mods []modInfo + ) + + for _, block := range nodes { + var instName string + var modAliases []string + if len(block.Args) == 0 { + instName = block.Name + } else { + instName = block.Args[0] + modAliases = block.Args[1:] + } + + modName := block.Name + + endpFactory := module.GetEndpoint(modName) + if endpFactory != nil { + inst, err := endpFactory(modName, block.Args) + if err != nil { + return nil, err + } + + endpoints = append(endpoints, modInfo{instance: inst, cfg: block}) + continue + } + + factory := module.Get(modName) + if factory == nil { + return nil, config.NodeErr(&block, "unknown module: %s", modName) + } + + if module.HasInstance(instName) { + return nil, config.NodeErr(&block, "config block named %s already exists", instName) + } + + inst, err := factory(modName, instName, modAliases, nil) + if err != nil { + return nil, err + } + + block := block + module.RegisterInstance(inst, config.NewMap(globals, &block)) + for _, alias := range modAliases { + if module.HasInstance(alias) { + return nil, config.NodeErr(&block, "config block named %s already exists", alias) + } + module.RegisterAlias(alias, instName) + } + mods = append(mods, modInfo{instance: inst, cfg: block}) + } + + for _, endp := range endpoints { + if err := endp.instance.Init(config.NewMap(globals, &endp.cfg)); err != nil { + return nil, err + } + } + + // We initialize all non-endpoint modules defined at top-level + // just for purpose of checking that they have a valid configuration. + // + // Modules that are actually used will be pulled by lazy initialization + // logic during endpoint initialization. + for _, inst := range mods { + if module.Initialized[inst.instance.InstanceName()] { + continue + } + + log.Printf("%s (%s) is not used anywhere", inst.instance.InstanceName(), inst.instance.Name()) + + module.Initialized[inst.instance.InstanceName()] = true + if err := inst.instance.Init(config.NewMap(globals, &inst.cfg)); err != nil { + return nil, err + } + } + + res := make([]module.Module, 0, len(mods)+len(endpoints)) + for _, endp := range endpoints { + res = append(res, endp.instance) + } + for _, mod := range mods { + res = append(res, mod.instance) + } + return res, nil +} diff --git a/module/instances.go b/module/instances.go index 260b69f..9a30d2f 100644 --- a/module/instances.go +++ b/module/instances.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/foxcpp/maddy/config" - "github.com/foxcpp/maddy/log" ) var ( @@ -64,11 +63,9 @@ func GetInstance(name string) (Module, error) { // Break circular dependencies. if Initialized[name] { - log.Debugln("module init", mod.mod.Name(), name, "(lazy init skipped)") return mod.mod, nil } - log.Debugln("module init", mod.mod.Name(), name, "(lazy)") Initialized[name] = true if err := mod.mod.Init(mod.cfg); err != nil { return mod.mod, err diff --git a/module/module.go b/module/module.go index 90fb2de..82f86c9 100644 --- a/module/module.go +++ b/module/module.go @@ -55,3 +55,17 @@ type Module interface { // If module is defined inline, instName will be empty and all values // specified after module name in configuration will be in inlineArgs. type FuncNewModule func(modName, instName string, aliases, inlineArgs []string) (Module, error) + +// FuncNewEndpoint is a function that creates new instance of endpoint +// module. +// +// Compared to regular modules, endpoint module instances are: +// - Not registered in the global registry. +// - Can't be defined inline. +// - Don't have an unique name +// - All config arguments are always passed as an 'addrs' slice and not used as +// names. +// +// As a consequence of having no per-instance name, InstanceName of the module +// object always returns the same value as Name. +type FuncNewEndpoint func(modName string, addrs []string) (Module, error) diff --git a/module/registry.go b/module/registry.go index 7d1e280..31229f9 100644 --- a/module/registry.go +++ b/module/registry.go @@ -4,8 +4,11 @@ import ( "sync" ) -var modules = make(map[string]FuncNewModule) -var modulesLock sync.RWMutex +var ( + modules = make(map[string]FuncNewModule) + endpoints = make(map[string]FuncNewEndpoint) + modulesLock sync.RWMutex +) // Register adds module factory function to global registry. // @@ -26,6 +29,8 @@ func Register(name string, factory FuncNewModule) { // Get returns module from global registry. // +// This function does not return endpoint-type modules, use GetEndpoint for +// that. // Nil is returned if no module with specified name is registered. func Get(name string) FuncNewModule { modulesLock.RLock() @@ -33,3 +38,28 @@ func Get(name string) FuncNewModule { return modules[name] } + +// GetEndpoints returns an endpoint module from global registry. +// +// Nil is returned if no module with specified name is registered. +func GetEndpoint(name string) FuncNewEndpoint { + modulesLock.RLock() + defer modulesLock.RUnlock() + + return endpoints[name] +} + +// RegisterEndpoint registers an endpoint module. +// +// See FuncNewEndpoint for information about +// differences of endpoint modules from regular modules. +func RegisterEndpoint(name string, factory FuncNewEndpoint) { + modulesLock.Lock() + defer modulesLock.Unlock() + + if _, ok := endpoints[name]; ok { + panic("Register: module with specified name is already registered: " + name) + } + + endpoints[name] = factory +} diff --git a/signal.go b/signal.go index a7a1f7e..37ae5f0 100644 --- a/signal.go +++ b/signal.go @@ -10,7 +10,13 @@ import ( "github.com/foxcpp/maddy/log" ) -func waitForSignal() os.Signal { +// handleSignals function creates and listens on OS signals channel. +// +// OS-specific signals that correspond to the program termination +// (SIGTERM, SIGHUP, SIGINT) will cause this function to return. +// +// SIGUSR1 will call reinitLogging without returning. +func handleSignals() os.Signal { sig := make(chan os.Signal, 5) signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGINT, syscall.SIGUSR1) @@ -21,7 +27,7 @@ func waitForSignal() os.Signal { reinitLogging() default: go func() { - s := waitForSignal() + s := handleSignals() log.Printf("forced shutdown due to signal (%v)!", s) os.Exit(1) }() diff --git a/signal_nonposix.go b/signal_nonposix.go index 8a96c06..45da4d2 100644 --- a/signal_nonposix.go +++ b/signal_nonposix.go @@ -10,7 +10,7 @@ import ( "github.com/foxcpp/maddy/log" ) -func waitForSignal() os.Signal { +func handleSignals() os.Signal { sig := make(chan os.Signal, 5) signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP, syscall.SIGINT)