more general, but less strict peer list schema

also handle test errors caused by removing space filter

inventory_provider/juniper.py

@@ -109,28 +109,29 @@ UNIT_SCHEMA = """<?xml version="1.1" encoding="UTF-8" ?>
 </xs:schema>
 """  # noqa: E501
 
-PEERING_LIST_SCHEMA = {
-    "$schema": "http://json-schema.org/draft-07/schema#",
-    "definitions": {
-        "peering": {
-            "type": "object",
-            "properties": {
-                "group": {"type": "string"},
-                "description": {"type": "string"},
-                "address": {"type": "string"},
-                "remote-asn": {"type": "integer"},
-                "local-asn": {"type": "integer"},
-                "instance": {"type": "string"},
-                "logical-system": {"type": "string"},
-            },
-            # lots of internal peerings - so maybe no explicit asn's
-            "required": ["group", "address"],
-            "additionalProperties": False
-        }
-    },
-    "type": "array",
-    "items": {"$ref": "#/definitions/peering"}
-}
+# PEERING_LIST_SCHEMA = {
+#     "$schema": "http://json-schema.org/draft-07/schema#",
+#     "definitions": {
+#         "peering": {
+#             "type": "object",
+#             "properties": {
+#                 "group": {"type": "string"},
+#                 "description": {"type": "string"},
+#                 "address": {"type": "string"},
+#                 "remote-asn": {"type": "integer"},
+#                 "local-asn": {"type": "integer"},
+#                 "instance": {"type": "string"},
+#                 "logical-system": {"type": "string"},
+#                 "hostname": {"type": "string"}
+#             },
+#             # lots of internal peerings - so maybe no explicit asn's
+#             "required": ["group", "address", "hostname"],
+#             "additionalProperties": False
+#         }
+#     },
+#     "type": "array",
+#     "items": {"$ref": "#/definitions/peering"}
+# }
 
 
 class NetconfHandlingError(Exception):

inventory_provider/routes/testing.py

@@ -169,15 +169,16 @@ def bgp_configs(hostname):
             status=404,
             mimetype="text/html")
 
