diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 964ab35e..ce76b68e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,7 +42,7 @@ jobs: with: go-version: "1.24" - name: golangci-lint - uses: golangci/golangci-lint-action@4696ba8babb6127d732c3c6dde519db15edab9ea # v6.5.1 + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8.0.0 with: - version: v1.64.7 + version: v2.5.0 skip-cache: true diff --git a/.golangci.yaml b/.golangci.yaml index 25d7acf7..4d49440d 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,313 +1,186 @@ -# This code is licensed under the terms of the MIT license https://opensource.org/license/mit -# Copyright (c) 2021 Marat Reymers - -## Golden config for golangci-lint v1.56.2 -# -# This is the best config for golangci-lint based on my experience and opinion. -# It is very strict, but not extremely strict. -# Feel free to adapt and change it for your needs. - -run: - # Timeout for analysis, e.g. 30s, 5m. - # Default: 1m - timeout: 3m - -# This file contains only configs which differ from defaults. -# All possible options can be found here https://github.com/golangci/golangci-lint/blob/master/.golangci.reference.yml -linters-settings: - cyclop: - # The maximal code complexity to report. - # Default: 10 - max-complexity: 30 - # The maximal average package complexity. - # If it's higher than 0.0 (float) the check is enabled - # Default: 0.0 - package-average: 10.0 - - errcheck: - # Report about not checking of errors in type assertions: `a := b.(MyStruct)`. - # Such cases aren't reported by default. - # Default: false - check-type-assertions: true - - exhaustive: - # Program elements to check for exhaustiveness. - # Default: [ switch ] - check: - - switch - - map - - exhaustruct: - # List of regular expressions to exclude struct packages and their names from checks. - # Regular expressions must match complete canonical struct package/name/structname. - # Default: [] - exclude: - # std libs - - "^net/http.Client$" - - "^net/http.Cookie$" - - "^net/http.Request$" - - "^net/http.Response$" - - "^net/http.Server$" - - "^net/http.Transport$" - - "^net/url.URL$" - - "^os/exec.Cmd$" - - "^reflect.StructField$" - # public libs - - "^github.com/Shopify/sarama.Config$" - - "^github.com/Shopify/sarama.ProducerMessage$" - - "^github.com/mitchellh/mapstructure.DecoderConfig$" - - "^github.com/prometheus/client_golang/.+Opts$" - - "^github.com/spf13/cobra.Command$" - - "^github.com/spf13/cobra.CompletionOptions$" - - "^github.com/stretchr/testify/mock.Mock$" - - "^github.com/testcontainers/testcontainers-go.+Request$" - - "^github.com/testcontainers/testcontainers-go.FromDockerfile$" - - "^golang.org/x/tools/go/analysis.Analyzer$" - - "^google.golang.org/protobuf/.+Options$" - - "^gopkg.in/yaml.v3.Node$" - - funlen: - # Checks the number of lines in a function. - # If lower than 0, disable the check. - # Default: 60 - lines: 100 - # Checks the number of statements in a function. - # If lower than 0, disable the check. - # Default: 40 - statements: 50 - # Ignore comments when counting lines. - # Default false - ignore-comments: true - - gocognit: - # Minimal code complexity to report. - # Default: 30 (but we recommend 10-20) - min-complexity: 20 - - gocritic: - # Settings passed to gocritic. - # The settings key is the name of a supported gocritic checker. - # The list of supported checkers can be find in https://go-critic.github.io/overview. - settings: - captLocal: - # Whether to restrict checker to params only. - # Default: true - paramsOnly: false - underef: - # Whether to skip (*x).method() calls where x is a pointer receiver. - # Default: true - skipRecvDeref: false - - mnd: - # List of function patterns to exclude from analysis. - # Values always ignored: `time.Date`, - # `strconv.FormatInt`, `strconv.FormatUint`, `strconv.FormatFloat`, - # `strconv.ParseInt`, `strconv.ParseUint`, `strconv.ParseFloat`. - # Default: [] - ignored-functions: - - flag.Arg - - flag.Duration.* - - flag.Float.* - - flag.Int.* - - flag.Uint.* - - os.Chmod - - os.Mkdir.* - - os.OpenFile - - os.WriteFile - - prometheus.ExponentialBuckets.* - - prometheus.LinearBuckets - - math.Min - - govet: - # Enable all analyzers. - # Default: false - enable-all: true - # Disable analyzers by name. - # Run `go tool vet help` to see all analyzers. - # Default: [] - disable: - - fieldalignment # too strict - - shadow - # Settings per analyzer. - - inamedparam: - # Skips check for interface methods with only a single parameter. - # Default: false - skip-single-param: true - - nakedret: - # Make an issue if func has more lines of code than this setting, and it has naked returns. - # Default: 30 - max-func-lines: 0 - - nolintlint: - # Exclude following linters from requiring an explanation. - # Default: [] - allow-no-explanation: [funlen, gocognit, lll] - # Enable to require an explanation of nonzero length after each nolint directive. - # Default: false - require-explanation: true - # Enable to require nolint directives to mention the specific linter being suppressed. - # Default: false - require-specific: true - - rowserrcheck: - # database/sql is always checked - # Default: [] - packages: - - github.com/jmoiron/sqlx - - tenv: - # The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures. - # Otherwise, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked. - # Default: false - all: true - - wrapcheck: - ignoreSigs: - - ".Complete(" - - "client.IgnoreNotFound(" - - "fmt.Errorf(" - - nestif: - min-complexity: 8 - +version: "2" linters: - disable-all: true + default: none enable: - ## enabled by default - - errcheck # checking for unchecked errors, these unchecked errors can be critical bugs in some cases - - gosimple # specializes in simplifying a code - - govet # reports suspicious constructs, such as Printf calls whose arguments do not align with the format string - - ineffassign # detects when assignments to existing variables are not used - - staticcheck # is a go vet on steroids, applying a ton of static analysis checks - - typecheck # like the front-end of a Go compiler, parses and type-checks Go code - - unused # checks for unused constants, variables, functions and types - ## disabled by default - - asasalint # checks for pass []any as any in variadic func(...any) - - asciicheck # checks that your code does not contain non-ASCII identifiers - - bidichk # checks for dangerous unicode character sequences - - copyloopvar # Copyloopvar is a linter detects places where loop variables are copied. - - cyclop # checks function and package cyclomatic complexity - - dupl # tool for code clone detection - - errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error - - errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13 - - forbidigo # forbids identifiers - - funlen # tool for detection of long functions - - gocognit # computes and checks the cognitive complexity of functions - - goconst # finds repeated strings that could be replaced by a constant - - gocritic # provides diagnostics that check for bugs, performance and style issues - - gocyclo # computes and checks the cyclomatic complexity of functions - - goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt - - mnd # detects magic numbers - - goprintffuncname # checks that printf-like functions are named with f at the end - - gosec # inspects source code for security problems - - makezero # finds slice declarations with non-zero initial length - - nakedret # finds naked returns in functions greater than a specified function length - - nestif # reports deeply nested if statements - - nilerr # finds the code that returns nil even if it checks that the error is not nil - - nilnil # checks that there is no simultaneous return of nil error and an invalid value - - noctx # finds sending http request without context.Context - - nolintlint # reports ill-formed or insufficient nolint directives - - predeclared # finds code that shadows one of Go's predeclared identifiers - - promlinter # checks Prometheus metrics naming via promlint - - reassign # checks that package variables are not reassigned - - revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint - - sloglint # ensure consistent code style when using log/slog - - tenv # detects using os.Setenv instead of t.Setenv since Go1.17 - - testableexamples # checks if examples are testable (have an expected output) - - testifylint # checks usage of github.com/stretchr/testify - - testpackage # makes you use a separate _test package - - tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes - - unconvert # removes unnecessary type conversions - - unparam # reports unused function parameters - - usestdlibvars # detects the possibility to use variables/constants from the Go standard library - - wastedassign # finds wasted assignment statements - - whitespace # detects leading and trailing whitespace - - ## you may want to enable - #- exhaustive # checks exhaustiveness of enum switch statements - #- exhaustruct # checks if all structure fields are initialized - #- decorder # checks declaration order and count of types, constants, variables and functions - #- gci # controls golang package import order and makes it always deterministic - #- ginkgolinter # [if you use ginkgo/gomega] enforces standards of using ginkgo and gomega - #- godox # detects FIXME, TODO and other comment keywords - #- goheader # checks is file header matches to pattern - #- inamedparam # [great idea, but too strict, need to ignore a lot of cases by default] reports interfaces with unnamed method parameters - #- interfacebloat # checks the number of methods inside an interface - #- ireturn # accept interfaces, return concrete types - #- prealloc # [premature optimization, but can be used in some cases] finds slice declarations that could potentially be preallocated - #- tagalign # checks that struct tags are well aligned - #- varnamelen # [great idea, but too many false positives] checks that the length of a variable's name matches its scope - #- wrapcheck # checks that errors returned from external packages are wrapped - #- zerologlint # detects the wrong usage of zerolog that a user forgets to dispatch zerolog.Event - - ## disabled - #- godot # checks if comments end in a period - #- containedctx # detects struct contained context.Context field - #- contextcheck # [too many false positives] checks the function whether use a non-inherited context - #- depguard # [replaced by gomodguard] checks if package imports are in a list of acceptable packages - #- dogsled # checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) - #- dupword # [useless without config] checks for duplicate words in the source code - #- errchkjson # [don't see profit + I'm against of omitting errors like in the first example https://github.com/breml/errchkjson] checks types passed to the json encoding functions. Reports unsupported types and optionally reports occasions, where the check for the returned error can be omitted - #- forcetypeassert # [replaced by errcheck] finds forced type assertions - #- goerr113 # [too strict] checks the errors handling expressions - #- gofmt # [replaced by goimports] checks whether code was gofmt-ed - #- gofumpt # [replaced by goimports, gofumports is not available yet] checks whether code was gofumpt-ed - #- gosmopolitan # reports certain i18n/l10n anti-patterns in your Go codebase - #- grouper # analyzes expression groups - #- importas # enforces consistent import aliases - #- maintidx # measures the maintainability index of each function - #- misspell # [useless] finds commonly misspelled English words in comments - #- nlreturn # [too strict and mostly code is not more readable] checks for a new line before return and branch statements to increase code clarity - #- paralleltest # [too many false positives] detects missing usage of t.Parallel() method in your Go test - #- tagliatelle # checks the struct tags - #- thelper # detects golang test helpers without t.Helper() call and checks the consistency of test helpers - #- wsl # [too strict and mostly code is not more readable] whitespace linter forces you to use empty lines - - ## deprecated - #- deadcode # [deprecated, replaced by unused] finds unused code - #- exhaustivestruct # [deprecated, replaced by exhaustruct] checks if all struct's fields are initialized - #- golint # [deprecated, replaced by revive] golint differs from gofmt. Gofmt reformats Go source code, whereas golint prints out style mistakes - #- ifshort # [deprecated] checks that your code uses short syntax for if-statements whenever possible - #- interfacer # [deprecated] suggests narrower interface types - #- maligned # [deprecated, replaced by govet fieldalignment] detects Go structs that would take less memory if their fields were sorted - #- nosnakecase # [deprecated, replaced by revive var-naming] detects snake case of variable naming and function name - #- scopelint # [deprecated, replaced by exportloopref] checks for unpinned variables in go programs - #- structcheck # [deprecated, replaced by unused] finds unused struct fields - #- varcheck # [deprecated, replaced by unused] finds unused global variables and constants - + - asasalint + - asciicheck + - bidichk + - copyloopvar + - cyclop + - dupl + - errcheck + - errname + - errorlint + - forbidigo + - funlen + - gocognit + - goconst + - gocritic + - gocyclo + - goprintffuncname + - gosec + - govet + - ineffassign + - makezero + - mnd + - nakedret + - nestif + - nilerr + - nilnil + - noctx + - nolintlint + - predeclared + - promlinter + - reassign + - revive + - sloglint + - staticcheck + - testableexamples + - testifylint + - testpackage + - tparallel + - unconvert + - unparam + - unused + - usestdlibvars + - usetesting + - wastedassign + - whitespace + settings: + cyclop: + max-complexity: 30 + package-average: 10 + errcheck: + check-type-assertions: true + exhaustive: + check: + - switch + - map + exhaustruct: + exclude: + - ^net/http.Client$ + - ^net/http.Cookie$ + - ^net/http.Request$ + - ^net/http.Response$ + - ^net/http.Server$ + - ^net/http.Transport$ + - ^net/url.URL$ + - ^os/exec.Cmd$ + - ^reflect.StructField$ + - ^github.com/Shopify/sarama.Config$ + - ^github.com/Shopify/sarama.ProducerMessage$ + - ^github.com/mitchellh/mapstructure.DecoderConfig$ + - ^github.com/prometheus/client_golang/.+Opts$ + - ^github.com/spf13/cobra.Command$ + - ^github.com/spf13/cobra.CompletionOptions$ + - ^github.com/stretchr/testify/mock.Mock$ + - ^github.com/testcontainers/testcontainers-go.+Request$ + - ^github.com/testcontainers/testcontainers-go.FromDockerfile$ + - ^golang.org/x/tools/go/analysis.Analyzer$ + - ^google.golang.org/protobuf/.+Options$ + - ^gopkg.in/yaml.v3.Node$ + funlen: + lines: 100 + statements: 50 + ignore-comments: true + gocognit: + min-complexity: 20 + gocritic: + settings: + captLocal: + paramsOnly: false + underef: + skipRecvDeref: false + govet: + disable: + - fieldalignment + - shadow + enable-all: true + inamedparam: + skip-single-param: true + mnd: + ignored-functions: + - flag.Arg + - flag.Duration.* + - flag.Float.* + - flag.Int.* + - flag.Uint.* + - os.Chmod + - os.Mkdir.* + - os.OpenFile + - os.WriteFile + - prometheus.ExponentialBuckets.* + - prometheus.LinearBuckets + - math.Min + nakedret: + max-func-lines: 0 + nestif: + min-complexity: 8 + nolintlint: + require-explanation: true + require-specific: true + allow-no-explanation: + - funlen + - gocognit + - lll + rowserrcheck: + packages: + - github.com/jmoiron/sqlx + wrapcheck: + ignore-sigs: + - .Complete( + - client.IgnoreNotFound( + - fmt.Errorf( + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling + rules: + - linters: + - godot + source: (noinspection|TODO) + - linters: + - gocritic + source: //noinspection + - linters: + - bodyclose + - dupl + - funlen + - goconst + - gosec + - noctx + - wrapcheck + path: _test\.go + - linters: + - funlen + source: ^func Test + - linters: + - revive + path: (.+)_test\.go + text: 'dot-imports: should not use dot imports' + - linters: + - unparam + path: internal/controller/shim_controller.go + source: ctrl.Result + - linters: + - revive + path: pkg/containerd/restart_unix.go + text: 'exported: type name will be used as containerd.ContainerdRestarter by other packages, and that stutters; consider calling this Restarter' + paths: + - third_party$ + - builtin$ + - examples$ issues: max-same-issues: 30 - - exclude-rules: - - source: "(noinspection|TODO)" - linters: [godot] - - source: "//noinspection" - linters: [gocritic] - - path: "_test\\.go" - linters: - - bodyclose - - dupl - - funlen - - goconst - - gosec - - noctx - - wrapcheck - - linters: - - funlen - # Disable 'funlen' linter for test functions. - # It's common for table-driven tests to be more than 60 characters long - source: "^func Test" - - path: '(.+)_test\.go' - linters: - - revive - text: "dot-imports: should not use dot imports" - - path: internal/controller/shim_controller.go - linters: - - unparam - source: "ctrl.Result" - - path: pkg/containerd/restart_unix.go - linters: - - revive - text: "exported: type name will be used as containerd.ContainerdRestarter by other packages, and that stutters; consider calling this Restarter" +formatters: + enable: + - goimports + exclusions: + generated: lax + paths: + - third_party$ + - builtin$ + - examples$ diff --git a/Makefile b/Makefile index b8ed3085..680c4161 100644 --- a/Makefile +++ b/Makefile @@ -78,7 +78,7 @@ test: manifests generate fmt vet envtest ## Run tests. KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test ./... -coverprofile cover.out GOLANGCI_LINT = $(shell pwd)/bin/golangci-lint -GOLANGCI_LINT_VERSION ?= v1.57.2 +GOLANGCI_LINT_VERSION ?= v2.5.0 golangci-lint: @[ -f $(GOLANGCI_LINT) ] || { \ set -e ;\ diff --git a/internal/containerd/restart_unix.go b/internal/containerd/restart_unix.go index ed441d45..ced9371f 100644 --- a/internal/containerd/restart_unix.go +++ b/internal/containerd/restart_unix.go @@ -20,6 +20,7 @@ package containerd import ( + "context" "fmt" "log/slog" "os" @@ -150,7 +151,7 @@ func ListSystemdUnits() ([]byte, error) { } func nsenterCmd(cmd ...string) *exec.Cmd { - return exec.Command("nsenter", + return exec.CommandContext(context.Background(), "nsenter", append([]string{fmt.Sprintf("-m/%s/proc/1/ns/mnt", os.Getenv("HOST_ROOT")), "--"}, cmd...)...) // #nosec G204 } diff --git a/internal/controller/job_controller.go b/internal/controller/job_controller.go index a684b3b2..6d2ca4a2 100644 --- a/internal/controller/job_controller.go +++ b/internal/controller/job_controller.go @@ -149,7 +149,7 @@ func (jr *JobReconciler) deleteNodeLabel(ctx context.Context, node *corev1.Node, func (jr *JobReconciler) getNode(ctx context.Context, nodeName string) (*corev1.Node, error) { node := corev1.Node{} - if err := jr.Client.Get(ctx, types.NamespacedName{Name: nodeName}, &node); err != nil { + if err := jr.Get(ctx, types.NamespacedName{Name: nodeName}, &node); err != nil { log.Err(err).Msg("Unable to fetch node") return &corev1.Node{}, client.IgnoreNotFound(err) } diff --git a/internal/controller/shim_controller.go b/internal/controller/shim_controller.go index 83bf520f..43724aca 100644 --- a/internal/controller/shim_controller.go +++ b/internal/controller/shim_controller.go @@ -106,7 +106,7 @@ func (sr *ShimReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl // 1. Check if the shim resource exists var shimResource rcmv1.Shim - if err := sr.Client.Get(ctx, req.NamespacedName, &shimResource); err != nil { + if err := sr.Get(ctx, req.NamespacedName, &shimResource); err != nil { log.Err(err).Msg("Unable to fetch shimResource") return ctrl.Result{}, client.IgnoreNotFound(err) } @@ -214,7 +214,7 @@ func (sr *ShimReconciler) updateStatus(ctx context.Context, shim *rcmv1.Shim, no } // Re-fetch shim to avoid "object has been modified" errors - if err := sr.Client.Get(ctx, types.NamespacedName{Name: shim.Name, Namespace: shim.Namespace}, shim); err != nil { + if err := sr.Get(ctx, types.NamespacedName{Name: shim.Name, Namespace: shim.Namespace}, shim); err != nil { log.Error().Msgf("Unable to re-fetch shim: %s", err) return fmt.Errorf("failed to fetch shim: %w", err) } @@ -269,7 +269,7 @@ func (sr *ShimReconciler) recreateStrategyRollout(ctx context.Context, shim *rcm func (sr *ShimReconciler) deployJobOnNode(ctx context.Context, shim *rcmv1.Shim, node corev1.Node, jobType string) error { log := log.Ctx(ctx) - if err := sr.Client.Get(ctx, types.NamespacedName{Name: node.Name}, &node); err != nil { + if err := sr.Get(ctx, types.NamespacedName{Name: node.Name}, &node); err != nil { log.Error().Msgf("Unable to re-fetch node: %s", err) return fmt.Errorf("failed to fetch node: %w", err) } @@ -311,7 +311,7 @@ func (sr *ShimReconciler) deployJobOnNode(ctx context.Context, shim *rcmv1.Shim, } // We rely on controller-runtime to rate limit us. - if err := sr.Client.Patch(ctx, job, patchMethod, patchOptions); err != nil { + if err := sr.Patch(ctx, job, patchMethod, patchOptions); err != nil { log.Error().Msgf("Unable to reconcile Job: %s", err) if err := sr.updateNodeLabels(ctx, &node, shim, "failed"); err != nil { log.Error().Msgf("Unable to update node label %s: %s", shim.Name, err) @@ -506,7 +506,7 @@ func (sr *ShimReconciler) handleDeployRuntimeClass(ctx context.Context, shim *rc } // Note that we reconcile even if the deployment is in a good state. We rely on controller-runtime to rate limit us. - if err := sr.Client.Patch(ctx, runtimeClass, patchMethod, patchOptions); err != nil { + if err := sr.Patch(ctx, runtimeClass, patchMethod, patchOptions); err != nil { log.Error().Msgf("Unable to reconcile RuntimeClass %s", err) return ctrl.Result{}, fmt.Errorf("failed to reconcile RuntimeClass: %w", err) } @@ -602,7 +602,7 @@ func (sr *ShimReconciler) runtimeClassExists(ctx context.Context, shim *rcmv1.Sh // getRuntimeClass finds a RuntimeClass. func (sr *ShimReconciler) getRuntimeClass(ctx context.Context, shim *rcmv1.Shim) (*nodev1.RuntimeClass, error) { rc := nodev1.RuntimeClass{} - err := sr.Client.Get(ctx, types.NamespacedName{Name: shim.Spec.RuntimeClass.Name, Namespace: shim.Namespace}, &rc) + err := sr.Get(ctx, types.NamespacedName{Name: shim.Spec.RuntimeClass.Name, Namespace: shim.Namespace}, &rc) if err != nil { return nil, fmt.Errorf("failed to get runtimeClass: %w", err) } @@ -613,7 +613,7 @@ func (sr *ShimReconciler) getRuntimeClass(ctx context.Context, shim *rcmv1.Shim) func (sr *ShimReconciler) removeFinalizerFromShim(ctx context.Context, shim *rcmv1.Shim) error { if controllerutil.ContainsFinalizer(shim, RCMOperatorFinalizer) { controllerutil.RemoveFinalizer(shim, RCMOperatorFinalizer) - if err := sr.Client.Update(ctx, shim); err != nil { + if err := sr.Update(ctx, shim); err != nil { return fmt.Errorf("failed to remove finalizer: %w", err) } } @@ -624,7 +624,7 @@ func (sr *ShimReconciler) removeFinalizerFromShim(ctx context.Context, shim *rcm func (sr *ShimReconciler) ensureFinalizerForShim(ctx context.Context, shim *rcmv1.Shim, finalizer string) error { if !controllerutil.ContainsFinalizer(shim, finalizer) { controllerutil.AddFinalizer(shim, finalizer) - if err := sr.Client.Update(ctx, shim); err != nil { + if err := sr.Update(ctx, shim); err != nil { return fmt.Errorf("failed to set finalizer: %w", err) } }