Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(*) make port validation consistent #2448

Merged
merged 2 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 12 additions & 19 deletions pkg/core/resources/apis/mesh/dataplane_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
}
Expand Down Expand Up @@ -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`)
}
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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")
}
Expand All @@ -268,6 +260,7 @@ func validateOutbound(outbound *mesh_proto.Dataplane_Networking_Outbound) valida
}
result.Add(validateTags(outbound.Tags))
}

return result
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/core/resources/apis/mesh/dataplane_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand All @@ -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: `
Expand Down Expand Up @@ -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: `
Expand Down Expand Up @@ -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: `
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/core/resources/apis/mesh/externalservice_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand All @@ -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: `
Expand All @@ -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: `
Expand Down
5 changes: 1 addition & 4 deletions pkg/core/resources/apis/mesh/mesh_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/core/resources/apis/mesh/validators.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
14 changes: 6 additions & 8 deletions pkg/core/resources/apis/mesh/zone_ingress_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/core/resources/apis/mesh/zone_ingress_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: `
Expand Down Expand Up @@ -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"]
Expand Down