From 0c08ac84c5d68087505d20acb8fb2e3891366476 Mon Sep 17 00:00:00 2001 From: Stephen Kitt Date: Fri, 30 Aug 2024 10:53:44 +0200 Subject: [PATCH] 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 --- .../healthchecker/healthchecker.go | 6 ++-- pkg/cableengine/healthchecker/pinger.go | 8 ++--- pkg/ipam/ippool.go | 32 ++++++++++--------- pkg/types/types.go | 4 +-- test/e2e/framework/dataplane.go | 16 ++++++---- 5 files changed, 35 insertions(+), 31 deletions(-) diff --git a/pkg/cableengine/healthchecker/healthchecker.go b/pkg/cableengine/healthchecker/healthchecker.go index 03f3adeaf9..ebe6e3b119 100644 --- a/pkg/cableengine/healthchecker/healthchecker.go +++ b/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 diff --git a/pkg/cableengine/healthchecker/pinger.go b/pkg/cableengine/healthchecker/pinger.go index 8f2d44336b..ef2675d45d 100644 --- a/pkg/cableengine/healthchecker/pinger.go +++ b/pkg/cableengine/healthchecker/pinger.go @@ -34,7 +34,7 @@ const ( ) var ( - defaultMaxPacketLossCount uint = 5 + defaultMaxPacketLossCount = 5 defaultPingInterval = 1 * time.Second @@ -59,7 +59,7 @@ type PingerConfig struct { IP string Interval time.Duration Timeout time.Duration - MaxPacketLossCount uint + MaxPacketLossCount int } type pingerInfo struct { @@ -67,7 +67,7 @@ type pingerInfo struct { 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() diff --git a/pkg/ipam/ippool.go b/pkg/ipam/ippool.go index 6fc0fbbdb2..7ed1d6e6e4 100644 --- a/pkg/ipam/ippool.go +++ b/pkg/ipam/ippool.go @@ -26,6 +26,7 @@ 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" ) @@ -33,7 +34,7 @@ import ( 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() diff --git a/pkg/types/types.go b/pkg/types/types.go index e376a645ec..6f5c8f3c9e 100644 --- a/pkg/types/types.go +++ b/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"` } diff --git a/test/e2e/framework/dataplane.go b/test/e2e/framework/dataplane.go index d6ac4283d9..a28927a795 100644 --- a/test/e2e/framework/dataplane.go +++ b/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,