middleware/{log,errors}: output everything to stdout (#684)

Limit the options in both errors and log middleware, just output to
stdout and let someone else (journald,docker) care about where to route
the logs. This removes syslog and logging to a file.

Fixes #573 #602

5 files changed, 28 insertions, 148 deletions

diff --git a/middleware/errors/README.md b/middleware/errors/README.md
index aa3b170af..461cdfec8 100644
--- a/middleware/errors/README.md
+++ b/middleware/errors/README.md
@@ -6,35 +6,21 @@ TODO: what are errors.
 ## Syntax
 
 ~~~
-errors [LOGFILE]
+errors [FILE]
 ~~~
 
-* **LOGFILE** is the path to the error log file to create (or append to), relative to the current
-  working directory. It can also be `stdout` or `stderr` to write to the console, syslog to write to the
-  system log (except on Windows), or visible to write the error (including full stack trace, if
-  applicable) to the response. Writing errors to the response is NOT advised except in local debug
-  situations. The default is stderr. The above syntax will simply enable error reporting on the
-  server. To specify custom error pages, open a block:
-
-~~~
-errors {
-    what where
-}
-~~~
-
-* `what` can only be `log`.
-* `where` is the path to the log file (as described above) and you can enable rotation to manage the log files.
+* **FILE** is the log file to create (or append to). The *only* valid name for **FILE** is *stdout*
 
 ## Examples
 
-Log errors into a file in the parent directory:
+Log errors to *stdout*.
 
 ~~~
-errors ../error.log
+errors
 ~~~
 
-Make errors visible to the client (for debugging only):
+Log errors to *stdout*.
 
 ~~~
-errors visible
+errors stdout
 ~~~
diff --git a/middleware/errors/errors.go b/middleware/errors/errors.go
index d2b6f3a22..057e65089 100644
--- a/middleware/errors/errors.go
+++ b/middleware/errors/errors.go
@@ -20,7 +20,6 @@ type errorHandler struct {
 	Next    middleware.Handler
 	LogFile string
 	Log     *log.Logger
-	Debug   bool // if true, errors are written out to client rather than to a log
 }
 
 // ServeDNS implements the middleware.Handler interface.
@@ -33,15 +32,6 @@ func (h errorHandler) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns
 		state := request.Request{W: w, Req: r}
 		errMsg := fmt.Sprintf("%s [ERROR %d %s %s] %v", time.Now().Format(timeFormat), rcode, state.Name(), state.Type(), err)
 
-		if h.Debug {
-			// Write error to response as a txt message instead of to log
-			answer := debugMsg(rcode, r)
-			txt, _ := dns.NewRR(". IN 0 TXT " + errMsg)
-			answer.Answer = append(answer.Answer, txt)
-			state.SizeAndDo(answer)
-			w.WriteMsg(answer)
-			return 0, err
-		}
 		h.Log.Println(errMsg)
 	}
 
@@ -56,7 +46,6 @@ func (h errorHandler) recovery(ctx context.Context, w dns.ResponseWriter, r *dns
 		return
 	}
 
-	state := request.Request{W: w, Req: r}
 	// Obtain source of panic
 	// From: https://gist.github.com/swdunlop/9629168
 	var name, file string // function name, file name
@@ -83,20 +72,8 @@ func (h errorHandler) recovery(ctx context.Context, w dns.ResponseWriter, r *dns
 	}
 
 	panicMsg := fmt.Sprintf("%s [PANIC %s %s] %s:%d - %v", time.Now().Format(timeFormat), r.Question[0].Name, dns.Type(r.Question[0].Qtype), file, line, rec)
-	if h.Debug {
-		// Write error and stack trace to the response rather than to a log
-		var stackBuf [4096]byte
-		stack := stackBuf[:runtime.Stack(stackBuf[:], false)]
-		answer := debugMsg(dns.RcodeServerFailure, r)
-		// add stack buf in TXT, limited to 255 chars for now.
-		txt, _ := dns.NewRR(". IN 0 TXT " + string(stack[:255]))
-		answer.Answer = append(answer.Answer, txt)
-		state.SizeAndDo(answer)
-		w.WriteMsg(answer)
-	} else {
-		// Currently we don't use the function name, since file:line is more conventional
-		h.Log.Printf(panicMsg)
-	}
+	// Currently we don't use the function name, since file:line is more conventional
+	h.Log.Printf(panicMsg)
 }
 
 // debugMsg creates a debug message that gets send back to the client.
diff --git a/middleware/errors/errors_test.go b/middleware/errors/errors_test.go
index 1d389ba47..5e565964c 100644
--- a/middleware/errors/errors_test.go
+++ b/middleware/errors/errors_test.go
@@ -66,30 +66,6 @@ func TestErrors(t *testing.T) {
 	}
 }
 
