_monitor_is_match needs to use expected for nested dict `config` too

[ross]

Cribbed from https://github.com/octodns/octodns-ns1/issues/35#issuecomment-1487065395

The false/persistent update is coming from _extra_changes

2023-03-28T07:55:32  [4644720128] INFO  Ns1Provider[ns1] _extra_changes: monitor mis-match for a.exxampled.com - A - 1.1.1.1

Which is triggered when _monitor_is_match returns False:

https://github.com/octodns/octodns-ns1/blob/bc542be4ac73d3914adcfd5d1ce2f56ff484e119/octodns_ns1/__init__.py#L1196-L1203

Printing out the two

{'expected': {'active': True,
              'config': {'connect_timeout': 2000,
                         'host': '1.1.1.1',
                         'port': 443,
                         'response_timeout': 10000,
                         'send': 'GET /_dns HTTP/1.0\\r\\nHost: '
                                 'a.exxampled.com\\r\\nUser-agent: '
                                 'NS1\\r\\n\\r\\n',
                         'ssl': True},
              'frequency': 60,
              'job_type': 'tcp',
              'name': 'a.exxampled.com - A - 1.1.1.1',
              'notes': 'host:a.exxampled.com type:A',
              'policy': 'quorum',
              'rapid_recheck': False,
              'region_scope': 'fixed',
              'regions': ['lga'],
              'rules': [{'comparison': 'contains',
                         'key': 'output',
                         'value': '200 OK'}]},
 'have': {'active': True,
          'config': {'connect_timeout': 2000,
                     'host': '1.1.1.1',
                     'ipv6': False,
                     'port': 443,
                     'response_timeout': 10000,
                     'send': 'GET /_dns HTTP/1.0\\r\\nHost: '
                             'a.exxampled.com\\r\\nUser-agent: NS1\\r\\n\\r\\n',
                     'ssl': True,
                     'tls_add_verify': False},
          'frequency': 60,
          'id': '6422ff1c65f383007cace839',
          'job_type': 'tcp',
          'mute': False,
          'name': 'a.exxampled.com - A - 1.1.1.1',
          'notes': 'host:a.exxampled.com type:A',
          'notify_delay': 0,
          'notify_failback': True,
          'notify_list': '62f29339f6491b0095148771',
          'notify_regional': False,
          'notify_repeat': 0,
          'policy': 'quorum',
          'rapid_recheck': False,
          'region_scope': 'fixed',
          'regions': ['lga'],
          'rules': [{'comparison': 'contains',
                     'key': 'output',
                     'value': '200 OK'}],
          'status': {'global': {'fail_set': ['lga'],
                                'since': 1680015196,
                                'status': 'down'},
                     'lga': {'fail_set': ['Failure for Rule: output contains '
                                          '200 OK'],
                             'since': 1680015196,
                             'status': 'down'}}}}

{'have.get': {'connect_timeout': 2000,
              'host': '1.1.1.1',
              'ipv6': False,
              'port': 443,
              'response_timeout': 10000,
              'send': 'GET /_dns HTTP/1.0\\r\\nHost: '
                      'a.exxampled.com\\r\\nUser-agent: NS1\\r\\n\\r\\n',
              'ssl': True,
              'tls_add_verify': False},
 'k': 'config',
 'mismatch': True,
 'v': {'connect_timeout': 2000,
       'host': '1.1.1.1',
       'port': 443,
       'response_timeout': 10000,
       'send': 'GET /_dns HTTP/1.0\\r\\nHost: a.exxampled.com\\r\\nUser-agent: '
               'NS1\\r\\n\\r\\n',
       'ssl': True}}

Looks like they've added a new keys to the config section: tls_add_verify and ipv6 and the current method of comparison sees that as a diff.

Working on a PR/solution now.

/cc Found when trying to reproduce https://github.com/octodns/octodns-ns1/issues/35 @istr

[istr]

@ross I locally applied the change, but this only fixes the situation with enforce_order: false in combination with reordering the entries in the configuration. So the issue brought up in #35 is not (yet) fixed with this PR. I still cannot have the config with enforce_order: true or without explicitly setting the flag (the implied default is True in def safe_load(stream, enforce_order=True):)

In short: the PR is good, it fixes an issue, but not #35.

[ross]

In short: the PR is good, it fixes an issue, but not #35.

Hrm. OK. I guess I ran into a persistent change problem trying to reproduce the issue and then lost the thread on the cause of the persistent change you ran into.

As a consequence, with the enforce_order flag set, every run creates a change, even though nothing changed.

I'll give reproducing the original issue another go.

[istr]

@viranch please check this, I guess that @ross is right that he wants to merge this one prior to #37.

[ross]

Does this need tests? LGTM either way!

I started out expecting to add some, but there were already things hitting the new case. I guess I could probably add something that verifies that "extra" info in the config section doesn't cause problems now (and did before.)