Merge "Fix auth_protocol and priv_protocol for SNMP v3"

5 files changed, 112 insertions, 2 deletions

diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py
index d21b075c8..5bf0a4981 100644
--- a/ironic/api/controllers/v1/node.py
+++ b/ironic/api/controllers/v1/node.py
@@ -1600,6 +1600,10 @@ def node_sanitize(node, fields, cdict=None,
             node['driver_info'] = strutils.mask_dict_password(
                 node['driver_info'], "******")
 
+            _mask_fields(node['driver_info'],
+                         ['snmp_auth_key', 'snmp_priv_key'],
+                         "******")
+
     if not show_instance_secrets and 'instance_info' in node_keys:
         node['instance_info'] = strutils.mask_dict_password(
             node['instance_info'], "******")
@@ -1663,6 +1667,12 @@ def _node_sanitize_extended(node, node_keys, target_dict, cdict):
                        'driver_internal_info permission. **'}
 
 
+def _mask_fields(dictionary, fields, secret):
+    for field in fields:
+        if dictionary.get(field):
+            dictionary[field] = secret
+
+
 def node_list_convert_with_links(nodes, limit, url, fields=None, **kwargs):
     cdict = api.request.context.to_policy_values()
     target_dict = dict(cdict)
diff --git a/ironic/drivers/modules/snmp.py b/ironic/drivers/modules/snmp.py
index d544d5687..435d24b78 100644
--- a/ironic/drivers/modules/snmp.py
+++ b/ironic/drivers/modules/snmp.py
@@ -392,9 +392,9 @@ def _get_client(snmp_info):
                       snmp_info.get("read_community"),
                       snmp_info.get("write_community"),
                       snmp_info.get("user"),
-                      snmp_info.get("auth_proto"),
+                      snmp_info.get("auth_protocol"),
                       snmp_info.get("auth_key"),
-                      snmp_info.get("priv_proto"),
+                      snmp_info.get("priv_protocol"),
                       snmp_info.get("priv_key"),
                       snmp_info.get("context_engine_id"),
                       snmp_info.get("context_name"))
diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py
index 27061737d..6f2c1fc97 100644
--- a/ironic/tests/unit/api/controllers/v1/test_node.py
+++ b/ironic/tests/unit/api/controllers/v1/test_node.py
@@ -843,6 +843,64 @@ class TestListNodes(test_api_base.BaseApiTest):
         self.assertIn('retired_reason', data['nodes'][0])
         self.assertIn('network_data', data['nodes'][0])
 
