Merge pull request #540 from tangenti/tangenti/539

Add extra config validation in node-local-cache config sync

cmd/node-cache/app/cache_app_test.go

@@ -170,7 +170,7 @@ func TestUpdateCoreFile(t *testing.T) {
 	}
 	customConfig := &config.Config{StubDomains: map[string][]string{
 		"acme.local":   {"1.1.1.1"},
-		"google.local": {"google-public-dns-a.google.com"},
+		"google.local": {"8.8.8.8"},
 		"widget.local": {"2.2.2.2:10053", "3.3.3.3"},
 	},
 		UpstreamNameservers: []string{"2.2.2.2:10053", "3.3.3.3"},
@@ -246,7 +246,7 @@ func TestUpdateIPv6CoreFile(t *testing.T) {
 	}
 	customConfig := &config.Config{StubDomains: map[string][]string{
 		"acme.local":   {"2001:db8:1:1:1::1"},
-		"google.local": {"google-public-dns-a.google.com"},
+		"google.local": {"2001:4860:4860::8888"},
 		"widget.local": {"[2001:db8:2:2:2::2]:10053", "2001:db8:3:3:3::3"},
 	},
 		UpstreamNameservers: []string{"[2001:db8:2:2:2::2]:10053", "2001:db8:3:3:3::3"},

cmd/node-cache/app/configmap.go

@@ -82,13 +82,19 @@ func getStubDomainStr(stubDomainMap map[string][]string, info *stubDomainInfo) s
 }
 
 func (c *CacheApp) updateCorefile(dnsConfig *config.Config) {
+	if err := dnsConfig.ValidateNodeLocalCacheConfig(); err != nil {
+		clog.Errorf("Invalid config: %v", err)
+		setupErrCount.WithLabelValues("configmap").Inc()
+		return
+	}
 	// construct part of the Corefile
 	baseConfig, err := ioutil.ReadFile(c.params.BaseCoreFile)
 	if err != nil {
 		clog.Errorf("Failed to read node-cache coreFile %s - %v", c.params.BaseCoreFile, err)
 		setupErrCount.WithLabelValues("configmap").Inc()
 		return
 	}
+
 	stubDomainStr := getStubDomainStr(dnsConfig.StubDomains, &stubDomainInfo{Port: c.params.LocalPort, CacheTTL: defaultTTL,
 		LocalIP: strings.Replace(c.params.LocalIPStr, ",", " ", -1)})
 	upstreamServers := strings.Join(dnsConfig.UpstreamNameservers, " ")

pkg/dns/config/config.go

@@ -21,6 +21,7 @@ import (
 	"net"
 	"strconv"
 
+	"github.com/coredns/coredns/plugin/pkg/parse"
 	types "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	fed "k8s.io/dns/pkg/dns/federation"
@@ -130,3 +131,39 @@ func (config *Config) validateUpstreamNameserver() error {
 	}
 	return nil
 }
+
+// ValidateNodeLocalCacheConfig returns nil if the config can be compiled
+// to a valid Corefile.
+func (config *Config) ValidateNodeLocalCacheConfig() error {
+	for domain, nameservers := range config.StubDomains {
+		if err := validateForwardProxy(nameservers...); err != nil {
+			return fmt.Errorf("invalid nameservers %s for the stub domain %s: %v", nameservers, domain, err)
+		}
+	}
+	if err := validateForwardProxy(config.UpstreamNameservers...); err != nil {
+		return fmt.Errorf("invalid upstream nameservers %s: %v", config.UpstreamNameservers, err)
+	}
+	return nil
+}
+
+// validateForwardProxy returns nil if the nameservers are valid proxy addresses
+// for the CoreDNS plugin forward.
+// The function is ported from coredns/plugin/forward:parseStanza
+func validateForwardProxy(nameservers ...string) error {
+	if len(nameservers) == 0 {
+		return nil
+	}
+	hosts, err := parse.HostPortOrFile(nameservers...)
+	if err != nil {
+		return err
+	}
+	for _, host := range hosts {
+		trans, _ := parse.Transport(host)
+		switch trans {
+		case "dns", "tls":
+		default:
+			return fmt.Errorf("unsupported transport %s of nameserver %s", trans, host)
+		}
+	}
+	return nil
+}

pkg/dns/config/config_test.go

@@ -64,3 +64,45 @@ func TestValidate(t *testing.T) {
 		assert.NotNil(t, err, "should not be valid: %+v", testCase)
 	}
 }
+
+func TestValidateNodeLocalCacheConfig(t *testing.T) {
+	for _, tc := range []struct {
+		name      string
+		config    Config
+		wantError bool
+	}{
+		{
+			name:   "empty config",
+			config: Config{},
+		},
+		{
+			name: "valid config",
+			config: Config{
+				StubDomains:         map[string][]string{"": {"1.1.1.1"}},
+				UpstreamNameservers: []string{"1.1.1.1", "2.2.2.2"},
+			},
+		},
+		{
+			name: "invalid config: FQDN stub domain",
+			config: Config{
+				StubDomains:         map[string][]string{"": {"cluster.local"}},
+				UpstreamNameservers: []string{"1.1.1.1", "2.2.2.2"},
+			},
+			wantError: true,
+		},
+		{
+			name: "invalid config: FQDN upstream name server",
+			config: Config{
+				StubDomains:         map[string][]string{"": {"1.1.1.1"}},
+				UpstreamNameservers: []string{"1.1.1.1", "cluster.local"},
+			},
+			wantError: true,
+		},
+	} {
+		err := tc.config.ValidateNodeLocalCacheConfig()
+		gotError := err != nil
+		if gotError != tc.wantError {
+			t.Fatalf("ValidateNodeLocalCacheConfig(%v) got error = %v, want = %v", tc.config, gotError, tc.wantError)
+		}
+	}
+}