[Freeipa-devel] [PATCH 0065] Use private ccache in ipa-server-install

[Tomas Babej]

On 06/04/2013 06:09 PM, Petr Viktorin wrote:
> On 06/04/2013 01:29 PM, Tomas Babej wrote:
>> On 06/03/2013 02:58 PM, Martin Kosek wrote:
>>> On 06/03/2013 02:43 PM, Tomas Babej wrote:
>>>> Hi,
>>>>
>>>> this patch fixes the installation problems on master on F19 with krb5
>>>> packages
>>>>> = 1.11.2-6
>>>> https://fedorahosted.org/freeipa/ticket/3666
>>>>
>>>> Tomas
>>> 1) Leaving cache_desc open:
>>>
>>> +        (cache_desc, cache_path) = tempfile.mkstemp(prefix='krbcc')
>>> +        os.environ['KRB5CCNAME'] = cache_path
>>>
>>> Why do we keep the descriptor open and close it at the and of the
>>> installation?
>>> Can we close it right after tempfile.mkstemp? I think we do it this
>>> way in
>>> other places in installation.
>>>
>>> 2) What about other installers where we handle Kerberos auth, like
>>> ipa-{replica,dns,ca}-install?
>>>
>>> A common function, other shared means, of handling KRB5CCNAME may be
>>> appropriate to avoid duplicating code too much.
>>>
>>> Martin
>> I moved the code responsible to PrivateCCache class, both for
>> readability and conciseness.
>>
>> Private ccache now used in replica,dns and ca the installers. I managed
>> to reproduce the error only with
>> dns-install though(fails on adding the service principal), but having a
>> private ccache for the installer should not hurt.
>>
>> Ipa-adtrust-install requires the admin ticket, so there shouldn't be an
>> issue.
>>
>> Tomas
>
> Shouldn't the context manager restore os.environ['KRB5CCNAME'] on exit?

There's no need to, since the value of the environment variable is 
inherited only into child processes (pkispawn, etc.). Hence the KRB5CCNAME
environment variable is not available outside the install script.

[root at vm-002 labtool]# ipa-server-install
[snip]
Be sure to back up the CA certificate stored in /root/cacert.p12
This file is required to create replicas. The password for this
file is the Directory Manager password

[root at vm-002 labtool]# echo $KRB5CCNAME

[root at vm-002 labtool]#
>
>
> Two nitpicks:
>
> ipaserver/install/installutils.py: new blank line at EOF
>
> For most context managers I prefer @contextlib.contextmanager rather 
> than writing out the class, it makes them shorter and easier to read 

Addressed in the updated patch.

Tomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tbabej-0065-3-Use-private-ccache-in-ipa-server-install.patch
Type: text/x-patch
Size: 6390 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130605/93766aac/attachment.bin>