From bba4bb8129863fe938a50bb422515ef101d17612 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 6 May 2022 13:29:50 +0200 Subject: [PATCH] enable unparam, exportloopref and revive linters unparam and exportloopref already work without changes. For revive I had to silence many naming issues. I decided to silence them instead of changing the name because I didn't want to break any code. Signed-off-by: Paul Holzinger --- .golangci.yml | 3 +++ libimage/load.go | 2 ++ libimage/runtime.go | 17 ++++++++--------- libnetwork/cni/config.go | 2 +- libnetwork/cni/run.go | 2 +- libnetwork/etchosts/hosts_test.go | 9 +++------ libnetwork/internal/util/create.go | 2 +- libnetwork/netavark/config.go | 2 +- libnetwork/network/interface.go | 3 +++ pkg/apparmor/apparmor_linux.go | 8 +++----- pkg/auth/auth.go | 6 +++--- pkg/config/config.go | 9 ++++++--- pkg/configmaps/configmaps.go | 6 ++++-- pkg/configmaps/configmapsdb.go | 3 +-- pkg/configmaps/filedriver/filedriver.go | 3 +-- pkg/filters/filters.go | 3 +++ pkg/machine/machine.go | 4 ++++ pkg/retry/retry.go | 5 +++++ pkg/seccomp/filter.go | 2 +- pkg/secrets/filedriver/filedriver.go | 3 +-- pkg/secrets/secrets.go | 9 +++++++-- pkg/secrets/secretsdb.go | 3 +-- pkg/sysinfo/nummem_linux.go | 2 ++ 23 files changed, 65 insertions(+), 43 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5ac0e7397..3ceb420fd 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -13,6 +13,9 @@ linters: enable: - dupl - gocritic + - unparam + - exportloopref + - revive linters-settings: errcheck: check-type-assertions: true diff --git a/libimage/load.go b/libimage/load.go index 4dfac7106..69ca06ad2 100644 --- a/libimage/load.go +++ b/libimage/load.go @@ -88,6 +88,8 @@ 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:") for _, err := range loadErrors { loadError = fmt.Errorf("%v\n * %v", loadError, err) diff --git a/libimage/runtime.go b/libimage/runtime.go index 974b50b50..368238511 100644 --- a/libimage/runtime.go +++ b/libimage/runtime.go @@ -224,16 +224,15 @@ func (r *Runtime) LookupImage(name string, options *LookupImageOptions) (*Image, } logrus.Debugf("Found image %q in local containers storage (%s)", name, storageRef.StringWithinTransport()) return r.storageToImage(img, storageRef), "", nil - } else { - // Docker compat: strip off the tag iff name is tagged and digested - // (e.g., fedora:latest@sha256...). In that case, the tag is stripped - // off and entirely ignored. The digest is the sole source of truth. - normalizedName, err := normalizeTaggedDigestedString(name) - if err != nil { - return nil, "", err - } - name = normalizedName } + // Docker compat: strip off the tag iff name is tagged and digested + // (e.g., fedora:latest@sha256...). In that case, the tag is stripped + // off and entirely ignored. The digest is the sole source of truth. + normalizedName, err := normalizeTaggedDigestedString(name) + if err != nil { + return nil, "", err + } + name = normalizedName byDigest := false originalName := name diff --git a/libnetwork/cni/config.go b/libnetwork/cni/config.go index c6967b600..f6954db05 100644 --- a/libnetwork/cni/config.go +++ b/libnetwork/cni/config.go @@ -96,7 +96,7 @@ func (n *cniNetwork) networkCreate(newNetwork *types.Network, defaultNet bool) ( newNetwork.ID = getNetworkIDFromName(newNetwork.Name) // when we do not have ipam we must disable dns - internalutil.IpamNoneDisableDns(newNetwork) + internalutil.IpamNoneDisableDNS(newNetwork) // FIXME: Should this be a hard error? if newNetwork.DNSEnabled && newNetwork.Internal && hasDNSNamePlugin(n.cniPluginDirs) { diff --git a/libnetwork/cni/run.go b/libnetwork/cni/run.go index c7fa86ed0..c5461d74c 100644 --- a/libnetwork/cni/run.go +++ b/libnetwork/cni/run.go @@ -106,7 +106,7 @@ func (n *cniNetwork) Setup(namespacePath string, options types.SetupOptions) (ma } // CNIResultToStatus convert the cni result to status block -// nolint:golint +// nolint:golint,revive func CNIResultToStatus(res cnitypes.Result) (types.StatusBlock, error) { result := types.StatusBlock{} cniResult, err := types040.GetResult(res) diff --git a/libnetwork/etchosts/hosts_test.go b/libnetwork/etchosts/hosts_test.go index 23a4682af..157b71934 100644 --- a/libnetwork/etchosts/hosts_test.go +++ b/libnetwork/etchosts/hosts_test.go @@ -296,9 +296,8 @@ func TestNew(t *testing.T) { if tt.wantErrString != "" { assert.ErrorContains(t, err, tt.wantErrString) return - } else { - assert.NoError(t, err, "New() failed") } + assert.NoError(t, err, "New() failed") content, err := ioutil.ReadFile(targetFile) assert.NoErrorf(t, err, "failed to read target host file: %v", err) @@ -362,9 +361,8 @@ func TestAdd(t *testing.T) { if tt.wantErrString != "" { assert.ErrorContains(t, err, tt.wantErrString) return - } else { - assert.NoError(t, err, "Add() failed") } + assert.NoError(t, err, "Add() failed") content, err := ioutil.ReadFile(hostFile) assert.NoErrorf(t, err, "failed to read host file: %v", err) @@ -459,9 +457,8 @@ func TestAddIfExists(t *testing.T) { if tt.wantErrString != "" { assert.ErrorContains(t, err, tt.wantErrString) return - } else { - assert.NoError(t, err, "AddIfExists() failed") } + assert.NoError(t, err, "AddIfExists() failed") content, err := ioutil.ReadFile(hostFile) assert.NoErrorf(t, err, "failed to read host file: %v", err) diff --git a/libnetwork/internal/util/create.go b/libnetwork/internal/util/create.go index c1a4bee75..d4d574065 100644 --- a/libnetwork/internal/util/create.go +++ b/libnetwork/internal/util/create.go @@ -41,7 +41,7 @@ func CommonNetworkCreate(n NetUtil, network *types.Network) error { return nil } -func IpamNoneDisableDns(network *types.Network) { +func IpamNoneDisableDNS(network *types.Network) { if network.IPAMOptions[types.Driver] == types.NoneIPAMDriver { logrus.Debugf("dns disabled for network %q because ipam driver is set to none", network.Name) network.DNSEnabled = false diff --git a/libnetwork/netavark/config.go b/libnetwork/netavark/config.go index 0bb0539b2..f2c72ab9e 100644 --- a/libnetwork/netavark/config.go +++ b/libnetwork/netavark/config.go @@ -121,7 +121,7 @@ func (n *netavarkNetwork) networkCreate(newNetwork *types.Network, defaultNet bo } // when we do not have ipam we must disable dns - internalutil.IpamNoneDisableDns(newNetwork) + internalutil.IpamNoneDisableDNS(newNetwork) // add gateway when not internal or dns enabled addGateway := !newNetwork.Internal || newNetwork.DNSEnabled diff --git a/libnetwork/network/interface.go b/libnetwork/network/interface.go index 893bdea2e..e70f096a4 100644 --- a/libnetwork/network/interface.go +++ b/libnetwork/network/interface.go @@ -46,6 +46,9 @@ const ( // 1. read ${graphroot}/defaultNetworkBackend // 2. find netavark binary (if not installed use CNI) // 3. check containers, images and CNI networks and if there are some we have an existing install and should continue to use CNI +// +// revive does not like the name because the package is already called network +//nolint:revive func NetworkBackend(store storage.Store, conf *config.Config, syslog bool) (types.NetworkBackend, types.ContainerNetwork, error) { backend := types.NetworkBackend(conf.Network.NetworkBackend) if backend == "" { diff --git a/pkg/apparmor/apparmor_linux.go b/pkg/apparmor/apparmor_linux.go index 35f79a1ad..1fd269255 100644 --- a/pkg/apparmor/apparmor_linux.go +++ b/pkg/apparmor/apparmor_linux.go @@ -251,19 +251,17 @@ func CheckProfileAndLoadDefault(name string) (string, error) { if unshare.IsRootless() { if name != "" { return "", errors.Wrapf(ErrApparmorRootless, "cannot load AppArmor profile %q", name) - } else { - logrus.Debug("Skipping loading default AppArmor profile (rootless mode)") - return "", nil } + logrus.Debug("Skipping loading default AppArmor profile (rootless mode)") + return "", nil } // Check if AppArmor is disabled and error out if a profile is to be set. if !runcaa.IsEnabled() { if name == "" { return "", nil - } else { - return "", errors.Errorf("profile %q specified but AppArmor is disabled on the host", name) } + return "", errors.Errorf("profile %q specified but AppArmor is disabled on the host", name) } if name == "" { diff --git a/pkg/auth/auth.go b/pkg/auth/auth.go index 6765c9e5b..188e06c12 100644 --- a/pkg/auth/auth.go +++ b/pkg/auth/auth.go @@ -26,8 +26,8 @@ func GetDefaultAuthFile() string { if authfile := os.Getenv("REGISTRY_AUTH_FILE"); authfile != "" { return authfile } - if auth_env := os.Getenv("DOCKER_CONFIG"); auth_env != "" { - return filepath.Join(auth_env, "config.json") + if authEnv := os.Getenv("DOCKER_CONFIG"); authEnv != "" { + return filepath.Join(authEnv, "config.json") } return "" } @@ -313,7 +313,7 @@ func Logout(systemContext *types.SystemContext, opts *LogoutOptions, args []stri fmt.Printf("Not logged into %s with current tool. Existing credentials were established via docker login. Please use docker logout instead.\n", key) return nil } - return errors.Errorf("Not logged into %s\n", key) + return errors.Errorf("not logged into %s", key) default: return errors.Wrapf(err, "logging out of %q", key) } diff --git a/pkg/config/config.go b/pkg/config/config.go index d362495e3..dcb072f10 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -550,6 +550,9 @@ type SecretConfig struct { } // ConfigMapConfig represents the "configmap" TOML config table +// +// revive does not like the name because the package is already called config +//nolint:revive type ConfigMapConfig struct { // Driver specifies the configmap driver to use. // Current valid value: @@ -1212,14 +1215,14 @@ func (c *Config) ActiveDestination() (uri, identity string, err error) { // FindHelperBinary will search the given binary name in the configured directories. // If searchPATH is set to true it will also search in $PATH. func (c *Config) FindHelperBinary(name string, searchPATH bool) (string, error) { - dir_list := c.Engine.HelperBinariesDir + dirList := c.Engine.HelperBinariesDir // If set, search this directory first. This is used in testing. if dir, found := os.LookupEnv("CONTAINERS_HELPER_BINARY_DIR"); found { - dir_list = append([]string{dir}, dir_list...) + dirList = append([]string{dir}, dirList...) } - for _, path := range dir_list { + for _, path := range dirList { fullpath := filepath.Join(path, name) if fi, err := os.Stat(fullpath); err == nil && fi.Mode().IsRegular() { return fullpath, nil diff --git a/pkg/configmaps/configmaps.go b/pkg/configmaps/configmaps.go index 91b0dbe0b..7c07395e1 100644 --- a/pkg/configmaps/configmaps.go +++ b/pkg/configmaps/configmaps.go @@ -79,6 +79,9 @@ type ConfigMap struct { // ConfigMapsDriver interfaces with the configMaps data store. // The driver stores the actual bytes of configMap data, as opposed to // the configMap metadata. +// +// revive does not like the name because the package is already called configmaps +//nolint:revive type ConfigMapsDriver interface { // List lists all configMap ids in the configMaps data store List() ([]string, error) @@ -272,9 +275,8 @@ func getDriver(name string, opts map[string]string) (ConfigMapsDriver, error) { if name == "file" { if path, ok := opts["path"]; ok { return filedriver.NewDriver(path) - } else { - return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver") } + return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver") } return nil, errInvalidDriver } diff --git a/pkg/configmaps/configmapsdb.go b/pkg/configmaps/configmapsdb.go index 7bc2b4495..ce1bbc7bd 100644 --- a/pkg/configmaps/configmapsdb.go +++ b/pkg/configmaps/configmapsdb.go @@ -31,9 +31,8 @@ func (s *ConfigMapManager) loadDB() error { // the db cache will show no entries anyway. // The file will be created later on a store() return nil - } else { - return err } + return err } // We check if the file has been modified after the last time it was loaded into the cache. diff --git a/pkg/configmaps/filedriver/filedriver.go b/pkg/configmaps/filedriver/filedriver.go index 87c227835..180df50b8 100644 --- a/pkg/configmaps/filedriver/filedriver.go +++ b/pkg/configmaps/filedriver/filedriver.go @@ -134,9 +134,8 @@ func (d *Driver) getAllData() (map[string][]byte, error) { if os.IsNotExist(err) { // the file will be created later on a store() return make(map[string][]byte), nil - } else { - return nil, err } + return nil, err } file, err := os.Open(d.configMapsDataFilePath) diff --git a/pkg/filters/filters.go b/pkg/filters/filters.go index e26e056ad..038fd5788 100644 --- a/pkg/filters/filters.go +++ b/pkg/filters/filters.go @@ -36,6 +36,9 @@ func ComputeUntilTimestamp(filterValues []string) (time.Time, error) { // // Please refer to https://github.com/containers/podman/issues/6899 for some // background. +// +// revive does not like the name because the package is already called filters +//nolint:revive func FiltersFromRequest(r *http.Request) ([]string, error) { var ( compatFilters map[string]map[string]bool diff --git a/pkg/machine/machine.go b/pkg/machine/machine.go index 465eeceaf..37e89a08e 100644 --- a/pkg/machine/machine.go +++ b/pkg/machine/machine.go @@ -9,6 +9,8 @@ import ( "github.com/sirupsen/logrus" ) +// TODO: change name to MachineMarker since package is already called machine +//nolint:revive type MachineMarker struct { Enabled bool Type string @@ -54,6 +56,8 @@ func IsPodmanMachine() bool { return GetMachineMarker().Enabled } +// TODO: change name to HostType since package is already called machine +//nolint:revive func MachineHostType() string { return GetMachineMarker().Type } diff --git a/pkg/retry/retry.go b/pkg/retry/retry.go index a9573e4e8..234fd3448 100644 --- a/pkg/retry/retry.go +++ b/pkg/retry/retry.go @@ -17,12 +17,17 @@ import ( ) // RetryOptions defines the option to retry +// revive does not like the name because the package is already called retry +//nolint:revive type RetryOptions struct { MaxRetry int // The number of times to possibly retry Delay time.Duration // The delay to use between retries, if set } // RetryIfNecessary retries the operation in exponential backoff with the retryOptions +// +// revive does not like the name because the package is already called retry +//nolint:revive func RetryIfNecessary(ctx context.Context, operation func() error, retryOptions *RetryOptions) error { err := operation() for attempt := 0; err != nil && isRetryable(err) && attempt < retryOptions.MaxRetry; attempt++ { diff --git a/pkg/seccomp/filter.go b/pkg/seccomp/filter.go index 5c278574c..609036c82 100644 --- a/pkg/seccomp/filter.go +++ b/pkg/seccomp/filter.go @@ -130,7 +130,7 @@ func matchSyscall(filter *libseccomp.ScmpFilter, call *Syscall) error { return errors.Wrapf(err, "create seccomp syscall condition for syscall %s", call.Name) } - argCounts[cond.Index] += 1 + argCounts[cond.Index]++ conditions = append(conditions, newCond) } diff --git a/pkg/secrets/filedriver/filedriver.go b/pkg/secrets/filedriver/filedriver.go index e0c275851..a162c2b6e 100644 --- a/pkg/secrets/filedriver/filedriver.go +++ b/pkg/secrets/filedriver/filedriver.go @@ -134,9 +134,8 @@ func (d *Driver) getAllData() (map[string][]byte, error) { if os.IsNotExist(err) { // the file will be created later on a store() return make(map[string][]byte), nil - } else { - return nil, err } + return nil, err } file, err := os.Open(d.secretsDataFilePath) diff --git a/pkg/secrets/secrets.go b/pkg/secrets/secrets.go index 4a04e2b2f..46d177eae 100644 --- a/pkg/secrets/secrets.go +++ b/pkg/secrets/secrets.go @@ -53,6 +53,9 @@ var secretsFile = "secrets.json" var secretNameRegexp = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9_.-]*$`) // SecretsManager holds information on handling secrets +// +// revive does not like the name because the package is already called secrets +//nolint:revive type SecretsManager struct { // secretsPath is the path to the db file where secrets are stored secretsDBPath string @@ -82,6 +85,9 @@ type Secret struct { // The driver stores the actual bytes of secret data, as opposed to // the secret metadata. // Currently only the unencrypted filedriver is implemented. +// +// revive does not like the name because the package is already called secrets +//nolint:revive type SecretsDriver interface { // List lists all secret ids in the secrets data store List() ([]string, error) @@ -276,9 +282,8 @@ func getDriver(name string, opts map[string]string) (SecretsDriver, error) { case "file": if path, ok := opts["path"]; ok { return filedriver.NewDriver(path) - } else { - return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver") } + return nil, errors.Wrap(errInvalidDriverOpt, "need path for filedriver") case "pass": return passdriver.NewDriver(opts) case "shell": diff --git a/pkg/secrets/secretsdb.go b/pkg/secrets/secretsdb.go index 4d2ca0fca..8d21aa754 100644 --- a/pkg/secrets/secretsdb.go +++ b/pkg/secrets/secretsdb.go @@ -31,9 +31,8 @@ func (s *SecretsManager) loadDB() error { // the db cache will show no entries anyway. // The file will be created later on a store() return nil - } else { - return err } + return err } // We check if the file has been modified after the last time it was loaded into the cache. diff --git a/pkg/sysinfo/nummem_linux.go b/pkg/sysinfo/nummem_linux.go index 859791e36..018c488be 100644 --- a/pkg/sysinfo/nummem_linux.go +++ b/pkg/sysinfo/nummem_linux.go @@ -12,6 +12,8 @@ import ( // NUMANodeCount queries the system for the count of Memory Nodes available // for use to this process. func NUMANodeCount() int { + // this is the correct flag name (not defined in the unix package) + //nolint:revive MPOL_F_MEMS_ALLOWED := (1 << 2) var mask [1024 / 64]uintptr _, _, err := unix.RawSyscall6(unix.SYS_GET_MEMPOLICY, 0, uintptr(unsafe.Pointer(&mask[0])), uintptr(len(mask)*8), 0, uintptr(MPOL_F_MEMS_ALLOWED), 0)