Adjust golangci-lint linters

.golangci.yml

@@ -53,6 +53,7 @@ linters:
     - asciicheck
     - bidichk
     - bodyclose
+    # - canonicalheader # This is a slow linter and we don't use the net/http.Header API
     - containedctx
     - contextcheck
     - copyloopvar
@@ -69,15 +70,19 @@ linters:
     - errname
     # - execinquery # No SQL
     - exhaustive
-    # - exhauststruct # Not recommended for general use - meant to be used only for special cases
+    # - exhaustruct # This is too cumbersome as it requires all string, int, pointer et al fields to be initialized even when the
+    # type's default suffices, which is most of the time
+    - fatcontext
     # - forbidigo # We don't forbid any statements
     # - forcetypeassert # There are many unchecked type assertions that would be the result of a programming error so the
     #                     reasonable recourse would be to panic anyway if checked so this doesn't seem useful
     # - funlen # gocyclo is enabled which is generally a better metric than simply LOC.
     - gci
     - ginkgolinter
+    - gocheckcompilerdirectives
     # - gochecknoglobals # We don't want to forbid global variable constants
     # - gochecknoinits # We use init functions for valid reasons
+    # - gochecksumtype # The usefulness is very narrow:w
     - gocognit
     - goconst
     - gocritic
@@ -89,46 +94,58 @@ linters:
     - gofumpt
     - goheader
     - goimports
-    # - gomnd # It doesn't seem useful in general to enforce constants for all numeric values
     # - gomoddirectives # We don't want to forbid the 'replace' directive
     # - gomodguard # We don't block any modules
     # - goprintffuncname # This doesn't seem useful at all
+    # - gosmopolitan # This is related to internationalization which is not a concern for us
     - gosec
     - gosimple
     - govet
     - grouper
+    - iface
     - importas
+    - inamedparam
     - ineffassign
     # - interfacebloat # We track complexity elsewhere
+    - intrange
     # - ireturn # The argument to always "Return Concrete Types" doesn't seem compelling. It is perfectly valid to return
     #             an interface to avoid exposing the entire underlying struct
     - lll
     - loggercheck
     - maintidx
     - makezero
+    - mirror
     - misspell
+    # - mnd # It doesn't seem useful in general to enforce constants for all numeric values
     - nakedret
     # - nestif # This calculates cognitive complexity but we're doing that elsewhere
     - nilerr
     - nilnil
     # - nlreturn # This is reasonable with a block-size of 2 but setting it above isn't honored
     # - noctx # We don't send HTTP requests
     - nolintlint
-    # - nonamedreturns # We don't forbid named returns
-    - nosprintfhostport
+    - nonamedreturns
+    # - nosprintfhostport # The use of this is very narrow
     # - paralleltest # Not relevant for Ginkgo UTs
+    - perfsprint
     - prealloc
     - predeclared
     - promlinter
+    - protogetter
     - reassign
+    - recvcheck
     - revive
     # - rowserrcheck # We don't use SQL
+    # - sloglint # We don't use log/slog
+    # - spancheck # We don't use OpenTelemetry/OpenCensus
     # - sqlclosecheck # We don't use SQL
     - staticcheck
     - stylecheck
+    - tagalign
     # - tagliatelle # Inconsistent with stylecheck and not as good
     # - tenv # Not relevant for our Ginkgo UTs
-    - testableexamples
+    # - testableexamples # We don't need this
+    # - testifylint # We don't use testify
     - testpackage
     # - thelper # Not relevant for our Ginkgo UTs
     # - tparallel # Not relevant for our Ginkgo UTs
@@ -142,6 +159,7 @@ linters:
     - whitespace
     - wrapcheck
     - wsl
+    # - zerologlint # We use zerolog indirectly so this isn't needed
 issues:
   exclude-rules:
     # Allow dot-imports for Gomega BDD directives per idiomatic Gomega

pkg/apis/submariner.io/v1/endpoint.go

