Merge pull request #1347 from tariq1890/add_linter

add more linters and improve code quality

.golangci.yml

@@ -17,3 +17,6 @@ linters:
     - ineffassign
     - golint
     - goimports
+    - misspell
+    - unconvert
+    - staticcheck

internal/testutils/endpoint.go

@@ -63,8 +63,8 @@ func SameEndpoints(a, b []*endpoint.Endpoint) bool {
 		return false
 	}
 
-	sa := a[:]
+	sa := a
-	sb := b[:]
+	sb := b
 	sort.Sort(byAllFields(sa))
 	sort.Sort(byAllFields(sb))
 
@@ -82,8 +82,8 @@ func SameEndpointLabels(a, b []*endpoint.Endpoint) bool {
 		return false
 	}
 
-	sa := a[:]
+	sa := a
-	sb := b[:]
+	sb := b
 	sort.Sort(byAllFields(sa))
 	sort.Sort(byAllFields(sb))
 

provider/alibaba_cloud.go

@@ -237,10 +237,10 @@ func (p *AlibabaCloudProvider) refreshStsToken(sleepTime time.Duration) {
 		sleepTime = p.nextExpire.Sub(nowTime)
 		p.clientLock.RUnlock()
 		log.Infof("Distance expiration time %v", sleepTime)
-		if sleepTime < time.Duration(10*time.Minute) {
+		if sleepTime < 10*time.Minute {
-			sleepTime = time.Duration(time.Second * 1)
+			sleepTime = time.Second * 1
 		} else {
-			sleepTime = time.Duration(9 * time.Minute)
+			sleepTime = 9 * time.Minute
 			log.Info("Next fetch sts sleep interval : ", sleepTime.String())
 			continue
 		}

provider/aws.go

@@ -749,9 +749,9 @@ func canonicalHostedZone(hostname string) string {
 }
 
 // cleanZoneID removes the "/hostedzone/" prefix
-func cleanZoneID(ID string) string {
+func cleanZoneID(id string) string {
-	if strings.HasPrefix(ID, "/hostedzone/") {
+	if strings.HasPrefix(id, "/hostedzone/") {
-		ID = strings.TrimPrefix(ID, "/hostedzone/")
+		id = strings.TrimPrefix(id, "/hostedzone/")
 	}
-	return ID
+	return id
 }

provider/aws_sd.go

@@ -283,14 +283,12 @@ func (p *AWSSDProvider) submitCreates(namespaces []*sd.NamespaceSummary, changes
 				}
 				// update local list of services
 				services[*srv.Name] = srv
-			} else {
+			} else if (ch.RecordTTL.IsConfigured() && *srv.DnsConfig.DnsRecords[0].TTL != int64(ch.RecordTTL)) ||
+				aws.StringValue(srv.Description) != ch.Labels[endpoint.AWSSDDescriptionLabel] {
 				// update service when TTL or Description differ
-				if (ch.RecordTTL.IsConfigured() && *srv.DnsConfig.DnsRecords[0].TTL != int64(ch.RecordTTL)) ||
+				err = p.UpdateService(srv, ch)
-					aws.StringValue(srv.Description) != ch.Labels[endpoint.AWSSDDescriptionLabel] {
+				if err != nil {
-					err = p.UpdateService(srv, ch)
+					return err
-					if err != nil {
-						return err
-					}
 				}
 			}
 
@@ -442,13 +440,13 @@ func (p *AWSSDProvider) CreateService(namespaceID *string, srvName *string, ep *
 			Name:        srvName,
 			Description: aws.String(ep.Labels[endpoint.AWSSDDescriptionLabel]),
 			DnsConfig: &sd.DnsConfig{
-				NamespaceId:   namespaceID,
 				RoutingPolicy: aws.String(routingPolicy),
 				DnsRecords: []*sd.DnsRecord{{
 					Type: aws.String(srvType),
 					TTL:  aws.Int64(ttl),
 				}},
 			},
+			NamespaceId: namespaceID,
 		})
 		if err != nil {
 			return nil, err

provider/aws_sd_test.go

@@ -59,10 +59,10 @@ func (s *AWSSDClientStub) CreateService(input *sd.CreateServiceInput) (*sd.Creat
 		CreatorRequestId: input.CreatorRequestId,
 	}
 
-	nsServices, ok := s.services[*input.DnsConfig.NamespaceId]
+	nsServices, ok := s.services[*input.NamespaceId]
 	if !ok {
 		nsServices = make(map[string]*sd.Service)
-		s.services[*input.DnsConfig.NamespaceId] = nsServices
+		s.services[*input.NamespaceId] = nsServices
 	}
 	nsServices[*srv.Id] = srv
 
@@ -539,13 +539,13 @@ func TestAWSSDProvider_CreateService(t *testing.T) {
 	expectedServices["A-srv"] = &sd.Service{
 		Name: aws.String("A-srv"),
 		DnsConfig: &sd.DnsConfig{
-			NamespaceId:   aws.String("private"),
 			RoutingPolicy: aws.String(sd.RoutingPolicyMultivalue),
 			DnsRecords: []*sd.DnsRecord{{
 				Type: aws.String(sd.RecordTypeA),
 				TTL:  aws.Int64(60),
 			}},
 		},
+		NamespaceId: aws.String("private"),
 	}
 
 	// CNAME type
@@ -557,13 +557,13 @@ func TestAWSSDProvider_CreateService(t *testing.T) {
 	expectedServices["CNAME-srv"] = &sd.Service{
 		Name: aws.String("CNAME-srv"),
 		DnsConfig: &sd.DnsConfig{
-			NamespaceId:   aws.String("private"),
 			RoutingPolicy: aws.String(sd.RoutingPolicyWeighted),
 			DnsRecords: []*sd.DnsRecord{{
 				Type: aws.String(sd.RecordTypeCname),
 				TTL:  aws.Int64(80),
 			}},
 		},
+		NamespaceId: aws.String("private"),
 	}
 
 	// ALIAS type
@@ -575,13 +575,13 @@ func TestAWSSDProvider_CreateService(t *testing.T) {
 	expectedServices["ALIAS-srv"] = &sd.Service{
 		Name: aws.String("ALIAS-srv"),
 		DnsConfig: &sd.DnsConfig{
-			NamespaceId:   aws.String("private"),
 			RoutingPolicy: aws.String(sd.RoutingPolicyWeighted),
 			DnsRecords: []*sd.DnsRecord{{
 				Type: aws.String(sd.RecordTypeA),
 				TTL:  aws.Int64(100),
 			}},
 		},
+		NamespaceId: aws.String("private"),
 	}
 
 	validateAWSSDServicesMapsEqual(t, expectedServices, api.services["private"])

provider/aws_test.go

@@ -75,7 +75,7 @@ func NewRoute53APIStub() *Route53APIStub {
 
 func (r *Route53APIStub) ListResourceRecordSetsPages(input *route53.ListResourceRecordSetsInput, fn func(p *route53.ListResourceRecordSetsOutput, lastPage bool) (shouldContinue bool)) error {
 	output := route53.ListResourceRecordSetsOutput{} // TODO: Support optional input args.
-	if len(r.recordSets) <= 0 {
+	if len(r.recordSets) == 0 {
 		output.ResourceRecordSets = []*route53.ResourceRecordSet{}
 	} else if _, ok := r.recordSets[aws.StringValue(input.HostedZoneId)]; !ok {
 		output.ResourceRecordSets = []*route53.ResourceRecordSet{}

provider/azure.go

@@ -149,7 +149,7 @@ func getAccessToken(cfg config, environment azure.Environment) (*adal.ServicePri
 		return token, nil
 	}
 
-	// Try to retrive token with MSI.
+	// Try to retrieve token with MSI.
 	if cfg.UseManagedIdentityExtension {
 		log.Info("Using managed identity extension to retrieve access token for Azure API.")
 		msiEndpoint, err := adal.GetMSIVMEndpoint()
@@ -193,7 +193,7 @@ func (p *AzureProvider) Records() (endpoints []*endpoint.Endpoint, _ error) {
 				log.Error("Skipping invalid record set with nil name or type.")
 				return true
 			}
-			recordType := strings.TrimLeft(*recordSet.Type, "Microsoft.Network/dnszones/")
+			recordType := strings.TrimPrefix(*recordSet.Type, "Microsoft.Network/dnszones/")
 			if !supportedRecordType(recordType) {
 				return true
 			}
@@ -208,7 +208,7 @@ func (p *AzureProvider) Records() (endpoints []*endpoint.Endpoint, _ error) {
 				ttl = endpoint.TTL(*recordSet.TTL)
 			}
 
-			ep := endpoint.NewEndpointWithTTL(name, recordType, endpoint.TTL(ttl), targets...)
+			ep := endpoint.NewEndpointWithTTL(name, recordType, ttl, targets...)
 			log.Debugf(
 				"Found %s record for '%s' with target '%s'.",
 				ep.RecordType,

provider/azure_private_dns.go

@@ -106,7 +106,7 @@ func (p *AzurePrivateDNSProvider) Records() (endpoints []*endpoint.Endpoint, _ e
 				log.Debugf("Skipping invalid record set with missing type.")
 				return
 			}
-			recordType = strings.TrimLeft(*recordSet.Type, "Microsoft.Network/privateDnsZones")
+			recordType = strings.TrimPrefix(*recordSet.Type, "Microsoft.Network/privateDnsZones")
 
 			var name string
 			if recordSet.Name == nil {
@@ -126,7 +126,7 @@ func (p *AzurePrivateDNSProvider) Records() (endpoints []*endpoint.Endpoint, _ e
 				ttl = endpoint.TTL(*recordSet.TTL)
 			}
 
-			ep := endpoint.NewEndpointWithTTL(name, recordType, endpoint.TTL(ttl), targets...)
+			ep := endpoint.NewEndpointWithTTL(name, recordType, ttl, targets...)
 			log.Debugf(
 				"Found %s record for '%s' with target '%s'.",
 				ep.RecordType,

provider/azure_privatedns_test.go

@@ -54,7 +54,7 @@ func (m *mockPrivateZoneListResultPageIterator) getNextPage(context.Context, pri
 	// it assumed that instances of this kind of iterator are only skimmed through once per test
 	// otherwise a real implementation is required, e.g. based on a linked list
 	if m.offset < len(m.results) {
-		m.offset = m.offset + 1
+		m.offset++
 		return m.results[m.offset-1], nil
 	}
 
@@ -73,7 +73,7 @@ func (m *mockPrivateRecordSetListResultPageIterator) getNextPage(context.Context
 	// it assumed that instances of this kind of iterator are only skimmed through once per test
 	// otherwise a real implementation is required, e.g. based on a linked list
 	if m.offset < len(m.results) {
-		m.offset = m.offset + 1
+		m.offset++
 		return m.results[m.offset-1], nil
 	}
 

provider/azure_test.go

@@ -56,7 +56,7 @@ func (m *mockZoneListResultPageIterator) getNextPage(context.Context, dns.ZoneLi
 	// it assumed that instances of this kind of iterator are only skimmed through once per test
 	// otherwise a real implementation is required, e.g. based on a linked list
 	if m.offset < len(m.results) {
-		m.offset = m.offset + 1
+		m.offset++
 		return m.results[m.offset-1], nil
 	}
 
@@ -75,7 +75,7 @@ func (m *mockRecordSetListResultPageIterator) getNextPage(context.Context, dns.R
 	// it assumed that instances of this kind of iterator are only skimmed through once per test
 	// otherwise a real implementation is required, e.g. based on a linked list
 	if m.offset < len(m.results) {
-		m.offset = m.offset + 1
+		m.offset++
 		return m.results[m.offset-1], nil
 	}
 

provider/digital_ocean.go

@@ -62,7 +62,7 @@ func NewDigitalOceanProvider(domainFilter DomainFilter, dryRun bool) (*DigitalOc
 	if !ok {
 		return nil, fmt.Errorf("No token found")
 	}
-	oauthClient := oauth2.NewClient(oauth2.NoContext, oauth2.StaticTokenSource(&oauth2.Token{
+	oauthClient := oauth2.NewClient(context.TODO(), oauth2.StaticTokenSource(&oauth2.Token{
 		AccessToken: token,
 	}))
 	client := godo.NewClient(oauthClient)

provider/dnsimple.go

@@ -156,7 +156,7 @@ func (p *dnsimpleProvider) Zones() (map[string]dnsimple.Zone, error) {
 	return zones, nil
 }
 
-// Records retuns a list of endpoints in a given zone
+// Records returns a list of endpoints in a given zone
 func (p *dnsimpleProvider) Records() (endpoints []*endpoint.Endpoint, _ error) {
 	zones, err := p.Zones()
 	if err != nil {

provider/dyn.go

@@ -40,7 +40,7 @@ const (
 	// when rate limit is hit retry up to 5 times after sleep 1m between retries
 	dynMaxRetriesOnErrRateLimited = 5
 
-	// two consecutive bad logins happen at least this many seconds appart
+	// two consecutive bad logins happen at least this many seconds apart
 	// While it is easy to get the username right, misconfiguring the password
 	// can get account blocked. Exit(1) is not a good solution
 	// as k8s will restart the pod and another login attempt will be made
@@ -51,7 +51,7 @@ const (
 )
 
 func unixNow() int64 {
-	return int64(time.Now().Unix())
+	return time.Now().Unix()
 }
 
 // DynConfig hold connection parameters to dyn.com and internal state

provider/google.go

@@ -320,7 +320,7 @@ func (p *GoogleProvider) submitChange(change *dns.Change) error {
 	return nil
 }
 
-// batchChange seperates a zone in multiple transaction.
+// batchChange separates a zone in multiple transaction.
 func batchChange(change *dns.Change, batchSize int) []*dns.Change {
 	changes := []*dns.Change{}
 
@@ -441,7 +441,7 @@ func newRecord(ep *endpoint.Endpoint) *dns.ResourceRecordSet {
 		targets[0] = ensureTrailingDot(targets[0])
 	}
 
-	// no annotation results in a Ttl of 0, default to 300 for backwards-compatability
+	// no annotation results in a Ttl of 0, default to 300 for backwards-compatibility
 	var ttl int64 = googleRecordTTL
 	if ep.RecordTTL.IsConfigured() {
 		ttl = int64(ep.RecordTTL)

provider/infoblox.go

@@ -108,7 +108,7 @@ func NewInfobloxProvider(infobloxConfig InfobloxConfig) (*InfobloxProvider, erro
 
 	var requestBuilder ibclient.HttpRequestBuilder
 	if infobloxConfig.MaxResults != 0 {
-		// use our own HttpRequestBuilder which sets _max_results paramter on GET requests
+		// use our own HttpRequestBuilder which sets _max_results parameter on GET requests
 		requestBuilder = NewMaxResultsRequestBuilder(infobloxConfig.MaxResults)
 	} else {
 		// use the default HttpRequestBuilder of the infoblox client

provider/infoblox_test.go

@@ -168,7 +168,7 @@ func (client *mockIBConnector) GetObject(obj ibclient.IBObject, ref string, res
 }
 
 func (client *mockIBConnector) DeleteObject(ref string) (refRes string, err error) {
-	re, _ := regexp.Compile(`([^/]+)/[^:]+:([^/]+)/default`)
+	re := regexp.MustCompile(`([^/]+)/[^:]+:([^/]+)/default`)
 	result := re.FindStringSubmatch(ref)
 
 	switch result[1] {

provider/linode.go

@@ -187,13 +187,11 @@ func (p *LinodeProvider) submitChanges(changes LinodeChanges) error {
 
 		if p.DryRun {
 			log.WithFields(logFields).Info("Would create record.")
-		} else {
+		} else if _, err := p.Client.CreateDomainRecord(context.TODO(), change.Domain.ID, change.Options); err != nil {
-			if _, err := p.Client.CreateDomainRecord(context.TODO(), change.Domain.ID, change.Options); err != nil {
+			log.WithFields(logFields).Errorf(
-				log.WithFields(logFields).Errorf(
+				"Failed to Create record: %v",
-					"Failed to Create record: %v",
+				err,
-					err,
+			)
-				)
-			}
 		}
 	}
 
@@ -210,13 +208,11 @@ func (p *LinodeProvider) submitChanges(changes LinodeChanges) error {
 
 		if p.DryRun {
 			log.WithFields(logFields).Info("Would delete record.")
-		} else {
+		} else if err := p.Client.DeleteDomainRecord(context.TODO(), change.Domain.ID, change.DomainRecord.ID); err != nil {
-			if err := p.Client.DeleteDomainRecord(context.TODO(), change.Domain.ID, change.DomainRecord.ID); err != nil {
+			log.WithFields(logFields).Errorf(
-				log.WithFields(logFields).Errorf(
+				"Failed to Delete record: %v",
-					"Failed to Delete record: %v",
+				err,
-					err,
+			)
-				)
-			}
 		}
 	}
 
@@ -233,13 +229,11 @@ func (p *LinodeProvider) submitChanges(changes LinodeChanges) error {
 
 		if p.DryRun {
 			log.WithFields(logFields).Info("Would update record.")
-		} else {
+		} else if _, err := p.Client.UpdateDomainRecord(context.TODO(), change.Domain.ID, change.DomainRecord.ID, change.Options); err != nil {
-			if _, err := p.Client.UpdateDomainRecord(context.TODO(), change.Domain.ID, change.DomainRecord.ID, change.Options); err != nil {
+			log.WithFields(logFields).Errorf(
-				log.WithFields(logFields).Errorf(
+				"Failed to Update record: %v",
-					"Failed to Update record: %v",
+				err,
-					err,
+			)
-				)
-			}
 		}
 	}
 

provider/oci_test.go

@@ -624,7 +624,7 @@ func TestMutableMockOCIDNSClient(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	// GetZoneRecords and check we're back in the origional state
+	// GetZoneRecords and check we're back in the original state
 	recordsResponse, err = client.GetZoneRecords(context.Background(), dns.GetZoneRecordsRequest{
 		ZoneNameOrId: zones[0].Id,
 	})

provider/pdns.go

@@ -42,7 +42,7 @@ type pdnsChangeType string
 const (
 	apiBase = "/api/v1"
 
-	// Unless we use something like pdnsproxy (discontinued upsteam), this value will _always_ be localhost
+	// Unless we use something like pdnsproxy (discontinued upstream), this value will _always_ be localhost
 	defaultServerID = "localhost"
 	defaultTTL      = 300
 
@@ -455,7 +455,7 @@ func (p *PDNSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes)
 	// valid call and a no-op, but we might as well not make the call to
 	// prevent unnecessary logging
 	if len(changes.Create) > 0 {
-		// "Replacing" non-existant records creates them
+		// "Replacing" non-existent records creates them
 		err := p.mutateRecords(changes.Create, PdnsReplace)
 		if err != nil {
 			return err

provider/pdns_test.go

@@ -536,7 +536,7 @@ func (c *PDNSAPIClientStub) PatchZone(zoneID string, zoneStruct pgo.Zone) (*http
 /******************************************************************************/
 // API that returns a zones with no records
 type PDNSAPIClientStubEmptyZones struct {
-	// Keep track of all zones we recieve via PatchZone
+	// Keep track of all zones we receive via PatchZone
 	patchedZones []pgo.Zone
 }
 

provider/rcode0.go

@@ -129,10 +129,8 @@ func (p *RcodeZeroProvider) Records() ([]*endpoint.Endpoint, error) {
 						}
 					}
 
-				} else {
+				} else if !r.Records[0].Disabled {
-					if !r.Records[0].Disabled {
+					endpoints = append(endpoints, endpoint.NewEndpointWithTTL(r.Name, r.Type, endpoint.TTL(r.TTL), r.Records[0].Content))
-						endpoints = append(endpoints, endpoint.NewEndpointWithTTL(r.Name, r.Type, endpoint.TTL(r.TTL), r.Records[0].Content))
-					}
 				}
 
 			}

provider/rdns_test.go

@@ -65,7 +65,7 @@ func (c fakeEtcdv3Client) List(rootDomain string) ([]RDNSRecord, error) {
 
 		k := &mvccpb.KeyValue{
 			Key:   []byte(v.Key),
-			Value: []byte(b),
+			Value: b,
 		}
 
 		r.Kvs = append(r.Kvs, k)

provider/vinyldns.go

@@ -97,7 +97,7 @@ func (p *vinyldnsProvider) Records() (endpoints []*endpoint.Endpoint, _ error) {
 		}
 
 		for _, r := range records {
-			if supportedRecordType(string(r.Type)) {
+			if supportedRecordType(r.Type) {
 				recordsCount := len(r.Records)
 				log.Debugf(fmt.Sprintf("%s.%s.%d.%s", r.Name, r.Type, recordsCount, zone.Name))
 
@@ -115,7 +115,7 @@ func (p *vinyldnsProvider) Records() (endpoints []*endpoint.Endpoint, _ error) {
 						}
 					}
 
-					endpoints = append(endpoints, endpoint.NewEndpointWithTTL(r.Name+"."+zone.Name, string(r.Type), endpoint.TTL(r.TTL), targets...))
+					endpoints = append(endpoints, endpoint.NewEndpointWithTTL(r.Name+"."+zone.Name, r.Type, endpoint.TTL(r.TTL), targets...))
 				}
 			}
 		}

source/connector.go

@@ -60,7 +60,7 @@ func (cs *connectorSource) Endpoints() ([]*endpoint.Endpoint, error) {
 		return nil, err
 	}
 
-	log.Debugf("Recieved endpoints: %#v", endpoints)
+	log.Debugf("Received endpoints: %#v", endpoints)
 
 	return endpoints, nil
 }

source/node.go

@@ -171,8 +171,8 @@ func (ns *nodeSource) Endpoints() ([]*endpoint.Endpoint, error) {
 // basically what k8s.io/kubernetes/pkg/util/node.GetPreferredNodeAddress does
 func (ns *nodeSource) nodeAddresses(node *v1.Node) ([]string, error) {
 	addresses := map[v1.NodeAddressType][]string{
-		v1.NodeExternalIP: []string{},
+		v1.NodeExternalIP: {},
-		v1.NodeInternalIP: []string{},
+		v1.NodeInternalIP: {},
 	}
 
 	for _, addr := range node.Status.Addresses {