add validation for etc resolve parsing

pkg/kubelet/network/dns/BUILD

@@ -12,6 +12,7 @@ go_library(
         "//pkg/kubelet/container:go_default_library",
         "//pkg/kubelet/util/format:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
         "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
         "//staging/src/k8s.io/client-go/tools/record:go_default_library",
         "//vendor/github.com/golang/glog:go_default_library",

pkg/kubelet/network/dns/dns.go

@@ -26,6 +26,7 @@ import (
 	"strings"
 
 	"k8s.io/api/core/v1"
+	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/kubernetes/pkg/apis/core/validation"
@@ -209,6 +210,7 @@ func parseResolvConf(reader io.Reader) (nameservers []string, searches []string,
 	// Each option is recorded as an element in the array.
 	options = []string{}
 
+	var allErrors []error
 	lines := strings.Split(string(file), "\n")
 	for l := range lines {
 		trimmed := strings.TrimSpace(lines[l])
@@ -219,8 +221,12 @@ func parseResolvConf(reader io.Reader) (nameservers []string, searches []string,
 		if len(fields) == 0 {
 			continue
 		}
-		if fields[0] == "nameserver" && len(fields) >= 2 {
+		if fields[0] == "nameserver" {
+			if len(fields) >= 2 {
 				nameservers = append(nameservers, fields[1])
+			} else {
+				allErrors = append(allErrors, fmt.Errorf("nameserver list is empty "))
+			}
 		}
 		if fields[0] == "search" {
 			searches = fields[1:]
@@ -230,7 +236,7 @@ func parseResolvConf(reader io.Reader) (nameservers []string, searches []string,
 		}
 	}
 
-	return nameservers, searches, options, nil
+	return nameservers, searches, options, utilerrors.NewAggregate(allErrors)
 }
 
 func (c *Configurer) getHostDNSConfig(pod *v1.Pod) (*runtimeapi.DNSConfig, error) {

pkg/kubelet/network/dns/dns_test.go

@@ -53,40 +53,48 @@ func TestParseResolvConf(t *testing.T) {
 		nameservers []string
 		searches    []string
 		options     []string
+		isErr       bool
 	}{
-		{"", []string{}, []string{}, []string{}},
-		{" ", []string{}, []string{}, []string{}},
-		{"\n", []string{}, []string{}, []string{}},
-		{"\t\n\t", []string{}, []string{}, []string{}},
-		{"#comment\n", []string{}, []string{}, []string{}},
-		{" #comment\n", []string{}, []string{}, []string{}},
-		{"#comment\n#comment", []string{}, []string{}, []string{}},
-		{"#comment\nnameserver", []string{}, []string{}, []string{}},
-		{"#comment\nnameserver\nsearch", []string{}, []string{}, []string{}},
-		{"nameserver 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}},
-		{" nameserver 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}},
-		{"\tnameserver 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}},
-		{"nameserver\t1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}},
-		{"nameserver \t 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}},
-		{"nameserver 1.2.3.4\nnameserver 5.6.7.8", []string{"1.2.3.4", "5.6.7.8"}, []string{}, []string{}},
-		{"nameserver 1.2.3.4 #comment", []string{"1.2.3.4"}, []string{}, []string{}},
-		{"search foo", []string{}, []string{"foo"}, []string{}},
-		{"search foo bar", []string{}, []string{"foo", "bar"}, []string{}},
-		{"search foo bar bat\n", []string{}, []string{"foo", "bar", "bat"}, []string{}},
-		{"search foo\nsearch bar", []string{}, []string{"bar"}, []string{}},
-		{"nameserver 1.2.3.4\nsearch foo bar", []string{"1.2.3.4"}, []string{"foo", "bar"}, []string{}},
-		{"nameserver 1.2.3.4\nsearch foo\nnameserver 5.6.7.8\nsearch bar", []string{"1.2.3.4", "5.6.7.8"}, []string{"bar"}, []string{}},
-		{"#comment\nnameserver 1.2.3.4\n#comment\nsearch foo\ncomment", []string{"1.2.3.4"}, []string{"foo"}, []string{}},
-		{"options ndots:5 attempts:2", []string{}, []string{}, []string{"ndots:5", "attempts:2"}},
-		{"options ndots:1\noptions ndots:5 attempts:3", []string{}, []string{}, []string{"ndots:5", "attempts:3"}},
-		{"nameserver 1.2.3.4\nsearch foo\nnameserver 5.6.7.8\nsearch bar\noptions ndots:5 attempts:4", []string{"1.2.3.4", "5.6.7.8"}, []string{"bar"}, []string{"ndots:5", "attempts:4"}},
+		{"", []string{}, []string{}, []string{}, false},
+		{" ", []string{}, []string{}, []string{}, false},
+		{"\n", []string{}, []string{}, []string{}, false},
+		{"\t\n\t", []string{}, []string{}, []string{}, false},
+		{"#comment\n", []string{}, []string{}, []string{}, false},
+		{" #comment\n", []string{}, []string{}, []string{}, false},
+		{"#comment\n#comment", []string{}, []string{}, []string{}, false},
+		{"#comment\nnameserver", []string{}, []string{}, []string{}, true},                           // nameserver empty
+		{"#comment\nnameserver\nsearch", []string{}, []string{}, []string{}, true},                   // nameserver and search empty
+		{"#comment\nnameserver 1.2.3.4\nsearch", []string{"1.2.3.4"}, []string{}, []string{}, false}, // nameserver specified and search empty
+		{"nameserver 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}, false},
+		{" nameserver 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}, false},
+		{"\tnameserver 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}, false},
+		{"nameserver\t1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}, false},
+		{"nameserver \t 1.2.3.4", []string{"1.2.3.4"}, []string{}, []string{}, false},
+		{"nameserver 1.2.3.4\nnameserver 5.6.7.8", []string{"1.2.3.4", "5.6.7.8"}, []string{}, []string{}, false},
+		{"nameserver 1.2.3.4 #comment", []string{"1.2.3.4"}, []string{}, []string{}, false},
+		{"search ", []string{}, []string{}, []string{}, false}, // search empty
+		{"search foo", []string{}, []string{"foo"}, []string{}, false},
+		{"search foo bar", []string{}, []string{"foo", "bar"}, []string{}, false},
+		{"search foo bar bat\n", []string{}, []string{"foo", "bar", "bat"}, []string{}, false},
+		{"search foo\nsearch bar", []string{}, []string{"bar"}, []string{}, false},
+		{"nameserver 1.2.3.4\nsearch foo bar", []string{"1.2.3.4"}, []string{"foo", "bar"}, []string{}, false},
+		{"nameserver 1.2.3.4\nsearch foo\nnameserver 5.6.7.8\nsearch bar", []string{"1.2.3.4", "5.6.7.8"}, []string{"bar"}, []string{}, false},
+		{"#comment\nnameserver 1.2.3.4\n#comment\nsearch foo\ncomment", []string{"1.2.3.4"}, []string{"foo"}, []string{}, false},
+		{"options ", []string{}, []string{}, []string{}, false},
+		{"options ndots:5 attempts:2", []string{}, []string{}, []string{"ndots:5", "attempts:2"}, false},
+		{"options ndots:1\noptions ndots:5 attempts:3", []string{}, []string{}, []string{"ndots:5", "attempts:3"}, false},
+		{"nameserver 1.2.3.4\nsearch foo\nnameserver 5.6.7.8\nsearch bar\noptions ndots:5 attempts:4", []string{"1.2.3.4", "5.6.7.8"}, []string{"bar"}, []string{"ndots:5", "attempts:4"}, false},
 	}
 	for i, tc := range testCases {
 		ns, srch, opts, err := parseResolvConf(strings.NewReader(tc.data))
+		if !tc.isErr {
 			require.NoError(t, err)
 			assert.EqualValues(t, tc.nameservers, ns, "test case [%d]: name servers", i)
 			assert.EqualValues(t, tc.searches, srch, "test case [%d] searches", i)
 			assert.EqualValues(t, tc.options, opts, "test case [%d] options", i)
+		} else {
+			require.Error(t, err, "tc.searches %v", tc.searches)
+		}
 	}
 }