Fix middleware log

middleware/log/log.go

@@ -18,36 +18,36 @@ type Logger struct {
 }
 
 func (l Logger) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+	state := middleware.State{W: w, Req: r}
 	for _, rule := range l.Rules {
-		/*
-			if middleware.Path(r.URL.Path).Matches(rule.PathScope) {
-				responseRecorder := middleware.NewResponseRecorder(w)
-				status, err := l.Next.ServeHTTP(responseRecorder, r)
-				if status >= 400 {
-					// There was an error up the chain, but no response has been written yet.
-					// The error must be handled here so the log entry will record the response size.
-					if l.ErrorFunc != nil {
-						l.ErrorFunc(responseRecorder, r, status)
-					} else {
-						// Default failover error handler
-						responseRecorder.WriteHeader(status)
-						fmt.Fprintf(responseRecorder, "%d %s", status, http.StatusText(status))
-					}
-					status = 0
+		if middleware.Name(state.Name()).Matches(rule.NameScope) {
+			responseRecorder := middleware.NewResponseRecorder(w)
+			rcode, err := l.Next.ServeDNS(ctx, responseRecorder, r)
+			if rcode > 0 {
+				// There was an error up the chain, but no response has been written yet.
+				// The error must be handled here so the log entry will record the response size.
+				if l.ErrorFunc != nil {
+					l.ErrorFunc(responseRecorder, r, rcode)
+				} else {
+					// Default failover error handler
+					answer := new(dns.Msg)
+					answer.SetRcode(r, rcode)
+					w.WriteMsg(answer)
 				}
-				rep := middleware.NewReplacer(r, responseRecorder, CommonLogEmptyValue)
-				rule.Log.Println(rep.Replace(rule.Format))
-				return status, err
+				rcode = 0
 			}
-		*/
-		rule = rule
+			rep := middleware.NewReplacer(r, responseRecorder, CommonLogEmptyValue)
+			rule.Log.Println(rep.Replace(rule.Format))
+			return rcode, err
+
+		}
 	}
 	return l.Next.ServeDNS(ctx, w, r)
 }
 
 // Rule configures the logging middleware.
 type Rule struct {
-	PathScope  string
+	NameScope  string
 	OutputFile string
 	Format     string
 	Log        *log.Logger
@@ -56,13 +56,13 @@ type Rule struct {
 
 const (
 	// DefaultLogFilename is the default log filename.
-	DefaultLogFilename = "access.log"
+	DefaultLogFilename = "query.log"
 	// CommonLogFormat is the common log format.
 	CommonLogFormat = `{remote} ` + CommonLogEmptyValue + ` [{when}] "{type} {name} {proto}" {rcode} {size}`
 	// CommonLogEmptyValue is the common empty log value.
 	CommonLogEmptyValue = "-"
 	// CombinedLogFormat is the combined log format.
-	CombinedLogFormat = CommonLogFormat + ` "{>Referer}" "{>User-Agent}"` // Something here as well
+	CombinedLogFormat = CommonLogFormat + ` "{>opcode}"`
 	// DefaultLogFormat is the default log format.
 	DefaultLogFormat = CommonLogFormat
 )

middleware/log/log_test.go

@@ -1,17 +1,27 @@
 package log
 
-/*
+import (
+	"bytes"
+	"log"
+	"strings"
+	"testing"
+
+	"github.com/miekg/coredns/middleware"
+	"github.com/miekg/dns"
+	"golang.org/x/net/context"
+)
+
 type erroringMiddleware struct{}
 
-func (erroringMiddleware) ServeDNS(w dns.ResponseWriter, r *dns.Msg) (int, error) {
-	return http.StatusNotFound, nil
+func (erroringMiddleware) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+	return dns.RcodeServerFailure, nil
 }
 
 func TestLoggedStatus(t *testing.T) {
 	var f bytes.Buffer
 	var next erroringMiddleware
 	rule := Rule{
-		PathScope: "/",
+		NameScope: ".",
 		Format:    DefaultLogFormat,
 		Log:       log.New(&f, "", 0),
 	}
@@ -21,21 +31,19 @@ func TestLoggedStatus(t *testing.T) {
 		Next:  next,
 	}
 
-	r, err := http.NewRequest("GET", "/", nil)
-	if err != nil {
-		t.Fatal(err)
-	}
+	ctx := context.TODO()
+	r := new(dns.Msg)
+	r.SetQuestion("example.org.", dns.TypeA)
 
-	rec := httptest.NewRecorder()
+	rec := middleware.NewResponseRecorder(&middleware.TestResponseWriter{})
 
-	status, err := logger.ServeHTTP(rec, r)
-	if status != 0 {
-		t.Error("Expected status to be 0 - was", status)
+	rcode, _ := logger.ServeDNS(ctx, rec, r)
+	if rcode != 0 {
+		t.Error("Expected rcode to be 0 - was", rcode)
 	}
 
 	logged := f.String()
-	if !strings.Contains(logged, "404 13") {
-		t.Error("Expected 404 to be logged. Logged string -", logged)
+	if !strings.Contains(logged, "A example.org. udp") {
+		t.Error("Expected it to be logged. Logged string -", logged)
 	}
 }
-*/

middleware/name.go

@@ -0,0 +1,15 @@
+package middleware
+
+import "strings"
+
+// Name represents a domain name.
+type Name string
+
+// Matches checks to see if other matches n.
+//
+// Name matching will probably not always be a direct
+// comparison; this method assures that names can be
+// easily and consistently matched.
+func (n Name) Matches(other string) bool {
+	return strings.HasSuffix(string(n), other)
+}

middleware/path.go

@@ -1,18 +0,0 @@
-package middleware
-
-import "strings"
-
-
-// TODO(miek): matches for names.
-
-// Path represents a URI path, maybe with pattern characters.
-type Path string
-
-// Matches checks to see if other matches p.
-//
-// Path matching will probably not always be a direct
-// comparison; this method assures that paths can be
-// easily and consistently matched.
-func (p Path) Matches(other string) bool {
-	return strings.HasPrefix(string(p), other)
-}

middleware/proxy/upstream.go

@@ -4,12 +4,10 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
-	"path"
 	"strconv"
 	"time"
 
 	"github.com/miekg/coredns/core/parse"
-	"github.com/miekg/coredns/middleware"
 )
 
 var (
@@ -226,10 +224,13 @@ func (u *staticUpstream) Select() *UpstreamHost {
 }
 
 func (u *staticUpstream) IsAllowedPath(requestPath string) bool {
-	for _, ignoredSubPath := range u.IgnoredSubPaths {
-		if middleware.Path(path.Clean(requestPath)).Matches(path.Join(u.From(), ignoredSubPath)) {
-			return false
+	/*
+		TODO(miek): fix to use Name
+		for _, ignoredSubPath := range u.IgnoredSubPaths {
+			if middleware.Path(path.Clean(requestPath)).Matches(path.Join(u.From(), ignoredSubPath)) {
+				return false
+			}
 		}
-	}
+	*/
 	return true
 }

middleware/recorder.go

@@ -1,6 +1,7 @@
 package middleware
 
 import (
+	"net"
 	"time"
 
 	"github.com/miekg/dns"
@@ -68,3 +69,24 @@ func (r *ResponseRecorder) Hijack() {
 	r.ResponseWriter.Hijack()
 	return
 }
+
+type TestResponseWriter struct{}
+
+func (t *TestResponseWriter) LocalAddr() net.Addr {
+	ip := net.ParseIP("127.0.0.1")
+	port := 53
+	return &net.UDPAddr{IP: ip, Port: port, Zone: ""}
+}
+
+func (t *TestResponseWriter) RemoteAddr() net.Addr {
+	ip := net.ParseIP("10.240.0.1")
+	port := 40212
+	return &net.UDPAddr{IP: ip, Port: port, Zone: ""}
+}
+
+func (t *TestResponseWriter) WriteMsg(m *dns.Msg) error     { return nil }
+func (t *TestResponseWriter) Write(buf []byte) (int, error) { return len(buf), nil }
+func (t *TestResponseWriter) Close() error                  { return nil }
+func (t *TestResponseWriter) TsigStatus() error             { return nil }
+func (t *TestResponseWriter) TsigTimersOnly(bool)           { return }
+func (t *TestResponseWriter) Hijack()                       { return }

middleware/replacer.go

@@ -53,6 +53,11 @@ func NewReplacer(r *dns.Msg, rr *ResponseRecorder, emptyValue string) Replacer {
 		rep.replacements["{latency}"] = time.Since(rr.start).String()
 	}
 
+	// Header placeholders (case-insensitive)
+	// TODO(miek): syntax for flags
+	rep.replacements[headerReplacer+"id}"] = strconv.Itoa(int(r.Id))
+	rep.replacements[headerReplacer+"opcode}"] = strconv.Itoa(int(r.Opcode))
+
 	return rep
 }