On 25.2.2016 14:13, David Kupka wrote:
On 24/02/16 15:07, Rob Crittenden wrote:
David Kupka wrote:
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?
I don't know all the callers of this. It may be perfectly safe to assume
that a serverid is always there, but the implication if it isn't is that
some tracking cert won't be updated properly right? That potentially
could mean no renewal.
So the consequences could be severe, I just don't know the likelihood.
In other words, a comment (# can never get here) might be perfectly
adequate.
rob
freeipa-dkupka-0087.1
freeipa-dkupka-0088.1
freeipa-tjaalton-0011.2
freeipa-dkupka-0092.0
Currently the function is called only from one place (also added in this
patch) but to avoid problems in the future I made the serverid parameter
mandatory.
Also I squashed the version bump into Timo's patch.
Updated patches attached. Apply in this order:
freeipa-dkupka-0091.1
freeipa-dkupka-0087.2
freeipa-dkupka-0088.2
freeipa-tjaalton-0011.3
Works for me, ACK.
Pushed to:
master: 872d5903d0d278914d740575b4ef92fa75c44a45
ipa-4-3: 8231f870e6adff2463a6911acd0191f1efbc0335
--
Jan Cholasta
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
