refactor: clean up charm and test code

src/charm.py

@@ -43,7 +43,7 @@
     SourceFieldsMissingError,
 )
 from charms.hydra.v0.oauth import (
-    ClientConfig,
+    ClientConfig as OauthClientConfig,
     OAuthInfoChangedEvent,
     OAuthInfoRemovedEvent,
     OAuthRequirer,
@@ -116,7 +116,6 @@
 TRUSTED_CA_TEMPLATE = string.Template(
     "/usr/local/share/ca-certificates/trusted-ca-cert-$rel_id-ca.crt"
 )
-OAUTH = "oauth"
 OAUTH_SCOPES = "openid email offline_access"
 OAUTH_GRANT_TYPES = ["authorization_code", "refresh_token"]
 
@@ -313,8 +312,6 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None:
 
     def _on_ingress_ready(self, _) -> None:
         """Once Traefik tells us our external URL, make sure we reconfigure Grafana."""
-        self.oauth.update_client_config(client_config=self._oauth_client_config)
-
         self._configure()
 
     def _configure_ingress(self, event: HookEvent) -> None:
@@ -477,6 +474,8 @@ def _configure(self, force_restart: bool = False) -> None:
 
             restart = True
 
+        self.oauth.update_client_config(client_config=self._oauth_client_config)
+
         if self._check_datasource_provisioning():
             # Non-leaders will get updates from litestream
             if self.unit.is_leader():
@@ -736,17 +735,11 @@ def _generate_grafana_config(self) -> str:
         For now, this only creates database information, since everything else
         can be set in ENV variables, but leave for expansion later so we can
         hide auth secrets
-
-        The feature toggle accessTokenExpirationCheck is also set here. It's needed
-        for the oauth relation to provide refresh tokens.
         """
         configs = []
         if self.has_db:
             configs.append(self._generate_database_config())
 
-        if self.oauth.is_client_created():
-            configs.append(self._generate_oauth_refresh_config())
-
         return "\n".join(configs)
 
     def _generate_database_config(self) -> str:
@@ -969,6 +962,8 @@ def _build_layer(self) -> Layer:
                     "GF_AUTH_GENERIC_OAUTH_TOKEN_URL": oauth_provider_info.token_endpoint,
                     "GF_AUTH_GENERIC_OAUTH_API_URL": oauth_provider_info.userinfo_endpoint,
                     "GF_AUTH_GENERIC_OAUTH_USE_REFRESH_TOKEN": "True",
+                    # TODO: This toggle will be removed on grafana v10.3, remove it
+                    "GF_FEATURE_TOGGLES_ENABLE": "accessTokenExpirationCheck",
                 }
             )
 
@@ -1445,34 +1440,22 @@ def _on_trusted_certificate_removed(self, event: CertificateRemovedEvent):
         self.restart_grafana()
 
     @property
-    def _oauth_client_config(self) -> ClientConfig:
-        return ClientConfig(
+    def _oauth_client_config(self) -> OauthClientConfig:
+        return OauthClientConfig(
             os.path.join(self.external_url, "login/generic_oauth"),
             OAUTH_SCOPES,
             OAUTH_GRANT_TYPES,
         )
 
     def _on_oauth_info_changed(self, event: OAuthInfoChangedEvent) -> None:
         """Event handler for the oauth_info_changed event."""
-        logger.info(f"Received oauth provider info: {self.oauth.get_provider_info()}")
-
         self._configure()
 
     def _on_oauth_info_removed(self, event: OAuthInfoRemovedEvent) -> None:
         """Event handler for the oauth_info_removed event."""
-        logger.info("Oauth relation is broken, removing related settings")
-
         # Reset generic_oauth settings
         self._configure()
 
-    def _generate_oauth_refresh_config(self) -> str:
-        """Generate a configuration for automatic refreshing of oauth authentication.
-
-        Returns:
-            A string containing the required feature toggle information to be stubbed into the config file.
-        """
-        return "[feature_toggles]\naccessTokenExpirationCheck = true\n"
-
 
 if __name__ == "__main__":
     main(GrafanaCharm, use_juju_for_storage=True)

tests/unit/test_charm.py

@@ -76,13 +76,6 @@
 """
 
 
-OAUTH_CONFIG_INI = """[feature_toggles]
-accessTokenExpirationCheck = true
-"""
-OAUTH_CLIENT_ID = "grafana_client_id"
-OAUTH_CLIENT_SECRET = "s3cR#T"
-
-
 AUTH_PROVIDER_APPLICATION = "auth_provider"
 
 
@@ -121,7 +114,7 @@ def cli_arg(plan, cli_opt):
 )
 
 
-class TestCharm(unittest.TestCase):
+class BaseTestCharm(unittest.TestCase):
     def setUp(self, *unused):
         self.harness = Harness(GrafanaCharm)
         self.addCleanup(self.harness.cleanup)
@@ -152,6 +145,8 @@ def setUp(self, *unused):
             str(yaml.dump(MINIMAL_DATASOURCES_CONFIG)).encode("utf-8")
         ).hexdigest()
 