@@ -68,11 +68,11 @@ func parsePort(port string) (int32, error) {
 
 func (ep *EndpointSpec) GenerateName() (string, error) {
 	if ep.ClusterID == "" {
-		return "", fmt.Errorf("ClusterID cannot be empty")
+		return "", errors.New("ClusterID cannot be empty")
 	}
 
 	if ep.CableName == "" {
-		return "", fmt.Errorf("CableName cannot be empty")
+		return "", errors.New("CableName cannot be empty")
 	}
 
 	return resource.EnsureValidName(fmt.Sprintf("%s-%s", ep.ClusterID, ep.CableName)), nil

pkg/cable/libreswan/libreswan.go

@@ -320,7 +320,7 @@ func extractSubnets(endpoint *subv1.EndpointSpec) []string {
 func whack(args ...string) error {
 	var err error
 
-	for i := 0; i < 3; i++ {
+	for range 3 {
 		err = func() error {
 			ctx, cancel := context.WithTimeout(context.TODO(), whackTimeout)
 			defer cancel()
@@ -653,7 +653,7 @@ func (i *libreswan) waitForControlSocket() error {
 	const retryInterval = 100 * time.Millisecond
 	const controlSocketPath = "/run/pluto/pluto.ctl"
 
-	for i := 0; i < maxAttempts; i++ {
+	for range maxAttempts {
 		_, err := os.Stat(controlSocketPath)
 		if err == nil {
 			return nil

pkg/cable/wireguard/driver.go

@@ -104,7 +104,7 @@ func NewDriver(localEndpoint *endpoint.Local, _ *types.SubmarinerCluster) (cable
 	// Create the controller.
 	if w.client, err = wgctrl.New(); err != nil {
 		if os.IsNotExist(err) {
-			return nil, fmt.Errorf("wgctrl is not available on this system")
+			return nil, errors.New("wgctrl is not available on this system")
 		}
 
 		return nil, errors.Wrap(err, "failed to open wgctl client")
@@ -312,7 +312,7 @@ func (w *wireguard) ConnectToEndpoint(endpointInfo *natdiscovery.NATEndpointInfo
 func keyFromSpec(ep *v1.EndpointSpec) (*wgtypes.Key, error) {
 	s, found := ep.BackendConfig[PublicKey]
 	if !found {
-		return nil, fmt.Errorf("endpoint is missing public key")
+		return nil, errors.New("endpoint is missing public key")
 	}
 
 	key, err := wgtypes.ParseKey(s)

pkg/cableengine/syncer/syncer_test.go

@@ -96,7 +96,7 @@ func testGatewaySyncing() {
 		It("should periodically update the Gateway resource timestamp", func() {
 			var lastTimestamp int64
 
-			for i := 0; i < 3; i++ {
+			for range 3 {
 				var currentTimestamp int64
 
 				Eventually(func() int64 {
@@ -675,6 +675,6 @@ func (m *equalGatewayMatcher) FailureMessage(actual interface{}) string {
 	return format.Message(actual, "to equal", m.expected)
 }
 
-func (m *equalGatewayMatcher) NegatedFailureMessage(actual interface{}) (message string) {
+func (m *equalGatewayMatcher) NegatedFailureMessage(actual interface{}) string {
 	return format.Message(actual, "not to equal", m.expected)
 }

pkg/globalnet/controllers/egress_pod_watcher.go

@@ -19,8 +19,6 @@ limitations under the License.
 package controllers
 
 import (
-	"fmt"
-
 	"github.com/pkg/errors"
 	"github.com/submariner-io/admiral/pkg/log"
 	"github.com/submariner-io/admiral/pkg/watcher"
@@ -53,7 +51,7 @@ func startEgressPodWatcher(name, namespace string, namedSet packetfilter.NamedSe
 		Scheme:     config.Scheme,
 		ResourceConfigs: []watcher.ResourceConfig{
 			{
-				Name:         fmt.Sprintf("Pod watcher %s", name),
+				Name:         "Pod watcher " + name,
 				ResourceType: &corev1.Pod{},
 				Handler: watcher.EventHandlerFuncs{
 					OnCreateFunc: pw.onCreateOrUpdate,

pkg/globalnet/main.go

@@ -113,7 +113,7 @@ func main() {
 
 	var localCluster *submarinerv1.Cluster
 	// During installation, sometimes creation of clusterCRD by submariner-gateway-pod would take few secs.
-	for i := 0; i < 100; i++ {
+	for range 100 {
 		localCluster, err = submarinerClient.SubmarinerV1().Clusters(spec.Namespace).Get(context.TODO(), spec.ClusterID,
 			metav1.GetOptions{})
 		if err == nil {

pkg/ipset/ipset.go

@@ -50,6 +50,7 @@ limitations under the License.
 package ipset
 
 import (
+	"errors"
 	"fmt"
 	"os/exec"
 	"regexp"
@@ -384,7 +385,7 @@ func (runner *runner) ListSets() ([]string, error) {
 // ListEntries lists all the entries from a named set.
 func (runner *runner) ListEntries(set string) ([]string, error) {
 	if set == "" {
-		return nil, fmt.Errorf("set name can't be empty")
+		return nil, errors.New("set name can't be empty")
 	}
 
 	out, err := runner.runWithOutput([]string{"list", set}, "error listing set %q", set)

pkg/natdiscovery/proto/methods.go

@@ -19,17 +19,9 @@ limitations under the License.
 package proto
 
 func (x *SubmarinerNATDiscoveryResponse) GetSenderEndpointID() string {
-	if x != nil && x.Sender != nil {
-		return x.Sender.EndpointId
-	}
-
-	return ""
+	return x.GetSender().GetEndpointId()
 }
 
 func (x *SubmarinerNATDiscoveryResponse) GetReceiverEndpointID() string {
-	if x != nil && x.Receiver != nil {
-		return x.Receiver.EndpointId
-	}
-
-	return ""
+	return x.GetReceiver().GetEndpointId()
 }

pkg/natdiscovery/request_handle.go

@@ -31,19 +31,19 @@ func (nd *natDiscovery) handleRequestFromAddress(req *proto.SubmarinerNATDiscove
 	localEndpointSpec := nd.localEndpoint.Spec()
 
 	response := proto.SubmarinerNATDiscoveryResponse{
-		RequestNumber: req.RequestNumber,
+		RequestNumber: req.GetRequestNumber(),
 		Sender: &proto.EndpointDetails{
 			ClusterId:  localEndpointSpec.ClusterID,
 			EndpointId: localEndpointSpec.CableName,
 		},
-		Receiver: req.Sender,
+		Receiver: req.GetSender(),
 		ReceivedSrc: &proto.IPPortPair{
 			Port: int32(addr.Port), //nolint:gosec // We can safely ignore integer conversion error
 			IP:   addr.IP.String(),
 		},
 	}
 
-	if req.Receiver == nil || req.Sender == nil || req.UsingDst == nil || req.UsingSrc == nil {
+	if req.GetReceiver() == nil || req.GetSender() == nil || req.GetUsingDst() == nil || req.GetUsingSrc() == nil {
 		logger.Warningf("Received NAT discovery packet %#v from %s which seems to be malformed ", req, addr.String())
 
 		response.Response = proto.ResponseType_MALFORMED
@@ -52,53 +52,53 @@ func (nd *natDiscovery) handleRequestFromAddress(req *proto.SubmarinerNATDiscove
 	}
 
 	logger.V(log.DEBUG).Infof("Received request from %s:%d - REQUEST_NUMBER: 0x%x, SENDER: %q, RECEIVER: %q",
-		addr.IP.String(), addr.Port, req.RequestNumber, req.Sender.EndpointId, req.Receiver.EndpointId)
+		addr.IP.String(), addr.Port, req.GetRequestNumber(), req.GetSender().GetEndpointId(), req.GetReceiver().GetEndpointId())
 
-	if req.Receiver.GetClusterId() != localEndpointSpec.ClusterID {
-		logger.Warningf("Received NAT discovery packet for cluster %q, but we are cluster %q", req.Receiver.GetClusterId(),
+	if req.GetReceiver().GetClusterId() != localEndpointSpec.ClusterID {
+		logger.Warningf("Received NAT discovery packet for cluster %q, but we are cluster %q", req.GetReceiver().GetClusterId(),
 			localEndpointSpec.ClusterID)
 
 		response.Response = proto.ResponseType_UNKNOWN_DST_CLUSTER
 
 		return nd.sendResponseToAddress(&response, addr)
 	}
 
-	if req.Receiver.GetEndpointId() != localEndpointSpec.CableName {
+	if req.GetReceiver().GetEndpointId() != localEndpointSpec.CableName {
 		logger.Warningf("Received NAT discovery packet for endpoint %q, but we are endpoint %q "+
-			"if the port for NAT discovery has been mapped somewhere an error may exist", req.Receiver.GetEndpointId(),
+			"if the port for NAT discovery has been mapped somewhere an error may exist", req.GetReceiver().GetEndpointId(),
 			localEndpointSpec.CableName)
 
 		response.Response = proto.ResponseType_UNKNOWN_DST_ENDPOINT
 
 		return nd.sendResponseToAddress(&response, addr)
 	}
 
-	if req.UsingSrc.GetIP() != "" && req.UsingSrc.GetIP() != addr.IP.String() {
+	if req.GetUsingSrc().GetIP() != "" && req.GetUsingSrc().GetIP() != addr.IP.String() {
 		logger.V(log.DEBUG).Infof("Received NAT packet from endpoint %q, cluster %q, where NAT has been detected, "+
 			"source IP changed",
-			req.Sender.GetEndpointId(), req.Sender.GetClusterId())
-		logger.V(log.DEBUG).Infof("Original src IP was %q, received src IP is %q", req.UsingSrc.IP, addr.IP.String())
+			req.GetSender().GetEndpointId(), req.GetSender().GetClusterId())
+		logger.V(log.DEBUG).Infof("Original src IP was %q, received src IP is %q", req.GetUsingSrc().GetIP(), addr.IP.String())
 
 		response.SrcIpNatDetected = true
 	}
 
-	if int(req.UsingSrc.Port) != addr.Port {
+	if int(req.GetUsingSrc().GetPort()) != addr.Port {
 		logger.V(log.DEBUG).Infof("Received NAT packet from endpoint %q, cluster %q, where NAT on the source has been detected, "+
 			"src port changed",
-			req.Sender.GetEndpointId(), req.Sender.GetClusterId())
-		logger.V(log.DEBUG).Infof("Original src IP was %q, received src IP is %q", req.UsingSrc.IP, addr.IP.String())
+			req.GetSender().GetEndpointId(), req.GetSender().GetClusterId())
+		logger.V(log.DEBUG).Infof("Original src IP was %q, received src IP is %q", req.GetUsingSrc().GetIP(), addr.IP.String())
 
 		response.SrcPortNatDetected = true
 	}
 
 	// Detect DST NAT with a naive implementation that assumes that we always receive on the PrivateIP,
 	// if we will listen at some point on multiple addresses we will need to implement the
 	// unix.IP_RECVORIGDSTADDR on the UDP socket, and the go recvmsg implementation instead of readfrom
-	if req.UsingDst.IP != localEndpointSpec.PrivateIP {
+	if req.GetUsingDst().GetIP() != localEndpointSpec.PrivateIP {
 		response.DstIpNatDetected = true
 	}
 
-	if response.SrcPortNatDetected || response.SrcIpNatDetected || response.DstIpNatDetected {
+	if response.GetSrcPortNatDetected() || response.GetSrcIpNatDetected() || response.GetDstIpNatDetected() {
 		response.Response = proto.ResponseType_NAT_DETECTED
 	} else {
 		response.Response = proto.ResponseType_OK
@@ -117,7 +117,7 @@ func (nd *natDiscovery) sendResponseToAddress(response *proto.SubmarinerNATDisco
 	}
 
 	logger.V(log.DEBUG).Infof("Sending response to %s:%d - REQUEST_NUMBER: 0x%x, RESPONSE: %v, SENDER: %q, RECEIVER: %q",
-		addr.IP.String(), addr.Port, response.RequestNumber, response.Response, response.GetSenderEndpointID(),
+		addr.IP.String(), addr.Port, response.GetRequestNumber(), response.GetResponse(), response.GetSenderEndpointID(),
 		response.GetReceiverEndpointID())
 
 	if length, err := nd.serverUDPWrite(buf, addr); err != nil {