[Freeipa-devel] [PATCHES] 152-158 ipa-server-certinstall fixes

[Petr Viktorin]

On 08/19/2013 03:50 PM, Jan Cholasta wrote:
> On 19.8.2013 14:02, Petr Viktorin wrote:
>> Thanks!
>> I've read the patches and have some initial comments; I'll get to
>> functional testing (and writing related CA-less tests) right away.
>>
>> The patches need a small rebase (attached since I did it anyway).
>>
>> Patch 152: OK (I saw some issues but they're fixed later on)
>> Patch 153: You can use log_file_name = '/var/log/ipa/default.log' on the
>> ServerCertInstall class to keep the default log file.
>
> What is the benefit in doing this? All ipa-server-certinstall did when
> using this file was complain about /var/log/ipa being non-existent.

Ah, okay. If it was a deliberate change, please mention it in the commit 
message.

>> Patch 154: OK
>> Patch 155: All this is removed by patch 157, please squash them together.
>> Patch 156: OK
>> Patch 157: Please add the delete_cert method to the NSSDatabase class,
>> and have CertDB call it (see e.g. run_certutil, find_server_certs,
>> import_pkcs12). The CertDB is only meant for IPA-specific functionality.
>> Patch 158: OK


The usage looks a bit strange to me. Having the --dirsrv_pin and 
--http_pin options doesn't make sense if there's only one certificate. 
Should we add a --pin option, and make these deprecated aliases of it? 
Or make the -d and -w options take individual arguments (which would be 
backwards incompatible)?
Also, it should be possible to enter the pin(s) and DM password 
interactively.

-- 
Petr³