From f69da5ea9581f21fbf2153667cf135bf6c3b8c1c Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Wed, 19 Feb 2020 14:21:52 +0100
Subject: [PATCH 01/16] Add initial low-level API library for CNaaS-NMS

This will change. I do not have access to an actual CNaaS-NMS instance
at the moment, and have too little experience with simple_rest_client to
do a proper test.
---
 python/nav/portadmin/cnaas_nms/__init__.py | 21 ++++++++
 python/nav/portadmin/cnaas_nms/lowlevel.py | 63 ++++++++++++++++++++++
 requirements/base.txt                      |  3 ++
 3 files changed, 87 insertions(+)
 create mode 100644 python/nav/portadmin/cnaas_nms/__init__.py
 create mode 100644 python/nav/portadmin/cnaas_nms/lowlevel.py

diff --git a/python/nav/portadmin/cnaas_nms/__init__.py b/python/nav/portadmin/cnaas_nms/__init__.py
new file mode 100644
index 0000000000..7d2fc33d94
--- /dev/null
+++ b/python/nav/portadmin/cnaas_nms/__init__.py
@@ -0,0 +1,21 @@
+#
+# Copyright (C) 2021 UNINETT
+#
+# This file is part of Network Administration Visualized (NAV).
+#
+# NAV is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License version 3 as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.  You should have received a copy of the GNU General Public License
+# along with NAV. If not, see <http://www.gnu.org/licenses/>.
+#
+"""This package implements functionality to proxy all PortAdmin configuration
+operations through the CNaaS-NMS API.
+
+See https://github.com/SUNET/cnaas-nms for reference
+
+"""
diff --git a/python/nav/portadmin/cnaas_nms/lowlevel.py b/python/nav/portadmin/cnaas_nms/lowlevel.py
new file mode 100644
index 0000000000..786779edb7
--- /dev/null
+++ b/python/nav/portadmin/cnaas_nms/lowlevel.py
@@ -0,0 +1,63 @@
+#
+# Copyright (C) 2021 UNINETT
+#
+# This file is part of Network Administration Visualized (NAV).
+#
+# NAV is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License version 3 as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.  You should have received a copy of the GNU General Public License
+# along with NAV. If not, see <http://www.gnu.org/licenses/>.
+#
+"""Low-level CNaaS-NMS REST API access using simple_rest_client"""
+from simple_rest_client.api import API
+from simple_rest_client.resource import Resource
+
+
+def get_api(url, token):
+    """Returns a CNaaS NMS API instance from a URL and a token"""
+    default_headers = {
+        "Authorization": "Bearer {}".format(token),
+    }
+    api = API(
+        api_root_url=url,
+        headers=default_headers,
+        params={},
+        timeout=2,
+        append_slash=False,
+        json_encode_body=True,
+    )
+    api.add_resource(resource_name="devices", resource_class=DeviceResource)
+    api.add_resource(resource_name="interfaces", resource_class=InterfaceResource)
+    api.add_resource(resource_name="device_sync", resource_class=DeviceSyncResource)
+    api.add_resource(resource_name="job")
+
+    return api
+
+
+class DeviceResource(Resource):
+    """Defines operations on the devices endpoint"""
+
+    actions = {"retrieve": {"method": "GET", "url": "/devices?filter[hostname]={}"}}
+
+
+class InterfaceResource(Resource):
+    """Defines operations on the interface sub-resource of a device"""
+
+    actions = {
+        "list": {"method": "GET", "url": "/device/{}/interfaces"},
+        "configure": {
+            "method": "PUT",
+            "url": "/device/{}/interfaces",
+        },
+    }
+
+
+class DeviceSyncResource(Resource):
+    """Defines the API syncto operations"""
+
+    actions = {"syncto": {"method": "POST", "url": "/device_syncto"}}
diff --git a/requirements/base.txt b/requirements/base.txt
index ad76311084..9e40ac3384 100644
--- a/requirements/base.txt
+++ b/requirements/base.txt
@@ -25,3 +25,6 @@ pynetsnmp-2==0.1.5
 libsass==0.15.1
 
 napalm==3.0.1
+
+# For 3rd party REST API access
+simple_rest_client==1.0.8

From 4a11fe519e52c7690de1b98bf9e384b9886701a1 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Thu, 20 Feb 2020 11:14:35 +0100
Subject: [PATCH 02/16] Add PortAdmin config options for CNaaS NMS API

---
 python/nav/portadmin/config.py | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/python/nav/portadmin/config.py b/python/nav/portadmin/config.py
index fe85dc8600..9ffee3c715 100644
--- a/python/nav/portadmin/config.py
+++ b/python/nav/portadmin/config.py
@@ -15,10 +15,13 @@
 #
 """Tools to handle PortAdmin configuration database/file"""
 from os.path import join
+from collections import namedtuple
 
-from nav.config import NAVConfigParser
+from nav.config import NAVConfigParser, ConfigurationError
 from nav.portadmin.vlan import FantasyVlan
 
+CNaaSNMSConfig = namedtuple("CNaasNMSConfig", ["url", "token"])
+
 
 class PortAdminConfig(NAVConfigParser):
     """"PortAdmin config parser"""
@@ -42,6 +45,15 @@ class PortAdminConfig(NAVConfigParser):
 
 [dot1x]
 enabled = false
+
+[cnaas-nms]
+# These options can be used to configure PortAdmin to proxy all device write
+# operations through a CNaaS-NMS instance.
+# Refer to https://github.com/SUNET/cnaas-nms
+
+enabled = off
+#url=https://cnaas-nms.example.org/api/v1.0
+#token=very_long_and_secret_api_access_token
 """
 
     def is_vlan_authorization_enabled(self):
@@ -105,5 +117,21 @@ def is_cisco_voice_cdp_enabled(self):
         """Checks if the CDP config option is enabled"""
         return self.getboolean("general", "cisco_voice_cdp", fallback=False)
 
+    def is_cnaas_nms_enabled(self):
+        return self.getboolean("cnaas-nms", "enabled", fallback=False)
+
+    def get_cnaas_nms_config(
+        self, section="cnaas-nms", url_option="url", token_option="token"
+    ):
+        """Returns a CNaaSNMSConfig namedtuple if a CNaaS-NMS proxy is enabled"""
+        if not self.has_option(section, url_option):
+            raise ConfigurationError("Missing CNaaS-NMS API URL in configuration")
+        if not self.has_option(section, token_option):
+            raise ConfigurationError("Missing CNaaS-NMS API token in configuration")
+
+        return CNaaSNMSConfig(
+            self.get(section, url_option), self.get(section, token_option)
+        )
+
 
 CONFIG = PortAdminConfig()

From 655028dc8ada4ab00c96c0a530a7ba178fa7952d Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Thu, 20 Feb 2020 11:12:28 +0100
Subject: [PATCH 03/16] Add mostly empty CNaaSNMSMixIn implementation

Because:
- We want a MixIn class to build hybrid Portadmin handler classes on the
  fly, where read operations can be handled directly by the
  vendor-specific ManagementHandler and write operations can instead be
  dispatched to the CNaaS NMS API.
---
 python/nav/portadmin/cnaas_nms/proxy.py | 44 +++++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 python/nav/portadmin/cnaas_nms/proxy.py

diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
new file mode 100644
index 0000000000..5e8b1423c5
--- /dev/null
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -0,0 +1,44 @@
+#
+# Copyright (C) 2021 UNINETT
+#
+# This file is part of Network Administration Visualized (NAV).
+#
+# NAV is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License version 3 as published by
+# the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
+# details.  You should have received a copy of the GNU General Public License
+# along with NAV. If not, see <http://www.gnu.org/licenses/>.
+#
+from nav.models import manage
+from nav.portadmin.config import CONFIG
+from nav.portadmin.cnaas_nms.lowlevel import get_api
+from nav.portadmin.handlers import ManagementHandler
+
+
+class CNaaSNMSMixIn(ManagementHandler):
+    """MixIn to override all write-operations from
+    nav.portadmin.handlers.ManagementHandler and instead direct them through a CNaaS
+    NMS instance's REST API.
+
+    """
+
+    def __init__(self, netbox: manage.Netbox, **kwargs):
+        super().__init__(netbox, **kwargs)
+        config = CONFIG.get_cnaas_nms_config()
+        self._api = get_api(config.url, config.token)
+
+    def set_interface_description(self, interface: manage.Interface, description: str):
+        raise NotImplementedError
+
+    def set_interface_down(self, interface: manage.Interface):
+        raise NotImplementedError
+
+    def set_interface_up(self, interface: manage.Interface):
+        raise NotImplementedError
+
+    def commit_configuration(self):
+        raise NotImplementedError

From d072b86e5ff4c2fb7fc09b6117c413bddf6e6f06 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Thu, 20 Feb 2020 11:15:04 +0100
Subject: [PATCH 04/16] Make CNaaS NMS+ManagementHandler hybrid handlers

Because:
- When CNaaS NMS proxying is enabled, read operations should proceed as
  normal, directly against the device, using whatever ManagementHandler
  implementation deemed appropriate, while write operations should pass
  through the CNaaS NMS API.
---
 python/nav/portadmin/management.py | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/python/nav/portadmin/management.py b/python/nav/portadmin/management.py
index 6018ac0742..1bfa38a873 100644
--- a/python/nav/portadmin/management.py
+++ b/python/nav/portadmin/management.py
@@ -17,6 +17,8 @@
 from nav.errors import NoNetboxTypeError
 from nav.models import manage
 from nav.portadmin.handlers import ManagementHandler
+from nav.portadmin.cnaas_nms.proxy import CNaaSNMSMixIn
+from nav.portadmin.config import CONFIG
 from nav.portadmin.snmp.base import SNMPHandler
 from nav.portadmin.snmp.cisco import Cisco
 from nav.portadmin.snmp.dell import Dell
@@ -33,13 +35,33 @@ class ManagementFactory(object):
 
     @classmethod
     def get_instance(cls, netbox: manage.Netbox, **kwargs) -> ManagementHandler:
-        """Get and SNMP-handle depending on vendor type"""
+        """Returns a ManagementHandler implementation, depending on Netbox vendor and
+        configured management protocol.
+        """
         if not netbox.type:
             raise NoNetboxTypeError()
 
         vendor_id = netbox.type.get_enterprise_id()
         handler = VENDOR_MAP.get(vendor_id, SNMPHandler)
+
+        if CONFIG.is_cnaas_nms_enabled():
+            handler = cls._hybridize_cnaas_nms_handler(handler)
+
         return handler(netbox, **kwargs)
 
+    @classmethod
+    def _hybridize_cnaas_nms_handler(cls, handler):
+        """Builds and returns a hybrid ManagementHandler class.
+
+        The class will have two base classes, the CNaaSNMSMixIn class and handler,
+        thereby letting the CNaaSMixIn implementation override methods from handler
+        as it sees fit.
+        """
+
+        class HybridProxyHandler(CNaaSNMSMixIn, handler):
+            pass
+
+        return HybridProxyHandler
+
     def __init__(self):
         pass

From 5394c343b8e6dd5c68a235c9eabfaeba80e61bda Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Fri, 21 Feb 2020 09:32:39 +0100
Subject: [PATCH 05/16] Partially implement CNaaS NMS API handler methods

---
 python/nav/portadmin/cnaas_nms/proxy.py | 48 ++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 5 deletions(-)

diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
index 5e8b1423c5..f1ff99ecb3 100644
--- a/python/nav/portadmin/cnaas_nms/proxy.py
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -16,7 +16,11 @@
 from nav.models import manage
 from nav.portadmin.config import CONFIG
 from nav.portadmin.cnaas_nms.lowlevel import get_api
-from nav.portadmin.handlers import ManagementHandler
+from nav.portadmin.handlers import (
+    ManagementHandler,
+    DeviceNotConfigurableError,
+    ProtocolError,
+)
 
 
 class CNaaSNMSMixIn(ManagementHandler):
@@ -32,13 +36,47 @@ def __init__(self, netbox: manage.Netbox, **kwargs):
         self._api = get_api(config.url, config.token)
 
     def set_interface_description(self, interface: manage.Interface, description: str):
-        raise NotImplementedError
+        data = {"description": interface.ifalias}
+        payload = {"interfaces": {interface.ifdescr: data}}
+        self._api.interfaces.configure(self.netbox.sysname, body=payload)
 
     def set_interface_down(self, interface: manage.Interface):
-        raise NotImplementedError
+        self._set_interface_enabled(interface, enabled=False)
 
     def set_interface_up(self, interface: manage.Interface):
-        raise NotImplementedError
+        self._set_interface_enabled(interface, enabled=True)
+
+    def _set_interface_enabled(self, interface: manage.Interface, enabled=True):
+        data = {"enabled": enabled}
+        payload = {"interfaces": {interface.ifdescr: data}}
+        self._api.interfaces.configure(self.netbox.sysname, body=payload)
 
     def commit_configuration(self):
-        raise NotImplementedError
+        payload = {"hostname": self.netbox.sysname, "dry_run": False, "auto_push": True}
+        self._api.device_sync.syncto(body=payload)
+        # TODO: Get a job number from the syncto call
+        # TODO: Poll the job API for "status": "FINISHED"
+
+    def raise_if_not_configurable(self):
+        response = self._api.devices.retrieve(self.netbox.sysname)
+        payload = response.body
+        if response.status_code == 200 and payload.get("status") == "success":
+            if len(payload.get("devices", []) < 0):
+                raise CNaaSNMSApiError("No devices match this name in CNaaS NMS")
+            device = payload["devices"][0]
+            if not (
+                device.get("state") == "MANAGED"
+                and device.get("device_type") == "ACCESS"
+                and device.get("synchronized")
+            ):
+                raise DeviceNotConfigurableError(
+                    "CNaaS NMS will not permit configuration of this device"
+                )
+        else:
+            raise CNaaSNMSApiError("Cannot verify device status in CNaaS NMS")
+
+
+class CNaaSNMSApiError(ProtocolError):
+    """An exception raised whenever there is a problem with the responses from the
+    CNaaS NMS API
+    """

From 6dae13bb084ca13f20aa1b5ce39dc2eb8d58c8a9 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Thu, 5 Mar 2020 14:41:40 +0100
Subject: [PATCH 06/16] Attempt to appease PyLint

Because:
- HybridProxyHandler does not implement anything
- The simple_rest_client API class produces dynamic member attributes
  based on the API definition. PyLint is unable to determine what valid
  member attributes exists anyway.
---
 python/nav/portadmin/management.py | 3 +++
 python/pylint.rc                   | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/python/nav/portadmin/management.py b/python/nav/portadmin/management.py
index 1bfa38a873..7f1fc9e122 100644
--- a/python/nav/portadmin/management.py
+++ b/python/nav/portadmin/management.py
@@ -58,6 +58,9 @@ def _hybridize_cnaas_nms_handler(cls, handler):
         as it sees fit.
         """
 
+        # This class is constructed on the fly, no need to warn about missing
+        # implementations:
+        # pylint: disable=abstract-method
         class HybridProxyHandler(CNaaSNMSMixIn, handler):
             pass
 
diff --git a/python/pylint.rc b/python/pylint.rc
index ef862f093d..bf5d730dda 100644
--- a/python/pylint.rc
+++ b/python/pylint.rc
@@ -166,7 +166,7 @@ ignored-modules=ldap,django.utils.six.moves,_MovedItems,nav.junos.nav_views,jnpr
 
 # List of classes names for which member attributes should not be checked
 # (useful for classes with attributes dynamically set).
-ignored-classes=twisted.internet.reactor,Meta,Account,Arp,Cam,Category,Device,GwPortPrefix,Interface,Location,Module,NetType,Netbox,NetboxInfo,NetboxCategory,NetboxSnmpOid,NetboxType,Organization,Prefix,Room,Shadow,SwPortAllowedVlan,SwPortVlan,SwPortBlocked,Usage,Vendor,Vlan,CharField,OneToOneField,ForeignKey,Filter,Subcategory,EventType,Memory,AlertQueue,AlertQueueMessage,AlertType,Memory,AccountGroup,AlertProfile,AlertSubscription,Expression,Operator,AccountAlertQueue,AlertSender,AccountProperty,AlertAddress,TimePeriod,AlertPreference,MatchField,FilterGroup,FilterGroupContent,AccountNavbar,AlertHistory,AlertHistoryMessage,AlertHistoryVariable,Cabling,EventQueue,EventQueueVar,Message,NavbarLink,Patch,Privilege,SMSQueue,Service,ServiceProperty,StatusPreference,StatusPreferenceCategory,StatusPreferenceOrganization,Subsystem,AdjacencyCandidate,UnrecognizedNeighbor,ThresholdRule,AlertHistoryManager,HStoreManager,PrefixManager,MaintenanceTaskManager,APIToken,AccountDashboard,GatewayPeerSession,Sensor,RackManager,Rack,POEGroup
+ignored-classes=twisted.internet.reactor,Meta,Account,Arp,Cam,Category,Device,GwPortPrefix,Interface,Location,Module,NetType,Netbox,NetboxInfo,NetboxCategory,NetboxSnmpOid,NetboxType,Organization,Prefix,Room,Shadow,SwPortAllowedVlan,SwPortVlan,SwPortBlocked,Usage,Vendor,Vlan,CharField,OneToOneField,ForeignKey,Filter,Subcategory,EventType,Memory,AlertQueue,AlertQueueMessage,AlertType,Memory,AccountGroup,AlertProfile,AlertSubscription,Expression,Operator,AccountAlertQueue,AlertSender,AccountProperty,AlertAddress,TimePeriod,AlertPreference,MatchField,FilterGroup,FilterGroupContent,AccountNavbar,AlertHistory,AlertHistoryMessage,AlertHistoryVariable,Cabling,EventQueue,EventQueueVar,Message,NavbarLink,Patch,Privilege,SMSQueue,Service,ServiceProperty,StatusPreference,StatusPreferenceCategory,StatusPreferenceOrganization,Subsystem,AdjacencyCandidate,UnrecognizedNeighbor,ThresholdRule,AlertHistoryManager,HStoreManager,PrefixManager,MaintenanceTaskManager,APIToken,AccountDashboard,GatewayPeerSession,Sensor,RackManager,Rack,POEGroup,simple_rest_client.api.API
 
 # When zope mode is activated, add a predefined set of Zope acquired attributes
 # to generated-members.

