Merge pull request #26 from canonical/IAM-684

refactor: refactor the decorators used in the charm

pyproject.toml

@@ -30,7 +30,12 @@ target-version = ["py38"]
 # Linting tools configuration
 [tool.ruff]
 line-length = 99
+include = ["pyproject.toml", "src/**/*.py", "tests/**/*.py", "lib/charms/glauth_k8s/**/.py"]
+extend-exclude = ["__pycache__", "*.egg_info"]
+
+[too.ruff.lint]
 select = ["E", "W", "F", "C", "N", "D", "I001"]
+ignore = ["D100", "D101", "D102", "D103", "D105", "D107", "E501", "N818"]
 extend-ignore = [
     "D203",
     "D204",
@@ -44,15 +49,12 @@ extend-ignore = [
     "D409",
     "D413",
 ]
-include = ["pyproject.toml", "src/**/*.py", "tests/**/*.py", "lib/charms/glauth_k8s/**/.py"]
-extend-exclude = ["__pycache__", "*.egg_info"]
-ignore = ["D100", "D101", "D102", "D103", "D105", "D107", "E501", "N818"]
 per-file-ignores = {"tests/*" = ["D100","D101","D102","D103","D104"]}
 
-[tool.ruff.mccabe]
+[tool.ruff.lint.mccabe]
 max-complexity = 10
 
-[tool.ruff.pydocstyle]
+[tool.ruff.lint.pydocstyle]
 convention = "google"
 
 [tool.mypy]

src/charm.py

@@ -23,6 +23,7 @@
 from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider
 from configs import ConfigFile, DatabaseConfig, StartTLSConfig, pebble_layer
 from constants import (
+    CERTIFICATES_INTEGRATION_NAME,
     CERTIFICATES_TRANSFER_INTEGRATION_NAME,
     DATABASE_INTEGRATION_NAME,
     GLAUTH_CONFIG_DIR,
@@ -57,12 +58,13 @@
 from ops.pebble import ChangeError
 from utils import (
     after_config_updated,
-    block_on_missing,
-    demand_tls_certificates,
+    block_when,
+    container_not_connected,
+    database_not_ready,
+    integration_not_exists,
     leader_unit,
-    validate_container_connectivity,
-    validate_database_resource,
-    validate_integration_exists,
+    tls_certificates_not_ready,
+    wait_when,
 )
 
 logger = logging.getLogger(__name__)
@@ -158,10 +160,15 @@ def _restart_glauth_service(self) -> None:
                 "Failed to restart the service, please check the logs"
             )
 
-    @validate_container_connectivity
-    @demand_tls_certificates
-    @validate_integration_exists(DATABASE_INTEGRATION_NAME, on_missing=block_on_missing)
-    @validate_database_resource
+    @block_when(
+        integration_not_exists(DATABASE_INTEGRATION_NAME),
+        integration_not_exists(CERTIFICATES_INTEGRATION_NAME),
+    )
+    @wait_when(
+        container_not_connected,
+        database_not_ready,
+        tls_certificates_not_ready,
+    )
     def _handle_event_update(self, event: HookEvent) -> None:
         self.unit.status = MaintenanceStatus("Configuring GLAuth container")
 
@@ -230,7 +237,7 @@ def _on_config_changed(self, event: ConfigChangedEvent) -> None:
             data=self._ldap_integration.provider_base_data
         )
 
-    @validate_container_connectivity
+    @wait_when(container_not_connected)
     def _on_pebble_ready(self, event: PebbleReadyEvent) -> None:
         if not self._container.isdir(LOG_DIR):
             self._container.make_dir(path=LOG_DIR, make_parents=True)
@@ -239,7 +246,7 @@ def _on_pebble_ready(self, event: PebbleReadyEvent) -> None:
         self._handle_event_update(event)
 
     @leader_unit