+
+class TestCharm(BaseTestCharm):
     def test_datasource_config_is_updated_by_raw_grafana_source_relation(self):
         self.harness.set_leader(True)
 
@@ -380,79 +375,6 @@ def test_config_is_updated_with_authentication_config(self):
         self.assertIn("GF_AUTH_PROXY_ENABLED", services["environment"].keys())
         self.assertEqual(services["environment"]["GF_AUTH_PROXY_ENABLED"], "True")
 
-    def test_config_is_updated_with_oauth_relation_data(self):
-        self.harness.set_leader(True)
-        self.harness.container_pebble_ready("grafana")
-
-        oauth_provider_info = {
-            "authorization_endpoint": "https://example.oidc.com/oauth2/auth",
-            "introspection_endpoint": "https://example.oidc.com/admin/oauth2/introspect",
-            "issuer_url": "https://example.oidc.com",
-            "jwks_endpoint": "https://example.oidc.com/.well-known/jwks.json",
-            "scope": "openid profile email phone",
-            "token_endpoint": "https://example.oidc.com/oauth2/token",
-            "userinfo_endpoint": "https://example.oidc.com/userinfo",
-        }
-
-        # add oauth relation with provider endpoints details
-        rel_id = self.harness.add_relation("oauth", "hydra")
-        self.harness.add_relation_unit(rel_id, "hydra/0")
-        self.harness.update_relation_data(
-            rel_id,
-            "hydra",
-            oauth_provider_info,
-        )
-
-        # update databag with client details - received once a grafana client is created in hydra
-        secret_id = self.harness.add_model_secret("hydra", {"secret": OAUTH_CLIENT_SECRET})
-        self.harness.grant_secret(secret_id, "grafana-k8s")
-        self.harness.update_relation_data(
-            rel_id,
-            "hydra",
-            {
-                "client_id": OAUTH_CLIENT_ID,
-                "client_secret_id": secret_id,
-            },
-        )
-
-        # assert that generic_oauth config is updated
-        services = (
-            self.harness.charm.containers["workload"].get_plan().services["grafana"].to_dict()
-        )
-
-        self.assertEqual(services["environment"]["GF_AUTH_GENERIC_OAUTH_ENABLED"], "True")
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_NAME"], "external identity provider"
-        )
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_ID"], OAUTH_CLIENT_ID
-        )
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET"], OAUTH_CLIENT_SECRET
-        )
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_SCOPES"], "openid email offline_access"
-        )
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_AUTH_URL"],
-            oauth_provider_info["authorization_endpoint"],
-        )
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_TOKEN_URL"],
-            oauth_provider_info["token_endpoint"],
-        )
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_API_URL"],
-            oauth_provider_info["userinfo_endpoint"],
-        )
-        self.assertEqual(
-            services["environment"]["GF_AUTH_GENERIC_OAUTH_USE_REFRESH_TOKEN"],
-            "True",
-        )
-
-        config = self.harness.charm.containers["workload"].pull(CONFIG_PATH)
-        self.assertEqual(config.read(), OAUTH_CONFIG_INI)
-
 
 class TestCharmReplication(unittest.TestCase):
     def setUp(self, *unused):

tests/unit/test_oauth.py

