httpcaddyfile: Support repeated use of cert_issuer global option

This changes the signature of UnmarshalGlobalFunc but this is probably OK since it's only used by this repo as far as we know.

We need this change in order to "remember" the previous value in case a global option appears more than once, which is now a possibility with the cert_issuer option since Caddy now supports multiple issuers in the order defined by the user.

Bonus: the issuer subdirective of tls now supports one-liner for "acme" when all you need to set is the directory:

issuer acme <dir>

5 files changed, 51 insertions, 40 deletions

diff --git a/caddyconfig/httpcaddyfile/directives.go b/caddyconfig/httpcaddyfile/directives.go
index 4ab8778..b79b811 100644
--- a/caddyconfig/httpcaddyfile/directives.go
+++ b/caddyconfig/httpcaddyfile/directives.go
@@ -498,9 +498,10 @@ type (
 	UnmarshalHandlerFunc func(h Helper) (caddyhttp.MiddlewareHandler, error)
 
 	// UnmarshalGlobalFunc is a function which can unmarshal Caddyfile
-	// tokens into a global option config value using a Helper type.
-	// These are passed in a call to RegisterGlobalOption.
-	UnmarshalGlobalFunc func(d *caddyfile.Dispenser) (interface{}, error)
+	// tokens from a global option. It is passed the tokens to parse and
+	// existing value from the previous instance of this global option
+	// (if any). It returns the value to associate with this global option.
+	UnmarshalGlobalFunc func(d *caddyfile.Dispenser, existingVal interface{}) (interface{}, error)
 )
 
 var registeredDirectives = make(map[string]UnmarshalFunc)
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index 406d8b9..f16a0df 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -315,7 +315,7 @@ func (ServerType) evaluateGlobalOptionsBlock(serverBlocks []serverBlock, options
 			return nil, fmt.Errorf("%s:%d: unrecognized global option: %s", tkn.File, tkn.Line, opt)
 		}
 
-		val, err = optFunc(disp)
+		val, err = optFunc(disp, options[opt])
 		if err != nil {
 			return nil, fmt.Errorf("parsing caddyfile tokens for '%s': %v", opt, err)
 		}
