plugin/timeouts - Allow ability to configure listening server timeouts (#5784)

5 files changed, 297 insertions, 0 deletions

diff --git a/plugin/pkg/durations/durations.go b/plugin/pkg/durations/durations.go
new file mode 100644
index 000000000..37771e79d
--- /dev/null
+++ b/plugin/pkg/durations/durations.go
@@ -0,0 +1,26 @@
+package durations
+
+import (
+	"fmt"
+	"strconv"
+	"time"
+)
+
+// NewDurationFromArg returns a time.Duration from a configuration argument
+// (string) which has come from the Corefile. The argument has some basic
+// validation applied before returning a time.Duration. If the argument has no
+// time unit specified and is numeric the argument will be treated as seconds
+// rather than GO's default of nanoseconds.
+func NewDurationFromArg(arg string) (time.Duration, error) {
+	_, err := strconv.Atoi(arg)
+	if err == nil {
+		arg = arg + "s"
+	}
+
+	d, err := time.ParseDuration(arg)
+	if err != nil {
+		return 0, fmt.Errorf("failed to parse duration '%s'", arg)
+	}
+
+	return d, nil
+}
diff --git a/plugin/pkg/durations/durations_test.go b/plugin/pkg/durations/durations_test.go
new file mode 100644
index 000000000..12008a713
--- /dev/null
+++ b/plugin/pkg/durations/durations_test.go
@@ -0,0 +1,51 @@
+package durations
+
+import (
+	"testing"
+	"time"
+)
+
+func TestNewDurationFromArg(t *testing.T) {
+	tests := []struct {
+		name    string
+		arg     string
+		wantErr bool
+		want    time.Duration
+	}{
+		{
+			name: "valid GO duration - seconds",
+			arg:  "30s",
+			want: 30 * time.Second,
+		},
+		{
+			name: "valid GO duration - minutes",
+			arg:  "2m",
+			want: 2 * time.Minute,
+		},
+		{
+			name: "number - fallback to seconds",
+			arg:  "30",
+			want: 30 * time.Second,
+		},
+		{
+			name:    "invalid duration",
+			arg:     "twenty seconds",
+			wantErr: true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual, err := NewDurationFromArg(test.arg)
+			if test.wantErr && err == nil {
+				t.Error("error was expected")
+			}
+			if !test.wantErr && err != nil {
+				t.Error("error was not expected")
+			}
+
+			if test.want != actual {
+				t.Errorf("expected '%v' got '%v'", test.want, actual)
+			}
+		})
+	}
+}
diff --git a/plugin/timeouts/README.md b/plugin/timeouts/README.md
new file mode 100644
index 000000000..098c9ccac
--- /dev/null
+++ b/plugin/timeouts/README.md
@@ -0,0 +1,76 @@
+# timeouts
+
+## Name
+
+*timeouts* - allows you to configure the server read, write and idle timeouts for the TCP, TLS and DoH servers.
+
+## Description
+
+CoreDNS is configured with sensible timeouts for server connections by default.
+However in some cases for example where CoreDNS is serving over a slow mobile
+data connection the default timeouts are not optimal.
+
+Additionally some routers hold open connections when using DNS over TLS or DNS
+over HTTPS. Allowing a longer idle timeout helps performance and reduces issues
+with such routers.
+
+The *timeouts* "plugin" allows you to configure CoreDNS server read, write and
+idle timeouts.
+
+## Syntax
+
+~~~ txt
+timeouts {
+	read DURATION
+	write DURATION
+	idle DURATION
+}
+~~~
+
+For any timeouts that are not provided, default values are used which may vary
+depending on the server type. At least one timeout must be specified otherwise
+the entire timeouts block should be omitted.
+
+## Examples
+
+Start a DNS-over-TLS server that picks up incoming DNS-over-TLS queries on port
+5553 and uses the nameservers defined in `/etc/resolv.conf` to resolve the
+query. This proxy path uses plain old DNS. A 10 second read timeout, 20
+second write timeout and a 60 second idle timeout have been configured.
+
+~~~
+tls://.:5553 {
+	tls cert.pem key.pem ca.pem
+	timeouts {
+		read 10s
+		write 20s
+		idle 60s
+	}
+	forward . /etc/resolv.conf
+}
+~~~
+
+Start a DNS-over-HTTPS server that is similar to the previous example. Only the
+read timeout has been configured for 1 minute.
+
+~~~
+https://. {
+	tls cert.pem key.pem ca.pem
+	timeouts {
+		read 1m
+	}
+	forward . /etc/resolv.conf
+}
+~~~
+
+Start a standard TCP/UDP server on port 1053. A read and write timeout has been
+configured. The timeouts are only applied to the TCP side of the server.
+~~~
+.:1053 {
+	timeouts {
+		read 15s
+                write 30s
+	}
+	forward . /etc/resolv.conf
+}
+~~~
diff --git a/plugin/timeouts/timeouts.go b/plugin/timeouts/timeouts.go
new file mode 100644
index 000000000..eea6a6488
--- /dev/null
+++ b/plugin/timeouts/timeouts.go
@@ -0,0 +1,69 @@
+package timeouts
+
+import (
+	"time"
+
+	"github.com/coredns/caddy"
+	"github.com/coredns/coredns/core/dnsserver"
+	"github.com/coredns/coredns/plugin"
+	"github.com/coredns/coredns/plugin/pkg/durations"
+)
+
+func init() { plugin.Register("timeouts", setup) }
+
+func setup(c *caddy.Controller) error {
+	err := parseTimeouts(c)
+	if err != nil {
+		return plugin.Error("timeouts", err)
+	}
+	return nil
+}
+
+func parseTimeouts(c *caddy.Controller) error {
+	config := dnsserver.GetConfig(c)
+
+	for c.Next() {
+		args := c.RemainingArgs()
+		if len(args) > 0 {
+			return plugin.Error("timeouts", c.ArgErr())
+		}
+
+		b := 0
+		for c.NextBlock() {
+			block := c.Val()
+			timeoutArgs := c.RemainingArgs()
+			if len(timeoutArgs) != 1 {
+				return c.ArgErr()
+			}
+
+			timeout, err := durations.NewDurationFromArg(timeoutArgs[0])
+			if err != nil {
+				return c.Err(err.Error())
+			}
+
+			if timeout < (1*time.Second) || timeout > (24*time.Hour) {
+				return c.Errf("timeout provided '%s' needs to be between 1 second and 24 hours", timeout)
+			}
+
+			switch block {
+			case "read":
+				config.ReadTimeout = timeout
+
+			case "write":
+				config.WriteTimeout = timeout
+
+			case "idle":
+				config.IdleTimeout = timeout
+
+			default:
+				return c.Errf("unknown option: '%s'", block)
+			}
+			b++
+		}
+
+		if b == 0 {
+			return plugin.Error("timeouts", c.Err("timeouts block with no timeouts specified"))
+		}
+	}
+	return nil
+}
diff --git a/plugin/timeouts/timeouts_test.go b/plugin/timeouts/timeouts_test.go
new file mode 100644
index 000000000..c01d3a072
--- /dev/null
+++ b/plugin/timeouts/timeouts_test.go
@@ -0,0 +1,75 @@
+package timeouts
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/coredns/caddy"
+)
+
+func TestTimeouts(t *testing.T) {
+	tests := []struct {
+		input              string
+		shouldErr          bool
+		expectedRoot       string // expected root, set to the controller. Empty for negative cases.
+		expectedErrContent string // substring from the expected error. Empty for positive cases.
+	}{
+		// positive
+		{`timeouts {
+			read 30s
+		}`, false, "", ""},
+		{`timeouts {
+			read 1m
+			write 2m
+		}`, false, "", ""},
+		{` timeouts {
+			idle 1h
+		}`, false, "", ""},
+		{`timeouts {
+			read 10
+			write 20
+			idle 60
+		}`, false, "", ""},
+		// negative
+		{`timeouts`, true, "", "block with no timeouts specified"},
+		{`timeouts {
+		}`, true, "", "block with no timeouts specified"},
+		{`timeouts {
+			read 10s
+			giraffe 30s
+		}`, true, "", "unknown option"},
+		{`timeouts {
+			read 10s 20s
+			write 30s
+		}`, true, "", "Wrong argument"},
+		{`timeouts {
+			write snake
+		}`, true, "", "failed to parse duration"},
+		{`timeouts {
+			idle 0s
+		}`, true, "", "needs to be between"},
+		{`timeouts {
+			read 48h
+		}`, true, "", "needs to be between"},
+	}
+
+	for i, test := range tests {
+		c := caddy.NewTestController("dns", test.input)
+		err := setup(c)
+		//cfg := dnsserver.GetConfig(c)
+
+		if test.shouldErr && err == nil {
+			t.Errorf("Test %d: Expected error but found %s for input %s", i, err, test.input)
+		}
+
+		if err != nil {
+			if !test.shouldErr {
+				t.Errorf("Test %d: Expected no error but found one for input %s. Error was: %v", i, test.input, err)
+			}
+
+			if !strings.Contains(err.Error(), test.expectedErrContent) {
+				t.Errorf("Test %d: Expected error to contain: %v, found error: %v, input: %s", i, test.expectedErrContent, err, test.input)
+			}
+		}
+	}
+}