lint: update golangci-lint to v1.56.2

Adds a couple of new linters with many new warnings:
- perfsprint (fixed in code, excluded for tests)
- revive (fix the problems, disabled the dot import check as we use it
  for ginkgo)
- inamedparam (disabled because it is opinionated)
- testifylint (great linter but we have like 1000+ issues that I have no
  time fixing now)

Signed-off-by: Paul Holzinger <pholzing@redhat.com>

common/.github/workflows/validate.yml

@@ -11,7 +11,7 @@ on:
 permissions: read-all
 
 env:
-  LINT_VERSION: v1.53
+  LINT_VERSION: v1.56.2
 
 jobs:
   codespell:

common/.golangci.yml

@@ -71,6 +71,8 @@ linters:
     - 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
@@ -80,6 +82,10 @@ linters-settings:
     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
@@ -88,6 +94,7 @@ issues:
     - path: _test\.go
       linters:
         - dupl
+        - perfsprint
     # Exclude "should pass the context parameter" for libimage.LookupImage because of backward compatibility.
     - path: "libimage"
       text: "LookupImage"

common/Makefile

@@ -85,7 +85,7 @@ vendor:
 .PHONY: install.tools
 install.tools: build/golangci-lint .install.md2man
 
-build/golangci-lint: VERSION=v1.53.3
+build/golangci-lint: VERSION=v1.56.2
 build/golangci-lint:
 	curl -fsSL https://raw.githubusercontent.com/golangci/golangci-lint/$(VERSION)/install.sh | sh -s -- -b ./build $(VERSION)
 

common/libimage/copier.go

@@ -432,12 +432,12 @@ func (c *copier) copy(ctx context.Context, source, destination types.ImageRefere
 	// Sanity checks for Buildah.
 	if sourceInsecure != nil && *sourceInsecure {
 		if c.systemContext.DockerInsecureSkipTLSVerify == types.OptionalBoolFalse {
-			return nil, fmt.Errorf("can't require tls verification on an insecured registry")
+			return nil, errors.New("can't require tls verification on an insecured registry")
 		}
 	}
 	if destinationInsecure != nil && *destinationInsecure {
 		if c.systemContext.DockerInsecureSkipTLSVerify == types.OptionalBoolFalse {
-			return nil, fmt.Errorf("can't require tls verification on an insecured registry")
+			return nil, errors.New("can't require tls verification on an insecured registry")
 		}
 	}
 

common/libimage/filters.go

@@ -4,6 +4,7 @@ package libimage
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"path"
 	"strconv"
@@ -226,7 +227,7 @@ func (r *Runtime) containers(duplicate map[string]string, key, value string, ext
 	case "false", "true":
 	case "external":
 		if externalFunc == nil {
-			return fmt.Errorf("libimage error: external containers filter without callback")
+			return errors.New("libimage error: external containers filter without callback")
 		}
 	default:
 		return fmt.Errorf("unsupported value %q for containers filter", value)

common/libimage/load.go

@@ -4,6 +4,7 @@ package libimage
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"time"
@@ -91,7 +92,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
-	loadError := fmt.Errorf("payload does not match any of the supported image formats:")
+	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)
 	}

common/libimage/manifests/manifests.go

