diff --git a/common/.github/workflows/validate.yml b/common/.github/workflows/validate.yml index 14eb0ce22f..28b2413c1d 100644 --- a/common/.github/workflows/validate.yml +++ b/common/.github/workflows/validate.yml @@ -11,7 +11,7 @@ on: permissions: read-all env: - LINT_VERSION: v1.64.8 + LINT_VERSION: v2.0 jobs: codespell: @@ -37,13 +37,13 @@ jobs: sudo apt-get -qq update sudo apt-get -qq install libseccomp-dev - name: lint - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v7 with: version: "${{ env.LINT_VERSION }}" args: --verbose # Extra linters, only checking new code from a pull request. - name: lint-extra - uses: golangci/golangci-lint-action@v6 + uses: golangci/golangci-lint-action@v7 with: args: --config=.golangci-extra.yml version: "${{ env.LINT_VERSION }}" diff --git a/common/.golangci-extra.yml b/common/.golangci-extra.yml index 849b7fcb44..f77d1811f6 100644 --- a/common/.golangci-extra.yml +++ b/common/.golangci-extra.yml @@ -6,7 +6,10 @@ # The idea is to impose additional rules for newly added code only # (rules we can not realistically satisfy for existing code). +version: "2" + run: + timeout: 5m build-tags: - apparmor - seccomp @@ -14,9 +17,13 @@ run: - systemd - exclude_graphdriver_btrfs - containers_image_openpgp - timeout: 5m linters: - disable-all: true + default: none enable: - godot + - staticcheck + settings: + staticcheck: + checks: + - all diff --git a/common/.golangci.yml b/common/.golangci.yml index d71b6b310b..c5e98a9909 100644 --- a/common/.golangci.yml +++ b/common/.golangci.yml @@ -1,5 +1,7 @@ ---- +version: "2" + run: + timeout: 5m build-tags: - apparmor - seccomp @@ -8,81 +10,80 @@ run: - exclude_graphdriver_btrfs - containers_image_openpgp - cni - timeout: 5m -linters: - enable-all: true - disable: - # linters explicitly disabled for the below mentioned reasons ... - - funlen # too aggressive/wishful size/statement limit - - gochecknoinits # too many hard to fix init() funcs across the code - - gocognit # too aggressive default - - wsl # useful but too opinionated - # others to be re-enabled one-by-one ... - - goconst - - godox - - lll - - nestif - - cyclop - - depguard - - errchkjson - - errname - - errorlint - - exhaustive - - gochecknoglobals - - err113 - - nolintlint - - wrapcheck - - varnamelen - - testpackage - - tenv - - tagliatelle - - stylecheck - - paralleltest - - nonamedreturns - - nlreturn - - nakedret - - musttag - - maintidx - - ireturn - - exhaustruct - - gosec - - godot - - gocyclo - - dogsled - - tparallel - - thelper - - mnd #way to many false positives - - nilnil - - nilerr - - interfacebloat - - forcetypeassert - - gomoddirectives - - testifylint # way to many issues to fix right now, however it is a great linter for better test errors - - inamedparam # opinionated style -linters-settings: - errcheck: - check-type-assertions: true - gocyclo: - min-complexity: 35 - gofmt: - rewrite-rules: - - pattern: 'interface{}' - replacement: 'any' - revive: - rules: - - name: dot-imports - disabled: true -issues: - # Excluding configuration per-path, per-linter, per-text and per-source - exclude-rules: - # Exclude some linters from running on tests files. - - path: _test\.go - linters: - - dupl - - perfsprint - # Exclude "should pass the context parameter" for libimage.LookupImage because of backward compatibility. - - path: "libimage" - text: "LookupImage" - linters: - - contextcheck +formatters: + enable: + - gofumpt + +linters: + enable: + - asasalint + - asciicheck + - bidichk + - bodyclose + - canonicalheader + - containedctx + - contextcheck + - copyloopvar + - decorder + - dupl + - dupword + - durationcheck + - exptostd + - fatcontext + - forbidigo + - ginkgolinter + - gocheckcompilerdirectives + - gochecksumtype + - gocritic + - goprintffuncname + - gosmopolitan + - iface + - intrange + - makezero + - mirror + - misspell + - nilnesserr + - noctx + - nosprintfhostport + - perfsprint + - prealloc + - predeclared + - reassign + - recvcheck + - revive + - unconvert + - unparam + - usestdlibvars + - usetesting + - wastedassign + - whitespace + settings: + errcheck: + check-type-assertions: true + gocyclo: + min-complexity: 35 + revive: + rules: + - name: dot-imports + disabled: true + staticcheck: + checks: + - all + - -ST1003 # https://staticcheck.dev/docs/checks/#ST1003 Poorly chosen identifier. + - -QF1008 # https://staticcheck.dev/docs/checks/#QF1008 Omit embedded fields from selector expression. + exclusions: + generated: strict + presets: + - comments + - common-false-positives + - std-error-handling + rules: + - linters: + - dupl + - perfsprint + path: _test\.go + - linters: + - contextcheck + path: libimage + text: LookupImage diff --git a/common/libimage/image.go b/common/libimage/image.go index 91e694c16e..8f300e8399 100644 --- a/common/libimage/image.go +++ b/common/libimage/image.go @@ -463,13 +463,13 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma skipRemove := false numNames := len(i.Names()) - // NOTE: the `numNames == 1` check is not only a performance + // NOTE: the `numNames != 1` check is not only a performance // optimization but also preserves existing Podman/Docker behaviour. // If image "foo" is used by a container and has only this tag/name, // an `rmi foo` will not untag "foo" but instead attempt to remove the // entire image. If there's a container using "foo", we should get an // error. - if !(referencedBy == "" || numNames == 1) { + if referencedBy != "" && numNames != 1 { byID := strings.HasPrefix(i.ID(), referencedBy) byDigest := strings.HasPrefix(referencedBy, "sha256:") if !options.Force { diff --git a/common/libimage/load.go b/common/libimage/load.go index 48f255e8b1..2f69fd639b 100644 --- a/common/libimage/load.go +++ b/common/libimage/load.go @@ -115,7 +115,7 @@ func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) ( // Give a decent error message if nothing above worked. // we want the colon here for the multiline error - //nolint:revive + //nolint:revive,staticcheck loadError := errors.New("payload does not match any of the supported image formats:") for _, err := range loadErrors { loadError = fmt.Errorf("%v\n * %v", loadError, err) diff --git a/common/libimage/normalize.go b/common/libimage/normalize.go index b00af66a01..1dab82fe27 100644 --- a/common/libimage/normalize.go +++ b/common/libimage/normalize.go @@ -30,7 +30,7 @@ func NormalizeName(name string) (reference.Named, error) { // Enforce "localhost" if needed. registry := reference.Domain(named) - if !(strings.ContainsAny(registry, ".:") || registry == "localhost") { + if !strings.ContainsAny(registry, ".:") && registry != "localhost" { name = toLocalImageName(ref.String()) } diff --git a/common/libimage/search.go b/common/libimage/search.go index 6ab016b3b5..2745b52bb3 100644 --- a/common/libimage/search.go +++ b/common/libimage/search.go @@ -217,7 +217,7 @@ func (r *Runtime) searchImageInRegistry(ctx context.Context, term, registry stri paramsArr := []SearchResult{} for i := range limit { // Check whether query matches filters - if !(filterMatchesAutomatedFilter(&options.Filter, results[i]) && filterMatchesOfficialFilter(&options.Filter, results[i]) && filterMatchesStarFilter(&options.Filter, results[i])) { + if !filterMatchesAutomatedFilter(&options.Filter, results[i]) || !filterMatchesOfficialFilter(&options.Filter, results[i]) || !filterMatchesStarFilter(&options.Filter, results[i]) { continue } official := "" diff --git a/common/libnetwork/cni/network.go b/common/libnetwork/cni/network.go index 7e001fab0e..ab3a05b555 100644 --- a/common/libnetwork/cni/network.go +++ b/common/libnetwork/cni/network.go @@ -191,7 +191,7 @@ func (n *cniNetwork) loadNetworks() error { } if !types.NameRegex.MatchString(conf.Name) { - logrus.Warnf("CNI config list %s has invalid name, skipping: %v", file, types.RegexError) + logrus.Warnf("CNI config list %s has invalid name, skipping: %v", file, types.ErrInvalidName) continue } diff --git a/common/libnetwork/internal/util/bridge.go b/common/libnetwork/internal/util/bridge.go index 44cd555c17..e69c388290 100644 --- a/common/libnetwork/internal/util/bridge.go +++ b/common/libnetwork/internal/util/bridge.go @@ -19,7 +19,7 @@ func CreateBridge(n NetUtil, network *types.Network, usedNetworks []*net.IPNet, } } if !types.NameRegex.MatchString(network.NetworkInterface) { - return fmt.Errorf("bridge name %s invalid: %w", network.NetworkInterface, types.RegexError) + return fmt.Errorf("bridge name %s invalid: %w", network.NetworkInterface, types.ErrInvalidName) } } else { var err error diff --git a/common/libnetwork/internal/util/create.go b/common/libnetwork/internal/util/create.go index 376a57315b..42609d1ba7 100644 --- a/common/libnetwork/internal/util/create.go +++ b/common/libnetwork/internal/util/create.go @@ -23,7 +23,7 @@ func CommonNetworkCreate(n NetUtil, network *types.Network) error { // validate the name when given if network.Name != "" { if !types.NameRegex.MatchString(network.Name) { - return fmt.Errorf("network name %s invalid: %w", network.Name, types.RegexError) + return fmt.Errorf("network name %s invalid: %w", network.Name, types.ErrInvalidName) } if _, err := n.Network(network.Name); err == nil { return fmt.Errorf("network name %s already used: %w", network.Name, types.ErrNetworkExists) diff --git a/common/libnetwork/netavark/config.go b/common/libnetwork/netavark/config.go index 4916f725e1..6c17bd0a19 100644 --- a/common/libnetwork/netavark/config.go +++ b/common/libnetwork/netavark/config.go @@ -294,10 +294,7 @@ func createIpvlanOrMacvlan(network *types.Network) error { } driver := network.Driver - isMacVlan := true - if driver == types.IPVLANNetworkDriver { - isMacVlan = false - } + isMacVlan := driver != types.IPVLANNetworkDriver // always turn dns off with macvlan, it is not implemented in netavark // and makes little sense to support with macvlan diff --git a/common/libnetwork/netavark/network.go b/common/libnetwork/netavark/network.go index 985d0db2dd..cf8b0d490e 100644 --- a/common/libnetwork/netavark/network.go +++ b/common/libnetwork/netavark/network.go @@ -248,7 +248,7 @@ func (n *netavarkNetwork) loadNetworks() error { } if !types.NameRegex.MatchString(network.Name) { - logrus.Warnf("Network config %q has invalid name: %q, skipping: %v", path, network.Name, types.RegexError) + logrus.Warnf("Network config %q has invalid name: %q, skipping: %v", path, network.Name, types.ErrInvalidName) continue } diff --git a/common/libnetwork/slirp4netns/slirp4netns.go b/common/libnetwork/slirp4netns/slirp4netns.go index 524cabc42a..afb54f60d6 100644 --- a/common/libnetwork/slirp4netns/slirp4netns.go +++ b/common/libnetwork/slirp4netns/slirp4netns.go @@ -682,7 +682,7 @@ func openSlirp4netnsPort(apiSocket, proto, hostip string, hostport, guestport ui if err != nil { return fmt.Errorf("cannot marshal JSON for slirp4netns: %w", err) } - if _, err := conn.Write([]byte(fmt.Sprintf("%s\n", data))); err != nil { + if _, err := fmt.Fprintf(conn, "%s\n", data); err != nil { return fmt.Errorf("cannot write to control socket %s: %w", apiSocket, err) } //nolint:errcheck // This cast should never fail, if it does we get a interface diff --git a/common/libnetwork/types/define.go b/common/libnetwork/types/define.go index 0bc688103e..e5929c3e87 100644 --- a/common/libnetwork/types/define.go +++ b/common/libnetwork/types/define.go @@ -24,8 +24,10 @@ var ( // NameRegex is a regular expression to validate names. // This must NOT be changed. NameRegex = regexp.Delayed("^[a-zA-Z0-9][a-zA-Z0-9_.-]*$") - // RegexError is thrown in presence of an invalid name. - RegexError = fmt.Errorf("names must match [a-zA-Z0-9][a-zA-Z0-9_.-]*: %w", ErrInvalidArg) // nolint:revive // This lint is new and we do not want to break the API. + // ErrInvalidName is thrown in presence of an invalid name. + ErrInvalidName = fmt.Errorf("names must match [a-zA-Z0-9][a-zA-Z0-9_.-]*: %w", ErrInvalidArg) + // Deprecated: use [ErrInvalidName] instead. + RegexError = ErrInvalidName // NotHexRegex is a regular expression to check if a string is // a hexadecimal string. diff --git a/common/pkg/auth/auth.go b/common/pkg/auth/auth.go index a3d333a99a..4bcd490db3 100644 --- a/common/pkg/auth/auth.go +++ b/common/pkg/auth/auth.go @@ -173,10 +173,10 @@ func Login(ctx context.Context, systemContext *types.SystemContext, opts *LoginO if opts.StdinPassword { var stdinPasswordStrBuilder strings.Builder if opts.Password != "" { - return errors.New("Can't specify both --password-stdin and --password") + return errors.New("can't specify both --password-stdin and --password") } if opts.Username == "" { - return errors.New("Must provide --username with --password-stdin") + return errors.New("must provide --username with --password-stdin") } scanner := bufio.NewScanner(opts.Stdin) for scanner.Scan() { diff --git a/common/pkg/cgroups/utils_linux.go b/common/pkg/cgroups/utils_linux.go index 848a59d236..505ac1b29f 100644 --- a/common/pkg/cgroups/utils_linux.go +++ b/common/pkg/cgroups/utils_linux.go @@ -221,7 +221,7 @@ func MoveUnderCgroup(cgroup, subtree string, processes []uint32) error { } // root cgroup, skip it - if parts[2] == "/" && !(unifiedMode && parts[1] == "") { + if parts[2] == "/" && (!unifiedMode || parts[1] != "") { continue } @@ -261,7 +261,7 @@ func MoveUnderCgroup(cgroup, subtree string, processes []uint32) error { if len(processes) > 0 { for _, pid := range processes { - if _, err := f.WriteString(fmt.Sprintf("%d\n", pid)); err != nil { + if _, err := fmt.Fprintf(f, "%d\n", pid); err != nil { logrus.Debugf("Cannot move process %d to cgroup %q: %v", pid, newCgroup, err) } } diff --git a/common/pkg/configmaps/configmaps.go b/common/pkg/configmaps/configmaps.go index 092cb01534..8fc831c8ea 100644 --- a/common/pkg/configmaps/configmaps.go +++ b/common/pkg/configmaps/configmaps.go @@ -132,7 +132,7 @@ func (s *ConfigMapManager) Store(name string, data []byte, driverType string, dr return "", err } - if !(len(data) > 0 && len(data) < maxConfigMapSize) { + if len(data) == 0 || len(data) >= maxConfigMapSize { return "", errDataSize } diff --git a/common/pkg/parse/parse_unix.go b/common/pkg/parse/parse_unix.go index 86563b3eb6..3e4e1d140d 100644 --- a/common/pkg/parse/parse_unix.go +++ b/common/pkg/parse/parse_unix.go @@ -17,7 +17,7 @@ func DeviceFromPath(device string) ([]devices.Device, error) { return nil, err } if unshare.IsRootless() && src != dst { - return nil, fmt.Errorf("Renaming device %s to %s is not supported in rootless containers", src, dst) + return nil, fmt.Errorf("renaming device %s to %s is not supported in rootless containers", src, dst) } srcInfo, err := os.Stat(src) if err != nil { diff --git a/common/pkg/retry/retry.go b/common/pkg/retry/retry.go index d47e47e101..adc42c3b34 100644 --- a/common/pkg/retry/retry.go +++ b/common/pkg/retry/retry.go @@ -47,7 +47,7 @@ func IfNecessary(ctx context.Context, operation func() error, options *Options) logrus.Warnf("Failed, retrying in %s ... (%d/%d). Error: %v", delay, attempt+1, options.MaxRetry, err) select { case <-time.After(delay): - break + // Do nothing. case <-ctx.Done(): return err } diff --git a/common/pkg/secrets/secrets.go b/common/pkg/secrets/secrets.go index 113c6c43b3..7d8d33a114 100644 --- a/common/pkg/secrets/secrets.go +++ b/common/pkg/secrets/secrets.go @@ -166,7 +166,7 @@ func (s *SecretsManager) Store(name string, data []byte, driverType string, opti return "", err } - if !(len(data) > 0 && len(data) < maxSecretSize) { + if len(data) == 0 || len(data) >= maxSecretSize { return "", errDataSize } var secr *Secret diff --git a/common/pkg/secrets/secrets_test.go b/common/pkg/secrets/secrets_test.go index 7677d7191c..550bf5ee89 100644 --- a/common/pkg/secrets/secrets_test.go +++ b/common/pkg/secrets/secrets_test.go @@ -64,7 +64,7 @@ func TestAddSecretAndLookupData(t *testing.T) { s, _, err = manager.LookupSecretData("mysecret") require.NoError(t, err) - if s.CreatedAt == s.UpdatedAt { + if s.CreatedAt.Equal(s.UpdatedAt) { t.Errorf("error: secret CreatedAt should not equal UpdatedAt after a Replace") } diff --git a/common/pkg/subscriptions/subscriptions.go b/common/pkg/subscriptions/subscriptions.go index 2c78cf1e65..202eb6d3c5 100644 --- a/common/pkg/subscriptions/subscriptions.go +++ b/common/pkg/subscriptions/subscriptions.go @@ -322,7 +322,7 @@ func addSubscriptionsFromMountsFile(filePath, mountLabel, containerRunDir string func containerHasEtcSystemFips(subscriptionsDir, mountPoint string) (bool, error) { containerEtc, err := securejoin.SecureJoin(mountPoint, "etc") if err != nil { - return false, fmt.Errorf("Container /etc resolution error: %w", err) + return false, fmt.Errorf("container /etc resolution error: %w", err) } if fileutils.Lexists(filepath.Join(containerEtc, "system-fips")) != nil { logrus.Debug("/etc/system-fips does not exist in the container, not creating /run/secrets/system-fips") @@ -331,7 +331,7 @@ func containerHasEtcSystemFips(subscriptionsDir, mountPoint string) (bool, error fipsFileTarget, err := securejoin.SecureJoin(mountPoint, "etc/system-fips") if err != nil { - return false, fmt.Errorf("Container /etc/system-fips resolution error: %w", err) + return false, fmt.Errorf("container /etc/system-fips resolution error: %w", err) } if fipsFileTarget != filepath.Join(mountPoint, subscriptionsDir, "system-fips") { logrus.Warnf("/etc/system-fips exists in the container, but is not a symlink to %[1]v/system-fips; not creating %[1]v/system-fips", subscriptionsDir) @@ -448,24 +448,24 @@ func addFIPSMounts(mounts *[]rspec.Mount, containerRunDir, mountPoint, mountLabe destPolicyConfig := "/etc/crypto-policies/config" srcPolicyConfigOnHost, err := securejoin.SecureJoin(mountPoint, srcPolicyConfig) if err != nil { - return fmt.Errorf("Could not expand %q in container: %w", srcPolicyConfig, err) + return fmt.Errorf("could not expand %q in container: %w", srcPolicyConfig, err) } if err = fileutils.Exists(srcPolicyConfigOnHost); err != nil { if !errors.Is(err, os.ErrNotExist) { - return fmt.Errorf("Could not check whether %q exists in container: %w", srcPolicyConfig, err) + return fmt.Errorf("could not check whether %q exists in container: %w", srcPolicyConfig, err) } // /usr/share/crypto-policies/default-fips-config does not exist, let's create it ourselves cryptoPoliciesConfigFile := filepath.Join(containerRunDir, "fips-config") if err := os.WriteFile(cryptoPoliciesConfigFile, []byte("FIPS\n"), 0o644); err != nil { - return fmt.Errorf("Failed to write fips config file in container for FIPS mode: %w", err) + return fmt.Errorf("failed to write fips config file in container for FIPS mode: %w", err) } if err = label.Relabel(cryptoPoliciesConfigFile, mountLabel, false); err != nil { - return fmt.Errorf("Failed to apply correct labels on fips config file: %w", err) + return fmt.Errorf("failed to apply correct labels on fips config file: %w", err) } if err := os.Chown(cryptoPoliciesConfigFile, uid, gid); err != nil { - return fmt.Errorf("Failed to chown fips config file: %w", err) + return fmt.Errorf("failed to chown fips config file: %w", err) } srcPolicyConfigOnHost = cryptoPoliciesConfigFile diff --git a/common/pkg/timetype/timestamp.go b/common/pkg/timetype/timestamp.go index 519884c553..ce03c784c5 100644 --- a/common/pkg/timetype/timestamp.go +++ b/common/pkg/timetype/timestamp.go @@ -31,8 +31,9 @@ func GetTimestamp(value string, reference time.Time) (string, error) { } var format string - // if the string has a Z or a + or three dashes use parse otherwise use parseinlocation - parseInLocation := !(strings.ContainsAny(value, "zZ+") || strings.Count(value, "-") == 3) + // If the string has a Z, or a +, or three dashes, + // then use time.Parse, otherwise use time.ParseInLocation. + parseInLocation := !strings.ContainsAny(value, "zZ+") && strings.Count(value, "-") != 3 switch { case strings.Contains(value, "."): diff --git a/common/pkg/timezone/timezone.go b/common/pkg/timezone/timezone.go index b82dc309d0..e606614578 100644 --- a/common/pkg/timezone/timezone.go +++ b/common/pkg/timezone/timezone.go @@ -20,10 +20,10 @@ import ( // It returns the path of the created /etc/localtime file if needed. func ConfigureContainerTimeZone(timezone, containerRunDir, mountPoint, etcPath, containerID string) (localTimePath string, err error) { var timezonePath string - switch { - case timezone == "": + switch timezone { + case "": return "", nil - case timezone == "local": + case "local": timezonePath, err = filepath.EvalSymlinks("/etc/localtime") if err != nil { return "", fmt.Errorf("finding local timezone for container %s: %w", containerID, err)