-    routes = list(juniper.all_bgp_peers(
+    peers = list(juniper.all_bgp_peers(
         etree.XML(netconf_string.decode('utf-8'))))
-    if not routes:
+
+    if not peers:
         return Response(
             response="no interfaces found for '%s'" % hostname,
             status=404,
             mimetype="text/html")
 
-    return jsonify(routes)
+    return jsonify(peers)
 
 
 @routes.route("snmp/<hostname>", methods=['GET', 'POST'])

test/per_router/test_celery_worker.py

@@ -56,6 +56,12 @@ def test_snmp_refresh_peerings(mocked_worker_module, router):
 
 
 def test_reload_router_config(mocked_worker_module, router, mocker):
+
+    if router.startswith('qfx'):
+        # test env hack (router doesn't have snmp acl for us)
+        mocker.patch('inventory_provider.juniper.snmp_community_string') \
+            .return_value = 'blah'
+
     saved_data = {}
     for key in ('netconf:' + router, 'snmp-interfaces:' + router):
         saved_data[key] = backend_db().pop(key)

test/per_router/test_data_routes.py

+import copy
 import json
 import pytest
 import jsonschema
-from inventory_provider.juniper import PEERING_LIST_SCHEMA
+from inventory_provider.routes import msr
 
 DEFAULT_REQUEST_HEADERS = {
     "Content-type": "application/json",
@@ -76,10 +77,12 @@ def test_snmp_ids(router, client):
 
 def test_router_bgp_routes(router, client):
 
-    ROUTERS_WITHOUT_PEERING_DATA = [
-        'mx2.bru.be.geant.net'
-    ]
-    if router in ROUTERS_WITHOUT_PEERING_DATA:
+    def _no_peering_data_expected(h):
+        if h == 'mx2.bru.be.geant.net':
+            return None
+        return h.startswith('qfx')
+
+    if _no_peering_data_expected(router):
         pytest.skip('%s is not expected to have bgp peers' % router)
         return
 
@@ -87,7 +90,11 @@ def test_router_bgp_routes(router, client):
         "/testing/bgp/" + router,
         headers=DEFAULT_REQUEST_HEADERS)
 
+    internal_peering_list_schema = copy.deepcopy(msr.PEERING_LIST_SCHEMA)
+    internal_peering_list_schema[
+        'definitions']['peering-instance']['required'].remove('hostname')
+
     assert rv.status_code == 200
     response = json.loads(rv.data.decode("utf-8"))
-    jsonschema.validate(response, PEERING_LIST_SCHEMA)
+    jsonschema.validate(response, internal_peering_list_schema)
     assert response  # at least shouldn't be empty

test/per_router/test_juniper_data.py

+import copy
 import ipaddress
 import jsonschema
 import pytest
 from inventory_provider import juniper
+from inventory_provider.routes import msr
 
 
 def test_interface_list(netconf_doc):
@@ -39,6 +41,10 @@ def test_interface_list(netconf_doc):
 
 def test_bgp_peering_data(netconf_doc):
 
+    internal_peering_list_schema = copy.deepcopy(msr.PEERING_LIST_SCHEMA)
+    internal_peering_list_schema[
+        'definitions']['peering-instance']['required'].remove('hostname')
+
     active_peers = set()
     inactive_peers = set()
     for neighbor in netconf_doc.xpath('//group/neighbor'):
@@ -58,7 +64,7 @@ def test_bgp_peering_data(netconf_doc):
         pytest.skip('no peerings configured for this router')
 
     peerings = list(juniper.all_bgp_peers(netconf_doc))
-    jsonschema.validate(peerings, juniper.PEERING_LIST_SCHEMA)
+    jsonschema.validate(peerings, internal_peering_list_schema)
     assert peerings  # there's always at least one
 
     # confirm the addresses are in canonical (exploded) form
@@ -81,7 +87,9 @@ def test_bgp_peering_data(netconf_doc):
         f'{inactive_peers & returned_addresses}')
 
 
-def test_snmp_community_string(mocked_netifaces, netconf_doc):
+def test_snmp_community_string(mocked_netifaces, router, netconf_doc):
+    if router.startswith('qfx'):
+        pytest.skip(f'no snmp community string expected for {router}')
     assert juniper.snmp_community_string(netconf_doc) == '0pBiFbD'
 
 

test/test_worker_utils.py

 """
 tests of a few worker utilities
 """
+import copy
 import ipaddress
 import json
 import re
@@ -11,7 +12,6 @@ from inventory_provider.tasks import worker
 from inventory_provider.tasks import common
 from inventory_provider.routes import msr
 
-
 def backend_db():
     return common._get_redis({
         'redis': {
@@ -92,72 +92,99 @@ def test_build_juniper_peering_db(mocked_worker_module):
     # same as inventory_provider.juniper.PEERING_LIST_SCHEMA,
     # but with "hostname" in every returned record
 
-    LOGICAL_SYSTEM_PEERING_SCHEMA = {
-        "type": "object",
-        "properties": {
-            "logical-system": {"type": "string"},
-            "group": {"type": "string"},
-            "description": {"type": "string"},
-            "address": {"type": "string"},
-            "remote-asn": {"type": "integer"},
-            "local-asn": {"type": "integer"},
-            "hostname": {"type": "string"}
-        },
-        # local/remote-asn and/or description are not always present,
-        # just based on empirical tests - not a problem
-        "required": ["logical-system", "group", "address"],
-        "additionalProperties": False
-    }
-
-    TOP_LEVEL_PEERING_SCHEMA = {
-        "type": "object",
-        "properties": {
-            "group": {"type": "string"},
-            "description": {"type": "string"},
-            "address": {"type": "string"},
-            "remote-asn": {"type": "integer"},
-            "local-asn": {"type": "integer"},
-            "hostname": {"type": "string"}
-        },
-        # lots of internal peerings - so maybe no explicit asn's
-        "required": ["group", "address"],
-        "additionalProperties": False
-    }
-
-    INSTANCE_PEERING = {
-        "type": "object",
-        "properties": {
-            "instance": {"type": "string"},
-            "group": {"type": "string"},
-            "description": {"type": "string"},
-            "address": {"type": "string"},
-            "remote-asn": {"type": "integer"},
-            "local-asn": {"type": "integer"},
-            "hostname": {"type": "string"}
-        },
-        # description and-or local-asn is not always present,
-        # just based on empirical tests - not a problem
-        "required": ["instance", "group", "address", "remote-asn"],
-        "additionalProperties": False
-    }
-
-    DETAILED_PEERING_LIST_SCHEMA = {
-        "$schema": "http://json-schema.org/draft-07/schema#",
-        "definitions": {
-            "top-level-peering": TOP_LEVEL_PEERING_SCHEMA,
-            "instance-peering": INSTANCE_PEERING,
-            "logical-system-peering": LOGICAL_SYSTEM_PEERING_SCHEMA,
-            "peering": {
-                "oneOf": [
-                    {"$ref": "#/definitions/top-level-peering"},
-                    {"$ref": "#/definitions/instance-peering"},
-                    {"$ref": "#/definitions/logical-system-peering"}
-                ]
-            }
-        },
-        "type": "array",
-        "items": {"$ref": "#/definitions/peering"}
-    }
+    # LOGICAL_SYSTEM_PEERING_SCHEMA = {
+    #     "type": "object",
+    #     "properties": {
+    #         "logical-system": {"type": "string"},
+    #         "group": {"type": "string"},
+    #         "description": {"type": "string"},
+    #         "address": {"type": "string"},
+    #         "remote-asn": {"type": "integer"},
+    #         "local-asn": {"type": "integer"},
+    #         "hostname": {"type": "string"}
+    #     },
+    #     # local/remote-asn and/or description are not always present,
+    #     # just based on empirical tests - not a problem
+    #     "required": ["logical-system", "group", "address"],
+    #     "additionalProperties": False
+    # }
+    #
+    # TOP_LEVEL_PEERING_SCHEMA = {
+    #     "type": "object",
+    #     "properties": {
+    #         "group": {"type": "string"},
+    #         "description": {"type": "string"},
+    #         "address": {"type": "string"},
+    #         "remote-asn": {"type": "integer"},
+    #         "local-asn": {"type": "integer"},
+    #         "hostname": {"type": "string"}
+    #     },
+    #     # lots of internal peerings - so maybe no explicit asn's
+    #     "required": ["group", "address"],
+    #     "additionalProperties": False
+    # }
+    #
+    # INSTANCE_PEERING = {
+    #     "type": "object",
+    #     "properties": {
+    #         "instance": {"type": "string"},
+    #         "group": {"type": "string"},
+    #         "description": {"type": "string"},
+    #         "address": {"type": "string"},
+    #         "remote-asn": {"type": "integer"},
+    #         "local-asn": {"type": "integer"},
+    #         "hostname": {"type": "string"}
+    #     },
+    #     # description and-or local-asn is not always present,
+    #     # just based on empirical tests - not a problem
+    #     "required": ["instance", "group", "address", "remote-asn"],
+    #     "additionalProperties": False
+    # }
+    #
+    # DETAILED_PEERING_LIST_SCHEMA = {
+    #     "$schema": "http://json-schema.org/draft-07/schema#",
+    #     "definitions": {
+    #         "top-level-peering": TOP_LEVEL_PEERING_SCHEMA,
+    #         "instance-peering": INSTANCE_PEERING,
+    #         "logical-system-peering": LOGICAL_SYSTEM_PEERING_SCHEMA,
+    #         "peering": {
+    #             "oneOf": [
+    #                 {"$ref": "#/definitions/top-level-peering"},
+    #                 {"$ref": "#/definitions/instance-peering"},
+    #                 {"$ref": "#/definitions/logical-system-peering"}
+    #             ]
+    #         }
+    #     },
+    #     "type": "array",
+    #     "items": {"$ref": "#/definitions/peering"}
+    # }
+
+    # PEERING_LIST_SCHEMA = {
+    #     "$schema": "http://json-schema.org/draft-07/schema#",
+    #     "definitions": {
+    #         "peering": {
+    #             "type": "object",
+    #             "properties": {
+    #                 "group": {"type": "string"},
+    #                 "description": {"type": "string"},
+    #                 "address": {"type": "string"},
+    #                 "remote-asn": {"type": "integer"},
+    #                 "local-asn": {"type": "integer"},
+    #                 "instance": {"type": "string"},
+    #                 "logical-system": {"type": "string"},
+    #             },
+    #             # lots of internal peerings - so maybe no explicit asn's
+    #             "required": ["group", "address"],
+    #             "additionalProperties": False
+    #         }
+    #     },
+    #     "type": "array",
+    #     "items": {"$ref": "#/definitions/peering"}
+    # }
+
+    logical_system_peering_list_schema = copy.deepcopy(msr.PEERING_LIST_SCHEMA)
+    logical_system_peering_list_schema[
+        'definitions']['peering-instance']['required'].append('logical-system')
 
     db = backend_db()  # also forces initialization
 
@@ -194,16 +221,15 @@ def test_build_juniper_peering_db(mocked_worker_module):
                 assert address == canonical
             continue
 
-        jsonschema.validate(value, DETAILED_PEERING_LIST_SCHEMA)
+        jsonschema.validate(value, msr.PEERING_LIST_SCHEMA)
 
         if 'logical-system:' in key:
-            jsonschema.validate(value, msr.PEERING_LIST_SCHEMA)
+            jsonschema.validate(value, logical_system_peering_list_schema)
             m = re.match(r'.*logical-system:(.+)$', key)
             assert all(p['logical-system'] == m.group(1) for p in value)
             found_logical_system = True
 
         if 'group:' in key:
-            jsonschema.validate(value, msr.PEERING_LIST_SCHEMA)
             m = re.match(r'.*group:(.+)$', key)
             assert all(p['group'] == m.group(1) for p in value)
             found_group = True