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

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)

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"))

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(

ironic/tests/unit/drivers/modules/test_snmp.py

@@ -45,6 +45,41 @@ class SNMPClientTestCase(base.TestCase):
         self.oid = (1, 3, 6, 1, 1, 1, 0)
         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)

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.