From 48933afd6ec9572b58e5e9fed242ee43a3bb4614 Mon Sep 17 00:00:00 2001 From: dushu Date: Tue, 26 Dec 2023 18:55:33 -0600 Subject: [PATCH 01/10] ci: enforce the development process --- .pre-commit-config.yaml | 26 +++++++++++++++++ fmt-requirements.txt | 2 +- lib/charms/glauth_k8s/v0/ldap.py | 44 ++++++++--------------------- lint-requirements.txt | 8 +----- pyproject.toml | 48 +++++++++++++++++--------------- tests/unit/conftest.py | 5 ++-- tests/unit/test_charm.py | 5 ++-- tests/unit/test_validators.py | 3 +- tox.ini | 43 +++++++++++++++++++++------- 9 files changed, 104 insertions(+), 80 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 00000000..cdd783fb --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,26 @@ +repos: +- repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: check-added-large-files + - id: debug-statements + - id: detect-private-key + - id: end-of-file-fixer + - id: requirements-txt-fixer + - id: trailing-whitespace +- repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.1.9 + hooks: + - id: ruff + args: [--fix, --exit-non-zero-on-fix] +- repo: https://github.com/psf/black + rev: 23.12.1 + hooks: + - id: black +- repo: https://github.com/pre-commit/mirrors-mypy + rev: v1.8.0 + hooks: + - id: mypy + args: ["--config-file", "pyproject.toml"] + additional_dependencies: + - types-PyYAML diff --git a/fmt-requirements.txt b/fmt-requirements.txt index 7559a405..dc109e20 100644 --- a/fmt-requirements.txt +++ b/fmt-requirements.txt @@ -1,2 +1,2 @@ black -isort +ruff diff --git a/lib/charms/glauth_k8s/v0/ldap.py b/lib/charms/glauth_k8s/v0/ldap.py index 691e91a5..8f3b7a79 100644 --- a/lib/charms/glauth_k8s/v0/ldap.py +++ b/lib/charms/glauth_k8s/v0/ldap.py @@ -1,7 +1,7 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -"""# Juju Charm Library for the `ldap` Juju Interface +"""# Juju Charm Library for the `ldap` Juju Interface. This juju charm library contains the Provider and Requirer classes for handling the `ldap` interface. @@ -199,9 +199,7 @@ class LdapRequestedEvent(RelationEvent): def data(self) -> Optional[LdapRequirerData]: relation_data = self.relation.data.get(self.relation.app) return ( - from_dict(data_class=LdapRequirerData, data=relation_data) - if relation_data - else None + from_dict(data_class=LdapRequirerData, data=relation_data) if relation_data else None ) @@ -244,20 +242,12 @@ def __init__( @leader_unit def _on_relation_changed(self, event: RelationChangedEvent) -> None: - """Handle the event emitted when the requirer charm provides the - necessary data.""" - + """Handle the event emitted when the requirer charm provides the necessary data.""" self.on.ldap_requested.emit(event.relation) - def update_relation_app_data( - self, /, relation_id: int, data: LdapProviderData - ) -> None: - """An API for the provider charm to provide the LDAP related - information.""" - - relation = self.charm.model.get_relation( - self._relation_name, relation_id - ) + def update_relation_app_data(self, /, relation_id: int, data: LdapProviderData) -> None: + """An API for the provider charm to provide the LDAP related information.""" + relation = self.charm.model.get_relation(self._relation_name, relation_id) _update_relation_app_databag(self.charm, relation, asdict(data)) @@ -296,17 +286,12 @@ def __init__( def _on_ldap_relation_created(self, event: RelationCreatedEvent) -> None: """Handle the event emitted when an LDAP integration is created.""" - - user = self._data.user or self.app.name - group = self._data.group or self.model.name - _update_relation_app_databag( - self.charm, event.relation, {"user": user, "group": group} - ) + user = self._data.user if self._data else self.app.name + group = self._data.group if self._data else self.model.name + _update_relation_app_databag(self.charm, event.relation, {"user": user, "group": group}) def _on_ldap_relation_changed(self, event: RelationChangedEvent) -> None: - """Handle the event emitted when the LDAP related information is - ready.""" - + """Handle the event emitted when the LDAP related information is ready.""" provider_app = event.relation.app if not event.relation.data.get(provider_app): @@ -316,7 +301,6 @@ def _on_ldap_relation_changed(self, event: RelationChangedEvent) -> None: def _on_ldap_relation_broken(self, event: RelationBrokenEvent) -> None: """Handle the event emitted when the LDAP integration is broken.""" - self.on.ldap_unavailable.emit(event.relation) def consume_ldap_relation_data( @@ -324,12 +308,8 @@ def consume_ldap_relation_data( /, relation_id: Optional[int] = None, ) -> Optional[LdapProviderData]: - """An API for the requirer charm to consume the LDAP related - information in the application databag.""" - - relation = self.charm.model.get_relation( - self._relation_name, relation_id - ) + """An API for the requirer charm to consume the LDAP related information in the application databag.""" + relation = self.charm.model.get_relation(self._relation_name, relation_id) if not relation: return None diff --git a/lint-requirements.txt b/lint-requirements.txt index 5b62c153..0943b53e 100644 --- a/lint-requirements.txt +++ b/lint-requirements.txt @@ -1,9 +1,3 @@ black -flake8==6.0.0 -flake8-docstrings -flake8-copyright -flake8-builtins -pyproject-flake8 -pep8-naming -isort codespell +ruff diff --git a/pyproject.toml b/pyproject.toml index 0694585a..571f0a78 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,29 +27,32 @@ log_cli_level = "INFO" line-length = 99 target-version = ["py38"] -[tool.isort] -line_length = 99 -profile = "black" - # Linting tools configuration -[tool.flake8] -max-line-length = 99 -max-doc-length = 99 +[tool.ruff] +line-length = 99 +select = ["E", "W", "F", "C", "N", "D", "I001"] +extend-ignore = [ + "D203", + "D204", + "D213", + "D215", + "D400", + "D404", + "D406", + "D407", + "D408", + "D409", + "D413", +] +ignore = ["D100", "D101", "D102", "D103", "D105", "D107", "E501", "N818"] +extend-exclude = ["__pycache__", "*.egg_info"] +per-file-ignores = {"tests/*" = ["D100","D101","D102","D103","D104"]} + +[tool.ruff.mccabe] max-complexity = 10 -exclude = [".git", "__pycache__", ".tox", "build", "dist", "*.egg_info", "venv"] -select = ["E", "W", "F", "C", "N", "R", "D", "H"] -# https://www.flake8rules.com/ -# https://www.pydocstyle.org/en/latest/error_codes.html#error-codes -# Ignore W503, E501 because using black creates errors with this -# Ignore D107 Missing docstring in __init__ -ignore = ["W503", "E501", "D100", "D101", "D102", "D103", "D107"] -# D100, D101, D102, D103: Ignore missing docstrings in tests -per-file-ignores = ["tests/*:D100,D101,D102,D103,D104"] -docstring-convention = "google" -# Check for properly formatted copyright header in each file -copyright-check = "True" -copyright-author = "Canonical Ltd." -copyright-regexp = "Copyright\\s\\d{4}([-,]\\d{4})*\\s+%(author)s" + +[tool.ruff.pydocstyle] +convention = "google" [tool.mypy] pretty = true @@ -67,8 +70,9 @@ check_untyped_defs = true allow_redefinition = true disallow_incomplete_defs = true disallow_untyped_defs = true +disable_error_code = "attr-defined" +ignore_missing_imports = true # Ignore libraries that do not have type hint nor stubs [[tool.mypy.overrides]] module = ["ops.*", "pytest.*", "pytest_operator.*", "urllib3.*", "jinja2.*", "lightkube.*", "pytest_mock.*"] -ignore_missing_imports = true diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 06b405c6..44b3114e 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -4,11 +4,10 @@ from unittest.mock import MagicMock import pytest -from ops.testing import Harness -from pytest_mock import MockerFixture - from charm import GLAuthCharm from constants import DATABASE_INTEGRATION_NAME +from ops.testing import Harness +from pytest_mock import MockerFixture DB_APP = "postgresql-k8s" DB_USERNAME = "relation_id" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 75d2a3e4..9a57a096 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -4,13 +4,12 @@ from unittest.mock import MagicMock import pytest +from constants import WORKLOAD_CONTAINER, WORKLOAD_SERVICE +from kubernetes_resource import KubernetesResourceError from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from ops.testing import Harness from pytest_mock import MockerFixture -from constants import WORKLOAD_CONTAINER, WORKLOAD_SERVICE -from kubernetes_resource import KubernetesResourceError - class TestInstallEvent: def test_on_install_non_leader_unit(self, harness: Harness, mocker: MockerFixture) -> None: diff --git a/tests/unit/test_validators.py b/tests/unit/test_validators.py index 0eb8989f..594ca4c3 100644 --- a/tests/unit/test_validators.py +++ b/tests/unit/test_validators.py @@ -3,11 +3,10 @@ from unittest.mock import MagicMock, sentinel +from constants import DATABASE_INTEGRATION_NAME, WORKLOAD_CONTAINER from ops.charm import CharmBase, HookEvent from ops.model import BlockedStatus, WaitingStatus from ops.testing import Harness - -from constants import DATABASE_INTEGRATION_NAME, WORKLOAD_CONTAINER from validators import ( leader_unit, validate_container_connectivity, diff --git a/tox.ini b/tox.ini index cfcaa2a5..6f177262 100644 --- a/tox.ini +++ b/tox.ini @@ -10,7 +10,7 @@ envlist = fmt, lint, unit src_path = {toxinidir}/src/ tst_path = {toxinidir}/tests/ lib_path = {toxinidir}/lib/charms/glauth_k8s -all_path = {[vars]src_path} {[vars]tst_path} +all_path = {[vars]src_path} {[vars]tst_path} {[vars]lib_path} [testenv] setenv = @@ -22,13 +22,23 @@ passenv = CHARM_BUILD_DIR MODEL_SETTINGS +[testenv:dev] +description = Prepare local development tools +deps = + pre-commit + mypy + types-PyYAML +commands = + pre-commit install + pre-commit autoupdate + [testenv:fmt] description = Apply coding style standards to code deps = -r{toxinidir}/fmt-requirements.txt commands = - isort {[vars]all_path} black {[vars]all_path} + ruff --fix {[vars]all_path} [testenv:lint] description = Check code against coding style standards @@ -36,12 +46,16 @@ deps = -r{toxinidir}/lint-requirements.txt commands = codespell {[vars]lib_path} - codespell {toxinidir}/ --skip {toxinidir}/.git --skip {toxinidir}/.tox \ - --skip {toxinidir}/build --skip {toxinidir}/lib --skip {toxinidir}/venv \ - --skip {toxinidir}/.mypy_cache --skip {toxinidir}/icon.svg - # pflake8 wrapper supports config from pyproject.toml - pflake8 {[vars]all_path} - isort --check-only --diff {[vars]all_path} + codespell {toxinidir} \ + --skip {toxinidir}/.git \ + --skip {toxinidir}/.tox \ + --skip {toxinidir}/build \ + --skip {toxinidir}/lib \ + --skip {toxinidir}/venv \ + --skip {toxinidir}/.mypy_cache \ + --skip {toxinidir}/icon.svg + + ruff {[vars]all_path} black --check --diff {[vars]all_path} [testenv:unit] @@ -50,7 +64,11 @@ deps = -r{toxinidir}/unit-requirements.txt commands = coverage run --source={[vars]src_path},{[vars]lib_path} \ - -m pytest --ignore={[vars]tst_path}integration -vv --tb native -s {posargs} + -m pytest \ + --ignore={[vars]tst_path}integration \ + -vv \ + --tb native \ + -s {posargs} coverage report [testenv:integration] @@ -58,4 +76,9 @@ description = Run integration tests deps = -r{toxinidir}/integration-requirements.txt commands = - pytest -v --tb native {[vars]tst_path}integration --log-cli-level=INFO -s {posargs} + pytest -v \ + -s \ + --tb native \ + {[vars]tst_path}integration \ + --log-cli-level=INFO \ + {posargs} From 1c24d2d2cc420fdce9fe8917d13659d8f40d1b55 Mon Sep 17 00:00:00 2001 From: dushu Date: Sat, 30 Dec 2023 11:01:37 -0600 Subject: [PATCH 02/10] refactor: refactor charm configs --- src/charm.py | 90 +++++++++++++---------------------------- src/configs.py | 69 +++++++++++++++++++++++++++++++ src/validators.py | 2 +- templates/glauth.cfg.j2 | 4 +- tests/unit/conftest.py | 2 +- 5 files changed, 100 insertions(+), 67 deletions(-) create mode 100644 src/configs.py diff --git a/src/charm.py b/src/charm.py index a5bf8000..9c926508 100755 --- a/src/charm.py +++ b/src/charm.py @@ -18,23 +18,9 @@ from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer, PromtailDigestError from charms.observability_libs.v0.kubernetes_service_patch import KubernetesServicePatch from charms.prometheus_k8s.v0.prometheus_scrape import MetricsEndpointProvider -from jinja2 import Template -from lightkube import Client -from ops.charm import ( - CharmBase, - ConfigChangedEvent, - HookEvent, - InstallEvent, - PebbleReadyEvent, - RemoveEvent, -) -from ops.main import main -from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus -from ops.pebble import ChangeError, Layer - +from configs import ConfigFile, DatabaseConfig, pebble_layer from constants import ( DATABASE_INTEGRATION_NAME, - GLAUTH_COMMANDS, GLAUTH_CONFIG_DIR, GLAUTH_LDAP_PORT, GRAFANA_DASHBOARD_INTEGRATION_NAME, @@ -43,9 +29,20 @@ LOKI_API_PUSH_INTEGRATION_NAME, PROMETHEUS_SCRAPE_INTEGRATION_NAME, WORKLOAD_CONTAINER, - WORKLOAD_SERVICE, ) from kubernetes_resource import ConfigMapResource, StatefulSetResource +from lightkube import Client +from ops.charm import ( + CharmBase, + ConfigChangedEvent, + HookEvent, + InstallEvent, + PebbleReadyEvent, + RemoveEvent, +) +from ops.main import main +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus +from ops.pebble import ChangeError from validators import ( leader_unit, validate_container_connectivity, @@ -68,7 +65,7 @@ def __init__(self, *args: Any): self._statefulset = StatefulSetResource(client=self._k8s_client, name=self.app.name) self._db_name = f"{self.model.name}_{self.app.name}" - self.database = DatabaseRequires( + self.database_requirer = DatabaseRequires( self, relation_name=DATABASE_INTEGRATION_NAME, database_name=self._db_name, @@ -94,31 +91,18 @@ def __init__(self, *args: Any): self.framework.observe(self.on.config_changed, self._on_config_changed) self.framework.observe(self.on.remove, self._on_remove) self.framework.observe(self.on.glauth_pebble_ready, self._on_pebble_ready) - self.framework.observe(self.database.on.database_created, self._on_database_created) - self.framework.observe(self.database.on.endpoints_changed, self._on_database_changed) + self.framework.observe( + self.database_requirer.on.database_created, self._on_database_created + ) + self.framework.observe( + self.database_requirer.on.endpoints_changed, self._on_database_changed + ) self.framework.observe( self.loki_consumer.on.promtail_digest_error, self._on_promtail_error, ) - @property - def _pebble_layer(self) -> Layer: - pebble_layer = { - "summary": "GLAuth layer", - "description": "pebble layer for GLAuth service", - "services": { - WORKLOAD_SERVICE: { - "override": "replace", - "summary": "GLAuth Operator layer", - "startup": "disabled", - "command": '/bin/sh -c "{} 2>&1 | tee {}"'.format( - GLAUTH_COMMANDS, - LOG_FILE, - ), - } - }, - } - return Layer(pebble_layer) + self.config_file = ConfigFile(base_dn=self.config.get("base_dn")) def _restart_glauth_service(self) -> None: try: @@ -136,37 +120,14 @@ def _handle_event_update(self, event: HookEvent) -> None: self.unit.status = MaintenanceStatus("Configuring GLAuth container") self._update_glauth_config() - self._container.add_layer(WORKLOAD_CONTAINER, self._pebble_layer, combine=True) + self._container.add_layer(WORKLOAD_CONTAINER, pebble_layer, combine=True) self._restart_glauth_service() self.unit.status = ActiveStatus() - def _fetch_database_relation_data(self) -> dict: - relation_id = self.database.relations[0].id - relation_data = self.database.fetch_relation_data()[relation_id] - - return { - "username": relation_data.get("username"), - "password": relation_data.get("password"), - "endpoints": relation_data.get("endpoints"), - "database_name": self._db_name, - } - - def _render_config_file(self) -> str: - with open("templates/glauth.cfg.j2", mode="r") as file: - template = Template(file.read()) - - rendered = template.render( - db_info=self._fetch_database_relation_data(), - ldap_port=GLAUTH_LDAP_PORT, - base_dn=self.config.get("base_dn"), - ) - return rendered - @leader_unit def _update_glauth_config(self) -> None: - conf = self._render_config_file() - self._configmap.patch({"glauth.cfg": conf}) + self._configmap.patch({"glauth.cfg": self.config_file.content}) @leader_unit def _mount_glauth_config(self) -> None: @@ -202,16 +163,19 @@ 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_config(self.database_requirer) self._update_glauth_config() self._mount_glauth_config() - self._container.add_layer(WORKLOAD_CONTAINER, self._pebble_layer, combine=True) + self._container.add_layer(WORKLOAD_CONTAINER, pebble_layer, combine=True) self._restart_glauth_service() self.unit.status = ActiveStatus() def _on_database_changed(self, event: DatabaseEndpointsChangedEvent) -> None: + self.config_file.database_config = DatabaseConfig.load_config(self.database_requirer) self._handle_event_update(event) def _on_config_changed(self, event: ConfigChangedEvent) -> None: + self.config_file.base_dn = self.config.get("base_dn") self._handle_event_update(event) @validate_container_connectivity diff --git a/src/configs.py b/src/configs.py new file mode 100644 index 00000000..cdea8079 --- /dev/null +++ b/src/configs.py @@ -0,0 +1,69 @@ +from dataclasses import asdict, dataclass +from typing import Any, Optional + +from constants import GLAUTH_COMMANDS, LOG_FILE, WORKLOAD_SERVICE +from jinja2 import Template +from ops.pebble import Layer + + +@dataclass +class DatabaseConfig: + endpoint: Optional[str] = None + database: Optional[str] = None + username: Optional[str] = None + password: Optional[str] = None + + @classmethod + def load_config(cls, requirer: Any) -> "DatabaseConfig": + if not (database_integrations := requirer.relations): + return DatabaseConfig() + + integration_id = database_integrations[0].id + integration_data = requirer.fetch_relation_data()[integration_id] + + return DatabaseConfig( + endpoint=integration_data.get("endpoints"), + database=requirer.database, + username=integration_data.get("username"), + password=integration_data.get("password"), + ) + + +@dataclass +class ConfigFile: + base_dn: Optional[str] = None + database_config: Optional[DatabaseConfig] = None + + @property + def content(self) -> str: + return self.render() + + def render(self) -> str: + with open("templates/glauth.cfg.j2", mode="r") as file: + template = Template(file.read()) + + database_config = self.database_config or DatabaseConfig() + rendered = template.render( + base_dn=self.base_dn, + database=asdict(database_config), + ) + return rendered + + +pebble_layer = Layer( + { + "summary": "GLAuth layer", + "description": "pebble layer for GLAuth service", + "services": { + WORKLOAD_SERVICE: { + "override": "replace", + "summary": "GLAuth Operator layer", + "startup": "disabled", + "command": '/bin/sh -c "{} 2>&1 | tee {}"'.format( + GLAUTH_COMMANDS, + LOG_FILE, + ), + } + }, + } +) diff --git a/src/validators.py b/src/validators.py index 6a879eca..efca3758 100644 --- a/src/validators.py +++ b/src/validators.py @@ -68,7 +68,7 @@ def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: event, *_ = args logger.debug(f"Handling event: {event}") - if not self.database.is_resource_created(): + if not self.database_requirer.is_resource_created(): logger.debug(f"Database has not been created yet, defer event {event}") event.defer() diff --git a/templates/glauth.cfg.j2 b/templates/glauth.cfg.j2 index be4460f6..7608163b 100644 --- a/templates/glauth.cfg.j2 +++ b/templates/glauth.cfg.j2 @@ -3,7 +3,7 @@ structuredlog = true [ldap] enabled = true - listen = "0.0.0.0:{{ ldap_port }}" + listen = "0.0.0.0:3893" [ldaps] enabled = false @@ -13,7 +13,7 @@ structuredlog = true plugin = "/bin/postgres.so" pluginhandler = "NewPostgresHandler" baseDN = "{{ base_dn }}" - database = "postgres://{{ db_info.get('username') }}:{{ db_info.get('password') }}@{{ db_info.get('endpoints') }}/{{ db_info.get('database_name') }}?sslmode=disable" + database = "postgres://{{ database.get('username') }}:{{ database.get('password') }}@{{ database.get('endpoint') }}/{{ database.get('database') }}?sslmode=disable" [behaviors] # Ignore all capabilities restrictions, for instance allowing every user to perform a search diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 44b3114e..4e5f224e 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -70,7 +70,7 @@ def database_resource( mocked_statefulset: MagicMock, database_relation: int, ) -> None: - mocker.patch("charm.GLAuthCharm._render_config_file") + mocker.patch("charm.GLAuthCharm._update_glauth_config") harness.update_relation_data( database_relation, From 53d146772efba4e3ae57ddf725ad958a55e9af0f Mon Sep 17 00:00:00 2001 From: dushu Date: Mon, 1 Jan 2024 16:40:58 -0600 Subject: [PATCH 03/10] fix: solve the mounted configmap update delay issue --- requirements.txt | 5 +++-- src/charm.py | 6 ++++-- src/kubernetes_resource.py | 7 +++++- src/utils.py | 26 ++++++++++++++++++++++ src/validators.py | 30 +++++++++++++------------- tests/unit/conftest.py | 44 ++++++++++++++++++++++++-------------- tests/unit/test_charm.py | 30 ++++++++++++++------------ 7 files changed, 98 insertions(+), 50 deletions(-) create mode 100644 src/utils.py diff --git a/requirements.txt b/requirements.txt index 3d9383bf..acff6bce 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,6 @@ cosl -ops >= 2.2.0 +Jinja2 lightkube lightkube-models -Jinja2 +ops >= 2.2.0 +tenacity ~= 8.2.3 diff --git a/src/charm.py b/src/charm.py index 9c926508..4de88b2a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -43,6 +43,7 @@ from ops.main import main from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus from ops.pebble import ChangeError +from utils import after_config_updated from validators import ( leader_unit, validate_container_connectivity, @@ -104,6 +105,7 @@ def __init__(self, *args: Any): self.config_file = ConfigFile(base_dn=self.config.get("base_dn")) + @after_config_updated def _restart_glauth_service(self) -> None: try: self._container.restart(WORKLOAD_CONTAINER) @@ -156,7 +158,8 @@ def _mount_glauth_config(self) -> None: @leader_unit def _on_install(self, event: InstallEvent) -> None: - self._configmap.create() + self._configmap.create(data={"glauth.cfg": self.config_file.content}) + self._mount_glauth_config() @leader_unit def _on_remove(self, event: RemoveEvent) -> None: @@ -165,7 +168,6 @@ def _on_remove(self, event: RemoveEvent) -> None: def _on_database_created(self, event: DatabaseCreatedEvent) -> None: self.config_file.database_config = DatabaseConfig.load_config(self.database_requirer) self._update_glauth_config() - self._mount_glauth_config() self._container.add_layer(WORKLOAD_CONTAINER, pebble_layer, combine=True) self._restart_glauth_service() self.unit.status = ActiveStatus() diff --git a/src/kubernetes_resource.py b/src/kubernetes_resource.py index 98dc7d51..e5ed88a4 100644 --- a/src/kubernetes_resource.py +++ b/src/kubernetes_resource.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. import logging +from typing import Optional from lightkube import Client from lightkube.core.client import AllNamespacedResource @@ -34,7 +35,10 @@ def get(self) -> AllNamespacedResource: except ApiError as e: logging.error(f"Error fetching ConfigMap: {e}") - def create(self) -> None: + def create(self, data: Optional[dict] = None) -> None: + if self.get(): + return + cm = ConfigMap( apiVersion="v1", kind="ConfigMap", @@ -44,6 +48,7 @@ def create(self) -> None: "app.kubernetes.io/managed-by": "juju", }, ), + data=data, ) try: diff --git a/src/utils.py b/src/utils.py new file mode 100644 index 00000000..6d295be8 --- /dev/null +++ b/src/utils.py @@ -0,0 +1,26 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +from functools import wraps +from typing import Any, Callable, Optional + +from constants import GLAUTH_CONFIG_FILE +from ops.charm import CharmBase +from tenacity import Retrying, TryAgain, wait_fixed + + +def after_config_updated(func: Callable) -> Callable: + @wraps(func) + def wrapper(charm: CharmBase, *args: Any, **kwargs: Any) -> Optional[Any]: + for attempt in Retrying( + wait=wait_fixed(3), + ): + with attempt: + expected_config = charm.config_file.content + current_config = charm._container.pull(GLAUTH_CONFIG_FILE).read() + if expected_config != current_config: + raise TryAgain + + return func(charm, *args, **kwargs) + + return wrapper diff --git a/src/validators.py b/src/validators.py index efca3758..8f323985 100644 --- a/src/validators.py +++ b/src/validators.py @@ -13,28 +13,28 @@ def leader_unit(func: Callable) -> Callable: @wraps(func) - def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: - if not self.unit.is_leader(): + def wrapper(charm: CharmBase, *args: Any, **kwargs: Any) -> Optional[Any]: + if not charm.unit.is_leader(): return None - return func(self, *args, **kwargs) + return func(charm, *args, **kwargs) return wrapper def validate_container_connectivity(func: Callable) -> Callable: @wraps(func) - def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: + def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: event, *_ = args logger.debug(f"Handling event: {event}") - if not self._container.can_connect(): + if not charm._container.can_connect(): logger.debug(f"Cannot connect to container, defer event {event}.") event.defer() - self.unit.status = WaitingStatus("Waiting to connect to container.") + charm.unit.status = WaitingStatus("Waiting to connect to container.") return None - return func(self, *args, **kwargs) + return func(charm, *args, **kwargs) return wrapper @@ -42,20 +42,20 @@ def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: def validate_integration_exists(integration_name: str) -> Callable: def decorator(func: Callable) -> Callable: @wraps(func) - def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: + def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: event, *_ = args logger.debug(f"Handling event: {event}") - if not self.model.relations[integration_name]: + if not charm.model.relations[integration_name]: logger.debug(f"Integration {integration_name} is missing, defer event {event}.") event.defer() - self.unit.status = BlockedStatus( + charm.unit.status = BlockedStatus( f"Missing required integration {integration_name}" ) return None - return func(self, *args, **kwargs) + return func(charm, *args, **kwargs) return wrapper @@ -64,17 +64,17 @@ def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: def validate_database_resource(func: Callable) -> Callable: @wraps(func) - def wrapper(self: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: + def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: event, *_ = args logger.debug(f"Handling event: {event}") - if not self.database_requirer.is_resource_created(): + if not charm.database_requirer.is_resource_created(): logger.debug(f"Database has not been created yet, defer event {event}") event.defer() - self.unit.status = WaitingStatus("Waiting for database creation") + charm.unit.status = WaitingStatus("Waiting for database creation") return None - return func(self, *args, **kwargs) + return func(charm, *args, **kwargs) return wrapper diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 4e5f224e..3442ad73 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,11 +1,13 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +from typing import Callable from unittest.mock import MagicMock import pytest from charm import GLAuthCharm -from constants import DATABASE_INTEGRATION_NAME +from constants import DATABASE_INTEGRATION_NAME, WORKLOAD_CONTAINER +from ops.charm import CharmBase from ops.testing import Harness from pytest_mock import MockerFixture @@ -21,14 +23,14 @@ def k8s_client(mocker: MockerFixture) -> MagicMock: return mocked_k8s_client -@pytest.fixture() +@pytest.fixture def mocked_kubernetes_service_patcher(mocker: MockerFixture) -> MagicMock: mocked_service_patcher = mocker.patch("charm.KubernetesServicePatch") mocked_service_patcher.return_value = lambda x, y: None return mocked_service_patcher -@pytest.fixture() +@pytest.fixture def harness(mocked_kubernetes_service_patcher: MagicMock) -> Harness: harness = Harness(GLAuthCharm) harness.set_model_name("unit-test") @@ -40,38 +42,48 @@ def harness(mocked_kubernetes_service_patcher: MagicMock) -> Harness: harness.cleanup() -@pytest.fixture() +@pytest.fixture def mocked_hook_event(mocker: MockerFixture) -> MagicMock: return mocker.patch("ops.charm.HookEvent", autospec=True) -@pytest.fixture() -def mocked_configmap_patch(mocker: MockerFixture) -> MagicMock: - return mocker.patch("charm.ConfigMapResource.patch") +@pytest.fixture +def mocked_configmap(mocker: MockerFixture, harness: Harness) -> MagicMock: + mocked = mocker.patch("charm.ConfigMapResource", autospec=True) + harness.charm._configmap = mocked + return mocked -@pytest.fixture() -def mocked_statefulset(mocker: MockerFixture) -> MagicMock: - return mocker.patch("charm.StatefulSetResource", autospec=True) +@pytest.fixture +def mocked_statefulset(mocker: MockerFixture, harness: Harness) -> MagicMock: + mocked = mocker.patch("charm.StatefulSetResource", autospec=True) + harness.charm._statefulset = mocked + return mocked -@pytest.fixture() +@pytest.fixture def database_relation(harness: Harness) -> int: relation_id = harness.add_relation(DATABASE_INTEGRATION_NAME, DB_APP) harness.add_relation_unit(relation_id, "postgresql-k8s/0") return relation_id -@pytest.fixture() +@pytest.fixture +def mocked_restart_glauth_service(mocker: MockerFixture, harness: Harness) -> Callable: + def mock_restart_glauth_service(charm: CharmBase) -> None: + charm._container.restart(WORKLOAD_CONTAINER) + + return mocker.patch("charm.GLAuthCharm._restart_glauth_service", mock_restart_glauth_service) + + +@pytest.fixture def database_resource( - mocker: MockerFixture, harness: Harness, - mocked_configmap_patch: MagicMock, + mocked_configmap: MagicMock, mocked_statefulset: MagicMock, database_relation: int, + mocked_restart_glauth_service: Callable, ) -> None: - mocker.patch("charm.GLAuthCharm._update_glauth_config") - harness.update_relation_data( database_relation, DB_APP, diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9a57a096..25605807 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -12,19 +12,22 @@ class TestInstallEvent: - def test_on_install_non_leader_unit(self, harness: Harness, mocker: MockerFixture) -> None: - mocked = mocker.patch("charm.ConfigMapResource.create") - + def test_on_install_non_leader_unit( + self, harness: Harness, mocked_configmap: MagicMock, mocked_statefulset: MagicMock + ) -> None: harness.set_leader(False) harness.charm.on.install.emit() - mocked.assert_not_called() + mocked_configmap.create.assert_not_called() + mocked_statefulset.patch.assert_not_called() - def test_on_install(self, harness: Harness, mocker: MockerFixture) -> None: - mocked = mocker.patch("charm.ConfigMapResource.create") + def test_on_install( + self, harness: Harness, mocked_configmap: MagicMock, mocked_statefulset: MagicMock + ) -> None: harness.charm.on.install.emit() - mocked.assert_called_once() + mocked_configmap.create.assert_called_once() + mocked_statefulset.patch.assert_called_once() def test_configmap_creation_failed(self, harness: Harness, mocker: MockerFixture) -> None: mocked = mocker.patch("charm.ConfigMapResource.create") @@ -37,19 +40,18 @@ def test_configmap_creation_failed(self, harness: Harness, mocker: MockerFixture class TestRemoveEvent: - def test_on_remove_non_leader_unit(self, harness: Harness, mocker: MockerFixture) -> None: - mocked = mocker.patch("charm.ConfigMapResource.delete") - + def test_on_remove_non_leader_unit( + self, harness: Harness, mocked_configmap: MagicMock + ) -> None: harness.set_leader(False) harness.charm.on.remove.emit() - mocked.assert_not_called() + mocked_configmap.delete.assert_not_called() - def test_on_remove(self, harness: Harness, mocker: MockerFixture) -> None: - mocked = mocker.patch("charm.ConfigMapResource.delete") + def test_on_remove(self, harness: Harness, mocked_configmap: MagicMock) -> None: harness.charm.on.remove.emit() - mocked.assert_called_once() + mocked_configmap.delete.assert_called_once() class TestPebbleReadyEvent: From 5ee495c2b183571ac00bef45768759fe8fe683f2 Mon Sep 17 00:00:00 2001 From: dushu Date: Thu, 4 Jan 2024 22:30:53 -0600 Subject: [PATCH 04/10] feat: ldap interface integration in the glauth-k8s --- config.yaml | 8 +- lib/charms/glauth_k8s/v0/ldap.py | 7 +- metadata.yaml | 4 + requirements.txt | 3 + src/charm.py | 32 +++++++- src/configs.py | 13 ++- src/constants.py | 5 ++ src/database.py | 76 ++++++++++++++++++ src/integrations.py | 68 ++++++++++++++++ src/utils.py | 79 +++++++++++++++++- src/validators.py | 80 ------------------- .../{test_validators.py => test_utils.py} | 4 +- 12 files changed, 285 insertions(+), 94 deletions(-) create mode 100644 src/database.py create mode 100644 src/integrations.py delete mode 100644 src/validators.py rename tests/unit/{test_validators.py => test_utils.py} (98%) diff --git a/config.yaml b/config.yaml index aaee4562..883ef46d 100644 --- a/config.yaml +++ b/config.yaml @@ -1,12 +1,16 @@ options: log_level: description: | - Configures the log level. + Configures the log level. Acceptable values are: "info", "debug", "warning", "error" and "critical" default: "info" type: string base_dn: - description: base DN + description: The base DN default: "dc=glauth,dc=com" type: string + hostname: + description: The hostname of the LDAP server + default: "ldap.canonical.com" + type: string diff --git a/lib/charms/glauth_k8s/v0/ldap.py b/lib/charms/glauth_k8s/v0/ldap.py index 8f3b7a79..a7f2ebf1 100644 --- a/lib/charms/glauth_k8s/v0/ldap.py +++ b/lib/charms/glauth_k8s/v0/ldap.py @@ -245,8 +245,13 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None: """Handle the event emitted when the requirer charm provides the necessary data.""" self.on.ldap_requested.emit(event.relation) - def update_relation_app_data(self, /, relation_id: int, data: LdapProviderData) -> None: + def update_relation_app_data( + self, /, relation_id: int, data: Optional[LdapProviderData] + ) -> None: """An API for the provider charm to provide the LDAP related information.""" + if data is None: + return + relation = self.charm.model.get_relation(self._relation_name, relation_id) _update_relation_app_databag(self.charm, relation, asdict(data)) diff --git a/metadata.yaml b/metadata.yaml index c81208cd..8983a9de 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -36,3 +36,7 @@ provides: description: | Forwards the built-in grafana dashboard(s) for monitoring GLAuth. interface: grafana_dashboard + ldap: + description: | + Provides LDAP configuration data + interface: ldap diff --git a/requirements.txt b/requirements.txt index acff6bce..d9d34d91 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,6 +1,9 @@ cosl +dacite ~= 1.8.0 Jinja2 lightkube lightkube-models ops >= 2.2.0 +psycopg[binary] +SQLAlchemy tenacity ~= 8.2.3 diff --git a/src/charm.py b/src/charm.py index 4de88b2a..3a6559e0 100755 --- a/src/charm.py +++ b/src/charm.py @@ -14,6 +14,7 @@ DatabaseEndpointsChangedEvent, DatabaseRequires, ) +from charms.glauth_k8s.v0.ldap import LdapProvider, LdapRequestedEvent from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer, PromtailDigestError from charms.observability_libs.v0.kubernetes_service_patch import KubernetesServicePatch @@ -30,6 +31,7 @@ PROMETHEUS_SCRAPE_INTEGRATION_NAME, WORKLOAD_CONTAINER, ) +from integrations import LdapIntegration from kubernetes_resource import ConfigMapResource, StatefulSetResource from lightkube import Client from ops.charm import ( @@ -43,8 +45,8 @@ from ops.main import main from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus from ops.pebble import ChangeError -from utils import after_config_updated -from validators import ( +from utils import ( + after_config_updated, leader_unit, validate_container_connectivity, validate_database_resource, @@ -73,6 +75,12 @@ def __init__(self, *args: Any): extra_user_roles="SUPERUSER", ) + self.ldap_provider = LdapProvider(self) + self.framework.observe( + self.ldap_provider.on.ldap_requested, + self._on_ldap_requested, + ) + self.service_patcher = KubernetesServicePatch(self, [("ldap", GLAUTH_LDAP_PORT)]) self.loki_consumer = LogProxyConsumer( @@ -104,6 +112,7 @@ def __init__(self, *args: Any): ) self.config_file = ConfigFile(base_dn=self.config.get("base_dn")) + self._ldap_integration = LdapIntegration(self) @after_config_updated def _restart_glauth_service(self) -> None: @@ -121,6 +130,8 @@ def _restart_glauth_service(self) -> None: def _handle_event_update(self, event: HookEvent) -> None: self.unit.status = MaintenanceStatus("Configuring GLAuth container") + self.config_file.database_config = DatabaseConfig.load(self.database_requirer) + self._update_glauth_config() self._container.add_layer(WORKLOAD_CONTAINER, pebble_layer, combine=True) @@ -166,14 +177,13 @@ 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_config(self.database_requirer) + 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() def _on_database_changed(self, event: DatabaseEndpointsChangedEvent) -> None: - self.config_file.database_config = DatabaseConfig.load_config(self.database_requirer) self._handle_event_update(event) def _on_config_changed(self, event: ConfigChangedEvent) -> None: @@ -188,6 +198,20 @@ def _on_pebble_ready(self, event: PebbleReadyEvent) -> None: self._handle_event_update(event) + @validate_database_resource + 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 " f"necessary data." + ) + return + + self._ldap_integration.load_bind_account(requirer_data.user, requirer_data.group) + self.ldap_provider.update_relation_app_data( + event.relation.id, + self._ldap_integration.provider_data, + ) + def _on_promtail_error(self, event: PromtailDigestError) -> None: logger.error(event.message) diff --git a/src/configs.py b/src/configs.py index cdea8079..c71789dd 100644 --- a/src/configs.py +++ b/src/configs.py @@ -1,7 +1,7 @@ from dataclasses import asdict, dataclass from typing import Any, Optional -from constants import GLAUTH_COMMANDS, LOG_FILE, WORKLOAD_SERVICE +from constants import GLAUTH_COMMANDS, LOG_FILE, POSTGRESQL_DSN_TEMPLATE, WORKLOAD_SERVICE from jinja2 import Template from ops.pebble import Layer @@ -13,8 +13,17 @@ class DatabaseConfig: username: Optional[str] = None password: Optional[str] = None + @property + def dsn(self) -> str: + return POSTGRESQL_DSN_TEMPLATE.substitute( + username=self.username, + password=self.password, + endpoint=self.endpoint, + database=self.database, + ) + @classmethod - def load_config(cls, requirer: Any) -> "DatabaseConfig": + def load(cls, requirer: Any) -> "DatabaseConfig": if not (database_integrations := requirer.relations): return DatabaseConfig() diff --git a/src/constants.py b/src/constants.py index c43604e3..c6350b0c 100644 --- a/src/constants.py +++ b/src/constants.py @@ -2,6 +2,7 @@ # See LICENSE file for licensing details. from pathlib import PurePath +from string import Template DATABASE_INTEGRATION_NAME = "pg-database" LOKI_API_PUSH_INTEGRATION_NAME = "logging" @@ -18,3 +19,7 @@ WORKLOAD_CONTAINER = "glauth" WORKLOAD_SERVICE = "glauth" + +DEFAULT_UID = 5001 +DEFAULT_GID = 5501 +POSTGRESQL_DSN_TEMPLATE = Template("postgresql+psycopg://$username:$password@$endpoint/$database") diff --git a/src/database.py b/src/database.py new file mode 100644 index 00000000..4bf458dc --- /dev/null +++ b/src/database.py @@ -0,0 +1,76 @@ +import logging +from typing import Any, Optional, Type + +from sqlalchemy import ( + ColumnExpressionArgument, + Integer, + ScalarResult, + String, + create_engine, + select, +) +from sqlalchemy.orm import DeclarativeBase, Mapped, Session, mapped_column + +logger = logging.getLogger(__name__) + + +class Base(DeclarativeBase): + pass + + +# https://github.com/glauth/glauth-postgres/blob/main/postgres.go +class User(Base): + __tablename__ = "users" + + id: Mapped[int] = mapped_column(primary_key=True) + name: Mapped[str] = mapped_column(String, name="name", unique=True) + uid_number: Mapped[int] = mapped_column(name="uidnumber") + gid_number: Mapped[int] = mapped_column(name="primarygroup") + password_sha256: Mapped[Optional[str]] = mapped_column(name="passsha256") + password_bcrypt: Mapped[Optional[str]] = mapped_column(name="passbcrypt") + + +class Group(Base): + __tablename__ = "groups" + + id = mapped_column(Integer, primary_key=True) + name: Mapped[str] = mapped_column(name="name", unique=True) + gid_number: Mapped[int] = mapped_column(name="gidnumber") + + +class Capability(Base): + __tablename__ = "capabilities" + + id = mapped_column(Integer, primary_key=True) + user_id: Mapped[int] = mapped_column(name="userid") + action: Mapped[str] = mapped_column(default="search") + object: Mapped[str] = mapped_column(default="*") + + +class Operation: + def __init__(self, dsn: str) -> None: + self._dsn = dsn + + def __enter__(self) -> "Operation": + engine = create_engine(self._dsn) + self._session = Session(engine) + return self + + def __exit__(self, exc_type: Any, exc_val: Any, exc_tb: Any) -> None: + if exc_type: + logger.error( + f"The database operation failed. The exception " f"{exc_type} raised: {exc_val}" + ) + self._session.rollback() + else: + self._session.commit() + + self._session.close() + + def select( + self, table: Type[Base], *criteria: ColumnExpressionArgument + ) -> Optional[ScalarResult]: + return self._session.scalars(select(table).filter(*criteria)).first() + + def add(self, entity: Base) -> None: + self._session.add(entity) diff --git a/src/integrations.py b/src/integrations.py new file mode 100644 index 00000000..3f112603 --- /dev/null +++ b/src/integrations.py @@ -0,0 +1,68 @@ +import hashlib +import logging +from dataclasses import dataclass +from secrets import token_bytes +from typing import Optional + +from charms.glauth_k8s.v0.ldap import LdapProviderData +from configs import DatabaseConfig +from constants import DEFAULT_GID, DEFAULT_UID, GLAUTH_LDAP_PORT +from database import Capability, Group, Operation, User +from ops.charm import CharmBase + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class BindAccount: + cn: Optional[str] = None + ou: Optional[str] = None + password: Optional[str] = None + + +def _create_bind_account(dsn: str, user_name: str, group_name: str) -> BindAccount: + with Operation(dsn) as op: + if not op.select(Group, Group.name == group_name): + group = Group(name=group_name, gid_number=DEFAULT_GID) + op.add(group) + + if not (user := op.select(User, User.name == user_name)): + new_password = hashlib.sha256(token_bytes()).hexdigest() + user = User( + name=user_name, + uid_number=DEFAULT_UID, + gid_number=DEFAULT_GID, + password_sha256=new_password, + ) + op.add(user) + password = user.password_bcrypt or user.password_sha256 + + if not op.select(Capability, Capability.user_id == DEFAULT_UID): + capability = Capability(user_id=DEFAULT_UID) + op.add(capability) + + return BindAccount(user_name, group_name, password) + + +class LdapIntegration: + def __init__(self, charm: CharmBase): + self._charm = charm + self._bind_account: Optional[BindAccount] = None + + def load_bind_account(self, user: str, group: str) -> None: + database_config = DatabaseConfig.load(self._charm.database_requirer) + self._bind_account = _create_bind_account(database_config.dsn, user, group) + + @property + def provider_data(self) -> Optional[LdapProviderData]: + if not self._bind_account: + return None + + return LdapProviderData( + url=f"ldap://{self._charm.config.get('hostname')}:{GLAUTH_LDAP_PORT}", + base_dn=self._charm.config.get("base_dn"), + bind_dn=f"cn={self._bind_account.cn},ou={self._bind_account.ou},{self._charm.config.get('base_dn')}", + bind_password_secret=self._bind_account.password or "", + auth_method="simple", + starttls=True, + ) diff --git a/src/utils.py b/src/utils.py index 6d295be8..eb9f3b77 100644 --- a/src/utils.py +++ b/src/utils.py @@ -1,13 +1,86 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. +import logging from functools import wraps from typing import Any, Callable, Optional from constants import GLAUTH_CONFIG_FILE -from ops.charm import CharmBase +from ops.charm import CharmBase, EventBase +from ops.model import BlockedStatus, WaitingStatus from tenacity import Retrying, TryAgain, wait_fixed +logger = logging.getLogger(__name__) + + +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) + + return wrapper + + +def validate_integration_exists(integration_name: str) -> 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]: + logger.debug(f"Integration {integration_name} is missing, defer event {event}.") + event.defer() + + charm.unit.status = BlockedStatus( + f"Missing required integration {integration_name}" + ) + return None + + return func(charm, *args, **kwargs) + + return wrapper + + return decorator + + +def validate_database_resource(func: Callable) -> Callable: + @wraps(func) + def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: + event, *_ = args + logger.info(f"Handling event: {event}") + + if not charm.database_requirer.is_resource_created(): + logger.info(f"Database has not been created yet, defer event {event}") + event.defer() + + charm.unit.status = WaitingStatus("Waiting for database creation") + return None + + return func(charm, *args, **kwargs) + + return wrapper + def after_config_updated(func: Callable) -> Callable: @wraps(func) @@ -15,9 +88,9 @@ def wrapper(charm: CharmBase, *args: Any, **kwargs: Any) -> Optional[Any]: for attempt in Retrying( wait=wait_fixed(3), ): + expected_config = charm.config_file.content + current_config = charm._container.pull(GLAUTH_CONFIG_FILE).read() with attempt: - expected_config = charm.config_file.content - current_config = charm._container.pull(GLAUTH_CONFIG_FILE).read() if expected_config != current_config: raise TryAgain diff --git a/src/validators.py b/src/validators.py deleted file mode 100644 index 8f323985..00000000 --- a/src/validators.py +++ /dev/null @@ -1,80 +0,0 @@ -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. - -import logging -from functools import wraps -from typing import Any, Callable, Optional - -from ops.charm import CharmBase, EventBase -from ops.model import BlockedStatus, WaitingStatus - -logger = logging.getLogger(__name__) - - -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) - - return wrapper - - -def validate_integration_exists(integration_name: str) -> 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]: - logger.debug(f"Integration {integration_name} is missing, defer event {event}.") - event.defer() - - charm.unit.status = BlockedStatus( - f"Missing required integration {integration_name}" - ) - return None - - return func(charm, *args, **kwargs) - - return wrapper - - 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}") - - if not charm.database_requirer.is_resource_created(): - logger.debug(f"Database has not been created yet, defer event {event}") - event.defer() - - charm.unit.status = WaitingStatus("Waiting for database creation") - return None - - return func(charm, *args, **kwargs) - - return wrapper diff --git a/tests/unit/test_validators.py b/tests/unit/test_utils.py similarity index 98% rename from tests/unit/test_validators.py rename to tests/unit/test_utils.py index 594ca4c3..de436db1 100644 --- a/tests/unit/test_validators.py +++ b/tests/unit/test_utils.py @@ -7,7 +7,7 @@ from ops.charm import CharmBase, HookEvent from ops.model import BlockedStatus, WaitingStatus from ops.testing import Harness -from validators import ( +from utils import ( leader_unit, validate_container_connectivity, validate_database_resource, @@ -15,7 +15,7 @@ ) -class TestValidators: +class TestUtils: def test_leader_unit(self, harness: Harness) -> None: @leader_unit def wrapped_func(charm: CharmBase) -> sentinel: From a013ee18d0df45cf06ecbb76ad5b5a7e5d6acfd4 Mon Sep 17 00:00:00 2001 From: dushu Date: Fri, 5 Jan 2024 16:27:17 -0600 Subject: [PATCH 05/10] doc: add more documentation --- CONTRIBUTING.md | 23 ++++++++++++----------- README.md | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c56e2c7d..32fcfb2a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,21 +29,22 @@ this operator. ## Developing -You can use the environments created by `tox` for development: +You can use the environments created by `tox` for development. It helps +install `pre-commit` and `mypy` type checker. ```shell -tox --notest -e unit -source .tox/unit/bin/activate +$ tox -e dev +$ source .tox/unit/bin/activate ``` ## Testing ```shell -tox -e fmt # update your code according to linting rules -tox -e lint # code style -tox -e unit # unit tests -tox -e integration # integration tests -tox # runs 'fmt', 'lint', and 'unit' environments +$ tox -e fmt # update your code according to linting rules +$ tox -e lint # code style +$ tox -e unit # unit tests +$ tox -e integration # integration tests +$ tox # runs 'fmt', 'lint', and 'unit' environments ``` ## Build the charm @@ -58,13 +59,13 @@ $ charmcraft pack ```shell # Create a juju model -juju add-model dev +$ juju add-model dev # Enable DEBUG logging -juju model-config logging-config="=INFO;unit=DEBUG" +$ juju model-config logging-config="=INFO;unit=DEBUG" # Deploy the charm -juju deploy ./glauth-k8s_ubuntu-*-amd64.charm --resource oci-image=$(yq eval '.resources.oci-image.upstream-source' metadata.yaml) +$ juju deploy ./glauth-k8s_ubuntu-*-amd64.charm --resource oci-image=$(yq eval '.resources.oci-image.upstream-source' metadata.yaml) ``` ## Observability diff --git a/README.md b/README.md index 09bcb02d..a9c5d824 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,9 @@ ![Ubuntu](https://img.shields.io/badge/Ubuntu-22.04-E95420?label=Ubuntu&logo=ubuntu&logoColor=white) [![License](https://img.shields.io/github/license/canonical/glauth-k8s-operator?label=License)](https://github.com/canonical/glauth-k8s-operator/blob/main/LICENSE) +[![Continuous Integration Status](https://github.com/canonical/glauth-k8s-operator/actions/workflows/on_push.yaml/badge.svg?branch=main)](https://github.com/canonical/glauth-k8s-operator/actions?query=branch%3Amain) +[![pre-commit](https://img.shields.io/badge/pre--commit-enabled-brightgreen?logo=pre-commit)](https://github.com/pre-commit/pre-commit) + This repository holds the Juju Kubernetes charmed operator for [GLAuth](https://github.com/glauth/glauth), an open-sourced LDAP server. @@ -29,15 +32,52 @@ $ juju integrate glauth-k8s postgresql-k8s ## Integrations -TBD. +### `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 +LDAP client charmed operator following +the [`ldap` interface protocol](https://github.com/canonical/charm-relation-interfaces/tree/main/interfaces/ldap/v0). + +```shell +$ juju integrate :ldap glauth-k8s:ldap +``` + +### `glauth_auxiliary` Integration + +The `glauth-k8s` charmed operator provides the `glauth_auxiliary` +integration with +the [`glauth-utils` charmed operator](https://github.com/canonical/glauth-utils) +to deliver necessary auxiliary configurations. + +```shell +$ juju integrate glauth-k8s glauth-utils +``` ## Configurations -TBD. +The `glauth-k8s` charmed operator offers the following charm configuration +options. -## Actions +| Charm Config Option | Description | Example | +|:-------------------:|------------------------------------------------------------------|------------------------------------------------------| +| `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"` | -TBD. +> ⚠️ **NOTE** +> +> - The `hostname` should **NOT** contain the ldap scheme (e.g. `ldap://`) and +> port. +> - Please refer to the `config.yaml` for more details about the configurations. ## Contributing From 324c06a9be9b7d6965d6834ab48990ea72a07877 Mon Sep 17 00:00:00 2001 From: dushu Date: Sun, 7 Jan 2024 22:21:57 -0600 Subject: [PATCH 06/10] refactor: use pydantic for ldap interface lib --- config.yaml | 2 +- integration-requirements.txt | 6 +-- lib/charms/glauth_k8s/v0/ldap.py | 81 +++++++++++++++++++++----------- requirements.txt | 2 +- src/charm.py | 9 ++-- src/integrations.py | 23 +++++++-- 6 files changed, 83 insertions(+), 40 deletions(-) diff --git a/config.yaml b/config.yaml index 883ef46d..a208a399 100644 --- a/config.yaml +++ b/config.yaml @@ -12,5 +12,5 @@ options: type: string hostname: description: The hostname of the LDAP server - default: "ldap.canonical.com" + default: "ldap.glauth.com" type: string diff --git a/integration-requirements.txt b/integration-requirements.txt index f2916108..2b79851c 100644 --- a/integration-requirements.txt +++ b/integration-requirements.txt @@ -1,6 +1,4 @@ -protobuf~=3.20.1 -pytest +-r requirements.txt juju +pytest pytest-operator -requests --r requirements.txt diff --git a/lib/charms/glauth_k8s/v0/ldap.py b/lib/charms/glauth_k8s/v0/ldap.py index a7f2ebf1..7e4807e0 100644 --- a/lib/charms/glauth_k8s/v0/ldap.py +++ b/lib/charms/glauth_k8s/v0/ldap.py @@ -109,7 +109,7 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None: ldap_data = ... # Update the integration data - self.ldap_provider.update_relation_app_data( + self.ldap_provider.update_relations_app_data( relation.id, ldap_data, ) @@ -122,11 +122,9 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None: LDAP related information in order to connect and authenticate to the LDAP server """ -from dataclasses import asdict, dataclass from functools import wraps -from typing import Any, Callable, Optional, Union +from typing import Any, Callable, Literal, Optional, Union -from dacite import Config, from_dict from ops.charm import ( CharmBase, RelationBrokenEvent, @@ -136,6 +134,14 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None: ) from ops.framework import EventSource, Object, ObjectEvents from ops.model import Relation +from pydantic import ( + BaseModel, + ConfigDict, + StrictBool, + ValidationError, + field_serializer, + field_validator, +) # The unique CharmHub library identifier, never change it LIBID = "5a535b3c4d0b40da98e29867128e57b9" @@ -147,7 +153,7 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None: # to 0 if you are raising the major API version LIBPATCH = 2 -PYDEPS = ["dacite~=1.8.0"] +PYDEPS = ["pydantic~=2.5.3"] DEFAULT_RELATION_NAME = "ldap" @@ -176,18 +182,43 @@ def _update_relation_app_databag( relation.data[ldap.app].update(data) -@dataclass(frozen=True) -class LdapProviderData: +class LdapProviderBaseData(BaseModel): + model_config = ConfigDict(frozen=True) + url: str base_dn: str + + @field_validator("url") + @classmethod + def validate_ldap_url(cls, v: str) -> str: + if not v.startswith("ldap://"): + raise ValidationError("Invalid LDAP URL scheme.") + + return v + + +class LdapProviderData(LdapProviderBaseData): bind_dn: str bind_password_secret: str - auth_method: str - starttls: bool + auth_method: Literal["simple"] + starttls: StrictBool + + @field_validator("starttls", mode="before") + @classmethod + def deserialize_bool(cls, v: str | bool) -> bool: + if isinstance(v, str): + return True if v.casefold() == "true" else False + + return v + @field_serializer("starttls") + def serialize_bool(self, starttls: bool) -> str: + return str(starttls) + + +class LdapRequirerData(BaseModel): + model_config = ConfigDict(frozen=True) -@dataclass(frozen=True) -class LdapRequirerData: user: str group: str @@ -198,9 +229,7 @@ class LdapRequestedEvent(RelationEvent): @property def data(self) -> Optional[LdapRequirerData]: relation_data = self.relation.data.get(self.relation.app) - return ( - from_dict(data_class=LdapRequirerData, data=relation_data) if relation_data else None - ) + return LdapRequirerData(**relation_data) if relation_data else None class LdapProviderEvents(ObjectEvents): @@ -245,15 +274,21 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None: """Handle the event emitted when the requirer charm provides the necessary data.""" self.on.ldap_requested.emit(event.relation) - def update_relation_app_data( - self, /, relation_id: int, data: Optional[LdapProviderData] + def update_relations_app_data( + self, /, data: Optional[LdapProviderBaseData] = None, relation_id: Optional[int] = None ) -> None: """An API for the provider charm to provide the LDAP related information.""" if data is None: return - relation = self.charm.model.get_relation(self._relation_name, relation_id) - _update_relation_app_databag(self.charm, relation, asdict(data)) + if not (relations := self.charm.model.relations.get(self._relation_name)): + return + + if relation_id is not None: + relations = [relation for relation in relations if relation.id == relation_id] + + for relation in relations: + _update_relation_app_databag(self.charm, relation, data.model_dump()) class LdapRequirer(Object): @@ -320,12 +355,4 @@ def consume_ldap_relation_data( return None provider_data = relation.data.get(relation.app) - return ( - from_dict( - data_class=LdapProviderData, - data=provider_data, - config=Config(cast=[bool]), - ) - if provider_data - else None - ) + return LdapProviderData(**provider_data) if provider_data else None diff --git a/requirements.txt b/requirements.txt index d9d34d91..8628906f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,9 +1,9 @@ cosl -dacite ~= 1.8.0 Jinja2 lightkube lightkube-models ops >= 2.2.0 psycopg[binary] +pydantic ~= 2.5.3 SQLAlchemy tenacity ~= 8.2.3 diff --git a/src/charm.py b/src/charm.py index 3a6559e0..0a945fd6 100755 --- a/src/charm.py +++ b/src/charm.py @@ -189,6 +189,9 @@ 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._handle_event_update(event) + self.ldap_provider.update_relations_app_data( + data=self._ldap_integration.provider_base_data + ) @validate_container_connectivity def _on_pebble_ready(self, event: PebbleReadyEvent) -> None: @@ -207,9 +210,9 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None: return self._ldap_integration.load_bind_account(requirer_data.user, requirer_data.group) - self.ldap_provider.update_relation_app_data( - event.relation.id, - self._ldap_integration.provider_data, + self.ldap_provider.update_relations_app_data( + relation_id=event.relation.id, + data=self._ldap_integration.provider_data, ) def _on_promtail_error(self, event: PromtailDigestError) -> None: diff --git a/src/integrations.py b/src/integrations.py index 3f112603..dfdc215f 100644 --- a/src/integrations.py +++ b/src/integrations.py @@ -4,7 +4,7 @@ from secrets import token_bytes from typing import Optional -from charms.glauth_k8s.v0.ldap import LdapProviderData +from charms.glauth_k8s.v0.ldap import LdapProviderBaseData, LdapProviderData from configs import DatabaseConfig from constants import DEFAULT_GID, DEFAULT_UID, GLAUTH_LDAP_PORT from database import Capability, Group, Operation, User @@ -53,15 +53,30 @@ def load_bind_account(self, user: str, group: str) -> None: database_config = DatabaseConfig.load(self._charm.database_requirer) self._bind_account = _create_bind_account(database_config.dsn, user, group) + @property + def ldap_url(self) -> str: + return f"ldap://{self._charm.config.get('hostname')}:{GLAUTH_LDAP_PORT}" + + @property + def base_dn(self) -> str: + return self._charm.config.get("base_dn") + + @property + def provider_base_data(self) -> LdapProviderBaseData: + return LdapProviderBaseData( + url=self.ldap_url, + base_dn=self.base_dn, + ) + @property def provider_data(self) -> Optional[LdapProviderData]: if not self._bind_account: return None return LdapProviderData( - url=f"ldap://{self._charm.config.get('hostname')}:{GLAUTH_LDAP_PORT}", - base_dn=self._charm.config.get("base_dn"), - bind_dn=f"cn={self._bind_account.cn},ou={self._bind_account.ou},{self._charm.config.get('base_dn')}", + url=self.ldap_url, + base_dn=self.base_dn, + 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, From 9a08e17f73dddf952fdea422d0e65ce07dfd6315 Mon Sep 17 00:00:00 2001 From: dushu Date: Mon, 8 Jan 2024 21:35:48 -0600 Subject: [PATCH 07/10] feat: glauth-auxiliary interface integration --- .pre-commit-config.yaml | 2 +- metadata.yaml | 4 ++++ src/charm.py | 25 ++++++++++++++++++++++++- src/integrations.py | 17 +++++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index cdd783fb..5ab468b0 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.9 + rev: v0.1.11 hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] diff --git a/metadata.yaml b/metadata.yaml index 8983a9de..e44f941d 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -40,3 +40,7 @@ provides: description: | Provides LDAP configuration data interface: ldap + glauth-auxiliary: + description: | + Provides auxiliary data for glauth-utils charmed operator + interface: glauth_auxiliary diff --git a/src/charm.py b/src/charm.py index 0a945fd6..97cf27cc 100755 --- a/src/charm.py +++ b/src/charm.py @@ -15,6 +15,7 @@ DatabaseRequires, ) from charms.glauth_k8s.v0.ldap import LdapProvider, LdapRequestedEvent +from charms.glauth_utils.v0.glauth_auxiliary import AuxiliaryProvider, AuxiliaryRequestedEvent from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer, PromtailDigestError from charms.observability_libs.v0.kubernetes_service_patch import KubernetesServicePatch @@ -31,7 +32,7 @@ PROMETHEUS_SCRAPE_INTEGRATION_NAME, WORKLOAD_CONTAINER, ) -from integrations import LdapIntegration +from integrations import AuxiliaryIntegration, LdapIntegration from kubernetes_resource import ConfigMapResource, StatefulSetResource from lightkube import Client from ops.charm import ( @@ -81,6 +82,12 @@ def __init__(self, *args: Any): self._on_ldap_requested, ) + self.auxiliary_provider = AuxiliaryProvider(self) + self.framework.observe( + self.auxiliary_provider.on.auxiliary_requested, + self._on_auxiliary_requested, + ) + self.service_patcher = KubernetesServicePatch(self, [("ldap", GLAUTH_LDAP_PORT)]) self.loki_consumer = LogProxyConsumer( @@ -113,6 +120,7 @@ def __init__(self, *args: Any): self.config_file = ConfigFile(base_dn=self.config.get("base_dn")) self._ldap_integration = LdapIntegration(self) + self._auxiliary_integration = AuxiliaryIntegration(self) @after_config_updated def _restart_glauth_service(self) -> None: @@ -179,12 +187,20 @@ def _on_remove(self, event: RemoveEvent) -> None: 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.auxiliary_provider.update_relation_app_data( + data=self._auxiliary_integration.auxiliary_data, + ) + def _on_database_changed(self, event: DatabaseEndpointsChangedEvent) -> None: self._handle_event_update(event) + self.auxiliary_provider.update_relation_app_data( + data=self._auxiliary_integration.auxiliary_data, + ) def _on_config_changed(self, event: ConfigChangedEvent) -> None: self.config_file.base_dn = self.config.get("base_dn") @@ -215,6 +231,13 @@ def _on_ldap_requested(self, event: LdapRequestedEvent) -> None: data=self._ldap_integration.provider_data, ) + @validate_database_resource + 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, + ) + def _on_promtail_error(self, event: PromtailDigestError) -> None: logger.error(event.message) diff --git a/src/integrations.py b/src/integrations.py index dfdc215f..7665ac8a 100644 --- a/src/integrations.py +++ b/src/integrations.py @@ -5,6 +5,7 @@ from typing import Optional from charms.glauth_k8s.v0.ldap import LdapProviderBaseData, LdapProviderData +from charms.glauth_utils.v0.glauth_auxiliary import AuxiliaryData from configs import DatabaseConfig from constants import DEFAULT_GID, DEFAULT_UID, GLAUTH_LDAP_PORT from database import Capability, Group, Operation, User @@ -81,3 +82,19 @@ def provider_data(self) -> Optional[LdapProviderData]: auth_method="simple", starttls=True, ) + + +class AuxiliaryIntegration: + def __init__(self, charm: CharmBase): + self._charm = charm + + @property + def auxiliary_data(self) -> AuxiliaryData: + database_config = DatabaseConfig.load(self._charm.database_requirer) + + return AuxiliaryData( + database=database_config.database, + endpoint=database_config.endpoint, + username=database_config.username, + password=database_config.password, + ) From 3446b0257368a361a199988f7972226cbb5ef524 Mon Sep 17 00:00:00 2001 From: dushu Date: Thu, 11 Jan 2024 11:17:24 -0600 Subject: [PATCH 08/10] ci: add release lib workflow --- .github/workflows/release_libs.yaml | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .github/workflows/release_libs.yaml diff --git a/.github/workflows/release_libs.yaml b/.github/workflows/release_libs.yaml new file mode 100644 index 00000000..608b8ee3 --- /dev/null +++ b/.github/workflows/release_libs.yaml @@ -0,0 +1,23 @@ +name: Release Charm Library + +on: + push: + branches: + - main + paths: + - "lib/charms/glauth_k8s/**" + +jobs: + release-libs: + name: Release charm library + runs-on: ubuntu-22.04 + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Release charm library + uses: canonical/charming-actions/release-libraries@main + with: + credentials: ${{ secrets.CHARMCRAFT_CREDENTIALS }} + github-token: ${{ secrets.GITHUB_TOKEN }} From d8a9878bfb2c0174872547aa58b4836d05372d80 Mon Sep 17 00:00:00 2001 From: dushu Date: Mon, 15 Jan 2024 10:12:24 -0600 Subject: [PATCH 09/10] lib: add the glauth-auxiliary library --- .../glauth_utils/v0/glauth_auxiliary.py | 275 ++++++++++++++++++ 1 file changed, 275 insertions(+) create mode 100644 lib/charms/glauth_utils/v0/glauth_auxiliary.py diff --git a/lib/charms/glauth_utils/v0/glauth_auxiliary.py b/lib/charms/glauth_utils/v0/glauth_auxiliary.py new file mode 100644 index 00000000..dd175b92 --- /dev/null +++ b/lib/charms/glauth_utils/v0/glauth_auxiliary.py @@ -0,0 +1,275 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""# Juju Charm Library for the `glauth_auxiliary` Juju Interface. + +This juju charm library contains the Provider and Requirer classes for handling +the `glauth_auxiliary` interface. + +## Requirer Charm + +The requirer charm is expected to: + +- Listen to the custom juju event `AuxiliaryReadyEvent` to consume the +auxiliary data from the integration +- Listen to the custom juju event `AuxiliaryUnavailableEvent` to handle the +situation when the auxiliary integration is broken + +```python + +from charms.glauth_utils.v0.glauth_auxiliary import ( + AuxiliaryRequirer, + AuxiliaryReadyEvent, +) + +class RequirerCharm(CharmBase): + # Auxiliary requirer charm that integrates with an auxiliary provider charm. + + def __init__(self, *args): + super().__init__(*args) + + self.auxiliary_requirer = AuxiliaryRequirer(self) + self.framework.observe( + self.auxiliary_requirer.on.auxiliary_ready, + self._on_auxiliary_ready, + ) + self.framework.observe( + self.auxiliary_requirer.on.auxiliary_unavailable, + self._on_auxiliary_unavailable, + ) + + def _on_auxiliary_ready(self, event: AuxiliaryReadyEvent) -> None: + # Consume the auxiliary data + auxiliary_data = self.auxiliary_requirer.consume_auxiliary_relation_data( + event.relation.id, + ) + + def _on_auxiliary_unavailable(self, event: AuxiliaryUnavailableEvent) -> None: + # Handle the situation where the auxiliary integration is broken + ... +``` + +As shown above, the library offers custom juju event to handle the specific +situation, which are listed below: + +- auxiliary_ready: event emitted when the auxiliary data is ready for +requirer charm to use. +- auxiliary_unavailable: event emitted when the auxiliary integration is broken. + +Additionally, the requirer charmed operator needs to declare the `auxiliary` +interface in the `metadata.yaml`: + +```yaml +requires: + glauth-auxiliary: + interface: glauth_auxiliary + limit: 1 +``` + +## Provider Charm + +The provider charm is expected to: + +- Listen to the custom juju event `AuxiliaryRequestedEvent` to provide the +auxiliary data in the integration + +```python + +from charms.glauth_utils.v0.glauth_auxiliary import ( + AuxiliaryProvider, + AuxiliaryRequestedEvent, +) + +class ProviderCharm(CharmBase): + # Auxiliary provider charm. + + def __init__(self, *args): + super().__init__(*args) + + self.auxiliary_provider = AuxiliaryProvider(self) + self.framework.observe( + self.auxiliary_provider.on.auxiliary_requested, + self._on_auxiliary_requested, + ) + + def _on_auxiliary_requested(self, event: AuxiliaryRequestedEvent) -> None: + # Prepare the auxiliary data + auxiliary_data = ... + + # Update the integration data + self.auxiliary_provider.update_relation_app_data( + relation.id, + auxiliary_data, + ) +``` + +As shown above, the library offers custom juju event to handle the specific +situation, which are listed below: + +- auxiliary_requested: event emitted when the requirer charm integrates with +the provider charm + +""" + +from functools import wraps +from typing import Any, Callable, Optional, Union + +from ops.charm import ( + CharmBase, + RelationBrokenEvent, + RelationChangedEvent, + RelationCreatedEvent, + RelationEvent, +) +from ops.framework import EventSource, Object, ObjectEvents +from pydantic import BaseModel, ConfigDict + +# The unique Charmhub library identifier, never change it +LIBID = "8c3a907cf23345ea8be7fccfe15b2cf7" + +# Increment this major API version when introducing breaking changes +LIBAPI = 0 + +# Increment this PATCH version before using `charmcraft publish-lib` or reset +# to 0 if you are raising the major API version +LIBPATCH = 1 + +PYDEPS = ["pydantic~=2.5.3"] + +DEFAULT_RELATION_NAME = "glauth-auxiliary" + + +def leader_unit(func: Callable) -> Callable: + @wraps(func) + def wrapper( + obj: Union["AuxiliaryProvider", "AuxiliaryRequirer"], + *args: Any, + **kwargs: Any, + ) -> Any: + if not obj.unit.is_leader(): + return None + + return func(obj, *args, **kwargs) + + return wrapper + + +class AuxiliaryData(BaseModel): + model_config = ConfigDict(frozen=True) + + database: str + endpoint: str + username: str + password: str + + +class AuxiliaryRequestedEvent(RelationEvent): + """An event emitted when the auxiliary integration is built.""" + + +class AuxiliaryReadyEvent(RelationEvent): + """An event emitted when the auxiliary data is ready.""" + + +class AuxiliaryUnavailableEvent(RelationEvent): + """An event emitted when the auxiliary integration is unavailable.""" + + +class AuxiliaryProviderEvents(ObjectEvents): + auxiliary_requested = EventSource(AuxiliaryRequestedEvent) + + +class AuxiliaryRequirerEvents(ObjectEvents): + auxiliary_ready = EventSource(AuxiliaryReadyEvent) + auxiliary_unavailable = EventSource(AuxiliaryUnavailableEvent) + + +class AuxiliaryProvider(Object): + on = AuxiliaryProviderEvents() + + def __init__( + self, + charm: CharmBase, + relation_name: str = DEFAULT_RELATION_NAME, + ) -> None: + super().__init__(charm, relation_name) + + self.charm = charm + self.app = charm.app + self.unit = charm.unit + self._relation_name = relation_name + + self.framework.observe( + self.charm.on[self._relation_name].relation_created, + self._on_relation_created, + ) + + @leader_unit + def _on_relation_created(self, event: RelationCreatedEvent) -> None: + """Handle the event emitted when an auxiliary integration is created.""" + self.on.auxiliary_requested.emit(event.relation) + + @leader_unit + def update_relation_app_data( + self, /, data: AuxiliaryData, relation_id: Optional[int] = None + ) -> None: + """An API for the provider charm to provide the auxiliary data.""" + if not (relations := self.charm.model.relations.get(self._relation_name)): + return + + if relation_id is not None: + relations = [relation for relation in relations if relation.id == relation_id] + + for relation in relations: + relation.data[self.app].update(data.model_dump()) + + +class AuxiliaryRequirer(Object): + on = AuxiliaryRequirerEvents() + + def __init__( + self, + charm: CharmBase, + relation_name: str = DEFAULT_RELATION_NAME, + ) -> None: + super().__init__(charm, relation_name) + + self.charm = charm + self.app = charm.app + self.unit = charm.unit + self._relation_name = relation_name + + self.framework.observe( + self.charm.on[self._relation_name].relation_changed, + self._on_relation_changed, + ) + self.framework.observe( + self.charm.on[self._relation_name].relation_broken, + self._on_auxiliary_relation_broken, + ) + + @leader_unit + def _on_relation_changed(self, event: RelationChangedEvent) -> None: + """Handle the event emitted when auxiliary data is ready.""" + if not event.relation.data.get(event.relation.app): + return + + self.on.auxiliary_ready.emit(event.relation) + + def _on_auxiliary_relation_broken(self, event: RelationBrokenEvent) -> None: + """Handle the event emitted when the auxiliary integration is broken.""" + self.on.auxiliary_unavailable.emit(event.relation) + + def consume_auxiliary_relation_data( + self, + /, + relation_id: Optional[int] = None, + ) -> Optional[AuxiliaryData]: + """An API for the requirer charm to consume the auxiliary data.""" + if not (relation := self.charm.model.get_relation(self._relation_name, relation_id)): + return None + + if not (auxiliary_data := relation.data.get(relation.app)): + return None + + return AuxiliaryData(**auxiliary_data) if auxiliary_data else None From a3ba848f8b9a8beaf5151b1e268c4c02f572c78a Mon Sep 17 00:00:00 2001 From: dushu Date: Tue, 16 Jan 2024 16:32:04 -0600 Subject: [PATCH 10/10] misc: resolve comments --- .pre-commit-config.yaml | 2 +- CODEOWNERS | 1 + config.yaml | 5 ++++- src/charm.py | 3 ++- src/utils.py | 27 ++++++++++++++++++++------- tests/unit/test_utils.py | 23 ++++++++++++++++++----- 6 files changed, 46 insertions(+), 15 deletions(-) create mode 100644 CODEOWNERS diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5ab468b0..cabc3a25 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.11 + rev: v0.1.13 hooks: - id: ruff args: [--fix, --exit-non-zero-on-fix] diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 00000000..ffeec6e5 --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1 @@ +* @canonical/identity diff --git a/config.yaml b/config.yaml index a208a399..25a4c66e 100644 --- a/config.yaml +++ b/config.yaml @@ -11,6 +11,9 @@ options: default: "dc=glauth,dc=com" type: string hostname: - description: The hostname of the LDAP server + description: | + The hostname of the LDAP server. + + The hostname should NOT contain the LDAP scheme (e.g. ldap://) and port. default: "ldap.glauth.com" type: string diff --git a/src/charm.py b/src/charm.py index 97cf27cc..d7ff175c 100755 --- a/src/charm.py +++ b/src/charm.py @@ -48,6 +48,7 @@ from ops.pebble import ChangeError from utils import ( after_config_updated, + block_on_missing, leader_unit, validate_container_connectivity, validate_database_resource, @@ -133,7 +134,7 @@ def _restart_glauth_service(self) -> None: ) @validate_container_connectivity - @validate_integration_exists(DATABASE_INTEGRATION_NAME) + @validate_integration_exists(DATABASE_INTEGRATION_NAME, on_missing=block_on_missing) @validate_database_resource def _handle_event_update(self, event: HookEvent) -> None: self.unit.status = MaintenanceStatus("Configuring GLAuth container") diff --git a/src/utils.py b/src/utils.py index eb9f3b77..74e52d5e 100644 --- a/src/utils.py +++ b/src/utils.py @@ -13,6 +13,18 @@ logger = logging.getLogger(__name__) +def _default_on_missing(charm: CharmBase, event: EventBase, **kwargs: Any) -> None: + logger.debug(f"Integration {kwargs.get('integration_name')} is missing.") + + +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 leader_unit(func: Callable) -> Callable: @wraps(func) def wrapper(charm: CharmBase, *args: Any, **kwargs: Any) -> Optional[Any]: @@ -41,7 +53,11 @@ def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: return wrapper -def validate_integration_exists(integration_name: str) -> Callable: +def validate_integration_exists( + integration_name: str, on_missing: Optional[Callable] = None +) -> Callable: + on_missing_request = on_missing or _default_on_missing + def decorator(func: Callable) -> Callable: @wraps(func) def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: @@ -49,12 +65,7 @@ def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: logger.debug(f"Handling event: {event}") if not charm.model.relations[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}" - ) + on_missing_request(charm, event, integration_name=integration_name) return None return func(charm, *args, **kwargs) @@ -85,6 +96,8 @@ def wrapper(charm: CharmBase, *args: EventBase, **kwargs: Any) -> Optional[Any]: 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.") + for attempt in Retrying( wait=wait_fixed(3), ): diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index de436db1..bc863f56 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -5,9 +5,10 @@ from constants import DATABASE_INTEGRATION_NAME, WORKLOAD_CONTAINER from ops.charm import CharmBase, HookEvent -from ops.model import BlockedStatus, WaitingStatus +from ops.model import ActiveStatus, BlockedStatus, WaitingStatus from ops.testing import Harness from utils import ( + block_on_missing, leader_unit, validate_container_connectivity, validate_database_resource, @@ -51,28 +52,40 @@ 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_when_database_relation_integrated( + def test_when_relation_exists_with_block_request( self, harness: Harness, database_relation: int, mocked_hook_event: MagicMock, ) -> None: - @validate_integration_exists(DATABASE_INTEGRATION_NAME) + @validate_integration_exists(DATABASE_INTEGRATION_NAME, on_missing=block_on_missing) def wrapped(charm: CharmBase, event: HookEvent) -> sentinel: return sentinel assert wrapped(harness.charm, mocked_hook_event) is sentinel - def test_when_database_relation_not_integrated( + def test_when_relation_not_exists_with_block_request( self, harness: Harness, mocked_hook_event: MagicMock ) -> None: - @validate_integration_exists(DATABASE_INTEGRATION_NAME) + @validate_integration_exists(DATABASE_INTEGRATION_NAME, on_missing=block_on_missing) def wrapped(charm: CharmBase, event: HookEvent) -> sentinel: return sentinel assert wrapped(harness.charm, mocked_hook_event) is None assert isinstance(harness.model.unit.status, BlockedStatus) + def test_when_relation_not_exists_without_request( + self, harness: Harness, mocked_hook_event: MagicMock + ) -> None: + harness.model.unit.status = ActiveStatus() + + @validate_integration_exists(DATABASE_INTEGRATION_NAME) + def wrapped(charm: CharmBase, event: HookEvent) -> sentinel: + return sentinel + + assert wrapped(harness.charm, mocked_hook_event) is None + assert isinstance(harness.model.unit.status, ActiveStatus) + def test_database_resource_created( self, harness: Harness, database_resource: MagicMock, mocked_hook_event: MagicMock ) -> None: