From 1d1bef4d0d82c0235dbfba3b665f6ebe5fb4260b Mon Sep 17 00:00:00 2001 From: Thomas <9749173+uhthomas@users.noreply.github.com> Date: Thu, 9 May 2024 21:35:24 +0100 Subject: [PATCH] chore: fix staticcheck issues (#164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/iptables/iptables.go | 39 +++++++++++++------------ internal/it/suite_test.go | 24 +++++++-------- pkg/api/v1alpha1/wireguard_types.go | 6 ++-- pkg/controllers/suite_test.go | 2 -- pkg/controllers/wireguard_controller.go | 6 ++-- pkg/wireguard/wireguard.go | 29 ++++++++++++------ 6 files changed, 57 insertions(+), 49 deletions(-) diff --git a/internal/iptables/iptables.go b/internal/iptables/iptables.go index 08888d1f..56af28db 100644 --- 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 index 2fb1a1ee..50986808 100644 --- 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 index b6262fe4..9fea8d0d 100644 --- 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 index 4420c9f6..85bf042b 100644 --- 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 index 1716c6df..b35f22d5 100644 --- 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 index ecb2a761..c6407a01 100644 --- 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