Rework integer type selection

Following the recent gosec-related int work (to annotate known-safe
conversions), this furthers that by:

* using ints instead of uints for healthcheck configuration and
  tracking
* using uint32 instead of int for IP pool sizes, since their use is
  tied to uint32 representations of IP addresses
* using fmt.Sprintf instead of manual string concatenation to
  construct the connectivity verification command

Signed-off-by: Stephen Kitt <skitt@redhat.com>

pkg/cableengine/healthchecker/healthchecker.go

@@ -55,8 +55,8 @@ type Config struct {
 	WatcherConfig      *watcher.Config
 	EndpointNamespace  string
 	ClusterID          string
-	PingInterval       uint
-	MaxPacketLossCount uint
+	PingInterval       int
+	MaxPacketLossCount int
 	NewPinger          func(PingerConfig) PingerInterface
 }
 
@@ -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) //nolint:gosec // We can safely ignore integer conversion error
+		pingerConfig.Interval = time.Second * time.Duration(h.config.PingInterval)
 	}
 
 	newPingerFunc := h.config.NewPinger

pkg/cableengine/healthchecker/pinger.go

@@ -34,7 +34,7 @@ const (
 )
 
 var (
-	defaultMaxPacketLossCount uint = 5
+	defaultMaxPacketLossCount = 5
 
 	defaultPingInterval = 1 * time.Second
 
@@ -59,15 +59,15 @@ type PingerConfig struct {
 	IP                 string
 	Interval           time.Duration
 	Timeout            time.Duration
-	MaxPacketLossCount uint
+	MaxPacketLossCount int
 }
 
 type pingerInfo struct {
 	sync.Mutex
 	ip                 string
 	pingInterval       time.Duration
 	pingTimeout        time.Duration
-	maxPacketLossCount uint
+	maxPacketLossCount int
 	statistics         statistics
 	failureMsg         string
 	connectionStatus   ConnectionStatus
@@ -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 uint(pinger.PacketsSent-pinger.PacketsRecv) > p.maxPacketLossCount {
+		if pinger.PacketsSent-pinger.PacketsRecv > p.maxPacketLossCount {
 			p.Lock()
 			defer p.Unlock()
 

pkg/ipam/ippool.go

@@ -26,14 +26,15 @@ import (
 	"sync"
 
 	"github.com/emirpasic/gods/maps/treemap"
+	"github.com/emirpasic/gods/utils"
 	"github.com/pkg/errors"
 	"github.com/submariner-io/submariner/pkg/globalnet/metrics"
 )
 
 type IPPool struct {
 	cidr      string
 	network   *net.IPNet
-	size      int
+	size      uint32
 	available *treemap.Map // int IP is the key, string IP is the value
 	mutex     sync.RWMutex
 }
@@ -46,48 +47,49 @@ func NewIPPool(cidr string) (*IPPool, error) {
 
 	ones, totalbits := network.Mask.Size()
 
-	size := int(math.Exp2(float64(totalbits-ones))) - 2 // don't count net and broadcast
-	if size < 2 {
+	size := uint32(math.Exp2(float64(totalbits - ones)))
+	if size < 4 {
 		return nil, fmt.Errorf("invalid prefix for CIDR %q", cidr)
 	}
+	size -= 2 // don't count net and broadcast
 
 	pool := &IPPool{
 		cidr:      cidr,
 		network:   network,
 		size:      size,
-		available: treemap.NewWithIntComparator(),
+		available: treemap.NewWith(utils.UInt32Comparator),
 	}
 
 	startingIP := ipToInt(pool.network.IP) + 1
 
-	for i := 0; i < pool.size; i++ {
+	for i := uint32(0); i < pool.size; i++ {
 		intIP := startingIP + i
 		ip := intToIP(intIP).String()
 		pool.available.Put(intIP, ip)
 	}
 
-	metrics.RecordAvailability(cidr, size)
+	metrics.RecordAvailability(cidr, int(size))
 
 	return pool, nil
 }
 
-func ipToInt(ip net.IP) int {
+func ipToInt(ip net.IP) uint32 {
 	intIP := ip
 	if len(ip) == 16 {
 		intIP = ip[12:16]
 	}
 
-	return int(binary.BigEndian.Uint32(intIP))
+	return binary.BigEndian.Uint32(intIP)
 }
 
-func intToIP(ip int) net.IP {
+func intToIP(ip uint32) net.IP {
 	netIP := make(net.IP, 4)
-	binary.BigEndian.PutUint32(netIP, uint32(ip)) //nolint:gosec // int -> uint32 conversion won't overflow here
+	binary.BigEndian.PutUint32(netIP, ip)
 
 	return netIP
 }
 
-func StringIPToInt(stringIP string) int {
+func StringIPToInt(stringIP string) uint32 {
 	return ipToInt(net.ParseIP(stringIP))
 }
 
@@ -125,11 +127,11 @@ func (p *IPPool) Allocate(num int) ([]string, error) {
 
 	retIPs := make([]string, num)
 
-	var prevIntIP, firstIntIP, current int
+	var prevIntIP, firstIntIP, current uint32
 
 	iter := p.available.Iterator()
 	for iter.Next() {
-		intIP := iter.Key().(int)
+		intIP := iter.Key().(uint32)
 		retIPs[current] = iter.Value().(string)
 
 		if current == 0 || prevIntIP+1 != intIP {
@@ -144,7 +146,7 @@ func (p *IPPool) Allocate(num int) ([]string, error) {
 		prevIntIP = intIP
 		current++
 
-		if current == num {
+		if int(current) == num {
 			for i := 0; i < num; i++ {
 				p.available.Remove(firstIntIP)
 
@@ -183,7 +185,7 @@ func (p *IPPool) Reserve(ips ...string) error {
 		return nil
 	}
 
-	intIPs := make([]int, num)
+	intIPs := make([]uint32, num)
 
 	p.mutex.Lock()
 	defer p.mutex.Unlock()

pkg/types/types.go

@@ -46,7 +46,7 @@ type SubmarinerSpecification struct {
 	HealthCheckEnabled            bool `default:"true"`
 	Uninstall                     bool
 	HaltOnCertError               bool `split_words:"true"`
-	HealthCheckInterval           uint
-	HealthCheckMaxPacketLossCount uint
+	HealthCheckInterval           int
+	HealthCheckMaxPacketLossCount int
 	MetricsPort                   int `default:"32780"`
 }

test/e2e/framework/dataplane.go

@@ -21,7 +21,6 @@ package framework
 import (
 	"context"
 	"fmt"
-	"strconv"
 
 	. "github.com/onsi/gomega"
 	resourceUtil "github.com/submariner-io/admiral/pkg/resource"
@@ -157,12 +156,15 @@ 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.FormatUint(uint64(connector.Config.NumOfDataBufs), 10) + "); do echo" +
-			" [dataplane] connector says " + connector.Config.Data + "; 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"}
+		cmd := []string{
+			"sh",
+			"-c",
+			fmt.Sprintf("for j in $(seq 1 %d); do echo [dataplane] connector says %s; done"+
+				" | for i in $(seq %d); do if nc -v %s %d -w %d; then break; else sleep %d; fi; done",
+				connector.Config.NumOfDataBufs, connector.Config.Data, listener.Config.ConnectionAttempts, remoteIP, connector.Config.Port,
+				listener.Config.ConnectionTimeout, listener.Config.ConnectionTimeout/2,
+			),
+		}
 
 		stdOut, _, err := p.Framework.ExecWithOptions(context.TODO(), &framework.ExecOptions{
 			Command:            cmd,