On Fri, 19 Feb 2016, Petr Vobornik wrote:
On 02/19/2016 11:12 AM, Alexander Bokovoy wrote:
On Fri, 19 Feb 2016, Martin Basti wrote:
WIP patch attached
https://fedorahosted.org/freeipa/ticket/5665
Comments inline.
+ # we need to run sidgen task
+ sidgen_task_dn = DN("cn=sidgen,cn=ipa-sidgen-task,cn=tasks,"
+ "cn=config")
+ sidgen_tasks_attr = {
+ "objectclass": ["top", "extensibleObject"],
+ "cn": ["sidgen"],
+ "delay": [0],
+ "nsslapd-basedn": [self.api.env.basedn],
+ }
May be you are better to name this task more uniquely?
Something like 'cn=generate domain sid,cn=...'?
+
+ task_entry = ldap.make_entry(sidgen_task_dn,
+ **sidgen_tasks_attr)
+ try:
+ ldap.add_entry(task_entry)
+ except errors.DuplicateEntry:
+ self.log.debug("sidgen task already created")
+ else:
+ self.log.debug("sidgen task has been created")
There could be multiple tasks running in parallel, that's why it could
be good to use a different and unique name.
+ # we have to check all trusts domains which have been added
after the
+ # upgrade that caused bug was done.
+
+ base_dn = DN(self.api.env.container_adtrusts,
self.api.env.basedn)
+ trust_domain_entries, truncated = ldap.find_entries(
+ base_dn=base_dn,
+ scope=ldap.SCOPE_ONELEVEL,
+ attrs_list=[attr_name, "cn"],
+ )
+
+ if truncated:
+ self.log.warning("update_sids: Search results were
truncated")
+
+ for entry in trust_domain_entries:
+ if entry.single_value[attr_name] is None:
+ domain = entry.single_value["cn"]
+ self.log.error(
+ "Your trust to %s is broken. Please re-create it
by "
+ "running 'ipa trust-add' again", domain)
+
+ sysupgrade.set_upgrade_state('sidgen', 'update_sids', False)
+ return False, ()
This part looks fine. Basically, a similar check needs to be added to
trust_find, trust_show, and may be trust_add.
Why trust-add?
I'm not a big fan of cluttering existing commands(find, show, mod)
with logic to fix one upgrade bug. But I understand a need to
communicate it somehow.
Would it make sense to move such logic to a separate command, e.g.
trust-check/trust-verify? The command can do additional check in a
future.
No. What is the value of trust-add if it says you 'Trust established and
verified' while in reality it didn't? This specific issue is very easy
to catch.
--
/ Alexander Bokovoy
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
