Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a cloud-provider hook to scrub DNS for pods #16219

Merged
merged 1 commit into from
Oct 26, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkg/cloudprovider/cloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions pkg/cloudprovider/providers/aws/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/cloudprovider/providers/fake/fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
15 changes: 15 additions & 0 deletions pkg/cloudprovider/providers/gce/gce.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"net"
"net/http"
"path"
"regexp"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -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
Expand Down
41 changes: 40 additions & 1 deletion pkg/cloudprovider/providers/gce/gce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ limitations under the License.

package gce

import "testing"
import (
"reflect"
"testing"
)

func TestGetRegion(t *testing.T) {
gce := &GCECloud{
Expand Down Expand Up @@ -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)
}
}
}
5 changes: 5 additions & 0 deletions pkg/cloudprovider/providers/mesos/mesos.go
Original file line number Diff line number Diff line change
Expand Up @@ -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!)
Expand Down
5 changes: 5 additions & 0 deletions pkg/cloudprovider/providers/openstack/openstack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/cloudprovider/providers/ovirt/ovirt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions pkg/cloudprovider/providers/rackspace/rackspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/cloudprovider/providers/vagrant/vagrant.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 20 additions & 2 deletions pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
28 changes: 27 additions & 1 deletion pkg/kubelet/kubelet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -908,7 +917,21 @@ 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
}
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)
}

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
Expand All @@ -919,6 +942,9 @@ func TestParseResolvConf(t *testing.T) {
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)
}
}
}

Expand Down