core: hide registerHandler (#964)

* core: hide registerHandler

Remove RegisterHandler and just make it implicit when we look at the
handler compilation step.

* Rename GetHandler to just Handler

Update callers and make auto check Hander in OnStartup.

* Up test coverage in erratic

* up test coverage
This commit is contained in:
Miek Gieben
2017-08-22 14:21:42 +01:00
committed by GitHub
parent 65009b5578
commit 8931ede142
11 changed files with 41 additions and 45 deletions

View File

@@ -124,9 +124,9 @@ func (c *Config) AddMiddleware(m middleware.Middleware) {
c.Middleware = append(c.Middleware, m) c.Middleware = append(c.Middleware, m)
} }
// RegisterHandler adds a handler to a site's handler registration. Handlers // registerHandler adds a handler to a site's handler registration. Handlers
// should use this if the want to announce that they exist to other middleware. // use this to announce that they exist to other middleware.
func (c *Config) RegisterHandler(h middleware.Handler) { func (c *Config) registerHandler(h middleware.Handler) {
if c.Registry == nil { if c.Registry == nil {
c.Registry = make(map[string]middleware.Handler) c.Registry = make(map[string]middleware.Handler)
} }
@@ -135,12 +135,11 @@ func (c *Config) RegisterHandler(h middleware.Handler) {
c.Registry[h.Name()] = h c.Registry[h.Name()] = h
} }
// GetHandler returns the middleware handler that has been added to the config under its name. // Handler returns the middleware handler that has been added to the config under its name.
// This is useful to inspect if a certain middleware is active in this server. // This is useful to inspect if a certain middleware is active in this server.
// Note that this is order dependent and the order is defined in directives.go, i.e. if your middleware // Note that this is order dependent and the order is defined in directives.go, i.e. if your middleware
// comes before the middleware you are checking; it will not be there (yet). // comes before the middleware you are checking; it will not be there (yet).
// See RegisterHandler on how to register the middleware with this server. func (c *Config) Handler(name string) middleware.Handler {
func (c *Config) GetHandler(name string) middleware.Handler {
if c.Registry == nil { if c.Registry == nil {
return nil return nil
} }

View File

@@ -66,6 +66,10 @@ func NewServer(addr string, group []*Config) (*Server, error) {
var stack middleware.Handler var stack middleware.Handler
for i := len(site.Middleware) - 1; i >= 0; i-- { for i := len(site.Middleware) - 1; i >= 0; i-- {
stack = site.Middleware[i](stack) stack = site.Middleware[i](stack)
// register the *handler* also
site.registerHandler(stack)
if s.trace == nil && stack.Name() == "trace" { if s.trace == nil && stack.Name() == "trace" {
// we have to stash away the middleware, not the // we have to stash away the middleware, not the
// Tracer object, because the Tracer won't be initialized yet // Tracer object, because the Tracer won't be initialized yet

View File

@@ -31,13 +31,14 @@ func setup(c *caddy.Controller) error {
return middleware.Error("auto", err) return middleware.Error("auto", err)
} }
// If we have enabled prometheus we should add newly discovered zones to it. c.OnStartup(func() error {
// This does not have to happen in a on.Startup because monitoring is one of the first m := dnsserver.GetConfig(c).Handler("prometheus")
// to be initialized. if m == nil {
met := dnsserver.GetConfig(c).GetHandler("prometheus") return nil
if met != nil { }
a.metrics = met.(*metrics.Metrics) (&a).metrics = m.(*metrics.Metrics)
} return nil
})
walkChan := make(chan bool) walkChan := make(chan bool)

View File

@@ -29,7 +29,7 @@ func setup(c *caddy.Controller) error {
// Do this in OnStartup, so all middleware has been initialized. // Do this in OnStartup, so all middleware has been initialized.
c.OnStartup(func() error { c.OnStartup(func() error {
// TODO(miek): fabricate test to proof this is not thread safe. // TODO(miek): fabricate test to proof this is not thread safe.
m := dnsserver.GetConfig(c).GetHandler(mw) m := dnsserver.GetConfig(c).Handler(mw)
if m == nil { if m == nil {
return nil return nil
} }
@@ -50,8 +50,7 @@ func setup(c *caddy.Controller) error {
return nil return nil
} }
// allowedMiddleware has a list of middleware that can be used by autopath. For this to work, they // allowedMiddleware has a list of middleware that can be used by autopath.
// need to register themselves with dnsserver.RegisterHandler.
var allowedMiddleware = map[string]bool{ var allowedMiddleware = map[string]bool{
"@kubernetes": true, "@kubernetes": true,
"@erratic": true, "@erratic": true,

View File

@@ -28,9 +28,6 @@ func setupErratic(c *caddy.Controller) error {
return e return e
}) })
// Also register erratic for use in autopath.
dnsserver.GetConfig(c).RegisterHandler(e)
return nil return nil
} }

View File

@@ -47,14 +47,33 @@ func TestParseErratic(t *testing.T) {
delay 3 1ms delay 3 1ms
}`, false, 0, 3, 2}, }`, false, 0, 3, 2},
{`erraric {
drop 3
delay
}`, false, 3, 2, 0},
// fails // fails
{`erratic { {`erratic {
drop -1 drop -1
}`, true, 0, 0, 0}, }`, true, 0, 0, 0},
{`erratic {
delay -1
}`, true, 0, 0, 0},
{`erratic {
delay 1 2 4
}`, true, 0, 0, 0},
{`erratic {
delay 15.a
}`, true, 0, 0, 0},
{`erraric { {`erraric {
drop 3 drop 3
delay 3 bla delay 3 bla
}`, true, 0, 0, 0}, }`, true, 0, 0, 0},
{`erraric {
truncate 15.a
}`, true, 0, 0, 0},
{`erraric {
something-else
}`, true, 0, 0, 0},
} }
for i, test := range tests { for i, test := range tests {
c := caddy.NewTestController("dns", test.input) c := caddy.NewTestController("dns", test.input)

View File

@@ -26,7 +26,7 @@ func setup(c *caddy.Controller) error {
// Do this in OnStartup, so all middleware has been initialized. // Do this in OnStartup, so all middleware has been initialized.
c.OnStartup(func() error { c.OnStartup(func() error {
m := dnsserver.GetConfig(c).GetHandler("kubernetes") m := dnsserver.GetConfig(c).Handler("kubernetes")
if m == nil { if m == nil {
return nil return nil
} }

View File

@@ -55,9 +55,6 @@ func setup(c *caddy.Controller) error {
return kubernetes return kubernetes
}) })
// Also register kubernetes for use in autopath.
dnsserver.GetConfig(c).RegisterHandler(kubernetes)
return nil return nil
} }

View File

@@ -38,9 +38,6 @@ func setup(c *caddy.Controller) error {
} }
c.OnFinalShutdown(m.OnShutdown) c.OnFinalShutdown(m.OnShutdown)
// Also register metrics for use in other middleware.
dnsserver.GetConfig(c).RegisterHandler(m)
return nil return nil
} }
@@ -75,24 +72,6 @@ func prometheusParse(c *caddy.Controller) (*Metrics, error) {
default: default:
return met, c.ArgErr() return met, c.ArgErr()
} }
for c.NextBlock() {
switch c.Val() {
case "address":
args = c.RemainingArgs()
if len(args) != 1 {
return met, c.ArgErr()
}
met.Addr = args[0]
// expecting something that resembles a host-port
_, _, e := net.SplitHostPort(met.Addr)
if e != nil {
return met, e
}
default:
return met, c.Errf("unknown property: %s", c.Val())
}
}
} }
return met, err return met, err
} }

View File

@@ -18,6 +18,7 @@ func TestPrometheusParse(t *testing.T) {
// fails // fails
{`prometheus {}`, true, ""}, {`prometheus {}`, true, ""},
{`prometheus /foo`, true, ""}, {`prometheus /foo`, true, ""},
{`prometheus a b c`, true, ""},
} }
for i, test := range tests { for i, test := range tests {
c := caddy.NewTestController("dns", test.input) c := caddy.NewTestController("dns", test.input)

View File

@@ -20,7 +20,7 @@ func setup(c *caddy.Controller) error {
return middleware.Error("proxy", err) return middleware.Error("proxy", err)
} }
t := dnsserver.GetConfig(c).GetHandler("trace") t := dnsserver.GetConfig(c).Handler("trace")
P := &Proxy{Trace: t} P := &Proxy{Trace: t}
dnsserver.GetConfig(c).AddMiddleware(func(next middleware.Handler) middleware.Handler { dnsserver.GetConfig(c).AddMiddleware(func(next middleware.Handler) middleware.Handler {
P.Next = next P.Next = next