Adjust golangci-lint linters

.golangci.yml

@@ -50,28 +50,40 @@ linters-settings:
 linters:
   disable-all: true
   enable:
+    - asasalint
     - asciicheck
     - bidichk
     - bodyclose
+    # - canonicalheader # This is a slow linter and we don't use the net/http.Header API
+    - containedctx
     - contextcheck
     - copyloopvar
     # - cyclop # This is equivalent to gocyclo
+    - decorder
     # - depguard # depguard now denies by default, it should only be enabled if we actually use it
     - dogsled
     - dupl
+    - dupword
     - durationcheck
     - errcheck
+    - errchkjson
     - errorlint
     - errname
+    # - execinquery # No SQL
     - exhaustive
-    # - exhaustivestruct # 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
     - gocognit
     - goconst
     - gocritic
@@ -83,50 +95,66 @@ linters:
     - gofumpt
     - goheader
     - goimports
-    # - golint # Deprecated since v1.41.0
-    # - 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
-    # - ifshort # This is a style preference and doesn't seem compelling
+    - 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
-    # - interfacer # Deprecated since v1.38.0
     - lll
+    - loggercheck
+    - maintidx
     - makezero
-    # - maligned # Deprecated since v1.38.0
+    - 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
+    # - nosprintfhostport # The use of this is very narrow
     # - paralleltest # Not relevant for Ginkgo UTs
+    - perfsprint
     - prealloc
     - predeclared
     - promlinter
+    # - protogetter # We don't use protobuf
+    - reassign
+    - recvcheck
     - revive
     # - rowserrcheck # We don't use SQL
+    # - sloglint # We don't use log/slog
     # - scopelint # Deprecated since v1.39.0
     # - 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 # 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
     - typecheck
     - unconvert
     - unparam
     - unused
+    - usestdlibvars
     # - varnamelen # It doesn't seem necessary to enforce a minimum variable name length
     - wastedassign
     - whitespace

pkg/aws/ec2helpers.go

@@ -19,7 +19,6 @@ limitations under the License.
 package aws
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/service/ec2/types"
@@ -41,7 +40,7 @@ func ec2Tag(key, value string) types.Tag {
 }
 
 func ec2FilterByTag(tag types.Tag) types.Filter {
-	return ec2Filter(fmt.Sprintf("tag:%s", *tag.Key), *tag.Value)
+	return ec2Filter("tag:"+*tag.Key, *tag.Value)
 }
 
 func hasTag(tags []types.Tag, desired types.Tag) bool {

pkg/aws/errors.go

@@ -30,7 +30,7 @@ type notFoundError struct {
 }
 
 func (e notFoundError) Error() string {
-	return fmt.Sprintf("%s not found", e.s)
+	return e.s + " not found"
 }
 
 func newNotFoundError(msg string, args ...interface{}) error {

pkg/aws/ocpgwdeployer_test.go

@@ -212,7 +212,7 @@ func testCleanup() {
 				delete(t.machineSets, *t.subnets[i].AvailabilityZone)
 			}
 
-			Expect(t.machineSets).To(HaveLen(0), "Unexpected machine sets deleted: %#v", t.machineSets)
+			Expect(t.machineSets).To(BeEmpty(), "Unexpected machine sets deleted: %#v", t.machineSets)
 		})
 	})
 
@@ -369,7 +369,7 @@ func (t *gatewayDeployerTestDriver) testDeploySuccess(msgPrefix, msgSuffix strin
 			delete(t.machineSets, *t.expectedSubnetsDeployed[i].AvailabilityZone)
 		}
 
-		Expect(t.machineSets).To(HaveLen(0), "Unexpected machine sets deployed: %#v", t.machineSets)
+		Expect(t.machineSets).To(BeEmpty(), "Unexpected machine sets deployed: %#v", t.machineSets)
 	})
 }
 

pkg/aws/securitygroups.go

@@ -20,7 +20,6 @@ package aws
 
 import (
 	"context"
-	"fmt"
 	"strings"
 	"time"
 
@@ -145,19 +144,19 @@ func (ac *awsCloud) allowPortInCluster(vpcID string, port uint16, protocol strin
 		}
 	}
 
-	err = ac.createClusterSGRule(workerGroupID, workerGroupID, port, protocol, fmt.Sprintf("%s between the workers", internalTraffic))
+	err = ac.createClusterSGRule(workerGroupID, workerGroupID, port, protocol, internalTraffic+" between the workers")
 	if err != nil {
 		return err
 	}
 
 	err = ac.createClusterSGRule(workerGroupID, controlPlaneGroupID, port, protocol,
-		fmt.Sprintf("%s from worker to control plane nodes", internalTraffic))
+		internalTraffic+" from worker to control plane nodes")
 	if err != nil {
 		return err
 	}
 
 	return ac.createClusterSGRule(controlPlaneGroupID, workerGroupID, port, protocol,
-		fmt.Sprintf("%s from control plane to worker nodes", internalTraffic))
+		internalTraffic+" from control plane to worker nodes")
 }
 
 func (ac *awsCloud) createPublicSGRule(groupID *string, port uint16, protocol, description string) error {

pkg/azure/cloud_info.go

@@ -369,7 +369,7 @@ func (c *CloudInfo) getPublicIP(ctx context.Context, publicIPName string, pubIPC
 }
 
 func (c *CloudInfo) createPublicIP(ctx context.Context, ipName string, ipClient *armnetwork.PublicIPAddressesClient,
-) (ip armnetwork.PublicIPAddress, err error) {
+) (armnetwork.PublicIPAddress, error) {
 	ipVersion := armnetwork.IPVersionIPv4
 	ipAllocMethod := armnetwork.IPAllocationMethodStatic
 	skuName := armnetwork.PublicIPAddressSKUNameStandard
@@ -390,18 +390,18 @@ func (c *CloudInfo) createPublicIP(ctx context.Context, ipName string, ipClient
 			},
 		}, nil)
 	if err != nil {
-		return ip, errors.Wrapf(err, "cannot create public ip address: %q", ipName)
+		return armnetwork.PublicIPAddress{}, errors.Wrapf(err, "cannot create public ip address: %q", ipName)
 	}
 
 	resp, err := poller.PollUntilDone(ctx, nil)
 	if err != nil {
-		return ip, errors.Wrapf(err, "cannot get public ip address create or update response: %q", ipName)
+		return armnetwork.PublicIPAddress{}, errors.Wrapf(err, "cannot get public ip address create or update response: %q", ipName)
 	}
 
 	return resp.PublicIPAddress, nil
 }
 
