From abdf8bbc028655d6f931082663865f6efc301bc2 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Wed, 23 Apr 2025 08:22:43 +0100 Subject: [PATCH] chore(refactore): added lint checks Signed-off-by: ivan katliarchuk --- .golangci.yml | 6 ++- plan/conflict.go | 10 ++--- provider/akamai/akamai.go | 10 +---- provider/alibabacloud/alibaba_cloud_test.go | 40 ++++++++----------- provider/aws/aws.go | 36 ++++++++--------- provider/azure/config_test.go | 2 +- .../azure/{ => fixtures}/config_test.json | 14 +++---- provider/exoscale/exoscale.go | 10 ++--- provider/godaddy/godaddy.go | 23 +++-------- provider/oci/oci.go | 12 +++--- registry/dynamodb.go | 20 +++++----- source/crd.go | 2 +- 12 files changed, 81 insertions(+), 104 deletions(-) rename provider/azure/{ => fixtures}/config_test.json (95%) diff --git a/.golangci.yml b/.golangci.yml index 99b3dde04..17669081a 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,7 +1,8 @@ +# https://golangci-lint.run/usage/configuration/ version: "2" linters: default: none - enable: + enable: # golangci-lint help linters - dogsled - goprintffuncname - govet @@ -13,6 +14,9 @@ linters: - unconvert - unused - whitespace + - predeclared # Find code that shadows one of Go's predeclared identifiers + - sloglint # Ensure consistent code style when using log/slog + - asciicheck # Checks that all code identifiers does not have non-ASCII symbols in the name settings: exhaustive: default-signifies-exhaustive: false diff --git a/plan/conflict.go b/plan/conflict.go index 3044059b7..bf153dcfc 100644 --- a/plan/conflict.go +++ b/plan/conflict.go @@ -25,7 +25,7 @@ import ( ) // ConflictResolver is used to make a decision in case of two or more different kubernetes resources -// are trying to acquire same DNS name +// are trying to acquire the same DNS name type ConflictResolver interface { ResolveCreate(candidates []*endpoint.Endpoint) *endpoint.Endpoint ResolveUpdate(current *endpoint.Endpoint, candidates []*endpoint.Endpoint) *endpoint.Endpoint @@ -38,13 +38,13 @@ type PerResource struct{} // ResolveCreate is invoked when dns name is not owned by any resource // ResolveCreate takes "minimal" (string comparison of Target) endpoint to acquire the DNS record func (s PerResource) ResolveCreate(candidates []*endpoint.Endpoint) *endpoint.Endpoint { - var min *endpoint.Endpoint + var minE *endpoint.Endpoint for _, ep := range candidates { - if min == nil || s.less(ep, min) { - min = ep + if minE == nil || s.less(ep, minE) { + minE = ep } } - return min + return minE } // ResolveUpdate is invoked when dns name is already owned by "current" endpoint diff --git a/provider/akamai/akamai.go b/provider/akamai/akamai.go index bd36fd663..36ac0a194 100644 --- a/provider/akamai/akamai.go +++ b/provider/akamai/akamai.go @@ -90,14 +90,6 @@ type akamaiZone struct { func NewAkamaiProvider(akamaiConfig AkamaiConfig, akaService AkamaiDNSService) (provider.Provider, error) { var edgeGridConfig edgegrid.Config - /* - log.Debugf("Host: %s", akamaiConfig.ServiceConsumerDomain) - log.Debugf("ClientToken: %s", akamaiConfig.ClientToken) - log.Debugf("ClientSecret: %s", akamaiConfig.ClientSecret) - log.Debugf("AccessToken: %s", akamaiConfig.AccessToken) - log.Debugf("EdgePath: %s", akamaiConfig.EdgercPath) - log.Debugf("EdgeSection: %s", akamaiConfig.EdgercSection) - */ // environment overrides edgerc file but config needs to be complete if akamaiConfig.ServiceConsumerDomain == "" || akamaiConfig.ClientToken == "" || akamaiConfig.ClientSecret == "" || akamaiConfig.AccessToken == "" { // Kubernetes config incomplete or non existent. Can't mix and match. @@ -106,7 +98,7 @@ func NewAkamaiProvider(akamaiConfig AkamaiConfig, akaService AkamaiDNSService) ( edgeGridConfig, err = edgegrid.Init(akamaiConfig.EdgercPath, akamaiConfig.EdgercSection) // use default .edgerc location and section if err != nil { log.Errorf("Edgegrid Init Failed") - return &AkamaiProvider{}, err // return empty provider for backward compatibility + return &AkamaiProvider{}, err // return an empty provider for backward compatibility } edgeGridConfig.HeaderToSign = append(edgeGridConfig.HeaderToSign, "X-External-DNS") } else { diff --git a/provider/alibabacloud/alibaba_cloud_test.go b/provider/alibabacloud/alibaba_cloud_test.go index 8d4b47464..5bdb77a42 100644 --- a/provider/alibabacloud/alibaba_cloud_test.go +++ b/provider/alibabacloud/alibaba_cloud_test.go @@ -22,6 +22,7 @@ import ( "github.com/aliyun/alibaba-cloud-sdk-go/services/alidns" "github.com/aliyun/alibaba-cloud-sdk-go/services/pvtz" + "github.com/stretchr/testify/assert" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" @@ -223,18 +224,7 @@ func newTestAlibabaCloudProvider(private bool) *AlibabaCloudProvider { cfg := alibabaCloudConfig{ VPCID: "vpc-xxxxxx", } - // - //dnsClient, _ := alidns.NewClientWithAccessKey( - // cfg.RegionID, - // cfg.AccessKeyID, - // cfg.AccessKeySecret, - //) - // - //pvtzClient, _ := pvtz.NewClientWithAccessKey( - // "cn-hangzhou", - // cfg.AccessKeyID, - // cfg.AccessKeySecret, - //) + domainFilterTest := endpoint.NewDomainFilter([]string{"container-service.top.", "example.org"}) return &AlibabaCloudProvider{ @@ -256,8 +246,8 @@ func TestAlibabaCloudPrivateProvider_Records(t *testing.T) { if len(endpoints) != 2 { t.Errorf("Incorrect number of records: %d", len(endpoints)) } - for _, endpoint := range endpoints { - t.Logf("Endpoint for %++v", *endpoint) + for _, ep := range endpoints { + t.Logf("Endpoint for %++v", *ep) } } } @@ -271,8 +261,8 @@ func TestAlibabaCloudProvider_Records(t *testing.T) { if len(endpoints) != 2 { t.Errorf("Incorrect number of records: %d", len(endpoints)) } - for _, endpoint := range endpoints { - t.Logf("Endpoint for %++v", *endpoint) + for _, ep := range endpoints { + t.Logf("Endpoint for %++v", *ep) } } } @@ -313,7 +303,8 @@ func TestAlibabaCloudProvider_ApplyChanges(t *testing.T) { }, } ctx := context.Background() - p.ApplyChanges(ctx, &changes) + err := p.ApplyChanges(ctx, &changes) + assert.NoError(t, err) endpoints, err := p.Records(ctx) if err != nil { t.Errorf("Failed to get records: %v", err) @@ -321,8 +312,8 @@ func TestAlibabaCloudProvider_ApplyChanges(t *testing.T) { if len(endpoints) != 3 { t.Errorf("Incorrect number of records: %d", len(endpoints)) } - for _, endpoint := range endpoints { - t.Logf("Endpoint for %++v", *endpoint) + for _, ep := range endpoints { + t.Logf("Endpoint for %++v", *ep) } } for _, ep := range endpoints { @@ -343,8 +334,8 @@ func TestAlibabaCloudProvider_Records_PrivateZone(t *testing.T) { if len(endpoints) != 2 { t.Errorf("Incorrect number of records: %d", len(endpoints)) } - for _, endpoint := range endpoints { - t.Logf("Endpoint for %++v", *endpoint) + for _, ep := range endpoints { + t.Logf("Endpoint for %++v", *ep) } } } @@ -378,7 +369,8 @@ func TestAlibabaCloudProvider_ApplyChanges_PrivateZone(t *testing.T) { }, } ctx := context.Background() - p.ApplyChanges(ctx, &changes) + err := p.ApplyChanges(ctx, &changes) + assert.NoError(t, err) endpoints, err := p.Records(ctx) if err != nil { t.Errorf("Failed to get records: %v", err) @@ -386,8 +378,8 @@ func TestAlibabaCloudProvider_ApplyChanges_PrivateZone(t *testing.T) { if len(endpoints) != 2 { t.Errorf("Incorrect number of records: %d", len(endpoints)) } - for _, endpoint := range endpoints { - t.Logf("Endpoint for %++v", *endpoint) + for _, ep := range endpoints { + t.Logf("Endpoint for %++v", *ep) } } } diff --git a/provider/aws/aws.go b/provider/aws/aws.go index c11977433..97f35b509 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -232,7 +232,7 @@ type profiledZone struct { } func (cs Route53Changes) Route53Changes() []route53types.Change { - ret := []route53types.Change{} + var ret []route53types.Change for _, c := range cs { ret = append(ret, c.Change) } @@ -313,7 +313,7 @@ type AWSConfig struct { // NewAWSProvider initializes a new AWS Route53 based Provider. func NewAWSProvider(awsConfig AWSConfig, clients map[string]Route53API) (*AWSProvider, error) { - provider := &AWSProvider{ + pr := &AWSProvider{ clients: clients, domainFilter: awsConfig.DomainFilter, zoneIDFilter: awsConfig.ZoneIDFilter, @@ -331,7 +331,7 @@ func NewAWSProvider(awsConfig AWSConfig, clients map[string]Route53API) (*AWSPro failedChangesQueue: make(map[string]Route53Changes), } - return provider, nil + return pr, nil } // Zones returns the list of hosted zones. @@ -561,33 +561,33 @@ func (p *AWSProvider) records(ctx context.Context, zones map[string]*profiledZon } // Identify if old and new endpoints require DELETE/CREATE instead of UPDATE. -func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, new *endpoint.Endpoint) bool { - // a change of record type - if old.RecordType != new.RecordType { +func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, newE *endpoint.Endpoint) bool { + // a change of a record type + if old.RecordType != newE.RecordType { return true } // an ALIAS record change to/from an A if old.RecordType == endpoint.RecordTypeA { oldAlias, _ := old.GetProviderSpecificProperty(providerSpecificAlias) - newAlias, _ := new.GetProviderSpecificProperty(providerSpecificAlias) + newAlias, _ := newE.GetProviderSpecificProperty(providerSpecificAlias) if oldAlias != newAlias { return true } } // a set identifier change - if old.SetIdentifier != new.SetIdentifier { + if old.SetIdentifier != newE.SetIdentifier { return true } // a change of routing policy - // default to true for geolocation properties if any geolocation property exists in old/new but not the other + // defaults to true for geolocation properties if any geolocation property exists in old/new but not the other for _, propType := range [7]string{providerSpecificWeight, providerSpecificRegion, providerSpecificFailover, providerSpecificFailover, providerSpecificGeolocationContinentCode, providerSpecificGeolocationCountryCode, providerSpecificGeolocationSubdivisionCode} { _, oldPolicy := old.GetProviderSpecificProperty(propType) - _, newPolicy := new.GetProviderSpecificProperty(propType) + _, newPolicy := newE.GetProviderSpecificProperty(propType) if oldPolicy != newPolicy { return true } @@ -601,14 +601,14 @@ func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint var creates []*endpoint.Endpoint var updates []*endpoint.Endpoint - for i, new := range newEndpoints { - old := oldEndpoints[i] - if p.requiresDeleteCreate(old, new) { - deletes = append(deletes, old) - creates = append(creates, new) + for i, newE := range newEndpoints { + oldE := oldEndpoints[i] + if p.requiresDeleteCreate(oldE, newE) { + deletes = append(deletes, oldE) + creates = append(creates, newE) } else { // Safe to perform an UPSERT. - updates = append(updates, new) + updates = append(updates, newE) } } @@ -760,8 +760,8 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, func (p *AWSProvider) newChanges(action route53types.ChangeAction, endpoints []*endpoint.Endpoint) Route53Changes { changes := make(Route53Changes, 0, len(endpoints)) - for _, endpoint := range endpoints { - change := p.newChange(action, endpoint) + for _, ep := range endpoints { + change := p.newChange(action, ep) changes = append(changes, change) } diff --git a/provider/azure/config_test.go b/provider/azure/config_test.go index 1099515d3..956599a90 100644 --- a/provider/azure/config_test.go +++ b/provider/azure/config_test.go @@ -50,7 +50,7 @@ func TestGetCloudConfiguration(t *testing.T) { func TestOverrideConfiguration(t *testing.T) { _, filename, _, _ := runtime.Caller(0) - configFile := path.Join(path.Dir(filename), "config_test.json") + configFile := path.Join(path.Dir(filename), "fixtures/config_test.json") cfg, err := getConfig(configFile, "subscription-override", "rg-override", "", "aad-endpoint-override") if err != nil { t.Errorf("got unexpected err %v", err) diff --git a/provider/azure/config_test.json b/provider/azure/fixtures/config_test.json similarity index 95% rename from provider/azure/config_test.json rename to provider/azure/fixtures/config_test.json index ddcaa7695..36863ed7e 100644 --- a/provider/azure/config_test.json +++ b/provider/azure/fixtures/config_test.json @@ -1,7 +1,7 @@ -{ - "tenantId": "tenant", - "subscriptionId": "subscription", - "resourceGroup": "rg", - "aadClientId": "clientId", - "aadClientSecret": "clientSecret" -} +{ + "tenantId": "tenant", + "subscriptionId": "subscription", + "resourceGroup": "rg", + "aadClientId": "clientId", + "aadClientSecret": "clientSecret" +} diff --git a/provider/exoscale/exoscale.go b/provider/exoscale/exoscale.go index 6e13f14f2..ff83e82b7 100644 --- a/provider/exoscale/exoscale.go +++ b/provider/exoscale/exoscale.go @@ -310,10 +310,10 @@ func (f *zoneFilter) EndpointZoneID(endpoint *endpoint.Endpoint, zones map[strin func merge(updateOld, updateNew []*endpoint.Endpoint) []*endpoint.Endpoint { findMatch := func(template *endpoint.Endpoint) *endpoint.Endpoint { - for _, new := range updateNew { - if template.DNSName == new.DNSName && - template.RecordType == new.RecordType { - return new + for _, record := range updateNew { + if template.DNSName == record.DNSName && + template.RecordType == record.RecordType { + return record } } return nil @@ -323,7 +323,7 @@ func merge(updateOld, updateNew []*endpoint.Endpoint) []*endpoint.Endpoint { for _, old := range updateOld { matchingNew := findMatch(old) if matchingNew == nil { - // no match, shouldn't happen + // no match shouldn't happen continue } diff --git a/provider/godaddy/godaddy.go b/provider/godaddy/godaddy.go index ede7bf154..58e5465fa 100644 --- a/provider/godaddy/godaddy.go +++ b/provider/godaddy/godaddy.go @@ -19,8 +19,8 @@ package godaddy import ( "context" "encoding/json" - "errors" "fmt" + "slices" "strings" log "github.com/sirupsen/logrus" @@ -46,9 +46,6 @@ var actionNames = []string{ const domainsURI = "/v1/domains?statuses=ACTIVE,PENDING_DNS_ACTIVE" -// ErrRecordToMutateNotFound when ApplyChange has to update/delete and didn't found the record in the existing zone (Change with no record ID) -var ErrRecordToMutateNotFound = errors.New("record to mutate not found in current zone") - type gdClient interface { Patch(string, interface{}, interface{}) error Post(string, interface{}, interface{}) error @@ -294,14 +291,14 @@ func (p *GDProvider) groupByNameAndType(zoneRecords []gdRecords) []*endpoint.End recordName = strings.TrimPrefix(fmt.Sprintf("%s.%s", records[0].Name, zoneName), ".") } - endpoint := endpoint.NewEndpointWithTTL( + ep := endpoint.NewEndpointWithTTL( recordName, records[0].Type, endpoint.TTL(records[0].TTL), targets..., ) - endpoints = append(endpoints, endpoint) + endpoints = append(endpoints, ep) } } @@ -584,8 +581,8 @@ func countTargets(p *plan.Changes) int { count := 0 for _, endpoints := range changes { - for _, endpoint := range endpoints { - count += len(endpoint.Targets) + for _, ep := range endpoints { + count += len(ep.Targets) } } @@ -593,15 +590,7 @@ func countTargets(p *plan.Changes) int { } func maxOf(vars ...int64) int64 { - max := vars[0] - - for _, i := range vars { - if max < i { - max = i - } - } - - return max + return slices.Max(vars) } func toString(obj interface{}) string { diff --git a/provider/oci/oci.go b/provider/oci/oci.go index 87f57f78a..e56cbe3e0 100644 --- a/provider/oci/oci.go +++ b/provider/oci/oci.go @@ -23,7 +23,7 @@ import ( "strings" "time" - yaml "github.com/goccy/go-yaml" + "github.com/goccy/go-yaml" "github.com/oracle/oci-go-sdk/v65/common" "github.com/oracle/oci-go-sdk/v65/common/auth" "github.com/oracle/oci-go-sdk/v65/dns" @@ -153,7 +153,7 @@ func (p *OCIProvider) zones(ctx context.Context) (map[string]dns.ZoneSummary, er } zones := make(map[string]dns.ZoneSummary) scopes := []dns.GetZoneScopeEnum{dns.GetZoneScopeEnum(p.zoneScope)} - // If zone scope is empty, list all zones types. + // If the zone scope is empty, list all zones types. if p.zoneScope == "" { scopes = dns.GetGetZoneScopeEnumValues() } @@ -232,7 +232,7 @@ func (p *OCIProvider) addPaginatedZones(ctx context.Context, zones map[string]dn } func (p *OCIProvider) newFilteredRecordOperations(endpoints []*endpoint.Endpoint, opType dns.RecordOperationOperationEnum) []dns.RecordOperation { - ops := []dns.RecordOperation{} + var ops []dns.RecordOperation for _, ep := range endpoints { if ep == nil { continue @@ -261,7 +261,7 @@ func (p *OCIProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) return nil, provider.NewSoftError(fmt.Errorf("getting zones: %w", err)) } - endpoints := []*endpoint.Endpoint{} + var endpoints []*endpoint.Endpoint for _, zone := range zones { var page *string for { @@ -303,7 +303,7 @@ func (p *OCIProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error) func (p *OCIProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { log.Debugf("Processing changes: %+v", changes) - ops := []dns.RecordOperation{} + var ops []dns.RecordOperation ops = append(ops, p.newFilteredRecordOperations(changes.Create, dns.RecordOperationOperationAdd)...) ops = append(ops, p.newFilteredRecordOperations(changes.UpdateNew, dns.RecordOperationOperationAdd)...) @@ -349,7 +349,7 @@ func (p *OCIProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e // AdjustEndpoints modifies the endpoints as needed by the specific provider func (p *OCIProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { - adjustedEndpoints := []*endpoint.Endpoint{} + var adjustedEndpoints []*endpoint.Endpoint for _, e := range endpoints { // OCI DNS does not support the set-identifier attribute, so we remove it to avoid plan failure if e.SetIdentifier != "" { diff --git a/registry/dynamodb.go b/registry/dynamodb.go index 2088fda50..5b83ceeb0 100644 --- a/registry/dynamodb.go +++ b/registry/dynamodb.go @@ -300,12 +300,12 @@ func (im *DynamoDBRegistry) ApplyChanges(ctx context.Context, changes *plan.Chan if err != nil { return err } - for i, endpoint := range filteredChanges.Create { - if endpoint.Key() == key { - log.Infof("Skipping endpoint %v because owner does not match", endpoint) + for i, ep := range filteredChanges.Create { + if ep.Key() == key { + log.Infof("Skipping endpoint %v because owner does not match", ep) filteredChanges.Create = append(filteredChanges.Create[:i], filteredChanges.Create[i+1:]...) // The dynamodb insertion failed; remove from our cache. - im.removeFromCache(endpoint) + im.removeFromCache(ep) delete(im.labels, key) return nil } @@ -466,7 +466,7 @@ func toDynamoLabels(labels endpoint.Labels) dynamodbtypes.AttributeValue { return &dynamodbtypes.AttributeValueMemberM{Value: labelMap} } -func (im *DynamoDBRegistry) appendInsert(statements []dynamodbtypes.BatchStatementRequest, key endpoint.EndpointKey, new endpoint.Labels) []dynamodbtypes.BatchStatementRequest { +func (im *DynamoDBRegistry) appendInsert(statements []dynamodbtypes.BatchStatementRequest, key endpoint.EndpointKey, newL endpoint.Labels) []dynamodbtypes.BatchStatementRequest { return append(statements, dynamodbtypes.BatchStatementRequest{ Statement: aws.String(fmt.Sprintf("INSERT INTO %q VALUE {'k':?, 'o':?, 'l':?}", im.table)), ConsistentRead: aws.Bool(true), @@ -475,16 +475,16 @@ func (im *DynamoDBRegistry) appendInsert(statements []dynamodbtypes.BatchStateme &dynamodbtypes.AttributeValueMemberS{ Value: im.ownerID, }, - toDynamoLabels(new), + toDynamoLabels(newL), }, }) } -func (im *DynamoDBRegistry) appendUpdate(statements []dynamodbtypes.BatchStatementRequest, key endpoint.EndpointKey, old endpoint.Labels, new endpoint.Labels) []dynamodbtypes.BatchStatementRequest { - if len(old) == len(new) { +func (im *DynamoDBRegistry) appendUpdate(statements []dynamodbtypes.BatchStatementRequest, key endpoint.EndpointKey, old endpoint.Labels, newE endpoint.Labels) []dynamodbtypes.BatchStatementRequest { + if len(old) == len(newE) { equal := true for k, v := range old { - if newV, exists := new[k]; !exists || v != newV { + if newV, exists := newE[k]; !exists || v != newV { equal = false break } @@ -497,7 +497,7 @@ func (im *DynamoDBRegistry) appendUpdate(statements []dynamodbtypes.BatchStateme return append(statements, dynamodbtypes.BatchStatementRequest{ Statement: aws.String(fmt.Sprintf("UPDATE %q SET \"l\"=? WHERE \"k\"=?", im.table)), Parameters: []dynamodbtypes.AttributeValue{ - toDynamoLabels(new), + toDynamoLabels(newE), toDynamoKey(key), }, }) diff --git a/source/crd.go b/source/crd.go index 70a724073..31a775b06 100644 --- a/source/crd.go +++ b/source/crd.go @@ -148,7 +148,7 @@ func (cs *crdSource) AddEventHandler(ctx context.Context, handler func()) { AddFunc: func(obj interface{}) { handler() }, - UpdateFunc: func(old interface{}, new interface{}) { + UpdateFunc: func(old interface{}, newI interface{}) { handler() }, DeleteFunc: func(obj interface{}) {