+    def test_detail_snmpv3(self):
+        driver_info = {
+            'snmp_version': 3,
+            'snmp_user': 'test-user',
+            'snmp_auth_protocol': 'sha',
+            'snmp_auth_key': 'test-auth-key',
+            'snmp_priv_protocol': 'aes',
+            'snmp_priv_key': 'test-priv-key'
+        }
+        sanitized_driver_info = driver_info.copy()
+        sanitized_driver_info['snmp_auth_key'] = '******'
+        sanitized_driver_info['snmp_priv_key'] = '******'
+
+        node = obj_utils.create_test_node(self.context,
+                                          chassis_id=self.chassis.id,
+                                          driver_info=driver_info)
+        data = self.get_json(
+            '/nodes/detail',
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertEqual(node.uuid, data['nodes'][0]["uuid"])
+        self.assertIn('name', data['nodes'][0])
+        self.assertIn('driver', data['nodes'][0])
+        self.assertIn('driver_info', data['nodes'][0])
+        self.assertEqual(sanitized_driver_info,
+                         data['nodes'][0]['driver_info'])
+        self.assertIn('extra', data['nodes'][0])
+        self.assertIn('properties', data['nodes'][0])
+        self.assertIn('chassis_uuid', data['nodes'][0])
+        self.assertIn('reservation', data['nodes'][0])
+        self.assertIn('maintenance', data['nodes'][0])
+        self.assertIn('console_enabled', data['nodes'][0])
+        self.assertIn('target_power_state', data['nodes'][0])
+        self.assertIn('target_provision_state', data['nodes'][0])
+        self.assertIn('provision_updated_at', data['nodes'][0])
+        self.assertIn('inspection_finished_at', data['nodes'][0])
+        self.assertIn('inspection_started_at', data['nodes'][0])
+        self.assertIn('raid_config', data['nodes'][0])
+        self.assertIn('target_raid_config', data['nodes'][0])
+        self.assertIn('network_interface', data['nodes'][0])
+        self.assertIn('resource_class', data['nodes'][0])
+        for field in api_utils.V31_FIELDS:
+            self.assertIn(field, data['nodes'][0])
+        self.assertIn('storage_interface', data['nodes'][0])
+        self.assertIn('traits', data['nodes'][0])
+        self.assertIn('conductor_group', data['nodes'][0])
+        self.assertIn('automated_clean', data['nodes'][0])
+        self.assertIn('protected', data['nodes'][0])
+        self.assertIn('protected_reason', data['nodes'][0])
+        self.assertIn('owner', data['nodes'][0])
+        self.assertIn('lessee', data['nodes'][0])
+        # never expose the chassis_id
+        self.assertNotIn('chassis_id', data['nodes'][0])
+        self.assertNotIn('allocation_id', data['nodes'][0])
+        self.assertIn('allocation_uuid', data['nodes'][0])
+        self.assertIn('retired', data['nodes'][0])
+        self.assertIn('retired_reason', data['nodes'][0])
+        self.assertIn('network_data', data['nodes'][0])
+
     def test_detail_instance_uuid(self):
         instance_uuid = '6eccd391-961c-4da5-b3c5-e2fa5cfbbd9d'
         node = obj_utils.create_test_node(
diff --git a/ironic/tests/unit/drivers/modules/test_snmp.py b/ironic/tests/unit/drivers/modules/test_snmp.py
index 5391d7ac5..e1b6fc1df 100644
--- a/ironic/tests/unit/drivers/modules/test_snmp.py
+++ b/ironic/tests/unit/drivers/modules/test_snmp.py
@@ -46,6 +46,41 @@ class SNMPClientTestCase(base.TestCase):
         self.value = 'value'
 
     @mock.patch.object(pysnmp, 'SnmpEngine', autospec=True)
+    def test__get_client(self, mock_snmpengine):
+        driver_info = db_utils.get_test_snmp_info(
+            snmp_address=self.address,
+            snmp_port=self.port,
+            snmp_user='test-user',
+            snmp_auth_protocol='sha',
+            snmp_auth_key='test-auth-key',
+            snmp_priv_protocol='aes',
+            snmp_priv_key='test-priv-key',
+            snmp_context_engine_id='test-engine-id',
+            snmp_context_name='test-context-name',
+            snmp_version='3')
+        node = obj_utils.get_test_node(
+            self.context,
+            driver_info=driver_info)
+        info = snmp._parse_driver_info(node)
+
+        client = snmp._get_client(info)
+
+        mock_snmpengine.assert_called_once_with()
+        self.assertEqual(self.address, client.address)
+        self.assertEqual(int(self.port), client.port)
+        self.assertEqual(snmp.SNMP_V3, client.version)
+        self.assertNotIn('read_community', client.__dict__)
+        self.assertNotIn('write_community', client.__dict__)
+        self.assertEqual('test-user', client.user)
+        self.assertEqual(pysnmp.usmHMACSHAAuthProtocol, client.auth_proto)
+        self.assertEqual('test-auth-key', client.auth_key)
+        self.assertEqual(pysnmp.usmAesCfb128Protocol, client.priv_proto)
+        self.assertEqual('test-priv-key', client.priv_key)
+        self.assertEqual('test-engine-id', client.context_engine_id)
+        self.assertEqual('test-context-name', client.context_name)
+        self.assertEqual(mock_snmpengine.return_value, client.snmp_engine)
+
+    @mock.patch.object(pysnmp, 'SnmpEngine', autospec=True)
     def test___init__(self, mock_snmpengine):
         client = snmp.SNMPClient(self.address, self.port, snmp.SNMP_V1)
         mock_snmpengine.assert_called_once_with()
diff --git a/releasenotes/notes/bug-2010613-3ab1f32aaa776f28.yaml b/releasenotes/notes/bug-2010613-3ab1f32aaa776f28.yaml
new file mode 100644
index 000000000..89769bfb8
--- /dev/null
+++ b/releasenotes/notes/bug-2010613-3ab1f32aaa776f28.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    [`bug 2010613 <https://storyboard.openstack.org/#!/story/2010613>`_]
+    Fixes issue with SNMP v3 auth protocol and priv protocol set in
+    driver info not being retrieved correctly when a SNMP client is
+    initialized.