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

[David Kupka]

On 23/02/16 16:41, Rob Crittenden wrote:
> David Kupka wrote:
>> 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
>
> A cert could silently fail to be tracked in
> start_tracking_certificates() if no serverid can be found.

In that case it also wouldn't be stopped. The behavior is the same as in 
existing stop_tracking_certificates(). Should we rather raise and stop 
the upgrade? I guess not but warning would be probably useful. What 
solution would you prefer, Rob?

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


-- 
David Kupka