Merge pull request #13997 from Luap99/gocritic

enable gocritic linter

16 files changed, 68 insertions, 62 deletions

diff --git a/libpod/container.go b/libpod/container.go
index 6e2b7f528..3e7ab7b0a 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -1180,7 +1180,7 @@ func (c *Container) Umask() string {
 	return c.config.Umask
 }
 
-//Secrets return the secrets in the container
+// Secrets return the secrets in the container
 func (c *Container) Secrets() []*ContainerSecret {
 	return c.config.Secrets
 }
diff --git a/libpod/container_api.go b/libpod/container_api.go
index fe41998c0..a6fcf709d 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -229,8 +229,7 @@ func (c *Container) Kill(signal uint) error {
 // This function returns when the attach finishes. It does not hold the lock for
 // the duration of its runtime, only using it at the beginning to verify state.
 func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-chan define.TerminalSize) error {
-	switch c.LogDriver() {
-	case define.PassthroughLogging:
+	if c.LogDriver() == define.PassthroughLogging {
 		return errors.Wrapf(define.ErrNoLogs, "this container is using the 'passthrough' log driver, cannot attach")
 	}
 	if !c.batched {
diff --git a/libpod/container_exec.go b/libpod/container_exec.go
index d782bebf8..c05e7fd94 100644
--- a/libpod/container_exec.go
+++ b/libpod/container_exec.go
@@ -817,16 +817,16 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi
 // Please be careful when using this function since it might temporarily unlock
 // the container when os.RemoveAll($bundlePath) fails with ENOTEMPTY or EBUSY
 // errors.
-func (c *Container) cleanupExecBundle(sessionID string) (Err error) {
+func (c *Container) cleanupExecBundle(sessionID string) (err error) {
 	path := c.execBundlePath(sessionID)
 	for attempts := 0; attempts < 50; attempts++ {
-		Err = os.RemoveAll(path)
-		if Err == nil || os.IsNotExist(Err) {
+		err = os.RemoveAll(path)
+		if err == nil || os.IsNotExist(err) {
 			return nil
 		}
-		if pathErr, ok := Err.(*os.PathError); ok {
-			Err = pathErr.Err
-			if errors.Cause(Err) == unix.ENOTEMPTY || errors.Cause(Err) == unix.EBUSY {
+		if pathErr, ok := err.(*os.PathError); ok {
+			err = pathErr.Err
+			if errors.Cause(err) == unix.ENOTEMPTY || errors.Cause(err) == unix.EBUSY {
 				// give other processes a chance to use the container
 				if !c.batched {
 					if err := c.save(); err != nil {
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 9f8b7c686..31edff762 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -505,8 +505,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 			}
 
 			for _, o := range namedVol.Options {
-				switch o {
-				case "U":
+				if o == "U" {
 					if err := c.ChangeHostPathOwnership(mountPoint, true, int(hostUID), int(hostGID)); err != nil {
 						return nil, err
 					}
@@ -596,8 +595,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 
 		// Check overlay volume options
 		for _, o := range overlayVol.Options {
-			switch o {
-			case "U":
+			if o == "U" {
 				if err := c.ChangeHostPathOwnership(overlayVol.Source, true, int(hostUID), int(hostGID)); err != nil {
 					return nil, err
 				}
@@ -2144,11 +2142,9 @@ func (c *Container) makeBindMounts() error {
 				return err
 			}
 		}
-	} else {
-		if !c.config.UseImageHosts && c.state.BindMounts["/etc/hosts"] == "" {
-			if err := c.createHosts(); err != nil {
-				return errors.Wrapf(err, "error creating hosts file for container %s", c.ID())
-			}
+	} else if !c.config.UseImageHosts && c.state.BindMounts["/etc/hosts"] == "" {
+		if err := c.createHosts(); err != nil {
+			return errors.Wrapf(err, "error creating hosts file for container %s", c.ID())
 		}
 	}
 
@@ -2267,7 +2263,7 @@ rootless=%d
 			base := "/run/secrets"
 			if secret.Target != "" {
 				secretFileName = secret.Target
-				//If absolute path for target given remove base.
+				// If absolute path for target given remove base.
 				if filepath.IsAbs(secretFileName) {
 					base = ""
 				}
@@ -2351,7 +2347,7 @@ func (c *Container) generateResolvConf() (string, error) {
 		return "", errors.Wrapf(err, "error parsing host resolv.conf")
 	}
 
-	dns := make([]net.IP, 0, len(c.runtime.config.Containers.DNSServers))
+	dns := make([]net.IP, 0, len(c.runtime.config.Containers.DNSServers)+len(c.config.DNSServer))
 	for _, i := range c.runtime.config.Containers.DNSServers {
 		result := net.ParseIP(i)
 		if result == nil {
@@ -2359,13 +2355,13 @@ func (c *Container) generateResolvConf() (string, error) {
 		}
 		dns = append(dns, result)
 	}
-	dnsServers := append(dns, c.config.DNSServer...)
+	dns = append(dns, c.config.DNSServer...)
 	// If the user provided dns, it trumps all; then dns masq; then resolv.conf
 	var search []string
 	switch {
-	case len(dnsServers) > 0:
+	case len(dns) > 0:
 		// We store DNS servers as net.IP, so need to convert to string
-		for _, server := range dnsServers {
+		for _, server := range dns {
 			nameservers = append(nameservers, server.String())
 		}
 	default:
@@ -2890,11 +2886,11 @@ func (c *Container) generateUserPasswdEntry(addedUID int) (string, error) {
 
 func (c *Container) passwdEntry(username string, uid, gid, name, homeDir string) string {
 	s := c.config.PasswdEntry
-	s = strings.Replace(s, "$USERNAME", username, -1)
-	s = strings.Replace(s, "$UID", uid, -1)
-	s = strings.Replace(s, "$GID", gid, -1)
-	s = strings.Replace(s, "$NAME", name, -1)
-	s = strings.Replace(s, "$HOME", homeDir, -1)
+	s = strings.ReplaceAll(s, "$USERNAME", username)
+	s = strings.ReplaceAll(s, "$UID", uid)
+	s = strings.ReplaceAll(s, "$GID", gid)
+	s = strings.ReplaceAll(s, "$NAME", name)
+	s = strings.ReplaceAll(s, "$HOME", homeDir)
 	return s + "\n"
 }
 
diff --git a/libpod/container_stat_linux.go b/libpod/container_stat_linux.go
index 9e225de2e..bbe3edbb3 100644
--- a/libpod/container_stat_linux.go
+++ b/libpod/container_stat_linux.go
@@ -94,15 +94,16 @@ func (c *Container) stat(containerMountPoint string, containerPath string) (*def
 		}
 	}
 
-	if statInfo.IsSymlink {
+	switch {
+	case statInfo.IsSymlink:
 		// Symlinks are already evaluated and always relative to the
 		// container's mount point.
 		absContainerPath = statInfo.ImmediateTarget
-	} else if strings.HasPrefix(resolvedPath, containerMountPoint) {
+	case strings.HasPrefix(resolvedPath, containerMountPoint):
 		// If the path is on the container's mount point, strip it off.
 		absContainerPath = strings.TrimPrefix(resolvedPath, containerMountPoint)
 		absContainerPath = filepath.Join("/", absContainerPath)
-	} else {
+	default:
 		// No symlink and not on the container's mount point, so let's
 		// move it back to the original input.  It must have evaluated
 		// to a volume or bind mount but we cannot return host paths.
diff --git a/libpod/define/container_inspect.go b/libpod/define/container_inspect.go
index ae2ce9724..6cdffb8b7 100644
--- a/libpod/define/container_inspect.go
+++ b/libpod/define/container_inspect.go
@@ -100,7 +100,7 @@ type InspectRestartPolicy struct {
 // InspectLogConfig holds information about a container's configured log driver
 type InspectLogConfig struct {
 	Type   string            `json:"Type"`
-	Config map[string]string `json:"Config"` //idk type, TODO
+	Config map[string]string `json:"Config"`
 	// Path specifies a path to the log file
 	Path string `json:"Path"`
 	// Tag specifies a custom log tag for the container
@@ -680,7 +680,7 @@ type InspectContainerData struct {
 	SizeRootFs      int64                       `json:"SizeRootFs,omitempty"`
 	Mounts          []InspectMount              `json:"Mounts"`
 	Dependencies    []string                    `json:"Dependencies"`
-	NetworkSettings *InspectNetworkSettings     `json:"NetworkSettings"` //TODO
+	NetworkSettings *InspectNetworkSettings     `json:"NetworkSettings"`
 	Namespace       string                      `json:"Namespace"`
 	IsInfra         bool                        `json:"IsInfra"`
 	Config          *InspectContainerConfig     `json:"Config"`
diff --git a/libpod/define/info.go b/libpod/define/info.go
index 48ad51c22..713129ada 100644
--- a/libpod/define/info.go
+++ b/libpod/define/info.go
@@ -12,7 +12,7 @@ type Info struct {
 	Version    Version                `json:"version"`
 }
 
-//HostInfo describes the libpod host
+// HostInfo describes the libpod host
 type SecurityInfo struct {
 	AppArmorEnabled     bool   `json:"apparmorEnabled"`
 	DefaultCapabilities string `json:"capabilities"`
diff --git a/libpod/kube.go b/libpod/kube.go
index eb62643fe..cbb1a8799 100644
--- a/libpod/kube.go
+++ b/libpod/kube.go
@@ -220,7 +220,7 @@ func ConvertV1PodToYAMLPod(pod *v1.Pod) *YAMLPod {
 		cs = append(cs, &YAMLContainer{Container: cc, Resources: res})
 	}
 	mpo := &YAMLPod{Pod: *pod}
-	mpo.Spec = &YAMLPodSpec{PodSpec: (*pod).Spec, Containers: cs}
+	mpo.Spec = &YAMLPodSpec{PodSpec: pod.Spec, Containers: cs}
 	for _, ctr := range pod.Spec.Containers {
 		if ctr.SecurityContext == nil || ctr.SecurityContext.SELinuxOptions == nil {
 			continue
@@ -527,7 +527,7 @@ func simplePodWithV1Containers(ctx context.Context, ctrs []*Container) (*v1.Pod,
 	// Check if the pod name and container name will end up conflicting
 	// Append -pod if so
 	if util.StringInSlice(podName, ctrNames) {
-		podName = podName + "-pod"
+		podName += "-pod"
 	}
 
 	return newPodObject(
@@ -633,7 +633,7 @@ func containerToV1Container(ctx context.Context, c *Container) (v1.Container, []
 
 	kubeContainer.Ports = ports
 	// This should not be applicable
-	//container.EnvFromSource =
+	// container.EnvFromSource =
 	kubeContainer.SecurityContext = kubeSec
 	kubeContainer.StdinOnce = false
 	kubeContainer.TTY = c.config.Spec.Process.Terminal
@@ -885,7 +885,7 @@ func convertVolumePathToName(hostSourcePath string) (string, error) {
 	}
 	// First, trim trailing slashes, then replace slashes with dashes.
 	// Thus, /mnt/data/ will become mnt-data
-	return strings.Replace(strings.Trim(hostSourcePath, "/"), "/", "-", -1), nil
+	return strings.ReplaceAll(strings.Trim(hostSourcePath, "/"), "/", "-"), nil
 }
 
 func determineCapAddDropFromCapabilities(defaultCaps, containerCaps []string) *v1.Capabilities {
@@ -927,14 +927,20 @@ func capAddDrop(caps *specs.LinuxCapabilities) (*v1.Capabilities, error) {
 	if err != nil {
 		return nil, err
 	}
+
+	defCaps := g.Config.Process.Capabilities
 	// Combine all the default capabilities into a slice
-	defaultCaps := append(g.Config.Process.Capabilities.Ambient, g.Config.Process.Capabilities.Bounding...)
-	defaultCaps = append(defaultCaps, g.Config.Process.Capabilities.Effective...)
-	defaultCaps = append(defaultCaps, g.Config.Process.Capabilities.Inheritable...)
-	defaultCaps = append(defaultCaps, g.Config.Process.Capabilities.Permitted...)
+	defaultCaps := make([]string, 0, len(defCaps.Ambient)+len(defCaps.Bounding)+len(defCaps.Effective)+len(defCaps.Inheritable)+len(defCaps.Permitted))
+	defaultCaps = append(defaultCaps, defCaps.Ambient...)
+	defaultCaps = append(defaultCaps, defCaps.Bounding...)
+	defaultCaps = append(defaultCaps, defCaps.Effective...)
+	defaultCaps = append(defaultCaps, defCaps.Inheritable...)
+	defaultCaps = append(defaultCaps, defCaps.Permitted...)
 
 	// Combine all the container's capabilities into a slice
-	containerCaps := append(caps.Ambient, caps.Bounding...)
+	containerCaps := make([]string, 0, len(caps.Ambient)+len(caps.Bounding)+len(caps.Effective)+len(caps.Inheritable)+len(caps.Permitted))
+	containerCaps = append(containerCaps, caps.Ambient...)
+	containerCaps = append(containerCaps, caps.Bounding...)
 	containerCaps = append(containerCaps, caps.Effective...)
 	containerCaps = append(containerCaps, caps.Inheritable...)
 	containerCaps = append(containerCaps, caps.Permitted...)
@@ -1042,7 +1048,7 @@ func generateKubeVolumeDeviceFromLinuxDevice(devices []specs.LinuxDevice) []v1.V
 }
 
 func removeUnderscores(s string) string {
-	return strings.Replace(s, "_", "", -1)
+	return strings.ReplaceAll(s, "_", "")
 }
 
 // getAutoUpdateAnnotations searches for auto-update container labels
diff --git a/libpod/logs/log.go b/libpod/logs/log.go
index 0eb3bb922..4d7d5ac58 100644
--- a/libpod/logs/log.go
+++ b/libpod/logs/log.go
@@ -28,7 +28,7 @@ const (
 	// FullLogType signifies a log line is full
 	FullLogType = "F"
 
-	//ANSIEscapeResetCode is a code that resets all colors and text effects
+	// ANSIEscapeResetCode is a code that resets all colors and text effects
 	ANSIEscapeResetCode = "\033[0m"
 )
 
@@ -167,7 +167,7 @@ func getTailLog(path string, tail int) ([]*LogLine, error) {
 	return tailLog, nil
 }
 
-//getColor returns a ANSI escape code for color based on the colorID
+// getColor returns a ANSI escape code for color based on the colorID
 func getColor(colorID int64) string {
 	colors := map[int64]string{
 		0: "\033[37m", // Light Gray
diff --git a/libpod/networking_linux.go b/libpod/networking_linux.go
index a312f5a0c..2770b040e 100644
--- a/libpod/networking_linux.go
+++ b/libpod/networking_linux.go
@@ -579,7 +579,7 @@ func (r *Runtime) GetRootlessNetNs(new bool) (*RootlessNetNS, error) {
 	// lets add /usr/sbin to $PATH ourselves.
 	path = os.Getenv("PATH")
 	if !strings.Contains(path, "/usr/sbin") {
-		path = path + ":/usr/sbin"
+		path += ":/usr/sbin"
 		os.Setenv("PATH", path)
 	}
 
@@ -1148,7 +1148,7 @@ func resultToBasicNetworkConfig(result types.StatusBlock) define.InspectBasicNet
 		for _, netAddress := range netInt.Subnets {
 			size, _ := netAddress.IPNet.Mask.Size()
 			if netAddress.IPNet.IP.To4() != nil {
-				//ipv4
+				// ipv4
 				if config.IPAddress == "" {
 					config.IPAddress = netAddress.IPNet.IP.String()
 					config.IPPrefixLen = size
@@ -1157,7 +1157,7 @@ func resultToBasicNetworkConfig(result types.StatusBlock) define.InspectBasicNet
 					config.SecondaryIPAddresses = append(config.SecondaryIPAddresses, define.Address{Addr: netAddress.IPNet.IP.String(), PrefixLength: size})
 				}
 			} else {
-				//ipv6
+				// ipv6
 				if config.GlobalIPv6Address == "" {
 					config.GlobalIPv6Address = netAddress.IPNet.IP.String()
 					config.GlobalIPv6PrefixLen = size
@@ -1508,7 +1508,7 @@ func ocicniPortsToNetTypesPorts(ports []types.OCICNIPortMapping) []types.PortMap
 			ports[i].Protocol == currentPort.Protocol &&
 			ports[i].HostPort-int32(currentPort.Range) == int32(currentPort.HostPort) &&
 			ports[i].ContainerPort-int32(currentPort.Range) == int32(currentPort.ContainerPort) {
-			currentPort.Range = currentPort.Range + 1
+			currentPort.Range++
 		} else {
 			newPorts = append(newPorts, currentPort)
 			currentPort = types.PortMapping{
diff --git a/libpod/networking_machine.go b/libpod/networking_machine.go
index 73089c474..7b8eb94df 100644
--- a/libpod/networking_machine.go
+++ b/libpod/networking_machine.go
@@ -33,9 +33,9 @@ type machineExpose struct {
 func requestMachinePorts(expose bool, ports []types.PortMapping) error {
 	url := "http://" + machineGvproxyEndpoint + "/services/forwarder/"
 	if expose {
-		url = url + "expose"
+		url += "expose"
 	} else {
-		url = url + "unexpose"
+		url += "unexpose"
 	}
 	ctx := context.Background()
 	client := &http.Client{
diff --git a/libpod/networking_slirp4netns.go b/libpod/networking_slirp4netns.go
index 4a0ef0b3a..788834435 100644
--- a/libpod/networking_slirp4netns.go
+++ b/libpod/networking_slirp4netns.go
@@ -82,7 +82,9 @@ func checkSlirpFlags(path string) (*slirpFeatures, error) {
 }
 
 func parseSlirp4netnsNetworkOptions(r *Runtime, extraOptions []string) (*slirp4netnsNetworkOptions, error) {
-	slirpOptions := append(r.config.Engine.NetworkCmdOptions, extraOptions...)
+	slirpOptions := make([]string, 0, len(r.config.Engine.NetworkCmdOptions)+len(extraOptions))
+	slirpOptions = append(slirpOptions, r.config.Engine.NetworkCmdOptions...)
+	slirpOptions = append(slirpOptions, extraOptions...)
 	slirp4netnsOpts := &slirp4netnsNetworkOptions{
 		// overwrite defaults
 		disableHostLoopback: true,
diff --git a/libpod/oci_conmon_exec_linux.go b/libpod/oci_conmon_exec_linux.go
index b8fd82591..6d2f13525 100644
--- a/libpod/oci_conmon_exec_linux.go
+++ b/libpod/oci_conmon_exec_linux.go
@@ -766,13 +766,11 @@ func prepareProcessExec(c *Container, options *ExecOptions, env []string, sessio
 	if execUser.Uid == 0 {
 		pspec.Capabilities.Effective = pspec.Capabilities.Bounding
 		pspec.Capabilities.Permitted = pspec.Capabilities.Bounding
-	} else {
-		if user == c.config.User {
-			pspec.Capabilities.Effective = ctrSpec.Process.Capabilities.Effective
-			pspec.Capabilities.Inheritable = ctrSpec.Process.Capabilities.Effective
-			pspec.Capabilities.Permitted = ctrSpec.Process.Capabilities.Effective
-			pspec.Capabilities.Ambient = ctrSpec.Process.Capabilities.Effective
-		}
+	} else if user == c.config.User {
+		pspec.Capabilities.Effective = ctrSpec.Process.Capabilities.Effective
+		pspec.Capabilities.Inheritable = ctrSpec.Process.Capabilities.Effective
+		pspec.Capabilities.Permitted = ctrSpec.Process.Capabilities.Effective
+		pspec.Capabilities.Ambient = ctrSpec.Process.Capabilities.Effective
 	}
 
 	hasHomeSet := false
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index dc1c75cea..c232702e9 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -1371,7 +1371,7 @@ func (r *ConmonOCIRuntime) sharedConmonArgs(ctr *Container, cuuid, bundlePath, p
 	case define.JSONLogging:
 		fallthrough
 	//lint:ignore ST1015 the default case has to be here
-	default: //nolint:stylecheck
+	default: //nolint:stylecheck,gocritic
 		// No case here should happen except JSONLogging, but keep this here in case the options are extended
 		logrus.Errorf("%s logging specified but not supported. Choosing k8s-file logging instead", ctr.LogDriver())
 		fallthrough
diff --git a/libpod/options.go b/libpod/options.go
index 4e597201a..57e2d7cf6 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -1662,7 +1662,7 @@ func WithTimezone(path string) CtrCreateOption {
 			if err != nil {
 				return err
 			}
-			//We don't want to mount a timezone directory
+			// We don't want to mount a timezone directory
 			if file.IsDir() {
 				return errors.New("Invalid timezone: is a directory")
 			}
diff --git a/libpod/runtime.go b/libpod/runtime.go
index 877e151a9..6c2323d88 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -550,6 +550,10 @@ func makeRuntime(runtime *Runtime) (retErr error) {
 				// Check if the pause process was created.  If it was created, then
 				// move it to its own systemd scope.
 				utils.MovePauseProcessToScope(pausePid)
+
+				// gocritic complains because defer is not run on os.Exit()
+				// However this is fine because the lock is released anyway when the process exits
+				//nolint:gocritic
 				os.Exit(ret)
 			}
 		}