-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 12/08/2009 11:27 PM, Jakub Hrozek wrote: > On 12/09/2009 04:57 AM, Stephen Gallagher wrote: >> Patch 0001: Adds SSSDDomain.set_name() function to change the name of a >> domain. > > > This patch also resets the verbosity of TestRunner, it would perhaps be > better to split it into a separate patch.
Done, see attached. > >> Patch 0002: Fixes the broken SSSDConfig.set() command. Previously it had >> a net zero effect when attempting to overwrite an existing option. Now >> it will properly replace it. > > > ACK > > Sorry for doing such a stupid bug. > >> Patch 0003: Properly handle activation and deactivation of configured >> domains. Creates two public functions for doing this at the SSSDConfig >> level (rather than having to open a domain and call set_active(), then >> save it again) and uses them in the save_domain() function to keep the >> active domain list in sync. > > > ACK - -- Stephen Gallagher RHCE 804006346421761 Delivering value year after year. Red Hat ranks #1 in value among software vendors. http://www.redhat.com/promo/vendor/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/ iEYEARECAAYFAksfJ8MACgkQeiVVYja6o6P3EgCgrBzBfb40aEwEhMUesXBIHYUT QvUAn0uLmgqgg2lIFr3kT06Ae6uzxJH1 =YNMG -----END PGP SIGNATURE-----
From bfe6fc5f60da90e198477df57a340cef1639b50e Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Tue, 8 Dec 2009 23:27:52 -0500 Subject: [PATCH 1/4] Add SSSDDomain.set_name() function to SSSDConfig API This function will change the name of an existing domain --- server/config/SSSDConfig.py | 44 ++++++++++++++++++++++++++++++++++++-- server/config/SSSDConfigTest.py | 36 +++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/server/config/SSSDConfig.py b/server/config/SSSDConfig.py index 2abafe1..3b8c128 100644 --- a/server/config/SSSDConfig.py +++ b/server/config/SSSDConfig.py @@ -743,6 +743,29 @@ class SSSDDomain(SSSDConfigObject): else: self.options[option] = value + def set_name(self, newname): + """ + Change the name of the domain + + newname: + New name for this domain + + === Returns === + No return value. + + === Errors === + TypeError: + newname was not a string + """ + + if type(newname) != str: + raise TypeError + + if not self.oldname: + # Only set the oldname once + self.oldname = self.name + self.name = newname + def add_provider(self, provider, provider_type): """ Add a new provider type to the domain @@ -819,14 +842,14 @@ class SSSDDomain(SSSDConfigObject): # Remove any unused options when removing the provider. options = self.list_provider_options(provider, provider_type) - + # Trim any options that are used by other providers, # if that provider is in use for (prov, ptype) in self.providers: # Ignore the one being removed if (prov, ptype) == (provider, provider_type): continue - + provider_options = self.list_provider_options(prov, ptype) overlap = options_overlap(options.keys(), provider_options.keys()) for opt in overlap: @@ -1286,6 +1309,18 @@ class SSSDConfig(SSSDChangeConf): raise TypeError name = domain.get_name() + + oldindex = None + if domain.oldname and domain.oldname != name: + # We are renaming this domain + # Remove the old section + oldindex = self.delete_option('section', 'domain/%s' % + domain.oldname) + + # Reset the oldname, in case we're not done with + # this domain object. + domain.oldname = None; + sectionname = 'domain/%s' % name # Ensure that the existing section is removed # This way we ensure that we are getting a @@ -1300,7 +1335,10 @@ class SSSDConfig(SSSDChangeConf): addkw.append( { 'type' : 'option', 'name' : option, 'value' : str(value) } ) - self.add_section(sectionname, addkw, index) + if oldindex: + self.add_section(sectionname, addkw, oldindex) + else: + self.add_section(sectionname, addkw, index) if domain.active: if domain.get_name not in self.list_active_domains(): diff --git a/server/config/SSSDConfigTest.py b/server/config/SSSDConfigTest.py index 3d8b596..41cd270 100644 --- a/server/config/SSSDConfigTest.py +++ b/server/config/SSSDConfigTest.py @@ -780,6 +780,23 @@ class SSSDConfigTestSSSDDomain(unittest.TestCase): domain.remove_option('nosuchoption') self.assertFalse('nosuchoption' in domain.get_all_options().keys()) + def testSetName(self): + domain = SSSDConfig.SSSDDomain('sssd', self.schema) + + # Positive test - Change the name once + domain.set_name('sssd2'); + self.assertEqual(domain.get_name(), 'sssd2') + self.assertEqual(domain.oldname, 'sssd') + + # Positive test - Change the name a second time + domain.set_name('sssd3') + self.assertEqual(domain.get_name(), 'sssd3') + self.assertEqual(domain.oldname, 'sssd') + + # Negative test - try setting the name to a non-string + self.assertRaises(TypeError, + domain.set_name, 4) + class SSSDConfigTestSSSDConfig(unittest.TestCase): def setUp(self): pass @@ -1122,9 +1139,11 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase): self.assertTrue('IPA' in sssdconfig.list_domains()) self.assertTrue('IPA' in sssdconfig.list_active_domains()) + self.assertTrue(sssdconfig.has_section('domain/IPA')) sssdconfig.delete_domain('IPA') self.assertFalse('IPA' in sssdconfig.list_domains()) self.assertFalse('IPA' in sssdconfig.list_active_domains()) + self.assertFalse(sssdconfig.has_section('domain/IPA')) def testSaveDomain(self): sssdconfig = SSSDConfig.SSSDConfig("etc/sssd.api.conf", @@ -1148,6 +1167,23 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase): # Negative Test - Type Error self.assertRaises(TypeError, sssdconfig.save_domain, self) + # Positive test - Change the domain name and save it + domain.set_name('example.com2') + self.assertEqual(domain.name,'example.com2') + self.assertEqual(domain.oldname,'example.com') + sssdconfig.save_domain(domain) + + self.assertTrue('example.com2' in sssdconfig.list_domains()) + self.assertTrue('example.com2' in sssdconfig.list_active_domains()) + self.assertTrue(sssdconfig.has_section('domain/example.com2')) + self.assertEqual(sssdconfig.get('domain/example.com2', + 'ldap_uri'), + 'ldap://ldap.example.com') + self.assertFalse('example.com' in sssdconfig.list_domains()) + self.assertFalse('example.com' in sssdconfig.list_active_domains()) + self.assertFalse('example.com' in sssdconfig.list_inactive_domains()) + self.assertFalse(sssdconfig.has_section('domain/example.com')) + if __name__ == "__main__": error = 0 -- 1.6.5.2
From 7c707bd720e274ca1fff7698f5aaa1d3998788f4 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Tue, 8 Dec 2009 23:28:12 -0500 Subject: [PATCH 2/4] Reduce the verbosity of the SSSDConfigTest Now it will report only failures or final success --- server/config/SSSDConfigTest.py | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/server/config/SSSDConfigTest.py b/server/config/SSSDConfigTest.py index 41cd270..1f8c4f8 100644 --- a/server/config/SSSDConfigTest.py +++ b/server/config/SSSDConfigTest.py @@ -1188,22 +1188,22 @@ if __name__ == "__main__": error = 0 suite = unittest.TestLoader().loadTestsFromTestCase(SSSDConfigTestSSSDService) - res = unittest.TextTestRunner(verbosity=99).run(suite) + res = unittest.TextTestRunner().run(suite) if not res.wasSuccessful(): error |= 0x1 suite = unittest.TestLoader().loadTestsFromTestCase(SSSDConfigTestSSSDDomain) - res = unittest.TextTestRunner(verbosity=99).run(suite) + res = unittest.TextTestRunner().run(suite) if not res.wasSuccessful(): error |= 0x2 suite = unittest.TestLoader().loadTestsFromTestCase(SSSDConfigTestSSSDConfig) - res = unittest.TextTestRunner(verbosity=99).run(suite) + res = unittest.TextTestRunner().run(suite) if not res.wasSuccessful(): error |= 0x4 suite = unittest.TestLoader().loadTestsFromTestCase(SSSDConfigTestValid) - res = unittest.TextTestRunner(verbosity=99).run(suite) + res = unittest.TextTestRunner().run(suite) if not res.wasSuccessful(): error |= 0x8 -- 1.6.5.2
From 6e0cf24dffd2c489ccf6c2f54a5a578fac6515d3 Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Tue, 8 Dec 2009 22:34:55 -0500 Subject: [PATCH 3/4] Fix broken SSSDChangeConf.set() function The set function didn't do anything at all. It needed to use the ipachangeconf.merge() function to behave properly instead of mergeNew() --- server/config/ipachangeconf.py | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/server/config/ipachangeconf.py b/server/config/ipachangeconf.py index f06ab4c..ea73a9b 100644 --- a/server/config/ipachangeconf.py +++ b/server/config/ipachangeconf.py @@ -528,7 +528,7 @@ class SSSDChangeConf(IPAChangeConf): }], 'action': 'set', } - self.mergeNew(self.opts, [ modkw ]) + self.opts = self.merge(self.opts, [ modkw ]) def add_section(self, name, optkw, index=0): optkw.append({'type':'empty', 'value':'empty'}) -- 1.6.5.2
From 9a4d699fa00d7f9fa4708a88b4ca4bb84140f15b Mon Sep 17 00:00:00 2001 From: Stephen Gallagher <sgall...@redhat.com> Date: Tue, 8 Dec 2009 22:51:35 -0500 Subject: [PATCH 4/4] Fix SSSDConfig API bugs around [de-]activation of domains Adds two new public functions: SSSDConfig.activate_domain() SSSDConfig.deactivate_domain() These two functions are used during the save_domain() call to ensure that the active domain list is always kept up to date. --- server/config/SSSDConfig.py | 96 ++++++++++++++++++++++++++++++++++++--- server/config/SSSDConfigTest.py | 63 +++++++++++++++++++++++++ 2 files changed, 152 insertions(+), 7 deletions(-) diff --git a/server/config/SSSDConfig.py b/server/config/SSSDConfig.py index 3b8c128..3a9ab4b 100644 --- a/server/config/SSSDConfig.py +++ b/server/config/SSSDConfig.py @@ -1266,6 +1266,89 @@ class SSSDConfig(SSSDChangeConf): self.save_domain(domain) return domain + def activate_domain(self, name): + """ + Activate a configured domain + + name: + The name of the configured domain to activate + + === Returns === + No return value + + === Errors === + NotInitializedError: + This SSSDConfig object has not had import_config() or new_config() + run on it yet. + NoDomainError: + No domain by this name is configured + """ + + if not self.initialized: + raise NotInitializedError + + if name not in self.list_domains(): + raise NoDomainError + + item = self.get_option_index('sssd', 'domains')[1] + if not item: + self.set('sssd','domains', name) + return + + # Turn the items into a set of dictionary keys + # This guarantees uniqueness and makes it easy + # to add a new value + domain_dict = dict.fromkeys(striplist(item['value'].split(','))) + if domain_dict.has_key(''): + del domain_dict[''] + + # Add a new key for the domain being activated + domain_dict[name] = None + + # Write out the joined keys + self.set('sssd','domains', ", ".join(domain_dict.keys())) + + def deactivate_domain(self, name): + """ + Deactivate a configured domain + + name: + The name of the configured domain to deactivate + + === Returns === + No return value + + === Errors === + NotInitializedError: + This SSSDConfig object has not had import_config() or new_config() + run on it yet. + NoDomainError: + No domain by this name is configured + """ + + if not self.initialized: + raise NotInitializedError + + if name not in self.list_domains(): + raise NoDomainError + item = self.get_option_index('sssd', 'domains')[1] + if not item: + self.set('sssd','domains', '') + return + + # Turn the items into a set of dictionary keys + # This guarantees uniqueness and makes it easy + # to remove the one unwanted value. + domain_dict = dict.fromkeys(striplist(item['value'].split(','))) + if domain_dict.has_key(''): + del domain_dict[''] + + # Add a new key for the domain being activated + del domain_dict[name] + + # Write out the joined keys + self.set('sssd','domains', ", ".join(domain_dict.keys())) + def delete_domain(self, name): """ Remove a domain from the SSSDConfig object. This function will also @@ -1282,6 +1365,9 @@ class SSSDConfig(SSSDChangeConf): """ if not self.initialized: raise NotInitializedError + + # Remove the domain from the active domains list if applicable + self.deactivate_domain(name) self.delete_option('section', 'domain/%s' % name) def save_domain(self, domain): @@ -1341,10 +1427,6 @@ class SSSDConfig(SSSDChangeConf): self.add_section(sectionname, addkw, index) if domain.active: - if domain.get_name not in self.list_active_domains(): - # Add it to the list of active domains - item = self.get_option_index('sssd', 'domains')[1] - if item: - item['value'] += ", %s" % domain.get_name() - else: - self.set('sssd', 'domains', domain.get_name()) + self.activate_domain(name) + else: + self.deactivate_domain(name) diff --git a/server/config/SSSDConfigTest.py b/server/config/SSSDConfigTest.py index 1f8c4f8..973ef07 100644 --- a/server/config/SSSDConfigTest.py +++ b/server/config/SSSDConfigTest.py @@ -1183,6 +1183,69 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase): self.assertFalse('example.com' in sssdconfig.list_active_domains()) self.assertFalse('example.com' in sssdconfig.list_inactive_domains()) self.assertFalse(sssdconfig.has_section('domain/example.com')) + self.assertEquals(domain.oldname, None) + + # Positive test - Set the domain inactive and save it + activelist = sssdconfig.list_active_domains() + inactivelist = sssdconfig.list_inactive_domains() + + domain.set_active(False) + sssdconfig.save_domain(domain) + + self.assertFalse('example.com2' in sssdconfig.list_active_domains()) + self.assertTrue('example.com2' in sssdconfig.list_inactive_domains()) + + self.assertEquals(len(sssdconfig.list_active_domains()), + len(activelist)-1) + self.assertEquals(len(sssdconfig.list_inactive_domains()), + len(inactivelist)+1) + + # Positive test - Set the domain active and save it + activelist = sssdconfig.list_active_domains() + inactivelist = sssdconfig.list_inactive_domains() + domain.set_active(True) + sssdconfig.save_domain(domain) + + self.assertTrue('example.com2' in sssdconfig.list_active_domains()) + self.assertFalse('example.com2' in sssdconfig.list_inactive_domains()) + + self.assertEquals(len(sssdconfig.list_active_domains()), + len(activelist)+1) + self.assertEquals(len(sssdconfig.list_inactive_domains()), + len(inactivelist)-1) + + # Positive test - Set the domain inactive and save it + activelist = sssdconfig.list_active_domains() + inactivelist = sssdconfig.list_inactive_domains() + + sssdconfig.deactivate_domain(domain.get_name()) + + self.assertFalse('example.com2' in sssdconfig.list_active_domains()) + self.assertTrue('example.com2' in sssdconfig.list_inactive_domains()) + + self.assertEquals(len(sssdconfig.list_active_domains()), + len(activelist)-1) + self.assertEquals(len(sssdconfig.list_inactive_domains()), + len(inactivelist)+1) + + # Positive test - Set the domain active and save it + activelist = sssdconfig.list_active_domains() + inactivelist = sssdconfig.list_inactive_domains() + + sssdconfig.activate_domain(domain.get_name()) + + self.assertTrue('example.com2' in sssdconfig.list_active_domains()) + self.assertFalse('example.com2' in sssdconfig.list_inactive_domains()) + + self.assertEquals(len(sssdconfig.list_active_domains()), + len(activelist)+1) + self.assertEquals(len(sssdconfig.list_inactive_domains()), + len(inactivelist)-1) + + + def testActivateDomain(self): + sssdconfig = SSSDConfig.SSSDConfig("etc/sssd.api.conf", + "etc/sssd.api.d") if __name__ == "__main__": error = 0 -- 1.6.5.2
0001-Add-SSSDDomain.set_name-function-to-SSSDConfig-API.patch.sig
Description: PGP signature
0002-Reduce-the-verbosity-of-the-SSSDConfigTest.patch.sig
Description: PGP signature
0003-Fix-broken-SSSDChangeConf.set-function.patch.sig
Description: PGP signature
0004-Fix-SSSDConfig-API-bugs-around-de-activation-of-doma.patch.sig
Description: PGP signature
_______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://fedorahosted.org/mailman/listinfo/sssd-devel