-func TestVisibleErrorWithPanic(t *testing.T) {
-	const panicMsg = "I'm a panic"
-	eh := errorHandler{
-		Debug: true,
-		Next: middleware.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
-			panic(panicMsg)
-		}),
-	}
-
-	ctx := context.TODO()
-	req := new(dns.Msg)
-	req.SetQuestion("example.org.", dns.TypeA)
-
-	rec := dnsrecorder.New(&test.ResponseWriter{})
-
-	code, err := eh.ServeDNS(ctx, rec, req)
-	if code != 0 {
-		t.Errorf("Expected error handler to return 0 (it should write to response), got status %d", code)
-	}
-	if err != nil {
-		t.Errorf("Expected error handler to return nil error (it should panic!), but got '%v'", err)
-	}
-}
-
 func genErrorHandler(rcode int, err error) middleware.Handler {
 	return middleware.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
 		return rcode, err
diff --git a/middleware/errors/setup.go b/middleware/errors/setup.go
index 787b89724..af9a7bbff 100644
--- a/middleware/errors/setup.go
+++ b/middleware/errors/setup.go
@@ -1,14 +1,13 @@
 package errors
 
 import (
-	"io"
+	"fmt"
 	"log"
 	"os"
 
 	"github.com/coredns/coredns/core/dnsserver"
 	"github.com/coredns/coredns/middleware"
 
-	"github.com/hashicorp/go-syslog"
 	"github.com/mholt/caddy"
 )
 
@@ -25,34 +24,7 @@ func setup(c *caddy.Controller) error {
 		return middleware.Error("errors", err)
 	}
 
-	var writer io.Writer
-
-	switch handler.LogFile {
-	case "visible":
-		handler.Debug = true
-	case "stdout":
-		writer = os.Stdout
-	case "stderr":
-		writer = os.Stderr
-	case "syslog":
-		writer, err = gsyslog.NewLogger(gsyslog.LOG_ERR, "LOCAL0", "coredns")
-		if err != nil {
-			return middleware.Error("errors", err)
-		}
-	default:
-		if handler.LogFile == "" {
-			writer = os.Stderr // default
-			break
-		}
-
-		var file *os.File
-		file, err = os.OpenFile(handler.LogFile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
-		if err != nil {
-			return middleware.Error("errors", err)
-		}
-		writer = file
-	}
-	handler.Log = log.New(writer, "", 0)
+	handler.Log = log.New(os.Stdout, "", 0)
 
 	dnsserver.GetConfig(c).AddMiddleware(func(next middleware.Handler) middleware.Handler {
 		handler.Next = next
@@ -65,47 +37,19 @@ func setup(c *caddy.Controller) error {
 func errorsParse(c *caddy.Controller) (errorHandler, error) {
 	handler := errorHandler{}
 
-	optionalBlock := func() (bool, error) {
-		var hadBlock bool
-
-		for c.NextBlock() {
-			hadBlock = true
-
-			what := c.Val()
-			if !c.NextArg() {
-				return hadBlock, c.ArgErr()
-			}
-			where := c.Val()
-
-			if what == "log" {
-				if where == "visible" {
-					handler.Debug = true
-				} else {
-					handler.LogFile = where
-				}
-			}
-		}
-		return hadBlock, nil
-	}
-
 	for c.Next() {
-		// Configuration may be in a block
-		hadBlock, err := optionalBlock()
-		if err != nil {
-			return handler, err
-		}
-
-		// Otherwise, the only argument would be an error log file name or 'visible'
-		if !hadBlock {
-			if c.NextArg() {
-				if c.Val() == "visible" {
-					handler.Debug = true
-				} else {
-					handler.LogFile = c.Val()
-				}
+		args := c.RemainingArgs()
+		switch len(args) {
+		case 0:
+			handler.LogFile = "stdout"
+		case 1:
+			if args[0] != "stdout" {
+				return handler, fmt.Errorf("invalid log file: %s", args[0])
 			}
+			handler.LogFile = args[0]
+		default:
+			return handler, c.ArgErr()
 		}
 	}
-
 	return handler, nil
 }
diff --git a/middleware/errors/setup_test.go b/middleware/errors/setup_test.go
index adae0009c..bae85da32 100644
--- a/middleware/errors/setup_test.go
+++ b/middleware/errors/setup_test.go
@@ -13,18 +13,19 @@ func TestErrorsParse(t *testing.T) {
 		expectedErrorHandler errorHandler
 	}{
 		{`errors`, false, errorHandler{
-			LogFile: "",
+			LogFile: "stdout",
 		}},
-		{`errors errors.txt`, false, errorHandler{
-			LogFile: "errors.txt",
+		{`errors stdout`, false, errorHandler{
+			LogFile: "stdout",
 		}},
-		{`errors visible`, false, errorHandler{
+		{`errors errors.txt`, true, errorHandler{
 			LogFile: "",
-			Debug:   true,
 		}},
-		{`errors { log visible }`, false, errorHandler{
+		{`errors visible`, true, errorHandler{
 			LogFile: "",
-			Debug:   true,
+		}},
+		{`errors { log visible }`, true, errorHandler{
+			LogFile: "stdout",
 		}},
 	}
 	for i, test := range tests {
@@ -40,9 +41,5 @@ func TestErrorsParse(t *testing.T) {
 			t.Errorf("Test %d expected LogFile to be %s, but got %s",
 				i, test.expectedErrorHandler.LogFile, actualErrorsRule.LogFile)
 		}
-		if actualErrorsRule.Debug != test.expectedErrorHandler.Debug {
-			t.Errorf("Test %d expected Debug to be %v, but got %v",
-				i, test.expectedErrorHandler.Debug, actualErrorsRule.Debug)
-		}
 	}
 }