From 3cf0d831def3be54dd6b326a79ec1f5e1a677df5 Mon Sep 17 00:00:00 2001 From: Brian Harring Date: Thu, 24 Jan 2019 14:40:06 -0800 Subject: [PATCH 1/2] Add info logging for any svcmgr TakeAction's triggered. While we're still tracing a bug involving a CA renewal constantly triggering actions for a spec, there is no info logging for an action being taken- thus add some so it's easier to debug operational failures. --- mgr/manager.go | 1 + 1 file changed, 1 insertion(+) diff --git a/mgr/manager.go b/mgr/manager.go index a1bccfbd..217bc693 100644 --- a/mgr/manager.go +++ b/mgr/manager.go @@ -29,6 +29,7 @@ type CertServiceManager struct { } func (csm *CertServiceManager) TakeAction(change_type string) error { + log.Infof("manager: executing configured action due to change type %s for %s", change_type, csm.Cert.Path) ca_path := "" if csm.CA.File != nil { ca_path = csm.CA.File.Path From 8f8c98ae593957474d9a235354aa0a080a11e448 Mon Sep 17 00:00:00 2001 From: Brian Harring Date: Thu, 24 Jan 2019 16:24:37 -0800 Subject: [PATCH 2/2] Fix invalid action triggering when CA has changed but we don't write it to disk. In 1f1c919d083d12d4fcf, multiple CA bugs were fixed- mostly issues where certmgr was triggering an action for a spec because it thought the CA had changed (but it had not). That commit moved the updating of the internal notion of the CA into the file serialization method; the problem is that that method was only invoked if the CA was being serialized to disk. If it wasn't being serialized to disk, if the CA changed it would result in certmgr triggering the action every time it checked that particular spec. Without this commit, the only work around is to restart certmgr itself. This bug affected v1.6.1, v1.6.2, and v1.6.3. --- cert/cert.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cert/cert.go b/cert/cert.go index daa9c7da..9fda9f1f 100644 --- a/cert/cert.go +++ b/cert/cert.go @@ -80,8 +80,6 @@ func (ca *CA) writeCert(cert []byte) error { } log.Infof("cert: wrote CA certificate: %s", ca.File.Path) - // see CA.Load(); strip prefix/trailing whitespace to ensure our bytes.Equal() checks don't false positive. - ca.pem = []byte(strings.TrimSpace(string(cert[:]))) err = ca.File.Set() return err } @@ -132,6 +130,11 @@ func (ca *CA) Refresh() (bool, error) { if ca.File != nil { err = ca.writeCert(cert) } + // If there were no errors, update our internal notion of what the CA is. + if err != nil { + // see CA.Load(); strip prefix/trailing whitespace to ensure our bytes.Equal() checks don't false positive. + ca.pem = []byte(strings.TrimSpace(string(cert[:]))) + } return true, err }