From 31103062963fd0811d0c5dca26ecdc934d846276 Mon Sep 17 00:00:00 2001
From: Miek Gieben <miek@miek.nl>
Date: Tue, 7 Jun 2016 20:57:45 +0100
Subject: [PATCH] middleware/etcd: Return json parsing errors (#158)

When coredns unmarshals a json value and it fails it will put the error
in the returned message iff the query was a debug query
(o-o.debug.<REST>).
---
 middleware/etcd/README.md  |  9 ++++++++-
 middleware/etcd/debug.go   | 18 ++++++++++++++++++
 middleware/etcd/etcd.go    |  3 ++-
 middleware/etcd/handler.go | 19 +++++++++++++------
 4 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/middleware/etcd/README.md b/middleware/etcd/README.md
index f5a373811..c3717ae58 100644
--- a/middleware/etcd/README.md
+++ b/middleware/etcd/README.md
@@ -39,7 +39,7 @@ etcd [zones...] {
   pointing to external names. If you want CoreDNS to act as a proxy for clients you'll need to add
   the proxy middleware.
 * `tls` followed the cert, key and the CA's cert filenames.
-* `debug` allow debug queries. Prefix the name with `o-o.debug.` to reveive extra information in the
+* `debug` allow debug queries. Prefix the name with `o-o.debug.` to retrieve extra information in the
   additional section of the reply in the form of text records:
 
     skydns.test.skydns.dom.a.	300	CH	TXT	"127.0.0.1:0(10,0,,false)[0,]"
@@ -47,6 +47,13 @@ etcd [zones...] {
   This shows the complete key as the owername, the rdata of the TXT record has:
   `host:port(priority,weight,txt content,mail)[targetstrip,group]`.
 
+  Any errors seen doing parsing will show up like this:
+
+    . 0 CH TXT "/skydns/local/skydns/r/a: invalid character '.' after object key:value pair"
+
+  which shows `a.r.skydns.local.` has a json encoding problem.
+
+
 ## Examples
 
 This is the default SkyDNS setup, with everying specified in full:
diff --git a/middleware/etcd/debug.go b/middleware/etcd/debug.go
index b8ca3344f..58e896680 100644
--- a/middleware/etcd/debug.go
+++ b/middleware/etcd/debug.go
@@ -36,3 +36,21 @@ func servicesToTxt(debug []msg.Service) []dns.RR {
 	}
 	return rr
 }
+
+func errorToTxt(err error) dns.RR {
+	if err == nil {
+		return nil
+	}
+	msg := err.Error()
+	if len(msg) > 255 {
+		msg = msg[:255]
+	}
+	t := new(dns.TXT)
+	t.Hdr.Class = dns.ClassCHAOS
+	t.Hdr.Ttl = 0
+	t.Hdr.Rrtype = dns.TypeTXT
+	t.Hdr.Name = "."
+
+	t.Txt = []string{msg}
+	return t
+}
diff --git a/middleware/etcd/etcd.go b/middleware/etcd/etcd.go
index 4176f1cb8..01cd3e13f 100644
--- a/middleware/etcd/etcd.go
+++ b/middleware/etcd/etcd.go
@@ -3,6 +3,7 @@ package etcd
 
 import (
 	"encoding/json"
+	"fmt"
 	"strings"
 	"time"
 
@@ -104,7 +105,7 @@ Nodes:
 		}
 		serv := new(msg.Service)
 		if err := json.Unmarshal([]byte(n.Value), serv); err != nil {
-			return nil, err
+			return nil, fmt.Errorf("%s: %s", n.Key, err.Error())
 		}
 		b := msg.Service{Host: serv.Host, Port: serv.Port, Priority: serv.Priority, Weight: serv.Weight, Text: serv.Text, Key: n.Key}
 		if _, ok := bx[b]; ok {
diff --git a/middleware/etcd/handler.go b/middleware/etcd/handler.go
index bf87afcd6..d27b274cd 100644
--- a/middleware/etcd/handler.go
+++ b/middleware/etcd/handler.go
@@ -82,14 +82,14 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 	}
 
 	if isEtcdNameError(err) {
-		return e.Err(zone, dns.RcodeNameError, state, debug)
+		return e.Err(zone, dns.RcodeNameError, state, debug, err)
 	}
 	if err != nil {
-		return dns.RcodeServerFailure, err
+		return e.Err(zone, dns.RcodeServerFailure, state, debug, err)
 	}
 
 	if len(records) == 0 {
-		return e.Err(zone, dns.RcodeSuccess, state, debug)
+		return e.Err(zone, dns.RcodeSuccess, state, debug, err)
 	}
 
 	m := new(dns.Msg)
@@ -109,15 +109,22 @@ func (e Etcd) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 }
 
 // Err write an error response to the client.
-func (e Etcd) Err(zone string, rcode int, state middleware.State, debug []msg.Service) (int, error) {
+func (e Etcd) Err(zone string, rcode int, state middleware.State, debug []msg.Service, err error) (int, error) {
 	m := new(dns.Msg)
 	m.SetRcode(state.Req, rcode)
 	m.Authoritative, m.RecursionAvailable, m.Compress = true, true, true
 	m.Ns, _, _ = e.SOA(zone, state)
-	m.Extra = servicesToTxt(debug)
+	if e.debug != "" {
+		m.Extra = servicesToTxt(debug)
+		txt := errorToTxt(err)
+		if txt != nil {
+			m.Extra = append(m.Extra, errorToTxt(err))
+		}
+	}
 	state.SizeAndDo(m)
 	state.W.WriteMsg(m)
-	return rcode, nil
+	// Return success as the rcode to signal we have written to the client.
+	return dns.RcodeSuccess, nil
 }
 
 func dedup(m *dns.Msg) *dns.Msg {