Merge pull request #883 from mmorel-35/linter-2

enable govet and unparam linters

.golangci.yml

@@ -8,6 +8,8 @@ issues:
       text: " and that stutters;"
 
 linters:
+  disable:
+    - errcheck
   enable:
     - contextcheck
     - durationcheck
@@ -16,6 +18,7 @@ linters:
     - gocritic
     - gofumpt
     - gosimple
+    - govet
     - ineffassign
     - misspell
     - nonamedreturns
@@ -23,10 +26,9 @@ linters:
     - revive
     - staticcheck
     - unconvert
+    - unparam
     - unused
     - wastedassign
-  disable:
-    - errcheck
 
 linters-settings:
   gci:

integration/integration_linux_test.go

@@ -161,15 +161,12 @@ var _ = Describe("Basic PTP using cnitool", func() {
 			Expect(basicBridgeIP).To(ContainSubstring("10.11.2."))
 
 			var chainedBridgeBandwidthPort, basicBridgePort int
-			var err error
 
 			By(fmt.Sprintf("starting echo server in %s\n\n", contNS1.ShortName()))
-			chainedBridgeBandwidthPort, chainedBridgeBandwidthSession, err = startEchoServerInNamespace(contNS1)
+			chainedBridgeBandwidthPort, chainedBridgeBandwidthSession = startEchoServerInNamespace(contNS1)
-			Expect(err).ToNot(HaveOccurred())
 
 			By(fmt.Sprintf("starting echo server in %s\n\n", contNS2.ShortName()))
-			basicBridgePort, basicBridgeSession, err = startEchoServerInNamespace(contNS2)
+			basicBridgePort, basicBridgeSession = startEchoServerInNamespace(contNS2)
-			Expect(err).ToNot(HaveOccurred())
 
 			packetInBytes := 20000 // The shaper needs to 'warm'. Send enough to cause it to throttle,
 			// balanced by run time.
@@ -242,7 +239,7 @@ func makeTCPClientInNS(netns string, address string, port int, numBytes int) {
 	Expect(string(out)).To(Equal(message))
 }
 
-func startEchoServerInNamespace(netNS Namespace) (int, *gexec.Session, error) {
+func startEchoServerInNamespace(netNS Namespace) (int, *gexec.Session) {
 	session, err := startInNetNS(echoServerBinaryPath, netNS)
 	Expect(err).NotTo(HaveOccurred())
 
@@ -259,7 +256,7 @@ func startEchoServerInNamespace(netNS Namespace) (int, *gexec.Session, error) {
 		io.Copy(GinkgoWriter, io.MultiReader(session.Out, session.Err))
 	}()
 
-	return port, session, nil
+	return port, session
 }
 
 func startInNetNS(binPath string, namespace Namespace) (*gexec.Session, error) {

pkg/ip/link_linux.go

@@ -84,7 +84,7 @@ func makeVeth(name, vethPeerName string, mtu int, mac string, hostNS ns.NetNS) (
 		veth, err = makeVethPair(name, peerName, mtu, mac, hostNS)
 		switch {
 		case err == nil:
-			return peerName, veth, err
+			return peerName, veth, nil
 
 		case os.IsExist(err):
 			if peerExists(peerName) && vethPeerName == "" {

plugins/main/host-device/host-device_test.go

@@ -86,14 +86,14 @@ func canonicalizeIP(ip *net.IP) error {
 // LoadIPAMConfig creates IPAMConfig using json encoded configuration provided
 // as `bytes`. At the moment values provided in envArgs are ignored so there
 // is no possibility to overload the json configuration using envArgs
-func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
+func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, error) {
 	n := Net{}
 	if err := json.Unmarshal(bytes, &n); err != nil {
-		return nil, "", err
+		return nil, err
 	}
 
 	if n.IPAM == nil {
-		return nil, "", fmt.Errorf("IPAM config missing 'ipam' key")
+		return nil, fmt.Errorf("IPAM config missing 'ipam' key")
 	}
 
 	// Validate all ranges
@@ -103,13 +103,13 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 	for i := range n.IPAM.Addresses {
 		ip, addr, err := net.ParseCIDR(n.IPAM.Addresses[i].AddressStr)
 		if err != nil {
-			return nil, "", fmt.Errorf("invalid CIDR %s: %s", n.IPAM.Addresses[i].AddressStr, err)
+			return nil, fmt.Errorf("invalid CIDR %s: %s", n.IPAM.Addresses[i].AddressStr, err)
 		}
 		n.IPAM.Addresses[i].Address = *addr
 		n.IPAM.Addresses[i].Address.IP = ip
 
 		if err := canonicalizeIP(&n.IPAM.Addresses[i].Address.IP); err != nil {
-			return nil, "", fmt.Errorf("invalid address %d: %s", i, err)
+			return nil, fmt.Errorf("invalid address %d: %s", i, err)
 		}
 
 		if n.IPAM.Addresses[i].Address.IP.To4() != nil {
@@ -123,7 +123,7 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 		e := IPAMEnvArgs{}
 		err := types.LoadArgs(envArgs, &e)
 		if err != nil {
-			return nil, "", err
+			return nil, err
 		}
 
 		if e.IP != "" {
@@ -132,7 +132,7 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 
 				ip, subnet, err := net.ParseCIDR(ipstr)
 				if err != nil {
-					return nil, "", fmt.Errorf("invalid CIDR %s: %s", ipstr, err)
+					return nil, fmt.Errorf("invalid CIDR %s: %s", ipstr, err)
 				}
 
 				addr := Address{Address: net.IPNet{IP: ip, Mask: subnet.Mask}}
@@ -149,7 +149,7 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 			for _, item := range strings.Split(string(e.GATEWAY), ",") {
 				gwip := net.ParseIP(strings.TrimSpace(item))
 				if gwip == nil {
-					return nil, "", fmt.Errorf("invalid gateway address: %s", item)
+					return nil, fmt.Errorf("invalid gateway address: %s", item)
 				}
 
 				for i := range n.IPAM.Addresses {
@@ -164,14 +164,14 @@ func LoadIPAMConfig(bytes []byte, envArgs string) (*IPAMConfig, string, error) {
 	// CNI spec 0.2.0 and below supported only one v4 and v6 address
 	if numV4 > 1 || numV6 > 1 {
 		if ok, _ := version.GreaterThanOrEqualTo(n.CNIVersion, "0.3.0"); !ok {
-			return nil, "", fmt.Errorf("CNI version %v does not support more than 1 address per family", n.CNIVersion)
+			return nil, fmt.Errorf("CNI version %v does not support more than 1 address per family", n.CNIVersion)
 		}
 	}
 
 	// Copy net name into IPAM so not to drag Net struct around
 	n.IPAM.Name = n.Name
 
-	return n.IPAM, n.CNIVersion, nil
+	return n.IPAM, nil
 }
 
 func buildOneConfig(name, cniVersion string, orig *Net, prevResult types.Result) (*Net, error) {
@@ -868,7 +868,7 @@ var _ = Describe("base functionality", func() {
 			err = json.Unmarshal([]byte(conf), &n)
 			Expect(err).NotTo(HaveOccurred())
 
-			n.IPAM, _, err = LoadIPAMConfig([]byte(conf), "")
+			n.IPAM, err = LoadIPAMConfig([]byte(conf), "")
 			Expect(err).NotTo(HaveOccurred())
 
 			if testutils.SpecVersionHasCHECK(ver) {
@@ -984,7 +984,7 @@ var _ = Describe("base functionality", func() {
 			err = json.Unmarshal([]byte(conf), &n)
 			Expect(err).NotTo(HaveOccurred())
 
-			n.IPAM, _, err = LoadIPAMConfig([]byte(conf), "")
+			n.IPAM, err = LoadIPAMConfig([]byte(conf), "")
 			Expect(err).NotTo(HaveOccurred())
 
 			if testutils.SpecVersionHasCHECK(ver) {

plugins/meta/bandwidth/bandwidth_linux_test.go

@@ -35,7 +35,7 @@ import (
 	"github.com/containernetworking/plugins/pkg/testutils"
 )
 
-func buildOneConfig(name, cniVersion string, orig *PluginConf, prevResult types.Result) (*PluginConf, []byte, error) {
+func buildOneConfig(name, cniVersion string, orig *PluginConf, prevResult types.Result) ([]byte, error) {
 	var err error
 
 	inject := map[string]interface{}{
@@ -54,12 +54,12 @@ func buildOneConfig(name, cniVersion string, orig *PluginConf, prevResult types.
 
 	confBytes, err := json.Marshal(orig)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	err = json.Unmarshal(confBytes, &config)
 	if err != nil {
-		return nil, nil, fmt.Errorf("unmarshal existing network bytes: %s", err)
+		return nil, fmt.Errorf("unmarshal existing network bytes: %s", err)
 	}
 
 	for key, value := range inject {
@@ -68,15 +68,15 @@ func buildOneConfig(name, cniVersion string, orig *PluginConf, prevResult types.
 
 	newBytes, err := json.Marshal(config)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	conf := &PluginConf{}
 	if err := json.Unmarshal(newBytes, &conf); err != nil {
-		return nil, nil, fmt.Errorf("error parsing configuration: %s", err)
+		return nil, fmt.Errorf("error parsing configuration: %s", err)
 	}
 
-	return conf, newBytes, nil
+	return newBytes, nil
 }
 
 var _ = Describe("bandwidth test", func() {
@@ -950,7 +950,7 @@ var _ = Describe("bandwidth test", func() {
 						EgressRate:   rateInBits,
 					}
 					tbfPluginConf.Type = "bandwidth"
-					_, newConfBytes, err := buildOneConfig("myBWnet", ver, tbfPluginConf, containerWithTbfResult)
+					newConfBytes, err := buildOneConfig("myBWnet", ver, tbfPluginConf, containerWithTbfResult)
 					Expect(err).NotTo(HaveOccurred())
 
 					args := &skel.CmdArgs{
@@ -977,7 +977,7 @@ var _ = Describe("bandwidth test", func() {
 						}
 						checkConf.Type = "bandwidth"
 
-						_, newCheckBytes, err := buildOneConfig("myBWnet", ver, checkConf, result)
+						newCheckBytes, err := buildOneConfig("myBWnet", ver, checkConf, result)
 						Expect(err).NotTo(HaveOccurred())
 
 						args = &skel.CmdArgs{
@@ -995,10 +995,8 @@ var _ = Describe("bandwidth test", func() {
 				})).To(Succeed())
 
 				By("starting a tcp server on both containers")
-				portServerWithTbf, echoServerWithTbf, err = startEchoServerInNamespace(containerWithTbfNS)
+				portServerWithTbf, echoServerWithTbf = startEchoServerInNamespace(containerWithTbfNS)
-				Expect(err).NotTo(HaveOccurred())
+				portServerWithoutTbf, echoServerWithoutTbf = startEchoServerInNamespace(containerWithoutTbfNS)
-				portServerWithoutTbf, echoServerWithoutTbf, err = startEchoServerInNamespace(containerWithoutTbfNS)
-				Expect(err).NotTo(HaveOccurred())
 			})
 
 			AfterEach(func() {

plugins/meta/bandwidth/bandwidth_suite_test.go

@@ -66,7 +66,7 @@ func startInNetNS(binPath string, netNS ns.NetNS) (*gexec.Session, error) {
 	return session, err
 }
 
-func startEchoServerInNamespace(netNS ns.NetNS) (int, *gexec.Session, error) {
+func startEchoServerInNamespace(netNS ns.NetNS) (int, *gexec.Session) {
 	session, err := startInNetNS(echoServerBinaryPath, netNS)
 	Expect(err).NotTo(HaveOccurred())
 
@@ -83,7 +83,7 @@ func startEchoServerInNamespace(netNS ns.NetNS) (int, *gexec.Session, error) {
 		io.Copy(GinkgoWriter, io.MultiReader(session.Out, session.Err))
 	}()
 
-	return port, session, nil
+	return port, session
 }
 
 func makeTCPClientInNS(netns string, address string, port int, numBytes int) {

plugins/meta/firewall/firewall_firewalld_test.go

@@ -45,18 +45,21 @@ func (f *fakeFirewalld) clear() {
 	f.source = ""
 }
 
+//nolint:unparam
 func (f *fakeFirewalld) AddSource(zone, source string) (string, *dbus.Error) {
 	f.zone = zone
 	f.source = source
 	return "", nil
 }
 
+//nolint:unparam
 func (f *fakeFirewalld) RemoveSource(zone, source string) (string, *dbus.Error) {
 	f.zone = zone
 	f.source = source
 	return "", nil
 }
 
+//nolint:unparam
 func (f *fakeFirewalld) QuerySource(zone, source string) (bool, *dbus.Error) {
 	if f.zone != zone {
 		return false, nil
@@ -101,7 +104,7 @@ func spawnSessionDbus(wg *sync.WaitGroup) (string, *exec.Cmd) {
 	return busAddr, cmd
 }
 
-func makeFirewalldConf(ver, ifname string, ns ns.NetNS) []byte {
+func makeFirewalldConf(ver string, ns ns.NetNS) []byte {
 	return []byte(fmt.Sprintf(`{
 	  "cniVersion": "%s",
 	  "name": "firewalld-test",
@@ -111,7 +114,7 @@ func makeFirewalldConf(ver, ifname string, ns ns.NetNS) []byte {
 	  "prevResult": {
 	    "cniVersion": "%s",
 	    "interfaces": [
-	      {"name": "%s", "sandbox": "%s"}
+	      {"name": "eth0", "sandbox": "%s"}
 	    ],
 	    "ips": [
 	      {
@@ -122,7 +125,7 @@ func makeFirewalldConf(ver, ifname string, ns ns.NetNS) []byte {
 	      }
 	    ]
 	  }
-	}`, ver, ver, ifname, ns.Path()))
+	}`, ver, ver, ns.Path()))
 }
 
 var _ = Describe("firewalld test", func() {
@@ -192,7 +195,7 @@ var _ = Describe("firewalld test", func() {
 		It(fmt.Sprintf("[%s] works with a config", ver), func() {
 			Expect(isFirewalldRunning()).To(BeTrue())
 
-			conf := makeFirewalldConf(ver, ifname, targetNs)
+			conf := makeFirewalldConf(ver, targetNs)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNs.Path(),
@@ -218,7 +221,7 @@ var _ = Describe("firewalld test", func() {
 		It(fmt.Sprintf("[%s] defaults to the firewalld backend", ver), func() {
 			Expect(isFirewalldRunning()).To(BeTrue())
 
-			conf := makeFirewalldConf(ver, ifname, targetNs)
+			conf := makeFirewalldConf(ver, targetNs)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNs.Path(),
@@ -236,7 +239,7 @@ var _ = Describe("firewalld test", func() {
 		It(fmt.Sprintf("[%s] passes through the prevResult", ver), func() {
 			Expect(isFirewalldRunning()).To(BeTrue())
 
-			conf := makeFirewalldConf(ver, ifname, targetNs)
+			conf := makeFirewalldConf(ver, targetNs)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNs.Path(),
@@ -260,7 +263,7 @@ var _ = Describe("firewalld test", func() {
 		It(fmt.Sprintf("[%s] works with Check", ver), func() {
 			Expect(isFirewalldRunning()).To(BeTrue())
 
-			conf := makeFirewalldConf(ver, ifname, targetNs)
+			conf := makeFirewalldConf(ver, targetNs)
 			args := &skel.CmdArgs{
 				ContainerID: "dummy",
 				Netns:       targetNs.Path(),

plugins/meta/firewall/iptables.go

@@ -116,19 +116,16 @@ func (ib *iptablesBackend) addRules(_ *FirewallNetConf, result *current.Result,
 	return nil
 }
 
-func (ib *iptablesBackend) delRules(_ *FirewallNetConf, result *current.Result, ipt *iptables.IPTables, proto iptables.Protocol) error {
+func (ib *iptablesBackend) delRules(_ *FirewallNetConf, result *current.Result, ipt *iptables.IPTables, proto iptables.Protocol) {
 	rules := make([][]string, 0)
 	for _, ip := range result.IPs {
 		if protoForIP(ip.Address) == proto {
 			rules = append(rules, getPrivChainRules(ipString(ip.Address))...)
 		}
 	}
-
 	if len(rules) > 0 {
 		cleanupRules(ipt, ib.privChainName, rules)
 	}
-
-	return nil
 }
 
 func (ib *iptablesBackend) checkRules(_ *FirewallNetConf, result *current.Result, ipt *iptables.IPTables, proto iptables.Protocol) error {

plugins/meta/portmap/portmap_integ_test.go

@@ -83,8 +83,7 @@ var _ = Describe("portmap integration tests", func() {
 		fmt.Fprintln(GinkgoWriter, "namespace:", targetNS.Path())
 
 		// Start an echo server and get the port
-		containerPort, session, err = StartEchoServerInNamespace(targetNS)
+		containerPort, session = StartEchoServerInNamespace(targetNS)
-		Expect(err).NotTo(HaveOccurred())
 	})
 
 	AfterEach(func() {
@@ -329,8 +328,7 @@ var _ = Describe("portmap integration tests", func() {
 				fmt.Fprintln(GinkgoWriter, "namespace:", targetNS2.Path())
 
 				// Start an echo server and get the port
-				containerPort, session2, err := StartEchoServerInNamespace(targetNS2)
+				containerPort, session2 := StartEchoServerInNamespace(targetNS2)
-				Expect(err).NotTo(HaveOccurred())
 
 				runtimeConfig2 := libcni.RuntimeConf{
 					ContainerID: fmt.Sprintf("unit-test2-%d", hostPort),

plugins/meta/portmap/portmap_suite_test.go

@@ -62,7 +62,7 @@ func startInNetNS(binPath string, netNS ns.NetNS) (*gexec.Session, error) {
 	return session, err
 }
 
-func StartEchoServerInNamespace(netNS ns.NetNS) (int, *gexec.Session, error) {
+func StartEchoServerInNamespace(netNS ns.NetNS) (int, *gexec.Session) {
 	session, err := startInNetNS(echoServerBinaryPath, netNS)
 	Expect(err).NotTo(HaveOccurred())
 
@@ -73,5 +73,5 @@ func StartEchoServerInNamespace(netNS ns.NetNS) (int, *gexec.Session, error) {
 
 	port, err := strconv.Atoi(portString)
 	Expect(err).NotTo(HaveOccurred())
-	return port, session, nil
+	return port, session
 }

plugins/meta/tuning/tuning_test.go

@@ -33,11 +33,11 @@ import (
 	"github.com/containernetworking/plugins/pkg/testutils"
 )
 
-func buildOneConfig(name, cniVersion string, orig *TuningConf, prevResult types.Result) (*TuningConf, []byte, error) {
+func buildOneConfig(cniVersion string, orig *TuningConf, prevResult types.Result) ([]byte, error) {
 	var err error
 
 	inject := map[string]interface{}{
-		"name":       name,
+		"name":       "testConfig",
 		"cniVersion": cniVersion,
 	}
 	// Add previous plugin result
@@ -50,12 +50,12 @@ func buildOneConfig(name, cniVersion string, orig *TuningConf, prevResult types.
 
 	confBytes, err := json.Marshal(orig)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	err = json.Unmarshal(confBytes, &config)
 	if err != nil {
-		return nil, nil, fmt.Errorf("unmarshal existing network bytes: %s", err)
+		return nil, fmt.Errorf("unmarshal existing network bytes: %s", err)
 	}
 
 	for key, value := range inject {
@@ -64,15 +64,15 @@ func buildOneConfig(name, cniVersion string, orig *TuningConf, prevResult types.
 
 	newBytes, err := json.Marshal(config)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	conf := &TuningConf{}
 	if err := json.Unmarshal(newBytes, &conf); err != nil {
-		return nil, nil, fmt.Errorf("error parsing configuration: %s", err)
+		return nil, fmt.Errorf("error parsing configuration: %s", err)
 	}
 
-	return conf, newBytes, nil
+	return newBytes, nil
 }
 
 func createSysctlAllowFile(sysctls []string) error {
@@ -256,7 +256,7 @@ var _ = Describe("tuning plugin", func() {
 					err = json.Unmarshal(conf, &n)
 					Expect(err).NotTo(HaveOccurred())
 
-					_, confString, err := buildOneConfig("testConfig", ver, n, r)
+					confString, err := buildOneConfig(ver, n, r)
 					Expect(err).NotTo(HaveOccurred())
 
 					args.StdinData = confString
@@ -398,7 +398,7 @@ var _ = Describe("tuning plugin", func() {
 					err = json.Unmarshal(conf, &n)
 					Expect(err).NotTo(HaveOccurred())
 
-					_, confString, err := buildOneConfig("testConfig", ver, n, r)
+					confString, err := buildOneConfig(ver, n, r)
 					Expect(err).NotTo(HaveOccurred())
 
 					args.StdinData = confString
@@ -544,7 +544,7 @@ var _ = Describe("tuning plugin", func() {
 					err = json.Unmarshal(conf, &n)
 					Expect(err).NotTo(HaveOccurred())
 
-					_, confString, err := buildOneConfig("testConfig", ver, n, r)
+					confString, err := buildOneConfig(ver, n, r)
 					Expect(err).NotTo(HaveOccurred())
 
 					args.StdinData = confString
@@ -690,7 +690,7 @@ var _ = Describe("tuning plugin", func() {
 					err = json.Unmarshal(conf, &n)
 					Expect(err).NotTo(HaveOccurred())
 
-					_, confString, err := buildOneConfig("testConfig", ver, n, r)
+					confString, err := buildOneConfig(ver, n, r)
 					Expect(err).NotTo(HaveOccurred())
 
 					args.StdinData = confString
@@ -842,7 +842,7 @@ var _ = Describe("tuning plugin", func() {
 					err = json.Unmarshal(conf, &n)
 					Expect(err).NotTo(HaveOccurred())
 
-					_, confString, err := buildOneConfig("testConfig", ver, n, r)
+					confString, err := buildOneConfig(ver, n, r)
 					Expect(err).NotTo(HaveOccurred())
 
 					args.StdinData = confString
@@ -921,7 +921,7 @@ var _ = Describe("tuning plugin", func() {
 					err = json.Unmarshal(conf, &n)
 					Expect(err).NotTo(HaveOccurred())
 
-					_, confString, err := buildOneConfig("testConfig", ver, n, r)
+					confString, err := buildOneConfig(ver, n, r)
 					Expect(err).NotTo(HaveOccurred())
 
 					args.StdinData = confString