From aa7744dc86455c89a2c8396b5ada71134064822d Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Wed, 12 Oct 2016 12:46:35 +0100 Subject: [PATCH] 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 | 3 +- middleware/file/tree/elem.go | 2 +- middleware/kubernetes/controller.go | 18 +++++------ middleware/kubernetes/kubernetes.go | 12 +++---- .../kubernetes/nametemplate/nametemplate.go | 31 ++++++++++--------- .../nametemplate/nametemplate_test.go | 30 +++++++++--------- middleware/kubernetes/setup.go | 2 +- middleware/pkg/dnsutil/cname_test.go | 4 +-- middleware/pkg/response/classify.go | 2 -- middleware/pkg/response/typify.go | 2 +- middleware/proxy/client.go | 12 +++---- middleware/proxy/lookup.go | 2 +- middleware/proxy/proxy.go | 2 +- middleware/proxy/setup.go | 2 +- request/request.go | 2 +- test/wildcard_test.go | 2 +- 16 files changed, 64 insertions(+), 64 deletions(-) diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index ef9a44bd6..b4e4bc13e 100644 --- a/core/dnsserver/server.go +++ b/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 ( diff --git a/middleware/file/tree/elem.go b/middleware/file/tree/elem.go index 75429c97a..234e2c848 100644 --- a/middleware/file/tree/elem.go +++ b/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 { diff --git a/middleware/kubernetes/controller.go b/middleware/kubernetes/controller.go index 943e4e32a..89715a61b 100644 --- a/middleware/kubernetes/controller.go +++ b/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 diff --git a/middleware/kubernetes/kubernetes.go b/middleware/kubernetes/kubernetes.go index 853cc311a..41f329e16 100644 --- a/middleware/kubernetes/kubernetes.go +++ b/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 diff --git a/middleware/kubernetes/nametemplate/nametemplate.go b/middleware/kubernetes/nametemplate/nametemplate.go index 930d4855e..e879006cc 100644 --- a/middleware/kubernetes/nametemplate/nametemplate.go +++ b/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 diff --git a/middleware/kubernetes/nametemplate/nametemplate_test.go b/middleware/kubernetes/nametemplate/nametemplate_test.go index 1f6bb1590..5d496a1ac 100644 --- a/middleware/kubernetes/nametemplate/nametemplate_test.go +++ b/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) diff --git a/middleware/kubernetes/setup.go b/middleware/kubernetes/setup.go index 7b5912dd6..98afc8334 100644 --- a/middleware/kubernetes/setup.go +++ b/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() { diff --git a/middleware/pkg/dnsutil/cname_test.go b/middleware/pkg/dnsutil/cname_test.go index d122fdf52..5fb8d3029 100644 --- a/middleware/pkg/dnsutil/cname_test.go +++ b/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) } diff --git a/middleware/pkg/response/classify.go b/middleware/pkg/response/classify.go index 0251623a0..e9f17fe00 100644 --- a/middleware/pkg/response/classify.go +++ b/middleware/pkg/response/classify.go @@ -68,6 +68,4 @@ func classify(t Type) Class { default: return Error } - // never reached - return All } diff --git a/middleware/pkg/response/typify.go b/middleware/pkg/response/typify.go index 6fdd0137a..20ec13453 100644 --- a/middleware/pkg/response/typify.go +++ b/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 diff --git a/middleware/proxy/client.go b/middleware/proxy/client.go index ba2f439cc..0a35c93e4 100644 --- a/middleware/proxy/client.go +++ b/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) diff --git a/middleware/proxy/lookup.go b/middleware/proxy/lookup.go index af39bc12b..549dfa0c7 100644 --- a/middleware/proxy/lookup.go +++ b/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: "", diff --git a/middleware/proxy/proxy.go b/middleware/proxy/proxy.go index 78383532a..a33024f12 100644 --- a/middleware/proxy/proxy.go +++ b/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 } diff --git a/middleware/proxy/setup.go b/middleware/proxy/setup.go index 5257d8bb0..26f01c11e 100644 --- a/middleware/proxy/setup.go +++ b/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 diff --git a/request/request.go b/request/request.go index 50b990897..b5da38771 100644 --- a/request/request.go +++ b/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 diff --git a/test/wildcard_test.go b/test/wildcard_test.go index 05a1b94f8..3a66dd7ca 100644 --- a/test/wildcard_test.go +++ b/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: