Allow to define module instances "inline" (see #42)

config.go

@@ -14,90 +14,142 @@ import (
 Config matchers for module interfaces.
 */
 
-func authProvider(modName string) (module.AuthProvider, error) {
-	modObj, err := module.GetInstance(modName)
+// createModule is a helper function for config matchers that can create inline modules.
+func createModule(args []string) (module.Module, error) {
+	modName := args[0]
+	var instName string
+	if len(args) >= 2 {
+		instName = args[1]
+		if module.HasInstance(instName) {
+			return nil, fmt.Errorf("module instance named %s already exists", instName)
+		}
+	}
+
+	newMod := module.Get(modName)
+	if newMod == nil {
+		return nil, fmt.Errorf("unknown module: %s", modName)
+	}
+
+	log.Debugln("module create", modName, instName, "(inline)")
+
+	return newMod(modName, instName)
+}
+
+func initInlineModule(modObj module.Module, globals map[string]interface{}, node *config.Node) error {
+	// This is to ensure modules Init will see expected node layout if it breaks
+	// Map abstraction and works with map.Values.
+	//
+	// Expected: modName modArgs { ... }
+	// Actual: something modName modArgs { ... }
+	node.Name = node.Args[0]
+	node.Args = node.Args[1:]
+
+	log.Debugln("module init", modObj.Name(), modObj.InstanceName(), "(inline)")
+	return modObj.Init(config.NewMap(globals, node))
+}
+
+func deliverDirective(m *config.Map, node *config.Node) (interface{}, error) {
+	return deliverTarget(m.Globals, node)
+}
+
+func deliverTarget(globals map[string]interface{}, node *config.Node) (module.DeliveryTarget, error) {
+	// First argument to make it compatible with config.Map.
+	if len(node.Args) == 0 {
+		return nil, config.NodeErr(node, "expected at least 1 argument")
+	}
+
+	var modObj module.Module
+	var err error
+	if node.Children != nil {
+		modObj, err = createModule(node.Args)
 		if err != nil {
+			return nil, config.NodeErr(node, "%s", err.Error())
+		}
+	} else {
+		modObj, err = module.GetInstance(node.Args[0])
+		if err != nil {
+			return nil, config.NodeErr(node, "%s", err.Error())
+		}
+	}
+
+	target, ok := modObj.(module.DeliveryTarget)
+	if !ok {
+		return nil, config.NodeErr(node, "module %s doesn't implement delivery target interface", modObj.Name())
+	}
+
+	if node.Children != nil {
+		if err := initInlineModule(modObj, globals, node); err != nil {
 			return nil, err
 		}
+	}
+
+	return target, nil
+}
+
+func authDirective(m *config.Map, node *config.Node) (interface{}, error) {
+	if len(node.Args) == 0 {
+		return nil, m.MatchErr("expected at least 1 argument")
+	}
+
+	var modObj module.Module
+	var err error
+	if node.Children != nil {
+		modObj, err = createModule(node.Args)
+		if err != nil {
+			return nil, m.MatchErr("%s", err.Error())
+		}
+	} else {
+		modObj, err = module.GetInstance(node.Args[0])
+		if err != nil {
+			return nil, m.MatchErr("%s", err.Error())
+		}
+	}
 
 	provider, ok := modObj.(module.AuthProvider)
 	if !ok {
-		return nil, fmt.Errorf("module %s doesn't implements auth. provider interface", modObj.Name())
+		return nil, m.MatchErr("module %s doesn't implement auth. provider interface", modObj.Name())
 	}
+
+	if node.Children != nil {
+		if err := initInlineModule(modObj, m.Globals, node); err != nil {
+			return nil, err
+		}
+	}
+
 	return provider, nil
 }
 
-func authDirective(m *config.Map, node *config.Node) (interface{}, error) {
-	if len(node.Args) != 1 {
-		return nil, m.MatchErr("expected 1 argument")
-	}
-	if len(node.Children) != 0 {
-		return nil, m.MatchErr("can't declare block here")
+func storageDirective(m *config.Map, node *config.Node) (interface{}, error) {
+	if len(node.Args) == 0 {
+		return nil, m.MatchErr("expected at least 1 argument")
 	}
 
-	modObj, err := authProvider(node.Args[0])
+	var modObj module.Module
+	var err error
+	if node.Children != nil {
+		modObj, err = createModule(node.Args)
 		if err != nil {
 			return nil, m.MatchErr("%s", err.Error())
 		}
-
-	return modObj, nil
-}
-
-func storageBackend(modName string) (module.Storage, error) {
-	modObj, err := module.GetInstance(modName)
+	} else {
+		modObj, err = module.GetInstance(node.Args[0])
 		if err != nil {
-		return nil, err
+			return nil, m.MatchErr("%s", err.Error())
+		}
 	}
 
 	backend, ok := modObj.(module.Storage)
 	if !ok {
-		return nil, fmt.Errorf("module %s doesn't implements storage interface", modObj.Name())
-	}
-	return backend, nil
+		return nil, m.MatchErr("module %s doesn't implement storage interface", modObj.Name())
 	}
 
-func storageDirective(m *config.Map, node *config.Node) (interface{}, error) {
-	if len(node.Args) != 1 {
-		return nil, m.MatchErr("expected 1 argument")
-	}
-	if len(node.Children) != 0 {
-		return nil, m.MatchErr("can't declare block here")
-	}
-
-	modObj, err := storageBackend(node.Args[0])
-	if err != nil {
-		return nil, m.MatchErr("%s", err.Error())
-	}
-
-	return modObj, nil
-}
-
-func deliveryTarget(modName string) (module.DeliveryTarget, error) {
-	mod, err := module.GetInstance(modName)
-	if mod == nil {
+	if node.Children != nil {
+		if err := initInlineModule(modObj, m.Globals, node); err != nil {
 			return nil, err
 		}
-
-	target, ok := mod.(module.DeliveryTarget)
-	if !ok {
-		return nil, fmt.Errorf("module %s doesn't implements delivery target interface", mod.Name())
-	}
-	return target, nil
 	}
 
-func deliverDirective(m *config.Map, node *config.Node) (interface{}, error) {
-	if len(node.Args) != 1 {
-		return nil, m.MatchErr("expected 1 argument")
-	}
-	if len(node.Children) != 0 {
-		return nil, m.MatchErr("can't declare block here")
-	}
-
-	modObj, err := deliveryTarget(node.Args[0])
-	if err != nil {
-		return nil, m.MatchErr("%s", err.Error())
-	}
-	return modObj, nil
+	return backend, nil
 }
 
 func logOutput(m *config.Map, node *config.Node) (interface{}, error) {

maddy.conf.5.scd

@@ -67,15 +67,13 @@ directive0 {
 }
 ```
 
-Level of nesting is limited, but you should not ever hit the limit with correct
+Level of nesting is limited, but you should never hit the limit with correct
 configuration.
 
-An empty block is equivalent to no block, the following directives are absolutely
-the same from maddy perspective.
-
+In most cases, an empty block is equivalent to no block:
 ```
 directive { }
-directive2
+directive2 # same as above
 ```
 
 ## Environment variables
@@ -190,6 +188,45 @@ is same as just
 Remaining man page sections describe various modules you can use in your
 configuration.
 
+## "Inline" configuration blocks
+
+In most cases where you are supposed to specify configuration block name, you
+can instead write module name and include configuration block itself.
+
+Like that:
+```
+something {
+    auth sql {
+        driver sqlite3
+        dsn auth.db
+    }
+}
+```
+instead of
+```
+sql thing_name {
+    driver sqlite3
+    dsn auth.db
+}
+
+something {
+    auth thing_name
+}
+```
+
+Exceptions to this rule are explicitly noted in the documentation.
+
+*Note* that in certain cases you also have to specify a name for "inline"
+configuration block. This is required when the used module uses configuration
+block name as a key to store persistent data.
+```
+smtp ... {
+    deliver queue block_name_here {
+        target remote
+    }
+}
+```
+
 
 # GLOBAL DIRECTIVES
 
@@ -478,14 +515,14 @@ will be returned to the message source (SMTP client).
 
 You can add any number of steps you want using the following directives:
 
-## filter <instnace_name> [opts]
+## filter <instnace_name>
 
 Apply a "filter" to a message, instance_name is the configuration block name.
 You can pass additional parameters to filter by adding key=value pairs to the
 end directive, you can omit the value and just specify key if it is
 supported by the filter.
 
-## deliver <instance_name> [opts]
+## deliver <instance_name>
 
 Same as the filter directive, but also executes certain pre-delivery
 operations required by RFC 5321 (SMTP), i.e. it adds Received header to

module/filter.go

@@ -61,9 +61,6 @@ type DeliveryContext struct {
 	// For example, spam filter may set Ctx[spam] to true to tell storage
 	// backend to mark message as spam.
 	Ctx map[string]interface{}
-
-	// Custom options passed to filter from server configuration.
-	Opts map[string]string
 }
 
 // DeepCopy creates a copy of the DeliveryContext structure, also
@@ -93,9 +90,6 @@ func (ctx *DeliveryContext) DeepCopy() *DeliveryContext {
 		cpy.To = append(cpy.To, rcpt)
 	}
 
-	// Opts should not be shared between calls.
-	cpy.Opts = nil
-
 	return &cpy
 }
 

queue.go

@@ -184,49 +184,7 @@ func (q *Queue) attemptDelivery(meta *QueueMetadata, body io.Reader) (shouldRetr
 	return false, nil
 }
 
-func filterRcpts(ctx *module.DeliveryContext) error {
-	newRcpts := make([]string, 0, len(ctx.To))
-	for _, rcpt := range ctx.To {
-		parts := strings.Split(rcpt, "@")
-		if len(parts) != 2 {
-			return fmt.Errorf("malformed address %s: missing domain part", rcpt)
-		}
-
-		if _, ok := ctx.Opts["local_only"]; ok {
-			hostname := ctx.Opts["hostname"]
-			if hostname == "" {
-				hostname = ctx.OurHostname
-			}
-
-			if parts[1] != hostname {
-				log.Debugf("local_only, skipping %s", rcpt)
-				continue
-			}
-		}
-		if _, ok := ctx.Opts["remote_only"]; ok {
-			hostname := ctx.Opts["hostname"]
-			if hostname == "" {
-				hostname = ctx.OurHostname
-			}
-
-			if parts[1] == hostname {
-				log.Debugf("remote_only, skipping %s", rcpt)
-				continue
-			}
-		}
-
-		newRcpts = append(newRcpts, rcpt)
-	}
-
-	ctx.To = newRcpts
-	return nil
-}
-
 func (q *Queue) Deliver(ctx module.DeliveryContext, msg io.Reader) error {
-	if err := filterRcpts(&ctx); err != nil {
-		return err
-	}
-
 	if len(ctx.To) == 0 {
 		return nil
 	}

smtp.go

@@ -175,7 +175,7 @@ func (endp *SMTPEndpoint) setConfig(cfg *config.Map) error {
 	endp.serv.ReadTimeout = time.Duration(readTimeoutSecs) * time.Second
 
 	for _, entry := range remainingDirs {
-		step, err := StepFromCfg(entry)
+		step, err := StepFromCfg(cfg.Globals, entry)
 		if err != nil {
 			return err
 		}

smtppipeline.go

@@ -17,6 +17,9 @@ import (
 	"github.com/emersion/maddy/module"
 )
 
+// TODO: Consider merging SMTPPipelineStep interface with module.Filter and
+// converting check_source_* into filters.
+
 type SMTPPipelineStep interface {
 	// Pass applies step's processing logic to the message.
 	//
@@ -40,11 +43,9 @@ func (requireAuthStep) Pass(ctx *module.DeliveryContext, _ io.Reader) (io.Reader
 
 type filterStep struct {
 	f module.Filter
-	opts map[string]string
 }
 
 func (step filterStep) Pass(ctx *module.DeliveryContext, msg io.Reader) (io.Reader, bool, error) {
-	ctx.Opts = step.opts
 	r, err := step.f.Apply(ctx, msg)
 	if err == module.ErrSilentDrop {
 		return nil, false, nil
@@ -57,7 +58,6 @@ func (step filterStep) Pass(ctx *module.DeliveryContext, msg io.Reader) (io.Read
 
 type deliverStep struct {
 	t module.DeliveryTarget
-	opts map[string]string
 }
 
 func LookupAddr(ip net.IP) (string, error) {
@@ -93,7 +93,6 @@ func (step deliverStep) Pass(ctx *module.DeliveryContext, msg io.Reader) (io.Rea
 
 	msg = io.MultiReader(strings.NewReader(received), msg)
 
-	ctx.Opts = step.opts
 	err := step.t.Deliver(*ctx, msg)
 	return nil, err == nil, err
 }
@@ -194,63 +193,49 @@ func stopStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
 	}
 }
 
-func filterStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
+func filterStepFromCfg(globals map[string]interface{}, node *config.Node) (SMTPPipelineStep, error) {
 	if len(node.Args) == 0 {
-		return nil, errors.New("filter: expected at least one argument")
+		return nil, config.NodeErr(node, "expected at least 1 argument")
 	}
 
-	modInst, err := module.GetInstance(node.Args[0])
+	var modObj module.Module
+	var err error
+	if node.Children != nil {
+		modObj, err = createModule(node.Args)
 		if err != nil {
-		return nil, fmt.Errorf("filter: unknown module instance: %s", node.Args[0])
+			return nil, config.NodeErr(node, "%s", err.Error())
 		}
-
-	filterInst, ok := modInst.(module.Filter)
-	if !ok {
-		return nil, fmt.Errorf("filter: module %s (%s) doesn't implements Filter", modInst.Name(), modInst.InstanceName())
-	}
-
-	return filterStep{
-		f:    filterInst,
-		opts: readOpts(node.Args[1:]),
-	}, nil
-}
-
-func deliverStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
-	if len(node.Args) == 0 {
-		return nil, errors.New("deliver: expected at least one argument")
-	}
-
-	modInst, err := module.GetInstance(node.Args[0])
-	if err != nil {
-		return nil, fmt.Errorf("deliver: unknown module instance: %s", node.Args[0])
-	}
-
-	deliveryInst, ok := modInst.(module.DeliveryTarget)
-	if !ok {
-		return nil, fmt.Errorf("deliver: module %s (%s) doesn't implements DeliveryTarget", modInst.Name(), modInst.InstanceName())
-	}
-
-	return deliverStep{
-		t:    deliveryInst,
-		opts: readOpts(node.Args[1:]),
-	}, nil
-}
-
-func readOpts(args []string) (res map[string]string) {
-	res = make(map[string]string)
-	for _, arg := range args {
-		parts := strings.SplitN(arg, "=", 1)
-		if len(parts) == 1 {
-			res[parts[0]] = ""
 	} else {
-			res[parts[0]] = parts[1]
+		modObj, err = module.GetInstance(node.Args[0])
+		if err != nil {
+			return nil, config.NodeErr(node, "%s", err.Error())
+		}
 	}
 
-	}
-	return
+	filter, ok := modObj.(module.Filter)
+	if !ok {
+		return nil, config.NodeErr(node, "module %s doesn't implement filter interface", modObj.Name())
 	}
 
-func matchStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
+	if node.Children != nil {
+		if err := initInlineModule(modObj, globals, node); err != nil {
+			return nil, err
+		}
+	}
+
+	return filterStep{filter}, nil
+}
+
+func deliverStepFromCfg(globals map[string]interface{}, node *config.Node) (SMTPPipelineStep, error) {
+	target, err := deliverTarget(globals, node)
+	if err != nil {
+		return nil, err
+	}
+
+	return deliverStep{target}, nil
+}
+
+func matchStepFromCfg(globals map[string]interface{}, node config.Node) (SMTPPipelineStep, error) {
 	if len(node.Args) != 3 && len(node.Args) != 2 {
 		return nil, errors.New("match: expected 3 or 2 arguments")
 	}
@@ -277,7 +262,7 @@ func matchStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
 	}
 
 	for _, child := range node.Children {
-		step, err := StepFromCfg(child)
+		step, err := StepFromCfg(globals, child)
 		if err != nil {
 			return nil, err
 		}
@@ -381,7 +366,7 @@ func passThroughPipeline(steps []SMTPPipelineStep, ctx *module.DeliveryContext,
 	return msg, true, nil
 }
 
-func destinationStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
+func destinationStepFromCfg(globals map[string]interface{}, node config.Node) (SMTPPipelineStep, error) {
 	if len(node.Args) == 0 {
 		return nil, errors.New("required at least one condition")
 	}
@@ -402,7 +387,7 @@ func destinationStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
 	}
 
 	for _, child := range node.Children {
-		substep, err := StepFromCfg(child)
+		substep, err := StepFromCfg(globals, child)
 		if err != nil {
 			return nil, err
 		}
@@ -412,16 +397,16 @@ func destinationStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
 	return &step, nil
 }
 
-func StepFromCfg(node config.Node) (SMTPPipelineStep, error) {
+func StepFromCfg(globals map[string]interface{}, node config.Node) (SMTPPipelineStep, error) {
 	switch node.Name {
 	case "filter":
-		return filterStepFromCfg(node)
+		return filterStepFromCfg(globals, &node)
 	case "deliver":
-		return deliverStepFromCfg(node)
+		return deliverStepFromCfg(globals, &node)
 	case "match":
-		return matchStepFromCfg(node)
+		return matchStepFromCfg(globals, node)
 	case "destination":
-		return destinationStepFromCfg(node)
+		return destinationStepFromCfg(globals, node)
 	case "stop":
 		return stopStepFromCfg(node)
 	case "require_auth":