From 1dd8509c93a32e549d25849a0237f72b8b55d47e Mon Sep 17 00:00:00 2001 From: dushu Date: Tue, 23 Jan 2024 23:12:42 -0500 Subject: [PATCH] feat: enable StartTLS support --- .pre-commit-config.yaml | 2 +- README.md | 46 +++++++++++++++++++------- config.yaml | 5 +++ lib/charms/glauth_k8s/v0/ldap.py | 16 +++++----- metadata.yaml | 2 +- src/charm.py | 19 +++++------ src/configs.py | 20 ++++++++++++ src/database.py | 2 +- src/integrations.py | 7 +++- src/utils.py | 34 ++++++++++++++++---- templates/glauth.cfg.j2 | 3 ++ tests/integration/test_charm.py | 1 + tests/unit/conftest.py | 5 +++ tests/unit/test_charm.py | 55 ++++++++++++++++++++++++++++---- tests/unit/test_utils.py | 28 ++++++++++++++++ 15 files changed, 198 insertions(+), 47 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cabc3a25..0da10afa 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -9,7 +9,7 @@ repos: - id: requirements-txt-fixer - id: trailing-whitespace - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.1.13 + rev: v0.1.14 hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] diff --git a/README.md b/README.md index a9c5d824..a3d77e39 100644 --- a/README.md +++ b/README.md @@ -32,16 +32,6 @@ $ juju integrate glauth-k8s postgresql-k8s ## Integrations -### `postgresql_client` Integration - -The `glauth-k8s` charmed operator requires the integration with the -`postgres-k8s` charmed operator following the [`postgresql_client` interface -protocol](https://github.com/canonical/charm-relation-interfaces/tree/main/interfaces/postgresql_client/v0). - -```shell -$ juju integrate glauth-k8s postgresql-k8s -``` - ### `ldap` Integration The `glauth-k8s` charmed operator offers the `ldap` integration with any @@ -60,7 +50,38 @@ the [`glauth-utils` charmed operator](https://github.com/canonical/glauth-utils) to deliver necessary auxiliary configurations. ```shell -$ juju integrate glauth-k8s glauth-utils +$ juju integrate glauth-utils glauth-k8s +``` + +### `certificate_transfer` Integration + +The `glauth-k8s` charmed operator provides the `certificate_transfer` +integration with any charmed operator following the [`certificate_transfer` +interface protocol](https://github.com/canonical/charm-relation-interfaces/tree/main/interfaces/certificate_transfer/v0). + +```shell +$ juju integrate glauth-k8s +``` + +### `postgresql_client` Integration + +The `glauth-k8s` charmed operator requires the integration with the +`postgres-k8s` charmed operator following the [`postgresql_client` interface +protocol](https://github.com/canonical/charm-relation-interfaces/tree/main/interfaces/postgresql_client/v0). + +```shell +$ juju integrate glauth-k8s postgresql-k8s +``` + +### `tls_certificates` Integration + +The `glauth-k8s` charmed operator requires the `tls-certificates` +integration with any charmed operator following the [`tls_certificates` +interface protocol](https://github.com/canonical/charm-relation-interfaces/tree/main/interfaces/tls_certificates/v0). +Take the `self-signed-certificates-operator` as an example: + +```shell +$ juju integrate glauth-k8s self-signed-certificates ``` ## Configurations @@ -72,11 +93,12 @@ options. |:-------------------:|------------------------------------------------------------------|------------------------------------------------------| | `base_dn` | The portion of the DIT in which to search for matching entries | `juju config base-dn="dc=glauth,dc=com"` | | `hostname` | The hostname of the LDAP server in `glauth-k8s` charmed operator | `juju config hostname="ldap.glauth.com"` | +| `starttls_enabled` | The switch to enable/disable StartTLS support | `juju config starttls_enabled=true` | > ⚠️ **NOTE** > > - The `hostname` should **NOT** contain the ldap scheme (e.g. `ldap://`) and -> port. + port. > - Please refer to the `config.yaml` for more details about the configurations. ## Contributing diff --git a/config.yaml b/config.yaml index 25a4c66e..12e89a0a 100644 --- a/config.yaml +++ b/config.yaml @@ -17,3 +17,8 @@ options: The hostname should NOT contain the LDAP scheme (e.g. ldap://) and port. default: "ldap.glauth.com" type: string + starttls_enabled: + description: | + Enable the StartTLS support or not. + default: true + type: boolean diff --git a/lib/charms/glauth_k8s/v0/ldap.py b/lib/charms/glauth_k8s/v0/ldap.py index 476faf3b..4320dabe 100644 --- a/lib/charms/glauth_k8s/v0/ldap.py +++ b/lib/charms/glauth_k8s/v0/ldap.py @@ -151,7 +151,7 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 PYDEPS = ["pydantic~=2.5.3"] @@ -187,6 +187,7 @@ class LdapProviderBaseData(BaseModel): url: str base_dn: str + starttls: StrictBool @field_validator("url") @classmethod @@ -196,13 +197,6 @@ def validate_ldap_url(cls, v: str) -> str: return v - -class LdapProviderData(LdapProviderBaseData): - bind_dn: str - bind_password_secret: str - auth_method: Literal["simple"] - starttls: StrictBool - @field_validator("starttls", mode="before") @classmethod def deserialize_bool(cls, v: str | bool) -> bool: @@ -216,6 +210,12 @@ def serialize_bool(self, starttls: bool) -> str: return str(starttls) +class LdapProviderData(LdapProviderBaseData): + bind_dn: str + bind_password_secret: str + auth_method: Literal["simple"] + + class LdapRequirerData(BaseModel): model_config = ConfigDict(frozen=True) diff --git a/metadata.yaml b/metadata.yaml index 9f84a1ed..210db58f 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -17,7 +17,7 @@ resources: oci-image: type: oci-image description: GLAuth oci-image - upstream-source: ghcr.io/canonical/glauth:2.2.1 + upstream-source: ghcr.io/canonical/glauth:2.3.0 requires: pg-database: diff --git a/src/charm.py b/src/charm.py index ea5a4798..a4797fb4 100755 --- a/src/charm.py +++ b/src/charm.py @@ -21,7 +21,7 @@ from charms.observability_libs.v0.cert_handler import CertChanged from charms.observability_libs.v0.kubernetes_service_patch import KubernetesServicePatch from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider -from configs import ConfigFile, DatabaseConfig, pebble_layer +from configs import ConfigFile, DatabaseConfig, StartTLSConfig, pebble_layer from constants import ( CERTIFICATES_TRANSFER_INTEGRATION_NAME, DATABASE_INTEGRATION_NAME, @@ -58,6 +58,7 @@ from utils import ( after_config_updated, block_on_missing, + demand_tls_certificates, leader_unit, validate_container_connectivity, validate_database_resource, @@ -140,7 +141,10 @@ def __init__(self, *args: Any): self._on_promtail_error, ) - self.config_file = ConfigFile(base_dn=self.config.get("base_dn")) + self.config_file = ConfigFile( + base_dn=self.config.get("base_dn"), + starttls_config=StartTLSConfig.load(self), + ) self._ldap_integration = LdapIntegration(self) self._auxiliary_integration = AuxiliaryIntegration(self) @@ -155,6 +159,7 @@ def _restart_glauth_service(self) -> None: ) @validate_container_connectivity + @demand_tls_certificates @validate_integration_exists(DATABASE_INTEGRATION_NAME, on_missing=block_on_missing) @validate_database_resource def _handle_event_update(self, event: HookEvent) -> None: @@ -207,13 +212,7 @@ def _on_remove(self, event: RemoveEvent) -> None: self._configmap.delete() def _on_database_created(self, event: DatabaseCreatedEvent) -> None: - self.config_file.database_config = DatabaseConfig.load(self.database_requirer) - self._update_glauth_config() - - self._container.add_layer(WORKLOAD_CONTAINER, pebble_layer, combine=True) - self._restart_glauth_service() - self.unit.status = ActiveStatus() - + self._handle_event_update(event) self.auxiliary_provider.update_relation_app_data( data=self._auxiliary_integration.auxiliary_data, ) @@ -226,6 +225,7 @@ def _on_database_changed(self, event: DatabaseEndpointsChangedEvent) -> None: def _on_config_changed(self, event: ConfigChangedEvent) -> None: self.config_file.base_dn = self.config.get("base_dn") + self.config_file.starttls_config = StartTLSConfig.load(self) self._handle_event_update(event) self.ldap_provider.update_relations_app_data( data=self._ldap_integration.provider_base_data @@ -268,6 +268,7 @@ def _on_cert_changed(self, event: CertChanged) -> None: ) return + self._handle_event_update(event) self._certs_transfer_integration.transfer_certificates( self._certs_integration.cert_data, ) diff --git a/src/configs.py b/src/configs.py index 22bd32b5..8b18888b 100644 --- a/src/configs.py +++ b/src/configs.py @@ -1,13 +1,17 @@ from dataclasses import asdict, dataclass +from pathlib import Path from typing import Any, Optional from constants import ( GLAUTH_COMMANDS, LOG_FILE, POSTGRESQL_DSN_TEMPLATE, + SERVER_CERT, + SERVER_KEY, WORKLOAD_SERVICE, ) from jinja2 import Template +from ops.charm import CharmBase from ops.pebble import Layer @@ -43,10 +47,24 @@ def load(cls, requirer: Any) -> "DatabaseConfig": ) +@dataclass +class StartTLSConfig: + enabled: bool = True + tls_key: Path = SERVER_KEY + tls_cert: Path = SERVER_CERT + + @classmethod + def load(cls, charm: CharmBase) -> "StartTLSConfig": + return StartTLSConfig( + enabled=charm.config.get("starttls_enabled", True), + ) + + @dataclass class ConfigFile: base_dn: Optional[str] = None database_config: Optional[DatabaseConfig] = None + starttls_config: Optional[StartTLSConfig] = None @property def content(self) -> str: @@ -57,9 +75,11 @@ def render(self) -> str: template = Template(file.read()) database_config = self.database_config or DatabaseConfig() + starttls_config = self.starttls_config or StartTLSConfig() rendered = template.render( base_dn=self.base_dn, database=asdict(database_config), + starttls=asdict(starttls_config), ) return rendered diff --git a/src/database.py b/src/database.py index 4bf458dc..9502f6fb 100644 --- a/src/database.py +++ b/src/database.py @@ -31,7 +31,7 @@ class User(Base): class Group(Base): - __tablename__ = "groups" + __tablename__ = "ldapgroups" id = mapped_column(Integer, primary_key=True) name: Mapped[str] = mapped_column(name="name", unique=True) diff --git a/src/integrations.py b/src/integrations.py index 271c06bc..3b2c7707 100644 --- a/src/integrations.py +++ b/src/integrations.py @@ -85,11 +85,16 @@ def ldap_url(self) -> str: def base_dn(self) -> str: return self._charm.config.get("base_dn") + @property + def starttls_enabled(self) -> bool: + return self._charm.config.get("starttls_enabled", True) + @property def provider_base_data(self) -> LdapProviderBaseData: return LdapProviderBaseData( url=self.ldap_url, base_dn=self.base_dn, + starttls=self.starttls_enabled, ) @property @@ -103,7 +108,7 @@ def provider_data(self) -> Optional[LdapProviderData]: bind_dn=f"cn={self._bind_account.cn},ou={self._bind_account.ou},{self.base_dn}", bind_password_secret=self._bind_account.password or "", auth_method="simple", - starttls=True, + starttls=self.starttls_enabled, ) diff --git a/src/utils.py b/src/utils.py index 3e51ecca..69e26aea 100644 --- a/src/utils.py +++ b/src/utils.py @@ -5,7 +5,7 @@ from functools import wraps from typing import Any, Callable, Optional -from constants import GLAUTH_CONFIG_FILE +from constants import GLAUTH_CONFIG_FILE, SERVER_CERT, SERVER_KEY from ops.charm import CharmBase, EventBase from ops.model import BlockedStatus, WaitingStatus from tenacity import Retrying, TryAgain, wait_fixed @@ -40,12 +40,12 @@ 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}") + 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.") + charm.unit.status = WaitingStatus("Waiting to connect to container") return None return func(charm, *args, **kwargs) @@ -62,7 +62,7 @@ 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}") + logger.debug(f"Handling event: {event}.") if not charm.model.relations[integration_name]: on_missing_request(charm, event, integration_name=integration_name) @@ -79,10 +79,10 @@ 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}") + 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}") + logger.debug(f"Database has not been created yet, defer event {event}.") event.defer() charm.unit.status = WaitingStatus("Waiting for database creation") @@ -93,10 +93,30 @@ def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: return wrapper +def demand_tls_certificates(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") + return None + + return func(charm, *args, **kwargs) + + return wrapper + + def after_config_updated(func: Callable) -> Callable: @wraps(func) def wrapper(charm: CharmBase, *args: Any, **kwargs: Any) -> Optional[Any]: - charm.unit.status = WaitingStatus("Waiting for configuration to be updated.") + charm.unit.status = WaitingStatus("Waiting for configuration to be updated") for attempt in Retrying( wait=wait_fixed(3), diff --git a/templates/glauth.cfg.j2 b/templates/glauth.cfg.j2 index 7608163b..68a1585d 100644 --- a/templates/glauth.cfg.j2 +++ b/templates/glauth.cfg.j2 @@ -4,6 +4,9 @@ structuredlog = true [ldap] enabled = true listen = "0.0.0.0:3893" + tls = {{ starttls.enabled|tojson }} + tlsKeyPath = "{{ starttls.tls_key|default("/etc/ssl/private/glauth.key", true) }}" + tlsCertPath = "{{ starttls.tls_cert|default("/usr/local/share/ca-certificates/glauth.crt", true) }}" [ldaps] enabled = false diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 55e20107..9c84ac0f 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -30,6 +30,7 @@ async def test_build_and_deploy(ops_test: OpsTest) -> None: str(charm_path), resources={"oci-image": GLAUTH_IMAGE}, application_name=GLAUTH_APP, + config={"starttls_enabled": False}, trust=True, series="jammy", ) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 749fe97b..960d47b9 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -118,6 +118,11 @@ def mocked_certificates_transfer_integration(mocker: MockerFixture, harness: Har return mocked +@pytest.fixture +def mocked_tls_certificates(mocker: MockerFixture, harness: Harness) -> MagicMock: + return mocker.patch("ops.model.Container.exists", return_value=True) + + @pytest.fixture def database_relation(harness: Harness) -> int: relation_id = harness.add_relation(DATABASE_INTEGRATION_NAME, DB_APP) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 78e8857a..3888eac4 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -74,15 +74,29 @@ def test_when_missing_database_relation(self, harness: Harness) -> None: assert isinstance(harness.model.unit.status, BlockedStatus) - def test_when_database_not_created(self, harness: Harness, database_relation: int) -> None: + def test_when_tls_certificates_not_exist(self, harness: Harness) -> None: container = harness.model.unit.get_container(WORKLOAD_CONTAINER) + harness.charm.on.glauth_pebble_ready.emit(container) + + assert isinstance(harness.model.unit.status, BlockedStatus) + def test_when_database_not_created( + self, + harness: Harness, + database_relation: int, + mocked_tls_certificates: MagicMock, + ) -> None: + container = harness.model.unit.get_container(WORKLOAD_CONTAINER) harness.charm.on.glauth_pebble_ready.emit(container) assert isinstance(harness.model.unit.status, WaitingStatus) def test_pebble_ready_event( - self, harness: Harness, database_relation: int, database_resource: MagicMock + self, + harness: Harness, + database_relation: int, + database_resource: MagicMock, + mocked_tls_certificates: MagicMock, ) -> None: container = harness.model.unit.get_container(WORKLOAD_CONTAINER) @@ -95,7 +109,11 @@ def test_pebble_ready_event( class TestDatabaseCreatedEvent: def test_database_created_event( - self, harness: Harness, database_relation: int, database_resource: MagicMock + self, + harness: Harness, + mocked_tls_certificates: MagicMock, + database_relation: int, + database_resource: MagicMock, ) -> None: container = harness.model.unit.get_container(WORKLOAD_CONTAINER) @@ -111,18 +129,39 @@ def test_when_container_not_connected(self, harness: Harness) -> None: assert isinstance(harness.model.unit.status, WaitingStatus) - def test_when_missing_database_relation(self, harness: Harness) -> None: + def test_when_tls_certificates_not_exist( + self, + harness: Harness, + database_relation: MagicMock, + database_resource: MagicMock, + ) -> None: harness.charm.on.config_changed.emit() assert isinstance(harness.model.unit.status, BlockedStatus) - def test_when_database_not_created(self, harness: Harness, database_relation: int) -> None: + def test_when_missing_database_relation( + self, harness: Harness, mocked_tls_certificates: MagicMock + ) -> None: + harness.charm.on.config_changed.emit() + + assert isinstance(harness.model.unit.status, BlockedStatus) + + def test_when_database_not_created( + self, + harness: Harness, + database_relation: int, + mocked_tls_certificates: MagicMock, + ) -> None: harness.charm.on.config_changed.emit() assert isinstance(harness.model.unit.status, WaitingStatus) def test_on_config_changed_event( - self, harness: Harness, database_relation: int, database_resource: MagicMock + self, + harness: Harness, + database_relation: int, + database_resource: MagicMock, + mocked_tls_certificates: MagicMock, ) -> None: container = harness.model.unit.get_container(WORKLOAD_CONTAINER) @@ -150,6 +189,7 @@ def test_when_requirer_data_not_ready( def test_when_ldap_requested( self, harness: Harness, + mocked_tls_certificates: MagicMock, database_resource: MagicMock, mocked_ldap_integration: MagicMock, ldap_relation: int, @@ -170,6 +210,7 @@ def test_when_database_not_created( def test_on_ldap_auxiliary_requested( self, harness: Harness, + mocked_tls_certificates: MagicMock, database_resource: MagicMock, ldap_auxiliary_relation: int, ldap_auxiliary_relation_data: MagicMock, @@ -189,7 +230,7 @@ def test_when_container_not_connected(self, harness: Harness) -> None: @patch( "charm.CertificatesIntegration.update_certificates", - side_effect=CertificatesError("Failed to update certificates."), + side_effect=CertificatesError, ) def test_when_update_certificates_failed( self, diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index ab7d5150..22d1abc9 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -11,6 +11,7 @@ from utils import ( after_config_updated, block_on_missing, + demand_tls_certificates, leader_unit, validate_container_connectivity, validate_database_resource, @@ -107,6 +108,33 @@ def wrapped(charm: CharmBase, event: HookEvent) -> sentinel: assert wrapped(harness.charm, mocked_hook_event) is None assert isinstance(harness.model.unit.status, WaitingStatus) + def test_tls_certificates_not_exist( + self, + mocked_tls_certificates: MagicMock, + harness: Harness, + mocked_hook_event: MagicMock, + ) -> None: + @demand_tls_certificates + def wrapped(charm: CharmBase, event: HookEvent) -> sentinel: + charm.unit.status = ActiveStatus() + return sentinel + + mocked_tls_certificates.return_value = False + assert wrapped(harness.charm, mocked_hook_event) is None + assert isinstance(harness.model.unit.status, BlockedStatus) + + def test_demand_tls_certificates( + self, + harness: Harness, + mocked_hook_event: MagicMock, + mocked_tls_certificates: MagicMock, + ) -> None: + @demand_tls_certificates + def wrapped(charm: CharmBase, event: HookEvent) -> sentinel: + return sentinel + + assert wrapped(harness.charm, mocked_hook_event) is sentinel + @patch("ops.model.Container.pull", return_value=StringIO("abc")) @patch("charm.ConfigFile.content", new_callable=PropertyMock, return_value="abc") def test_after_config_updated(