plugin/secondary: fix a bunch of things and tests (#1406)

Fix the error handling. Log when we have an error during any of the
transfer state. And if there isn't an error transfer the zones.

Also fix the tests in test/ so we, at least, check the initial transfer.

Update the docs to show more about how errors are handled.

Ref #1400

2 files changed, 45 insertions, 130 deletions

diff --git a/test/secondary_net_test.go b/test/secondary_net_test.go
deleted file mode 100644
index 451195442..000000000
--- a/test/secondary_net_test.go
+++ /dev/null
@@ -1,126 +0,0 @@
-// +build net
-
-package test
-
-import (
-	"testing"
-
-	"github.com/miekg/dns"
-)
-
-func TestSecondaryZoneTransfer(t *testing.T) {
-	/*
-		Test will only work when there is a CoreDNS running on part 32054
-		with example.com and willing to transfer
-		coredns -conf Corefile -dns.port 32054
-		Corefile:
-			example.com {
-			    file example.com {
-				transfer to 127.0.0.1:32053
-			    }
-			}
-		example.com:
-			example.com.		3600	IN	SOA	sns.dns.icann.org. noc.dns.icann.org. 2017042730 7200 3600 1209600 3600
-
-			example.com.		65118	IN	NS	a.iana-servers.net.
-			example.com.		65118	IN	NS	b.iana-servers.net.
-			cname.example.com.       434334  IN      CNAME   a.miek.nl.
-	*/
-
-	corefile := `example.com:32053 {
-		secondary {
-			transfer from 127.0.0.1:32054
-		}
-	}
-	`
-
-	sec, err := CoreDNSServer(corefile)
-	if err != nil {
-		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
-	}
-
-	defer sec.Stop()
-
-	m := new(dns.Msg)
-	m.SetQuestion("cname.example.com.", dns.TypeCNAME)
-
-	r, err := dns.Exchange(m, "127.0.0.1:32053")
-	if err != nil {
-		t.Fatalf("Expected to receive reply, but didn't: %s", err)
-	}
-
-	if len(r.Answer) == 0 {
-		t.Fatalf("Expected answer section")
-	}
-
-	if r.Answer[0].(*dns.CNAME).Target != "a.miek.nl." {
-		t.Fatalf("Expected target of %s, got %s", "a.miek.nl.", r.Answer[0].(*dns.CNAME).Target)
-	}
-
-	m = new(dns.Msg)
-	m.SetQuestion("example.com.", dns.TypeSOA)
-	r, err = dns.Exchange(m, "127.0.0.1:32053")
-	if err != nil {
-		t.Fatalf("Expected to receive reply, but didn't: %s", err)
-	}
-	if len(r.Answer) == 0 {
-		t.Fatalf("Expected answer section")
-	}
-	if r.Answer[0].(*dns.SOA).Serial != 2017042730 {
-		t.Fatalf("Expected serial of %d, got %d", 2017042730, r.Answer[0].(*dns.SOA).Serial)
-	}
-}
-
-func TestSecondaryZoneTransferUpstream(t *testing.T) {
-	/*
-		Test will only work when there is a CoreDNS running on part 32054
-		with example.com and willing to transfer
-		coredns -conf Corefile -dns.port 32054
-		Corefile:
-			example.com {
-			    file example.com {
-				transfer to 127.0.0.1:32053
-			    }
-			}
-		example.com:
-			example.com.		3600	IN	SOA	sns.dns.icann.org. noc.dns.icann.org. 2017042730 7200 3600 1209600 3600
-
-			example.com.		65118	IN	NS	a.iana-servers.net.
-			example.com.		65118	IN	NS	b.iana-servers.net.
-			cname.example.com.       434334  IN      CNAME   a.miek.nl.
-	*/
-
-	corefile := `example.com:32053 {
-		secondary {
-			transfer from 127.0.0.1:32054
-			upstream 8.8.8.8
-		}
-	}
-	`
-
-	sec, err := CoreDNSServer(corefile)
-	if err != nil {
-		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
-	}
-
-	defer sec.Stop()
-
-	m := new(dns.Msg)
-	m.SetQuestion("cname.example.com.", dns.TypeA)
-
-	r, err := dns.Exchange(m, "127.0.0.1:32053")
-	if err != nil {
-		t.Fatalf("Expected to receive reply, but didn't: %s", err)
-	}
-
-	if len(r.Answer) != 2 {
-		t.Fatalf("Expected answer section, with 2 records, got %d", len(r.Answer))
-	}
-
-	if r.Answer[0].(*dns.CNAME).Target != "a.miek.nl." {
-		t.Fatalf("Expected target of %s, got %s", "a.miek.nl.", r.Answer[0].(*dns.CNAME).Target)
-	}
-	if r.Answer[1].Header().Name != "a.miek.nl." {
-		t.Fatalf("Expected name of %s, got %s", "a.miek.nl.", r.Answer[1].Header().Name)
-	}
-}
diff --git a/test/secondary_test.go b/test/secondary_test.go
index 370d523c9..4b38a6208 100644
--- a/test/secondary_test.go
+++ b/test/secondary_test.go
@@ -1,8 +1,6 @@
 package test
 
 import (
-	"io/ioutil"
-	"log"
 	"testing"
 
 	"github.com/coredns/coredns/plugin/proxy"
@@ -27,8 +25,6 @@ func TestEmptySecondaryZone(t *testing.T) {
 	}
 	defer i.Stop()
 
-	log.SetOutput(ioutil.Discard)
-
 	p := proxy.NewLookup([]string{udp})
 	state := request.Request{W: &test.ResponseWriter{}, Req: new(dns.Msg)}
 
@@ -40,3 +36,48 @@ func TestEmptySecondaryZone(t *testing.T) {
 		t.Fatalf("Expected reply to be a SERVFAIL, got %d", resp.Rcode)
 	}
 }
+
+func TestSecondaryZoneTransfer(t *testing.T) {
+	name, rm, err := test.TempFile(".", exampleOrg)
+	if err != nil {
+		t.Fatalf("failed to create zone: %s", err)
+	}
+	defer rm()
+
+	corefile := `example.org:0 {
+       file ` + name + ` {
+	       transfer to *
+       }
+}
+`
+
+	i, _, tcp, err := CoreDNSServerAndPorts(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+	defer i.Stop()
+
+	corefile = `example.org:0 {
+		secondary {
+			transfer from ` + tcp + `
+		}
+}
+`
+	i1, udp, _, err := CoreDNSServerAndPorts(corefile)
+	if err != nil {
+		t.Fatalf("Could not get CoreDNS serving instance: %s", err)
+	}
+	defer i1.Stop()
+
+	m := new(dns.Msg)
+	m.SetQuestion("example.org.", dns.TypeSOA)
+
+	r, err := dns.Exchange(m, udp)
+	if err != nil {
+		t.Fatalf("Expected to receive reply, but didn't: %s", err)
+	}
+
+	if len(r.Answer) == 0 {
+		t.Fatalf("Expected answer section")
+	}
+}