-    @validate_database_resource
+    @wait_when(database_not_ready)
     def _on_ldap_requested(self, event: LdapRequestedEvent) -> None:
         if not (requirer_data := event.data):
             logger.error(f"The LDAP requirer {event.app.name} does not provide necessary data.")
@@ -251,14 +258,14 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None:
             data=self._ldap_integration.provider_data,
         )
 
-    @validate_database_resource
+    @wait_when(database_not_ready)
     def _on_auxiliary_requested(self, event: AuxiliaryRequestedEvent) -> None:
         self.auxiliary_provider.update_relation_app_data(
             relation_id=event.relation.id,
             data=self._auxiliary_integration.auxiliary_data,
         )
 
-    @validate_container_connectivity
+    @wait_when(container_not_connected)
     def _on_cert_changed(self, event: CertChanged) -> None:
         try:
             self._certs_integration.update_certificates()

src/constants.py

@@ -8,6 +8,7 @@
 LOKI_API_PUSH_INTEGRATION_NAME = "logging"
 PROMETHEUS_SCRAPE_INTEGRATION_NAME = "metrics-endpoint"
 GRAFANA_DASHBOARD_INTEGRATION_NAME = "grafana-dashboard"
+CERTIFICATES_INTEGRATION_NAME = "certificates"
 CERTIFICATES_TRANSFER_INTEGRATION_NAME = "send-ca-cert"
 
 GLAUTH_CONFIG_DIR = PurePath("/etc/config")

src/utils.py

@@ -12,61 +12,48 @@
 
 logger = logging.getLogger(__name__)
 
+ConditionEvaluation = tuple[bool, str]
+Condition = Callable[[CharmBase], ConditionEvaluation]
 
-def _default_on_missing(charm: CharmBase, event: EventBase, **kwargs: Any) -> None:
-    logger.debug(f"Integration {kwargs.get('integration_name')} is missing.")
 
+def container_not_connected(charm: CharmBase) -> ConditionEvaluation:
+    not_connected = not charm._container.can_connect()
+    return not_connected, ("Container is not connected yet" if not_connected else "")
 
-def block_on_missing(charm: CharmBase, event: EventBase, **kwargs: Any) -> None:
-    integration_name = kwargs.get("integration_name")
-    logger.debug(f"Integration {integration_name} is missing, defer event {event}.")
-    event.defer()
 
-    charm.unit.status = BlockedStatus(f"Missing required integration {integration_name}")
+def integration_not_exists(integration_name: str) -> Condition:
+    def wrapped(charm: CharmBase) -> ConditionEvaluation:
+        not_exists = not charm.model.relations[integration_name]
+        return not_exists, (f"Missing integration {integration_name}" if not_exists else "")
 
+    return wrapped
 
-def leader_unit(func: Callable) -> Callable:
-    @wraps(func)
-    def wrapper(charm: CharmBase, *args: Any, **kwargs: Any) -> Optional[Any]:
-        if not charm.unit.is_leader():
-            return None
-
-        return func(charm, *args, **kwargs)
-
-    return wrapper
-
-
-def validate_container_connectivity(func: Callable) -> Callable:
-    @wraps(func)
-    def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]:
-        event, *_ = args
-        logger.debug(f"Handling event: {event}.")
-        if not charm._container.can_connect():
-            logger.debug(f"Cannot connect to container, defer event {event}.")
-            event.defer()
-
-            charm.unit.status = WaitingStatus("Waiting to connect to container")
-            return None
 
-        return func(charm, *args, **kwargs)
+def tls_certificates_not_ready(charm: CharmBase) -> ConditionEvaluation:
+    not_exists = charm.config.get("starttls_enabled", True) and not (
+        charm._container.exists(SERVER_KEY) and charm._container.exists(SERVER_CERT)
+    )
+    return not_exists, ("Missing TLS certificate and private key" if not_exists else "")
 
-    return wrapper
 
+def database_not_ready(charm: CharmBase) -> ConditionEvaluation:
+    not_exists = not charm.database_requirer.is_resource_created()
+    return not_exists, ("Waiting for database creation" if not_exists else "")
 
