Fully deprecate TIMEOUT and NO_RELOAD (#2742)

* Fully deprecate NO_RELOAD
Signed-off-by: Xiao An <hac@zju.edu.cn>

* Fully deprecate TIMEOUT
Signed-off-by: Xiao An <hac@zju.edu.cn>

* Update CI tests to adapt to the deprecation of TIMEOUT
Signed-off-by: Xiao An <hac@zju.edu.cn>

* Add documents for directive transfer in plugin auto
Signed-off-by: Xiao An <hac@zju.edu.cn>

plugin/auto/README.md

@@ -15,9 +15,9 @@ zonefile. New or changed zones are automatically picked up from disk.
 
 ~~~
 auto [ZONES...] {
-    directory DIR [REGEXP ORIGIN_TEMPLATE [TIMEOUT]]
+    directory DIR [REGEXP ORIGIN_TEMPLATE]
+    transfer to ADDRESS...
     reload DURATION
-    no_reload
     upstream
 }
 ~~~
@@ -30,14 +30,13 @@ are used.
   like `{<number>}` are replaced with the respective matches in the file name, e.g. `{1}` is the
   first match, `{2}` is the second. The default is: `db\.(.*)  {1}` i.e. from a file with the
   name `db.example.com`, the extracted origin will be `example.com`.
-  **TIMEOUT** is deprecated and will be removed in a subsequent version. 
-  `reload` will be used, if not defined
-  (it specifies how often CoreDNS should scan the directory to watch for file removal and addition;
-  the default is every 60 seconds. This value is in seconds. The minimum value is 1 second.)
-* `reload` interval to perform reloads of zones if SOA version changes and zonefiles. Default is one minute.
+* `transfer` enables zone transfers. It may be specified multiples times. `To` or `from` signals
+  the direction. **ADDRESS** must be denoted in CIDR notation (e.g., 127.0.0.1/32) or just as plain
+  addresses. The special wildcard `*` means: the entire internet (only valid for 'transfer to').
+  When an address is specified a notify message will be send whenever the zone is reloaded.
+* `reload` interval to perform reloads of zones if SOA version changes and zonefiles. It specifies how often CoreDNS should scan the directory to watch for file removal and addition. Default is one minute.
   Value of `0` means to not scan for changes and reload. eg. `30s` checks zonefile every 30 seconds
   and reloads zone when serial changes.
-* `no_reload` deprecated. Sets reload to 0.
 * `upstream` defines upstream resolvers to be used resolve external names found (think CNAMEs)
   pointing to external names. CoreDNS will resolve CNAMEs against itself.
 
@@ -75,7 +74,8 @@ where `example.org` is the origin. Scan every 45 seconds.
 ~~~ corefile
 org {
     auto {
-        directory /etc/coredns/zones/org www\.db\.(.*) {1} 45
+        directory /etc/coredns/zones/org www\.db\.(.*) {1}
+        reload 45s
     }
 }
 ~~~

plugin/auto/auto.go

@@ -34,8 +34,6 @@ type (
 		transferTo     []string
 		ReloadInterval time.Duration
 		upstream       *upstream.Upstream // Upstream for looking up names during the resolution process.
-
-		duration time.Duration
 	}
 )
 

plugin/auto/setup.go

@@ -4,7 +4,6 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"strconv"
 	"time"
 
 	"github.com/coredns/coredns/core/dnsserver"
