reworked restart/update-cert path to always trigger update before res…

…tart (#270)

* reworked restart/update-cert path to always trigger update before restart

* update certs before build_layer

* lint

* some pr comments

* merge

src/charm.py

@@ -98,6 +98,10 @@
 CONFIG_PATH = "/etc/grafana/grafana-config.ini"
 PROVISIONING_PATH = "/etc/grafana/provisioning"
 DATASOURCES_PATH = "/etc/grafana/provisioning/datasources/datasources.yaml"
+GRAFANA_CRT_PATH = "/etc/grafana/grafana.crt"
+GRAFANA_KEY_PATH = "/etc/grafana/grafana.key"
+CA_CERT_PATH = Path("/usr/local/share/ca-certificates/cos-ca.crt")
+
 DATABASE = "database"
 PEER = "grafana"
 PORT = 3000
@@ -427,17 +431,19 @@ def _check_datasource_provisioning(self) -> bool:
             return True
         return False
 
-    def _configure(self) -> None:
+    def _configure(self, force_restart: bool = False) -> None:
         """Configure Grafana.
 
         Generate configuration files and check the sums against what is
         already stored in the charm. If either the base Grafana config
         or the datasource config differs, restart Grafana.
+
+        If ``force_restart``: restart grafana regardless.
         """
         if not self.containers["workload"].can_connect():
             return
         logger.debug("Handling grafana-k8s configuration change")
-        restart = False
+        restart = force_restart
 
         # Generate a new base config and see if it differs from what we have.
         # If it does, store it and signal that we should restart Grafana
@@ -779,6 +785,16 @@ def _on_pebble_ready(self, event) -> None:
                 "Cannot set workload version at this time: could not get Alertmanager version."
             )
 
+    def _cert_ready(self):
+        # Verify that the certificate and key are correctly configured
+        workload = self.containers["workload"]
+        return (
+            self.cert_handler.cert
+            and self.cert_handler.key
+            and workload.exists(GRAFANA_CRT_PATH)
+            and workload.exists(GRAFANA_KEY_PATH)
+        )
+
     def restart_grafana(self) -> None:
         """Restart the pebble container.
 
@@ -788,22 +804,22 @@ def restart_grafana(self) -> None:
 
         Note that Grafana does not support SIGHUP, so a full restart is needed.
         """
+        # Before building the layer, we update our certificates if tls is enabled.
+        # This is needed here to circumvent a code ordering issue that results in:
+        #   *api.HTTPServer run error: cert_file cannot be empty when using HTTPS
+        #   ERROR cannot start service: exited quickly with code 1
+        if self.cert_handler.enabled:
+            logger.debug("TLS enabled: updating certs")
+            self._update_cert()
+            # now that this is done, build_layer should include cert and key into the config and we'll
+            # be sure that the files are actually there before grafana is (re)started.
+
         layer = self._build_layer()
         try:
             self.containers["workload"].add_layer(self.name, layer, combine=True)
             if self.containers["workload"].get_service(self.name).is_running():
                 self.containers["workload"].stop(self.name)
 
-            # Before 'start', we force a cert update. This is needed here to circumvent a code
-            # ordering issue that results in:
-            #   *api.HTTPServer run error: cert_file cannot be empty when using HTTPS
-            #   ERROR cannot start service: exited quickly with code 1
-            if self.cert_handler.cert:
-                self._update_cert()
-
-            # If available, we collect all trusted certs from the receive-ca-cert relation
-            self._update_trusted_ca_certs()
-
             self.containers["workload"].start(self.name)
             logger.info("Restarted grafana-k8s")
 
@@ -816,6 +832,10 @@ def restart_grafana(self) -> None:
                     make_dirs=True,
                 )
 
+                # If available, we collect all trusted certs from the receive-ca-cert relation
+                # we do this here, downstream from a container readiness check
+                self._update_trusted_ca_certs()
+
                 pragma = self.containers["workload"].exec(
                     [
                         "/usr/local/bin/sqlite3",
@@ -898,13 +918,11 @@ def _build_layer(self) -> Layer:
             }
         )
 
-        if self.cert_handler.cert and self.containers["workload"].exists(
-            "/etc/grafana/grafana.crt"
-        ):
+        if self._cert_ready():
             extra_info.update(
                 {
-                    "GF_SERVER_CERT_KEY": "/etc/grafana/grafana.key",
-                    "GF_SERVER_CERT_FILE": "/etc/grafana/grafana.crt",
+                    "GF_SERVER_CERT_KEY": GRAFANA_KEY_PATH,
+                    "GF_SERVER_CERT_FILE": GRAFANA_CRT_PATH,
                 }
             )
 
@@ -1295,40 +1313,45 @@ def _scrape_jobs(self) -> list:
         return [job]
 
     def _on_server_cert_changed(self, _):
-        self._update_cert()
-        self._configure()
+        # this triggers a restart, which triggers a cert update.
+        self._configure(force_restart=True)
 
     def _update_cert(self):
         container = self.containers["workload"]
-        ca_cert_path = Path("/usr/local/share/ca-certificates/cos-ca.crt")
-        if self.cert_handler.cert and self.cert_handler.key and self.cert_handler.ca:
+        if self.cert_handler.cert:
             # Save the workload certificates
             container.push(
-                "/etc/grafana/grafana.crt",
+                GRAFANA_CRT_PATH,
                 self.cert_handler.cert,
                 make_dirs=True,
             )
+        else:
+            container.remove_path(GRAFANA_CRT_PATH, recursive=True)
+
+        if self.cert_handler.key:
             container.push(
-                "/etc/grafana/grafana.key",
+                GRAFANA_KEY_PATH,
                 self.cert_handler.key,
                 make_dirs=True,
             )
+        else:
+            container.remove_path(GRAFANA_KEY_PATH, recursive=True)
+
+        if self.cert_handler.ca:
             # Save the CA among the trusted CAs and trust it
             container.push(
-                ca_cert_path,
+                CA_CERT_PATH,
                 self.cert_handler.ca,
                 make_dirs=True,
             )
 
             # Repeat for the charm container. We need it there for grafana client requests.
-            ca_cert_path.parent.mkdir(exist_ok=True, parents=True)
-            ca_cert_path.write_text(self.cert_handler.ca)
+            CA_CERT_PATH.parent.mkdir(exist_ok=True, parents=True)
+            CA_CERT_PATH.write_text(self.cert_handler.ca)
         else:
-            container.remove_path("/etc/grafana/grafana.crt", recursive=True)
-            container.remove_path("/etc/grafana/grafana.key", recursive=True)
-            container.remove_path(ca_cert_path, recursive=True)
+            container.remove_path(CA_CERT_PATH, recursive=True)
             # Repeat for the charm container.
-            ca_cert_path.unlink(missing_ok=True)
+            CA_CERT_PATH.unlink(missing_ok=True)
 
         container.exec(["update-ca-certificates", "--fresh"]).wait()
         subprocess.run(["update-ca-certificates", "--fresh"])