Fix auth_protocol and priv_protocol for SNMP v3

SNMP driver was using the wrong dictionary key to retrieve auth_protocol
and priv_protocol from driver info.  As a result, the SNMP client was
created with empty strings for both those fields.  Any nodes configured
to use SNMP v3 with those fields failed because the SNMP driver was
unable to perform power related operations due to authentication error.

- Use correct keys for snmp auth_protocol and priv_protocol when
  creating SNMP client
- Sanitize snmp auth_key and priv_key in API results

Story: 2010613
Task: 47535

Change-Id: I5efd3c9f79a021f1a8e613c3d13b6596a7972672

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 d56652b1e..45ef4d26f 100644
--- a/ironic/tests/unit/api/controllers/v1/test_node.py
+++ b/ironic/tests/unit/api/controllers/v1/test_node.py
@@ -844,6 +844,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.