From f697b33283afe695554888eb0cf9f5451c481470 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Wed, 28 Feb 2018 18:16:05 -0800 Subject: [PATCH] return an error for multiple use of some plugins (#1559) * plugins: Return error for multiple use of some Return plugin.ErrOnce when a plugin that doesn't support it, is called mutliple times. This now adds it for: cache, dnssec, errors, forward, hosts, nsid. And changes it slightly in kubernetes, pprof, reload, root. * more tests --- plugin/cache/README.md | 2 ++ plugin/cache/setup.go | 10 ++++-- plugin/cache/setup_test.go | 54 +++++++++++++++++---------------- plugin/dnssec/README.md | 19 ++---------- plugin/dnssec/setup.go | 12 +++++++- plugin/dnssec/setup_test.go | 2 ++ plugin/errors/README.md | 2 ++ plugin/errors/setup.go | 6 ++++ plugin/errors/setup_test.go | 23 +++++--------- plugin/forward/README.md | 2 ++ plugin/forward/setup.go | 6 ++++ plugin/forward/setup_test.go | 2 ++ plugin/hosts/README.md | 2 ++ plugin/hosts/setup.go | 6 ++++ plugin/hosts/setup_test.go | 9 ++++++ plugin/kubernetes/README.md | 2 ++ plugin/kubernetes/setup.go | 16 +++++++--- plugin/kubernetes/setup_test.go | 2 +- plugin/nsid/README.md | 13 ++++++-- plugin/nsid/setup.go | 13 +++++--- plugin/nsid/setup_test.go | 18 ++++------- plugin/plugin.go | 3 ++ plugin/pprof/README.md | 2 ++ plugin/pprof/setup.go | 10 +++--- plugin/reload/README.md | 2 ++ plugin/root/README.md | 2 ++ plugin/route53/README.md | 5 ++- 27 files changed, 150 insertions(+), 95 deletions(-) diff --git a/plugin/cache/README.md b/plugin/cache/README.md index fe9e45c24..ade647599 100644 --- a/plugin/cache/README.md +++ b/plugin/cache/README.md @@ -10,6 +10,8 @@ With *cache* enabled, all records except zone transfers and metadata records wil 3600s. Caching is mostly useful in a scenario when fetching data from the backend (upstream, database, etc.) is expensive. +This plugin can only be used once per Server Block. + ## Syntax ~~~ txt diff --git a/plugin/cache/setup.go b/plugin/cache/setup.go index 41fa023b8..4ea51d510 100644 --- a/plugin/cache/setup.go +++ b/plugin/cache/setup.go @@ -61,7 +61,13 @@ func setup(c *caddy.Controller) error { func cacheParse(c *caddy.Controller) (*Cache, error) { ca := New() + j := 0 for c.Next() { + if j > 0 { + return nil, plugin.ErrOnce + } + j++ + // cache [ttl] [zones..] origins := make([]string, len(c.ServerBlockKeys)) copy(origins, c.ServerBlockKeys) @@ -180,9 +186,7 @@ func cacheParse(c *caddy.Controller) (*Cache, error) { ca.pcache = cache.New(ca.pcap) ca.ncache = cache.New(ca.ncap) - - return ca, nil } - return nil, nil + return ca, nil } diff --git a/plugin/cache/setup_test.go b/plugin/cache/setup_test.go index afc2ecc13..c735e0bb2 100644 --- a/plugin/cache/setup_test.go +++ b/plugin/cache/setup_test.go @@ -20,46 +20,48 @@ func TestSetup(t *testing.T) { {`cache`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 0}, {`cache {}`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 0}, {`cache example.nl { - success 10 - }`, false, defaultCap, 10, maxNTTL, maxTTL, 0}, + success 10 + }`, false, defaultCap, 10, maxNTTL, maxTTL, 0}, {`cache example.nl { - success 10 - denial 10 15 - }`, false, 10, 10, 15 * time.Second, maxTTL, 0}, + success 10 + denial 10 15 + }`, false, 10, 10, 15 * time.Second, maxTTL, 0}, {`cache 25 example.nl { - success 10 - denial 10 15 - }`, false, 10, 10, 15 * time.Second, 25 * time.Second, 0}, + success 10 + denial 10 15 + }`, false, 10, 10, 15 * time.Second, 25 * time.Second, 0}, {`cache aaa example.nl`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 0}, {`cache { - prefetch 10 - }`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 10}, + prefetch 10 + }`, false, defaultCap, defaultCap, maxNTTL, maxTTL, 10}, // fails {`cache example.nl { - success - denial 10 15 - }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, + success + denial 10 15 + }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, {`cache example.nl { - success 15 - denial aaa - }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, + success 15 + denial aaa + }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, {`cache example.nl { - positive 15 - negative aaa - }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, + positive 15 + negative aaa + }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, {`cache 0 example.nl`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, {`cache -1 example.nl`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, {`cache 1 example.nl { - positive 0 - }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, + positive 0 + }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, {`cache 1 example.nl { - positive 0 - prefetch -1 - }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, + positive 0 + prefetch -1 + }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, {`cache 1 example.nl { - prefetch 0 blurp - }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, + prefetch 0 blurp + }`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, + {`cache + cache`, true, defaultCap, defaultCap, maxTTL, maxTTL, 0}, } for i, test := range tests { c := caddy.NewTestController("dns", test.input) diff --git a/plugin/dnssec/README.md b/plugin/dnssec/README.md index b94e14ec1..4a090b06f 100644 --- a/plugin/dnssec/README.md +++ b/plugin/dnssec/README.md @@ -10,6 +10,8 @@ With *dnssec* any reply that doesn't (or can't) do DNSSEC will get signed on the denial of existence is implemented with NSEC black lies. Using ECDSA as an algorithm is preferred as this leads to smaller signatures (compared to RSA). NSEC3 is *not* supported. +This plugin can only be used once per Server Block. + ## Syntax ~~~ @@ -74,20 +76,3 @@ cluster.local { } } ~~~ - -## Bugs - -Multiple *dnssec* plugins inside one server stanza will silently overwrite earlier ones, here -`example.org` will overwrite the one for `cluster.local`. - -~~~ -. { - kubernetes cluster.local - dnssec cluster.local { - key file Kcluster.local+013+45129 - } - dnssec example.org { - key file Kexample.org.+013+45330 - } -} -~~~ diff --git a/plugin/dnssec/setup.go b/plugin/dnssec/setup.go index 26cdbe2f1..efcc1aa85 100644 --- a/plugin/dnssec/setup.go +++ b/plugin/dnssec/setup.go @@ -59,7 +59,14 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) { keys := []*DNSKEY{} capacity := defaultCap + + i := 0 for c.Next() { + if i > 0 { + return nil, nil, 0, plugin.ErrOnce + } + i++ + // dnssec [zones...] zones = make([]string, len(c.ServerBlockKeys)) copy(zones, c.ServerBlockKeys) @@ -69,7 +76,8 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) { } for c.NextBlock() { - switch c.Val() { + + switch x := c.Val(); x { case "key": k, e := keyParse(c) if e != nil { @@ -86,6 +94,8 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, error) { return nil, nil, 0, err } capacity = cacheCap + default: + return nil, nil, 0, c.Errf("unknown property '%s'", x) } } diff --git a/plugin/dnssec/setup_test.go b/plugin/dnssec/setup_test.go index 99a71279d..b4ca7484b 100644 --- a/plugin/dnssec/setup_test.go +++ b/plugin/dnssec/setup_test.go @@ -61,6 +61,8 @@ func TestSetupDnssec(t *testing.T) { key file }`, true, []string{"example.org."}, nil, defaultCap, "argument count", }, + {`dnssec + dnssec`, true, nil, nil, defaultCap, ""}, } for i, test := range tests { diff --git a/plugin/errors/README.md b/plugin/errors/README.md index db16eaea8..f15b271f6 100644 --- a/plugin/errors/README.md +++ b/plugin/errors/README.md @@ -8,6 +8,8 @@ Any errors encountered during the query processing will be printed to standard output. +This plugin can only be used once per Server Block. + ## Syntax ~~~ diff --git a/plugin/errors/setup.go b/plugin/errors/setup.go index 19bdcdb80..082f222b0 100644 --- a/plugin/errors/setup.go +++ b/plugin/errors/setup.go @@ -37,7 +37,13 @@ func setup(c *caddy.Controller) error { func errorsParse(c *caddy.Controller) (errorHandler, error) { handler := errorHandler{} + i := 0 for c.Next() { + if i > 0 { + return handler, plugin.ErrOnce + } + i++ + args := c.RemainingArgs() switch len(args) { case 0: diff --git a/plugin/errors/setup_test.go b/plugin/errors/setup_test.go index bae85da32..da7c251d5 100644 --- a/plugin/errors/setup_test.go +++ b/plugin/errors/setup_test.go @@ -12,21 +12,14 @@ func TestErrorsParse(t *testing.T) { shouldErr bool expectedErrorHandler errorHandler }{ - {`errors`, false, errorHandler{ - LogFile: "stdout", - }}, - {`errors stdout`, false, errorHandler{ - LogFile: "stdout", - }}, - {`errors errors.txt`, true, errorHandler{ - LogFile: "", - }}, - {`errors visible`, true, errorHandler{ - LogFile: "", - }}, - {`errors { log visible }`, true, errorHandler{ - LogFile: "stdout", - }}, + {`errors`, false, errorHandler{LogFile: "stdout"}}, + {`errors stdout`, false, errorHandler{LogFile: "stdout"}}, + {`errors errors.txt`, true, errorHandler{LogFile: ""}}, + {`errors visible`, true, errorHandler{LogFile: ""}}, + {`errors { log visible }`, true, errorHandler{LogFile: "stdout"}}, + {`errors + errors `, true, errorHandler{LogFile: "stdout"}}, + {`errors a b`, true, errorHandler{LogFile: ""}}, } for i, test := range tests { c := caddy.NewTestController("dns", test.inputErrorsRules) diff --git a/plugin/forward/README.md b/plugin/forward/README.md index e3ba86a80..f5011baaa 100644 --- a/plugin/forward/README.md +++ b/plugin/forward/README.md @@ -19,6 +19,8 @@ is performed and upstreams will always be considered healthy. When *all* upstreams are down it assumes health checking as a mechanism has failed and will try to connect to a random upstream (which may or may not work). +This plugin can only be used once per Server Block. + ## Syntax In its most basic form, a simple forwarder uses this syntax: diff --git a/plugin/forward/setup.go b/plugin/forward/setup.go index 779d085ae..8d80e779d 100644 --- a/plugin/forward/setup.go +++ b/plugin/forward/setup.go @@ -84,7 +84,13 @@ func parseForward(c *caddy.Controller) (*Forward, error) { protocols := map[int]int{} + i := 0 for c.Next() { + if i > 0 { + return nil, plugin.ErrOnce + } + i++ + if !c.Args(&f.from) { return f, c.ArgErr() } diff --git a/plugin/forward/setup_test.go b/plugin/forward/setup_test.go index f1776222f..bf77bb932 100644 --- a/plugin/forward/setup_test.go +++ b/plugin/forward/setup_test.go @@ -30,6 +30,8 @@ func TestSetup(t *testing.T) { // negative {"forward . a27.0.0.1", true, "", nil, 0, false, "not an IP"}, {"forward . 127.0.0.1 {\nblaatl\n}\n", true, "", nil, 0, false, "unknown property"}, + {`forward . ::1 + forward com ::2`, true, "", nil, 0, false, "plugin"}, } for i, test := range tests { diff --git a/plugin/hosts/README.md b/plugin/hosts/README.md index b435e0a88..3be25661a 100644 --- a/plugin/hosts/README.md +++ b/plugin/hosts/README.md @@ -11,6 +11,8 @@ file that exists on disk. It checks the file for changes and updates the zones a plugin only supports A, AAAA, and PTR records. The hosts plugin can be used with readily available hosts files that block access to advertising servers. +This plugin can only be used once per Server Block. + ## Syntax ~~~ diff --git a/plugin/hosts/setup.go b/plugin/hosts/setup.go index 57413feed..ca4635f0a 100644 --- a/plugin/hosts/setup.go +++ b/plugin/hosts/setup.go @@ -69,7 +69,13 @@ func hostsParse(c *caddy.Controller) (Hosts, error) { config := dnsserver.GetConfig(c) inline := []string{} + i := 0 for c.Next() { + if i > 0 { + return h, plugin.ErrOnce + } + i++ + args := c.RemainingArgs() if len(args) >= 1 { h.path = args[0] diff --git a/plugin/hosts/setup_test.go b/plugin/hosts/setup_test.go index d23170f1f..78b1cff86 100644 --- a/plugin/hosts/setup_test.go +++ b/plugin/hosts/setup_test.go @@ -57,6 +57,15 @@ func TestHostsParse(t *testing.T) { }`, false, "/etc/hosts", []string{"miek.nl.", "10.in-addr.arpa."}, fall.Root, }, + { + `hosts /etc/hosts { + fallthrough + } + hosts /etc/hosts { + fallthrough + }`, + true, "/etc/hosts", nil, fall.Root, + }, } for i, test := range tests { diff --git a/plugin/kubernetes/README.md b/plugin/kubernetes/README.md index 363b8b536..136b301f5 100644 --- a/plugin/kubernetes/README.md +++ b/plugin/kubernetes/README.md @@ -16,6 +16,8 @@ to deploy CoreDNS in Kubernetes](https://github.com/coredns/deployment/tree/mast [stubDomains and upstreamNameservers](http://blog.kubernetes.io/2017/04/configuring-private-dns-zones-upstream-nameservers-kubernetes.html) are implemented via the *proxy* plugin and kubernetes *upstream*. See example below. +This plugin can only be used once per Server Block. + ## Syntax ~~~ diff --git a/plugin/kubernetes/setup.go b/plugin/kubernetes/setup.go index 7d41fb64d..9eddadbb7 100644 --- a/plugin/kubernetes/setup.go +++ b/plugin/kubernetes/setup.go @@ -71,12 +71,18 @@ func (k *Kubernetes) RegisterKubeCache(c *caddy.Controller) { } func kubernetesParse(c *caddy.Controller) (*Kubernetes, error) { - var k8s *Kubernetes - var err error - for i := 1; c.Next(); i++ { - if i > 1 { - return nil, fmt.Errorf("only one kubernetes section allowed per server block") + var ( + k8s *Kubernetes + err error + ) + + i := 0 + for c.Next() { + if i > 0 { + return nil, plugin.ErrOnce } + i++ + k8s, err = ParseStanza(c) if err != nil { return k8s, err diff --git a/plugin/kubernetes/setup_test.go b/plugin/kubernetes/setup_test.go index 3ce837e98..63ea52f66 100644 --- a/plugin/kubernetes/setup_test.go +++ b/plugin/kubernetes/setup_test.go @@ -388,7 +388,7 @@ func TestKubernetesParse(t *testing.T) { `kubernetes coredns.local kubernetes cluster.local`, true, - "only one kubernetes section allowed per server block", + "this plugin", -1, 0, defaultResyncPeriod, diff --git a/plugin/nsid/README.md b/plugin/nsid/README.md index 10c60523f..0ff5cd764 100644 --- a/plugin/nsid/README.md +++ b/plugin/nsid/README.md @@ -6,9 +6,12 @@ ## Description -This plugin implements RFC 5001 and adds an EDNS0 OPT resource record to replies that uniquely -identify the server. This is useful in anycast setups to see which server was responsible for -generating the reply and for debugging. +This plugin implements [RFC 5001](https://tools.ietf.org/html/rfc5001) and adds an EDNS0 OPT +resource record to replies that uniquely identify the server. This is useful in anycast setups to +see which server was responsible for generating the reply and for debugging. + +This plugin can only be used once per Server Block. + ## Syntax @@ -48,3 +51,7 @@ And now a client with NSID support will see an OPT record with the NSID option: ;; QUESTION SECTION: ;whoami.example.org. IN A ~~~ + +## Also See + +[RFC 5001](https://tools.ietf.org/html/rfc5001) diff --git a/plugin/nsid/setup.go b/plugin/nsid/setup.go index 104bdfd9a..e6c5c5ae9 100644 --- a/plugin/nsid/setup.go +++ b/plugin/nsid/setup.go @@ -36,13 +36,16 @@ func nsidParse(c *caddy.Controller) (string, error) { if err != nil { nsid = "localhost" } + i := 0 for c.Next() { - args := c.RemainingArgs() - if len(args) == 0 { - return nsid, nil + if i > 0 { + return nsid, plugin.ErrOnce + } + i++ + args := c.RemainingArgs() + if len(args) > 0 { + nsid = strings.Join(args, " ") } - nsid = strings.Join(args, " ") - return nsid, nil } return nsid, nil } diff --git a/plugin/nsid/setup_test.go b/plugin/nsid/setup_test.go index 71ed90d0a..059b05bb5 100644 --- a/plugin/nsid/setup_test.go +++ b/plugin/nsid/setup_test.go @@ -19,18 +19,12 @@ func TestSetupNsid(t *testing.T) { expectedData string expectedErrContent string // substring from the expected error. Empty for positive cases. }{ - { - `nsid`, false, defaultNsid, "", - }, - { - `nsid "ps0"`, false, "ps0", "", - }, - { - `nsid "worker1"`, false, "worker1", "", - }, - { - `nsid "tf 2"`, false, "tf 2", "", - }, + {`nsid`, false, defaultNsid, ""}, + {`nsid "ps0"`, false, "ps0", ""}, + {`nsid "worker1"`, false, "worker1", ""}, + {`nsid "tf 2"`, false, "tf 2", ""}, + {`nsid + nsid`, true, "", "plugin"}, } for i, test := range tests { diff --git a/plugin/plugin.go b/plugin/plugin.go index a50f10830..2510dbaa6 100644 --- a/plugin/plugin.go +++ b/plugin/plugin.go @@ -104,3 +104,6 @@ const Namespace = "coredns" // TimeBuckets is based on Prometheus client_golang prometheus.DefBuckets var TimeBuckets = prometheus.ExponentialBuckets(0.00025, 2, 16) // from 0.25ms to 8 seconds + +// ErrOnce is returned when a plugin doesn't support multiple setups per server. +var ErrOnce = errors.New("this plugin can only be used once per Server Block") diff --git a/plugin/pprof/README.md b/plugin/pprof/README.md index a718d35a4..53de6e326 100644 --- a/plugin/pprof/README.md +++ b/plugin/pprof/README.md @@ -16,6 +16,8 @@ For more information, please see [Go's pprof documentation](https://golang.org/pkg/net/http/pprof/) and read [Profiling Go Programs](https://blog.golang.org/profiling-go-programs). +This plugin can only be used once per Server Block. + ## Syntax ~~~ diff --git a/plugin/pprof/setup.go b/plugin/pprof/setup.go index 22b82e94b..ea5e9ce4d 100644 --- a/plugin/pprof/setup.go +++ b/plugin/pprof/setup.go @@ -19,12 +19,15 @@ func init() { } func setup(c *caddy.Controller) error { - found := false h := &handler{addr: defaultAddr} + + i := 0 for c.Next() { - if found { - return plugin.Error("pprof", c.Err("pprof can only be specified once")) + if i > 0 { + return plugin.Error("pprof", plugin.ErrOnce) } + i++ + args := c.RemainingArgs() if len(args) == 1 { h.addr = args[0] @@ -39,7 +42,6 @@ func setup(c *caddy.Controller) error { if c.NextBlock() { return plugin.Error("pprof", c.ArgErr()) } - found = true } pprofOnce.Do(func() { diff --git a/plugin/reload/README.md b/plugin/reload/README.md index 98c7fd502..bb4eda179 100644 --- a/plugin/reload/README.md +++ b/plugin/reload/README.md @@ -26,6 +26,8 @@ reloads are graceful, and can be disabled by setting the jitter to `0s`. Jitter is re-calculated whenever the Corefile is reloaded. +This plugin can only be used once per Server Block. + ## Syntax ~~~ txt diff --git a/plugin/root/README.md b/plugin/root/README.md index 2cc3df258..1d21bc0eb 100644 --- a/plugin/root/README.md +++ b/plugin/root/README.md @@ -9,6 +9,8 @@ The default root is the current working directory of CoreDNS. The *root* plugin allows you to change this. A relative root path is relative to the current working directory. +This plugin can only be used once per Server Block. + ## Syntax ~~~ txt diff --git a/plugin/route53/README.md b/plugin/route53/README.md index a0c2235b9..2044ad8d7 100644 --- a/plugin/route53/README.md +++ b/plugin/route53/README.md @@ -6,9 +6,8 @@ ## Description -The route53 plugin is useful for serving zones from resource record sets in AWS route53. -This plugin only supports A and AAAA records. The route53 plugin can be used when -coredns is deployed on AWS. +The route53 plugin is useful for serving zones from resource record sets in AWS route53. This plugin +only supports A and AAAA records. The route53 plugin can be used when coredns is deployed on AWS. ## Syntax