aliases: Simplify lookup logic, remove alias-exists hook

This patch simplifies the internal alias lookup logic, unifying it
across Resolve and Exists.

As part of this, the `alias-exists` hook is removed. It was redundant to
begin with, although it enabled a potential optimization, it isn't worth
the complexity. The timeout for execution of both was the same.

This change should be backwards-compatible because `alias-resolve` is
still used, and the semantics haven't changed.

docs/aliases.md

@@ -85,11 +85,11 @@ aliases.
 
 ## Hooks
 
-There are two hooks that allow more sophisticated aliases resolution:
-`alias-exists` and `alias-resolve`.
+There is a hook that allows more sophisticated aliases resolution:
+`alias-resolve`.
 
-If they exist, they are invoked as part of the resolution process and the
-results are merged with the file-based resolution results.
+If it exists, it is invoked as part of the resolution process, and the results
+are merged with the file-based resolution results.
 
 See the [hooks](hooks.md) documentation for more details.
 

docs/hooks.md

@@ -73,17 +73,3 @@ without emitting any output.
 
 There is a 5 second timeout for hook execution. If the hook exits with an
 error, including timeout, delivery will fail.
-
-
-## Alias exists hook
-
-When chasquid needs to check whether an alias exists or not, it will run the
-command at `$config_dir/hooks/alias-exists` (if the file exists).
-
-The address to check will be passed as the single argument.
-
-If the commands exits successfuly (exit code 0), then the alias exists; any
-other exit code signals that the alias does not exist.
-
-There is a 5 second timeout for hook execution. If the hook times out, the
-alias will be assumed not to exist.

internal/aliases/aliases.go

@@ -112,8 +112,7 @@ type Resolver struct {
 	// Characters to drop from the user part.
 	DropChars string
 
-	// Path to resolve and exist hooks.
-	ExistsHook  string
+	// Path to the resolve hook.
 	ResolveHook string
 
 	// Map of domain -> alias files for that domain.
@@ -148,16 +147,23 @@ func (v *Resolver) Resolve(addr string) ([]Recipient, error) {
 // The clean address can be used to look it up in other databases, even if it
 // doesn't exist.
 func (v *Resolver) Exists(addr string) (string, bool) {
-	v.mu.Lock()
 	addr = v.cleanIfLocal(addr)
-	_, ok := v.aliases[addr]
+
+	rcpts, _ := v.lookup(addr)
+	return addr, len(rcpts) > 0
+}
+
+func (v *Resolver) lookup(addr string) ([]Recipient, error) {
+	v.mu.Lock()
+	rcpts := v.aliases[addr]
 	v.mu.Unlock()
 
-	if ok {
-		return addr, true
+	// Augment with the hook results.
+	hr, err := v.runResolveHook(addr)
+	if err != nil {
+		return nil, err
 	}
-
-	return addr, v.runExistsHook(addr)
+	return append(rcpts, hr...), nil
 }
 
 func (v *Resolver) resolve(rcount int, addr string) ([]Recipient, error) {
@@ -171,16 +177,10 @@ func (v *Resolver) resolve(rcount int, addr string) ([]Recipient, error) {
 	addr = v.cleanIfLocal(addr)
 
 	// Lookup in the aliases database.
-	v.mu.Lock()
-	rcpts := v.aliases[addr]
-	v.mu.Unlock()
-
-	// Augment with the hook results.
-	hr, err := v.runResolveHook(addr)
+	rcpts, err := v.lookup(addr)
 	if err != nil {
 		return nil, err
 	}
-	rcpts = append(rcpts, hr...)
 
 	if len(rcpts) == 0 {
 		return []Recipient{{addr, EMAIL}}, nil
@@ -435,35 +435,3 @@ func (v *Resolver) runResolveHook(addr string) ([]Recipient, error) {
 	hookResults.Add("resolve:success", 1)
 	return rs, nil
 }
-
-func (v *Resolver) runExistsHook(addr string) bool {
-	if v.ExistsHook == "" {
-		hookResults.Add("exists:notset", 1)
-		return false
-	}
-	// TODO: check if the file is executable.
-	if _, err := os.Stat(v.ExistsHook); os.IsNotExist(err) {
-		hookResults.Add("exists:skip", 1)
-		return false
-	}
-
-	// TODO: this should be done via a context propagated all the way through.
-	tr := trace.New("Hook.Alias-Exists", addr)
-	defer tr.Finish()
-
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-	defer cancel()
-	cmd := exec.CommandContext(ctx, v.ExistsHook, addr)
-
-	out, err := cmd.CombinedOutput()
-	tr.Debugf("output: %q", string(out))
-	if err != nil {
-		tr.Debugf("not exists: %v", err)
-		hookResults.Add("exists:false", 1)
-		return false
-	}
-
-	tr.Debugf("exists")
-	hookResults.Add("exists:true", 1)
-	return true
-}

internal/smtpsrv/server.go

@@ -134,7 +134,6 @@ func (s *Server) SetAliasesConfig(suffixSep, dropChars string) {
 	s.aliasesR.SuffixSep = suffixSep
 	s.aliasesR.DropChars = dropChars
 	s.aliasesR.ResolveHook = path.Join(s.HookPath, "alias-resolve")
-	s.aliasesR.ExistsHook = path.Join(s.HookPath, "alias-exists")
 }
 
 // InitDomainInfo initializes the domain info database.

test/t-04-aliases/alias-exists-hook

@@ -1,15 +0,0 @@
-#!/bin/bash
-
-case "$1" in
-"vicuña@testserver")
-	exit 0
-	;;
-"ñandú@testserver")
-	exit 0
-	;;
-"roto@testserver")
-	exit 0
-	;;
-esac
-
-exit 1

test/t-04-aliases/run.sh

@@ -25,7 +25,6 @@ function send_and_check() {
 
 # Remove the hooks that could be left over from previous failed tests.
 rm -f config/hooks/alias-resolve
-rm -f config/hooks/alias-exists
 
 # Test email aliases.
 send_and_check pepe jose
@@ -46,7 +45,6 @@ mail_diff content .data/pipe_alias_worked
 
 # Set up the hooks.
 mkdir -p config/hooks/
-cp alias-exists-hook config/hooks/alias-exists
 cp alias-resolve-hook config/hooks/alias-resolve
 
 # Test email aliases.
@@ -70,6 +68,5 @@ fi
 
 # Remove the hooks, leave a clean state.
 rm -f config/hooks/alias-resolve
-rm -f config/hooks/alias-exists
 
 success