-def validate_integration_exists(
-    integration_name: str, on_missing: Optional[Callable] = None
-) -> Callable:
-    on_missing_request = on_missing or _default_on_missing
 
+def block_when(*conditions: Condition) -> Callable:
     def decorator(func: Callable) -> Callable:
         @wraps(func)
         def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]:
             event, *_ = args
             logger.debug(f"Handling event: {event}.")
 
-            if not charm.model.relations[integration_name]:
-                on_missing_request(charm, event, integration_name=integration_name)
-                return None
+            for condition in conditions:
+                resp, msg = condition(charm)
+                if resp:
+                    event.defer()
+                    charm.unit.status = BlockedStatus(msg)
+                    return None
 
             return func(charm, *args, **kwargs)
 
@@ -75,37 +62,31 @@ def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]:
     return decorator
 
 
-def validate_database_resource(func: Callable) -> Callable:
-    @wraps(func)
-    def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]:
-        event, *_ = args
-        logger.debug(f"Handling event: {event}.")
+def wait_when(*conditions: Condition) -> Callable:
+    def decorator(func: Callable) -> Callable:
+        @wraps(func)
+        def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]:
+            event, *_ = args
+            logger.debug(f"Handling event: {event}.")
 
-        if not charm.database_requirer.is_resource_created():
-            logger.debug(f"Database has not been created yet, defer event {event}.")
-            event.defer()
+            for condition in conditions:
+                resp, msg = condition(charm)
+                if resp:
+                    event.defer()
+                    charm.unit.status = WaitingStatus(msg)
+                    return None
 
-            charm.unit.status = WaitingStatus("Waiting for database creation")
-            return None
+            return func(charm, *args, **kwargs)
 
-        return func(charm, *args, **kwargs)
+        return wrapper
 
-    return wrapper
+    return decorator
 
 
-def demand_tls_certificates(func: Callable) -> Callable:
+def leader_unit(func: Callable) -> Callable:
     @wraps(func)
     def wrapper(charm: CharmBase, *args: Any, **kwargs: Any) -> Optional[Any]:
-        event, *_ = args
-        logger.debug(f"Handling event: {event}.")
-
-        if charm.config.get("starttls_enabled", True) and not (
-            charm._container.exists(SERVER_KEY) and charm._container.exists(SERVER_CERT)
-        ):
-            logger.debug(f"TLS certificate and private key not ready. defer event {event}.")
-            event.defer()
-
-            charm.unit.status = BlockedStatus("Missing required TLS certificate and private key")
+        if not charm.unit.is_leader():
             return None
 
         return func(charm, *args, **kwargs)

tests/unit/conftest.py

@@ -8,6 +8,7 @@
 from charm import GLAuthCharm
 from charms.glauth_k8s.v0.ldap import LdapProviderData
 from constants import (
+    CERTIFICATES_INTEGRATION_NAME,
     CERTIFICATES_TRANSFER_INTEGRATION_NAME,
     DATABASE_INTEGRATION_NAME,
     WORKLOAD_CONTAINER,
@@ -40,6 +41,7 @@
     "password": DB_PASSWORD,
 }
 
+CERTIFICATE_PROVIDER_APP = "self-signed-certificates"
 CERTIFICATES_TRANSFER_CLIENT_APP = "sssd"
 
 
@@ -186,6 +188,15 @@ def ldap_auxiliary_relation_data(harness: Harness, ldap_auxiliary_relation: int)
     return LDAP_AUXILIARY_RELATION_DATA
 
 
+@pytest.fixture
+def certificates_relation(harness: Harness) -> int:
+    relation_id = harness.add_relation(
+        CERTIFICATES_INTEGRATION_NAME,
+        CERTIFICATE_PROVIDER_APP,
+    )
+    return relation_id
+
+
 @pytest.fixture
 def certificates_transfer_relation(harness: Harness) -> int:
     relation_id = harness.add_relation(