From 97831d60152faf0b855f278867e9267067251e33 Mon Sep 17 00:00:00 2001
From: Erik Reid <erik.reid@geant.org>
Date: Tue, 2 Mar 2021 15:07:31 +0100
Subject: [PATCH] more general, but less strict peer list schema
also handle test errors caused by removing space filter
---
inventory_provider/juniper.py | 45 +++----
inventory_provider/routes/testing.py | 7 +-
test/per_router/test_celery_worker.py | 6 +
test/per_router/test_data_routes.py | 19 ++-
test/per_router/test_juniper_data.py | 12 +-
test/test_worker_utils.py | 166 +++++++++++++++-----------
6 files changed, 152 insertions(+), 103 deletions(-)
diff --git a/inventory_provider/juniper.py b/inventory_provider/juniper.py
index ddd2fce0..a73096b8 100644
--- a/inventory_provider/juniper.py
+++ b/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):
diff --git a/inventory_provider/routes/testing.py b/inventory_provider/routes/testing.py
index c9bdc7a1..442e8bc1 100644
--- a/inventory_provider/routes/testing.py
+++ b/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'])
diff --git a/test/per_router/test_celery_worker.py b/test/per_router/test_celery_worker.py
index 4ed2e382..e071f5f8 100644
--- a/test/per_router/test_celery_worker.py
+++ b/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)
diff --git a/test/per_router/test_data_routes.py b/test/per_router/test_data_routes.py
index b1fbdbe0..b795eb6e 100644
--- a/test/per_router/test_data_routes.py
+++ b/test/per_router/test_data_routes.py
@@ -1,7 +1,8 @@
+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
diff --git a/test/per_router/test_juniper_data.py b/test/per_router/test_juniper_data.py
index a7ac6df9..34f1b35b 100644
--- a/test/per_router/test_juniper_data.py
+++ b/test/per_router/test_juniper_data.py
@@ -1,7 +1,9 @@
+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'
diff --git a/test/test_worker_utils.py b/test/test_worker_utils.py
index 27727533..c164421f 100644
--- a/test/test_worker_utils.py
+++ b/test/test_worker_utils.py
@@ -1,6 +1,7 @@
"""
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
--
GitLab