[Freeipa-devel] [PATCHES] 295-299 Allow changing chaining of the IPA CA certificate
Rob Crittenden
rcritten at redhat.com
Wed Jul 30 12:47:18 UTC 2014
Jan Cholasta wrote:
> 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.
ACK
rob
More information about the Freeipa-devel
mailing list