@@ -834,7 +834,7 @@ func (l *list) AddArtifact(ctx context.Context, sys *types.SystemContext, option
 	if configDescriptor.Size != 0 {
 		if len(configDescriptor.Data) == 0 {
 			if options.ConfigFile == "" {
-				return "", fmt.Errorf("needed config data file, but none was provided")
+				return "", errors.New("needed config data file, but none was provided")
 			}
 			filePath, err := filepath.Abs(options.ConfigFile)
 			if err != nil {

common/libimage/pull.go

@@ -426,7 +426,7 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference
 	for _, tag := range tags {
 		select { // Let's be gentle with Podman remote.
 		case <-ctx.Done():
-			return nil, fmt.Errorf("pulling cancelled")
+			return nil, errors.New("pulling cancelled")
 		default:
 			// We can continue.
 		}
@@ -466,7 +466,7 @@ func (r *Runtime) imagesIDsForManifest(manifestBytes []byte, sys *types.SystemCo
 	} else {
 		d, err := manifest.Digest(manifestBytes)
 		if err != nil {
-			return nil, fmt.Errorf("digesting manifest")
+			return nil, errors.New("digesting manifest")
 		}
 		imageDigest = d
 	}

common/libimage/runtime.go

@@ -690,7 +690,7 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem
 	}
 
 	if options.ExternalContainers && options.IsExternalContainerFunc == nil {
-		return nil, []error{fmt.Errorf("libimage error: cannot remove external containers without callback")}
+		return nil, []error{errors.New("libimage error: cannot remove external containers without callback")}
 	}
 
 	// The logic here may require some explanation.  Image removal is

common/libimage/save.go

@@ -53,7 +53,7 @@ func (r *Runtime) Save(ctx context.Context, names []string, format, path string,
 			return fmt.Errorf("unsupported format %q for saving multiple images (only docker-archive)", format)
 		}
 		if len(options.AdditionalTags) > 0 {
-			return fmt.Errorf("cannot save multiple images with multiple tags")
+			return errors.New("cannot save multiple images with multiple tags")
 		}
 	}
 

common/libnetwork/internal/util/validate.go

@@ -93,15 +93,15 @@ func ValidateRoutes(routes []types.Route) error {
 
 func ValidateRoute(route types.Route) error {
 	if route.Destination.IP == nil {
-		return fmt.Errorf("route destination ip nil")
+		return errors.New("route destination ip nil")
 	}
 
 	if route.Destination.Mask == nil {
-		return fmt.Errorf("route destination mask nil")
+		return errors.New("route destination mask nil")
 	}
 
 	if route.Gateway == nil {
-		return fmt.Errorf("route gateway nil")
+		return errors.New("route gateway nil")
 	}
 
 	// Reparse to ensure destination is valid.
@@ -112,7 +112,7 @@ func ValidateRoute(route types.Route) error {
 
 	// check that destination is a network and not an address
 	if !ip.Equal(ipNet.IP) {
-		return fmt.Errorf("route destination invalid")
+		return errors.New("route destination invalid")
 	}
 
 	return nil

common/libnetwork/netavark/config.go

@@ -309,7 +309,7 @@ func createIpvlanOrMacvlan(network *types.Network) error {
 			return errIpvlanNoDHCP
 		}
 		if len(network.Subnets) > 0 {
-			return fmt.Errorf("ipam driver dhcp set but subnets are set")
+			return errors.New("ipam driver dhcp set but subnets are set")
 		}
 	}
 

common/libnetwork/netavark/testplugin/main.go

@@ -32,7 +32,7 @@ func main() {
 	case "setup", "teardown":
 		// this is executed and tested in netavark so we not need it here
 	default:
-		errAndExit(fmt.Sprintf("unknown argument: %s", os.Args[1]))
+		errAndExit("unknown argument: " + os.Args[1])
 	}
 }
 

common/libnetwork/pasta/pasta.go

@@ -63,7 +63,7 @@ func Setup(opts *SetupOptions) error {
 			var addr string
 
 			if i.HostIP != "" {
-				addr = fmt.Sprintf("%s/", i.HostIP)
+				addr = i.HostIP + "/"
 			}
 
 			switch protocol {

common/libnetwork/slirp4netns/slirp4netns.go

@@ -210,7 +210,7 @@ func createBasicSlirpCmdArgs(options *networkOptions, features *slirpFeatures) (
 		cmdArgs = append(cmdArgs, "--disable-host-loopback")
 	}
 	if options.mtu > -1 && features.HasMTU {
-		cmdArgs = append(cmdArgs, fmt.Sprintf("--mtu=%d", options.mtu))
+		cmdArgs = append(cmdArgs, "--mtu="+strconv.Itoa(options.mtu))
 	}
 	if !options.noPivotRoot && features.HasEnableSandbox {
 		cmdArgs = append(cmdArgs, "--enable-sandbox")
@@ -221,33 +221,33 @@ func createBasicSlirpCmdArgs(options *networkOptions, features *slirpFeatures) (
 
 	if options.cidr != "" {
 		if !features.HasCIDR {
-			return nil, fmt.Errorf("cidr not supported")
+			return nil, errors.New("cidr not supported")
 		}
-		cmdArgs = append(cmdArgs, fmt.Sprintf("--cidr=%s", options.cidr))
+		cmdArgs = append(cmdArgs, "--cidr="+options.cidr)
 	}
 
 	if options.enableIPv6 {
 		if !features.HasIPv6 {
-			return nil, fmt.Errorf("enable_ipv6 not supported")
+			return nil, errors.New("enable_ipv6 not supported")
 		}
 		cmdArgs = append(cmdArgs, "--enable-ipv6")
 	}
 
 	if options.outboundAddr != "" {
 		if !features.HasOutboundAddr {
-			return nil, fmt.Errorf("outbound_addr not supported")
+			return nil, errors.New("outbound_addr not supported")
 		}
-		cmdArgs = append(cmdArgs, fmt.Sprintf("--outbound-addr=%s", options.outboundAddr))
+		cmdArgs = append(cmdArgs, "--outbound-addr="+options.outboundAddr)
 	}
 
 	if options.outboundAddr6 != "" {
 		if !features.HasOutboundAddr || !features.HasIPv6 {
-			return nil, fmt.Errorf("outbound_addr6 not supported")
+			return nil, errors.New("outbound_addr6 not supported")
 		}
 		if !options.enableIPv6 {
-			return nil, fmt.Errorf("enable_ipv6=true is required for outbound_addr6")
+			return nil, errors.New("enable_ipv6=true is required for outbound_addr6")
 		}
-		cmdArgs = append(cmdArgs, fmt.Sprintf("--outbound-addr6=%s", options.outboundAddr6))
+		cmdArgs = append(cmdArgs, "--outbound-addr6="+options.outboundAddr6)
 	}
 
 	return cmdArgs, nil
@@ -300,7 +300,7 @@ func Setup(opts *SetupOptions) (*SetupResult, error) {
 
 	var apiSocket string
 	if havePortMapping && netOptions.isSlirpHostForward {
-		apiSocket = filepath.Join(opts.Config.Engine.TmpDir, fmt.Sprintf("%s.net", opts.ContainerID))
+		apiSocket = filepath.Join(opts.Config.Engine.TmpDir, opts.ContainerID+".net")
 		cmdArgs = append(cmdArgs, "--api-socket", apiSocket)
 	}
 
@@ -610,7 +610,7 @@ func SetupRootlessPortMappingViaRLK(opts *SetupOptions, slirpSubnet *net.IPNet,
 		if stdoutStr != "" {
 			// err contains full debug log and too verbose, so return stdoutStr
 			logrus.Debug(err)
-			return fmt.Errorf("rootlessport " + strings.TrimSuffix(stdoutStr, "\n"))
+			return errors.New("rootlessport " + strings.TrimSuffix(stdoutStr, "\n"))
 		}
 		return err
 	}

common/pkg/auth/auth.go

@@ -285,7 +285,7 @@ func getUserAndPass(opts *LoginOptions, password, userFromAuthFile string) (user
 	username := opts.Username
 	if username == "" {
 		if opts.Stdin == nil {
-			return "", "", fmt.Errorf("cannot prompt for username without stdin")
+			return "", "", errors.New("cannot prompt for username without stdin")
 		}
 
 		if userFromAuthFile != "" {

common/pkg/cgroups/cgroups_linux.go

@@ -103,7 +103,7 @@ func getAvailableControllers(exclude map[string]controllerHandler, cgroup2 bool)
 			}
 			// userSlice already contains '/' so not adding here
 			basePath := cgroupRoot + userSlice
-			controllersFile = fmt.Sprintf("%s/cgroup.controllers", basePath)
+			controllersFile = basePath + "/cgroup.controllers"
 		}
 		controllersFileBytes, err := os.ReadFile(controllersFile)
 		if err != nil {
@@ -389,7 +389,7 @@ func Load(path string) (*CgroupControl, error) {
 // CreateSystemdUnit creates the systemd cgroup
 func (c *CgroupControl) CreateSystemdUnit(path string) error {
 	if !c.systemd {
-		return fmt.Errorf("the cgroup controller is not using systemd")
+		return errors.New("the cgroup controller is not using systemd")
 	}
 
 	conn, err := systemdDbus.NewWithContext(context.TODO())
@@ -404,7 +404,7 @@ func (c *CgroupControl) CreateSystemdUnit(path string) error {
 // CreateSystemdUserUnit creates the systemd cgroup for the specified user
 func (c *CgroupControl) CreateSystemdUserUnit(path string, uid int) error {
 	if !c.systemd {
-		return fmt.Errorf("the cgroup controller is not using systemd")
+		return errors.New("the cgroup controller is not using systemd")
 	}
 
 	conn, err := UserConnection(uid)
@@ -678,7 +678,7 @@ func cpusetCopyFileFromParent(dir, file string, cgroupv2 bool) ([]byte, error) {
 	path := filepath.Join(dir, file)
 	parentPath := path
 	if cgroupv2 {
-		parentPath = fmt.Sprintf("%s.effective", parentPath)
+		parentPath += ".effective"
 	}
 	data, err := os.ReadFile(parentPath)
 	if err != nil {

common/pkg/cgroups/systemd_linux.go

@@ -25,7 +25,7 @@ func systemdCreate(resources *configs.Resources, path string, c *systemdDbus.Con
 	var lastError error
 	for i := 0; i < 2; i++ {
 		properties := []systemdDbus.Property{
-			systemdDbus.PropDescription(fmt.Sprintf("cgroup %s", name)),
+			systemdDbus.PropDescription("cgroup " + name),
 			systemdDbus.PropWants(slice),
 		}
 		var ioString string

common/pkg/config/default.go

@@ -531,13 +531,13 @@ func (c EngineConfig) EventsLogMaxSize() uint64 {
 func (c *Config) SecurityOptions() []string {
 	securityOpts := []string{}
 	if c.Containers.SeccompProfile != "" && c.Containers.SeccompProfile != SeccompDefaultPath {
-		securityOpts = append(securityOpts, fmt.Sprintf("seccomp=%s", c.Containers.SeccompProfile))
+		securityOpts = append(securityOpts, "seccomp="+c.Containers.SeccompProfile)
 	}
 	if apparmor.IsEnabled() && c.Containers.ApparmorProfile != "" {
-		securityOpts = append(securityOpts, fmt.Sprintf("apparmor=%s", c.Containers.ApparmorProfile))
+		securityOpts = append(securityOpts, "apparmor="+c.Containers.ApparmorProfile)
 	}
 	if selinux.GetEnabled() && !c.Containers.EnableLabeling {
-		securityOpts = append(securityOpts, fmt.Sprintf("label=%s", selinux.DisableSecOpt()[0]))
+		securityOpts = append(securityOpts, "label="+selinux.DisableSecOpt()[0])
 	}
 	return securityOpts
 }

common/pkg/filters/filters.go

@@ -2,6 +2,7 @@ package filters
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"path/filepath"
@@ -17,7 +18,7 @@ import (
 func ComputeUntilTimestamp(filterValues []string) (time.Time, error) {
 	invalid := time.Time{}
 	if len(filterValues) != 1 {
-		return invalid, fmt.Errorf("specify exactly one timestamp for until")
+		return invalid, errors.New("specify exactly one timestamp for until")
 	}
 	ts, err := timetype.GetTimestamp(filterValues[0], time.Now())
 	if err != nil {

common/pkg/flag/flag_test.go

@@ -57,7 +57,7 @@ func TestOptionalBoolSet(t *testing.T) {
 	OptionalBoolFlag(app.PersistentFlags(), &globalOB, "global-OB", "")
 	cmd := &cobra.Command{
 		Use: "cmd",
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(_ *cobra.Command, args []string) error {
 			assert.False(t, globalOB.Present())
 			assert.False(t, commandOB.Present())
 			actionRun = true
@@ -110,7 +110,7 @@ func TestOptionalBoolIsBoolFlag(t *testing.T) {
 		app := &cobra.Command{Use: "app"}
 		cmd := &cobra.Command{
 			Use: "cmd",
-			RunE: func(cmd *cobra.Command, args []string) error {
+			RunE: func(_ *cobra.Command, args []string) error {
 				assert.Equal(t, c.expectedOB, ob)     // nolint
 				assert.Equal(t, c.expectedArgs, args) //nolint
 				actionRun = true
@@ -149,7 +149,7 @@ func TestOptionalStringSet(t *testing.T) {
 	app.PersistentFlags().Var(NewOptionalStringValue(&globalOS), "global-OS", "")
 	cmd := &cobra.Command{
 		Use: "cmd",
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: func(_ *cobra.Command, args []string) error {
 			assert.False(t, globalOS.Present())
 			assert.False(t, commandOS.Present())
 			actionRun = true

common/pkg/formats/templates.go

@@ -44,7 +44,7 @@ var headerFunctions = template.FuncMap{
 	"upper": func(v string) string {
 		return v
 	},
-	"truncate": func(v string, l int) string {
+	"truncate": func(v string, _ int) string {
 		return v
 	},
 }

common/pkg/ssh/connection_golang.go

@@ -291,7 +291,7 @@ func ValidateAndConfigure(uri *url.URL, iden string, insecureIsMachineConnection
 	}
 	if len(authMethods) == 0 {
 		authMethods = append(authMethods, ssh.PasswordCallback(func() (string, error) {
-			pass, err := ReadPassword(fmt.Sprintf("%s's login password:", uri.User.Username()))
+			pass, err := ReadPassword(uri.User.Username() + "'s login password:")
 			return string(pass), err
 		}))
 	}

common/pkg/ssh/connection_native.go

@@ -35,7 +35,7 @@ func nativeConnectionCreate(options ConnectionCreateOptions) error {
 	// test connection
 	ssh, err := exec.LookPath("ssh")
 	if err != nil {
-		return fmt.Errorf("no ssh binary found")
+		return err
 	}
 
 	if strings.Contains(uri.Host, "/run") {
@@ -109,7 +109,7 @@ func nativeConnectionExec(options ConnectionExecOptions, input io.Reader) (*Conn
 
 	ssh, err := exec.LookPath("ssh")
 	if err != nil {
-		return nil, fmt.Errorf("no ssh binary found")
+		return nil, err
 	}
 
 	output := &bytes.Buffer{}
@@ -157,7 +157,7 @@ func nativeConnectionScp(options ConnectionScpOptions) (*ConnectionScpReport, er
 
 	scp, err := exec.LookPath("scp")
 	if err != nil {
-		return nil, fmt.Errorf("no scp binary found")
+		return nil, err
 	}
 
 	conf, err := config.Default()

common/pkg/ssh/ssh.go

@@ -1,7 +1,7 @@
 package ssh
 
 import (
-	"fmt"
+	"errors"
 	"io"
 
 	"golang.org/x/crypto/ssh"
@@ -18,7 +18,7 @@ func Dial(options *ConnectionDialOptions, kind EngineMode) (*ssh.Client, error)
 	var rep *ConnectionDialReport
 	var err error
 	if kind == NativeMode {
-		return nil, fmt.Errorf("ssh dial failed: you cannot create a dial-able client with native ssh")
+		return nil, errors.New("ssh dial failed: you cannot create a dial-able client with native ssh")
 	}
 	rep, err = golangConnectionDial(*options)
 	if err != nil {

common/pkg/ssh/utils.go

@@ -1,6 +1,7 @@
 package ssh
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -80,7 +81,7 @@ func ReadPassword(prompt string) (pw []byte, err error) {
 			pw = append(pw, b[0])
 			// limit size, so that a wrong input won't fill up the memory
 			if len(pw) > 1024 {
-				err = fmt.Errorf("password too long, 1024 byte limit")
+				err = errors.New("password too long, 1024 byte limit")
 			}
 		}
 		if err != nil {
@@ -156,7 +157,7 @@ func ParseScpArgs(options ConnectionScpOptions) (string, string, string, bool, e
 	} else {
 		split = strings.Split(host, ":")
 		if len(split) != 2 {
-			return "", "", "", false, fmt.Errorf("no remote destination provided")
+			return "", "", "", false, errors.New("no remote destination provided")
 		}
 		host = split[0]
 		remotePath = split[1]

common/pkg/subscriptions/subscriptions.go

@@ -212,7 +212,7 @@ func MountsWithUIDGID(mountLabel, containerRunDir, mountFile, mountPoint string,
 }
 
 func rchown(chowndir string, uid, gid int) error {
-	return filepath.Walk(chowndir, func(filePath string, f os.FileInfo, err error) error {
+	return filepath.Walk(chowndir, func(filePath string, _ os.FileInfo, err error) error {
 		return os.Lchown(filePath, uid, gid)
 	})
 }

common/pkg/supplemented/supplemented.go

@@ -3,6 +3,7 @@ package supplemented
 import (
 	"container/list"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 
@@ -286,7 +287,7 @@ func (s *supplementedImageReference) NewImageSource(ctx context.Context, sys *ty
 }
 
 func (s *supplementedImageReference) DeleteImage(_ context.Context, _ *types.SystemContext) error {
-	return fmt.Errorf("deletion of images not implemented")
+	return errors.New("deletion of images not implemented")
 }
 
 func (s *supplementedImageSource) Close() error {

common/pkg/umask/umask_test.go

@@ -46,7 +46,7 @@ func TestMkdirAllIgnoreUmask(t *testing.T) {
 		{
 			name:    "success no dir to create",
 			prepare: os.TempDir,
-			assert: func(dir string, err error) {
+			assert: func(_ string, err error) {
 				assert.NoError(t, err)
 			},
 		},
@@ -99,7 +99,7 @@ func TestWriteFileIgnoreUmask(t *testing.T) {
 				require.NoError(t, os.RemoveAll(path))
 				return filepath.Join(path, "foo")
 			},
-			assert: func(path string, err error) {
+			assert: func(_ string, err error) {
 				assert.Error(t, err)
 			},
 		},