chore: fix staticcheck issues (#164)

Use [staticcheck](https://staticcheck.dev/) to find and fix correctness issues. ❯ go run honnef.co/go/tools/cmd/staticcheck@latest ./... internal/iptables/iptables.go:61:3: should replace loop with rules = append(rules, EgressNetworkPolicyToIpTableRules(policy, peerChain)...) (S1011) internal/iptables/iptables.go:80:22: unnecessary use of fmt.Sprintf (S1039) internal/it/suite_test.go:32:5: var cfg is unused (U1000) internal/it/suite_test.go:34:5: var testEnv is unused (U1000) internal/it/suite_test.go:172:2: this value of b is never used (SA4006) internal/it/suite_test.go:185:2: this value of b is never used (SA4006) internal/it/suite_test.go:222:2: this value of err is never used (SA4006) pkg/api/v1alpha1/wireguard_types.go:26:2: only the first constant in this group has an explicit type (SA9004) pkg/controllers/suite_test.go:41:5: var cfg is unused (U1000) pkg/controllers/wireguard_controller.go:151:13: error strings should not be capitalized (ST1005) pkg/controllers/wireguard_controller.go:407:3: this value of port is never used (SA4006) pkg/controllers/wireguard_controller.go:407:10: Sprint doesn't have side effects and its return value is ignored (SA4017) pkg/wireguard/wireguard.go:63:2: this value of err is never used (SA4006) pkg/wireguard/wireguard.go:104:2: this value of link is never used (SA4006) pkg/wireguard/wireguard.go:155:2: this value of err is never used (SA4006) pkg/wireguard/wireguard.go:293:6: should omit comparison to bool constant, can be simplified to peer.Spec.Disabled (S1002) pkg/wireguard/wireguard.go:343:2: this value of err is never used (SA4006) exit status 1 Refs: #160
jodevsa · May 9, 2024 · 1d1bef4 · 1d1bef4
1 parent acd2e77
commit 1d1bef4
Show file tree

Hide file tree

Showing 6 changed files with 57 additions and 49 deletions.
diff --git a/internal/iptables/iptables.go b/internal/iptables/iptables.go
@@ -38,29 +38,30 @@ func (it *Iptables) Sync(state agent.State) error {
 }
 
 func GenerateIptableRulesFromNetworkPolicies(policies v1alpha1.EgressNetworkPolicies, peerIp string, kubeDnsIp string, wgServerIp string) string {
-	var rules []string
+	peerChain := strings.ReplaceAll(peerIp, ".", "-")
 
-	// add a comment
-	rules = append(rules, fmt.Sprintf("# start of rules for peer %s", peerIp))
+	rules := []string{
+		// add a comment
+		fmt.Sprintf("# start of rules for peer %s", peerIp),
 
-	peerChain := strings.ReplaceAll(peerIp, ".", "-")
+		// create chain for peer
+		fmt.Sprintf(":%s - [0:0]", peerChain),
 
-	// create chain for peer
-	rules = append(rules, fmt.Sprintf(":%s - [0:0]", peerChain))
-	// associate peer chain to FORWARD chain
-	rules = append(rules, fmt.Sprintf("-A FORWARD -s %s -j %s", peerIp, peerChain))
+		// associate peer chain to FORWARD chain
+		fmt.Sprintf("-A FORWARD -s %s -j %s", peerIp, peerChain),
 
-	// allow peer to ping (ICMP) wireguard server for debugging purposes
-	rules = append(rules, fmt.Sprintf("-A %s -d %s -p icmp -j ACCEPT", peerChain, wgServerIp))
-	// allow peer to communicate with itself
-	rules = append(rules, fmt.Sprintf("-A %s -d %s -j ACCEPT", peerChain, peerIp))
-	// allow peer to communicate with kube-dns
-	rules = append(rules, fmt.Sprintf("-A %s -d %s -p UDP --dport 53 -j ACCEPT", peerChain, kubeDnsIp))
+		// allow peer to ping (ICMP) wireguard server for debugging purposes
+		fmt.Sprintf("-A %s -d %s -p icmp -j ACCEPT", peerChain, wgServerIp),
+
+		// allow peer to communicate with itself
+		fmt.Sprintf("-A %s -d %s -j ACCEPT", peerChain, peerIp),
+
+		// allow peer to communicate with kube-dns
+		fmt.Sprintf("-A %s -d %s -p UDP --dport 53 -j ACCEPT", peerChain, kubeDnsIp),
+	}
 
 	for _, policy := range policies {
-		for _, rule := range EgressNetworkPolicyToIpTableRules(policy, peerChain) {
-			rules = append(rules, rule)
-		}
+		rules = append(rules, EgressNetworkPolicyToIpTableRules(policy, peerChain)...)
 	}
 
 	// if policies are defined impose an implicit deny all
@@ -77,14 +78,14 @@ func GenerateIptableRulesFromNetworkPolicies(policies v1alpha1.EgressNetworkPoli
 func GenerateIptableRulesFromPeers(wgHostName string, dns string, peers []v1alpha1.WireguardPeer) string {
 	var rules []string
 
-	var natTableRules = fmt.Sprintf(`
+	var natTableRules = `
 *nat
 :PREROUTING ACCEPT [0:0]
 :INPUT ACCEPT [0:0]
 :OUTPUT ACCEPT [0:0]
 :POSTROUTING ACCEPT [0:0]
 -A POSTROUTING -s 10.8.0.0/24 -o eth0 -j MASQUERADE
-COMMIT`)
+COMMIT`
 
 	for _, peer := range peers {
 

diff --git a/internal/it/suite_test.go b/internal/it/suite_test.go
@@ -16,10 +16,8 @@ import (
 	v12 "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/scheme"
-	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/clientcmd"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/envtest"
 	"sigs.k8s.io/kind/pkg/apis/config/v1alpha4"
 	kind "sigs.k8s.io/kind/pkg/cluster"
 	log2 "sigs.k8s.io/kind/pkg/log"
@@ -29,9 +27,7 @@ import (
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
-var cfg *rest.Config
 var k8sClient client.Client
-var testEnv *envtest.Environment
 var releasePath string
 var agentImage string
 var managerImage string
@@ -169,9 +165,9 @@ var _ = BeforeSuite(func() {
 	}
 
 	// load locally built images
-	cmd := exec.Command(kindBinary, "load", "docker-image", managerImage, "--name", testClusterName)
-	b, err := cmd.Output()
-	if err != nil {
+	if _, err := exec.
+		Command(kindBinary, "load", "docker-image", managerImage, "--name", testClusterName).
+		Output(); err != nil {
 		if err != nil {
 			if exitError, ok := err.(*exec.ExitError); ok {
 				log.Info(string(exitError.Stderr))
@@ -182,17 +178,18 @@ var _ = BeforeSuite(func() {
 		log.Error(err, "unable to load local image manager:dev")
 		Expect(err).NotTo(HaveOccurred())
 	}
-	cmd = exec.Command(kindBinary, "load", "docker-image", agentImage, "--name", testClusterName)
-	b, err = cmd.Output()
-	if err != nil {
+
+	if _, err := exec.
+		Command(kindBinary, "load", "docker-image", agentImage, "--name", testClusterName).
+		Output(); err != nil {
 		log.Error(err, "unable to load local image agent:dev")
 		return
 	}
 
 	// simulate what users exactly do in real life.
-	cmd = exec.Command("kubectl", "apply", "-f", releasePath, "--context", testKindContextName)
-	b, err = cmd.Output()
-
+	b, err := exec.
+		Command("kubectl", "apply", "-f", releasePath, "--context", testKindContextName).
+		Output()
 	if err != nil {
 		log.Error(err, "unable to apply release.yaml")
 		return
@@ -221,6 +218,7 @@ var _ = BeforeSuite(func() {
 	Expect(err).NotTo(HaveOccurred())
 
 	k8sClient, err = client.New(c, client.Options{Scheme: scheme.Scheme})
+	Expect(err).NotTo(HaveOccurred())
 
 	// wait until operator is ready
 	Eventually(func() int {

diff --git a/pkg/api/v1alpha1/wireguard_types.go b/pkg/api/v1alpha1/wireguard_types.go
@@ -23,9 +23,9 @@ import (
 )
 
 const (
-	Pending string = "pending"
-	Error          = "error"
-	Ready          = "ready"
+	Pending = "pending"
+	Error   = "error"
+	Ready   = "ready"
 )
 
 type WgStatusReport struct {

diff --git a/pkg/controllers/suite_test.go b/pkg/controllers/suite_test.go
@@ -26,7 +26,6 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/client-go/kubernetes/scheme"
-	"k8s.io/client-go/rest"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/envtest"
@@ -38,7 +37,6 @@ import (
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
-var cfg *rest.Config
 var k8sClient client.Client
 var testEnv *envtest.Environment
 var wgTestImage = "test-image"

diff --git a/pkg/controllers/wireguard_controller.go b/pkg/controllers/wireguard_controller.go
@@ -21,6 +21,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"strconv"
 	"time"
 
 	"github.com/jodevsa/wireguard-operator/pkg/agent"
@@ -148,7 +149,7 @@ func getAvaialbleIp(cidr string, usedIps []string) (string, error) {
 		}
 	}
 
-	return "", fmt.Errorf("No available ip found in %s", cidr)
+	return "", fmt.Errorf("no available ip found in %s", cidr)
 }
 
 func (r *WireguardReconciler) getUsedIps(peers *v1alpha1.WireguardPeerList) []string {
@@ -404,8 +405,7 @@ func (r *WireguardReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			return ctrl.Result{}, nil
 		}
 
-		port = fmt.Sprint(svcFound.Spec.Ports[0].NodePort)
-		port = fmt.Sprint(svcFound.Spec.Ports[0].NodePort)
+		port = strconv.Itoa(int(svcFound.Spec.Ports[0].NodePort))
 
 		ips, err := r.getNodeIps(ctx, req)
 

diff --git a/pkg/wireguard/wireguard.go b/pkg/wireguard/wireguard.go
@@ -2,11 +2,12 @@ package wireguard
 
 import (
 	"fmt"
-	"github.com/go-logr/logr"
 	"net"
 	"os/exec"
 	"syscall"
 
+	"github.com/go-logr/logr"
+
 	"github.com/jodevsa/wireguard-operator/pkg/agent"
 	"github.com/jodevsa/wireguard-operator/pkg/api/v1alpha1"
 	"github.com/vishvananda/netlink"
@@ -60,9 +61,12 @@ func syncAddress(_ agent.State, iface string) error {
 	if len(addresses) != 0 {
 		return nil
 	}
-	err = netlink.AddrAdd(link, &netlink.Addr{
+
+	if err := netlink.AddrAdd(link, &netlink.Addr{
 		IPNet: &net.IPNet{IP: net.ParseIP("10.8.0.1")},
-	})
+	}); err != nil {
+		return fmt.Errorf("netlink addr add: %w", err)
+	}
 
 	if err := netlink.LinkSetUp(link); err != nil {
 		return err
@@ -101,7 +105,7 @@ func createLinkUsingKernalModule(iface string) error {
 }
 
 func SyncLink(_ agent.State, iface string, wgUserspaceImplementationFallback string, wgUseUserspaceImpl bool) error {
-	link, err := netlink.LinkByName(iface)
+	_, err := netlink.LinkByName(iface)
 	if err != nil {
 		if _, ok := err.(netlink.LinkNotFoundError); !ok {
 			return err
@@ -128,7 +132,8 @@ func SyncLink(_ agent.State, iface string, wgUserspaceImplementationFallback str
 			}
 		}
 
-		link, err = netlink.LinkByName(iface)
+		// TODO: Can this be removed?
+		link, err := netlink.LinkByName(iface)
 		if err != nil {
 			return err
 		}
@@ -137,7 +142,7 @@ func SyncLink(_ agent.State, iface string, wgUserspaceImplementationFallback str
 		}
 	}
 
-	link, err = netlink.LinkByName(iface)
+	link, err := netlink.LinkByName(iface)
 	if err != nil {
 		if _, ok := err.(netlink.LinkNotFoundError); !ok {
 			return err
@@ -152,9 +157,12 @@ func SyncLink(_ agent.State, iface string, wgUserspaceImplementationFallback str
 	if len(addresses) != 0 {
 		return nil
 	}
-	err = netlink.AddrAdd(link, &netlink.Addr{
+
+	if err := netlink.AddrAdd(link, &netlink.Addr{
 		IPNet: &getIP("10.8.0.1/32")[0],
-	})
+	}); err != nil {
+		return fmt.Errorf("netlink addr add: %w", err)
+	}
 
 	if err := netlink.LinkSetUp(link); err != nil {
 		return err
@@ -290,7 +298,7 @@ func createPeersConfiguration(state agent.State, iface string) ([]wgtypes.PeerCo
 
 	// add new peers
 	for _, peer := range state.Peers {
-		if peer.Spec.Disabled == true {
+		if peer.Spec.Disabled {
 			continue
 		}
 		if peer.Spec.PublicKey == "" {
@@ -341,6 +349,9 @@ func CreateWireguardConfiguration(state agent.State, iface string, listenPort in
 	cfg.ListenPort = &listenPort
 
 	peers, err := createPeersConfiguration(state, iface)
+	if err != nil {
+		return wgtypes.Config{}, err
+	}
 
 	cfg.Peers = peers