[Freeipa-devel] [PATCH 0011] Move freeipa certmonger helpers to libexecdir.

Tue Feb 23 13:33:22 UTC 2016

On 23/02/16 10:14, Martin Kosek wrote:
> On 02/23/2016 09:47 AM, David Kupka wrote:
>> On 22/02/16 16:15, Martin Kosek wrote:
>>> On 02/22/2016 04:04 PM, Jan Cholasta wrote:
>>>> On 22.2.2016 15:56, David Kupka wrote:
>>>>> On 22/02/16 07:28, Jan Cholasta wrote:
>>>>>> On 18.2.2016 10:10, David Kupka wrote:
>>>>>>> On 19/01/16 16:10, David Kupka wrote:
>>>>>>>> On 19/01/16 14:38, Jan Cholasta wrote:
>>>>>>>>> On 19.1.2016 14:26, Martin Kosek wrote:
>>>>>>>>>> On 01/19/2016 01:47 PM, David Kupka wrote:
>>>>>>>>>>> I've polished the patch attached to #5586 by Timo Aaltonen.
>>>>>>>>>>>
>>>>>>>>>>> Thanks for the patch. I've fixed the path in specfile and removed
>>>>>>>>>>> unused import
>>>>>>>>>>> but otherwise it works, ACK.
>>>>>>>>>>>
>>>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5586
>>>>>>>>>>
>>>>>>>>>> Won't this break existing certmonger requests depending on the old
>>>>>>>>>> path?
>>>>>>>>>
>>>>>>>>> It will, I don't see any upgrade code.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> # getcert list | grep '/usr/lib64/ipa/certmonger'
>>>>>>>>>>       pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert
>>>>>>>>>> "auditSigningCert
>>>>>>>>>> cert-pki-ca"
>>>>>>>>>>       pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert
>>>>>>>>>> "ocspSigningCert
>>>>>>>>>> cert-pki-ca"
>>>>>>>>>>       pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert
>>>>>>>>>> "subsystemCert
>>>>>>>>>> cert-pki-ca"
>>>>>>>>>>       pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert
>>>>>>>>>> "caSigningCert
>>>>>>>>>> cert-pki-ca"
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/renew_ra_cert
>>>>>>>>>>       pre-save command: /usr/lib64/ipa/certmonger/stop_pkicad
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/renew_ca_cert
>>>>>>>>>> "Server-Cert
>>>>>>>>>> cert-pki-ca"
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/restart_dirsrv
>>>>>>>>>> RHEL72
>>>>>>>>>>       post-save command: /usr/lib64/ipa/certmonger/restart_httpd
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>> You're right it will break the upgrade. I haven't noticed that
>>>>>>>> Server-Cert for DS and HTTPD are not handled by
>>>>>>>> certificate_renewal_update (ipaserver.install.server.upgrade) where all
>>>>>>>> the other trackings are stopped and then configured again with the
>>>>>>>> paths.CERTMONGER_COMMAND_TEMPLATE already updated.
>>>>>>>>
>>>>>>>> Thanks for the catch.
>>>>>>>>
>>>>>>>
>>>>>>> I've updated Timo's patch little more and added
>>>>>>> start_tracking_certificates() for dsinstance and httpinstance. Now the
>>>>>>> upgrade works as expected.
>>>>>>
>>>>>> The way the patches are split is kind of weird and apparently confusing
>>>>>> (see the other thread). IMO there should be 2 patches: the first should
>>>>>> add the ability to change DS and HTTP certmonger config during upgrade
>>>>>> (i.e. the start_tracking_certificates() methods and
>>>>>> certificate_renewal_update() changes), the second should move the
>>>>>> helpers (i.e. the actual move and certificate_renewal_update() version
>>>>>> bump).
>>>>>>
>>>>> Honza, do I understand it correctly that the code is OK but I did not
>>>>> split it to the patches correctly?
>>>>
>>>> Yes.
>>>
>>> Before acking or pushing, can you please explain for me how the upgrade of
>>> certmonger tracking requests work? I want to make sure this is right, so please
>>> bear with me:
>>>
>>> 1) How does it edit existing tracking requests with the new helper paths?
>>>
>>> 2) Does it go and try to edit the requests on every upgrade? Or is there some
>>> check that requests were updated?
>>>
>>> Thanks,
>>> Martin
>>>
>>
>> Whole upgrade of renewal requests is done in
>> ipaserver/install/server/upgrade.py in certificate_renewal_upgrade().
>>
>> First there is version of requests and if it's the same as in state file
>> upgrade is skipped.
>> Then every request is searched over certmonger's DBus interface and if at least
>> one is not found it means that there was change in request configuration. All
>> tracking requests are then stopped and started again with new configuration.
>>
>> So to answer you questions:
>> 1) By stopping the old request with the old parameters (including path) and
>> starting new with new parameters.
>>
>> 2) Only if version was bumped which happens only if some of the requests changes.
>
> Ah, so IIUC, if you bump the version, requests should be properly updated. The
> change looks fine then.
>

After discussion with Honza, we decided to drop the part comparing only 
base names of pre- and post-save commands and use it as whole. I've also 
split the patches so it's obvious what is going on.

Patches should be applied in this order:

freeipa-dkupka-0091.0
freeipa-dkupka-0087.1
freeipa-dkupka-0088.1
freeipa-tjaalton-0011.2
freeipa-dkupka-0092.0

-- 
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0087.1-dsinstance-add-start_tracking_certificates-method.patch
Type: text/x-patch
Size: 4240 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160223/01695ef1/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0088.1-httpinstance-add-start_tracking_certificates-method.patch
Type: text/x-patch
Size: 3767 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160223/01695ef1/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0091.0-upgrade-Match-whole-pre-post-command-not-just-basena.patch
Type: text/x-patch
Size: 4115 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160223/01695ef1/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0092.0-upgrade-Bump-certificate-tracking-requests-version.patch
Type: text/x-patch
Size: 866 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160223/01695ef1/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-tjaalton-0011.2-Move-freeipa-certmonger-helpers-to-libexecdir.patch
Type: text/x-patch
Size: 5557 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160223/01695ef1/attachment-0004.bin>