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)
-	if err != nil {
-		return nil, err
+// 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)
+		}
 	}
 
-	provider, ok := modObj.(module.AuthProvider)
-	if !ok {
-		return nil, fmt.Errorf("module %s doesn't implements auth. provider interface", modObj.Name())
+	newMod := module.Get(modName)
+	if newMod == nil {
+		return nil, fmt.Errorf("unknown module: %s", modName)
 	}
-	return provider, nil
+
+	log.Debugln("module create", modName, instName, "(inline)")
+
+	return newMod(modName, instName)
 }
 
-func authDirective(m *config.Map, node *config.Node) (interface{}, error) {
-	if len(node.Args) != 1 {
-		return nil, m.MatchErr("expected 1 argument")
+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")
 	}
-	if len(node.Children) != 0 {
-		return nil, m.MatchErr("can't declare block here")
+
+	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())
+		}
 	}
 
-	modObj, err := authProvider(node.Args[0])
-	if err != nil {
-		return nil, m.MatchErr("%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 modObj, nil
+	return target, nil
 }
 
-func storageBackend(modName string) (module.Storage, error) {
-	modObj, err := module.GetInstance(modName)
-	if err != nil {
-		return nil, err
+func authDirective(m *config.Map, node *config.Node) (interface{}, error) {
+	if len(node.Args) == 0 {
+		return nil, m.MatchErr("expected at least 1 argument")
 	}
 
-	backend, ok := modObj.(module.Storage)
-	if !ok {
-		return nil, fmt.Errorf("module %s doesn't implements storage interface", modObj.Name())
+	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())
+		}
 	}
-	return backend, nil
-}
 
-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")
+	provider, ok := modObj.(module.AuthProvider)
+	if !ok {
+		return nil, m.MatchErr("module %s doesn't implement auth. provider interface", modObj.Name())
 	}
 
-	modObj, err := storageBackend(node.Args[0])
-	if err != nil {
-		return nil, m.MatchErr("%s", err.Error())
+	if node.Children != nil {
+		if err := initInlineModule(modObj, m.Globals, node); err != nil {
+			return nil, err
+		}
 	}
 
-	return modObj, nil
+	return provider, nil
 }
 
-func deliveryTarget(modName string) (module.DeliveryTarget, error) {
-	mod, err := module.GetInstance(modName)
-	if mod == nil {
-		return nil, err
+func storageDirective(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())
+		}
 	}
 
-	target, ok := mod.(module.DeliveryTarget)
+	backend, ok := modObj.(module.Storage)
 	if !ok {
-		return nil, fmt.Errorf("module %s doesn't implements delivery target interface", mod.Name())
+		return nil, m.MatchErr("module %s doesn't implement storage interface", modObj.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")
+	if node.Children != nil {
+		if err := initInlineModule(modObj, m.Globals, node); err != nil {
+			return nil, err
+		}
 	}
 
-	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
 		}