From 883b3330da56613a60a8316a406150e586211e25 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Wed, 1 Jul 2020 13:08:28 +0200
Subject: [PATCH 07/16] Use IP address for CNaaS-NMS device lookups

Because:
- Device names aren't guaranteed to be the same in CNaaS-NMS and NAV:
  NAV's name is fetched from DNS, CNaaS-NMS' is set independently of
  DNS.
- The management IP address is expected to be the same in both systems.
---
 python/nav/portadmin/cnaas_nms/lowlevel.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/python/nav/portadmin/cnaas_nms/lowlevel.py b/python/nav/portadmin/cnaas_nms/lowlevel.py
index 786779edb7..9a79845a6f 100644
--- a/python/nav/portadmin/cnaas_nms/lowlevel.py
+++ b/python/nav/portadmin/cnaas_nms/lowlevel.py
@@ -42,7 +42,9 @@ def get_api(url, token):
 class DeviceResource(Resource):
     """Defines operations on the devices endpoint"""
 
-    actions = {"retrieve": {"method": "GET", "url": "/devices?filter[hostname]={}"}}
+    actions = {
+        "retrieve": {"method": "GET", "url": "/devices?filter[management_ip]={}"}
+    }
 
 
 class InterfaceResource(Resource):

From d47f800214e7c58470ff28c48823cbfbe6dd532a Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Wed, 1 Jul 2020 13:13:29 +0200
Subject: [PATCH 08/16] Extract _get_device_record

Because:
- CNaaSNMSMixIn will need a generic helper method to fetch a matching
  device record from CNaaS-NMS
- This helps separate concerns of generic device retrieval errors to
  that of verifying CNaaS NMS' willingness to actually configure this
  device.
---
 python/nav/portadmin/cnaas_nms/proxy.py | 35 ++++++++++++++++---------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
index f1ff99ecb3..7fe1221fd9 100644
--- a/python/nav/portadmin/cnaas_nms/proxy.py
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -58,22 +58,33 @@ def commit_configuration(self):
         # TODO: Poll the job API for "status": "FINISHED"
 
     def raise_if_not_configurable(self):
-        response = self._api.devices.retrieve(self.netbox.sysname)
+        device = self._get_device_record()
+        if not (
+            device.get("state") == "MANAGED"
+            and device.get("device_type") == "ACCESS"
+            and device.get("synchronized")
+        ):
+            raise DeviceNotConfigurableError(
+                "CNaaS NMS will not permit configuration of this device"
+            )
+
+    def _get_device_record(self):
+        response = self._api.devices.retrieve(self.netbox.ip)
         payload = response.body
         if response.status_code == 200 and payload.get("status") == "success":
