Default to upstream to self (#2436)

* Default to upstream to self

This is a backwards incompatible change.

This is a massive (cleanup) PR where we default to resolving external
names by the coredns process itself, instead of directly forwarding them
to some upstream.

This ignores any arguments `upstream` may have had and makes it depend
on proxy/forward configuration in the Corefile. This allows resolved
upstream names to be cached and we have better healthchecking of the
upstreams. It also means there is only one way to resolve names, by
either using the proxy or forward plugin.

The proxy/forward lookup.go functions have been removed. This also
lessen the dependency on proxy, meaning deprecating proxy will become
easier. Some tests have been removed as well, or moved to the top-level
test directory as they now require a full coredns process instead of
just the plugin.

For the etcd plugin, the entire StubZone resolving is *dropped*! This
was a hacky (but working) solution to say the least. If someone cares
deeply it can be brought back (maybe)?

The pkg/upstream is now very small and almost does nothing. Also the
New() function was changed to return a pointer to upstream.Upstream. It
also returns only one parameter, so any stragglers using it will
encounter a compile error.

All documentation has been adapted. This affected the following plugins:
* etcd
* file
* auto
* secondary
* federation
* template
* route53

A followup PR will make any upstream directives with arguments an error,
right now they are ignored.

Signed-off-by: Miek Gieben <miek@miek.nl>

* Fix etcd build - probably still fails unit test

Signed-off-by: Miek Gieben <miek@miek.nl>

* Slightly smarter lookup check in upstream

Signed-off-by: Miek Gieben <miek@miek.nl>

* Compilez

Signed-off-by: Miek Gieben <miek@miek.nl>

3 files changed, 4 insertions, 78 deletions

diff --git a/plugin/kubernetes/kubernetes.go b/plugin/kubernetes/kubernetes.go
index 304972af3..aef1882f0 100644
--- a/plugin/kubernetes/kubernetes.go
+++ b/plugin/kubernetes/kubernetes.go
@@ -32,7 +32,7 @@ import (
 type Kubernetes struct {
 	Next             plugin.Handler
 	Zones            []string
-	Upstream         upstream.Upstream
+	Upstream         *upstream.Upstream
 	APIServerList    []string
 	APIProxy         *apiProxy
 	APICertAuth      string
diff --git a/plugin/kubernetes/setup.go b/plugin/kubernetes/setup.go
index 06939f8f2..5ea62246f 100644
--- a/plugin/kubernetes/setup.go
+++ b/plugin/kubernetes/setup.go
@@ -241,12 +241,8 @@ func ParseStanza(c *caddy.Controller) (*Kubernetes, error) {
 		case "fallthrough":
 			k8s.Fall.SetZonesFromArgs(c.RemainingArgs())
 		case "upstream":
-			args := c.RemainingArgs()
-			u, err := upstream.New(args)
-			if err != nil {
-				return nil, err
-			}
-			k8s.Upstream = u
+			c.RemainingArgs() // eat remaining args
+			k8s.Upstream = upstream.New()
 		case "ttl":
 			args := c.RemainingArgs()
 			if len(args) == 0 {
diff --git a/plugin/kubernetes/setup_test.go b/plugin/kubernetes/setup_test.go
index 2bde16437..2a420e1b7 100644
--- a/plugin/kubernetes/setup_test.go
+++ b/plugin/kubernetes/setup_test.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/coredns/coredns/plugin/pkg/fall"
 
-	"github.com/coredns/coredns/plugin/proxy"
 	"github.com/mholt/caddy"
 	meta "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -23,7 +22,6 @@ func TestKubernetesParse(t *testing.T) {
 		expectedLabelSelector string        // expected label selector value
 		expectedPodMode       string
 		expectedFallthrough   fall.F
-		expectedUpstreams     []string
 	}{
 		// positive
 		{
@@ -36,7 +34,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local test.local`,
@@ -48,7 +45,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -61,7 +57,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -75,7 +70,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -89,7 +83,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -103,7 +96,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -117,7 +109,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -131,7 +122,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -145,7 +135,6 @@ func TestKubernetesParse(t *testing.T) {
 			"environment=prod",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -159,7 +148,6 @@ func TestKubernetesParse(t *testing.T) {
 			"application=nginx,environment in (production,qa,staging)",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local test.local {
@@ -177,7 +165,6 @@ func TestKubernetesParse(t *testing.T) {
 			"application=nginx,environment in (production,qa,staging)",
 			podModeDisabled,
 			fall.Root,
-			nil,
 		},
 		// negative
 		{
@@ -192,7 +179,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -206,7 +192,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -220,7 +205,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -234,7 +218,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -248,7 +231,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -262,7 +244,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -276,7 +257,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		// pods disabled
 		{
@@ -291,7 +271,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		// pods insecure
 		{
@@ -306,7 +285,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeInsecure,
 			fall.Zero,
-			nil,
 		},
 		// pods verified
 		{
@@ -321,7 +299,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeVerified,
 			fall.Zero,
-			nil,
 		},
 		// pods invalid
 		{
@@ -336,7 +313,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeVerified,
 			fall.Zero,
-			nil,
 		},
 		// fallthrough with zones
 		{
@@ -351,12 +327,11 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.F{Zones: []string{"ip6.arpa.", "inaddr.arpa.", "foo.com."}},
-			nil,
 		},
 		// Valid upstream
 		{
 			`kubernetes coredns.local {
-	upstream 13.14.15.16:53
+	upstream
 }`,
 			false,
 			"",
@@ -366,22 +341,6 @@ func TestKubernetesParse(t *testing.T) {
 			"",
 			podModeDisabled,
 			fall.Zero,
-			[]string{"13.14.15.16:53"},
-		},
-		// Invalid upstream
-		{
-			`kubernetes coredns.local {
-	upstream 13.14.15.16orange
-}`,
-			true,
-			"not an IP address or file: \"13.14.15.16orange\"",
-			-1,
-			0,
-			defaultResyncPeriod,
-			"",
-			podModeDisabled,
-			fall.Zero,
-			nil,
 		},
 		// More than one Kubernetes not allowed
 		{
@@ -395,7 +354,6 @@ kubernetes cluster.local`,
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -409,7 +367,6 @@ kubernetes cluster.local`,
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -423,7 +380,6 @@ kubernetes cluster.local`,
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -437,7 +393,6 @@ kubernetes cluster.local`,
 			"",
 			podModeDisabled,
 			fall.Zero,
-			nil,
 		},
 		{
 			`kubernetes coredns.local {
@@ -519,31 +474,6 @@ kubernetes cluster.local`,
 		if !k8sController.Fall.Equal(test.expectedFallthrough) {
 			t.Errorf("Test %d: Expected kubernetes controller to be initialized with fallthrough '%v'. Instead found fallthrough '%v' for input '%s'", i, test.expectedFallthrough, k8sController.Fall, test.input)
 		}
-		// upstream
-		var foundUpstreams *[]proxy.Upstream
-		if k8sController.Upstream.Forward != nil {
-			foundUpstreams = k8sController.Upstream.Forward.Upstreams
-		}
-		if test.expectedUpstreams == nil {
-			if foundUpstreams != nil {
-				t.Errorf("Test %d: Expected kubernetes controller to not be initialized with upstreams for input '%s'", i, test.input)
-			}
-		} else {
-			if foundUpstreams == nil {
-				t.Errorf("Test %d: Expected kubernetes controller to be initialized with upstreams for input '%s'", i, test.input)
-			} else {
-				if len(*foundUpstreams) != len(test.expectedUpstreams) {
-					t.Errorf("Test %d: Expected kubernetes controller to be initialized with %d upstreams. Instead found %d upstreams for input '%s'", i, len(test.expectedUpstreams), len(*foundUpstreams), test.input)
-				}
-				for j, want := range test.expectedUpstreams {
-					got := (*foundUpstreams)[j].Select().Name
-					if got != want {
-						t.Errorf("Test %d: Expected kubernetes controller to be initialized with upstream '%s'. Instead found upstream '%s' for input '%s'", i, want, got, test.input)
-					}
-				}
-
-			}
-		}
 	}
 }