[Freeipa-devel] [PATCH 0416-0417] fix broken configuration of sidgen and extdom plugins
Martin Basti
mbasti at redhat.com
Mon Feb 22 18:15:46 UTC 2016
On 22.02.2016 17:05, Martin Basti wrote:
>
>
> On 19.02.2016 15:02, Alexander Bokovoy wrote:
>> 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.
>>
> Patch attached, patch with warning will land soon.
>
>
Updated patches attached
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160222/d0e956dc/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0416.2-upgrade-fix-config-of-sidgen-and-extdom-plugins.patch
Type: text/x-patch
Size: 10744 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160222/d0e956dc/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0417-trusts-use-ipaNTTrustPartner-attribute-to-detect-tru.patch
Type: text/x-patch
Size: 2650 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160222/d0e956dc/attachment-0001.bin>
More information about the Freeipa-devel
mailing list