fix(NPC): parse NodePorts as unsigned ints

Also separates logic so that it can be tested more easily, and adds unit
tests to make sure there is no regression.

Fixes #1083

pkg/controllers/netpol/network_policy_controller.go

@@ -6,7 +6,6 @@ import (
 	"encoding/base32"
 	"fmt"
 	"net"
-	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -603,26 +602,9 @@ func NewNetworkPolicyController(clientset kubernetes.Interface,
 	npc.serviceClusterIPRange = *ipnet
 
 	// Validate and parse NodePort range
-	nodePortValidator := regexp.MustCompile(`^([0-9]+)[:-]([0-9]+)$`)
-	if matched := nodePortValidator.MatchString(config.NodePortRange); !matched {
-		return nil, fmt.Errorf("failed to parse node port range given: '%s' please see specification in help text", config.NodePortRange)
-	}
-	matches := nodePortValidator.FindStringSubmatch(config.NodePortRange)
-	if len(matches) != 3 {
-		return nil, fmt.Errorf("could not parse port number from range given: '%s'", config.NodePortRange)
-	}
-	port1, err := strconv.ParseInt(matches[1], 10, 16)
-	if err != nil {
-		return nil, fmt.Errorf("could not parse first port number from range given: '%s'", config.NodePortRange)
-	}
-	port2, err := strconv.ParseInt(matches[2], 10, 16)
-	if err != nil {
-		return nil, fmt.Errorf("could not parse second port number from range given: '%s'", config.NodePortRange)
-	}
-	if port1 >= port2 {
-		return nil, fmt.Errorf("port 1 is greater than or equal to port 2 in range given: '%s'", config.NodePortRange)
+	if npc.serviceNodePortRange, err = validateNodePortRange(config.NodePortRange); err != nil {
+		return nil, err
 	}
-	npc.serviceNodePortRange = fmt.Sprintf("%d:%d", port1, port2)
 
 	// Validate and parse ExternalIP service range
 	for _, externalIPRange := range config.ExternalIPCIDRs {

pkg/controllers/netpol/utils.go

@@ -0,0 +1,30 @@
+package netpol
+
+import (
+	"fmt"
+	"regexp"
+	"strconv"
+)
+
+func validateNodePortRange(nodePortOption string) (string, error) {
+	nodePortValidator := regexp.MustCompile(`^([0-9]+)[:-]([0-9]+)$`)
+	if matched := nodePortValidator.MatchString(nodePortOption); !matched {
+		return "", fmt.Errorf("failed to parse node port range given: '%s' please see specification in help text", nodePortOption)
+	}
+	matches := nodePortValidator.FindStringSubmatch(nodePortOption)
+	if len(matches) != 3 {
+		return "", fmt.Errorf("could not parse port number from range given: '%s'", nodePortOption)
+	}
+	port1, err := strconv.ParseUint(matches[1], 10, 16)
+	if err != nil {
+		return "", fmt.Errorf("could not parse first port number from range given: '%s'", nodePortOption)
+	}
+	port2, err := strconv.ParseUint(matches[2], 10, 16)
+	if err != nil {
+		return "", fmt.Errorf("could not parse second port number from range given: '%s'", nodePortOption)
+	}
+	if port1 >= port2 {
+		return "", fmt.Errorf("port 1 is greater than or equal to port 2 in range given: '%s'", nodePortOption)
+	}
+	return fmt.Sprintf("%d:%d", port1, port2), nil
+}

pkg/controllers/netpol/utils_test.go

@@ -0,0 +1,43 @@
+package netpol
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_NewNetworkPolicyController(t *testing.T) {
+	t.Run("Node Port range specified with a hyphen should pass validation", func(t *testing.T) {
+		portRange, err := validateNodePortRange("1000-2000")
+		assert.Nil(t, err)
+		assert.NotEmpty(t, portRange)
+	})
+	t.Run("Node Port range specified with a colon should pass validation", func(t *testing.T) {
+		portRange, err := validateNodePortRange("1000:2000")
+		assert.Nil(t, err)
+		assert.NotEmpty(t, portRange)
+	})
+	t.Run("Node Port range specified with a high port range should work", func(t *testing.T) {
+		portRange, err := validateNodePortRange("40000:42767")
+		assert.Nil(t, err)
+		assert.NotEmpty(t, portRange)
+		portRange, err = validateNodePortRange("50000:65535")
+		assert.Nil(t, err)
+		assert.NotEmpty(t, portRange)
+	})
+	t.Run("Node Port range specified with a higher start number should fail validation", func(t *testing.T) {
+		portRange, err := validateNodePortRange("2000:1000")
+		assert.Error(t, err)
+		assert.Empty(t, portRange)
+	})
+	t.Run("Node Port range specified with same start and end port should fail validation", func(t *testing.T) {
+		portRange, err := validateNodePortRange("2000:2000")
+		assert.Error(t, err)
+		assert.Empty(t, portRange)
+	})
+	t.Run("Node Port range specified with a port number higher than 16-bits unsigned should fail validation", func(t *testing.T) {
+		portRange, err := validateNodePortRange("65535:65537")
+		assert.Error(t, err)
+		assert.Empty(t, portRange)
+	})
+}