@@ -0,0 +1,126 @@
+# Copyright 2020 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+import unittest
+from tests.unit.test_charm import BaseTestCharm
+
+OAUTH_CLIENT_ID = "grafana_client_id"
+OAUTH_CLIENT_SECRET = "s3cR#T"
+OAUTH_PROVIDER_INFO = {
+    "authorization_endpoint": "https://example.oidc.com/oauth2/auth",
+    "introspection_endpoint": "https://example.oidc.com/admin/oauth2/introspect",
+    "issuer_url": "https://example.oidc.com",
+    "jwks_endpoint": "https://example.oidc.com/.well-known/jwks.json",
+    "scope": "openid profile email phone",
+    "token_endpoint": "https://example.oidc.com/oauth2/token",
+    "userinfo_endpoint": "https://example.oidc.com/userinfo",
+}
+
+
+class TestOauth(BaseTestCharm):
+    def test_config_is_updated_with_oauth_relation_data(self):
+        self.harness.set_leader(True)
+        self.harness.container_pebble_ready("grafana")
+
+        # add oauth relation with provider endpoints details
+        rel_id = self.harness.add_relation("oauth", "hydra")
+        self.harness.add_relation_unit(rel_id, "hydra/0")
+        secret_id = self.harness.add_model_secret("hydra", {"secret": OAUTH_CLIENT_SECRET})
+        self.harness.grant_secret(secret_id, "grafana-k8s")
+        self.harness.update_relation_data(
+            rel_id,
+            "hydra",
+            {
+                "client_id": OAUTH_CLIENT_ID,
+                "client_secret_id": secret_id,
+                **OAUTH_PROVIDER_INFO,
+            },
+        )
+
+        services = (
+            self.harness.charm.containers["workload"].get_plan().services["grafana"].to_dict()
+        )
+        env = services["environment"]
+
+        expected_env = {
+            "GF_AUTH_GENERIC_OAUTH_ENABLED": "True",
+            "GF_AUTH_GENERIC_OAUTH_NAME": "external identity provider",
+            "GF_AUTH_GENERIC_OAUTH_CLIENT_ID": OAUTH_CLIENT_ID,
+            "GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET": OAUTH_CLIENT_SECRET,
+            "GF_AUTH_GENERIC_OAUTH_SCOPES": "openid email offline_access",
+            "GF_AUTH_GENERIC_OAUTH_AUTH_URL": OAUTH_PROVIDER_INFO["authorization_endpoint"],
+            "GF_AUTH_GENERIC_OAUTH_TOKEN_URL": OAUTH_PROVIDER_INFO["token_endpoint"],
+            "GF_AUTH_GENERIC_OAUTH_API_URL": OAUTH_PROVIDER_INFO["userinfo_endpoint"],
+            "GF_AUTH_GENERIC_OAUTH_USE_REFRESH_TOKEN": "True",
+            "GF_FEATURE_TOGGLES_ENABLE": "accessTokenExpirationCheck",
+        }
+        all(self.assertEqual(env[k], v) for k, v in expected_env.items())
+
+    def test_config_with_empty_oauth_relation_data(self):
+        self.harness.set_leader(True)
+        self.harness.container_pebble_ready("grafana")
+
+        rel_id = self.harness.add_relation("oauth", "hydra")
+        self.harness.add_relation_unit(rel_id, "hydra/0")
+
+        services = (
+            self.harness.charm.containers["workload"].get_plan().services["grafana"].to_dict()
+        )
+        env = services["environment"]
+
+        oauth_env = {
+            "GF_AUTH_GENERIC_OAUTH_ENABLED",
+            "GF_AUTH_GENERIC_OAUTH_NAME",
+            "GF_AUTH_GENERIC_OAUTH_CLIENT_ID",
+            "GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET",
+            "GF_AUTH_GENERIC_OAUTH_SCOPES",
+            "GF_AUTH_GENERIC_OAUTH_AUTH_URL",
+            "GF_AUTH_GENERIC_OAUTH_TOKEN_URL",
+            "GF_AUTH_GENERIC_OAUTH_API_URL",
+            "GF_AUTH_GENERIC_OAUTH_USE_REFRESH_TOKEN",
+        }
+        all(self.assertNotIn(k, env) for k in oauth_env)
+
+    # The oauth library tries to access the relation databag
+    # when the relation is departing. This causes harness to throw an
+    # error, a behavior not implemented in juju. This will be fixed
+    # once https://github.com/canonical/operator/issues/940 is merged
+    @unittest.expectedFailure
+    def test_config_is_updated_with_oauth_relation_data_removed(self):
+        self.harness.set_leader(True)
+        self.harness.container_pebble_ready("grafana")
+
+        # add oauth relation with provider endpoints details
+        rel_id = self.harness.add_relation("oauth", "hydra")
+        self.harness.add_relation_unit(rel_id, "hydra/0")
+        secret_id = self.harness.add_model_secret("hydra", {"secret": OAUTH_CLIENT_SECRET})
+        self.harness.grant_secret(secret_id, "grafana-k8s")
+        self.harness.update_relation_data(
+            rel_id,
+            "hydra",
+            {
+                "client_id": OAUTH_CLIENT_ID,
+                "client_secret_id": secret_id,
+                **OAUTH_PROVIDER_INFO,
+            },
+        )
+        rel_id = self.harness.remove_relation(rel_id)
+
+        services = (
+            self.harness.charm.containers["workload"].get_plan().services["grafana"].to_dict()
+        )
+        env = services["environment"]
+
+        expected_env = {
+            "GF_AUTH_GENERIC_OAUTH_ENABLED": "True",
+            "GF_AUTH_GENERIC_OAUTH_NAME": "external identity provider",
+            "GF_AUTH_GENERIC_OAUTH_CLIENT_ID": OAUTH_CLIENT_ID,
+            "GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET": OAUTH_CLIENT_SECRET,
+            "GF_AUTH_GENERIC_OAUTH_SCOPES": "openid email offline_access",
+            "GF_AUTH_GENERIC_OAUTH_AUTH_URL": OAUTH_PROVIDER_INFO["authorization_endpoint"],
+            "GF_AUTH_GENERIC_OAUTH_TOKEN_URL": OAUTH_PROVIDER_INFO["token_endpoint"],
+            "GF_AUTH_GENERIC_OAUTH_API_URL": OAUTH_PROVIDER_INFO["userinfo_endpoint"],
+            "GF_AUTH_GENERIC_OAUTH_USE_REFRESH_TOKEN": "True",
+            "GF_FEATURE_TOGGLES_ENABLE": "accessTokenExpirationCheck",
+        }
+        all(self.assertEqual(env[k], v) for k, v in expected_env.items())