chore(*) make port validation consistent (#2448) (#2464)

Introduce a helper to make TCP/UDP port validation checks
consistent across the API.

Signed-off-by: James Peach <james.peach@konghq.com>
(cherry picked from commit bb07923)

Co-authored-by: James Peach <james.peach@konghq.com>

pkg/core/resources/apis/mesh/dataplane_validator.go

@@ -97,14 +97,10 @@ func validateProbes(probes *mesh_proto.Dataplane_Probes) validators.ValidationEr
 	}
 	var err validators.ValidationError
 	path := validators.RootedAt("probes")
-	if probes.Port < 1 || probes.Port > 65535 {
-		err.AddViolationAt(path.Field("port"), `port has to be in range of [1, 65535]`)
-	}
+	err.Add(ValidatePort(path.Field("port"), probes.GetPort()))
 	for i, endpoint := range probes.Endpoints {
 		indexPath := path.Field("endpoints").Index(i)
-		if endpoint.InboundPort < 1 || endpoint.InboundPort > 65535 {
-			err.AddViolationAt(indexPath.Field("inboundPort"), `port has to be in range of [1, 65535]`)
-		}
+		err.Add(ValidatePort(indexPath.Field("inboundPort"), endpoint.GetInboundPort()))
 		if _, URIErr := url.ParseRequestURI(endpoint.InboundPath); URIErr != nil {
 			err.AddViolationAt(indexPath.Field("inboundPath"), `should be a valid URL Path`)
 		}
@@ -141,9 +137,7 @@ func validateIngressNetworking(networking *mesh_proto.Dataplane_Networking) vali
 	}
 	for i, inbound := range networking.GetInbound() {
 		p := path.Field("inbound").Index(i)
-		if inbound.Port > 65535 {
-			err.AddViolationAt(p.Field("port"), `port has to be in range of [1, 65535]`)
-		}
+		err.Add(ValidatePort(p.Field("port"), inbound.GetPort()))
 		if inbound.ServicePort != 0 {
 			err.AddViolationAt(p.Field("servicePort"), `cannot be defined in the ingress mode`)
 		}
@@ -177,8 +171,8 @@ func validateIngress(path validators.PathBuilder, ingress *mesh_proto.Dataplane_
 	if ingress.GetPublicAddress() != "" {
 		err.Add(validateAddress(path.Field("publicAddress"), ingress.GetPublicAddress()))
 	}
-	if ingress.GetPublicPort() > 65535 {
-		err.AddViolationAt(path.Field("publicPort"), `port has to be in range of [1, 65535]`)
+	if ingress.GetPublicPort() != 0 {
+		err.Add(ValidatePort(path.Field("publicPort"), ingress.GetPublicPort()))
 	}
 	for i, ingressInterface := range ingress.GetAvailableServices() {
 		p := path.Field("availableService").Index(i)
@@ -192,11 +186,9 @@ func validateIngress(path validators.PathBuilder, ingress *mesh_proto.Dataplane_
 
 func validateInbound(inbound *mesh_proto.Dataplane_Networking_Inbound, dpAddress string) validators.ValidationError {
 	var result validators.ValidationError
-	if inbound.Port < 1 || inbound.Port > 65535 {
-		result.AddViolationAt(validators.RootedAt("port"), `port has to be in range of [1, 65535]`)
-	}
-	if inbound.ServicePort > 65535 {
-		result.AddViolationAt(validators.RootedAt("servicePort"), `servicePort has to be in range of [0, 65535]`)
+	result.Add(ValidatePort(validators.RootedAt("port"), inbound.GetPort()))
+	if inbound.GetServicePort() != 0 {
+		result.Add(ValidatePort(validators.RootedAt("servicePort"), inbound.GetServicePort()))
 	}
 	if inbound.ServiceAddress != "" {
 		if net.ParseIP(inbound.ServiceAddress) == nil {
@@ -250,9 +242,9 @@ func validateServiceProbe(serviceProbe *mesh_proto.Dataplane_Networking_Inbound_
 
 func validateOutbound(outbound *mesh_proto.Dataplane_Networking_Outbound) validators.ValidationError {
 	var result validators.ValidationError
-	if outbound.Port < 1 || outbound.Port > 65535 {
-		result.AddViolation("port", "port has to be in range of [1, 65535]")
-	}
+
+	result.Add(ValidatePort(validators.RootedAt("port"), outbound.GetPort()))
+
 	if outbound.Address != "" && net.ParseIP(outbound.Address) == nil {
 		result.AddViolation("address", "address has to be valid IP address")
 	}
@@ -268,6 +260,7 @@ func validateOutbound(outbound *mesh_proto.Dataplane_Networking_Outbound) valida
 		}
 		result.Add(validateTags(outbound.Tags))
 	}
+
 	return result
 }
 

pkg/core/resources/apis/mesh/dataplane_validator_test.go

@@ -464,9 +464,9 @@ var _ = Describe("Dataplane", func() {
 			expected: `
                 violations:
                 - field: networking.inbound[0].port
-                  message: port has to be in range of [1, 65535]
+                  message: port must be in the range [1, 65535]
                 - field: networking.inbound[1].port
-                  message: port has to be in range of [1, 65535]`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("networking.inbound: servicePort out of the range", testCase{
 			dataplane: `
@@ -486,7 +486,7 @@ var _ = Describe("Dataplane", func() {
 			expected: `
                 violations:
                 - field: networking.inbound[0].servicePort
-                  message: servicePort has to be in range of [0, 65535]`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("networking.inbound: invalid address", testCase{
 			dataplane: `
@@ -704,9 +704,9 @@ var _ = Describe("Dataplane", func() {
 			expected: `
                 violations:
                 - field: networking.outbound[0].port
-                  message: port has to be in range of [1, 65535]
+                  message: port must be in the range [1, 65535]
                 - field: networking.outbound[1].port
-                  message: port has to be in range of [1, 65535]`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("networking.outbound: invalid address", testCase{
 			dataplane: `
@@ -924,7 +924,7 @@ var _ = Describe("Dataplane", func() {
                 - field: networking.ingress.publicAddress.address
                   message: address has to be valid IP address or domain name
                 - field: networking.ingress.publicPort
-                  message: port has to be in range of [1, 65535]`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("inbound service address", testCase{
 			dataplane: `
@@ -1071,9 +1071,9 @@ var _ = Describe("Dataplane", func() {
 			expected: `
                 violations:
                 - field: probes.port
-                  message: port has to be in range of [1, 65535]
+                  message: port must be in the range [1, 65535]
                 - field: probes.endpoints[1].inboundPort
-                  message: port has to be in range of [1, 65535]
+                  message: port must be in the range [1, 65535]
                 - field: probes.endpoints[1].inboundPath
                   message: should be a valid URL Path
                 - field: probes.endpoints[1].path

pkg/core/resources/apis/mesh/externalservice_validator.go

@@ -54,12 +54,12 @@ func validateExtarnalServiceAddress(path validators.PathBuilder, address string)
 		err.AddViolationAt(path.Field("address"), "address has to be a valid IP address or a domain name")
 	}
 
-	iport, e := strconv.Atoi(port)
+	iport, e := strconv.ParseUint(port, 10, 32)
 	if e != nil {
 		err.AddViolationAt(path.Field("address"), "unable to parse port in address")
 	}
-	if iport < 1 || iport > 65535 {
-		err.AddViolationAt(path.Field("address"), "port has to be in range of [1, 65535]")
-	}
+
+	err.Add(ValidatePort(path.Field("address"), uint32(iport)))
+
 	return err
 }

pkg/core/resources/apis/mesh/externalservice_validator_test.go

@@ -157,7 +157,7 @@ var _ = Describe("ExternalService", func() {
                 - field: networking.address
                   message: unable to parse port in address
                 - field: networking.address
-                  message: port has to be in range of [1, 65535]`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("networking.address: invalid format IPv6", testCase{
 			dataplane: `
@@ -178,7 +178,7 @@ var _ = Describe("ExternalService", func() {
                 - field: networking.address
                   message: unable to parse port in address
                 - field: networking.address
-                  message: port has to be in range of [1, 65535]`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("networking: port out of range", testCase{
 			dataplane: `
@@ -193,7 +193,7 @@ var _ = Describe("ExternalService", func() {
 			expected: `
                 violations:
                 - field: networking.address
-                  message: port has to be in range of [1, 65535]`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("tags: empty service tag", testCase{
 			dataplane: `

pkg/core/resources/apis/mesh/mesh_validator.go

@@ -165,10 +165,7 @@ func validateDatadog(cfgStr *structpb.Struct) validators.ValidationError {
 		verr.AddViolation("address", "cannot be empty")
 	}
 
-	if cfg.Port > 0xFFFF || cfg.Port < 1 {
-		verr.AddViolation("port", "must be in the range 1 to 65535")
-	}
-
+	verr.Add(ValidatePort(validators.RootedAt("port"), cfg.GetPort()))
 	return verr
 }
 

pkg/core/resources/apis/mesh/validators.go

@@ -140,6 +140,17 @@ func ValidateThreshold(path validators.PathBuilder, threshold uint32) (err valid
 	return
 }
 
+// ValidatePort validates that port is a valid TCP or UDP port number.
+func ValidatePort(path validators.PathBuilder, port uint32) validators.ValidationError {
+	err := validators.ValidationError{}
+
+	if port == 0 || port > 65535 {
+		err.AddViolationAt(path, "port must be in the range [1, 65535]")
+	}
+
+	return err
+}
+
 func AllowedValuesHint(values ...string) string {
 	options := strings.Join(values, ", ")
 	if len(values) == 0 {

pkg/core/resources/apis/mesh/zone_ingress_validator.go

@@ -26,15 +26,13 @@ func (r *ZoneIngressResource) validateNetworking(path validators.PathBuilder, ne
 	if networking.GetAdvertisedAddress() != "" {
 		err.Add(validateAddress(path.Field("advertisedAddress"), networking.GetAdvertisedAddress()))
 	}
-	if networking.GetPort() == 0 {
-		err.AddViolationAt(path.Field("port"), `port has to be defined`)
-	}
-	if networking.GetPort() > 65535 {
-		err.AddViolationAt(path.Field("port"), `port has to be in range of [1, 65535]`)
-	}
-	if networking.GetAdvertisedPort() > 65535 {
-		err.AddViolationAt(path.Field("advertisedPort"), `port has to be in range of [1, 65535]`)
+
+	err.Add(ValidatePort(path.Field("port"), networking.GetPort()))
+
+	if networking.GetAdvertisedPort() != 0 {
+		err.Add(ValidatePort(path.Field("advertisedPort"), networking.GetAdvertisedPort()))
 	}
+
 	return err
 }
 

pkg/core/resources/apis/mesh/zone_ingress_validator_test.go

@@ -120,7 +120,7 @@ var _ = Describe("Dataplane", func() {
 			expected: `
                 violations:
                 - field: networking.port
-                  message: port has to be defined`,
+                  message: port must be in the range [1, 65535]`,
 		}),
 		Entry("invalid advertised address, port and available service", testCase{
 			dataplane: `
@@ -152,7 +152,7 @@ var _ = Describe("Dataplane", func() {
                 - field: networking.advertisedAddress.address
                   message: address has to be valid IP address or domain name
                 - field: networking.advertisedPort
-                  message: port has to be in range of [1, 65535]
+                  message: port must be in the range [1, 65535]
                 - field: availableService[2].tags["kuma.io/service"]
                   message: cannot be empty
                 - field: availableService[3].tags.tags["kuma.io/service"]