diff --git a/caddyconfig/httpcaddyfile/options.go b/caddyconfig/httpcaddyfile/options.go
index 3a3cdf6..119295b 100644
--- a/caddyconfig/httpcaddyfile/options.go
+++ b/caddyconfig/httpcaddyfile/options.go
@@ -30,7 +30,6 @@ func init() {
 	RegisterGlobalOption("https_port", parseOptHTTPSPort)
 	RegisterGlobalOption("default_sni", parseOptSingleString)
 	RegisterGlobalOption("order", parseOptOrder)
-	RegisterGlobalOption("experimental_http3", parseOptTrue)
 	RegisterGlobalOption("storage", parseOptStorage)
 	RegisterGlobalOption("acme_ca", parseOptSingleString)
 	RegisterGlobalOption("acme_ca_root", parseOptSingleString)
@@ -46,11 +45,9 @@ func init() {
 	RegisterGlobalOption("servers", parseServerOptions)
 }
 
-func parseOptTrue(d *caddyfile.Dispenser) (interface{}, error) {
-	return true, nil
-}
+func parseOptTrue(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) { return true, nil }
 
-func parseOptHTTPPort(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptHTTPPort(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	var httpPort int
 	for d.Next() {
 		var httpPortStr string
@@ -66,7 +63,7 @@ func parseOptHTTPPort(d *caddyfile.Dispenser) (interface{}, error) {
 	return httpPort, nil
 }
 
-func parseOptHTTPSPort(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptHTTPSPort(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	var httpsPort int
 	for d.Next() {
 		var httpsPortStr string
@@ -82,7 +79,7 @@ func parseOptHTTPSPort(d *caddyfile.Dispenser) (interface{}, error) {
 	return httpsPort, nil
 }
 
-func parseOptOrder(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptOrder(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	newOrder := directiveOrder
 
 	for d.Next() {
@@ -158,7 +155,7 @@ func parseOptOrder(d *caddyfile.Dispenser) (interface{}, error) {
 	return newOrder, nil
 }
 
-func parseOptStorage(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptStorage(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	if !d.Next() { // consume option name
 		return nil, d.ArgErr()
 	}
@@ -177,7 +174,7 @@ func parseOptStorage(d *caddyfile.Dispenser) (interface{}, error) {
 	return storage, nil
 }
 
-func parseOptACMEDNS(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptACMEDNS(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	if !d.Next() { // consume option name
 		return nil, d.ArgErr()
 	}
@@ -196,7 +193,7 @@ func parseOptACMEDNS(d *caddyfile.Dispenser) (interface{}, error) {
 	return prov, nil
 }
 
-func parseOptACMEEAB(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptACMEEAB(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	eab := new(acme.EAB)
 	for d.Next() {
 		if d.NextArg() {
@@ -224,26 +221,30 @@ func parseOptACMEEAB(d *caddyfile.Dispenser) (interface{}, error) {
 	return eab, nil
 }
 
-func parseOptCertIssuer(d *caddyfile.Dispenser) (interface{}, error) {
-	if !d.Next() { // consume option name
-		return nil, d.ArgErr()
-	}
-	if !d.Next() { // get issuer module name
-		return nil, d.ArgErr()
+func parseOptCertIssuer(d *caddyfile.Dispenser, existing interface{}) (interface{}, error) {
+	var issuers []certmagic.Issuer
+	if existing != nil {
+		issuers = existing.([]certmagic.Issuer)
 	}
-	modID := "tls.issuance." + d.Val()
-	unm, err := caddyfile.UnmarshalModule(d, modID)
-	if err != nil {
-		return nil, err
-	}
-	iss, ok := unm.(certmagic.Issuer)
-	if !ok {
-		return nil, d.Errf("module %s (%T) is not a certmagic.Issuer", modID, unm)
+	for d.Next() { // consume option name
+		if !d.Next() { // get issuer module name
+			return nil, d.ArgErr()
+		}
+		modID := "tls.issuance." + d.Val()
+		unm, err := caddyfile.UnmarshalModule(d, modID)
+		if err != nil {
+			return nil, err
+		}
+		iss, ok := unm.(certmagic.Issuer)
+		if !ok {
+			return nil, d.Errf("module %s (%T) is not a certmagic.Issuer", modID, unm)
+		}
+		issuers = append(issuers, iss)
 	}
-	return iss, nil
+	return issuers, nil
 }
 
-func parseOptSingleString(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptSingleString(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	d.Next() // consume parameter name
 	if !d.Next() {
 		return "", d.ArgErr()
@@ -255,7 +256,7 @@ func parseOptSingleString(d *caddyfile.Dispenser) (interface{}, error) {
 	return val, nil
 }
 
-func parseOptAdmin(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptAdmin(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	adminCfg := new(caddy.AdminConfig)
 	for d.Next() {
 		if d.NextArg() {
@@ -291,7 +292,7 @@ func parseOptAdmin(d *caddyfile.Dispenser) (interface{}, error) {
 	return adminCfg, nil
 }
 
-func parseOptOnDemand(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptOnDemand(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	var ond *caddytls.OnDemandConfig
 	for d.Next() {
 		if d.NextArg() {
@@ -351,7 +352,7 @@ func parseOptOnDemand(d *caddyfile.Dispenser) (interface{}, error) {
 	return ond, nil
 }
 
-func parseOptAutoHTTPS(d *caddyfile.Dispenser) (interface{}, error) {
+func parseOptAutoHTTPS(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	d.Next() // consume parameter name
 	if !d.Next() {
 		return "", d.ArgErr()
@@ -366,6 +367,6 @@ func parseOptAutoHTTPS(d *caddyfile.Dispenser) (interface{}, error) {
 	return val, nil
 }
 
-func parseServerOptions(d *caddyfile.Dispenser) (interface{}, error) {
+func parseServerOptions(d *caddyfile.Dispenser, _ interface{}) (interface{}, error) {
 	return unmarshalCaddyfileServerOptions(d)
 }
diff --git a/caddyconfig/httpcaddyfile/tlsapp.go b/caddyconfig/httpcaddyfile/tlsapp.go
index a66146d..25b800a 100644
--- a/caddyconfig/httpcaddyfile/tlsapp.go
+++ b/caddyconfig/httpcaddyfile/tlsapp.go
@@ -414,11 +414,11 @@ func fillInGlobalACMEDefaults(issuer certmagic.Issuer, options map[string]interf
 // returned if there are no default/global options. However, if always is
 // true, a non-nil value will always be returned (unless there is an error).
 func newBaseAutomationPolicy(options map[string]interface{}, warnings []caddyconfig.Warning, always bool) (*caddytls.AutomationPolicy, error) {
-	issuer, hasIssuer := options["cert_issuer"]
+	issuers, hasIssuers := options["cert_issuer"]
 	_, hasLocalCerts := options["local_certs"]
 	keyType, hasKeyType := options["key_type"]
 
-	hasGlobalAutomationOpts := hasIssuer || hasLocalCerts || hasKeyType
+	hasGlobalAutomationOpts := hasIssuers || hasLocalCerts || hasKeyType
 
 	// if there are no global options related to automation policies
 	// set, then we can just return right away
@@ -434,12 +434,12 @@ func newBaseAutomationPolicy(options map[string]interface{}, warnings []caddycon
 		ap.KeyType = keyType.(string)
 	}
 
-	if hasIssuer && hasLocalCerts {
+	if hasIssuers && hasLocalCerts {
 		return nil, fmt.Errorf("global options are ambiguous: local_certs is confusing when combined with cert_issuer, because local_certs is also a specific kind of issuer")
 	}
 
-	if hasIssuer {
-		ap.Issuers = []certmagic.Issuer{issuer.(certmagic.Issuer)}
+	if hasIssuers {
+		ap.Issuers = issuers.([]certmagic.Issuer)
 	} else if hasLocalCerts {
 		ap.Issuers = []certmagic.Issuer{new(caddytls.InternalIssuer)}
 	}
diff --git a/modules/caddytls/acmeissuer.go b/modules/caddytls/acmeissuer.go
index 43e758f..64725c9 100644
--- a/modules/caddytls/acmeissuer.go
+++ b/modules/caddytls/acmeissuer.go
@@ -233,7 +233,7 @@ func (iss *ACMEIssuer) GetACMEIssuer() *ACMEIssuer { return iss }
 
 // UnmarshalCaddyfile deserializes Caddyfile tokens into iss.
 //
-//     ... acme {
+//     ... acme [<directory_url>] {
 //         dir <directory_url>
 //         test_dir <test_directory_url>
 //         email <email>
@@ -250,9 +250,18 @@ func (iss *ACMEIssuer) GetACMEIssuer() *ACMEIssuer { return iss }
 //
 func (iss *ACMEIssuer) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 	for d.Next() {
+		if d.NextArg() {
+			iss.CA = d.Val()
+			if d.NextArg() {
+				return d.ArgErr()
+			}
+		}
 		for nesting := d.Nesting(); d.NextBlock(nesting); {
 			switch d.Val() {
 			case "dir":
+				if iss.CA != "" {
+					return d.Errf("directory is already specified: %s", iss.CA)
+				}
 				if !d.AllArgs(&iss.CA) {
 					return d.ArgErr()
 				}