From a0ddd970abd232dad74c643f03ffb5527bbd4e52 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 14 Mar 2024 14:47:39 +0100 Subject: [PATCH] Enable and fix more linters Signed-off-by: Sascha Grunert --- .golangci.yml | 18 +++++++---- pkg/ocicni/ocicni.go | 4 +-- pkg/ocicni/ocicni_test.go | 66 +++++++++++++++++++-------------------- pkg/ocicni/types.go | 10 +++--- pkg/ocicni/types_unix.go | 5 +-- pkg/ocicni/util_linux.go | 2 +- 6 files changed, 55 insertions(+), 50 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8bd9f4b7..2be17149 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -25,12 +25,16 @@ linters: - exportloopref - forbidigo - forcetypeassert + - gci + - ginkgolinter - gocheckcompilerdirectives + - gochecknoglobals - gochecknoinits - gochecksumtype - goconst - gocritic - gocyclo + - godot - godox - gofmt - gofumpt @@ -54,6 +58,7 @@ linters: - misspell - musttag - nakedret + - nestif - nilerr - nilnil - noctx @@ -66,6 +71,7 @@ linters: - promlinter - protogetter - reassign + - revive - rowserrcheck - sloglint - spancheck @@ -90,26 +96,26 @@ linters: # - depguard # - exhaustruct # - funlen - # - gci - # - ginkgolinter - # - gochecknoglobals # - gocognit - # - godot # - goerr113 # - gomnd # - gosec # - ireturn # - lll - # - nestif # - nlreturn # - nonamedreturns - # - revive # - tagliatelle # - testpackage # - varnamelen # - wrapcheck # - wsl linters-settings: + revive: + rules: + - name: dot-imports + disabled: true + nestif: + min-complexity: 10 gocritic: enabled-checks: - appendAssign diff --git a/pkg/ocicni/ocicni.go b/pkg/ocicni/ocicni.go index ac96f530..6c7a457d 100644 --- a/pkg/ocicni/ocicni.go +++ b/pkg/ocicni/ocicni.go @@ -59,7 +59,7 @@ type cniNetwork struct { config *libcni.NetworkConfigList } -var errMissingDefaultNetwork = "No CNI configuration file in %s. Has your network provider started?" +const errMissingDefaultNetwork = "no CNI configuration file in %s. Has your network provider started?" type podLock struct { // Count of in-flight operations for this pod; when this reaches zero @@ -210,7 +210,7 @@ func InitCNINoInotify(defaultNetName, confDir, cacheDir string, binDirs ...strin return initCNI(nil, cacheDir, defaultNetName, confDir, false, binDirs...) } -// Internal function to allow faking out exec functions for testing +// Internal function to allow faking out exec functions for testing. func initCNI(exec cniinvoke.Exec, cacheDir, defaultNetName, confDir string, useInotify bool, binDirs ...string) (CNIPlugin, error) { if confDir == "" { confDir = DefaultConfDir diff --git a/pkg/ocicni/ocicni_test.go b/pkg/ocicni/ocicni_test.go index 4d7e6f06..95c3fbee 100644 --- a/pkg/ocicni/ocicni_test.go +++ b/pkg/ocicni/ocicni_test.go @@ -12,17 +12,15 @@ import ( "reflect" "strings" - "github.com/vishvananda/netlink" - "github.com/containernetworking/cni/libcni" "github.com/containernetworking/cni/pkg/types" cniv04 "github.com/containernetworking/cni/pkg/types/040" "github.com/containernetworking/cni/pkg/version" "github.com/containernetworking/plugins/pkg/ns" "github.com/containernetworking/plugins/pkg/testutils" - . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/vishvananda/netlink" ) func writeConfig(dir, fileName, netName, plugin, vers string) (conf, confPath string, err error) { @@ -88,7 +86,7 @@ func (f *fakeExec) addPlugin(expectedEnv []string, expectedConf string, result t }) } -// Ensure everything in needles is also present in haystack +// Ensure everything in needles is also present in haystack. func matchArray(needles, haystack []string) { Expect(len(needles)).To(BeNumerically("<=", len(haystack))) for _, e1 := range needles { @@ -170,7 +168,7 @@ func (f *fakeExec) ExecPlugin(ctx context.Context, pluginPath string, stdinData } func (f *fakeExec) FindInPath(plugin string, paths []string) (string, error) { - Expect(len(paths)).To(BeNumerically(">", 0)) + Expect(paths).ToNot(BeEmpty()) if f.failFind { return "", fmt.Errorf("failed to find plugin %q in path %s", plugin, paths) @@ -233,7 +231,7 @@ var _ = Describe("ocicni operations", func() { Expect(ok).To(BeTrue()) net := tmp.getDefaultNetwork() Expect(net.name).To(Equal("test")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) Expect(ocicni.Shutdown()).NotTo(HaveOccurred()) @@ -256,7 +254,7 @@ var _ = Describe("ocicni operations", func() { Expect(ok).To(BeTrue()) net := tmp.getDefaultNetwork() Expect(net.name).To(Equal("test")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) Expect(ocicni.Shutdown()).NotTo(HaveOccurred()) @@ -314,7 +312,7 @@ var _ = Describe("ocicni operations", func() { Expect(ok).To(BeTrue()) net := tmp.getDefaultNetwork() Expect(net.name).To(Equal("test")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) // If a CNI config file is updated, default network name should be reloaded real-time @@ -325,7 +323,7 @@ var _ = Describe("ocicni operations", func() { net = tmp.getDefaultNetwork() Expect(net.name).To(Equal("secondary")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("testplugin")) Expect(ocicni.Shutdown()).NotTo(HaveOccurred()) @@ -347,7 +345,7 @@ var _ = Describe("ocicni operations", func() { Expect(ok).To(BeTrue()) net := tmp.getDefaultNetwork() Expect(net.name).To(Equal("test")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) // If a CNI config file is updated, default network name should be reloaded real-time @@ -358,7 +356,7 @@ var _ = Describe("ocicni operations", func() { net = tmp.getDefaultNetwork() Expect(net.name).To(Equal("test")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("testplugin")) Expect(ocicni.Shutdown()).NotTo(HaveOccurred()) @@ -377,7 +375,7 @@ var _ = Describe("ocicni operations", func() { Expect(ok).To(BeTrue()) net := tmp.getDefaultNetwork() Expect(net.name).To(Equal("test")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) // Delete the default network config, ensure ocicni begins to @@ -410,7 +408,7 @@ var _ = Describe("ocicni operations", func() { Expect(ok).To(BeTrue()) net := tmp.getDefaultNetwork() Expect(net.name).To(Equal("test")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) // If a new CNI config file is added, default network name should be reloaded real-time @@ -422,7 +420,7 @@ var _ = Describe("ocicni operations", func() { net = tmp.getDefaultNetwork() Expect(net.name).To(Equal("newdefault")) - Expect(len(net.config.Plugins)).To(BeNumerically(">", 0)) + Expect(net.config.Plugins).ToNot(BeEmpty()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) Expect(ocicni.Shutdown()).NotTo(HaveOccurred()) @@ -442,7 +440,7 @@ var _ = Describe("ocicni operations", func() { cniConfig := libcni.NewCNIConfig([]string{"/opt/cni/bin"}, &fakeExec{}) netMap, defname, err := loadNetworks(context.TODO(), tmpDir, cniConfig) Expect(err).NotTo(HaveOccurred()) - Expect(len(netMap)).To(Equal(4)) + Expect(netMap).To(HaveLen(4)) // filenames are sorted asciibetically Expect(defname).To(Equal("network2")) }) @@ -451,7 +449,7 @@ var _ = Describe("ocicni operations", func() { cniConfig := libcni.NewCNIConfig([]string{"/opt/cni/bin"}, &fakeExec{}) netMap, defname, err := loadNetworks(context.TODO(), tmpDir, cniConfig) Expect(err).NotTo(HaveOccurred()) - Expect(len(netMap)).To(Equal(0)) + Expect(netMap).To(BeEmpty()) // filenames are sorted asciibetically Expect(defname).To(Equal("")) }) @@ -471,7 +469,7 @@ var _ = Describe("ocicni operations", func() { // We expect the type=myplugin2 network be ignored since it // was read earlier than the type=myplugin network with the same name - Expect(len(netMap)).To(Equal(2)) + Expect(netMap).To(HaveLen(2)) net, ok := netMap["network2"] Expect(ok).To(BeTrue()) Expect(net.config.Plugins[0].Network.Type).To(Equal("myplugin")) @@ -500,7 +498,7 @@ var _ = Describe("ocicni operations", func() { runtimeConfig = &RuntimeConfig{IP: "172.16.0.1"} rt, err = buildCNIRuntimeConf(podNetwork, ifName, runtimeConfig) Expect(err).NotTo(HaveOccurred()) - Expect(len(rt.Args)).To(Equal(6)) + Expect(rt.Args).To(HaveLen(6)) Expect(rt.Args[5][1]).To(Equal("172.16.0.1")) // runtimeConfig with invalid MAC @@ -512,14 +510,14 @@ var _ = Describe("ocicni operations", func() { runtimeConfig = &RuntimeConfig{MAC: "9e:0c:d9:b2:f0:a6"} rt, err = buildCNIRuntimeConf(podNetwork, ifName, runtimeConfig) Expect(err).NotTo(HaveOccurred()) - Expect(len(rt.Args)).To(Equal(6)) + Expect(rt.Args).To(HaveLen(6)) Expect(rt.Args[5][1]).To(Equal("9e:0c:d9:b2:f0:a6")) // runtimeConfig with valid IP and valid MAC runtimeConfig = &RuntimeConfig{IP: "172.16.0.1", MAC: "9e:0c:d9:b2:f0:a6"} rt, err = buildCNIRuntimeConf(podNetwork, ifName, runtimeConfig) Expect(err).NotTo(HaveOccurred()) - Expect(len(rt.Args)).To(Equal(7)) + Expect(rt.Args).To(HaveLen(7)) Expect(rt.Args[5][1]).To(Equal("172.16.0.1")) Expect(rt.Args[6][1]).To(Equal("9e:0c:d9:b2:f0:a6")) @@ -538,8 +536,8 @@ var _ = Describe("ocicni operations", func() { rt, err = buildCNIRuntimeConf(podNetwork, ifName, runtimeConfig) Expect(err).NotTo(HaveOccurred()) pm, ok := rt.CapabilityArgs["portMappings"].([]PortMapping) - Expect(ok).To(Equal(true)) - Expect(len(pm)).To(Equal(1)) + Expect(ok).To(BeTrue()) + Expect(pm).To(HaveLen(1)) Expect(pm[0].HostPort).To(Equal(int32(100))) Expect(pm[0].ContainerPort).To(Equal(int32(50))) Expect(pm[0].Protocol).To(Equal("tcp")) @@ -560,7 +558,7 @@ var _ = Describe("ocicni operations", func() { rt, err = buildCNIRuntimeConf(podNetwork, ifName, runtimeConfig) Expect(err).NotTo(HaveOccurred()) bw, ok := rt.CapabilityArgs["bandwidth"].(map[string]uint64) - Expect(ok).To(Equal(true)) + Expect(ok).To(BeTrue()) Expect(bw["ingressRate"]).To(Equal(uint64(1))) Expect(bw["ingressBurst"]).To(Equal(uint64(2))) Expect(bw["egressRate"]).To(Equal(uint64(3))) @@ -581,16 +579,16 @@ var _ = Describe("ocicni operations", func() { rt, err = buildCNIRuntimeConf(podNetwork, ifName, runtimeConfig) Expect(err).NotTo(HaveOccurred()) ir, ok := rt.CapabilityArgs["ipRanges"].([][]IpRange) - Expect(ok).To(Equal(true)) - Expect(len(ir)).To(Equal(1)) - Expect(len(ir[0])).To(Equal(1)) + Expect(ok).To(BeTrue()) + Expect(ir).To(HaveLen(1)) + Expect(ir[0]).To(HaveLen(1)) Expect(ir[0][0].Gateway).To(Equal("192.168.0.254")) runtimeConfig = &RuntimeConfig{CgroupPath: "/slice/pod/testing"} rt, err = buildCNIRuntimeConf(podNetwork, ifName, runtimeConfig) Expect(err).NotTo(HaveOccurred()) cg, ok := rt.CapabilityArgs["cgroupPath"].(string) - Expect(ok).To(Equal(true)) + Expect(ok).To(BeTrue()) Expect(cg).To(Equal("/slice/pod/testing")) }) @@ -631,7 +629,7 @@ var _ = Describe("ocicni operations", func() { results, err := ocicni.SetUpPod(podNet) Expect(err).NotTo(HaveOccurred()) Expect(fake.addIndex).To(Equal(len(fake.plugins))) - Expect(len(results)).To(Equal(1)) + Expect(results).To(HaveLen(1)) r, ok := results[0].Result.(*cniv04.Result) Expect(ok).To(BeTrue()) Expect(reflect.DeepEqual(r, expectedResult)).To(BeTrue()) @@ -717,7 +715,7 @@ var _ = Describe("ocicni operations", func() { results, err := ocicni.SetUpPod(podNet) Expect(err).NotTo(HaveOccurred()) Expect(fake.addIndex).To(Equal(len(fake.plugins))) - Expect(len(results)).To(Equal(2)) + Expect(results).To(HaveLen(2)) r, ok := results[0].Result.(*cniv04.Result) Expect(ok).To(BeTrue()) Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) @@ -797,7 +795,7 @@ var _ = Describe("ocicni operations", func() { results, err := ocicni.SetUpPod(podNet) Expect(err).NotTo(HaveOccurred()) Expect(fake.addIndex).To(Equal(len(fake.plugins))) - Expect(len(results)).To(Equal(2)) + Expect(results).To(HaveLen(2)) r, ok := results[0].Result.(*cniv04.Result) Expect(ok).To(BeTrue()) Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) @@ -807,7 +805,7 @@ var _ = Describe("ocicni operations", func() { resultsStatus, errStatus := ocicni.GetPodNetworkStatus(podNet) Expect(errStatus).NotTo(HaveOccurred()) - Expect(len(resultsStatus)).To(Equal(2)) + Expect(resultsStatus).To(HaveLen(2)) r, ok = resultsStatus[0].Result.(*cniv04.Result) Expect(ok).To(BeTrue()) Expect(reflect.DeepEqual(r, expectedResult1)).To(BeTrue()) @@ -897,16 +895,16 @@ var _ = Describe("ocicni operations", func() { podNet.Networks = []NetAttachment{} tmp, ok := ocicni.(*cniNetworkPlugin) Expect(ok).To(BeTrue()) - Expect(len(tmp.pods)).To(Equal(0)) + Expect(tmp.pods).To(BeEmpty()) tmp.podLock(&podNet) - Expect(len(tmp.pods)).To(Equal(1)) + Expect(tmp.pods).To(HaveLen(1)) }) It("verifies that network operations can be unlocked for a pod using cached networks", func() { podNet.Networks = []NetAttachment{} tmp, ok := ocicni.(*cniNetworkPlugin) Expect(ok).To(BeTrue()) tmp.podUnlock(&podNet) - Expect(len(tmp.pods)).To(Equal(0)) + Expect(tmp.pods).To(BeEmpty()) }) }) diff --git a/pkg/ocicni/types.go b/pkg/ocicni/types.go index 39800855..96db061e 100644 --- a/pkg/ocicni/types.go +++ b/pkg/ocicni/types.go @@ -7,9 +7,9 @@ import ( ) const ( - // DefaultInterfaceName is the string to be used for the interface name inside the net namespace + // DefaultInterfaceName is the string to be used for the interface name inside the net namespace. DefaultInterfaceName = "eth0" - // CNIPluginName is the default name of the plugin + // CNIPluginName is the default name of the plugin. CNIPluginName = "cni" ) @@ -106,7 +106,7 @@ type PodNetwork struct { Aliases map[string][]string } -// NetAttachment describes a container network attachment +// NetAttachment describes a container network attachment. type NetAttachment struct { // NetName contains the name of the CNI network to which the container // should be or is attached @@ -115,7 +115,7 @@ type NetAttachment struct { Ifname string } -// NetResult contains the result the network attachment operation +// NetResult contains the result the network attachment operation. type NetResult struct { // Result is the CNI Result Result types.Result @@ -124,7 +124,7 @@ type NetResult struct { NetAttachment } -// CNIPlugin is the interface that needs to be implemented by a plugin +// CNIPlugin is the interface that needs to be implemented by a plugin. type CNIPlugin interface { // Name returns the plugin's name. This will be used when searching // for a plugin by name, e.g. diff --git a/pkg/ocicni/types_unix.go b/pkg/ocicni/types_unix.go index e8208263..ab87105a 100644 --- a/pkg/ocicni/types_unix.go +++ b/pkg/ocicni/types_unix.go @@ -4,8 +4,9 @@ package ocicni const ( - // DefaultConfDir is the default place to look for CNI Network + // DefaultConfDir is the default place to look for CNI Network. DefaultConfDir = "/etc/cni/net.d" - // DefaultBinDir is the default place to look for CNI config files + + // DefaultBinDir is the default place to look for CNI config files. DefaultBinDir = "/opt/cni/bin" ) diff --git a/pkg/ocicni/util_linux.go b/pkg/ocicni/util_linux.go index 295d5945..9c73bab5 100644 --- a/pkg/ocicni/util_linux.go +++ b/pkg/ocicni/util_linux.go @@ -14,7 +14,7 @@ import ( "github.com/vishvananda/netlink" ) -var defaultNamespaceEnterCommandName = "nsenter" +const defaultNamespaceEnterCommandName = "nsenter" type nsManager struct { nsenterPath string