Merge pull request #13812 from tyabu12/fix/port-validation

Fix port validation error on specifying tcp/udp or range of ports.
kubernetes · Apr 13, 2022 · a9cf475 · a9cf475
2 parents 913ac55 + afb3956
commit a9cf475
Show file tree

Hide file tree

Showing 2 changed files with 107 additions and 24 deletions.
diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go
@@ -34,6 +34,7 @@ import (
 
 	"github.com/Delta456/box-cli-maker/v2"
 	"github.com/blang/semver/v4"
+	"github.com/docker/go-connections/nat"
 	"github.com/docker/machine/libmachine/ssh"
 	"github.com/google/go-containerregistry/pkg/authn"
 	"github.com/google/go-containerregistry/pkg/name"
@@ -1246,26 +1247,22 @@ func validateFlags(cmd *cobra.Command, drvName string) {
 	validateInsecureRegistry()
 }
 
-// This function validates that the --ports are not below 1024 for the host and not outside range
+// validatePorts validates that the --ports are not below 1024 for the host and not outside range
 func validatePorts(ports []string) error {
-	for _, portDuplet := range ports {
-		parts := strings.Split(portDuplet, ":")
-		if len(parts) > 2 {
-			ip := parts[0]
-			if net.ParseIP(ip) == nil {
-				return errors.Errorf("Sorry, the IP address provided with --ports flag is invalid: %s", ip)
-			}
-			parts = parts[1:]
-		}
-		for i, port := range parts {
-			p, err := strconv.Atoi(port)
+	_, portBindingsMap, err := nat.ParsePortSpecs(ports)
+	if err != nil {
+		return errors.Errorf("Sorry, one of the ports provided with --ports flag is not valid %s (%v)", ports, err)
+	}
+	for _, portBindings := range portBindingsMap {
+		for _, portBinding := range portBindings {
+			p, err := strconv.Atoi(portBinding.HostPort)
 			if err != nil {
 				return errors.Errorf("Sorry, one of the ports provided with --ports flag is not valid %s", ports)
 			}
 			if p > 65535 || p < 1 {
 				return errors.Errorf("Sorry, one of the ports provided with --ports flag is outside range %s", ports)
 			}
-			if detect.IsMicrosoftWSL() && p < 1024 && i == 0 {
+			if detect.IsMicrosoftWSL() && p < 1024 {
 				return errors.Errorf("Sorry, you cannot use privileged ports on the host (below 1024) %s", ports)
 			}
 		}

diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go
@@ -462,40 +462,126 @@ func TestValidateRuntime(t *testing.T) {
 }
 
 func TestValidatePorts(t *testing.T) {
+	isMicrosoftWSL := detect.IsMicrosoftWSL()
 	type portTest struct {
+		// isTarget indicates whether or not the test case is covered
+		// because validatePorts behaves differently depending on whether process is running in WSL in windows or not.
+		isTarget bool
 		ports    []string
 		errorMsg string
 	}
 	var tests = []portTest{
 		{
-			ports:    []string{"test:80"},
-			errorMsg: "Sorry, one of the ports provided with --ports flag is not valid [test:80]",
+			isTarget: true,
+			ports:    []string{"8080:80"},
+			errorMsg: "",
+		},
+		{
+			isTarget: true,
+			ports:    []string{"8080:80/tcp", "8080:80/udp"},
+			errorMsg: "",
+		},
+		{
+			isTarget: true,
+			ports:    []string{"test:8080"},
+			errorMsg: "Sorry, one of the ports provided with --ports flag is not valid [test:8080] (Invalid hostPort: test)",
 		},
 		{
+			isTarget: true,
 			ports:    []string{"0:80"},
 			errorMsg: "Sorry, one of the ports provided with --ports flag is outside range [0:80]",
 		},
 		{
-			ports:    []string{"8080:80", "6443:443"},
+			isTarget: true,
+			ports:    []string{"0:80/tcp"},
+			errorMsg: "Sorry, one of the ports provided with --ports flag is outside range [0:80/tcp]",
+		},
+		{
+			isTarget: true,
+			ports:    []string{"65536:80/udp"},
+			errorMsg: "Sorry, one of the ports provided with --ports flag is not valid [65536:80/udp] (Invalid hostPort: 65536)",
+		},
+		{
+			isTarget: true,
+			ports:    []string{"0-1:80-81/tcp"},
+			errorMsg: "Sorry, one of the ports provided with --ports flag is outside range [0-1:80-81/tcp]",
+		},
+		{
+			isTarget: true,
+			ports:    []string{"0-1:80-81/udp"},
+			errorMsg: "Sorry, one of the ports provided with --ports flag is outside range [0-1:80-81/udp]",
+		},
+		{
+			isTarget: !isMicrosoftWSL,
+			ports:    []string{"80:80", "1023-1025:8023-8025", "1023-1025:8023-8025/tcp", "1023-1025:8023-8025/udp"},
 			errorMsg: "",
 		},
 		{
-			ports:    []string{"127.0.0.1:80:80"},
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"80:80"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [80:80]",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"1023-1025:8023-8025"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [1023-1025:8023-8025]",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"1023-1025:8023-8025/tcp"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [1023-1025:8023-8025/tcp]",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"1023-1025:8023-8025/udp"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [1023-1025:8023-8025/udp]",
+		},
+		{
+			isTarget: true,
+			ports:    []string{"127.0.0.1:8080:80", "127.0.0.1:8081:80/tcp", "127.0.0.1:8081:80/udp", "127.0.0.1:8082-8083:8082-8083/tcp"},
 			errorMsg: "",
 		},
 		{
+			isTarget: true,
 			ports:    []string{"1000.0.0.1:80:80"},
-			errorMsg: "Sorry, the IP address provided with --ports flag is invalid: 1000.0.0.1",
+			errorMsg: "Sorry, one of the ports provided with --ports flag is not valid [1000.0.0.1:80:80] (Invalid ip address: 1000.0.0.1)",
+		},
+		{
+			isTarget: !isMicrosoftWSL,
+			ports:    []string{"127.0.0.1:80:80", "127.0.0.1:81:81/tcp", "127.0.0.1:81:81/udp", "127.0.0.1:82-83:82-83/tcp", "127.0.0.1:82-83:82-83/udp"},
+			errorMsg: "",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"127.0.0.1:80:80"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [127.0.0.1:80:80]",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"127.0.0.1:81:81/tcp"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [127.0.0.1:81:81/tcp]",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"127.0.0.1:81:81/udp"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [127.0.0.1:81:81/udp]",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"127.0.0.1:80-83:80-83/tcp"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [127.0.0.1:80-83:80-83/tcp]",
+		},
+		{
+			isTarget: isMicrosoftWSL,
+			ports:    []string{"127.0.0.1:80-83:80-83/udp"},
+			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [127.0.0.1:80-83:80-83/udp]",
 		},
-	}
-	if detect.IsMicrosoftWSL() {
-		tests = append(tests, portTest{
-			ports:    []string{"80:80"},
-			errorMsg: "Sorry, you cannot use privileged ports on the host (below 1024) [80:80]",
-		})
 	}
 	for _, test := range tests {
 		t.Run(strings.Join(test.ports, ","), func(t *testing.T) {
+			if !test.isTarget {
+				return
+			}
 			gotError := ""
 			got := validatePorts(test.ports)
 			if got != nil {