From eac939e9a73cce3878f6e142d128580223107e5a Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 8 Feb 2019 12:25:01 -0700 Subject: [PATCH 1/5] caddytls: Change clustering to be a plugin to the caddytls package Should resolve the failure in https://github.com/coredns/coredns/pull/2541. This change is breaking to clustering plugin developers (not Caddy users), but logical, since only the caddytls package uses CertMagic directly (the httpserver package also uses it, but only because it also uses the caddytls plugin); and it is early enough that no clustering plugins really exist yet. This will also require a change of devportal so that it looks for a different registration function, which has moved to the caddytls package. --- caddy.go | 22 ---------------------- caddytls/setup.go | 26 +++++++++++++++++++++++++- caddytls/tls.go | 16 ++++++++++++++++ plugins.go | 21 --------------------- 4 files changed, 41 insertions(+), 44 deletions(-) diff --git a/caddy.go b/caddy.go index b29a0280..c1f9ecae 100644 --- a/caddy.go +++ b/caddy.go @@ -41,12 +41,10 @@ import ( "strconv" "strings" "sync" - "sync/atomic" "time" "github.com/mholt/caddy/caddyfile" "github.com/mholt/caddy/telemetry" - "github.com/mholt/certmagic" ) // Configurable application parameters @@ -472,26 +470,6 @@ func (i *Instance) Caddyfile() Input { // // This function blocks until all the servers are listening. func Start(cdyfile Input) (*Instance, error) { - // set up the clustering plugin, if there is one (and there should - // always be one) -- this should be done exactly once, but we can't - // do it during init while plugins are still registering, so do it - // when starting the first instance) - if atomic.CompareAndSwapInt32(&clusterPluginSetup, 0, 1) { - clusterPluginName := os.Getenv("CADDY_CLUSTERING") - if clusterPluginName == "" { - clusterPluginName = "file" // name of default storage plugin as registered in caddytls package - } - clusterFn, ok := clusterProviders[clusterPluginName] - if !ok { - return nil, fmt.Errorf("unrecognized cluster plugin (was it included in the Caddy build?): %s", clusterPluginName) - } - storage, err := clusterFn() - if err != nil { - return nil, fmt.Errorf("constructing cluster plugin %s: %v", clusterPluginName, err) - } - certmagic.DefaultStorage = storage - } - inst := &Instance{serverType: cdyfile.ServerType(), wg: new(sync.WaitGroup), Storage: make(map[interface{}]interface{})} err := startWithListenerFds(cdyfile, inst, nil) if err != nil { diff --git a/caddytls/setup.go b/caddytls/setup.go index 9ac89a07..e7673238 100644 --- a/caddytls/setup.go +++ b/caddytls/setup.go @@ -26,6 +26,7 @@ import ( "path/filepath" "strconv" "strings" + "sync/atomic" "github.com/mholt/caddy" "github.com/mholt/caddy/telemetry" @@ -36,13 +37,34 @@ func init() { caddy.RegisterPlugin("tls", caddy.Plugin{Action: setupTLS}) // ensure the default Storage implementation is plugged in - caddy.RegisterClusterPlugin("file", constructDefaultClusterPlugin) + RegisterClusterPlugin("file", constructDefaultClusterPlugin) } // setupTLS sets up the TLS configuration and installs certificates that // are specified by the user in the config file. All the automatic HTTPS // stuff comes later outside of this function. func setupTLS(c *caddy.Controller) error { + // set up the clustering plugin, if there is one (and there should always + // be one since this tls plugin requires it) -- this should be done exactly + // once, but we can't do it during init while plugins are still registering, + // so do it as soon as we run a setup) + if atomic.CompareAndSwapInt32(&clusterPluginSetup, 0, 1) { + clusterPluginName := os.Getenv("CADDY_CLUSTERING") + if clusterPluginName == "" { + clusterPluginName = "file" // name of default storage plugin + } + clusterFn, ok := clusterProviders[clusterPluginName] + if ok { + storage, err := clusterFn() + if err != nil { + return fmt.Errorf("constructing cluster plugin %s: %v", clusterPluginName, err) + } + certmagic.DefaultStorage = storage + } else { + return fmt.Errorf("unrecognized cluster plugin (was it included in the Caddy build?): %s", clusterPluginName) + } + } + configGetter, ok := configGetters[c.ServerType()] if !ok { return fmt.Errorf("no caddytls.ConfigGetter for %s server type; must call RegisterConfigGetter", c.ServerType()) @@ -423,3 +445,5 @@ func loadCertsInDir(cfg *Config, c *caddy.Controller, dir string) error { func constructDefaultClusterPlugin() (certmagic.Storage, error) { return &certmagic.FileStorage{Path: caddy.AssetsPath()}, nil } + +var clusterPluginSetup int32 // access atomically diff --git a/caddytls/tls.go b/caddytls/tls.go index 9a9d5c63..f6a375b7 100644 --- a/caddytls/tls.go +++ b/caddytls/tls.go @@ -108,3 +108,19 @@ func RegisterDNSProvider(name string, provider DNSProviderConstructor) { dnsProviders[name] = provider caddy.RegisterPlugin("tls.dns."+name, caddy.Plugin{}) } + +// ClusterPluginConstructor is a function type that is used to +// instantiate a new implementation of both certmagic.Storage +// and certmagic.Locker, which are required for successful +// use in cluster environments. +type ClusterPluginConstructor func() (certmagic.Storage, error) + +// clusterProviders is the list of storage providers +var clusterProviders = make(map[string]ClusterPluginConstructor) + +// RegisterClusterPlugin registers provider by name for facilitating +// cluster-wide operations like storage and synchronization. +func RegisterClusterPlugin(name string, provider ClusterPluginConstructor) { + clusterProviders[name] = provider + caddy.RegisterPlugin("tls.cluster."+name, caddy.Plugin{}) +} diff --git a/plugins.go b/plugins.go index 56f3679a..cd4497fa 100644 --- a/plugins.go +++ b/plugins.go @@ -22,7 +22,6 @@ import ( "sync" "github.com/mholt/caddy/caddyfile" - "github.com/mholt/certmagic" ) // These are all the registered plugins. @@ -107,11 +106,6 @@ func ListPlugins() map[string][]string { p["caddyfile_loaders"] = append(p["caddyfile_loaders"], defaultCaddyfileLoader.name) } - // cluster plugins in registration order - for name := range clusterProviders { - p["clustering"] = append(p["clustering"], name) - } - // List the event hook plugins eventHooks.Range(func(k, _ interface{}) bool { p["event_hooks"] = append(p["event_hooks"], k.(string)) @@ -456,21 +450,6 @@ func loadCaddyfileInput(serverType string) (Input, error) { return caddyfileToUse, nil } -// ClusterPluginConstructor is a function type that is used to -// instantiate a new implementation of both certmagic.Storage -// and certmagic.Locker, which are required for successful -// use in cluster environments. -type ClusterPluginConstructor func() (certmagic.Storage, error) - -// clusterProviders is the list of storage providers -var clusterProviders = make(map[string]ClusterPluginConstructor) - -// RegisterClusterPlugin registers provider by name for facilitating -// cluster-wide operations like storage and synchronization. -func RegisterClusterPlugin(name string, provider ClusterPluginConstructor) { - clusterProviders[name] = provider -} - // OnProcessExit is a list of functions to run when the process // exits -- they are ONLY for cleanup and should not block, // return errors, or do anything fancy. They will be run with From 7d737427a930fa04b0690269b05e024f89ed135c Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 8 Feb 2019 12:28:27 -0700 Subject: [PATCH 2/5] Remove unused variable --- caddy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/caddy.go b/caddy.go index c1f9ecae..44fc1a2d 100644 --- a/caddy.go +++ b/caddy.go @@ -999,7 +999,6 @@ var ( DefaultConfigFile = "Caddyfile" ) -var clusterPluginSetup int32 // access atomically // CtxKey is a value type for use with context.WithValue. type CtxKey string From 59e7a8864ad9f0709a6874afbc5fa56ec5d5dff6 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 8 Feb 2019 12:43:20 -0700 Subject: [PATCH 3/5] caddyhttp: Fix test (adjust plugin counting) --- caddyhttp/caddyhttp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/caddyhttp/caddyhttp_test.go b/caddyhttp/caddyhttp_test.go index 4e759b54..47b084a7 100644 --- a/caddyhttp/caddyhttp_test.go +++ b/caddyhttp/caddyhttp_test.go @@ -25,9 +25,9 @@ import ( // ensure that the standard plugins are in fact plugged in // and registered properly; this is a quick/naive way to do it. func TestStandardPlugins(t *testing.T) { - numStandardPlugins := 31 // importing caddyhttp plugs in this many plugins + numStandardPlugins := 32 // importing caddyhttp plugs in this many plugins s := caddy.DescribePlugins() - if got, want := strings.Count(s, "\n"), numStandardPlugins+7; got != want { + if got, want := strings.Count(s, "\n"), numStandardPlugins+4; got != want { t.Errorf("Expected all standard plugins to be plugged in, got:\n%s", s) } } From 22db8bcf3d32b2a2f84134bcb98c80435484afa2 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 8 Feb 2019 12:56:51 -0700 Subject: [PATCH 4/5] ummmm, remove extra line break somehow VS Code didn't fmt on save... weird. --- caddy.go | 1 - 1 file changed, 1 deletion(-) diff --git a/caddy.go b/caddy.go index 44fc1a2d..1363a1b1 100644 --- a/caddy.go +++ b/caddy.go @@ -999,6 +999,5 @@ var ( DefaultConfigFile = "Caddyfile" ) - // CtxKey is a value type for use with context.WithValue. type CtxKey string From 0a95b5d359195dc462458f63d98d0bdb112b9f73 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 14 Feb 2019 17:20:06 -0700 Subject: [PATCH 5/5] caddytls: Move config of certmagic storage to NewConfig (fixes #2465) Breaking API change for server type plugins that use caddytls package. Now an error value is returned from NewConfig as well. Sorry about that. --- caddyhttp/httpserver/plugin.go | 5 ++++- caddytls/config.go | 28 ++++++++++++++++++++++++++-- caddytls/setup.go | 24 ------------------------ 3 files changed, 30 insertions(+), 27 deletions(-) diff --git a/caddyhttp/httpserver/plugin.go b/caddyhttp/httpserver/plugin.go index a62baf5d..2e1fa5e6 100644 --- a/caddyhttp/httpserver/plugin.go +++ b/caddyhttp/httpserver/plugin.go @@ -190,7 +190,10 @@ func (h *httpContext) InspectServerBlocks(sourceFile string, serverBlocks []cadd // Make our caddytls.Config, which has a pointer to the // instance's certificate cache and enough information // to use automatic HTTPS when the time comes - caddytlsConfig := caddytls.NewConfig(h.instance) + caddytlsConfig, err := caddytls.NewConfig(h.instance) + if err != nil { + return nil, fmt.Errorf("creating new caddytls configuration: %v", err) + } caddytlsConfig.Hostname = addr.Host caddytlsConfig.Manager.AltHTTPPort = altHTTPPort caddytlsConfig.Manager.AltTLSALPNPort = altTLSALPNPort diff --git a/caddytls/config.go b/caddytls/config.go index c8318b92..214eb73b 100644 --- a/caddytls/config.go +++ b/caddytls/config.go @@ -19,6 +19,8 @@ import ( "crypto/x509" "fmt" "io/ioutil" + "os" + "sync/atomic" "github.com/xenolf/lego/challenge/tlsalpn01" @@ -95,11 +97,31 @@ type Config struct { // NewConfig returns a new Config with a pointer to the instance's // certificate cache. You will usually need to set other fields on // the returned Config for successful practical use. -func NewConfig(inst *caddy.Instance) *Config { +func NewConfig(inst *caddy.Instance) (*Config, error) { inst.StorageMu.RLock() certCache, ok := inst.Storage[CertCacheInstStorageKey].(*certmagic.Cache) inst.StorageMu.RUnlock() if !ok || certCache == nil { + // set up the clustering plugin, if there is one (and there should always + // be one since this tls plugin requires it) -- this should be done exactly + // once, but we can't do it during init while plugins are still registering, + // so do it as soon as we run a setup) + if atomic.CompareAndSwapInt32(&clusterPluginSetup, 0, 1) { + clusterPluginName := os.Getenv("CADDY_CLUSTERING") + if clusterPluginName == "" { + clusterPluginName = "file" // name of default storage plugin + } + clusterFn, ok := clusterProviders[clusterPluginName] + if ok { + storage, err := clusterFn() + if err != nil { + return nil, fmt.Errorf("constructing cluster plugin %s: %v", clusterPluginName, err) + } + certmagic.DefaultStorage = storage + } else { + return nil, fmt.Errorf("unrecognized cluster plugin (was it included in the Caddy build?): %s", clusterPluginName) + } + } certCache = certmagic.NewCache(certmagic.DefaultStorage) inst.OnShutdown = append(inst.OnShutdown, func() error { certCache.Stop() @@ -111,7 +133,7 @@ func NewConfig(inst *caddy.Instance) *Config { } return &Config{ Manager: certmagic.NewWithCache(certCache, certmagic.Config{}), - } + }, nil } // buildStandardTLSConfig converts cfg (*caddytls.Config) to a *tls.Config @@ -519,6 +541,8 @@ var defaultCurves = []tls.CurveID{ tls.CurveP256, } +var clusterPluginSetup int32 // access atomically + // CertCacheInstStorageKey is the name of the key for // accessing the certificate storage on the *caddy.Instance. const CertCacheInstStorageKey = "tls_cert_cache" diff --git a/caddytls/setup.go b/caddytls/setup.go index e7673238..b8f1af3a 100644 --- a/caddytls/setup.go +++ b/caddytls/setup.go @@ -26,7 +26,6 @@ import ( "path/filepath" "strconv" "strings" - "sync/atomic" "github.com/mholt/caddy" "github.com/mholt/caddy/telemetry" @@ -44,27 +43,6 @@ func init() { // are specified by the user in the config file. All the automatic HTTPS // stuff comes later outside of this function. func setupTLS(c *caddy.Controller) error { - // set up the clustering plugin, if there is one (and there should always - // be one since this tls plugin requires it) -- this should be done exactly - // once, but we can't do it during init while plugins are still registering, - // so do it as soon as we run a setup) - if atomic.CompareAndSwapInt32(&clusterPluginSetup, 0, 1) { - clusterPluginName := os.Getenv("CADDY_CLUSTERING") - if clusterPluginName == "" { - clusterPluginName = "file" // name of default storage plugin - } - clusterFn, ok := clusterProviders[clusterPluginName] - if ok { - storage, err := clusterFn() - if err != nil { - return fmt.Errorf("constructing cluster plugin %s: %v", clusterPluginName, err) - } - certmagic.DefaultStorage = storage - } else { - return fmt.Errorf("unrecognized cluster plugin (was it included in the Caddy build?): %s", clusterPluginName) - } - } - configGetter, ok := configGetters[c.ServerType()] if !ok { return fmt.Errorf("no caddytls.ConfigGetter for %s server type; must call RegisterConfigGetter", c.ServerType()) @@ -445,5 +423,3 @@ func loadCertsInDir(cfg *Config, c *caddy.Controller, dir string) error { func constructDefaultClusterPlugin() (certmagic.Storage, error) { return &certmagic.FileStorage{Path: caddy.AssetsPath()}, nil } - -var clusterPluginSetup int32 // access atomically