cleanups: go vet/golint (#331)

Go vet and golint the new code once again.

Drop Name from NameTemplate - it's cleaner: nametemplate.Template.

core/dnsserver/server.go

@@ -1,5 +1,4 @@
-// Package DNS server implements all the interfaces from Caddy, so that
-// CoreDNS can be a servertype plugin.
+// Package dnsserver implements all the interfaces from Caddy, so that CoreDNS can be a servertype plugin.
 package dnsserver
 
 import (

middleware/file/tree/elem.go

@@ -52,7 +52,7 @@ func (e *Elem) Name() string {
 	return ""
 }
 
-// Wildcard returns true if this name starts with a wildcard label (*.)
+// IsWildcard returns true if this name starts with a wildcard label (*.)
 func (e *Elem) IsWildcard() bool {
 	n := e.Name()
 	if len(n) < 2 {

middleware/kubernetes/controller.go

@@ -51,7 +51,7 @@ type dnsController struct {
 	stopCh   chan struct{}
 }
 
-// newDNSController creates a controller for coredns
+// newDNSController creates a controller for CoreDNS.
 func newdnsController(kubeClient *client.Client, resyncPeriod time.Duration, lselector *labels.Selector) *dnsController {
 	dns := dnsController{
 		client:   kubeClient,
@@ -168,7 +168,7 @@ func (dns *dnsController) Run() {
 	<-dns.stopCh
 }
 
-func (dns *dnsController) GetNamespaceList() *api.NamespaceList {
+func (dns *dnsController) NamespaceList() *api.NamespaceList {
 	nsList, err := dns.nsLister.List()
 	if err != nil {
 		return &api.NamespaceList{}
@@ -177,7 +177,7 @@ func (dns *dnsController) GetNamespaceList() *api.NamespaceList {
 	return &nsList
 }
 
-func (dns *dnsController) GetServiceList() []*api.Service {
+func (dns *dnsController) ServiceList() []*api.Service {
 	svcs, err := dns.svcLister.List(labels.Everything())
 	if err != nil {
 		return []*api.Service{}
@@ -186,10 +186,11 @@ func (dns *dnsController) GetServiceList() []*api.Service {
 	return svcs
 }
 
-// GetServicesByNamespace returns a map of
+// ServicesByNamespace returns a map of:
+//
 // namespacename :: [ kubernetesService ]
-func (dns *dnsController) GetServicesByNamespace() map[string][]api.Service {
-	k8sServiceList := dns.GetServiceList()
+func (dns *dnsController) ServicesByNamespace() map[string][]api.Service {
+	k8sServiceList := dns.ServiceList()
 	items := make(map[string][]api.Service, len(k8sServiceList))
 	for _, i := range k8sServiceList {
 		namespace := i.Namespace
@@ -199,9 +200,8 @@ func (dns *dnsController) GetServicesByNamespace() map[string][]api.Service {
 	return items
 }
 
-// GetServiceInNamespace returns the Service that matches
-// servicename in the namespace
-func (dns *dnsController) GetServiceInNamespace(namespace string, servicename string) *api.Service {
+// ServiceInNamespace returns the Service that matches servicename in the namespace
+func (dns *dnsController) ServiceInNamespace(namespace, servicename string) *api.Service {
 	svcObj, err := dns.svcLister.Services(namespace).Get(servicename)
 	if err != nil {
 		// TODO(...): should return err here

middleware/kubernetes/kubernetes.go

@@ -35,7 +35,7 @@ type Kubernetes struct {
 	APIClientKey  string
 	APIConn       *dnsController
 	ResyncPeriod  time.Duration
-	NameTemplate  *nametemplate.NameTemplate
+	NameTemplate  *nametemplate.Template
 	Namespaces    []string
 	LabelSelector *unversionedapi.LabelSelector
 	Selector      *labels.Selector
@@ -125,7 +125,7 @@ func (k *Kubernetes) getZoneForName(name string) (string, []string) {
 // for instance.
 func (k *Kubernetes) Records(name string, exact bool) ([]msg.Service, error) {
 	// TODO: refector this.
-	// Right now GetNamespaceFromSegmentArray do not supports PRE queries
+	// Right now NamespaceFromSegmentArray do not supports PRE queries
 	ip := dnsutil.ExtractAddressFromReverse(name)
 	if ip != "" {
 		records := k.getServiceRecordForIP(ip, name)
@@ -142,9 +142,9 @@ func (k *Kubernetes) Records(name string, exact bool) ([]msg.Service, error) {
 	// TODO: Implementation above globbed together segments for the serviceName if
 	//       multiple segments remained. Determine how to do similar globbing using
 	//		 the template-based implementation.
-	namespace = k.NameTemplate.GetNamespaceFromSegmentArray(serviceSegments)
-	serviceName = k.NameTemplate.GetServiceFromSegmentArray(serviceSegments)
-	typeName = k.NameTemplate.GetTypeFromSegmentArray(serviceSegments)
+	namespace = k.NameTemplate.NamespaceFromSegmentArray(serviceSegments)
+	serviceName = k.NameTemplate.ServiceFromSegmentArray(serviceSegments)
+	typeName = k.NameTemplate.TypeFromSegmentArray(serviceSegments)
 
 	if namespace == "" {
 		err := errors.New("Parsing query string did not produce a namespace value. Assuming wildcard namespace.")
@@ -205,7 +205,7 @@ func (k *Kubernetes) getRecordsForServiceItems(serviceItems []*api.Service, valu
 
 // Get performs the call to the Kubernetes http API.
 func (k *Kubernetes) Get(namespace string, nsWildcard bool, servicename string, serviceWildcard bool) ([]*api.Service, error) {
-	serviceList := k.APIConn.GetServiceList()
+	serviceList := k.APIConn.ServiceList()
 
 	var resultItems []*api.Service
 

middleware/kubernetes/nametemplate/nametemplate.go

@@ -61,14 +61,16 @@ var requiredSymbols = []string{
 //		 symbol consumes the other segments. Most likely this would be the servicename.
 //		 Also consider how to handle static strings in the format template.
 
-type NameTemplate struct {
+// TODO(infoblox): docs
+type Template struct {
 	formatString string
 	splitFormat  []string
 	// Element is a map of element name :: index in the segmented record name for the named element
 	Element map[string]int
 }
 
-func (t *NameTemplate) SetTemplate(s string) error {
+// TODO: docs
+func (t *Template) SetTemplate(s string) error {
 	var err error
 
 	t.Element = map[string]int{}
@@ -105,7 +107,7 @@ func (t *NameTemplate) SetTemplate(s string) error {
 //		 to treat the query string segments as a reverse stack and
 //       step down the stack to find the right element.
 
-func (t *NameTemplate) GetZoneFromSegmentArray(segments []string) string {
+func (t *Template) ZoneFromSegmentArray(segments []string) string {
 	index, ok := t.Element["zone"]
 	if !ok {
 		return ""
@@ -113,16 +115,16 @@ func (t *NameTemplate) GetZoneFromSegmentArray(segments []string) string {
 	return strings.Join(segments[index:len(segments)], ".")
 }
 
-func (t *NameTemplate) GetNamespaceFromSegmentArray(segments []string) string {
-	return t.GetSymbolFromSegmentArray("namespace", segments)
+func (t *Template) NamespaceFromSegmentArray(segments []string) string {
+	return t.SymbolFromSegmentArray("namespace", segments)
 }
 
-func (t *NameTemplate) GetServiceFromSegmentArray(segments []string) string {
-	return t.GetSymbolFromSegmentArray("service", segments)
+func (t *Template) ServiceFromSegmentArray(segments []string) string {
+	return t.SymbolFromSegmentArray("service", segments)
 }
 
-func (t *NameTemplate) GetTypeFromSegmentArray(segments []string) string {
-	typeSegment := t.GetSymbolFromSegmentArray("type", segments)
+func (t *Template) TypeFromSegmentArray(segments []string) string {
+	typeSegment := t.SymbolFromSegmentArray("type", segments)
 
 	// Limit type to known types symbols
 	if dns_strings.StringInSlice(typeSegment, types) {
@@ -132,7 +134,7 @@ func (t *NameTemplate) GetTypeFromSegmentArray(segments []string) string {
 	return typeSegment
 }
 
-func (t *NameTemplate) GetSymbolFromSegmentArray(symbol string, segments []string) string {
+func (t *Template) SymbolFromSegmentArray(symbol string, segments []string) string {
 	index, ok := t.Element[symbol]
 	if !ok {
 		return ""
@@ -140,9 +142,9 @@ func (t *NameTemplate) GetSymbolFromSegmentArray(symbol string, segments []strin
 	return segments[index]
 }
 
-// GetRecordNameFromNameValues returns the string produced by applying the
+// RecordNameFromNameValues returns the string produced by applying the
 // values to the NameTemplate format string.
-func (t *NameTemplate) GetRecordNameFromNameValues(values NameValues) string {
+func (t *Template) RecordNameFromNameValues(values NameValues) string {
 	recordName := make([]string, len(t.splitFormat))
 	copy(recordName[:], t.splitFormat)
 
@@ -164,8 +166,8 @@ func (t *NameTemplate) GetRecordNameFromNameValues(values NameValues) string {
 	return strings.Join(recordName, ".")
 }
 
-func (t *NameTemplate) IsValid() bool {
-	// This is *only* used in a test, for the test this should be a private method.
+// IsValid returns true if the template has all the required symbols, false otherwise.
+func (t *Template) IsValid() bool {
 	result := true
 
 	// Ensure that all requiredSymbols are found in NameTemplate
@@ -179,6 +181,7 @@ func (t *NameTemplate) IsValid() bool {
 	return result
 }
 
+// TODO(infoblox): what's this?
 type NameValues struct {
 	ServiceName string
 	Namespace   string

middleware/kubernetes/nametemplate/nametemplate_test.go

@@ -22,7 +22,7 @@ var exampleTemplates = map[string][]int{
 func TestSetTemplate(t *testing.T) {
 	for s, expectedValue := range exampleTemplates {
 
-		n := new(NameTemplate)
+		n := new(Template)
 		n.SetTemplate(s)
 
 		// check the indexes resulting from calling SetTemplate() against expectedValues
@@ -34,9 +34,9 @@ func TestSetTemplate(t *testing.T) {
 	}
 }
 
-func TestGetServiceFromSegmentArray(t *testing.T) {
+func TestServiceFromSegmentArray(t *testing.T) {
 	var (
-		n               *NameTemplate
+		n               *Template
 		formatString    string
 		queryString     string
 		splitQuery      []string
@@ -45,37 +45,37 @@ func TestGetServiceFromSegmentArray(t *testing.T) {
 	)
 
 	// Case where template contains {service}
-	n = new(NameTemplate)
+	n = new(Template)
 	formatString = "{service}.{namespace}.{zone}"
 	n.SetTemplate(formatString)
 
 	queryString = "myservice.mynamespace.coredns"
 	splitQuery = strings.Split(queryString, ".")
 	expectedService = "myservice"
-	actualService = n.GetServiceFromSegmentArray(splitQuery)
+	actualService = n.ServiceFromSegmentArray(splitQuery)
 
 	if actualService != expectedService {
 		t.Errorf("Expected service name '%v', instead got service name '%v' for query string '%v' and format '%v'", expectedService, actualService, queryString, formatString)
 	}
 
 	// Case where template does not contain {service}
-	n = new(NameTemplate)
+	n = new(Template)
 	formatString = "{namespace}.{zone}"
 	n.SetTemplate(formatString)
 
 	queryString = "mynamespace.coredns"
 	splitQuery = strings.Split(queryString, ".")
 	expectedService = ""
-	actualService = n.GetServiceFromSegmentArray(splitQuery)
+	actualService = n.ServiceFromSegmentArray(splitQuery)
 
 	if actualService != expectedService {
 		t.Errorf("Expected service name '%v', instead got service name '%v' for query string '%v' and format '%v'", expectedService, actualService, queryString, formatString)
 	}
 }
 
-func TestGetZoneFromSegmentArray(t *testing.T) {
+func TestZoneFromSegmentArray(t *testing.T) {
 	var (
-		n            *NameTemplate
+		n            *Template
 		formatString string
 		queryString  string
 		splitQuery   []string
@@ -84,42 +84,42 @@ func TestGetZoneFromSegmentArray(t *testing.T) {
 	)
 
 	// Case where template contains {zone}
-	n = new(NameTemplate)
+	n = new(Template)
 	formatString = "{service}.{namespace}.{zone}"
 	n.SetTemplate(formatString)
 
 	queryString = "myservice.mynamespace.coredns"
 	splitQuery = strings.Split(queryString, ".")
 	expectedZone = "coredns"
-	actualZone = n.GetZoneFromSegmentArray(splitQuery)
+	actualZone = n.ZoneFromSegmentArray(splitQuery)
 
 	if actualZone != expectedZone {
 		t.Errorf("Expected zone name '%v', instead got zone name '%v' for query string '%v' and format '%v'", expectedZone, actualZone, queryString, formatString)
 	}
 
 	// Case where template does not contain {zone}
-	n = new(NameTemplate)
+	n = new(Template)
 	formatString = "{service}.{namespace}"
 	n.SetTemplate(formatString)
 
 	queryString = "mynamespace.coredns"
 	splitQuery = strings.Split(queryString, ".")
 	expectedZone = ""
-	actualZone = n.GetZoneFromSegmentArray(splitQuery)
+	actualZone = n.ZoneFromSegmentArray(splitQuery)
 
 	if actualZone != expectedZone {
 		t.Errorf("Expected zone name '%v', instead got zone name '%v' for query string '%v' and format '%v'", expectedZone, actualZone, queryString, formatString)
 	}
 
 	// Case where zone is multiple segments
-	n = new(NameTemplate)
+	n = new(Template)
 	formatString = "{service}.{namespace}.{zone}"
 	n.SetTemplate(formatString)
 
 	queryString = "myservice.mynamespace.coredns.cluster.local"
 	splitQuery = strings.Split(queryString, ".")
 	expectedZone = "coredns.cluster.local"
-	actualZone = n.GetZoneFromSegmentArray(splitQuery)
+	actualZone = n.ZoneFromSegmentArray(splitQuery)
 
 	if actualZone != expectedZone {
 		t.Errorf("Expected zone name '%v', instead got zone name '%v' for query string '%v' and format '%v'", expectedZone, actualZone, queryString, formatString)

middleware/kubernetes/setup.go

@@ -52,7 +52,7 @@ func setup(c *caddy.Controller) error {
 
 func kubernetesParse(c *caddy.Controller) (*Kubernetes, error) {
 	k8s := &Kubernetes{ResyncPeriod: defaultResyncPeriod}
-	k8s.NameTemplate = new(nametemplate.NameTemplate)
+	k8s.NameTemplate = new(nametemplate.Template)
 	k8s.NameTemplate.SetTemplate(defaultNameTemplate)
 
 	for c.Next() {

middleware/pkg/dnsutil/cname_test.go

@@ -36,14 +36,14 @@ func TestDuplicateCNAME(t *testing.T) {
 	for i, test := range tests {
 		cnameRR, err := dns.NewRR(test.cname)
 		if err != nil {
-			t.Fatal("Test %d, cname ('%s') error (%s)!", i, test.cname, err)
+			t.Fatalf("Test %d, cname ('%s') error (%s)!", i, test.cname, err)
 		}
 		cname := cnameRR.(*dns.CNAME)
 		records := []dns.RR{}
 		for j, r := range test.records {
 			rr, err := dns.NewRR(r)
 			if err != nil {
-				t.Fatal("Test %d, record %d ('%s') error (%s)!", i, j, r, err)
+				t.Fatalf("Test %d, record %d ('%s') error (%s)!", i, j, r, err)
 			}
 			records = append(records, rr)
 		}

middleware/pkg/response/classify.go

@@ -68,6 +68,4 @@ func classify(t Type) Class {
 	default:
 		return Error
 	}
-	// never reached
-	return All
 }

middleware/pkg/response/typify.go

@@ -10,7 +10,7 @@ import (
 type Type int
 
 const (
-	// Success indicates a positive reply
+	// NoError indicates a positive reply
 	NoError Type = iota
 	// NameError is a NXDOMAIN in header, SOA in auth.
 	NameError

middleware/proxy/client.go

@@ -10,19 +10,19 @@ import (
 	"github.com/miekg/dns"
 )
 
-type Client struct {
+type client struct {
 	Timeout time.Duration
 
 	group *singleflight.Group
 }
 
-func NewClient() *Client {
-	return &Client{Timeout: defaultTimeout, group: new(singleflight.Group)}
+func newClient() *client {
+	return &client{Timeout: defaultTimeout, group: new(singleflight.Group)}
 }
 
 // ServeDNS does not satisfy middleware.Handler, instead it interacts with the upstream
 // and returns the respons or an error.
-func (c *Client) ServeDNS(w dns.ResponseWriter, r *dns.Msg, u *UpstreamHost) (*dns.Msg, error) {
+func (c *client) ServeDNS(w dns.ResponseWriter, r *dns.Msg, u *UpstreamHost) (*dns.Msg, error) {
 	co, err := net.DialTimeout(request.Proto(w), u.Name, c.Timeout)
 	if err != nil {
 		return nil, err
@@ -47,7 +47,7 @@ func (c *Client) ServeDNS(w dns.ResponseWriter, r *dns.Msg, u *UpstreamHost) (*d
 	return reply, nil
 }
 
-func (c *Client) Exchange(m *dns.Msg, co net.Conn) (*dns.Msg, time.Duration, error) {
+func (c *client) Exchange(m *dns.Msg, co net.Conn) (*dns.Msg, time.Duration, error) {
 	t := "nop"
 	if t1, ok := dns.TypeToString[m.Question[0].Qtype]; ok {
 		t = t1
@@ -76,7 +76,7 @@ func (c *Client) Exchange(m *dns.Msg, co net.Conn) (*dns.Msg, time.Duration, err
 
 // exchange does *not* return a pointer to dns.Msg because that leads to buffer reuse when
 // group.Do is used in Exchange.
-func (c *Client) exchange(m *dns.Msg, co net.Conn) (dns.Msg, error) {
+func (c *client) exchange(m *dns.Msg, co net.Conn) (dns.Msg, error) {
 	opt := m.IsEdns0()
 
 	udpsize := uint16(dns.MinMsgSize)

middleware/proxy/lookup.go

@@ -13,7 +13,7 @@ import (
 
 // New create a new proxy with the hosts in host and a Random policy.
 func New(hosts []string) Proxy {
-	p := Proxy{Next: nil, Client: NewClient()}
+	p := Proxy{Next: nil, Client: newClient()}
 
 	upstream := &staticUpstream{
 		from:        "",

middleware/proxy/proxy.go

@@ -17,7 +17,7 @@ var errUnreachable = errors.New("unreachable backend")
 // Proxy represents a middleware instance that can proxy requests to another DNS server.
 type Proxy struct {
 	Next      middleware.Handler
-	Client    *Client
+	Client    *client
 	Upstreams []Upstream
 }
 

middleware/proxy/setup.go

@@ -20,7 +20,7 @@ func setup(c *caddy.Controller) error {
 		return middleware.Error("proxy", err)
 	}
 	dnsserver.GetConfig(c).AddMiddleware(func(next middleware.Handler) middleware.Handler {
-		return Proxy{Next: next, Client: NewClient(), Upstreams: upstreams}
+		return Proxy{Next: next, Client: newClient(), Upstreams: upstreams}
 	})
 
 	return nil

request/request.go

@@ -1,4 +1,4 @@
-// Package requests abstract a client's request so that all middleware
+// Package request abstracts a client's request so that all middleware
 // will handle them in an unified way.
 package request
 

test/wildcard_test.go

@@ -43,7 +43,7 @@ func TestLookupWildcard(t *testing.T) {
 	for _, lookup := range []string{"w.example.org.", "a.w.example.org.", "a.a.w.example.org."} {
 		resp, err := p.Lookup(state, lookup, dns.TypeTXT)
 		if err != nil || resp == nil {
-			t.Fatal("Expected to receive reply, but didn't for %s", lookup)
+			t.Fatalf("Expected to receive reply, but didn't for %s", lookup)
 		}
 
 		// ;; ANSWER SECTION: