Bump shipyard and address golangci-lint v1.60.3 errors

Mostly gosec "integer overflow conversion" issues.

Signed-off-by: Tom Pantelis <tompantelis@gmail.com>

.golangci.yml

@@ -55,6 +55,7 @@ linters:
     - bodyclose
     - containedctx
     - contextcheck
+    - copyloopvar
     # - cyclop # This is equivalent to gocyclo
     - decorder
     - depguard
@@ -69,7 +70,6 @@ linters:
     # - execinquery # No SQL
     - exhaustive
     # - exhauststruct # Not recommended for general use - meant to be used only for special cases
-    - exportloopref
     # - 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

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/coreos/go-iptables v0.7.0
 	github.com/emirpasic/gods v1.18.1
 	github.com/kelseyhightower/envconfig v1.4.0
-	github.com/onsi/ginkgo/v2 v2.19.1
+	github.com/onsi/ginkgo/v2 v2.20.0
 	github.com/onsi/gomega v1.34.1
 	github.com/ovn-org/libovsdb v0.7.0
 	github.com/ovn-org/ovn-kubernetes/go-controller v0.0.0-20220511131059-ac1ce4691c0f
@@ -16,7 +16,7 @@ require (
 	github.com/prometheus-community/pro-bing v0.4.1
 	github.com/prometheus/client_golang v1.19.1
 	github.com/submariner-io/admiral v0.19.0-m2.0.20240819074554-489d309b67d9
-	github.com/submariner-io/shipyard v0.19.0-m2
+	github.com/submariner-io/shipyard v0.19.0-m2.0.20240827150103-0a3b7a0f1ca0
 	github.com/vishvananda/netlink v1.3.0
 	golang.org/x/net v0.28.0
 	golang.org/x/sys v0.24.0
@@ -54,7 +54,7 @@ require (
 	github.com/google/gnostic-models v0.6.8 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/gofuzz v1.2.0 // indirect
-	github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af // indirect
+	github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/websocket v1.5.0 // indirect
 	github.com/imdario/mergo v0.3.12 // indirect
@@ -89,7 +89,7 @@ require (
 	golang.org/x/term v0.23.0 // indirect
 	golang.org/x/text v0.17.0 // indirect
 	golang.org/x/time v0.6.0 // indirect
-	golang.org/x/tools v0.23.0 // indirect
+	golang.org/x/tools v0.24.0 // indirect
 	golang.zx2c4.com/wireguard v0.0.0-20230325221338-052af4a8072b // indirect
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect

go.sum

@@ -218,8 +218,8 @@ github.com/google/gofuzz v1.2.0 h1:xRy4A+RhZaiKjJ1bPfwQ8sedCA+YS2YcCHW6ec7JMi0=
 github.com/google/gofuzz v1.2.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/martian v2.1.0+incompatible/go.mod h1:9I4somxYTbIHy5NJKHRl3wXiIaQGbYVAs8BPL6v8lEs=
 github.com/google/pprof v0.0.0-20181206194817-3ea8567a2e57/go.mod h1:zfwlbNMJ+OItoe0UupaVj+oy1omPYYDuagoSzA8v9mc=
-github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af h1:kmjWCqn2qkEml422C2Rrd27c3VGxi6a/6HNq8QmHRKM=
-github.com/google/pprof v0.0.0-20240525223248-4bfdf5a9a2af/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo=
+github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8 h1:FKHo8hFI3A+7w0aUQuYXQ+6EN5stWmeY/AZqtM8xk9k=
+github.com/google/pprof v0.0.0-20240727154555-813a5fbdbec8/go.mod h1:K1liHPHnj73Fdn/EKuT8nrFqBihUSKXoLYU0BuatOYo=
 github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
@@ -344,8 +344,8 @@ github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108
 github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY=
 github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc=
 github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
-github.com/onsi/ginkgo/v2 v2.19.1 h1:QXgq3Z8Crl5EL1WBAC98A5sEBHARrAJNzAmMxzLcRF0=
-github.com/onsi/ginkgo/v2 v2.19.1/go.mod h1:O3DtEWQkPa/F7fBMgmZQKKsluAy8pd3rEQdrjkPb9zA=
+github.com/onsi/ginkgo/v2 v2.20.0 h1:PE84V2mHqoT1sglvHc8ZdQtPcwmvvt29WLEEO3xmdZw=
+github.com/onsi/ginkgo/v2 v2.20.0/go.mod h1:lG9ey2Z29hR41WMVthyJBGUBcBhGOtoPF2VFMvBXFCI=
 github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
 github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
 github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY=
@@ -442,8 +442,8 @@ github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsT
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 github.com/submariner-io/admiral v0.19.0-m2.0.20240819074554-489d309b67d9 h1:fHQMyLyHCQQl29rW4Plt8Wuq46d2rBK1+cWgGuRf4Dg=
 github.com/submariner-io/admiral v0.19.0-m2.0.20240819074554-489d309b67d9/go.mod h1:f4xzFML4SRqETWcyZ32iTy8DH0gLUt2+jrYu6VGXL4Q=
-github.com/submariner-io/shipyard v0.19.0-m2 h1:vOVbz4p+391s6H9lO6JL4t+gNoRdhmbESbx2vzmcGQg=
-github.com/submariner-io/shipyard v0.19.0-m2/go.mod h1:91eQ4sF5XgbJxJUrXvY7u33gGGjaz5ofjLYaZyYXEls=
+github.com/submariner-io/shipyard v0.19.0-m2.0.20240827150103-0a3b7a0f1ca0 h1:6Z3RbaML+SZfoW5h5fuDB76ApJASdXm23c6OIIKGyFQ=
+github.com/submariner-io/shipyard v0.19.0-m2.0.20240827150103-0a3b7a0f1ca0/go.mod h1:CZTfxB5frorCIECWNwZ4asC8o2qtJLeFXoXY5UNX2Sg=
 github.com/tidwall/pretty v1.0.0/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20170815181823-89b8d40f7ca8/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
 github.com/tmc/grpc-websocket-proxy v0.0.0-20190109142713-0ad062ec5ee5/go.mod h1:ncp9v5uamzpCO7NfCPTXjqaC+bZgJeR0sMTm6dMHP7U=
@@ -599,8 +599,8 @@ golang.org/x/tools v0.0.0-20190920225731-5eefd052ad72/go.mod h1:b+2E5dAYhXwXZwtn
 golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
 golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE=
 golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
-golang.org/x/tools v0.23.0 h1:SGsXPZ+2l4JsgaCKkx+FQ9YZ5XEtA1GZYuoDjenLjvg=
-golang.org/x/tools v0.23.0/go.mod h1:pnu6ufv6vQkll6szChhK3C3L/ruaIv5eBeztNG8wtsI=
+golang.org/x/tools v0.24.0 h1:J1shsA93PJUEVaUSaay7UXAyE8aimq3GW0pjlolpa24=
+golang.org/x/tools v0.24.0/go.mod h1:YhNqVBIfWHdzvTLs0d8LCuMhkKUgSUKldakyV7W/WDQ=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

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

@@ -63,6 +63,7 @@ func parsePort(port string) (int32, error) {
 		return -1, errors.Errorf("port %s is > 65535", port)
 	}
 
+	//nolint:gosec // We can safely ignore integer conversion error
 	return int32(portInt), nil
 }
 

pkg/cable/libreswan/libreswan.go

@@ -90,8 +90,6 @@ type specification struct {
 	NATTPort    string `default:"4500"`
 }
 
-const defaultNATTPort = "4500"
-
 // NewLibreswan starts an IKE daemon using Libreswan and configures it to manage Submariner's endpoints.
 func NewLibreswan(localEndpoint *submendpoint.Local, _ *types.SubmarinerCluster) (cable.Driver, error) {
 	// We'll panic if localEndpoint is nil, this is intentional
@@ -102,12 +100,14 @@ func NewLibreswan(localEndpoint *submendpoint.Local, _ *types.SubmarinerCluster)
 		return nil, errors.Wrapf(err, "error processing environment config for %s", cable.IPSecEnvPrefix)
 	}
 
-	defaultNATTPort, err := strconv.ParseUint(ipSecSpec.NATTPort, 10, 16)
+	port, err := strconv.ParseUint(ipSecSpec.NATTPort, 10, 16)
 	if err != nil {
 		return nil, errors.Wrap(err, "error parsing CR_IPSEC_NATTPORT environment variable")
 	}
 
-	nattPort, err := localEndpoint.Spec().GetBackendPort(subv1.UDPPortConfig, int32(defaultNATTPort))
+	defaultNATTPort := int32(port) //nolint:gosec // We can safely ignore integer conversion error
+
+	nattPort, err := localEndpoint.Spec().GetBackendPort(subv1.UDPPortConfig, defaultNATTPort)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error parsing %q from local endpoint", subv1.UDPPortConfig)
 	}
@@ -143,7 +143,7 @@ func NewLibreswan(localEndpoint *submendpoint.Local, _ *types.SubmarinerCluster)
 		debug:                 ipSecSpec.Debug,
 		logFile:               ipSecSpec.LogFile,
 		ipSecNATTPort:         strconv.Itoa(int(nattPort)),
-		defaultNATTPort:       int32(defaultNATTPort),
+		defaultNATTPort:       defaultNATTPort,
 		localEndpoint:         *localEndpoint.Spec(),
 		connections:           []subv1.Connection{},
 		forceUDPEncapsulation: ipSecSpec.ForceEncaps,

pkg/cable/libreswan/libreswan_internal_test.go

@@ -66,7 +66,7 @@ func testIPsecPortConfiguration() {
 
 	When("NewLibreswan is called with no port environment variables set", func() {
 		It("should set the port fields from the defaults in the specification definition", func() {
-			Expect(t.driver.ipSecNATTPort).To(Equal(defaultNATTPort))
+			Expect(t.driver.ipSecNATTPort).To(Equal("4500"))
 		})
 	})
 

pkg/cable/wireguard/driver.go

@@ -70,7 +70,7 @@ func init() {
 
 type specification struct {
 	PSK      string `default:"default psk"`
-	NATTPort int    `default:"4500"`
+	NATTPort int32  `default:"4500"`
 }
 
 type wireguard struct {
@@ -133,7 +133,7 @@ func NewDriver(localEndpoint *endpoint.Local, _ *types.SubmarinerCluster) (cable
 		return nil, errors.Wrap(err, "error generating private key")
 	}
 
-	port, err := localEndpoint.Spec().GetBackendPort(v1.UDPPortConfig, int32(w.spec.NATTPort))
+	port, err := localEndpoint.Spec().GetBackendPort(v1.UDPPortConfig, w.spec.NATTPort)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error parsing %q from local endpoint", v1.UDPPortConfig)
 	}
@@ -266,7 +266,7 @@ func (w *wireguard) ConnectToEndpoint(endpointInfo *natdiscovery.NATEndpointInfo
 	logger.V(log.DEBUG).Infof("Adding connection for cluster %s, %v", remoteEndpoint.Spec.ClusterID, connection)
 	w.connections[remoteEndpoint.Spec.ClusterID] = connection
 
-	port, err := remoteEndpoint.Spec.GetBackendPort(v1.UDPPortConfig, int32(w.spec.NATTPort))
+	port, err := remoteEndpoint.Spec.GetBackendPort(v1.UDPPortConfig, w.spec.NATTPort)
 	if err != nil {
 		logger.Warningf("Error parsing %q from remote endpoint %q - using port %dº instead: %v", v1.UDPPortConfig,
 			remoteEndpoint.Spec.CableName, w.spec.NATTPort, err)

pkg/cableengine/healthchecker/healthchecker.go

@@ -161,7 +161,7 @@ func (h *controller) endpointCreatedOrUpdated(obj runtime.Object, _ int) bool {
 	}
 
 	if h.config.PingInterval != 0 {
-		pingerConfig.Interval = time.Second * time.Duration(h.config.PingInterval)
+		pingerConfig.Interval = time.Second * time.Duration(h.config.PingInterval) //nolint:gosec // We can safely ignore integer conversion error
 	}
 
 	newPingerFunc := h.config.NewPinger

pkg/cableengine/healthchecker/pinger.go

@@ -153,7 +153,7 @@ func (p *pingerInfo) doPing() error {
 		}
 
 		// Pinger will mark a connection as an error if the packet loss reaches the threshold
-		if pinger.PacketsSent-pinger.PacketsRecv > int(p.maxPacketLossCount) {
+		if uint(pinger.PacketsSent-pinger.PacketsRecv) > p.maxPacketLossCount {
 			p.Lock()
 			defer p.Unlock()
 
@@ -204,16 +204,20 @@ func (p *pingerInfo) GetLatencyInfo() *LatencyInfo {
 	p.Lock()
 	defer p.Unlock()
 
+	toDurationString := func(v uint64) string {
+		return time.Duration(v).String() //nolint:gosec // We can safely ignore integer conversion error
+	}
+
 	return &LatencyInfo{
 		IP:               p.ip,
 		ConnectionStatus: p.connectionStatus,
 		ConnectionError:  p.failureMsg,
 		Spec: &submarinerv1.LatencyRTTSpec{
-			Last:    time.Duration(p.statistics.lastRtt).String(),
-			Min:     time.Duration(p.statistics.minRtt).String(),
-			Average: time.Duration(p.statistics.mean).String(),
-			Max:     time.Duration(p.statistics.maxRtt).String(),
-			StdDev:  time.Duration(p.statistics.stdDev).String(),
+			Last:    toDurationString(p.statistics.lastRtt),
+			Min:     toDurationString(p.statistics.minRtt),
+			Average: toDurationString(p.statistics.mean),
+			Max:     toDurationString(p.statistics.maxRtt),
+			StdDev:  toDurationString(p.statistics.stdDev),
 		},
 	}
 }

pkg/cableengine/healthchecker/statistics.go

@@ -46,6 +46,8 @@ func (s *statistics) update(rtt uint64) {
 		s.previousRtts[1] = s.previousRtts[s.size-1]
 		s.sum = s.previousRtts[0] + s.previousRtts[1]
 		s.mean = s.sum / 2
+
+		//nolint:gosec // Ignore "integer overflow conversion uint64 -> int64" - we subtract uint64's and want to preserve sign.
 		s.sqrDiff = uint64(int64(s.previousRtts[0]-s.mean)*int64(s.previousRtts[0]-s.mean) +
 			int64(s.previousRtts[1]-s.mean)*int64(s.previousRtts[1]-s.mean))
 	}
@@ -63,6 +65,8 @@ func (s *statistics) update(rtt uint64) {
 		s.sum += rtt
 		oldMean := s.mean
 		s.mean = s.sum / (s.index + 1)
+
+		//nolint:gosec // Ignore "integer overflow conversion uint64 -> int64" - we subtract uint64's and want to preserve sign.
 		s.sqrDiff += uint64(int64(rtt-oldMean) * int64(rtt-s.mean))
 		s.stdDev = uint64(math.Sqrt(float64(s.sqrDiff / (s.index + 1))))
 	} else {

pkg/ipam/ippool.go

@@ -82,7 +82,7 @@ func ipToInt(ip net.IP) int {
 
 func intToIP(ip int) net.IP {
 	netIP := make(net.IP, 4)
-	binary.BigEndian.PutUint32(netIP, uint32(ip))
+	binary.BigEndian.PutUint32(netIP, uint32(ip)) //nolint:gosec // int -> uint32 conversion won't overflow here
 
 	return netIP
 }

pkg/natdiscovery/request_handle.go

@@ -38,7 +38,7 @@ func (nd *natDiscovery) handleRequestFromAddress(req *proto.SubmarinerNATDiscove
 		},
 		Receiver: req.Sender,
 		ReceivedSrc: &proto.IPPortPair{
-			Port: uint32(addr.Port),
+			Port: uint32(addr.Port), //nolint:gosec // We can safely ignore integer conversion error
 			IP:   addr.IP.String(),
 		},
 	}

test/e2e/dataplane/tcp_gn_pod_connectivity.go

@@ -98,8 +98,8 @@ var _ = Describe("Basic TCP connectivity tests across overlapping clusters witho
 
 					err := util.Update(context.Background(), resource.ForService(framework.KubeClients[lpConfig.Cluster],
 						f.Namespace), service, func(existing *v1.Service) (*v1.Service, error) {
-						existing.Spec.Ports[0].Port = int32(lpConfig.Port)
-						existing.Spec.Ports[0].TargetPort = intstr.FromInt32(int32(lpConfig.Port))
+						existing.Spec.Ports[0].Port = lpConfig.Port
+						existing.Spec.Ports[0].TargetPort = intstr.FromInt32(lpConfig.Port)
 						return existing, nil
 					})
 					Expect(err).To(Succeed())

test/e2e/framework/dataplane.go

@@ -157,12 +157,12 @@ func verifyGlobalnetDatapathConnectivity(p tcp.ConnectivityTestParams, gn Global
 	}
 
 	verifyConnectivity := func(listener *framework.NetworkPod, connector *framework.NetworkPod) {
-		cmd := []string{"sh", "-c", "for j in $(seq 1 " + strconv.Itoa(int(connector.Config.NumOfDataBufs)) + "); do echo" +
+		cmd := []string{"sh", "-c", "for j in $(seq 1 " + strconv.FormatUint(uint64(connector.Config.NumOfDataBufs), 10) + "); do echo" +
 			" [dataplane] connector says " + connector.Config.Data + "; done" +
-			" | for i in $(seq " + strconv.Itoa(int(listener.Config.ConnectionAttempts)) + ");" +
-			" do if nc -v " + remoteIP + " " + strconv.Itoa(connector.Config.Port) + " -w " +
-			strconv.Itoa(int(listener.Config.ConnectionTimeout)) + ";" +
-			" then break; else sleep " + strconv.Itoa(int(listener.Config.ConnectionTimeout/2)) + "; fi; done"}
+			" | for i in $(seq " + strconv.FormatUint(uint64(listener.Config.ConnectionAttempts), 10) + ");" +
+			" do if nc -v " + remoteIP + " " + strconv.FormatUint(uint64(connector.Config.Port), 10) + " -w " +
+			strconv.FormatUint(uint64(listener.Config.ConnectionTimeout), 10) + ";" +
+			" then break; else sleep " + strconv.FormatUint(uint64(listener.Config.ConnectionTimeout/2), 10) + "; fi; done"}
 
 		stdOut, _, err := p.Framework.ExecWithOptions(context.TODO(), &framework.ExecOptions{
 			Command:            cmd,

test/external/dataplane/gn_connectivity.go

@@ -511,7 +511,7 @@ func getPodGlobalIPs(p testParams, g globalnetTestParams, np *framework.NetworkP
 }
 
 func createHeadlessTCPServiceWithoutSelector(f *framework.Framework, cluster framework.ClusterIndex,
-	svcName, portName string, port int,
+	svcName, portName string, port int32,
 ) *v1.Service {
 	serviceSpec := f.NewService(svcName, portName, port, v1.ProtocolTCP, nil, true)
 	sc := framework.KubeClients[cluster].CoreV1().Services(f.Namespace)