diff --git a/.golangci.yml b/.golangci.yml index fcb5072d..d0571a1c 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,36 +1,202 @@ run: timeout: 5m + issues-exit-code: 1 + +output: + formats: + - format: colored-line-number + +linters-settings: + cyclop: + max-complexity: 15 + + depguard: + rules: + main: + files: + - "$all" + - "!$test" + deny: + - pkg: "reflect" + desc: "Reflection is never clear." + - pkg: "gob" + desc: "Please convert types manually" + + dupl: + threshold: 100 + + errcheck: + check-type-assertions: true + check-blank: true + exclude-functions: + - fmt:.* + - io/ioutil:^Read.* + + gci: + sections: + - standard + - default + - blank + - dot + - prefix(github.com/linode/linode-cloud-controller-manager) + + goconst: + min-len: 3 + min-occurrences: 5 + + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + + settings: + captLocal: + paramsOnly: true + rangeValCopy: + sizeThreshold: 32 + + gofmt: + simplify: true + + goimports: + local-prefixes: github.com/linode/linode-cloud-controller-manager + + govet: + enable: + - shadow + + gosec: + confidence: "medium" + excludes: + - G115 + + nolintlint: + require-explanation: true + require-specific: true + + prealloc: + simple: true + range-loops: true + for-loops: true + + varnamelen: + min-name-length: 2 linters: - disable-all: true + #disable-all: true + disable: + - errcheck + - errchkjson + - errorlint + - govet + - testifylint enable: # these are enabled by default - - errcheck + #- errcheck - gosimple - - govet + #- govet - ineffassign - staticcheck - typecheck - unused - # cherry picked from https://golangci-lint.run/usage/linters/ - # - ginkgolinter # to be enabled once #158 is merged + - ginkgolinter + - asasalint + - asciicheck + - bidichk - bodyclose + - containedctx + - contextcheck - copyloopvar + #- cyclop + - decorder + #- depguard + - dogsled + - dupl + - dupword + - durationcheck + #- errchkjson + - errname + #- errorlint + - exhaustive + #- forbidigo + #- forcetypeassert + #- gci - gocheckcompilerdirectives - gofmt - goimports + #- gocognit + #- goconst + #- gocritic + - gofumpt + - goprintffuncname + #- gosec - importas - loggercheck + - maintidx - makezero + - misspell + #- mnd + - musttag + #- nestif - nilerr - - prealloc + - nilnil + - noctx + - nolintlint + - nosprintfhostport + #- paralleltest + #- prealloc + - predeclared - reassign - - tenv + #- tenv + #- thelper - unconvert - - wastedassign - unparam - - gofumpt - - nosprintfhostport - - musttag - - exhaustive - - nilnil + - usestdlibvars + #- varnamelen + - wastedassign + - whitespace + + presets: + - bugs + - unused + fast: false + +issues: + exclude-rules: + # Exclude some linters from running on tests files. + - path: _test(ing)?\.go + linters: + - gocyclo + - maintidx + - errcheck + - dupl + - gosec + - copyloopvar + - unparam + - varnamelen + + # Ease some gocritic warnings on test files. + - path: _test\.go + text: "(unnamedResult|exitAfterDefer)" + linters: + - gocritic + + - text: "G101:" + linters: + - gosec + - gas + + - text: "G104:" + linters: + - gosec + - gas + + exclude-use-default: false + new: false + max-issues-per-linter: 0 + max-same-issues: 0 + exclude-files: + - "zz_generated\\..+\\.go$" diff --git a/cloud/linode/cilium_loadbalancers.go b/cloud/linode/cilium_loadbalancers.go index 71dc5632..f69bc0c8 100644 --- a/cloud/linode/cilium_loadbalancers.go +++ b/cloud/linode/cilium_loadbalancers.go @@ -13,7 +13,6 @@ import ( ciliumclient "github.com/cilium/cilium/pkg/k8s/client/clientset/versioned/typed/cilium.io/v2alpha1" slimv1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/meta/v1" "github.com/google/uuid" - "github.com/linode/linode-cloud-controller-manager/cloud/annotations" "github.com/linode/linodego" v1 "k8s.io/api/core/v1" k8serrors "k8s.io/apimachinery/pkg/api/errors" @@ -23,6 +22,8 @@ import ( "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "k8s.io/utils/ptr" + + "github.com/linode/linode-cloud-controller-manager/cloud/annotations" ) const ( diff --git a/cloud/linode/cilium_loadbalancers_test.go b/cloud/linode/cilium_loadbalancers_test.go index f03bfaeb..ea5102ae 100644 --- a/cloud/linode/cilium_loadbalancers_test.go +++ b/cloud/linode/cilium_loadbalancers_test.go @@ -10,11 +10,12 @@ import ( k8sClient "github.com/cilium/cilium/pkg/k8s/client" fakev2alpha1 "github.com/cilium/cilium/pkg/k8s/client/clientset/versioned/typed/cilium.io/v2alpha1/fake" "github.com/golang/mock/gomock" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" "github.com/linode/linodego" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) var ( diff --git a/cloud/linode/client/client.go b/cloud/linode/client/client.go index def8097e..609f88e1 100644 --- a/cloud/linode/client/client.go +++ b/cloud/linode/client/client.go @@ -11,9 +11,10 @@ import ( "os" "time" - _ "github.com/hexdigest/gowrap" "github.com/linode/linodego" "k8s.io/klog/v2" + + _ "github.com/hexdigest/gowrap" ) const ( diff --git a/cloud/linode/cloud.go b/cloud/linode/cloud.go index c15a9fd7..c901891f 100644 --- a/cloud/linode/cloud.go +++ b/cloud/linode/cloud.go @@ -77,7 +77,7 @@ func init() { func newLinodeClientWithPrometheus(apiToken string, timeout time.Duration) (client.Client, error) { linodeClient, err := client.New(apiToken, timeout) if err != nil { - return nil, fmt.Errorf("client was not created succesfully: %w", err) + return nil, fmt.Errorf("client was not created successfully: %w", err) } if Options.LinodeGoDebug { diff --git a/cloud/linode/cloud_test.go b/cloud/linode/cloud_test.go index c6f2c97d..a69cf689 100644 --- a/cloud/linode/cloud_test.go +++ b/cloud/linode/cloud_test.go @@ -6,9 +6,10 @@ import ( "testing" "github.com/golang/mock/gomock" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" "github.com/stretchr/testify/assert" cloudprovider "k8s.io/cloud-provider" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) func TestNewCloudRouteControllerDisabled(t *testing.T) { @@ -75,7 +76,7 @@ func TestNewCloud(t *testing.T) { }() _, err := newCloud() assert.NoError(t, err, "expected no error if deprecated flag vpcname is set") - assert.Equal(t, Options.VPCNames, "tt", "expected vpcnames to be set to vpcname") + assert.Equal(t, "tt", Options.VPCNames, "expected vpcnames to be set to vpcname") }) t.Run("should fail if incorrect loadbalancertype is set", func(t *testing.T) { diff --git a/cloud/linode/fake_linode_test.go b/cloud/linode/fake_linode_test.go index dbedc245..43f366a6 100644 --- a/cloud/linode/fake_linode_test.go +++ b/cloud/linode/fake_linode_test.go @@ -174,7 +174,7 @@ func (f *fakeAPI) setupRoutes() { f.mux.HandleFunc("GET /v4/nodebalancers/{nodeBalancerId}", func(w http.ResponseWriter, r *http.Request) { nb, found := f.nb[r.PathValue("nodeBalancerId")] if !found { - w.WriteHeader(404) + w.WriteHeader(http.StatusNotFound) resp := linodego.APIError{ Errors: []linodego.APIErrorReason{ {Reason: "Not Found"}, @@ -284,7 +284,7 @@ func (f *fakeAPI) setupRoutes() { firewallDevices, found := f.fwd[fwdId] if !found { - w.WriteHeader(404) + w.WriteHeader(http.StatusNotFound) resp := linodego.APIError{ Errors: []linodego.APIErrorReason{ {Reason: "Not Found"}, @@ -728,7 +728,7 @@ func (f *fakeAPI) setupRoutes() { return } - w.WriteHeader(404) + w.WriteHeader(http.StatusNotFound) resp := linodego.APIError{ Errors: []linodego.APIErrorReason{ {Reason: "Not Found"}, @@ -767,7 +767,7 @@ func (f *fakeAPI) setupRoutes() { return } - w.WriteHeader(404) + w.WriteHeader(http.StatusNotFound) resp := linodego.APIError{ Errors: []linodego.APIErrorReason{ {Reason: "Not Found"}, diff --git a/cloud/linode/firewall/firewalls.go b/cloud/linode/firewall/firewalls.go index 8fe4c713..1e700984 100644 --- a/cloud/linode/firewall/firewalls.go +++ b/cloud/linode/firewall/firewalls.go @@ -8,9 +8,8 @@ import ( "strconv" "strings" - "golang.org/x/exp/slices" - "github.com/linode/linodego" + "golang.org/x/exp/slices" v1 "k8s.io/api/core/v1" "k8s.io/klog/v2" @@ -23,6 +22,8 @@ const ( maxFirewallRuleDescLen = 100 maxIPsPerFirewall = 255 maxRulesPerFirewall = 25 + accept = "ACCEPT" + drop = "DROP" ) var ( @@ -135,7 +136,7 @@ func ruleChanged(old linodego.FirewallRuleSet, newACL aclConfig) bool { var ips *linodego.NetworkAddresses if newACL.AllowList != nil { // this is a allowList, this means that the rules should have `DROP` as inboundpolicy - if old.InboundPolicy != "DROP" { + if old.InboundPolicy != drop { return true } if (newACL.AllowList.IPv4 != nil || newACL.AllowList.IPv6 != nil) && len(old.Inbound) == 0 { @@ -145,7 +146,7 @@ func ruleChanged(old linodego.FirewallRuleSet, newACL aclConfig) bool { } if newACL.DenyList != nil { - if old.InboundPolicy != "ACCEPT" { + if old.InboundPolicy != accept { return true } @@ -252,13 +253,13 @@ func processACL(fwcreateOpts *linodego.FirewallCreateOptions, aclType, label, sv }) } - fwcreateOpts.Rules.OutboundPolicy = "ACCEPT" - if aclType == "ACCEPT" { + fwcreateOpts.Rules.OutboundPolicy = accept + if aclType == accept { // if an allowlist is present, we drop everything else. - fwcreateOpts.Rules.InboundPolicy = "DROP" + fwcreateOpts.Rules.InboundPolicy = drop } else { // if a denylist is present, we accept everything else. - fwcreateOpts.Rules.InboundPolicy = "ACCEPT" + fwcreateOpts.Rules.InboundPolicy = accept } if len(fwcreateOpts.Rules.Inbound) > maxRulesPerFirewall { @@ -494,7 +495,7 @@ func CreateFirewallOptsForSvc(label string, tags []string, svc *v1.Service) (*li servicePorts = append(servicePorts, strconv.Itoa(int(port.Port))) } - portsString := strings.Join(servicePorts[:], ",") + portsString := strings.Join(servicePorts, ",") var acl aclConfig if err := json.Unmarshal([]byte(aclString), &acl); err != nil { return nil, err @@ -504,10 +505,10 @@ func CreateFirewallOptsForSvc(label string, tags []string, svc *v1.Service) (*li return nil, ErrInvalidFWConfig } - aclType := "ACCEPT" + aclType := accept allowedIPs := acl.AllowList if acl.DenyList != nil { - aclType = "DROP" + aclType = drop allowedIPs = acl.DenyList } diff --git a/cloud/linode/health_check.go b/cloud/linode/health_check.go index dc0d0e30..458a90a9 100644 --- a/cloud/linode/health_check.go +++ b/cloud/linode/health_check.go @@ -4,9 +4,10 @@ import ( "context" "time" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" ) type healthChecker struct { diff --git a/cloud/linode/health_check_test.go b/cloud/linode/health_check_test.go index bd1570d0..7ba7cd57 100644 --- a/cloud/linode/health_check_test.go +++ b/cloud/linode/health_check_test.go @@ -5,8 +5,9 @@ import ( "time" "github.com/golang/mock/gomock" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" "github.com/linode/linodego" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) func TestHealthCheck(t *testing.T) { diff --git a/cloud/linode/instances.go b/cloud/linode/instances.go index c93a9fa7..e6608f94 100644 --- a/cloud/linode/instances.go +++ b/cloud/linode/instances.go @@ -100,7 +100,6 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) newNodes := make(map[int]linodeInstance, len(instances)) for i, instance := range instances { - // if running within VPC, only store instances in cache which are part of VPC if Options.VPCNames != "" && len(vpcNodes[instance.ID]) == 0 { continue diff --git a/cloud/linode/instances_test.go b/cloud/linode/instances_test.go index 6cd6e183..97edf43a 100644 --- a/cloud/linode/instances_test.go +++ b/cloud/linode/instances_test.go @@ -10,12 +10,13 @@ import ( "testing" "github.com/golang/mock/gomock" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" "github.com/linode/linodego" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" cloudprovider "k8s.io/cloud-provider" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) func nodeWithProviderID(providerID string) *v1.Node { @@ -94,7 +95,7 @@ func TestMetadataRetrieval(t *testing.T) { node := nodeWithName(name) meta, err := instances.InstanceMetadata(ctx, node) - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, providerIDPrefix+strconv.Itoa(expectedInstance.ID), meta.ProviderID) }) @@ -352,7 +353,7 @@ func TestMetadataRetrieval(t *testing.T) { meta, err := instances.InstanceMetadata(ctx, node) - assert.Equal(t, err, test.expectedErr) + assert.Equal(t, test.expectedErr, err) if test.expectedErr == nil { assert.Equal(t, region, meta.Region) assert.Equal(t, linodeType, meta.InstanceType) @@ -420,9 +421,9 @@ func TestMetadataRetrieval(t *testing.T) { meta, err := instances.InstanceMetadata(ctx, &node) if test.expectedErr != nil { assert.Nil(t, meta) - assert.Equal(t, err, test.expectedErr) + assert.Equal(t, test.expectedErr, err) } else { - assert.Nil(t, err) + assert.NoError(t, err) assert.Equal(t, providerIDPrefix+strconv.Itoa(expectedInstance.ID), meta.ProviderID) } }) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index 23c0c36b..093ab45b 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -289,7 +289,6 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri return lbStatus, nil } -//nolint:funlen func (l *loadbalancers) updateNodeBalancer( ctx context.Context, clusterName string, @@ -416,7 +415,7 @@ func (l *loadbalancers) updateNodeBalancer( currentNBCfg, err = l.client.CreateNodeBalancerConfig(ctx, nb.ID, createOpts) if err != nil { sentry.CaptureError(ctx, err) - return fmt.Errorf("[port %d] error creating NodeBalancer config: %v", int(port.Port), err) + return fmt.Errorf("[port %d] error creating NodeBalancer config: %w", int(port.Port), err) } rebuildOpts = currentNBCfg.GetRebuildOptions() @@ -432,7 +431,7 @@ func (l *loadbalancers) updateNodeBalancer( if _, err = l.client.RebuildNodeBalancerConfig(ctx, nb.ID, currentNBCfg.ID, rebuildOpts); err != nil { sentry.CaptureError(ctx, err) - return fmt.Errorf("[port %d] error rebuilding NodeBalancer config: %v", int(port.Port), err) + return fmt.Errorf("[port %d] error rebuilding NodeBalancer config: %w", int(port.Port), err) } } @@ -469,7 +468,7 @@ func (l *loadbalancers) UpdateLoadBalancer(ctx context.Context, clusterName stri serviceWithStatus := service.DeepCopy() serviceWithStatus.Status.LoadBalancer, err = l.getLatestServiceLoadBalancerStatus(ctx, service) if err != nil { - return fmt.Errorf("failed to get latest LoadBalancer status for service (%s): %s", getServiceNn(service), err) + return fmt.Errorf("failed to get latest LoadBalancer status for service (%s): %w", getServiceNn(service), err) } nb, err := l.getNodeBalancerForService(ctx, serviceWithStatus) @@ -711,7 +710,6 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri return l.client.CreateNodeBalancer(ctx, createOpts) } -//nolint:funlen func (l *loadbalancers) buildNodeBalancerConfig(ctx context.Context, service *v1.Service, port int) (linodego.NodeBalancerConfig, error) { portConfig, err := getPortConfig(service, port) if err != nil { diff --git a/cloud/linode/metrics.go b/cloud/linode/metrics.go index a447dfdb..7973b7a1 100644 --- a/cloud/linode/metrics.go +++ b/cloud/linode/metrics.go @@ -3,9 +3,9 @@ package linode import ( "sync" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" - "k8s.io/component-base/metrics/legacyregistry" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" ) var registerOnce sync.Once diff --git a/cloud/linode/node_controller_test.go b/cloud/linode/node_controller_test.go index 409bc24d..65e093a5 100644 --- a/cloud/linode/node_controller_test.go +++ b/cloud/linode/node_controller_test.go @@ -9,8 +9,6 @@ import ( "time" "github.com/golang/mock/gomock" - "github.com/linode/linode-cloud-controller-manager/cloud/annotations" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" "github.com/linode/linodego" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" @@ -18,6 +16,9 @@ import ( "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/util/workqueue" + + "github.com/linode/linode-cloud-controller-manager/cloud/annotations" + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) func TestNodeController_Run(t *testing.T) { diff --git a/cloud/linode/service_controller.go b/cloud/linode/service_controller.go index 684cac7e..6d24f45b 100644 --- a/cloud/linode/service_controller.go +++ b/cloud/linode/service_controller.go @@ -40,7 +40,7 @@ func (s *serviceController) Run(stopCh <-chan struct{}) { return } - if service.Spec.Type != "LoadBalancer" { + if service.Spec.Type != v1.ServiceTypeLoadBalancer { return } @@ -57,7 +57,7 @@ func (s *serviceController) Run(stopCh <-chan struct{}) { return } - if newSvc.Spec.Type != "LoadBalancer" && oldSvc.Spec.Type == "LoadBalancer" { + if newSvc.Spec.Type != v1.ServiceTypeLoadBalancer && oldSvc.Spec.Type == v1.ServiceTypeLoadBalancer { klog.Infof("ServiceController will handle service (%s) LoadBalancer deletion", getServiceNn(oldSvc)) s.queue.Add(oldSvc) } diff --git a/cloud/linode/service_controller_test.go b/cloud/linode/service_controller_test.go index 8d90d9ea..2bf04001 100644 --- a/cloud/linode/service_controller_test.go +++ b/cloud/linode/service_controller_test.go @@ -6,12 +6,13 @@ import ( "time" "github.com/golang/mock/gomock" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/util/workqueue" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) func Test_serviceController_Run(t *testing.T) { diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go index ad72e346..472306bd 100644 --- a/cloud/linode/vpc.go +++ b/cloud/linode/vpc.go @@ -9,9 +9,10 @@ import ( "strings" "sync" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" "github.com/linode/linodego" "k8s.io/klog/v2" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" ) var ( diff --git a/cloud/linode/vpc_test.go b/cloud/linode/vpc_test.go index 0197712d..8086b7b2 100644 --- a/cloud/linode/vpc_test.go +++ b/cloud/linode/vpc_test.go @@ -9,9 +9,10 @@ import ( "testing" "github.com/golang/mock/gomock" - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" "github.com/linode/linodego" "github.com/stretchr/testify/assert" + + "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) func TestGetAllVPCIDs(t *testing.T) { diff --git a/main.go b/main.go index e312ae4c..ba8a62a3 100644 --- a/main.go +++ b/main.go @@ -7,10 +7,6 @@ import ( "net" "os" - "k8s.io/component-base/logs" - - "github.com/linode/linode-cloud-controller-manager/cloud/linode" - "github.com/linode/linode-cloud-controller-manager/sentry" "github.com/linode/linodego" "github.com/spf13/pflag" cloudprovider "k8s.io/cloud-provider" @@ -19,8 +15,12 @@ import ( "k8s.io/cloud-provider/names" "k8s.io/cloud-provider/options" utilflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" "k8s.io/klog/v2" + "github.com/linode/linode-cloud-controller-manager/cloud/linode" + "github.com/linode/linode-cloud-controller-manager/sentry" + _ "k8s.io/component-base/metrics/prometheus/clientgo" // for client metric registration _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration )