reverseproxy: notify cert change only in cron job

notifying this in ensureCertificate does not work if provider changed in the middle anyway.
might as well get them to be in sync in the cronjob.

this change also resulted in tls addon getting restarted non-stop if you change from wildcard
to non-wildcard since ensureCertificate notifies the change.

src/reverseproxy.js

@@ -158,8 +158,7 @@ function validateCertificate(subdomain, domain, certificate) {
 }
 
 async function notifyCertChange() {
-    // let other parts of code know about any cert changes. apptask can trigger a renewal, provider can change, for example
-    await mail.handleCertChanged();
+    await mail.checkCertificate();
     await shell.promises.sudo('notifyCertChange', [ RESTART_SERVICE_CMD, 'box' ], {}); // directory server
     const allApps = (await apps.list()).filter(app => app.runState !== apps.RSTATE_STOPPED);
     for (const app of allApps) {
@@ -216,7 +215,7 @@ async function setFallbackCertificate(domain, certificate) {
     if (!safe.fs.writeFileSync(path.join(paths.NGINX_CERT_DIR, `${domain}.host.key`), certificate.key)) throw new BoxError(BoxError.FS_ERROR, safe.error.message);
 
     await reload();
-    await notifyCertChange();
+    await notifyCertChange(); // if domain uses fallback certs, propagate immediately
 }
 
 async function restoreFallbackCertificates() {
@@ -285,39 +284,51 @@ async function getDirectoryServerCertificate() {
     return await getCertificate({ domain: settings.dashboardDomain(), fqdn: settings.dashboardFqdn(), certificate: null, type: apps.LOCATION_TYPE_DIRECTORY_SERVER });
 }
 
-// write if contents mismatch
+// write if contents mismatch (thus preserving mtime)
 function writeFileSync(filePath, data) {
     assert.strictEqual(typeof filePath, 'string');
     assert.strictEqual(typeof data, 'string');
 
     const curData = safe.fs.readFileSync(filePath, { encoding: 'utf8' });
-    if (curData === data) return;
+    if (curData === data) return false;
     if (!safe.fs.writeFileSync(filePath, data)) throw new BoxError(BoxError.FS_ERROR, safe.error.message);
+    return true;
 }
 
 async function setupTlsAddon(app) {
     assert.strictEqual(typeof app, 'object');
 
     const certificateDir = `${paths.PLATFORM_DATA_DIR}/tls/${app.id}`;
-
-    // clean up any certs of old locations
-    const filenames = safe.fs.readdirSync(certificateDir) || [];
-    for (const filename of filenames) {
-        safe.fs.unlinkSync(path.join(certificateDir, filename));
-    }
-
+    const contents = [];
     for (const location of getAppLocationsSync(app)) {
         const certificate = await getCertificate(location);
-        writeFileSync(`${certificateDir}/${location.fqdn}.cert`, certificate.cert);
-        writeFileSync(`${certificateDir}/${location.fqdn}.key`, certificate.key);
+        contents.push({ filename: `${location.fqdn}.cert`, data: certificate.cert });
+        contents.push({ filename: `${location.fqdn}.key`, data: certificate.key });
 
         if (location.type === apps.LOCATION_TYPE_PRIMARY) { // backward compat
-            writeFileSync(`${certificateDir}/tls_cert.pem`, certificate.cert);
-            writeFileSync(`${certificateDir}/tls_key.pem`, certificate.key);
+            contents.push({ filename: 'tls_cert.pem', data: certificate.cert });
+            contents.push({ filename: 'tls_key.pem', data: certificate.key });
         }
     }
 
-    await docker.restartContainer(app.id);
+    let changed = 0;
+    for (const content of contents) {
+        if (writeFileSync(`${certificateDir}/${content.filename}`, content.data)) ++changed;
+    }
+    debug(`setupTlsAddon: ${changed} files changed`);
+
+    // clean up any certs of old locations
+    const filenamesInUse = new Set(contents.map(c => c.filename));
+    const filenames = safe.fs.readdirSync(certificateDir) || [];
+    let removed = 0;
+    for (const filename of filenames) {
+        if (filenamesInUse.has(filename)) continue;
+        safe.fs.unlinkSync(path.join(certificateDir, filename));
+        ++removed;
+    }
+    debug(`setupTlsAddon: ${removed} files removed`);
+
+    if (changed || removed) await docker.restartContainer(app.id);
 }
 
 // writes latest certificate to disk and returns the path
@@ -402,8 +413,6 @@ async function ensureCertificate(location, auditSource) {
     const [error] = await safe(acme2.getCertificate(fqdn, domainObject));
     debug(`ensureCertificate: error: ${error ? error.message : 'null'}`);
 
-    if (!error) await notifyCertChange();
-
     await safe(eventlog.add(eventlog.ACTION_CERTIFICATE_NEW, auditSource, { domain: fqdn, errorMessage: error?.message || '' }));
 }
 
@@ -635,6 +644,8 @@ async function checkCerts(options, auditSource, progressCallback) {
         await writeDashboardConfig(settings.dashboardDomain());
         await notifyCertChange(); // this allows user to "rebuild" using UI just in case we crashed and went out of sync
         safe.fs.unlinkSync(paths.REVERSE_PROXY_REBUILD_FILE);
+    } else {
+        await notifyCertChange(); // propagate any cert changes to services
     }
 
     await cleanupCerts(locations, auditSource, progressCallback);