@@ -50,7 +49,7 @@ func setup(c *caddy.Controller) error {
 		}
 
 		go func() {
-			ticker := time.NewTicker(a.loader.duration)
+			ticker := time.NewTicker(a.loader.ReloadInterval)
 			for {
 				select {
 				case <-walkChan:
@@ -83,7 +82,6 @@ func autoParse(c *caddy.Controller) (Auto, error) {
 			template:       "${1}",
 			re:             regexp.MustCompile(`db\.(.*)`),
 			ReloadInterval: nilInterval,
-			duration:       nilInterval,
 		},
 		Zones: &Zones{},
 	}
@@ -105,7 +103,7 @@ func autoParse(c *caddy.Controller) (Auto, error) {
 
 		for c.NextBlock() {
 			switch c.Val() {
-			case "directory": // directory DIR [REGEXP [TEMPLATE] [DURATION]]
+			case "directory": // directory DIR [REGEXP TEMPLATE]
 				if !c.NextArg() {
 					return a, c.ArgErr()
 				}
@@ -138,17 +136,8 @@ func autoParse(c *caddy.Controller) (Auto, error) {
 					a.loader.template = rewriteToExpand(c.Val())
 				}
 
-				// duration
 				if c.NextArg() {
-					i, err := strconv.Atoi(c.Val())
-					if err != nil {
-						return a, err
-					}
-					if i < 1 {
-						i = 1
-					}
-					log.Warning("TIMEOUT of directory is deprecated. Use RELOAD instead. See https://coredns.io/plugins/auto/#syntax")
-					a.loader.duration = time.Duration(i) * time.Second
+					return Auto{}, c.ArgErr()
 				}
 
 			case "reload":
@@ -158,15 +147,11 @@ func autoParse(c *caddy.Controller) (Auto, error) {
 				}
 				a.loader.ReloadInterval = d
 
-			case "no_reload":
-				log.Warning("NO_RELOAD of directory is deprecated. Use RELOAD (set to 0) instead. See https://coredns.io/plugins/auto/#syntax")
-				a.loader.ReloadInterval = 0
-
 			case "upstream":
 				c.RemainingArgs() // eat remaining args
 				a.loader.upstream = upstream.New()
 
-			default:
+			case "transfer":
 				t, _, e := parse.Transfer(c, false)
 				if e != nil {
 					return a, e
@@ -174,17 +159,15 @@ func autoParse(c *caddy.Controller) (Auto, error) {
 				if t != nil {
 					a.loader.transferTo = append(a.loader.transferTo, t...)
 				}
+
+			default:
+				return Auto{}, c.Errf("unknown property '%s'", c.Val())
 			}
 		}
 	}
 
 	if a.loader.ReloadInterval == nilInterval {
-		if a.loader.duration == nilInterval {
-			a.loader.duration = 60 * time.Second
-		}
-		a.loader.ReloadInterval = a.loader.duration
-	} else if a.loader.duration == nilInterval {
-		a.loader.duration = a.loader.ReloadInterval
+		a.loader.ReloadInterval = 60 * time.Second
 	}
 
 	return a, nil

plugin/auto/setup_test.go

@@ -15,7 +15,6 @@ func TestAutoParse(t *testing.T) {
 		expectedTempl          string
 		expectedRe             string
 		expectedReloadInterval time.Duration
-		expectedDuration       time.Duration
 		expectedTo             []string
 	}{
 		{
@@ -23,46 +22,33 @@ func TestAutoParse(t *testing.T) {
 				directory /tmp
 				transfer to 127.0.0.1
 			}`,
-			false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, 60 * time.Second, []string{"127.0.0.1:53"},
+			false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, []string{"127.0.0.1:53"},
 		},
 		{
 			`auto 10.0.0.0/24 {
 				directory /tmp
 			}`,
-			false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, 60 * time.Second, nil,
+			false, "/tmp", "${1}", `db\.(.*)`, 60 * time.Second, nil,
 		},
 		{
 			`auto {
 				directory /tmp
-				no_reload
+				reload 0
 			}`,
-			false, "/tmp", "${1}", `db\.(.*)`, 0 * time.Second, 0 * time.Second, nil,
+			false, "/tmp", "${1}", `db\.(.*)`, 0 * time.Second, nil,
 		},
 		{
 			`auto {
 				directory /tmp (.*) bliep
 			}`,
-			false, "/tmp", "bliep", `(.*)`, 60 * time.Second, 60 * time.Second, nil,
-		},
-		{
-			`auto {
-				directory /tmp (.*) bliep 10
-			}`,
-			false, "/tmp", "bliep", `(.*)`, 10 * time.Second, 10 * time.Second, nil,
+			false, "/tmp", "bliep", `(.*)`, 60 * time.Second, nil,
 		},
 		{
 			`auto {
 				directory /tmp (.*) bliep
 				reload 10s
 			}`,
-			false, "/tmp", "bliep", `(.*)`, 10 * time.Second, 10 * time.Second, nil,
-		},
-		{
-			`auto {
-				directory /tmp (.*) bliep 20
-				reload 10s
-			}`,
-			false, "/tmp", "bliep", `(.*)`, 10 * time.Second, 20 * time.Second, nil,
+			false, "/tmp", "bliep", `(.*)`, 10 * time.Second, nil,
 		},
 		{
 			`auto {
@@ -71,44 +57,44 @@ func TestAutoParse(t *testing.T) {
 				transfer to 127.0.0.2
 				upstream 8.8.8.8
 			}`,
-			false, "/tmp", "bliep", `(.*)`, 60 * time.Second, 60 * time.Second, []string{"127.0.0.1:53", "127.0.0.2:53"},
+			false, "/tmp", "bliep", `(.*)`, 60 * time.Second, []string{"127.0.0.1:53", "127.0.0.2:53"},
 		},
 		// errors
+		// NO_RELOAD has been deprecated.
 		{
-			`auto example.org {
-				directory
-			}`,
-			true, "", "${1}", `db\.(.*)`, 60 * time.Second, 60 * time.Second, nil,
-		},
-		{
-			`auto example.org {
-				directory /tmp * {1}
+			`auto {
+				directory /tmp
+				no_reload
 			}`,
-			true, "", "${1}", ``, 60 * time.Second, 60 * time.Second, nil,
+			true, "/tmp", "${1}", `db\.(.*)`, 0 * time.Second, nil,
 		},
+		// TIMEOUT has been deprecated.
 		{
-			`auto example.org {
-				directory /tmp * {1} aa
+			`auto {
+				directory /tmp (.*) bliep 10
 			}`,
-			true, "", "${1}", ``, 60 * time.Second, 60 * time.Second, nil,
+			true, "/tmp", "bliep", `(.*)`, 10 * time.Second, nil,
 		},
+		// no directory specified.
 		{
 			`auto example.org {
-				directory /tmp .* {1}
+				directory
 			}`,
-			true, "", "${1}", ``, 60 * time.Second, 60 * time.Second, nil,
+			true, "", "${1}", `db\.(.*)`, 60 * time.Second, nil,
 		},
+		// illegal REGEXP.
 		{
 			`auto example.org {
-				directory /tmp .* {1}
+				directory /tmp * {1}
 			}`,
-			true, "", "${1}", ``, 60 * time.Second, 60 * time.Second, nil,
+			true, "/tmp", "${1}", ``, 60 * time.Second, nil,
 		},
+		// unexpected argument.
 		{
 			`auto example.org {
-				directory /tmp .* {1}
+				directory /tmp (.*) {1} aa
 			}`,
-			true, "", "${1}", ``, 60 * time.Second, 60 * time.Second, nil,
+			true, "/tmp", "${1}", ``, 60 * time.Second, nil,
 		},
 	}
 
@@ -133,9 +119,6 @@ func TestAutoParse(t *testing.T) {
 			if a.loader.ReloadInterval != test.expectedReloadInterval {
 				t.Fatalf("Test %d expected %v, got %v", i, test.expectedReloadInterval, a.loader.ReloadInterval)
 			}
-			if a.loader.duration != test.expectedDuration {
-				t.Fatalf("Test %d expected %v, got %v", i, test.expectedDuration, a.loader.duration)
-			}
 			if test.expectedTo != nil {
 				for j, got := range a.loader.transferTo {
 					if got != test.expectedTo[j] {

plugin/file/README.md

@@ -28,7 +28,6 @@ If you want to round-robin A and AAAA responses look at the *loadbalance* plugin
 file DBFILE [ZONES... ] {
     transfer to ADDRESS...
     reload DURATION
-    no_reload
     upstream
 }
 ~~~
@@ -40,7 +39,6 @@ file DBFILE [ZONES... ] {
 * `reload` interval to perform a reload of the zone if the SOA version changes. Default is one minute.
   Value of `0` means to not scan for changes and reload. For example, `30s` checks the zonefile every 30 seconds
   and reloads the zone when serial changes.
-* `no_reload` deprecated. Sets reload to 0.
 * `upstream` resolve external names found (think CNAMEs) pointing to external names. This is only
   really useful when CoreDNS is configured as a proxy; for normal authoritative serving you don't
   need *or* want to use this. CoreDNS will resolve CNAMEs against itself.

plugin/file/setup.go

@@ -112,10 +112,6 @@ func fileParse(c *caddy.Controller) (Zones, error) {
 				}
 				reload = d
 
-			case "no_reload":
-				log.Warning("NO_RELOAD of directory is deprecated. Use RELOAD (set to 0) instead. See https://coredns.io/plugins/file/#syntax")
-				reload = 0
-
 			case "upstream":
 				// ignore args, will be error later.
 				c.RemainingArgs() // clear buffer

plugin/file/setup_test.go

@@ -26,18 +26,6 @@ func TestFileParse(t *testing.T) {
 		shouldErr      bool
 		expectedZones  Zones
 	}{
-		{
-			`file ` + zoneFileName1 + ` miek.nl {
-				transfer from 127.0.0.1
-			}`,
-			true,
-			Zones{},
-		},
-		{
-			`file`,
-			true,
-			Zones{},
-		},
 		{
 			`file ` + zoneFileName1 + ` miek.nl.`,
 			false,
@@ -60,12 +48,32 @@ func TestFileParse(t *testing.T) {
 			false, // OK for now as we disregard any options for the `upstream`.
 			Zones{Names: []string{"example.net."}},
 		},
+		// errors.
+		{
+			`file ` + zoneFileName1 + ` miek.nl {
+				transfer from 127.0.0.1
+			}`,
+			true,
+			Zones{},
+		},
+		{
+			`file`,
+			true,
+			Zones{},
+		},
+		{
+			`file ` + zoneFileName1 + ` example.net. {
+				no_reload
+			}`,
+			true,
+			Zones{},
+		},
 		{
 			`file ` + zoneFileName1 + ` example.net. {
 				no_rebloat
 			}`,
 			true,
-			Zones{Names: []string{}},
+			Zones{},
 		},
 	}
 

test/auto_test.go

@@ -19,7 +19,8 @@ func TestAuto(t *testing.T) {
 
 	corefile := `org:0 {
 		auto {
-			directory ` + tmpdir + ` db\.(.*) {1} 1
+			directory ` + tmpdir + ` db\.(.*) {1}
+			reload 1s
 		}
 	}
 `
@@ -77,7 +78,8 @@ func TestAutoNonExistentZone(t *testing.T) {
 
 	corefile := `.:0 {
 		auto {
-			directory ` + tmpdir + ` (.*) {1} 1
+			directory ` + tmpdir + ` (.*) {1}
+			reload 1s
 		}
 		errors stdout
 	}
@@ -115,7 +117,8 @@ func TestAutoAXFR(t *testing.T) {
 
 	corefile := `org:0 {
 		auto {
-			directory ` + tmpdir + ` db\.(.*) {1} 1
+			directory ` + tmpdir + ` db\.(.*) {1}
+			reload 1s
 			transfer to *
 		}
 	}

test/metrics_test.go

@@ -77,7 +77,8 @@ func TestMetricsAuto(t *testing.T) {
 
 	corefile := `org:0 {
 		auto {
-			directory ` + tmpdir + ` db\.(.*) {1} 1
+			directory ` + tmpdir + ` db\.(.*) {1}
+			reload 1s
 		}
 		prometheus localhost:0
 	}