diff --git a/unit_tests/charm_tests/test_utils.py b/unit_tests/charm_tests/test_utils.py index ffb7f83..c1e8166 100644 --- a/unit_tests/charm_tests/test_utils.py +++ b/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): diff --git a/zaza/openstack/charm_tests/keystone/tests.py b/zaza/openstack/charm_tests/keystone/tests.py index 9abe78c..fb0c2f3 100644 --- a/zaza/openstack/charm_tests/keystone/tests.py +++ b/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") diff --git a/zaza/openstack/charm_tests/test_utils.py b/zaza/openstack/charm_tests/test_utils.py index 493a868..14ee0f3 100644 --- a/zaza/openstack/charm_tests/test_utils.py +++ b/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 '