From c637104078d5b5f2bc99ce2ef77af38f89bbdeea Mon Sep 17 00:00:00 2001 From: Bruno Inec Date: Sun, 7 Apr 2019 12:17:11 +0200 Subject: [PATCH] Allow Netbox device modification (#53631) * netbox_device: Allow device modification * Add ability to update and existing device * Allow check_mode * Fail when device name is missing * Fail when cannot resolve ID instead of taking ID 1 by default * netbox_device: Add diff output * netbox: Some refactoring * Add diff output and check_mode to netbox_ip_address * Deduplicate redundant code into netbox_utils * netbox_utils: A few unit tests --- .../net_tools/netbox/netbox_utils.py | 6 +- .../modules/net_tools/netbox/netbox_device.py | 115 +++++++------ .../net_tools/netbox/netbox_ip_address.py | 30 +++- .../module_utils/net_tools/test_netbox.py | 152 ++++++++++++++++++ 4 files changed, 248 insertions(+), 55 deletions(-) create mode 100644 test/units/module_utils/net_tools/test_netbox.py diff --git a/lib/ansible/module_utils/net_tools/netbox/netbox_utils.py b/lib/ansible/module_utils/net_tools/netbox/netbox_utils.py index 56fe9b572f2..a6b92f68137 100644 --- a/lib/ansible/module_utils/net_tools/netbox/netbox_utils.py +++ b/lib/ansible/module_utils/net_tools/netbox/netbox_utils.py @@ -237,7 +237,7 @@ def update_netbox_object(nb_obj, data, check_mode): if not check_mode: nb_obj.update(data) - udpated_obj = nb_obj.serialize() + updated_obj = nb_obj.serialize() diff = _build_diff(before=data_before, after=data_after) return updated_obj, diff @@ -312,7 +312,7 @@ def find_ids(nb, data): try: query_id = nb_endpoint.get(**{QUERY_TYPES.get(k, "q"): search}) except ValueError: - return ValueError( + raise ValueError( "Multiple results found while searching for key: %s" % (k) ) @@ -320,6 +320,8 @@ def find_ids(nb, data): data[k] = id_list elif query_id: data[k] = query_id.id + elif k in NO_DEFAULT_ID: + pass else: raise ValueError("Could not resolve id of %s: %s" % (k, v)) diff --git a/lib/ansible/modules/net_tools/netbox/netbox_device.py b/lib/ansible/modules/net_tools/netbox/netbox_device.py index a08be6e8ab8..d74dac6af9f 100644 --- a/lib/ansible/modules/net_tools/netbox/netbox_device.py +++ b/lib/ansible/modules/net_tools/netbox/netbox_device.py @@ -14,9 +14,9 @@ ANSIBLE_METADATA = {'metadata_version': '1.1', DOCUMENTATION = r''' --- module: netbox_device -short_description: Create or delete devices within Netbox +short_description: Create, update or delete devices within Netbox description: - - Creates or removes devices from Netbox + - Creates, updates or removes devices from Netbox notes: - Tags should be defined as a YAML list - This should be ran with connection C(local) and hosts C(localhost) @@ -42,12 +42,13 @@ options: name: description: - The name of the device + required: true device_type: description: - - Required if I(state=present) + - Required if I(state=present) and the device does not exist yet device_role: description: - - Required if I(state=present) + - Required if I(state=present) and the device does not exist yet tenant: description: - The tenant that the device will be assigned to @@ -62,7 +63,7 @@ options: - Asset tag that is associated to the device site: description: - - Required if I(state=present) + - Required if I(state=present) and the device does not exist yet rack: description: - The name of the rack to assign the device to @@ -119,7 +120,7 @@ EXAMPLES = r''' netbox_url: http://netbox.local netbox_token: thisIsMyToken data: - name: Test (not really required, but helpful) + name: Test Device device_type: C9410R device_role: Core Switch site: Main @@ -130,7 +131,7 @@ EXAMPLES = r''' netbox_url: http://netbox.local netbox_token: thisIsMyToken data: - name: Test + name: Test Device state: absent - name: Create device with tags @@ -138,7 +139,7 @@ EXAMPLES = r''' netbox_url: http://netbox.local netbox_token: thisIsMyToken data: - name: Test + name: Another Test Device device_type: C9410R device_role: Core Switch site: Main @@ -146,24 +147,22 @@ EXAMPLES = r''' - Schnozzberry state: present - - name: Create device and assign to rack and position + - name: Update the rack and position of an existing device netbox_device: netbox_url: http://netbox.local netbox_token: thisIsMyToken data: - name: Test - device_type: C9410R - device_role: Core Switch - site: Main + name: Test Device rack: Test Rack position: 10 face: Front + state: present ''' RETURN = r''' device: description: Serialized object as created or already existent within Netbox - returned: on creation + returned: success (when I(state=present)) type: dict msg: description: Message indicating failure or info about what has been achieved @@ -175,7 +174,15 @@ import json import traceback from ansible.module_utils.basic import AnsibleModule, missing_required_lib -from ansible.module_utils.net_tools.netbox.netbox_utils import find_ids, normalize_data, DEVICE_STATUS, FACE_ID +from ansible.module_utils.net_tools.netbox.netbox_utils import ( + find_ids, + normalize_data, + create_netbox_object, + delete_netbox_object, + update_netbox_object, + DEVICE_STATUS, + FACE_ID +) PYNETBOX_IMP_ERR = None try: @@ -198,13 +205,18 @@ def main(): validate_certs=dict(type="bool", default=True) ) + global module module = AnsibleModule(argument_spec=argument_spec, - supports_check_mode=False) + supports_check_mode=True) # Fail module if pynetbox is not installed if not HAS_PYNETBOX: module.fail_json(msg=missing_required_lib('pynetbox'), exception=PYNETBOX_IMP_ERR) + # Fail if device name is not given + if not module.params["data"].get("name"): + module.fail_json(msg="missing device name") + # Assign variables to be used with module app = 'dcim' endpoint = 'devices' @@ -228,59 +240,72 @@ def main(): norm_data = normalize_data(data) try: if 'present' in state: - return module.exit_json( - **ensure_device_present(nb, nb_endpoint, norm_data) - ) + result = ensure_device_present(nb, nb_endpoint, norm_data) else: - return module.exit_json( - **ensure_device_absent(nb_endpoint, norm_data) - ) + result = ensure_device_absent(nb_endpoint, norm_data) + return module.exit_json(**result) except pynetbox.RequestError as e: return module.fail_json(msg=json.loads(e.error)) + except ValueError as e: + return module.fail_json(msg=str(e)) + + +def _find_ids(nb, data): + if data.get("status"): + data["status"] = DEVICE_STATUS.get(data["status"].lower()) + if data.get("face"): + data["face"] = FACE_ID.get(data["face"].lower()) + return find_ids(nb, data) -def ensure_device_present(nb, nb_endpoint, data): +def ensure_device_present(nb, nb_endpoint, normalized_data): ''' - :returns dict(device, msg, changed): dictionary resulting of the request, + :returns dict(device, msg, changed, diff): dictionary resulting of the request, where `device` is the serialized device fetched or newly created in Netbox ''' + data = _find_ids(nb, normalized_data) nb_device = nb_endpoint.get(name=data["name"]) + result = {} if not nb_device: - device = _netbox_create_device(nb, nb_endpoint, data).serialize() - changed = True + device, diff = create_netbox_object(nb_endpoint, data, module.check_mode) msg = "Device %s created" % (data["name"]) + changed = True + result["diff"] = diff else: - msg = "Device %s already exists" % (data["name"]) - device = nb_device.serialize() - changed = False - - return {"device": device, "msg": msg, "changed": changed} - - -def _netbox_create_device(nb, nb_endpoint, data): - if data.get("status"): - data["status"] = DEVICE_STATUS.get(data["status"].lower(), 0) - if data.get("face"): - data["face"] = FACE_ID.get(data["face"].lower(), 0) - data = find_ids(nb, data) - return nb_endpoint.create(data) + device, diff = update_netbox_object(nb_device, data, module.check_mode) + if device is False: + module.fail_json( + msg="Request failed, couldn't update device: %s" % data["name"] + ) + if diff: + msg = "Device %s updated" % (data["name"]) + changed = True + result["diff"] = diff + else: + msg = "Device %s already exists" % (data["name"]) + changed = False + result.update({"device": device, "changed": changed, "msg": msg}) + return result def ensure_device_absent(nb_endpoint, data): ''' - :returns dict(msg, changed) + :returns dict(msg, changed, diff) ''' - device = nb_endpoint.get(name=data["name"]) - if device: - device.delete() + nb_device = nb_endpoint.get(name=data["name"]) + result = {} + if nb_device: + dummy, diff = delete_netbox_object(nb_device, module.check_mode) msg = 'Device %s deleted' % (data["name"]) changed = True + result["diff"] = diff else: msg = 'Device %s already absent' % (data["name"]) changed = False - return {"msg": msg, "changed": changed} + result.update({"changed": changed, "msg": msg}) + return result if __name__ == "__main__": diff --git a/lib/ansible/modules/net_tools/netbox/netbox_ip_address.py b/lib/ansible/modules/net_tools/netbox/netbox_ip_address.py index ff03d77a245..4b94f5ac5fd 100644 --- a/lib/ansible/modules/net_tools/netbox/netbox_ip_address.py +++ b/lib/ansible/modules/net_tools/netbox/netbox_ip_address.py @@ -223,7 +223,14 @@ import json import traceback from ansible.module_utils.basic import AnsibleModule, missing_required_lib -from ansible.module_utils.net_tools.netbox.netbox_utils import find_ids, normalize_data, IP_ADDRESS_ROLE, IP_ADDRESS_STATUS +from ansible.module_utils.net_tools.netbox.netbox_utils import ( + find_ids, + normalize_data, + create_netbox_object, + delete_netbox_object, + IP_ADDRESS_ROLE, + IP_ADDRESS_STATUS +) from ansible.module_utils.compat import ipaddress from ansible.module_utils._text import to_text @@ -249,8 +256,9 @@ def main(): validate_certs=dict(type="bool", default=True) ) + global module module = AnsibleModule(argument_spec=argument_spec, - supports_check_mode=False) + supports_check_mode=True) # Fail module if pynetbox is not installed if not HAS_PYNETBOX: @@ -383,11 +391,11 @@ def create_ip_address(nb_endpoint, data): changed = False return {"msg": data, "changed": changed} - ip_addr = nb_endpoint.create(data).serialize() + ip_addr, diff = create_netbox_object(nb_endpoint, data, module.check_mode) changed = True msg = "IP Addresses %s created" % (data["address"]) - return {"ip_address": ip_addr, "msg": msg, "changed": changed} + return {"ip_address": ip_addr, "msg": msg, "changed": changed, "diff": diff} def ensure_ip_in_prefix_present_on_netif(nb_app, nb_endpoint, data): @@ -424,21 +432,24 @@ def get_new_available_ip_address(nb_app, data): if data.get("vrf"): prefix_query["vrf_id"] = data["vrf"] + result = {} prefix = nb_app.prefixes.get(**prefix_query) if not prefix: changed = False msg = "%s does not exist - please create first" % (data["prefix"]) return {"msg": msg, "changed": changed} elif prefix.available_ips.list(): - ip_addr = prefix.available_ips.create(data) + ip_addr, diff = create_netbox_object(prefix.available_ips, data, module.check_mode) changed = True msg = "IP Addresses %s created" % (ip_addr["address"]) + result["diff"] = diff else: changed = False msg = "No available IPs available within %s" % (data['prefix']) return {"msg": msg, "changed": changed} - return {"ip_address": ip_addr, "msg": msg, "changed": changed} + result.update({"ip_address": ip_addr, "msg": msg, "changed": changed}) + return result def _get_prefix_id(nb_app, prefix, vrf_id=None): @@ -476,15 +487,18 @@ def ensure_ip_address_absent(nb_endpoint, data): except ValueError: return _error_multiple_ip_results(data) + result = {} if ip_addr: - ip_addr.delete() + dummy, diff = delete_netbox_object(ip_addr, module.check_mode) changed = True msg = "IP Address %s deleted" % (data["address"]) + result["diff"] = diff else: changed = False msg = "IP Address %s already absent" % (data["address"]) - return {"msg": msg, "changed": changed} + result.update({"msg": msg, "changed": changed}) + return result if __name__ == "__main__": diff --git a/test/units/module_utils/net_tools/test_netbox.py b/test/units/module_utils/net_tools/test_netbox.py new file mode 100644 index 00000000000..72584438051 --- /dev/null +++ b/test/units/module_utils/net_tools/test_netbox.py @@ -0,0 +1,152 @@ +# -*- coding: utf-8 -*- +# Copyright: (c) 2019, Bruno Inec (@sweenu) +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +import pytest + +from ansible.module_utils.net_tools.netbox.netbox_utils import ( + QUERY_TYPES, + _build_diff, + create_netbox_object, + delete_netbox_object, + update_netbox_object, + normalize_data, +) + + +def test_normalize_data(): + assert "name" not in QUERY_TYPES + assert QUERY_TYPES.get("rack") == "slug" + assert QUERY_TYPES.get("primary_ip") != "slug" + + raw_data = { + "name": "Some name", + "primary_ip": "10.3.72.74/31", + "rack": "Some rack", + } + normalized_data = raw_data.copy() + normalized_data["rack"] = "some-rack" + + assert normalize_data(raw_data) == normalized_data + + +def test_build_diff(): + before = "The state before" + after = {"A": "more", "complicated": "state"} + diff = _build_diff(before=before, after=after) + assert diff == {"before": before, "after": after} + + +@pytest.fixture +def nb_obj_mock(mocker): + serialized_object = {"The serialized": "object"} + nb_obj = mocker.Mock(name="nb_obj_mock") + nb_obj.delete.return_value = True + nb_obj.update.return_value = True + nb_obj.update.side_effect = serialized_object.update + nb_obj.serialize.return_value = serialized_object + + return nb_obj + + +@pytest.fixture +def endpoint_mock(mocker, nb_obj_mock): + endpoint = mocker.Mock(name="endpoint_mock") + endpoint.create.return_value = nb_obj_mock + + return endpoint + + +@pytest.fixture +def on_creation_diff(): + return _build_diff(before={"state": "absent"}, after={"state": "present"}) + + +@pytest.fixture +def on_deletion_diff(): + return _build_diff(before={"state": "present"}, after={"state": "absent"}) + + +@pytest.fixture +def data(): + return {"name": "Some Netbox object name"} + + +def test_create_netbox_object(endpoint_mock, data, on_creation_diff): + return_value = endpoint_mock.create().serialize() + + serialized_obj, diff = create_netbox_object( + endpoint_mock, data, check_mode=False + ) + assert endpoint_mock.create.called_once_with(data) + assert serialized_obj == return_value + assert diff == on_creation_diff + + +def test_create_netbox_object_in_check_mode(endpoint_mock, data, on_creation_diff): + serialized_obj, diff = create_netbox_object( + endpoint_mock, data, check_mode=True + ) + assert endpoint_mock.create.not_called() + assert serialized_obj == data + assert diff == on_creation_diff + + +def test_delete_netbox_object(nb_obj_mock, on_deletion_diff): + serialized_obj, diff = delete_netbox_object(nb_obj_mock, check_mode=False) + assert nb_obj_mock.delete.called_once() + assert serialized_obj == nb_obj_mock.serialize() + assert diff == on_deletion_diff + + +def test_delete_netbox_object_in_check_mode(nb_obj_mock, on_deletion_diff): + serialized_obj, diff = delete_netbox_object(nb_obj_mock, check_mode=True) + assert nb_obj_mock.delete.not_called() + assert serialized_obj == nb_obj_mock.serialize() + assert diff == on_deletion_diff + + +def test_update_netbox_object_no_changes(nb_obj_mock): + unchanged_data = nb_obj_mock.serialize() + serialized_obj, diff = update_netbox_object(nb_obj_mock, unchanged_data, check_mode=True) + assert nb_obj_mock.update.not_called() + assert serialized_obj == unchanged_data + assert diff is None + + +@pytest.fixture +def changed_serialized_obj(nb_obj_mock): + changed_serialized_obj = nb_obj_mock.serialize().copy() + changed_serialized_obj[list(changed_serialized_obj.keys())[0]] += " (modified)" + + return changed_serialized_obj + + +@pytest.fixture +def on_update_diff(nb_obj_mock, changed_serialized_obj): + return _build_diff(before=nb_obj_mock.serialize().copy(), after=changed_serialized_obj) + + +def test_update_netbox_object_with_changes( + nb_obj_mock, changed_serialized_obj, on_update_diff +): + serialized_obj, diff = update_netbox_object( + nb_obj_mock, changed_serialized_obj, check_mode=False + ) + assert nb_obj_mock.update.called_once_with(changed_serialized_obj) + assert serialized_obj == nb_obj_mock.serialize() + assert diff == on_update_diff + + +def test_update_netbox_object_with_changes_in_check_mode( + nb_obj_mock, changed_serialized_obj, on_update_diff +): + updated_serialized_obj = nb_obj_mock.serialize().copy() + updated_serialized_obj.update(changed_serialized_obj) + + serialized_obj, diff = update_netbox_object( + nb_obj_mock, changed_serialized_obj, check_mode=True + ) + assert nb_obj_mock.update.not_called() + + assert serialized_obj == updated_serialized_obj + assert diff == on_update_diff