octodns / octodns-ns1

Ns1Provider provider for octoDNS
MIT License
4 stars 13 forks source link

Revert order of self._COUNTRY_FILTER and self._REGION_FILTER #37

Closed istr closed 1 year ago

istr commented 1 year ago

Make synthetic ocean region work by moving the more specific rules to the top.

Fixes #36

viranch commented 1 year ago

I think this will not work when a continent needs to map to one pool and one or more countries from that continent need to map to another pool.

ross commented 1 year ago

The Oceana stuff has always been an annoying/problematic hack.

Been a long time since I was in this code and still trying to come back up to speed with it. Had to re-remember that it's filtering things out of consideration and not picking the thing it'll go with.

 'filters': [{'config': {}, 'filter': 'up'},
             {'config': {'remove_no_georegion': True},
              'filter': 'geofence_regional'},
             {'config': {'remove_no_location': True},
              'filter': 'geofence_country'},
             {'config': {}, 'filter': 'select_first_region'},
             {'config': {'eliminate': '1'}, 'filter': 'priority'},
             {'config': {}, 'filter': 'weighted_shuffle'},
             {'config': {'N': '1'}, 'filter': 'select_first_n'}],

I think this will not work when a continent needs to map to one pool and one or more countries from that continent need to map to another pool.

With the filtering nature of things I can't convince myself that it makes a difference which order the filters are in. My current (still coming back up to speed) thinking is that filtering things out of consideration removes them regardless of order.

Country is more specific than region and from that POV would need to happen first in the case of matching rules, but with filters (I think) it's pulling stuff from consideration and thus order doesn't really matter (unless remove_no_georegion is having an impact I don't grok.)

Was working on testing this with some known/existing dynamic records, but ran into https://github.com/octodns/octodns-ns1/issues/35.

istr commented 1 year ago

With the filtering nature of things I can't convince myself that it makes a difference which order the filters are in.

The order of geofence_regional and geofence_country in combination with select_first_region makes the difference.

The logic is this

Scenario 1: geofence_regional first

Scenario 2: geofence_country first

istr commented 1 year ago

@viranch

I think this will not work when a continent needs to map to one pool and one or more countries from that continent need to map to another pool.

I would say that the way it is now matches your statement (because then continent "wins" in any case). This PR will fix exactly that (because the country will "win" in any case).

viranch commented 1 year ago

Ah yes looks like putting country filter above the region filter is the right way. I'm gonna try to play around with this too.

istr commented 1 year ago

I would love to have some automated test that can check/prove the correct behavior. But probably this could only be done with a check against a real ns1 account in combination with a Geo DNS lookup service used to verify the correct answers. I did exactly that, but without proper test automation around it.

ross commented 1 year ago

I would love to have some automated test that can check/prove the correct behavior. But probably this could only be done with a check against a real ns1 account in combination with a Geo DNS lookup service used to verify the correct answers. I did exactly that, but without proper test automation around it.

It could mostly be done with things like Ripe Atlas, though it'd be more of a scoring than exact true/false sort of test since all of this stuff is fuzzy in the real world and things can change over time.

Non-trivial thing to build without someone paying for real $$ for it :grin:, though it'd be kind of fun

ross commented 1 year ago

Got pulled away as soon as I pulled up the docs, https://ns1.com/ns1-filter-chain.

Here's the defs of the filters in the order they're in the chain currently.

'filters': [{'config': {}, 'filter': 'up'},

UP Remove answers that are “down”

             {'config': {'remove_no_georegion': True},
              'filter': 'geofence_regional'},

GEOFENCE_REGIONAL Restrict to answers in same geographical region as requester

The details/options are laid out in https://help.ns1.com/hc/en-us/articles/360057943433-Fencing-filters, the critical bit in that is remove_no_georegion:

If you select Remove answers that don’t match location metadata if any match, answers are removed only if at least one answer contains location metadata that matches the requester’s location. If no answers meet this requirement, answers with no location are returned.

             {'config': {'remove_no_location': True},
              'filter': 'geofence_country'},

GEOFENCE_COUNTRY Restrict to answers in same country, country sub-division, US state, or Canadian province as requester

Same here for the remove_no_location flag.

             {'config': {}, 'filter': 'select_first_region'},

SELECT_FIRST_REGION Return all answers in the same region as the first answer

             {'config': {'eliminate': '1'}, 'filter': 'priority'},

PRIORITY Failover according to prioritized answer tiers

             {'config': {}, 'filter': 'weighted_shuffle'},

WEIGHTED_SHUFFLE Shuffle answers randomly based on their weight

             {'config': {'N': '1'}, 'filter': 'select_first_n'}],

SELECT_FIRST_N Return the first N answers

ross commented 1 year ago

OK, so that was the defs/docs, not for the thinking and discussion.

The order of geofence_regional and geofence_country in combination with select_first_region makes the difference.

Atm I'm not seeing definitive documentation on how the "first answer"'s region would be altered by the order of the fence rules. It seems possible that having the country rule before the region would do that, but I'm not seeing anything that says so.

ross commented 1 year ago

Sorry this discussion is kind of all over the place, but looking at the code that turns the rules into regions I believe any countries that are explicitly targeted are pulled out of the "region".

We first make a list of explicit countries:

https://github.com/octodns/octodns-ns1/blob/d05ee0df41d469497a808901e42f0c2d72d8c392/octodns_ns1/__init__.py#L1330-L1335

And then when running through the rules when we see a continent that uses a country mapping we remove any countries we have a separate explicit mapping for:

https://github.com/octodns/octodns-ns1/blob/d05ee0df41d469497a808901e42f0c2d72d8c392/octodns_ns1/__init__.py#L1372-L1383

So if I'm reading that correctly we shouldn't have a problem when there's a rule with OC-AU and a rule with OC.

All this is to say that I'm still not seeing how the order changes things.

Going to switch from reading docs & code to working through some examples now to see if that helps.

istr commented 1 year ago

OK, so that was the defs/docs, not for the thinking and discussion.

The order of geofence_regional and geofence_country in combination with select_first_region makes the difference.

Atm I'm not seeing definitive documentation on how the "first answer"'s region would be altered by the order of the fence rules. It seems possible that having the country rule before the region would do that, but I'm not seeing anything that says so.

At least I can observe that the order has exactly that effect. If I understand it correctly, the select_first_region seems to do the trick, because the answers from the regional and country filters do not share the answer group.

https://help.ns1.com/hc/en-us/articles/360020249093-Understanding-answer-groups

The Select First Group filter looks at the first answer in the list and selects the answer group for the first IP address answer. It removes all other answers that do not share the same answer group—limiting the answers to only those IP addresses in the selected IP answer group.

That is to say

Scenario 1: geofence_regional first

Scenario 2: geofence_country first

ross commented 1 year ago

So I just ran 2 ripe atlas measurements with the same probe set.

Details of the analysis and results are in:

https://github.com/ross/survey/blob/main/octodns-ns1-pr-37.ipynb

Relevant bit is that this PR does result in the desired in the case of AU and NZ. For some reason IR is in there too, but that could be a geo location error in the db 🤷

Screen Shot 2023-03-26 at 5 03 01 PM
viranch commented 1 year ago

Fwiw it may be easier to use dig to simulate different geos using ECS override:

dig cdn-dev.ordering.com. @dns3.p01.nsone.net +subnet=1.40.0.0 #au

I use https://www.nirsoft.net/countryip/au.html to lookup subnets for any given country.

ross commented 1 year ago

So it seems to logically make sense, though I wish there was some doc explaining it, and the results seem to get the intended behavior. Are we all good with moving forward with this?

ross commented 1 year ago

Fwiw it may be easier to use dig to simulate different geos using ECS override:

dig cdn-dev.ordering.com. @dns3.p01.nsone.net +subnet=1.40.0.0 #au

I use https://www.nirsoft.net/countryip/au.html to lookup subnets for any given country.

Will play around with that. Does seem like a good way to create a more automated test/analysis for the specific use case of geo-location.

The Ripe stuff is also really useful if you want to also analyze performance or reliability.

viranch commented 1 year ago

So it seems to logically make sense, though I wish there was some doc explaining it, and the results seem to get the intended behavior. Are we all good with moving forward with this?

I hand-tested a bunch of scenarios and this PR LGTM.