-            if len(payload.get("devices", []) < 0):
-                raise CNaaSNMSApiError("No devices match this name in CNaaS NMS")
-            device = payload["devices"][0]
-            if not (
-                device.get("state") == "MANAGED"
-                and device.get("device_type") == "ACCESS"
-                and device.get("synchronized")
-            ):
-                raise DeviceNotConfigurableError(
-                    "CNaaS NMS will not permit configuration of this device"
+            data = payload.get("data", {})
+            if len(data.get("devices", [])) < 0:
+                raise CNaaSNMSApiError(
+                    "No devices matched {} in CNaaS-NMS".format(self.netbox.ip)
                 )
+            device = data["devices"][0]
+            return device
         else:
-            raise CNaaSNMSApiError("Cannot verify device status in CNaaS NMS")
+            raise CNaaSNMSApiError(
+                "Unknown failure when talking to CNaaS-NMS (code={}, status={})".format(
+                    response.status_code, payload.get("status")
+                )
+            )
 
 
 class CNaaSNMSApiError(ProtocolError):

From 4ba4689efb18edc636a66736325c59214983d4f4 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Wed, 1 Jul 2020 13:16:13 +0200
Subject: [PATCH 09/16] Rewrite raise_if_not_configurable()

Because:
- Error message should be more specific about which CNaaS NMS management
  criteria aren't met.
---
 python/nav/portadmin/cnaas_nms/lowlevel.py |  1 +
 python/nav/portadmin/cnaas_nms/proxy.py    | 41 +++++++++++++++++-----
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/python/nav/portadmin/cnaas_nms/lowlevel.py b/python/nav/portadmin/cnaas_nms/lowlevel.py
index 9a79845a6f..9021a0fda1 100644
--- a/python/nav/portadmin/cnaas_nms/lowlevel.py
+++ b/python/nav/portadmin/cnaas_nms/lowlevel.py
@@ -16,6 +16,7 @@
 """Low-level CNaaS-NMS REST API access using simple_rest_client"""
 from simple_rest_client.api import API
 from simple_rest_client.resource import Resource
+from simple_rest_client.exceptions import ClientError
 
 
 def get_api(url, token):
diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
index 7fe1221fd9..cc3c308956 100644
--- a/python/nav/portadmin/cnaas_nms/proxy.py
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -15,7 +15,7 @@
 #
 from nav.models import manage
 from nav.portadmin.config import CONFIG
-from nav.portadmin.cnaas_nms.lowlevel import get_api
+from nav.portadmin.cnaas_nms.lowlevel import get_api, ClientError
 from nav.portadmin.handlers import (
     ManagementHandler,
     DeviceNotConfigurableError,
@@ -58,14 +58,39 @@ def commit_configuration(self):
         # TODO: Poll the job API for "status": "FINISHED"
 
     def raise_if_not_configurable(self):
-        device = self._get_device_record()
-        if not (
-            device.get("state") == "MANAGED"
-            and device.get("device_type") == "ACCESS"
-            and device.get("synchronized")
-        ):
+        """Raises an exception if this device cannot be configured by CNaaS-NMS for
+        some reason.
+        """
+        try:
+            device = self._get_device_record()
+            self.raise_on_unmatched_criteria(device)
+        except CNaaSNMSApiError as error:
+            raise DeviceNotConfigurableError(str(error)) from error
+        except ClientError as error:
             raise DeviceNotConfigurableError(
-                "CNaaS NMS will not permit configuration of this device"
+                "Unexpected error talking to the CNaaS-NMS backend: " + str(error)
+            ) from error
+
+    def raise_on_unmatched_criteria(self, device_record):
+        """Raises an exception if the device's CNaaS-NMS attributes do not match the
+        preset criteria for being managed via the API.
+        """
+        state = device_record.get("state")
+        device_type = device_record.get("device_type")
+        synchronized = device_record.get("synchronized")
+
+        if state != "MANAGED":
+            raise DeviceNotConfigurableError(
+                "CNaaS-NMS does not list this device as managed ({})".format(state)
+            )
+        if device_type != "ACCESS":
+            raise DeviceNotConfigurableError(
+                "Cannot use CNaaS-NMS to configure {} type devices".format(device_type)
+            )
+        if not synchronized:
+            raise DeviceNotConfigurableError(
+                "Device configuration is not synchronized with CNaaS-NMS, cannot make "
+                "changes at the moment. Please try againt later."
             )
 
     def _get_device_record(self):

From dae3eebb23c4da84376008c68bbd6c9587a8ccc3 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Thu, 15 Apr 2021 14:11:47 +0200
Subject: [PATCH 10/16] Extract _bounce_interfaces function

Because:
- Although the actual method to cycle may be different in different
  subclasses, the logic for verification/filtering of the incoming
  interfaces list remains largely the same.
- This moves the heavy lifting to a private method that can be
  overriden by subclasses who do not need to reimplement or change
  the verification/filter logic.
---
 python/nav/portadmin/handlers.py | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/python/nav/portadmin/handlers.py b/python/nav/portadmin/handlers.py
index 2f79c3b505..85918bc0f3 100644
--- a/python/nav/portadmin/handlers.py
+++ b/python/nav/portadmin/handlers.py
@@ -105,12 +105,22 @@ def cycle_interfaces(
         assert netbox == self.netbox, "Interfaces belong to wrong netbox"
 
         to_cycle = self._filter_oper_up_interfaces(interfaces)
-        if not to_cycle:
-            _logger.debug("No interfaces to cycle on %s", netbox.sysname)
-            return
+        if to_cycle:
+            self._bounce_interface(to_cycle)
+
+    def _bounce_interfaces(
+        self,
+        interfaces: Sequence[manage.Interface],
+        wait: float = 5.0,
+        commit: bool = False,
+    ):
+        """The actual implementation of cycle_interfaces(), without the verification
+        code.
 
+        Override this in subclasses to keep the common verification code path.
+        """
         _logger.debug("Taking interfaces administratively down")
-        for ifc in to_cycle:
+        for ifc in interfaces:
             self.set_interface_down(ifc)
             _logger.debug(ifc.ifname)
         if commit:
@@ -120,7 +130,7 @@ def cycle_interfaces(
             time.sleep(wait)
 
         _logger.debug("Taking interfaces administratively up again")
-        for ifc in to_cycle:
+        for ifc in interfaces:
             self.set_interface_up(ifc)
             _logger.debug(ifc.ifname)
         if commit:

From 045f963f8a7a7d5350f1b71c8a88d47295664632 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Thu, 15 Apr 2021 14:31:24 +0200
Subject: [PATCH 11/16] Implement CNaaS-NMS interface_status API endpoint

The interface_status endpoint can be used to list the status of each
interface of a device, but also to initiate a bounce operation for
a list of interfaces on said device.
---
 python/nav/portadmin/cnaas_nms/lowlevel.py | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/python/nav/portadmin/cnaas_nms/lowlevel.py b/python/nav/portadmin/cnaas_nms/lowlevel.py
index 9021a0fda1..613783c9ce 100644
--- a/python/nav/portadmin/cnaas_nms/lowlevel.py
+++ b/python/nav/portadmin/cnaas_nms/lowlevel.py
@@ -34,6 +34,9 @@ def get_api(url, token):
     )
     api.add_resource(resource_name="devices", resource_class=DeviceResource)
     api.add_resource(resource_name="interfaces", resource_class=InterfaceResource)
+    api.add_resource(
+        resource_name="interface_status", resource_class=InterfaceStatusResource
+    )
     api.add_resource(resource_name="device_sync", resource_class=DeviceSyncResource)
     api.add_resource(resource_name="job")
 
@@ -44,7 +47,7 @@ class DeviceResource(Resource):
     """Defines operations on the devices endpoint"""
 
     actions = {
-        "retrieve": {"method": "GET", "url": "/devices?filter[management_ip]={}"}
+        "retrieve": {"method": "GET", "url": "/devices?filter[management_ip]={}"},
     }
 
 
@@ -60,6 +63,18 @@ class InterfaceResource(Resource):
     }
 
 
+class InterfaceStatusResource(Resource):
+    """Defines operations on the interface_status sub-resource of a device"""
+
+    actions = {
+        "list": {"method": "GET", "url": "/device/{}/interface_status"},
+        "update": {
+            "method": "PUT",
+            "url": "/device/{}/interface_status",
+        },
+    }
+
+
 class DeviceSyncResource(Resource):
     """Defines the API syncto operations"""
 

From c45928ad6a079b31abe9a176de54611f7a9a1dda Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Thu, 15 Apr 2021 14:33:07 +0200
Subject: [PATCH 12/16] Implement interface cycling through CNaaS-NMS

Because:
- CNaaS-NMS supports this operation internally, so we do not need
  to use the inherited generic method from ManagementHandler.
---
 python/nav/portadmin/cnaas_nms/proxy.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
index cc3c308956..33875288c4 100644
--- a/python/nav/portadmin/cnaas_nms/proxy.py
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -13,6 +13,8 @@
 # details.  You should have received a copy of the GNU General Public License
 # along with NAV. If not, see <http://www.gnu.org/licenses/>.
 #
+from typing import Sequence
+
 from nav.models import manage
 from nav.portadmin.config import CONFIG
 from nav.portadmin.cnaas_nms.lowlevel import get_api, ClientError
@@ -51,6 +53,18 @@ def _set_interface_enabled(self, interface: manage.Interface, enabled=True):
         payload = {"interfaces": {interface.ifdescr: data}}
         self._api.interfaces.configure(self.netbox.sysname, body=payload)
 
+    def _bounce_interfaces(
+        self,
+        interfaces: Sequence[manage.Interface],
+        wait: float = 5.0,
+        commit: bool = False,
+    ):
+        """Offloads the entire bounce operation to CNaaS NMS. CNaaS NMS doesn't need
+        or care about the wait and commit arguments, so these are flatly ignored.
+        """
+        payload = {"bounce_interfaces": [ifc.ifdescr for ifc in interfaces]}
+        self._api.interface_status.update(self.netbox.sysname, body=payload)
+
     def commit_configuration(self):
         payload = {"hostname": self.netbox.sysname, "dry_run": False, "auto_push": True}
         self._api.device_sync.syncto(body=payload)

From 91da97b0487c74cdabca3f9b0a123b78c33cc267 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Fri, 16 Apr 2021 11:08:38 +0200
Subject: [PATCH 13/16] Add config option to disable SSL verification

Because:
- The test server runs on a self-signed certificate, which makes the
  underlying HTTP library raise an exception.
- We need to be able to disable SSL verification for testing purposes,
  (but we should never need this option in production!)
---
 python/nav/portadmin/cnaas_nms/lowlevel.py |  3 ++-
 python/nav/portadmin/cnaas_nms/proxy.py    |  2 +-
 python/nav/portadmin/config.py             | 12 +++++++++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/python/nav/portadmin/cnaas_nms/lowlevel.py b/python/nav/portadmin/cnaas_nms/lowlevel.py
index 613783c9ce..0cada4bc38 100644
--- a/python/nav/portadmin/cnaas_nms/lowlevel.py
+++ b/python/nav/portadmin/cnaas_nms/lowlevel.py
@@ -19,7 +19,7 @@
 from simple_rest_client.exceptions import ClientError
 
 
-def get_api(url, token):
+def get_api(url, token, ssl_verify=None):
     """Returns a CNaaS NMS API instance from a URL and a token"""
     default_headers = {
         "Authorization": "Bearer {}".format(token),
@@ -31,6 +31,7 @@ def get_api(url, token):
         timeout=2,
         append_slash=False,
         json_encode_body=True,
+        ssl_verify=ssl_verify,
     )
     api.add_resource(resource_name="devices", resource_class=DeviceResource)
     api.add_resource(resource_name="interfaces", resource_class=InterfaceResource)
diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
index 33875288c4..d9588f2aef 100644
--- a/python/nav/portadmin/cnaas_nms/proxy.py
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -35,7 +35,7 @@ class CNaaSNMSMixIn(ManagementHandler):
     def __init__(self, netbox: manage.Netbox, **kwargs):
         super().__init__(netbox, **kwargs)
         config = CONFIG.get_cnaas_nms_config()
-        self._api = get_api(config.url, config.token)
+        self._api = get_api(config.url, config.token, config.ssl_verify)
 
     def set_interface_description(self, interface: manage.Interface, description: str):
         data = {"description": interface.ifalias}
diff --git a/python/nav/portadmin/config.py b/python/nav/portadmin/config.py
index 9ffee3c715..affa1fdbdc 100644
--- a/python/nav/portadmin/config.py
+++ b/python/nav/portadmin/config.py
@@ -20,7 +20,7 @@
 from nav.config import NAVConfigParser, ConfigurationError
 from nav.portadmin.vlan import FantasyVlan
 
-CNaaSNMSConfig = namedtuple("CNaasNMSConfig", ["url", "token"])
+CNaaSNMSConfig = namedtuple("CNaasNMSConfig", ["url", "token", "ssl_verify"])
 
 
 class PortAdminConfig(NAVConfigParser):
@@ -121,7 +121,11 @@ def is_cnaas_nms_enabled(self):
         return self.getboolean("cnaas-nms", "enabled", fallback=False)
 
     def get_cnaas_nms_config(
-        self, section="cnaas-nms", url_option="url", token_option="token"
+        self,
+        section="cnaas-nms",
+        url_option="url",
+        token_option="token",
+        ssl_verify=None,
     ):
         """Returns a CNaaSNMSConfig namedtuple if a CNaaS-NMS proxy is enabled"""
         if not self.has_option(section, url_option):
@@ -130,7 +134,9 @@ def get_cnaas_nms_config(
             raise ConfigurationError("Missing CNaaS-NMS API token in configuration")
 
         return CNaaSNMSConfig(
-            self.get(section, url_option), self.get(section, token_option)
+            self.get(section, url_option),
+            self.get(section, token_option),
+            self.getboolean(section, "ssl_verify", fallback=ssl_verify),
         )
 
 

From 673db7cdf0850a1d2546d6514d36acab92b9e591 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Fri, 16 Apr 2021 12:16:19 +0200
Subject: [PATCH 14/16] Use correct device name in all API requests

Because:
- NAV's device sysname does not necessarily correspond to the hostname
  used by CNaaS-NMS, depending on the DNS setup.
- The best way to find the corresponding device in CNaaS-NMS is to
  look it up using its management IP.
- The code to look up the device already exists, so we re-use it to
  provide a device_name property that can be used in all generated
  API requests.
---
 python/nav/portadmin/cnaas_nms/proxy.py | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
index d9588f2aef..64bed0322c 100644
--- a/python/nav/portadmin/cnaas_nms/proxy.py
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -36,11 +36,12 @@ def __init__(self, netbox: manage.Netbox, **kwargs):
         super().__init__(netbox, **kwargs)
         config = CONFIG.get_cnaas_nms_config()
         self._api = get_api(config.url, config.token, config.ssl_verify)
+        self._device = None
 
     def set_interface_description(self, interface: manage.Interface, description: str):
         data = {"description": interface.ifalias}
         payload = {"interfaces": {interface.ifdescr: data}}
-        self._api.interfaces.configure(self.netbox.sysname, body=payload)
+        self._api.interfaces.configure(self.device_name, body=payload)
 
     def set_interface_down(self, interface: manage.Interface):
         self._set_interface_enabled(interface, enabled=False)
@@ -51,7 +52,7 @@ def set_interface_up(self, interface: manage.Interface):
     def _set_interface_enabled(self, interface: manage.Interface, enabled=True):
         data = {"enabled": enabled}
         payload = {"interfaces": {interface.ifdescr: data}}
-        self._api.interfaces.configure(self.netbox.sysname, body=payload)
+        self._api.interfaces.configure(self.device_name, body=payload)
 
     def _bounce_interfaces(
         self,
@@ -63,10 +64,10 @@ def _bounce_interfaces(
         or care about the wait and commit arguments, so these are flatly ignored.
         """
         payload = {"bounce_interfaces": [ifc.ifdescr for ifc in interfaces]}
-        self._api.interface_status.update(self.netbox.sysname, body=payload)
+        self._api.interface_status.update(self.device_name, body=payload)
 
     def commit_configuration(self):
-        payload = {"hostname": self.netbox.sysname, "dry_run": False, "auto_push": True}
+        payload = {"hostname": self.device_name, "dry_run": False, "auto_push": True}
         self._api.device_sync.syncto(body=payload)
         # TODO: Get a job number from the syncto call
         # TODO: Poll the job API for "status": "FINISHED"
@@ -107,6 +108,16 @@ def raise_on_unmatched_criteria(self, device_record):
                 "changes at the moment. Please try againt later."
             )
 
+    @property
+    def device_name(self) -> str:
+        """This returns the name used for this device in CNaaS NMS. It does not
+        necessarily correspond to the sysname NAV got from DNS, but is necessary to
+        construct most API operations against the device.
+        """
+        if not self._device:
+            self._device = self._get_device_record()
+        return self._device.get("hostname")
+
     def _get_device_record(self):
         response = self._api.devices.retrieve(self.netbox.ip)
         payload = response.body

From 93e0274be366e0554de2b0d94d98644d02f2412e Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Fri, 16 Apr 2021 13:51:39 +0200
Subject: [PATCH 15/16] Properly define the job API endpoints

---
 python/nav/portadmin/cnaas_nms/lowlevel.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/python/nav/portadmin/cnaas_nms/lowlevel.py b/python/nav/portadmin/cnaas_nms/lowlevel.py
index 0cada4bc38..604dce99be 100644
--- a/python/nav/portadmin/cnaas_nms/lowlevel.py
+++ b/python/nav/portadmin/cnaas_nms/lowlevel.py
@@ -39,7 +39,7 @@ def get_api(url, token, ssl_verify=None):
         resource_name="interface_status", resource_class=InterfaceStatusResource
     )
     api.add_resource(resource_name="device_sync", resource_class=DeviceSyncResource)
-    api.add_resource(resource_name="job")
+    api.add_resource(resource_name="jobs", resource_class=JobResource)
 
     return api
 
@@ -80,3 +80,12 @@ class DeviceSyncResource(Resource):
     """Defines the API syncto operations"""
 
     actions = {"syncto": {"method": "POST", "url": "/device_syncto"}}
+
+
+class JobResource(Resource):
+    """Defines operations on the job/jobs endpoints"""
+
+    actions = {
+        "list": {"method": "GET", "url": "/jobs"},
+        "retrieve": {"method": "GET", "url": "/job/{}"},
+    }

From 830a7bdff776020ddee2825fa0f1536e08ef7450 Mon Sep 17 00:00:00 2001
From: Morten Brekkevold <morten.brekkevold@uninett.no>
Date: Fri, 16 Apr 2021 13:54:40 +0200
Subject: [PATCH 16/16] Wait for configuration commit to complete

Because:
- We need to make sure the commit finishes without error before
  reporting success or failure to the end user.
---
 python/nav/portadmin/cnaas_nms/proxy.py | 53 +++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/python/nav/portadmin/cnaas_nms/proxy.py b/python/nav/portadmin/cnaas_nms/proxy.py
index 64bed0322c..a66bebec87 100644
--- a/python/nav/portadmin/cnaas_nms/proxy.py
+++ b/python/nav/portadmin/cnaas_nms/proxy.py
@@ -13,7 +13,9 @@
 # details.  You should have received a copy of the GNU General Public License
 # along with NAV. If not, see <http://www.gnu.org/licenses/>.
 #
+import time
 from typing import Sequence
+import logging
 
 from nav.models import manage
 from nav.portadmin.config import CONFIG
@@ -24,6 +26,8 @@
     ProtocolError,
 )
 
+_logger = logging.getLogger(__name__)
+
 
 class CNaaSNMSMixIn(ManagementHandler):
     """MixIn to override all write-operations from
@@ -67,10 +71,53 @@ def _bounce_interfaces(
         self._api.interface_status.update(self.device_name, body=payload)
 
     def commit_configuration(self):
+        job_id = self._device_syncto()
+        job = self._poll_job_until_finished(job_id)
+        self._raise_on_job_failure(job)
+
+    def _device_syncto(self) -> int:
+        """Runs the CNaaS-NMS device_syncto operation and returns its job number"""
         payload = {"hostname": self.device_name, "dry_run": False, "auto_push": True}
-        self._api.device_sync.syncto(body=payload)
-        # TODO: Get a job number from the syncto call
-        # TODO: Poll the job API for "status": "FINISHED"
+        response = self._api.device_sync.syncto(body=payload)
+        assert response.body.get("status") == "success"
+        job_id = response.body.get("job_id")
+        message = response.body.get("data")
+        _logger.debug(
+            "%s device_syncto response (job_id: %s): %s",
+            self.device_name,
+            job_id,
+            message,
+        )
+        return job_id
+
+    def _poll_job_until_finished(self, job_id: int, retry_delay: float = 1.0) -> dict:
+        job = None
+        status = "SCHEDULED"
+        while status.upper() in ("SCHEDULED", "RUNNING"):
+            time.sleep(retry_delay)
+            job = self._get_job(job_id)
+            status = job.get("status")
+            _logger.debug(
+                "%s polled job status for job_id=%s: %s",
+                self.device_name,
+                job_id,
+                status,
+            )
+        return job
+
+    def _get_job(self, job_id: int) -> dict:
+        response = self._api.jobs.retrieve(job_id)
+        return response.body.get("data").get("jobs")[0]
+
+    def _raise_on_job_failure(self, job: dict):
+        if job.get("status") != "FINISHED":
+            message = job.get("result", {}).get("message")
+            if not message:
+                message = "Unknown error from CNaaS-NMS job %s, please see logs".format(
+                    job.get("id")
+                )
+                _logger.error("%s device_syncto job failed: %r", job)
+            raise ProtocolError(message)
 
     def raise_if_not_configurable(self):
         """Raises an exception if this device cannot be configured by CNaaS-NMS for