[Freeipa-devel] [PATCHES] 295-299 Allow changing chaining of the IPA CA certificate

[Jan Cholasta]

Dne 29.7.2014 v 16:33 Rob Crittenden napsal(a):
> Rob Crittenden wrote:
>> Jan Cholasta wrote:
>>> Dne 28.7.2014 v 21:39 Rob Crittenden napsal(a):
>>>> This is oh-so close. AFAICT it generally does what it should, I think it
>>>> is ready for a wider audience. Just a few more things:
>>>>
>>>> 306: A while True loop is used for something which AFAICT can only ever
>>>> execute once. I'd think something like this is more readable:
>>>>
>>>> for ca_nick, ca_flags in db.list_certs():
>>>>       if db.has_nickname(ca_cert):
>>>>           try:
>>>>               db.delete_cert(ca_nick)
>>>>           except ipautil.CalledProcessError:
>>>>               syslog.syslog(
>>>>                   syslog.LOG_ERR,
>>>>                   "Failed to remove certificate %s" % ca_nick)
>>>
>>> Actually the while loop is necessary, because certutil -D (and in turn
>>> CertDB.delete_cert) deletes just a single cert with the nickname, but
>>> there may be more versions of it and we need to delete them all.
>>
>> Aha, excellent point. This would be a great comment in the code!
>>
>>>>
>>>> +1 on the additional syslogs. It will help figure out what's going on if
>>>> things go sideways.
>>>>
>>>> Otherwise things seem to be working. I think that fixing the above is
>>>> enough for a +57 with the promise of unit tests to back up some of these
>>>> new functions.
>>>
>>> I'm working on that.
>>>
>>>>
>>>> rob
>>>> rob
>>>>
>>>
>>> I have made a slight adjustment to patch 246 because of
>>> <https://fedorahosted.org/freeipa/ticket/4039>, see
>>> <http://www.redhat.com/archives/freeipa-devel/2014-July/msg00369.html>.
>>>
>>> Updated rebased patches attached.
>>>
>>> (once again, the correct order to apply them is 241-253, 262-294,
>>> 303-305, 295-299, 306-307)
>>>
>>
>> ACK on 246.
>>
>> IMHO this is ready to be pushed if you can add the comment on 306.

Added. Updated patch attached.

>>
>> A slight rebase on 251 is needed for freeipa.spec.in.
>
> Oh, or maybe not since you sent the whole shebang. I just did an
> interdiff of the current and old 246.

Yep, I remember fixing this particular merge conflict.

>
> rob
>


-- 
Jan Cholasta
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-jcholast-306.2-Update-external-CA-cert-in-Dogtag-NSS-DB-on-IPA-CA-c.patch
Type: text/x-patch
Size: 4278 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140730/9231dcd5/attachment.bin>