PPL cidr function

[salyh]

Description

Implement cidr function to check wether an ip address is with a given cidr

Issues Resolved

#671

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[salyh]

@YANG-DB @vamshin @anirudha

I pushed the scaffold for the CIDR function.
The real implementation which validates the ip address and check if its within the cidr is missing because we need to discuss the introduction of a new 3rd party library:

I suggest to add a dependency to https://github.com/seancfoley/IPAddress because this library seems to provide ip validation and in range checking for ipv4 and ipv6 addresses/cidrs. The library is Apache licensed.

Other options are the following libraries or code snippets (or of course manual implementation with java standard library which is error prone in my view):

• https://www.baeldung.com/java-check-ip-address-range
• https://github.com/apache/commons-net (ipv4 only)
• https://github.com/edazdarevic/CIDRUtils/blob/master/CIDRUtils.java (MIT license)

Let me know what you think and what the general process is to discuss new dependencies

[YANG-DB]

Hi @salyh
I agree - let's use https://github.com/seancfoley/IPAddress library since it's in use in our SQL repo already

[LantaoJin]

Hi @salyh
Can you check if the Guava library can meet our needs?

[salyh]

Hi @salyh Can you check if the Guava library can meet our needs?

I checked but it does not support cidr checks. https://github.com/seancfoley/IPAddress is also already used in the sql project.

[salyh]

Hi @salyh I agree - let's use https://github.com/seancfoley/IPAddress library since it's in use in our SQL repo already

done

[YANG-DB]

@salyh plz resolve conflict
thanks

[salyh]

@salyh plz resolve conflict thanks

done

[salyh]

@YANG-DB @vamshin @anirudha

Regarding validation of the CIDR function parameters:

The IPAddress library is very lenient regarding validation of ip addresss and cidr blocks.

I suggest we at least disallow partial ip address segements and empty or null adresses:

  IPAddressStringParameters valOptions = new IPAddressStringParameters.Builder()
                .allowEmpty(false)
                .setEmptyAsLoopback(false)
                .allow_inet_aton(false)
                .allowSingleSegment(false)
                .toParams();

Then the remaining question is wether we should enforce other syntax checks like which the library is not enforcing:

• check that cidr block is a really a in cicdr notation (containing a '/')
• check that ip address and cidr are either both ipv6 or both ipv4
• define the behaviour for edge cases like 0.0.0.0 (or just rely on the api)
• define handling of special ip adresses like multicast, anycast, 255...* etc

My suggestion is to only

• disallow partial ip address segements and empty or null adresses (like already implemented) and leave everything else
• disallow mixed ipv4/ipv6 adresses and cidr blocks

and leave everything else to the library.

[YANG-DB]

@salyh
please add the needed documentation in the following locations:

• ppl commands list
• ppl functions list
• ppl examples doc

[YANG-DB]

@YANG-DB @vamshin @anirudha

Regarding validation of the CIDR function parameters:

The IPAddress library is very lenient regarding validation of ip addresss and cidr blocks.

I suggest we at least disallow partial ip address segements and empty or null adresses:

  IPAddressStringParameters valOptions = new IPAddressStringParameters.Builder()
                .allowEmpty(false)
                .setEmptyAsLoopback(false)
                .allow_inet_aton(false)
                .allowSingleSegment(false)
                .toParams();

Then the remaining question is wether we should enforce other syntax checks like which the library is not enforcing:

• check that cidr block is a really a in cicdr notation (containing a '/')
• check that ip address and cidr are either both ipv6 or both ipv4
• define the behaviour for edge cases like 0.0.0.0 (or just rely on the api)
• define handling of special ip adresses like multicast, anycast, 255...* etc

My suggestion is to only

• disallow partial ip address segements and empty or null adresses (like already implemented) and leave everything else
• disallow mixed ipv4/ipv6 adresses and cidr blocks

and leave everything else to the library.

@salyh
Thanks for this review - I support your suggestion

• disallow partial ip address segements and empty or null adresses (like already implemented) and leave everything else
• disallow mixed ipv4/ipv6 adresses and cidr blocks

In addition plz document the additional validations as future work:

check that cidr block is a really a in cicdr notation (containing a '/')
check that ip address and cidr are either both ipv6 or both ipv4
define the behaviour for edge cases like 0.0.0.0 (or just rely on the api)
define handling of special ip adresses like multicast, anycast, 255...* etc

Thanks

[YANG-DB]

please add the needed documentation in the following locations:

ppl planning list

Once we approve we'll also add it onto the other docs:

• https://github.com/opensearch-project/opensearch-spark/blob/main/docs/ppl-lang/PPL-Example-Commands.md
• https://github.com/opensearch-project/opensearch-spark/blob/main/docs/ppl-lang/README.md

[YANG-DB]

@dr-lilienthal can u plz resolve the conflicts and also signOff the commits ?( DCO failure )
thanks

[YANG-DB]

@salyh @dr-lilienthal can u plz resolve the conflicts ?
thanks

[salyh]

@salyh @dr-lilienthal can u plz resolve the conflicts ? thanks

@YANG-DB done