Update Keystone LDAP tests

- Reversed ldap-config-flags vs charm configs precedence
  for https://review.opendev.org/748364
- Added new functional test to validate the precedence
- Added zaza helper function to assist handling charm configs
  that do not accept "" (like booleans and ints), and reverting
  to unset values through config_change()
- Fixed zaza reset helper function that apparently never
  worked ({}.keys() returns dict_keys which is not a sequence)

unit_tests/charm_tests/test_utils.py

@@ -97,7 +97,7 @@ class TestBaseCharmTest(ut_utils.BaseTestCase):
             self.set_application_config.reset_mock()
         self.assertFalse(self.set_application_config.called)
         self.reset_application_config.assert_called_once_with(
-            'anApp', alterna_config.keys(), model_name='aModel')
+            'anApp', list(alterna_config.keys()), model_name='aModel')
         self.wait_for_application_states.assert_has_calls([
             mock.call(model_name='aModel', states={}),
             mock.call(model_name='aModel', states={}),
@@ -125,6 +125,66 @@ class TestBaseCharmTest(ut_utils.BaseTestCase):
         self.assertFalse(self.wait_for_agent_status.called)
         self.assertFalse(self.wait_for_application_states.called)
 
+    def test_separate_non_string_config(self):
+        intended_cfg_keys = ['foo2', 'foo3', 'foo4', 'foo5']
+        current_config_mock = {
+            'foo2': None,
+            'foo3': 'old_bar3',
+            'foo4': None,
+            'foo5': 'old_bar5',
+        }
+        self.patch_target('config_current')
+        self.config_current.return_value = current_config_mock
+        non_string_type_keys = ['foo2', 'foo3', 'foo4']
+        expected_result_filtered = {
+            'foo3': 'old_bar3',
+            'foo5': 'old_bar5',
+        }
+        expected_result_special = {
+            'foo2': None,
+            'foo4': None,
+        }
+        current, non_string = (
+            self.target.config_current_separate_non_string_type_keys(
+                non_string_type_keys, intended_cfg_keys, 'application_name')
+        )
+
+        self.assertEqual(expected_result_filtered, current)
+        self.assertEqual(expected_result_special, non_string)
+
+        self.config_current.assert_called_once_with(
+            'application_name', intended_cfg_keys)
+
+    def test_separate_special_config_None_params(self):
+        current_config_mock = {
+            'foo1': 'old_bar1',
+            'foo2': None,
+            'foo3': 'old_bar3',
+            'foo4': None,
+            'foo5': 'old_bar5',
+        }
+        self.patch_target('config_current')
+        self.config_current.return_value = current_config_mock
+        non_string_type_keys = ['foo2', 'foo3', 'foo4']
+        expected_result_filtered = {
+            'foo1': 'old_bar1',
+            'foo3': 'old_bar3',
+            'foo5': 'old_bar5',
+        }
+        expected_result_special = {
+            'foo2': None,
+            'foo4': None,
+        }
+        current, non_string = (
+            self.target.config_current_separate_non_string_type_keys(
+                non_string_type_keys)
+        )
+
+        self.assertEqual(expected_result_filtered, current)
+        self.assertEqual(expected_result_special, non_string)
+
+        self.config_current.assert_called_once_with(None, None)
+
 
 class TestOpenStackBaseTest(ut_utils.BaseTestCase):
 

zaza/openstack/charm_tests/keystone/tests.py

@@ -380,7 +380,12 @@ class SecurityTests(BaseKeystoneTest):
 
 
 class LdapTests(BaseKeystoneTest):
-    """Keystone ldap tests tests."""
+    """Keystone ldap tests."""
+
+    non_string_type_keys = ('ldap-user-enabled-mask',
+                            'ldap-user-enabled-invert',
+                            'ldap-group-members-are-ids',
+                            'ldap-use-pool')
 
     @classmethod
     def setUpClass(cls):
@@ -434,31 +439,44 @@ class LdapTests(BaseKeystoneTest):
     def test_100_keystone_ldap_users(self):
         """Validate basic functionality of keystone API with ldap."""
         application_name = 'keystone-ldap'
-        config = self._get_ldap_config()
+        intended_cfg = self._get_ldap_config()
+        current_cfg, non_string_cfg = (
+            self.config_current_separate_non_string_type_keys(
+                self.non_string_type_keys, intended_cfg, application_name)
+        )
 
         with self.config_change(
-                self.config_current(application_name, config.keys()),
-                config,
-                application_name=application_name):
-            logging.info(
-                'Waiting for users to become available in keystone...'
-            )
-            test_config = lifecycle_utils.get_charm_config(fatal=False)
-            zaza.model.wait_for_application_states(
-                states=test_config.get("target_deploy_status", {})
-            )
+                {},
+                non_string_cfg,
+                application_name=application_name,
+                reset_to_charm_default=True):
+            with self.config_change(
+                    current_cfg,
+                    intended_cfg,
+                    application_name=application_name):
+                logging.info(
+                    'Waiting for users to become available in keystone...'
+                )
+                test_config = lifecycle_utils.get_charm_config(fatal=False)
+                zaza.model.wait_for_application_states(
+                    states=test_config.get("target_deploy_status", {})
+                )
 
-            with self.v3_keystone_preferred():
-                # NOTE(jamespage): Test fixture should have johndoe and janedoe
-                #                  accounts
-                johndoe = self._find_keystone_v3_user('john doe', 'userdomain')
-                self.assertIsNotNone(johndoe, "user 'john doe' was unknown")
-                janedoe = self._find_keystone_v3_user('jane doe', 'userdomain')
-                self.assertIsNotNone(janedoe, "user 'jane doe' was unknown")
+                with self.v3_keystone_preferred():
+                    # NOTE(jamespage): Test fixture should have
+                    #                  johndoe and janedoe accounts
+                    johndoe = self._find_keystone_v3_user(
+                        'john doe', 'userdomain')
+                    self.assertIsNotNone(
+                        johndoe, "user 'john doe' was unknown")
+                    janedoe = self._find_keystone_v3_user(
+                        'jane doe', 'userdomain')
+                    self.assertIsNotNone(
+                        janedoe, "user 'jane doe' was unknown")
 
 
 class LdapExplicitCharmConfigTests(LdapTests):
-    """Keystone ldap tests tests."""
+    """Keystone ldap tests."""
 
     def _get_ldap_config(self):
         """Generate ldap config for current model.
@@ -484,9 +502,61 @@ class LdapExplicitCharmConfigTests(LdapTests):
             'ldap-user-enabled-mask': 0,
             'ldap-user-enabled-default': 'True',
             'ldap-group-tree-dn': 'ou=groups',
-            'ldap-group-objectclass': 'groupOfNames',
+            'ldap-group-objectclass': '',
             'ldap-group-id-attribute': 'cn',
             'ldap-group-member-attribute': 'memberUid',
             'ldap-group-members-are-ids': True,
-            'ldap-config-flags': '{group_objectclass: "posixGroup"}',
+            'ldap-config-flags': '{group_objectclass: "posixGroup",'
+                                 ' use_pool: True,'
+                                 ' group_tree_dn: "group_tree_dn_foobar"}',
         }
+
+    def test_200_config_flags_precedence(self):
+        """Validates precedence when the same config options are used."""
+        application_name = 'keystone-ldap'
+        intended_cfg = self._get_ldap_config()
+        current_cfg, non_string_cfg = (
+            self.config_current_separate_non_string_type_keys(
+                self.non_string_type_keys, intended_cfg, application_name)
+        )
+
+        with self.config_change(
+                {},
+                non_string_cfg,
+                application_name=application_name,
+                reset_to_charm_default=True):
+            with self.config_change(
+                    current_cfg,
+                    intended_cfg,
+                    application_name=application_name):
+                logging.info(
+                    'Performing LDAP settings validation in keystone.conf...'
+                )
+                test_config = lifecycle_utils.get_charm_config(fatal=False)
+                zaza.model.wait_for_application_states(
+                    states=test_config.get("target_deploy_status", {})
+                )
+                units = zaza.model.get_units("keystone-ldap",
+                                             model_name=self.model_name)
+                result = zaza.model.run_on_unit(
+                    units[0].name,
+                    "cat /etc/keystone/domains/keystone.userdomain.conf")
+                # not present in charm config, but present in config flags
+                self.assertIn("use_pool = True", result['stdout'],
+                              "use_pool value is expected to be present and "
+                              "set to True in the config file")
+                # ldap-config-flags overriding empty charm config value
+                self.assertIn("group_objectclass = posixGroup",
+                              result['stdout'],
+                              "group_objectclass is expected to be present and"
+                              " set to posixGroup in the config file")
+                # overridden by charm config, not written to file
+                self.assertNotIn(
+                    "group_tree_dn_foobar",
+                    result['stdout'],
+                    "user_tree_dn ldap-config-flags value needs to be "
+                    "overridden by ldap-user-tree-dn in config file")
+                # complementing the above, value used is from charm setting
+                self.assertIn("group_tree_dn = ou=groups", result['stdout'],
+                              "user_tree_dn value is expected to be present "
+                              "and set to dc=test,dc=com in the config file")

zaza/openstack/charm_tests/test_utils.py

@@ -154,6 +154,46 @@ class BaseCharmTest(unittest.TestCase):
             model_name=cls.model_name)
         logging.debug('Leader unit is {}'.format(cls.lead_unit))
 
+    def config_current_separate_non_string_type_keys(
+            self, non_string_type_keys, config_keys=None,
+            application_name=None):
+        """Obtain current config and the non-string type config separately.
+
+        If the charm config option is not string, it will not accept being
+        reverted back in "config_change()" method if the current value is None.
+        Therefore, obtain the current config and separate those out, so they
+        can be used for a separate invocation of "config_change()" with
+        reset_to_charm_default set to True.
+
+        :param config_keys: iterable of strs to index into the current config.
+                            If None, return all keys from the config
+        :type config_keys:  Optional[Iterable[str]]
+        :param non_string_type_keys: list of non-string type keys to be
+                                     separated out only if their current value
+                                     is None
+        :type non_string_type_keys: list
+        :param application_name: String application name for use when called
+                                 by a charm under test other than the object's
+                                 application.
+        :type application_name:  Optional[str]
+        :return: Dictionary of current charm configs without the
+                 non-string type keys provided, and dictionary of the
+                 non-string keys found in the supplied config_keys list.
+        :rtype: Dict[str, Any], Dict[str, None]
+        """
+        current_config = self.config_current(application_name, config_keys)
+        non_string_type_config = {}
+        if config_keys is None:
+            config_keys = list(current_config.keys())
+        for key in config_keys:
+            # We only care if the current value is None, otherwise it will
+            # not face issues being reverted by "config_change()"
+            if key in non_string_type_keys and current_config[key] is None:
+                non_string_type_config[key] = None
+                current_config.pop(key)
+
+        return current_config, non_string_type_config
+
     def config_current(self, application_name=None, keys=None):
         """Get Current Config of an application normalized into key-values.
 
@@ -275,7 +315,7 @@ class BaseCharmTest(unittest.TestCase):
                           'charm default: "{}"'
                           .format(alternate_config.keys()))
             model.reset_application_config(application_name,
-                                           alternate_config.keys(),
+                                           list(alternate_config.keys()),
                                            model_name=self.model_name)
         elif default_config == alternate_config:
             logging.debug('default_config == alternate_config, not attempting '