core: Run startup/shutdown functions only once

Even if defined for multiple hosts. Startup or shutdown callbacks registered by any directive (startup, shutdown, markdown, git, log, etc.) will only run as many times as it appears in the Caddyfile, not repeated for each host that shares that server block. Fixing this involved refactoring three packages (yeesh) and we need to restore some tests that are no longer valid (that used to verify splitting a multiServerBlock into multiple serverBlocks).

config/config.go

@@ -23,7 +23,9 @@ const (
 	DefaultConfigFile = "Caddyfile"
 )
 
-func Load(filename string, input io.Reader) ([]server.Config, error) {
+// Load reads input (named filename) and parses it, returning server
+// configurations grouped by listening address.
+func Load(filename string, input io.Reader) (Group, error) {
 	var configs []server.Config
 
 	// turn off timestamp for parsing
@@ -32,69 +34,91 @@ func Load(filename string, input io.Reader) ([]server.Config, error) {
 
 	serverBlocks, err := parse.ServerBlocks(filename, input)
 	if err != nil {
-		return configs, err
+		return nil, err
 	}
 
-	// Each server block represents a single server/address.
+	// Each server block represents one or more servers/addresses.
 	// Iterate each server block and make a config for each one,
 	// executing the directives that were parsed.
 	for _, sb := range serverBlocks {
-		config := server.Config{
-			Host:       sb.Host,
-			Port:       sb.Port,
-			Root:       Root,
-			Middleware: make(map[string][]middleware.Middleware),
-			ConfigFile: filename,
-			AppName:    app.Name,
-			AppVersion: app.Version,
+		sharedConfig, err := serverBlockToConfig(filename, sb)
+		if err != nil {
+			return nil, err
 		}
 
-		// It is crucial that directives are executed in the proper order.
-		for _, dir := range directiveOrder {
-			// Execute directive if it is in the server block
-			if tokens, ok := sb.Tokens[dir.name]; ok {
-				// Each setup function gets a controller, which is the
-				// server config and the dispenser containing only
-				// this directive's tokens.
-				controller := &setup.Controller{
-					Config:    &config,
-					Dispenser: parse.NewDispenserTokens(filename, tokens),
-				}
-
-				midware, err := dir.setup(controller)
-				if err != nil {
-					return configs, err
-				}
-				if midware != nil {
-					// TODO: For now, we only support the default path scope /
-					config.Middleware["/"] = append(config.Middleware["/"], midware)
-				}
+		// Now share the config with as many hosts as share the server block
+		for i, addr := range sb.Addresses {
+			config := sharedConfig
+			config.Host = addr.Host
+			config.Port = addr.Port
+			if config.Port == "" {
+				config.Port = Port
 			}
+			if i == 0 {
+				sharedConfig.Startup = []func() error{}
+				sharedConfig.Shutdown = []func() error{}
+			}
+			configs = append(configs, config)
 		}
-
-		if config.Port == "" {
-			config.Port = Port
-		}
-
-		configs = append(configs, config)
 	}
 
 	// restore logging settings
 	log.SetFlags(flags)
 
-	return configs, nil
+	// Group by address/virtualhosts
+	return arrangeBindings(configs)
 }
 
-// ArrangeBindings groups configurations by their bind address. For example,
+// serverBlockToConfig makes a config for the server block
+// by executing the tokens that were parsed. The returned
+// config is shared among all hosts/addresses for the server
+// block, so Host and Port information is not filled out
+// here.
+func serverBlockToConfig(filename string, sb parse.ServerBlock) (server.Config, error) {
+	sharedConfig := server.Config{
+		Root:       Root,
+		Middleware: make(map[string][]middleware.Middleware),
+		ConfigFile: filename,
+		AppName:    app.Name,
+		AppVersion: app.Version,
+	}
+
+	// It is crucial that directives are executed in the proper order.
+	for _, dir := range directiveOrder {
+		// Execute directive if it is in the server block
+		if tokens, ok := sb.Tokens[dir.name]; ok {
+			// Each setup function gets a controller, which is the
+			// server config and the dispenser containing only
+			// this directive's tokens.
+			controller := &setup.Controller{
+				Config:    &sharedConfig,
+				Dispenser: parse.NewDispenserTokens(filename, tokens),
+			}
+
+			midware, err := dir.setup(controller)
+			if err != nil {
+				return sharedConfig, err
+			}
+			if midware != nil {
+				// TODO: For now, we only support the default path scope /
+				sharedConfig.Middleware["/"] = append(sharedConfig.Middleware["/"], midware)
+			}
+		}
+	}
+
+	return sharedConfig, nil
+}
+
+// arrangeBindings groups configurations by their bind address. For example,
 // a server that should listen on localhost and another on 127.0.0.1 will
 // be grouped into the same address: 127.0.0.1. It will return an error
 // if an address is malformed or a TLS listener is configured on the
 // same address as a plaintext HTTP listener. The return value is a map of
 // bind address to list of configs that would become VirtualHosts on that
 // server. Use the keys of the returned map to create listeners, and use
 // the associated values to set up the virtualhosts.
-func ArrangeBindings(allConfigs []server.Config) (map[*net.TCPAddr][]server.Config, error) {
-	addresses := make(map[*net.TCPAddr][]server.Config)
+func arrangeBindings(allConfigs []server.Config) (Group, error) {
+	addresses := make(Group)
 
 	// Group configs by bind address
 	for _, conf := range allConfigs {
@@ -206,20 +230,26 @@ func validDirective(d string) bool {
 	return false
 }
 
-// Default makes a default configuration which
-// is empty except for root, host, and port,
-// which are essentials for serving the cwd.
-func Default() server.Config {
+func NewDefault() server.Config {
 	return server.Config{
 		Root: Root,
 		Host: Host,
 		Port: Port,
 	}
 }
 
+// Default makes a default configuration which
+// is empty except for root, host, and port,
+// which are essentials for serving the cwd.
+func Default() (Group, error) {
+	return arrangeBindings([]server.Config{NewDefault()})
+}
+
 // These three defaults are configurable through the command line
 var (
 	Root = DefaultRoot
 	Host = DefaultHost
 	Port = DefaultPort
 )
+
+type Group map[*net.TCPAddr][]server.Config

config/config_test.go

@@ -6,7 +6,7 @@ import (
 	"github.com/mholt/caddy/server"
 )
 
-func TestReolveAddr(t *testing.T) {
+func TestResolveAddr(t *testing.T) {
 	// NOTE: If tests fail due to comparing to string "127.0.0.1",
 	// it's possible that system env resolves with IPv6, or ::1.
 	// If that happens, maybe we should use actualAddr.IP.IsLoopback()

config/parse/parse.go

@@ -6,7 +6,7 @@ import "io"
 // ServerBlocks parses the input just enough to organize tokens,
 // in order, by server block. No further parsing is performed.
 // Server blocks are returned in the order in which they appear.
-func ServerBlocks(filename string, input io.Reader) ([]serverBlock, error) {
+func ServerBlocks(filename string, input io.Reader) ([]ServerBlock, error) {
 	p := parser{Dispenser: NewDispenser(filename, input)}
 	blocks, err := p.parseAll()
 	return blocks, err

config/parse/parsing.go

@@ -9,34 +9,26 @@ import (
 
 type parser struct {
 	Dispenser
-	block multiServerBlock // current server block being parsed
-	eof   bool             // if we encounter a valid EOF in a hard place
+	block ServerBlock // current server block being parsed
+	eof   bool        // if we encounter a valid EOF in a hard place
 }
 
-func (p *parser) parseAll() ([]serverBlock, error) {
-	var blocks []serverBlock
+func (p *parser) parseAll() ([]ServerBlock, error) {
+	var blocks []ServerBlock
 
 	for p.Next() {
 		err := p.parseOne()
 		if err != nil {
 			return blocks, err
 		}
-
-		// explode the multiServerBlock into multiple serverBlocks
-		for _, addr := range p.block.addresses {
-			blocks = append(blocks, serverBlock{
-				Host:   addr.host,
-				Port:   addr.port,
-				Tokens: p.block.tokens,
-			})
-		}
+		blocks = append(blocks, p.block)
 	}
 
 	return blocks, nil
 }
 
 func (p *parser) parseOne() error {
-	p.block = multiServerBlock{tokens: make(map[string][]token)}
+	p.block = ServerBlock{Tokens: make(map[string][]token)}
 
 	err := p.begin()
 	if err != nil {
@@ -107,7 +99,7 @@ func (p *parser) addresses() error {
 		if err != nil {
 			return err
 		}
-		p.block.addresses = append(p.block.addresses, address{host, port})
+		p.block.Addresses = append(p.block.Addresses, Address{host, port})
 
 		// Advance token and possibly break out of loop or return error
 		hasNext := p.Next()
@@ -229,7 +221,7 @@ func (p *parser) directive() error {
 	}
 
 	// The directive itself is appended as a relevant token
-	p.block.tokens[dir] = append(p.block.tokens[dir], p.tokens[p.cursor])
+	p.block.Tokens[dir] = append(p.block.Tokens[dir], p.tokens[p.cursor])
 
 	for p.Next() {
 		if p.Val() == "{" {
@@ -242,7 +234,7 @@ func (p *parser) directive() error {
 		} else if p.Val() == "}" && nesting == 0 {
 			return p.Err("Unexpected '}' because no matching opening brace")
 		}
-		p.block.tokens[dir] = append(p.block.tokens[dir], p.tokens[p.cursor])
+		p.block.Tokens[dir] = append(p.block.Tokens[dir], p.tokens[p.cursor])
 	}
 
 	if nesting > 0 {
@@ -305,21 +297,15 @@ func standardAddress(str string) (host, port string, err error) {
 }
 
 type (
-	// serverBlock stores tokens by directive name for a
-	// single host:port (address)
-	serverBlock struct {
-		Host, Port string
-		Tokens     map[string][]token // directive name to tokens (including directive)
+	// ServerBlock associates tokens with a list of addresses
+	// and groups tokens by directive name.
+	ServerBlock struct {
+		Addresses []Address
+		Tokens    map[string][]token
 	}
 
-	// multiServerBlock is the same as serverBlock but for
-	// multiple addresses that share the same tokens
-	multiServerBlock struct {
-		addresses []address
-		tokens    map[string][]token
-	}
-
-	address struct {
-		host, port string
+	// Address represents a host and port.
+	Address struct {
+		Host, Port string
 	}
 )