Add IP filtering by network

[agraul]

What does this PR do?

Add IP filtering by network

IPs are filtered out if they don't belong to any of the given networks.
If no network is passed, all items are returned.

Example:

{% set networks = ['192.168.0.0/24', 'fe80::/64'] %}
{{ grains['ip_interfaces'] | filter_by_networks(networks) }}
{{ grains['ipv6'] | filter_by_networks(networks) }}
{{ grains['ipv4'] | filter_by_networks(networks) }}

What issues does this PR fix or reference?

Tests written?

Not yet, I will do it next.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

[aplanas]

I would not make filters optional, as this is counter-intuitive. For example if filters=[], the empty array, there is not possible match, and we would expect to return also the empty array. None and [] are semantically similar, but in this case would to just the opposite: will return the full set of values.

[darix]

This makes code with filters loaded from a pillar easier. "if no filter in pillar gimme everything" otherwise you would need to stick a default filter into the pillar.

[agraul]

We could check if filters is None to treat the [] case differently, but I don't know if that is the best approach. Passing an explicit default¹ as the filter conveys intention better than leaving the filter out in my opinion.

¹ such as ['0.0.0.0/0', '::/0']

[aplanas]

I see, but do not make the filter parameter optional. By default the pillar data is not optional (KeyError exception), and to make it optional you need to provide an explicit fallback value: pillar.get('networks', None)

I would expect that an empty list of networks will produce no match, but I guess that is OK to go to the opposite semantic if is documented.

[agraul]

do not make the filter parameter optional. By default the pillar data is not optional (KeyError exception)

okay, in that case I don't see a reason not to require a filter (now networks) parameter.

About the sematics of networks=None vs networks=[]: I definitely think that [] should return [], but I am not sure about None. I am leaning towards treating it the same as [], but I can see the argument that None is the absence of filtering.

[aplanas]

Yes. The semantic confuses me. For me [] and None are the same, so I would expect the same return value: [].

If we make it different, we need to document it, and we cannot change it later. @darix argument is not very convincing for me, as you can be explicit in the SLS and an {% if %} to apply the filter. But I can see how this is more verbose. Is OK to maintain the semantic where both a different, and see if another review have a stronger opinion.

[DmitryKuzmenko]

My opinion on this is the following: I see no need for the filters argument to be optional since if someone uses the filter they definitely wants to filter by some condition. The same is about empty filter values like None or []. So since it's documented I'm okay with this implementation.
@saltstack/team-core comments on this?

[dwoz]

I've re-started the failing windows build. When it passes this will get merged.

[DmitryKuzmenko]

Just re-started it once more because it timed out again.