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

[Martin Kosek]

On 09/25/2014 03:06 PM, Martin Basti wrote:
> 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

Pushed to:
master: 29ba9d9d26b92498902d40d71adae193308b5c92
ipa-4-1: 8e0f8bc7ad8e91bcf9e30e3cc8159838977348e6

Martin