diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index bf2293373..b14e56a82 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -25,7 +25,7 @@ build: test:lint: stage: test - image: golangci/golangci-lint:v1.64 + image: golangci/golangci-lint:v2.5 variables: GOLANGCI_LINT_CACHE: $CI_PROJECT_DIR/.cache/golangci-lint cache: @@ -36,7 +36,7 @@ test:lint: paths: - $GOLANGCI_LINT_CACHE script: - - golangci-lint run -v --timeout 5m + - golangci-lint run -v test:unit: stage: test diff --git a/.golangci.yaml b/.golangci.yaml index 8526901a3..3b17bcda4 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,67 +1,116 @@ ---- -linters-settings: - exhaustive: - default-signifies-exhaustive: true - gci: - sections: - - standard - - default - - prefix(github.com/hetznercloud) +version: "2" - importas: - no-unaliased: true - alias: - # Kubernetes - - pkg: k8s.io/api/core/v1 - alias: corev1 - - pkg: k8s.io/apimachinery/pkg/apis/meta/v1 - alias: metav1 - - - pkg: github.com/syself/hrobot-go - alias: hrobot - - pkg: github.com/syself/hrobot-go/models - alias: hrobotmodels - - misspell: - locale: "US" +run: + timeout: 5m linters: - disable-all: true enable: + - asasalint + - asciicheck + - bidichk - bodyclose + - contextcheck - copyloopvar + - durationcheck - errcheck + - errchkjson - errname + - errorlint - exhaustive - - gci + - forbidigo + - gocheckcompilerdirectives + - gochecksumtype - gocritic - godot - - goimports + - gomoddirectives - gomodguard - gosec - - gosimple + - gosmopolitan - govet - importas - ineffassign + - loggercheck + - makezero - misspell + - musttag + - nilerr + - nilnesserr + - noctx - prealloc + - protogetter + - reassign + - recvcheck - revive + - rowserrcheck + - spancheck - staticcheck - - typecheck + - testifylint - unparam - unused - whitespace -issues: - exclude-rules: - - path: internal/annotation/load_balancer.go - linters: - - godot - - path: (_test\.go|testing\.go|testsupport|e2etests) - linters: - - gosec - - errcheck - - path: internal/mocks - linters: - - unparam - - revive + settings: + exhaustive: + default-signifies-exhaustive: true + + staticcheck: + checks: ["all", "-QF1008"] + + gomoddirectives: + replace-allow-list: + - github.com/hetznercloud/hcloud-go/v2 + - k8s.io/cloud-provider + - k8s.io/controller-manager + + importas: + alias: + - pkg: k8s.io/api/core/v1 + alias: corev1 + - pkg: k8s.io/apimachinery/pkg/apis/meta/v1 + alias: metav1 + - pkg: github.com/syself/hrobot-go + alias: hrobot + - pkg: github.com/syself/hrobot-go/models + alias: hrobotmodels + no-unaliased: true + + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - std-error-handling + rules: + - linters: + - testifylint + text: require-error + - linters: + - godot + path: internal/annotation/load_balancer.go + - linters: + - errcheck + - gosec + path: (_test\.go|testing\.go|testsupport|e2etests) + - linters: + - revive + - unparam + path: internal/mocks + - linters: + - revive + path: internal/utils/.+\.go + text: var-naming + +formatters: + enable: + - gci + - goimports + + settings: + gci: + sections: + - standard + - default + - prefix(github.com/hetznercloud) + + exclusions: + generated: lax diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3952fe90b..31230dcb2 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -52,10 +52,9 @@ repos: - id: shellcheck - repo: https://github.com/golangci/golangci-lint - rev: v1.64.8 + rev: v2.5.0 hooks: - id: golangci-lint-full - args: [--timeout=5m] language_version: 1.25.1 # renovate: datasource=golang-version depName=go - repo: local diff --git a/hcloud/cloud.go b/hcloud/cloud.go index b90231282..153b65fe8 100644 --- a/hcloud/cloud.go +++ b/hcloud/cloud.go @@ -224,7 +224,7 @@ func serverIsAttachedToNetwork(metadataClient *metadata.Client, networkID int64) serverPrivateNetworks, err := metadataClient.PrivateNetworks() if err != nil { - return false, fmt.Errorf("%s: %s", op, err) + return false, fmt.Errorf("%s: %w", op, err) } return strings.Contains(serverPrivateNetworks, fmt.Sprintf("network_id: %d\n", networkID)), nil } diff --git a/hcloud/load_balancers.go b/hcloud/load_balancers.go index 2c113d0bc..dd405b605 100644 --- a/hcloud/load_balancers.go +++ b/hcloud/load_balancers.go @@ -78,7 +78,7 @@ func (l *loadBalancers) GetLoadBalancer( if errors.Is(err, hcops.ErrNotFound) { return nil, false, nil } - return nil, false, fmt.Errorf("%s: %v", op, err) + return nil, false, fmt.Errorf("%s: %w", op, err) } if v, ok := annotation.LBHostname.StringFromService(service); ok { @@ -89,7 +89,7 @@ func (l *loadBalancers) GetLoadBalancer( ingress, err := l.buildLoadBalancerStatusIngress(lb, service) if err != nil { - return nil, false, fmt.Errorf("%s: %v", op, err) + return nil, false, fmt.Errorf("%s: %w", op, err) } return &corev1.LoadBalancerStatus{Ingress: ingress}, true, nil @@ -128,7 +128,7 @@ func (l *loadBalancers) EnsureLoadBalancer( lb, err = l.lbOps.GetByK8SServiceUID(ctx, svc) if err != nil && !errors.Is(err, hcops.ErrNotFound) { - return nil, fmt.Errorf("%s: %v", op, err) + return nil, fmt.Errorf("%s: %w", op, err) } // Try the load balancer's name if we were not able to find it using the @@ -143,7 +143,7 @@ func (l *loadBalancers) EnsureLoadBalancer( if errors.Is(err, hcops.ErrNotFound) { lb, err = l.lbOps.GetByName(ctx, lbName) if err != nil && !errors.Is(err, hcops.ErrNotFound) { - return nil, fmt.Errorf("%s: %v", op, err) + return nil, fmt.Errorf("%s: %w", op, err) } } diff --git a/hcloud/routes.go b/hcloud/routes.go index cd0b0bef2..f5fab0fc6 100644 --- a/hcloud/routes.go +++ b/hcloud/routes.go @@ -90,7 +90,7 @@ func (r *routes) ListRoutes(ctx context.Context, _ string) ([]*cloudprovider.Rou routes := make([]*cloudprovider.Route, 0, len(r.network.Routes)) for _, route := range r.network.Routes { - ro, err := r.hcloudRouteToRoute(route) + ro, err := r.hcloudRouteToRoute(ctx, route) if err != nil { return routes, fmt.Errorf("%s: %w", op, err) } @@ -106,17 +106,17 @@ func (r *routes) CreateRoute(ctx context.Context, clusterName string, nameHint s const op = "hcloud/CreateRoute" metrics.OperationCalled.WithLabelValues(op).Inc() - srv, err := r.serverCache.ByName(string(route.TargetNode)) + srv, err := r.serverCache.ByName(ctx, string(route.TargetNode)) if err != nil { - return fmt.Errorf("%s: %v", op, err) + return fmt.Errorf("%s: %w", op, err) } privNet, ok := findServerPrivateNetByID(srv, r.network.ID) if !ok { r.serverCache.InvalidateCache() - srv, err = r.serverCache.ByName(string(route.TargetNode)) + srv, err = r.serverCache.ByName(ctx, string(route.TargetNode)) if err != nil { - return fmt.Errorf("%s: %v", op, err) + return fmt.Errorf("%s: %w", op, err) } privNet, ok = findServerPrivateNetByID(srv, r.network.ID) @@ -134,7 +134,7 @@ func (r *routes) CreateRoute(ctx context.Context, clusterName string, nameHint s clusterNetSize, _ := r.clusterCIDR.Mask.Size() destNetSize, _ := cidr.Mask.Size() - if !(r.clusterCIDR.Contains(cidr.IP) && destNetSize >= clusterNetSize) { + if !r.clusterCIDR.Contains(cidr.IP) || destNetSize < clusterNetSize { node := &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: string(route.TargetNode), @@ -250,7 +250,7 @@ func (r *routes) deleteRouteFromHcloud(ctx context.Context, cidr *net.IPNet, ip return nil } -func (r *routes) hcloudRouteToRoute(route hcloud.NetworkRoute) (*cloudprovider.Route, error) { +func (r *routes) hcloudRouteToRoute(ctx context.Context, route hcloud.NetworkRoute) (*cloudprovider.Route, error) { const op = "hcloud/hcloudRouteToRoute" metrics.OperationCalled.WithLabelValues(op).Inc() @@ -259,7 +259,7 @@ func (r *routes) hcloudRouteToRoute(route hcloud.NetworkRoute) (*cloudprovider.R Name: fmt.Sprintf("%s-%s", route.Gateway.String(), route.Destination.String()), } - srv, err := r.serverCache.ByPrivateIP(route.Gateway) + srv, err := r.serverCache.ByPrivateIP(ctx, route.Gateway) if err != nil { if errors.Is(err, hcops.ErrNotFound) { // Route belongs to non-existing target @@ -284,9 +284,9 @@ func (r *routes) checkIfRouteAlreadyExists(ctx context.Context, route *cloudprov for _, _route := range r.network.Routes { if _route.Destination.String() == route.DestinationCIDR { - srv, err := r.serverCache.ByName(string(route.TargetNode)) + srv, err := r.serverCache.ByName(ctx, string(route.TargetNode)) if err != nil { - return false, fmt.Errorf("%s: %v", op, err) + return false, fmt.Errorf("%s: %w", op, err) } privNet, ok := findServerPrivateNetByID(srv, r.network.ID) if !ok { @@ -295,7 +295,7 @@ func (r *routes) checkIfRouteAlreadyExists(ctx context.Context, route *cloudprov ip := privNet.IP if !_route.Gateway.Equal(ip) { - action, _, err := r.client.Network.DeleteRoute(context.Background(), r.network, hcloud.NetworkDeleteRouteOpts{ + action, _, err := r.client.Network.DeleteRoute(ctx, r.network, hcloud.NetworkDeleteRouteOpts{ Route: _route, }) if err != nil { diff --git a/internal/config/config.go b/internal/config/config.go index f8d937e19..801296770 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -259,7 +259,7 @@ func getEnvBool(key string, defaultValue bool) (bool, error) { b, err := strconv.ParseBool(v) if err != nil { - return false, fmt.Errorf("failed to parse %s: %v", key, err) + return false, fmt.Errorf("failed to parse %s: %w", key, err) } return b, nil @@ -275,7 +275,7 @@ func getEnvDuration(key string) (time.Duration, error) { b, err := time.ParseDuration(v) if err != nil { - return 0, fmt.Errorf("failed to parse %s: %v", key, err) + return 0, fmt.Errorf("failed to parse %s: %w", key, err) } return b, nil diff --git a/internal/hcops/certificates.go b/internal/hcops/certificates.go index 83311c108..957916787 100644 --- a/internal/hcops/certificates.go +++ b/internal/hcops/certificates.go @@ -53,7 +53,7 @@ func (co *CertificateOps) GetCertificateByLabel(ctx context.Context, label strin opts := hcloud.CertificateListOpts{ListOpts: hcloud.ListOpts{LabelSelector: label}} certs, err := co.CertClient.AllWithOpts(ctx, opts) if err != nil { - return nil, fmt.Errorf("%s: %v", op, err) + return nil, fmt.Errorf("%s: %w", op, err) } if len(certs) == 0 { return nil, fmt.Errorf("%s: %w", op, ErrNotFound) @@ -86,7 +86,7 @@ func (co *CertificateOps) CreateManagedCertificate( return fmt.Errorf("%s: %w", op, ErrAlreadyExists) } if err != nil { - return fmt.Errorf("%s: %v", op, err) + return fmt.Errorf("%s: %w", op, err) } return nil } diff --git a/internal/hcops/load_balancer.go b/internal/hcops/load_balancer.go index 775342441..39b2b453b 100644 --- a/internal/hcops/load_balancer.go +++ b/internal/hcops/load_balancer.go @@ -99,7 +99,7 @@ func (l *LoadBalancerOps) GetByK8SServiceUID(ctx context.Context, svc *corev1.Se } lbs, err := l.LBClient.AllWithOpts(ctx, opts) if err != nil { - return nil, fmt.Errorf("%s: api error: %v", op, err) + return nil, fmt.Errorf("%s: api error: %w", op, err) } if len(lbs) == 0 { return nil, fmt.Errorf("%s: %w", op, ErrNotFound) @@ -236,7 +236,7 @@ func (l *LoadBalancerOps) Delete(ctx context.Context, lb *hcloud.LoadBalancer) e return nil } if err != nil { - return fmt.Errorf("%s: %v", op, err) + return fmt.Errorf("%s: %w", op, err) } return nil } @@ -885,7 +885,7 @@ func (l *LoadBalancerOps) ReconcileHCLBServices( var changed bool if err := l.reconcileManagedCertificate(ctx, svc); err != nil { - return false, fmt.Errorf("%s: %v", op, err) + return false, fmt.Errorf("%s: %w", op, err) } hclbListenPorts := make(map[int]bool, len(lb.Services)) @@ -1001,7 +1001,7 @@ func (l *LoadBalancerOps) reconcileManagedCertificate(ctx context.Context, svc * return nil } if err != nil { - return fmt.Errorf("%s: %v", op, err) + return fmt.Errorf("%s: %w", op, err) } return nil } diff --git a/internal/hcops/server.go b/internal/hcops/server.go index 2a712289d..7872ed739 100644 --- a/internal/hcops/server.go +++ b/internal/hcops/server.go @@ -42,11 +42,11 @@ type AllServersCache struct { // Note that a pointer to the object stored in the cache is returned. Modifying // this object affects the cache and all other code parts holding a reference. // Furthermore, modifying the returned server is not concurrency safe. -func (c *AllServersCache) ByPrivateIP(ip net.IP) (*hcloud.Server, error) { +func (c *AllServersCache) ByPrivateIP(ctx context.Context, ip net.IP) (*hcloud.Server, error) { const op = "hcops/AllServersCache.ByPrivateIP" metrics.OperationCalled.WithLabelValues(op).Inc() - srv, err := c.getFromCache(func() (*hcloud.Server, bool) { + srv, err := c.getFromCache(ctx, func() (*hcloud.Server, bool) { srv, ok := c.byPrivIP[ip.String()] return srv, ok }) @@ -62,11 +62,11 @@ func (c *AllServersCache) ByPrivateIP(ip net.IP) (*hcloud.Server, error) { // Note that a pointer to the object stored in the cache is returned. Modifying // this object affects the cache and all other code parts holding a reference. // Furthermore, modifying the returned server is not concurrency safe. -func (c *AllServersCache) ByName(name string) (*hcloud.Server, error) { +func (c *AllServersCache) ByName(ctx context.Context, name string) (*hcloud.Server, error) { const op = "hcops/AllServersCache.ByName" metrics.OperationCalled.WithLabelValues(op).Inc() - srv, err := c.getFromCache(func() (*hcloud.Server, bool) { + srv, err := c.getFromCache(ctx, func() (*hcloud.Server, bool) { srv, ok := c.byName[name] return srv, ok }) @@ -78,7 +78,7 @@ func (c *AllServersCache) ByName(name string) (*hcloud.Server, error) { } // getFromCache wraps the cache maps with expiry time and "get-on-unavailable" functionality. -func (c *AllServersCache) getFromCache(retrieveFromCacheMaps func() (*hcloud.Server, bool)) (*hcloud.Server, error) { +func (c *AllServersCache) getFromCache(ctx context.Context, retrieveFromCacheMaps func() (*hcloud.Server, bool)) (*hcloud.Server, error) { const op = "hcops/AllServersCache.getCache" metrics.OperationCalled.WithLabelValues(op).Inc() @@ -89,7 +89,7 @@ func (c *AllServersCache) getFromCache(retrieveFromCacheMaps func() (*hcloud.Ser // Refresh the cache if its expired if c.isExpired() { - if err := c.refreshCache(); err != nil { + if err := c.refreshCache(ctx); err != nil { return nil, fmt.Errorf("%s: %w", op, err) } cacheRefreshed = true @@ -102,7 +102,7 @@ func (c *AllServersCache) getFromCache(retrieveFromCacheMaps func() (*hcloud.Ser // If the server was not in the cache, we want to refresh if we did not already in this call and if there is available limit. if !cacheRefreshed && c.CacheMissRefreshLimiter.Allow() { - if err := c.refreshCache(); err != nil { + if err := c.refreshCache(ctx); err != nil { return nil, fmt.Errorf("%s: %w", op, err) } @@ -117,7 +117,7 @@ func (c *AllServersCache) getFromCache(retrieveFromCacheMaps func() (*hcloud.Ser } // Caller must hold the mutex. -func (c *AllServersCache) refreshCache() error { +func (c *AllServersCache) refreshCache(ctx context.Context) error { const op = "hcops/AllServersCache.refreshCache" metrics.OperationCalled.WithLabelValues(op).Inc() @@ -125,7 +125,7 @@ func (c *AllServersCache) refreshCache() error { if to == 0 { to = 20 * time.Second } - ctx, cancel := context.WithTimeout(context.Background(), to) + ctx, cancel := context.WithTimeout(ctx, to) defer cancel() servers, err := c.LoadFunc(ctx) diff --git a/internal/hcops/server_test.go b/internal/hcops/server_test.go index fc4382406..142ddceb8 100644 --- a/internal/hcops/server_test.go +++ b/internal/hcops/server_test.go @@ -58,7 +58,7 @@ func TestAllServersCache_CacheHit(t *testing.T) { Once() // Perform any cache op to initialize caches - if _, err := tt.Cache.ByName(srv.Name); err != nil { + if _, err := tt.Cache.ByName(t.Context(), srv.Name); err != nil { t.Fatalf("SetUp: %v", err) } }, @@ -92,7 +92,7 @@ func TestAllServersCache_InvalidateCache(t *testing.T) { Times(2) // Perform any cache op to initialize caches - if _, err := tt.Cache.ByName(srv.Name); err != nil { + if _, err := tt.Cache.ByName(t.Context(), srv.Name); err != nil { t.Fatalf("SetUp: %v", err) } @@ -100,7 +100,7 @@ func TestAllServersCache_InvalidateCache(t *testing.T) { tt.Cache.InvalidateCache() // Perform a second cache lookup - if _, err := tt.Cache.ByName(srv.Name); err != nil { + if _, err := tt.Cache.ByName(t.Context(), srv.Name); err != nil { t.Fatalf("SetUp: %v", err) } }, @@ -132,7 +132,7 @@ func TestAllServersCache_CacheRefresh(t *testing.T) { On("All", mock.Anything). Return([]*hcloud.Server{srv}, nil) - if _, err := tt.Cache.ByName(srv.Name); err != nil { + if _, err := tt.Cache.ByName(t.Context(), srv.Name); err != nil { t.Fatalf("SetUp: %v", err) } // Set the maximum cache age to a ridiculously low time to @@ -173,7 +173,7 @@ func TestAllServersCache_CacheMissRefresh(t *testing.T) { Return([]*hcloud.Server{srv}, nil).Once() // Setup initial cache - result, err := tt.Cache.ByName(srv.Name) + result, err := tt.Cache.ByName(t.Context(), srv.Name) assert.Error(t, err) assert.Nil(t, result) }, @@ -205,11 +205,11 @@ func TestAllServersCache_CacheRefreshLimited(t *testing.T) { On("All", mock.Anything). Return([]*hcloud.Server{}, nil) - result, err := tt.Cache.ByName(srv.Name) + result, err := tt.Cache.ByName(t.Context(), srv.Name) assert.Error(t, err) assert.Nil(t, result) - result, err = tt.Cache.ByName(srv.Name) + result, err = tt.Cache.ByName(t.Context(), srv.Name) assert.Error(t, err) assert.Nil(t, result) }, @@ -292,7 +292,7 @@ func TestAllServersCache_CacheMissRefreshClientError(t *testing.T) { Return([]*hcloud.Server{}, nil).Once() // Load the cache once - result, err := tt.Cache.ByName(srv.Name) + result, err := tt.Cache.ByName(t.Context(), srv.Name) assert.Error(t, err) assert.Nil(t, result) @@ -367,13 +367,13 @@ func newAllServersCacheOps(t *testing.T, srv *hcloud.Server) map[string]allServe if ip == nil { t.Fatal("ByPrivateIP: server has no private ip") } - return c.ByPrivateIP(ip) + return c.ByPrivateIP(t.Context(), ip) }, "ByName": func(c *hcops.AllServersCache) (*hcloud.Server, error) { if srv.Name == "" { t.Fatal("ByName: server has no name") } - return c.ByName(srv.Name) + return c.ByName(t.Context(), srv.Name) }, } } @@ -405,7 +405,7 @@ func (tt *allServersCacheTestCase) run(t *testing.T) { return } if tt.ExpectedErr != nil { - assert.Truef(t, errors.Is(err, tt.ExpectedErr), "expected error: %v; got %v", tt.ExpectedErr, err) + assert.ErrorIsf(t, err, tt.ExpectedErr, "expected error: %v; got %v", tt.ExpectedErr, err) return } assert.Equal(t, tt.Expected, actual) diff --git a/internal/robot/ratelimit_test.go b/internal/robot/ratelimit_test.go index aac711c36..c041a26ba 100644 --- a/internal/robot/ratelimit_test.go +++ b/internal/robot/ratelimit_test.go @@ -19,7 +19,7 @@ func TestRateLimit(t *testing.T) { servers, err := client.ServerGetList() assert.NoError(t, err) - assert.Len(t, servers, 0) + assert.Empty(t, servers) mock.AssertNumberOfCalls(t, "ServerGetList", 1) mock.On("ServerGetList").Return(nil, hrobotmodels.Error{Code: hrobotmodels.ErrorCodeRateLimitExceeded, Message: "Rate limit exceeded"}).Once()