[Freeipa-devel] [PATCHES] 172-196 Refactor certificate renewal code

[Petr Viktorin]

On 03/19/2014 02:33 PM, Jan Cholasta wrote:
> On 13.3.2014 13:41, Jan Cholasta wrote:
>> On 12.3.2014 19:59, Petr Viktorin wrote:
>>> Certmonger is not configured/started in CA-less installs.
>>
>> That's expected.
>>
>>>
>>> I tested fresh installs and upgrades; renewals work fine for me.
>>>
>>> 161-184 look OK
>>>
>>> 185: one more nitpick:
>>>      cert = entry['usercertificate'][0]
>>> Shouldn't that use entry.single_value?
>>
>> I did not feel like changing this, because this is used in the original
>> code and the userCertificate LDAP attribute is multi-value.

Could you add a comment saying we don't care which of the certificates 
is returned? For me `entry[...][0]` is a red flag, since the order 
usually stays the same but it's not guaranteed, so it can change in the 
worst moment. If nothing else we shouldn't be leaving it in the code as 
an example of ipaldap usage.

>>
>>>
>>> 186-189 look OK
>>>
>>> 190: Is
>>>      fqdn = entries[0].dn[1].value
>>>      return api.env.host == fqdn
>>> safe? Can they differ in case, for example?
>>
>> I guess so, will fix.
>>
>>>
>>> 191-196 look OK
>>>
>>>> Note that patches 178 & 179 were already pushed. Also, patch 190 was
>>>> changed to store information about which CA instance is master in LDAP.
>
> Updated patches attached.
>
> Note that I changed the path for CSR export to /var/lib/ipa/ca.csr to
> make it more SELinux-friendly (not in the policy yet, see
> <https://bugzilla.redhat.com/show_bug.cgi?id=1077689>).
>


-- 
Petr³