-func (c *CloudInfo) deletePublicIP(ctx context.Context, ipClient *armnetwork.PublicIPAddressesClient, ipName string) (err error) {
+func (c *CloudInfo) deletePublicIP(ctx context.Context, ipClient *armnetwork.PublicIPAddressesClient, ipName string) error {
 	poller, err := ipClient.BeginDelete(ctx, c.BaseGroupName, ipName, nil)
 	if err != nil {
 		return errors.Wrapf(err, "failed to delete public ip : %q", ipName)

pkg/gcp/firewall_rules.go

@@ -33,7 +33,7 @@ const (
 	submarinerGatewayNodeTag = "submariner-io-gateway-node"
 )
 
-func newExternalFirewallRules(projectID, infraID string, ports []api.PortSpec) (ingress *compute.Firewall) {
+func newExternalFirewallRules(projectID, infraID string, ports []api.PortSpec) *compute.Firewall {
 	ingressName := generateRuleName(infraID, publicPortsRuleName)
 
 	// We want the external firewall rules to be applied only to Gateway nodes. So, we use the TargetTags
@@ -52,12 +52,12 @@ func newInternalFirewallRule(projectID, infraID string, ports []api.PortSpec) *c
 
 	rule := newFirewallRule(projectID, infraID, ingressName, ingressDirection, ports)
 	rule.TargetTags = []string{
-		fmt.Sprintf("%s-worker", infraID),
-		fmt.Sprintf("%s-master", infraID),
+		infraID + "-worker",
+		infraID + "-master",
 	}
 	rule.SourceTags = []string{
-		fmt.Sprintf("%s-worker", infraID),
-		fmt.Sprintf("%s-master", infraID),
+		infraID + "-worker",
+		infraID + "-master",
 	}
 
 	return rule
@@ -85,6 +85,6 @@ func newFirewallRule(projectID, infraID, name, direction string, ports []api.Por
 	}
 }
 
-func generateRuleName(infraID, name string) (ingressName string) {
+func generateRuleName(infraID, name string) string {
 	return fmt.Sprintf("%s-%s-ingress", infraID, name)
 }

pkg/ocp/machinesets.go

@@ -219,11 +219,11 @@ func (msd *k8sMachineSetDeployer) List() ([]unstructured.Unstructured, error) {
 func RemoveDuplicates(machineSets []unstructured.Unstructured, gwNodes []v1.Node) []v1.Node {
 	var resultNode []v1.Node
 
-	for i := 0; i < len(gwNodes); i++ {
+	for i := range gwNodes {
 		addToResult := true
 
-		for i := 0; i < len(machineSets); i++ {
-			if strings.Contains(gwNodes[i].GetName(), machineSets[i].GetName()) {
+		for j := range machineSets {
+			if strings.Contains(gwNodes[i].GetName(), machineSets[j].GetName()) {
 				addToResult = false
 				break
 			}

pkg/ocp/machinesets_test.go

@@ -184,7 +184,7 @@ var _ = Describe("K8s MachineSetDeployer", func() {
 				machineSetList, err := deployer.List()
 				Expect(err).To(Succeed())
 
-				Expect(len(machineSetList)).To(Equal(1))
+				Expect(machineSetList).To(HaveLen(1))
 				Expect(machineSetList[0].GetName()).To(Equal(machineSetName))
 			})
 		})
@@ -193,7 +193,7 @@ var _ = Describe("K8s MachineSetDeployer", func() {
 			It("should not return an error", func() {
 				machineSetList, err := deployer.List()
 				Expect(err).To(Succeed())
-				Expect(len(machineSetList)).To(Equal(0))
+				Expect(machineSetList).To(BeEmpty())
 			})
 		})
 	})

pkg/rhos/ocpgwdeployer.go

@@ -214,7 +214,7 @@ func (d *ocpGatewayDeployer) deployGWNode(gatewayCount int,
 func (d *ocpGatewayDeployer) deployDedicatedGWNode(gatewayNodesToDeploy int, useInternalSG bool,
 	status reporter.Interface,
 ) error {
-	for i := 0; i < gatewayNodesToDeploy; i++ {
+	for i := range gatewayNodesToDeploy {
 		gwNodeName := d.InfraID + "-submariner-gw" + strconv.Itoa(i)
 		status.Start("Deploying dedicated Submariner gateway node %s", gwNodeName)