[Freeipa-devel] [PATCH 0075] Change group ownership of CRL publish directory

[Martin Kosek]

On 06/21/2013 02:11 PM, Tomas Babej wrote:
> On 06/20/2013 06:00 PM, Simo Sorce wrote:
>> On Thu, 2013-06-20 at 17:47 +0200, Martin Kosek wrote:
>>> On 06/20/2013 05:44 PM, Simo Sorce wrote:
>>>> On Thu, 2013-06-20 at 17:33 +0200, Martin Kosek wrote:
>>>>> On 06/20/2013 05:15 PM, Tomas Babej wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Spec file modified so that /var/lib/ipa/pki-ca/publish/ is owned
>>>>>> by pkiuser group.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/3727
>>>>>>
>>>>>> Tomas
>>>>>>
>>>>> NACK. This won't fly. pkiuser is created by FreeIPA when server is installed,
>>>>> thus you cannot just simply change ownership in our spec file because in the
>>>>> time when package is installed or updated, pkiuser may not exist.
>>>>>
>>>>> I think you need to delete the %attr from spec file and set the correct
>>>>> ownership during ipa-{server,ca}-install. When CA is configured, we should
>>>>> also
>>>>> probably let ipa-upgradeconfig check this directory and amend when necessary
>>>>> (to fix affected IPA CA instances).
>>>> Probably even better to not create the directory via rpm at all, but
>>>> make ipa-ca-install create it and remove it when --uninstall is run.
>>>>
>>>> Simo.
>>> This could also work, sure. Could we then at least mark this directory in our
>>> spec file as %ghost? So that "rpm -qf /var/lib/ipa/pki-ca/publish/" gives some
>>> information?
>> I guess so.
>>
>> Simo.
>>
> 
> Updated version attached.
> 
> Tomas

Looks good by reading (I did not test it), maybe just one nitpick:

+                root_logger.warning("Error while removing CRL publish "
+                                    "directory: %s" % str(e))

This should read:
+                root_logger.warning("Error while removing CRL publish "
+                                    "directory: %s", e)

We do not need to format the string before it is really logged and we also do
not need to convert it to "str" as we already requested the conversion to
string by "%s".

Martin