diff --git a/.golangci.yml b/.golangci.yml index 32718813..2e3bd793 100644 --- a/.golangci.yml +++ b/.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: diff --git a/integration/integration_linux_test.go b/integration/integration_linux_test.go index fc673384..32d4ccce 100644 --- a/integration/integration_linux_test.go +++ b/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) - Expect(err).ToNot(HaveOccurred()) + chainedBridgeBandwidthPort, chainedBridgeBandwidthSession = startEchoServerInNamespace(contNS1) By(fmt.Sprintf("starting echo server in %s\n\n", contNS2.ShortName())) - basicBridgePort, basicBridgeSession, err = startEchoServerInNamespace(contNS2) - Expect(err).ToNot(HaveOccurred()) + basicBridgePort, basicBridgeSession = startEchoServerInNamespace(contNS2) 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) { diff --git a/pkg/ip/link_linux.go b/pkg/ip/link_linux.go index 4fe83fda..09a3876b 100644 --- a/pkg/ip/link_linux.go +++ b/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 == "" { diff --git a/plugins/main/host-device/host-device_test.go b/plugins/main/host-device/host-device_test.go index 21848929..4cf8ba59 100644 --- a/plugins/main/host-device/host-device_test.go +++ b/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) { diff --git a/plugins/meta/bandwidth/bandwidth_linux_test.go b/plugins/meta/bandwidth/bandwidth_linux_test.go index 59292b68..bce274f2 100644 --- a/plugins/meta/bandwidth/bandwidth_linux_test.go +++ b/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) - Expect(err).NotTo(HaveOccurred()) - portServerWithoutTbf, echoServerWithoutTbf, err = startEchoServerInNamespace(containerWithoutTbfNS) - Expect(err).NotTo(HaveOccurred()) + portServerWithTbf, echoServerWithTbf = startEchoServerInNamespace(containerWithTbfNS) + portServerWithoutTbf, echoServerWithoutTbf = startEchoServerInNamespace(containerWithoutTbfNS) }) AfterEach(func() { diff --git a/plugins/meta/bandwidth/bandwidth_suite_test.go b/plugins/meta/bandwidth/bandwidth_suite_test.go index 75ce7417..a6f5edd9 100644 --- a/plugins/meta/bandwidth/bandwidth_suite_test.go +++ b/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) { diff --git a/plugins/meta/firewall/firewall_firewalld_test.go b/plugins/meta/firewall/firewall_firewalld_test.go index da41f961..cfb3a1f6 100644 --- a/plugins/meta/firewall/firewall_firewalld_test.go +++ b/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(), diff --git a/plugins/meta/firewall/iptables.go b/plugins/meta/firewall/iptables.go index d00ca0a9..98e40f80 100644 --- a/plugins/meta/firewall/iptables.go +++ b/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 { diff --git a/plugins/meta/portmap/portmap_integ_test.go b/plugins/meta/portmap/portmap_integ_test.go index f25f6f4b..25b27deb 100644 --- a/plugins/meta/portmap/portmap_integ_test.go +++ b/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) - Expect(err).NotTo(HaveOccurred()) + containerPort, session = StartEchoServerInNamespace(targetNS) }) 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) - Expect(err).NotTo(HaveOccurred()) + containerPort, session2 := StartEchoServerInNamespace(targetNS2) runtimeConfig2 := libcni.RuntimeConf{ ContainerID: fmt.Sprintf("unit-test2-%d", hostPort), diff --git a/plugins/meta/portmap/portmap_suite_test.go b/plugins/meta/portmap/portmap_suite_test.go index a376f293..3434bd3a 100644 --- a/plugins/meta/portmap/portmap_suite_test.go +++ b/plugins/meta/portmap/portmap_suite_test.go @@ -65,7 +65,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()) @@ -76,5 +76,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 } diff --git a/plugins/meta/tuning/tuning_test.go b/plugins/meta/tuning/tuning_test.go index 9df0950a..60ed13bb 100644 --- a/plugins/meta/tuning/tuning_test.go +++ b/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