[Freeipa-devel] [PATCH 0116] Refactoring of service autobind

[Martin Basti]

On 25/09/14 14:47, Jan Cholasta wrote:
> Dne 25.9.2014 v 10:51 Martin Basti napsal(a):
>> On 19/09/14 14:30, Jan Cholasta wrote:
>>> Dne 19.9.2014 v 13:32 Martin Basti napsal(a):
>>>> On 01/09/14 16:26, Martin Basti wrote:
>>>>> On 28/08/14 14:01, Jan Cholasta wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Dne 27.8.2014 v 15:22 Martin Basti napsal(a):
>>>>>>> Patch attached.
>>>>>>>
>>>>>>
>>>>>> 1) Please rename object_exists to entry_exists.
>>>>>>
>>>>>>
>>>>>> 2) Use empty attribute list in get_entry() in
>>>>>> object_exists/entry_exists.
>>>>>>
>>>>>>
>>>>>> 3) Please update LDAPObject.get_dn_if_exists() to use
>>>>>> object_exists/entry_exists.
>>>>>>
>>>>>>
>>>>>> 4) I'm not a fan of how do_bind() is laid out, IMHO something like
>>>>>> this would be better (untested):
>>>>>>
>>>>>> +    def do_bind(self, dm_password=None, autobind=AUTOBIND_AUTO,
>>>>>> timeout=DEFAULT_TIMEOUT):
>>>>>> +        if dm_password:
>>>>>> +            self.do_simple_bind(bindpw=dm_password, 
>>>>>> timeout=timeout)
>>>>>> +            return
>>>>>> +
>>>>>> +        if autobind != AUTOBIND_DISABLED and os.getegid() == 0 and
>>>>>> self.ldapi:
>>>>>> +            try:
>>>>>> +                # autobind
>>>>>> +                pw_name = pwd.getpwuid(os.geteuid()).pw_name
>>>>>> +                self.do_external_bind(pw_name, timeout=timeout)
>>>>>> +                return
>>>>>> +            except errors.NotFound:
>>>>>> +                if autobind == AUTOBIND_ENABLED:
>>>>>> +                    # autobind was required and failed, raise
>>>>>> +                    # exception that it failed
>>>>>> +                    raise
>>>>>> +
>>>>>> +        # Fall back
>>>>>> +        self.do_sasl_gssapi_bind(timeout=timeout)
>>>>>>
>>>>>>
>>>>>> Honza
>>>>>>
>>>>> 3) skipped as we discuss on IRC
>>>>>
>>>>> Updated patch attached
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Freeipa-devel mailing list
>>>>> Freeipa-devel at redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>> Please review, this should be in 4.1
>>>
>>> 1) The patch need a rebase on top of current ipa-4-1.
>> I can apply it (Am I doing something wrong?)
>>>
>>>
>>> 2) You can remove import pwd from service.py, it is no longer used 
>>> there.
>>>
>>>
>>> 3) Are named constants for the autobind argument the right thing to
>>> do? It is a tri-state which can be expressed with None/True/False.
>>> (I'm just asking, I don't have a strong opinion on this.)
>>>
>> As we discussed on IRC, using None/True/False, is not good approach.
>> Updated patch attached
>>
>
> ACK, but the patch still does not apply cleanly on ipa-4-1:
>
> $ git apply 
> freeipa-mbasti-0116.3-Refactoring-of-autobind-object_exists.patch
> error: patch failed: ipaserver/install/service.py:20
> error: ipaserver/install/service.py: patch does not apply
>
Rebased patches attached

-- 
Martin Basti

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0116.3-ipa-4-1-Refactoring-of-autobind-object_exists.patch
Type: text/x-patch
Size: 9800 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140925/44d6063f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mbasti-0116.3-Refactoring-of-autobind-object_exists.patch
Type: text/x-patch
Size: 9784 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140925/44d6063f/attachment-0001.bin>