From 42c7fec490c13722a2883077e69468eec1c996f4 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Fri, 23 Oct 2015 17:01:49 -0700 Subject: [PATCH] Add a cloud-provider hook to scrub DNS for pods GCE needs this hook and it seems general enough to include. --- pkg/cloudprovider/cloud.go | 2 + pkg/cloudprovider/providers/aws/aws.go | 5 +++ pkg/cloudprovider/providers/fake/fake.go | 5 +++ pkg/cloudprovider/providers/gce/gce.go | 15 +++++++ pkg/cloudprovider/providers/gce/gce_test.go | 41 ++++++++++++++++++- pkg/cloudprovider/providers/mesos/mesos.go | 5 +++ .../providers/openstack/openstack.go | 5 +++ pkg/cloudprovider/providers/ovirt/ovirt.go | 5 +++ .../providers/rackspace/rackspace.go | 5 +++ .../providers/vagrant/vagrant.go | 5 +++ pkg/kubelet/kubelet.go | 22 +++++++++- pkg/kubelet/kubelet_test.go | 28 ++++++++++++- 12 files changed, 139 insertions(+), 4 deletions(-) diff --git a/pkg/cloudprovider/cloud.go b/pkg/cloudprovider/cloud.go index fff0bddf9f1..e64ed5dc0c2 100644 --- a/pkg/cloudprovider/cloud.go +++ b/pkg/cloudprovider/cloud.go @@ -39,6 +39,8 @@ type Interface interface { Routes() (Routes, bool) // ProviderName returns the cloud provider ID. ProviderName() string + // ScrubDNS provides an opportunity for cloud-provider-specific code to process DNS settings for pods. + ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) } // Clusters is an abstract, pluggable interface for clusters of containers. diff --git a/pkg/cloudprovider/providers/aws/aws.go b/pkg/cloudprovider/providers/aws/aws.go index 60f48d8a671..fbd846167a6 100644 --- a/pkg/cloudprovider/providers/aws/aws.go +++ b/pkg/cloudprovider/providers/aws/aws.go @@ -597,6 +597,11 @@ func (aws *AWSCloud) ProviderName() string { return ProviderName } +// ScrubDNS filters DNS settings for pods. +func (aws *AWSCloud) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + return nameservers, searches +} + // TCPLoadBalancer returns an implementation of TCPLoadBalancer for Amazon Web Services. func (s *AWSCloud) TCPLoadBalancer() (cloudprovider.TCPLoadBalancer, bool) { return s, true diff --git a/pkg/cloudprovider/providers/fake/fake.go b/pkg/cloudprovider/providers/fake/fake.go index beff4747d08..a11030d2adf 100644 --- a/pkg/cloudprovider/providers/fake/fake.go +++ b/pkg/cloudprovider/providers/fake/fake.go @@ -94,6 +94,11 @@ func (f *FakeCloud) ProviderName() string { return ProviderName } +// ScrubDNS filters DNS settings for pods. +func (f *FakeCloud) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + return nameservers, searches +} + // TCPLoadBalancer returns a fake implementation of TCPLoadBalancer. // Actually it just returns f itself. func (f *FakeCloud) TCPLoadBalancer() (cloudprovider.TCPLoadBalancer, bool) { diff --git a/pkg/cloudprovider/providers/gce/gce.go b/pkg/cloudprovider/providers/gce/gce.go index 9c9678a9b1c..bd33d7a27d1 100644 --- a/pkg/cloudprovider/providers/gce/gce.go +++ b/pkg/cloudprovider/providers/gce/gce.go @@ -22,6 +22,7 @@ import ( "net" "net/http" "path" + "regexp" "sort" "strconv" "strings" @@ -201,6 +202,20 @@ func (gce *GCECloud) ProviderName() string { return ProviderName } +// Known-useless DNS search path. +var uselessDNSSearchRE = regexp.MustCompile(`^[0-9]+.google.internal.$`) + +// ScrubDNS filters DNS settings for pods. +func (gce *GCECloud) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + // GCE has too many search paths by default. Filter the ones we know are useless. + for _, s := range searches { + if !uselessDNSSearchRE.MatchString(s) { + srchOut = append(srchOut, s) + } + } + return nameservers, srchOut +} + // TCPLoadBalancer returns an implementation of TCPLoadBalancer for Google Compute Engine. func (gce *GCECloud) TCPLoadBalancer() (cloudprovider.TCPLoadBalancer, bool) { return gce, true diff --git a/pkg/cloudprovider/providers/gce/gce_test.go b/pkg/cloudprovider/providers/gce/gce_test.go index 89b4bda820d..a614b3065f6 100644 --- a/pkg/cloudprovider/providers/gce/gce_test.go +++ b/pkg/cloudprovider/providers/gce/gce_test.go @@ -16,7 +16,10 @@ limitations under the License. package gce -import "testing" +import ( + "reflect" + "testing" +) func TestGetRegion(t *testing.T) { gce := &GCECloud{ @@ -96,3 +99,39 @@ func TestComparingHostURLs(t *testing.T) { } } } + +func TestScrubDNS(t *testing.T) { + tcs := []struct { + nameserversIn []string + searchesIn []string + nameserversOut []string + searchesOut []string + }{ + { + nameserversIn: []string{"1.2.3.4", "5.6.7.8"}, + nameserversOut: []string{"1.2.3.4", "5.6.7.8"}, + }, + { + searchesIn: []string{"c.prj.internal.", "12345678910.google.internal.", "google.internal."}, + searchesOut: []string{"c.prj.internal.", "google.internal."}, + }, + { + searchesIn: []string{"c.prj.internal.", "12345678910.google.internal.", "zone.c.prj.internal.", "google.internal."}, + searchesOut: []string{"c.prj.internal.", "zone.c.prj.internal.", "google.internal."}, + }, + { + searchesIn: []string{"c.prj.internal.", "12345678910.google.internal.", "zone.c.prj.internal.", "google.internal.", "unexpected"}, + searchesOut: []string{"c.prj.internal.", "zone.c.prj.internal.", "google.internal.", "unexpected"}, + }, + } + gce := &GCECloud{} + for i := range tcs { + n, s := gce.ScrubDNS(tcs[i].nameserversIn, tcs[i].searchesIn) + if !reflect.DeepEqual(n, tcs[i].nameserversOut) { + t.Errorf("Expected %v, got %v", tcs[i].nameserversOut, n) + } + if !reflect.DeepEqual(s, tcs[i].searchesOut) { + t.Errorf("Expected %v, got %v", tcs[i].searchesOut, s) + } + } +} diff --git a/pkg/cloudprovider/providers/mesos/mesos.go b/pkg/cloudprovider/providers/mesos/mesos.go index 053028e230c..fb1404e310c 100644 --- a/pkg/cloudprovider/providers/mesos/mesos.go +++ b/pkg/cloudprovider/providers/mesos/mesos.go @@ -124,6 +124,11 @@ func (c *MesosCloud) ProviderName() string { return ProviderName } +// ScrubDNS filters DNS settings for pods. +func (c *MesosCloud) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + return nameservers, searches +} + // ListClusters lists the names of the available Mesos clusters. func (c *MesosCloud) ListClusters() ([]string, error) { // Always returns a single cluster (this one!) diff --git a/pkg/cloudprovider/providers/openstack/openstack.go b/pkg/cloudprovider/providers/openstack/openstack.go index 77f6ae6aeca..cb739251f4e 100644 --- a/pkg/cloudprovider/providers/openstack/openstack.go +++ b/pkg/cloudprovider/providers/openstack/openstack.go @@ -399,6 +399,11 @@ func (os *OpenStack) ProviderName() string { return ProviderName } +// ScrubDNS filters DNS settings for pods. +func (os *OpenStack) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + return nameservers, searches +} + type LoadBalancer struct { network *gophercloud.ServiceClient compute *gophercloud.ServiceClient diff --git a/pkg/cloudprovider/providers/ovirt/ovirt.go b/pkg/cloudprovider/providers/ovirt/ovirt.go index cf97e7f9e92..a448a639743 100644 --- a/pkg/cloudprovider/providers/ovirt/ovirt.go +++ b/pkg/cloudprovider/providers/ovirt/ovirt.go @@ -123,6 +123,11 @@ func (v *OVirtCloud) ProviderName() string { return ProviderName } +// ScrubDNS filters DNS settings for pods. +func (v *OVirtCloud) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + return nameservers, searches +} + // TCPLoadBalancer returns an implementation of TCPLoadBalancer for oVirt cloud func (v *OVirtCloud) TCPLoadBalancer() (cloudprovider.TCPLoadBalancer, bool) { return nil, false diff --git a/pkg/cloudprovider/providers/rackspace/rackspace.go b/pkg/cloudprovider/providers/rackspace/rackspace.go index abfe2c556f3..9de46ed9e1b 100644 --- a/pkg/cloudprovider/providers/rackspace/rackspace.go +++ b/pkg/cloudprovider/providers/rackspace/rackspace.go @@ -362,6 +362,11 @@ func (os *Rackspace) ProviderName() string { return ProviderName } +// ScrubDNS filters DNS settings for pods. +func (os *Rackspace) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + return nameservers, searches +} + func (os *Rackspace) TCPLoadBalancer() (cloudprovider.TCPLoadBalancer, bool) { return nil, false } diff --git a/pkg/cloudprovider/providers/vagrant/vagrant.go b/pkg/cloudprovider/providers/vagrant/vagrant.go index 3deae992c4a..75b95bb8fa4 100644 --- a/pkg/cloudprovider/providers/vagrant/vagrant.go +++ b/pkg/cloudprovider/providers/vagrant/vagrant.go @@ -91,6 +91,11 @@ func (v *VagrantCloud) ProviderName() string { return ProviderName } +// ScrubDNS filters DNS settings for pods. +func (v *VagrantCloud) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + return nameservers, searches +} + // TCPLoadBalancer returns an implementation of TCPLoadBalancer for Vagrant cloud. func (v *VagrantCloud) TCPLoadBalancer() (cloudprovider.TCPLoadBalancer, bool) { return nil, false diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 675495c38ce..421701c4f4e 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1233,7 +1233,7 @@ func (kl *Kubelet) getClusterDNS(pod *api.Pod) ([]string, []string, error) { } defer f.Close() - hostDNS, hostSearch, err = parseResolvConf(f) + hostDNS, hostSearch, err = kl.parseResolvConf(f) if err != nil { return nil, nil, err } @@ -1270,7 +1270,20 @@ func (kl *Kubelet) getClusterDNS(pod *api.Pod) ([]string, []string, error) { } // Returns the list of DNS servers and DNS search domains. -func parseResolvConf(reader io.Reader) (nameservers []string, searches []string, err error) { +func (kl *Kubelet) parseResolvConf(reader io.Reader) (nameservers []string, searches []string, err error) { + var scrubber dnsScrubber + if kl.cloud != nil { + scrubber = kl.cloud + } + return parseResolvConf(reader, scrubber) +} + +// A helper for testing. +type dnsScrubber interface { + ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) +} + +func parseResolvConf(reader io.Reader, dnsScrubber dnsScrubber) (nameservers []string, searches []string, err error) { file, err := ioutil.ReadAll(reader) if err != nil { return nil, nil, err @@ -1299,6 +1312,11 @@ func parseResolvConf(reader io.Reader) (nameservers []string, searches []string, searches = fields[1:] } } + + // Give the cloud-provider a chance to post-process DNS settings. + if dnsScrubber != nil { + nameservers, searches = dnsScrubber.ScrubDNS(nameservers, searches) + } return nameservers, searches, nil } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index c4670fd1b64..7e2a45a5fe2 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -878,6 +878,15 @@ func TestRunInContainer(t *testing.T) { } } +type countingDNSScrubber struct { + counter *int +} + +func (cds countingDNSScrubber) ScrubDNS(nameservers, searches []string) (nsOut, srchOut []string) { + (*cds.counter)++ + return nameservers, searches +} + func TestParseResolvConf(t *testing.T) { testCases := []struct { data string @@ -908,7 +917,7 @@ func TestParseResolvConf(t *testing.T) { {"#comment\nnameserver 1.2.3.4\n#comment\nsearch foo\ncomment", []string{"1.2.3.4"}, []string{"foo"}}, } for i, tc := range testCases { - ns, srch, err := parseResolvConf(strings.NewReader(tc.data)) + ns, srch, err := parseResolvConf(strings.NewReader(tc.data), nil) if err != nil { t.Errorf("expected success, got %v", err) continue @@ -919,6 +928,23 @@ func TestParseResolvConf(t *testing.T) { if !reflect.DeepEqual(srch, tc.searches) { t.Errorf("[%d] expected searches %#v, got %#v", i, tc.searches, srch) } + + counter := 0 + cds := countingDNSScrubber{&counter} + ns, srch, err = parseResolvConf(strings.NewReader(tc.data), cds) + if err != nil { + t.Errorf("expected success, got %v", err) + continue + } + if !reflect.DeepEqual(ns, tc.nameservers) { + t.Errorf("[%d] expected nameservers %#v, got %#v", i, tc.nameservers, ns) + } + if !reflect.DeepEqual(srch, tc.searches) { + t.Errorf("[%d] expected searches %#v, got %#v", i, tc.searches, srch) + } + if counter != 1 { + t.Errorf("[%d] expected dnsScrubber to have been called: got %d", i, counter) + } } }