From e6ebdd37bde1289380cdde7e1334a440a1144256 Mon Sep 17 00:00:00 2001 From: "Frank A. Zdarsky" Date: Thu, 4 Aug 2016 20:24:37 +0200 Subject: [PATCH] Add jsonschema validation for os-net-config data The current network environment validation (see change Ie6bdfcaa78cf831bcb182a2de9fc813ae2fb83be) only performs basic checks of NIC config files, e.g. checking the multiplicity of bonds and interfaces on OvS bridges. This patch leverages the jsonschema validation added to os-net-config in change Ie4a905863b2d46c88d9cd6c3afc50e7d0a877090 to validate the network environment configs against a full schema, catching a larger class of errors (typos, missing required parameters, etc.) for all network elements (e.g. linux bridges, OvS tunnels) configurable through os-net-config. Change-Id: Ic16ee0bc353c46f8fe512454176a07ee95347346 Depends-On: Ie4a905863b2d46c88d9cd6c3afc50e7d0a877090 Signed-off-by: Frank A. Zdarsky --- ...vironment-validation-ce98d775d9e1b17f.yaml | 5 ++ requirements.txt | 1 + .../tests/library/test_network_environment.py | 90 +++++++++++++------ validations/library/network_environment.py | 41 +++------ 4 files changed, 84 insertions(+), 53 deletions(-) create mode 100644 releasenotes/notes/network-environment-validation-ce98d775d9e1b17f.yaml diff --git a/releasenotes/notes/network-environment-validation-ce98d775d9e1b17f.yaml b/releasenotes/notes/network-environment-validation-ce98d775d9e1b17f.yaml new file mode 100644 index 000000000..c445f111a --- /dev/null +++ b/releasenotes/notes/network-environment-validation-ce98d775d9e1b17f.yaml @@ -0,0 +1,5 @@ +--- +features: + - | + Updated the network environment validation to use the new schema-based + validation method that would detect more error cases. diff --git a/requirements.txt b/requirements.txt index 6e0b1f65c..787456c2f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,3 +6,4 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0 oslo.config!=4.3.0,!=4.4.0,>=4.0.0 # Apache-2.0 keystoneauth1>=2.21.0 # Apache-2.0 python-heatclient>=1.6.1 # Apache-2.0 +os-net-config>=7.1.0 # Apache-2.0 diff --git a/tripleo_validations/tests/library/test_network_environment.py b/tripleo_validations/tests/library/test_network_environment.py index 5ee91d292..f15ae9835 100644 --- a/tripleo_validations/tests/library/test_network_environment.py +++ b/tripleo_validations/tests/library/test_network_environment.py @@ -165,8 +165,7 @@ class TestNicConfigs(base.TestCase): nic_data = self.nic_data(None) errors = validation.check_nic_configs("controller.yaml", nic_data) self.assertEqual(len(errors), 1) - self.assertEqual("The 'network_config' property of 'foo' must be" - " a list.", errors[0]) + self.assertIn("'foo' must be a list", errors[0]) def test_bridge_has_type(self): nic_data = self.nic_data([{ @@ -175,7 +174,15 @@ class TestNicConfigs(base.TestCase): }]) errors = validation.check_nic_configs("controller.yaml", nic_data) self.assertEqual(len(errors), 1) - self.assertIn('must have a type', errors[0]) + self.assertIn("'type' is a required property", errors[0]) + + def test_bridge_is_of_known_type(self): + nic_data = self.nic_data([{ + 'type': 'intreface' + }]) + errors = validation.check_nic_configs("controller.yaml", nic_data) + self.assertEqual(len(errors), 1) + self.assertIn("{'type': 'intreface'} is not valid", errors[0]) def test_bridge_has_name(self): nic_data = self.nic_data([{ @@ -184,7 +191,18 @@ class TestNicConfigs(base.TestCase): }]) errors = validation.check_nic_configs("controller.yaml", nic_data) self.assertEqual(len(errors), 1) - self.assertIn('must have a name', errors[0]) + self.assertIn("'name' is a required property", errors[0]) + + def test_bridge_has_only_known_properties(self): + nic_data = self.nic_data([{ + 'type': 'ovs_bridge', + 'name': 'storage', + 'member': [], + }]) + errors = validation.check_nic_configs("controller.yaml", nic_data) + self.assertEqual(len(errors), 1) + self.assertIn("Additional properties are not allowed" + " ('member' was unexpected)", errors[0]) def test_ovs_bridge_has_members(self): nic_data = self.nic_data([{ @@ -194,7 +212,8 @@ class TestNicConfigs(base.TestCase): }]) errors = validation.check_nic_configs("controller.yaml", nic_data) self.assertEqual(len(errors), 1) - self.assertIn("must contain a 'members' list", errors[0]) + self.assertIn("members/type: 'None' is not of type 'array'", + errors[0]) def test_ovs_bridge_members_dict(self): nic_data = self.nic_data([{ @@ -203,42 +222,55 @@ class TestNicConfigs(base.TestCase): 'members': [None], }]) errors = validation.check_nic_configs("controller.yaml", nic_data) - self.assertEqual(len(errors), 2) - self.assertIn("must be a dictionary.", errors[0]) - self.assertIn("at least 1 interface", errors[1]) + self.assertEqual(len(errors), 1) + self.assertIn("members/items/oneOf: None is not valid under any" + " of the given schemas", errors[0]) - def test_bonds_have_type(self): + def test_bonds_have_known_type(self): nic_data = self.nic_data([{ - 'type': 'ovs_bridge', + 'type': 'magic_bridge', 'name': 'storage', 'members': [{}], }]) errors = validation.check_nic_configs("controller.yaml", nic_data) - self.assertEqual(len(errors), 2) - self.assertIn("must have a type.", errors[0]) - self.assertIn("at least 1 interface", errors[1]) + self.assertEqual(len(errors), 1) + self.assertIn("members/items/oneOf: {} is not valid under any" + " of the given schemas", errors[0]) def test_more_than_one_bond(self): nic_data = self.nic_data([{ 'type': 'ovs_bridge', 'name': 'storage', 'members': [ - {'type': 'ovs_bond'}, - {'type': 'ovs_bond'}, + { + 'type': 'ovs_bond', + 'name': 'bond0', + 'members': [ + {'type': 'interface', 'name': 'eth0'}, + {'type': 'interface', 'name': 'eth1'}, + ] + }, { + 'type': 'ovs_bond', + 'name': 'bond1', + 'members': [ + {'type': 'interface', 'name': 'eth2'}, + {'type': 'interface', 'name': 'eth3'}, + ] + }, ], }]) errors = validation.check_nic_configs("controller.yaml", nic_data) self.assertEqual(len(errors), 1) - self.assertIn('Invalid bonding: There are 2 bonds for bridge storage', - errors[0]) + self.assertIn('Invalid bonding: There are >= 2 bonds for bridge ' + 'storage', errors[0]) def test_multiple_interfaces_without_bond(self): nic_data = self.nic_data([{ 'type': 'ovs_bridge', 'name': 'storage', 'members': [ - {'type': 'interface'}, - {'type': 'interface'}, + {'type': 'interface', 'name': 'eth0'}, + {'type': 'interface', 'name': 'eth1'}, ], }]) errors = validation.check_nic_configs("controller.yaml", nic_data) @@ -251,7 +283,7 @@ class TestNicConfigs(base.TestCase): 'type': 'ovs_bridge', 'name': 'storage', 'members': [ - {'type': 'interface'}, + {'type': 'interface', 'name': 'eth0'}, ], }]) errors = validation.check_nic_configs("controller.yaml", nic_data) @@ -262,20 +294,28 @@ class TestNicConfigs(base.TestCase): 'type': 'ovs_bridge', 'name': 'storage', 'members': [ - {'type': 'ovs_bond'}, + {'type': 'ovs_bond', 'name': 'bond0', 'members': []}, ], }]) errors = validation.check_nic_configs("controller.yaml", nic_data) - self.assertEqual([], errors) + self.assertEqual(len(errors), 1) + self.assertIn('members/minItems: [] is too short', errors[0]) def test_one_bond_multiple_interfaces(self): nic_data = self.nic_data([{ 'type': 'ovs_bridge', 'name': 'storage', 'members': [ - {'type': 'ovs_bond'}, - {'type': 'interface'}, - {'type': 'interface'}, + { + 'type': 'ovs_bond', + 'name': 'bond0', + 'members': [ + {'type': 'interface', 'name': 'eth2'}, + {'type': 'interface', 'name': 'eth3'}, + ] + }, + {'type': 'interface', 'name': 'eth0'}, + {'type': 'interface', 'name': 'eth1'}, ], }]) errors = validation.check_nic_configs("controller.yaml", nic_data) diff --git a/validations/library/network_environment.py b/validations/library/network_environment.py index 4be412a33..aded6d530 100644 --- a/validations/library/network_environment.py +++ b/validations/library/network_environment.py @@ -23,6 +23,7 @@ import yaml import six from ansible.module_utils.basic import AnsibleModule +from os_net_config import validator def open_network_environment_files(netenv_path, template_files): @@ -149,47 +150,31 @@ def check_nic_configs(path, nic_data): # Not all resources contain a network config: if not bridges: continue + + # Validate the os_net_config object against the schema + v_errors = validator.validate_config(bridges, path) + errors.extend(v_errors) + if len(v_errors) > 0: + continue + + # If we get here, the nic config file conforms to the schema and + # there is no more need to check for existence and type of + # properties. for bridge in bridges: - if 'type' not in bridge: - errors.append("The bridge item {} in {} {} must have a type." - .format(bridge, name, path)) - continue - if 'name' not in bridge: - errors.append("The bridge item {} in {} {} must have a name." - .format(bridge, name, path)) - continue if bridge['type'] == 'ovs_bridge': bond_count = 0 interface_count = 0 - if not isinstance(bridge.get('members'), collections.Iterable): - errors.append( - "OVS bridge {} in {} {} must contain a 'members' list." - .format(bridge, name, path)) - continue for bridge_member in bridge['members']: - if not isinstance(bridge_member, collections.Mapping): - errors.append( - "The {} bridge member in {} {} must be a " - "dictionary." - .format(bridge_member, name, path)) - continue - if 'type' not in bridge_member: - errors.append( - "The {} bridge member in {} {} must have a type." - .format(bridge_member, name, path)) - continue if bridge_member['type'] in ('ovs_bond', 'ovs_dpdk_bond'): bond_count += 1 elif bridge_member['type'] == 'interface': interface_count += 1 else: - # TODO(mandre) should we add an error for unknown - # bridge member types? pass - if bond_count == 2: + if bond_count >= 2: errors.append( - 'Invalid bonding: There are 2 bonds for' + 'Invalid bonding: There are >= 2 bonds for' ' bridge {} of resource {} in {}'.format( bridge['name'], name, path)) if bond_count == 0 and interface_count > 1: