From 7b6812f38862fc9ffa4960d8505c2d0e7c07ae62 Mon Sep 17 00:00:00 2001 From: "Amanda H. L. de Andrade Katz" Date: Tue, 4 Jul 2023 09:23:34 -0300 Subject: [PATCH] feat(ISD-894): Add nginx-route integration (#12) --- .licenserc.yaml | 1 + .wokeignore | 1 + .../v0/nginx_route.py | 392 ++++++++++++++++++ metadata.yaml | 4 + requirements.txt | 2 +- src/charm.py | 10 + tests/integration/conftest.py | 33 +- tests/integration/test_charm.py | 30 +- 8 files changed, 467 insertions(+), 6 deletions(-) create mode 100644 .wokeignore create mode 100644 lib/charms/nginx_ingress_integrator/v0/nginx_route.py diff --git a/.licenserc.yaml b/.licenserc.yaml index 4e10e32d..4439f6fe 100644 --- a/.licenserc.yaml +++ b/.licenserc.yaml @@ -27,6 +27,7 @@ header: - '.trivyignore' - '.woke.yaml' - '.woke.yml' + - '.wokeignore' - 'CODEOWNERS' - 'icon.svg' - 'LICENSE' diff --git a/.wokeignore b/.wokeignore new file mode 100644 index 00000000..ec99ad85 --- /dev/null +++ b/.wokeignore @@ -0,0 +1 @@ +lib/charms/nginx_ingress_integrator/v0/nginx_route.py diff --git a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py new file mode 100644 index 00000000..fd2f54d6 --- /dev/null +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -0,0 +1,392 @@ +# Copyright 2023 Canonical Ltd. +# Licensed under the Apache2.0, see LICENCE file in charm source for details. +"""Library for the nginx-route relation. + +This library contains the require and provide functions for handling +the nginx-route interface. + +Import `require_nginx_route` in your charm, with four required keyword arguments: +- charm: (the charm itself) +- service_hostname +- service_name +- service_port + +Other optional arguments include: +- additional_hostnames +- backend_protocol +- limit_rps +- limit_whitelist +- max_body_size +- owasp_modsecurity_crs +- owasp_modsecurity_custom_rules +- path_routes +- retry_errors +- rewrite_target +- rewrite_enabled +- service_namespace +- session_cookie_max_age +- tls_secret_name + +See [the config section](https://charmhub.io/nginx-ingress-integrator/configure) for descriptions +of each, along with the required type. + +As an example, add the following to `src/charm.py`: +```python +from charms.nginx_ingress_integrator.v0.nginx_route import NginxRouteRequirer + +# In your charm's `__init__` method. +require_nginx_route( + charm=self, + service_hostname=self.config["external_hostname"], + service_name=self.app.name, + service_port=80 +) + +``` +And then add the following to `metadata.yaml`: +``` +requires: + nginx-route: + interface: nginx-route +``` +You _must_ require nginx route as part of the `__init__` method +rather than, for instance, a config-changed event handler, for the relation +changed event to be properly handled. +""" +import logging +import typing +import weakref + +import ops.charm +import ops.framework +import ops.model + +# The unique Charmhub library identifier, never change it +LIBID = "3c212b6ed3cf43dfbf9f2e322e634beb" + +# 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 = 2 + +__all__ = ["require_nginx_route", "provide_nginx_route"] + +logger = logging.getLogger(__name__) + + +class _NginxRouteAvailableEvent(ops.framework.EventBase): + """NginxRouteAvailableEvent custom event. + + This event indicates the nginx-route provider is available. + """ + + +class _NginxRouteBrokenEvent(ops.charm.RelationBrokenEvent): + """NginxRouteBrokenEvent custom event. + + This event indicates the nginx-route provider is broken. + """ + + +class _NginxRouteCharmEvents(ops.charm.CharmEvents): + """Custom charm events. + + Attrs: + nginx_route_available: Event to indicate that Nginx route relation is available. + nginx_route_broken: Event to indicate that Nginx route relation is broken. + """ + + nginx_route_available = ops.framework.EventSource(_NginxRouteAvailableEvent) + nginx_route_broken = ops.framework.EventSource(_NginxRouteBrokenEvent) + + +class _NginxRouteRequirer(ops.framework.Object): + """This class defines the functionality for the 'requires' side of the 'nginx-route' relation. + + Hook events observed: + - relation-changed + """ + + def __init__( + self, + charm: ops.charm.CharmBase, + config: typing.Dict[str, typing.Union[str, int, bool]], + nginx_route_relation_name: str = "nginx-route", + ): + """Init function for the NginxRouteRequires class. + + Args: + charm: The charm that requires the nginx-route relation. + config: Contains all the configuration options for nginx-route. + nginx_route_relation_name: Specifies the relation name of the relation handled by this + requirer class. The relation must have the nginx-route interface. + """ + super().__init__(charm, nginx_route_relation_name) + self._charm = charm + self._nginx_route_relation_name = nginx_route_relation_name + self._charm.framework.observe( + self._charm.on[self._nginx_route_relation_name].relation_changed, + self._config_reconciliation, + ) + # Set default values. + self._config: typing.Dict[str, typing.Union[str, int, bool]] = { + "service-namespace": self._charm.model.name, + **config, + } + self._config_reconciliation(None) + + def _config_reconciliation(self, _event: typing.Any = None) -> None: + """Update the nginx-route relation data to be exactly as defined by config.""" + if not self._charm.model.unit.is_leader(): + return + for relation in self._charm.model.relations[self._nginx_route_relation_name]: + relation_app_data = relation.data[self._charm.app] + delete_keys = { + relation_field + for relation_field in relation_app_data + if relation_field not in self._config + } + for delete_key in delete_keys: + del relation_app_data[delete_key] + relation_app_data.update({k: str(v) for k, v in self._config.items()}) + + +def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches + *, + charm: ops.charm.CharmBase, + service_hostname: str, + service_name: str, + service_port: int, + additional_hostnames: typing.Optional[str] = None, + backend_protocol: typing.Optional[str] = None, + limit_rps: typing.Optional[int] = None, + limit_whitelist: typing.Optional[str] = None, + max_body_size: typing.Optional[int] = None, + owasp_modsecurity_crs: typing.Optional[str] = None, + owasp_modsecurity_custom_rules: typing.Optional[str] = None, + path_routes: typing.Optional[str] = None, + retry_errors: typing.Optional[str] = None, + rewrite_target: typing.Optional[str] = None, + rewrite_enabled: typing.Optional[bool] = None, + service_namespace: typing.Optional[str] = None, + session_cookie_max_age: typing.Optional[int] = None, + tls_secret_name: typing.Optional[str] = None, + nginx_route_relation_name: str = "nginx-route", +) -> None: + """Set up nginx-route relation handlers on the requirer side. + + This function must be invoked in the charm class constructor. + + Args: + charm: The charm that requires the nginx-route relation. + service_hostname: configure Nginx ingress integrator + service-hostname option via relation. + service_name: configure Nginx ingress integrator service-name + option via relation. + service_port: configure Nginx ingress integrator service-port + option via relation. + additional_hostnames: configure Nginx ingress integrator + additional-hostnames option via relation, optional. + backend_protocol: configure Nginx ingress integrator + backend-protocol option via relation, optional. + limit_rps: configure Nginx ingress integrator limit-rps + option via relation, optional. + limit_whitelist: configure Nginx ingress integrator + limit-whitelist option via relation, optional. + max_body_size: configure Nginx ingress integrator + max-body-size option via relation, optional. + owasp_modsecurity_crs: configure Nginx ingress integrator + owasp-modsecurity-crs option via relation, optional. + owasp_modsecurity_custom_rules: configure Nginx ingress + integrator owasp-modsecurity-custom-rules option via + relation, optional. + path_routes: configure Nginx ingress integrator path-routes + option via relation, optional. + retry_errors: configure Nginx ingress integrator retry-errors + option via relation, optional. + rewrite_target: configure Nginx ingress integrator + rewrite-target option via relation, optional. + rewrite_enabled: configure Nginx ingress integrator + rewrite-enabled option via relation, optional. + service_namespace: configure Nginx ingress integrator + service-namespace option via relation, optional. + session_cookie_max_age: configure Nginx ingress integrator + session-cookie-max-age option via relation, optional. + tls_secret_name: configure Nginx ingress integrator + tls-secret-name option via relation, optional. + nginx_route_relation_name: Specifies the relation name of + the relation handled by this requirer class. The relation + must have the nginx-route interface. + """ + config: typing.Dict[str, typing.Union[str, int, bool]] = {} + if service_hostname is not None: + config["service-hostname"] = service_hostname + if service_name is not None: + config["service-name"] = service_name + if service_port is not None: + config["service-port"] = service_port + if additional_hostnames is not None: + config["additional-hostnames"] = additional_hostnames + if backend_protocol is not None: + config["backend-protocol"] = backend_protocol + if limit_rps is not None: + config["limit-rps"] = limit_rps + if limit_whitelist is not None: + config["limit-whitelist"] = limit_whitelist + if max_body_size is not None: + config["max-body-size"] = max_body_size + if owasp_modsecurity_crs is not None: + config["owasp-modsecurity-crs"] = owasp_modsecurity_crs + if owasp_modsecurity_custom_rules is not None: + config["owasp-modsecurity-custom-rules"] = owasp_modsecurity_custom_rules + if path_routes is not None: + config["path-routes"] = path_routes + if retry_errors is not None: + config["retry-errors"] = retry_errors + if rewrite_target is not None: + config["rewrite-target"] = rewrite_target + if rewrite_enabled is not None: + config["rewrite-enabled"] = rewrite_enabled + if service_namespace is not None: + config["service-namespace"] = service_namespace + if session_cookie_max_age is not None: + config["session-cookie-max-age"] = session_cookie_max_age + if tls_secret_name is not None: + config["tls-secret-name"] = tls_secret_name + + _NginxRouteRequirer( + charm=charm, config=config, nginx_route_relation_name=nginx_route_relation_name + ) + + +class _NginxRouteProvider(ops.framework.Object): + """Class containing the functionality for the 'provides' side of the 'nginx-route' relation. + + Attrs: + on: nginx-route relation event describer. + + Hook events observed: + - relation-changed + """ + + on = _NginxRouteCharmEvents() + + def __init__( + self, + charm: ops.charm.CharmBase, + nginx_route_relation_name: str = "nginx-route", + ): + """Init function for the NginxRouterProvides class. + + Args: + charm: The charm that provides the nginx-route relation. + nginx_route_relation_name: Specifies the relation name of the relation handled by this + provider class. The relation must have the nginx-route interface. + """ + # Observe the relation-changed hook event and bind + # self.on_relation_changed() to handle the event. + super().__init__(charm, nginx_route_relation_name) + self._charm = charm + self._charm.framework.observe( + self._charm.on[nginx_route_relation_name].relation_changed, self._on_relation_changed + ) + self._charm.framework.observe( + self._charm.on[nginx_route_relation_name].relation_broken, self._on_relation_broken + ) + + def _on_relation_changed(self, event: ops.charm.RelationChangedEvent) -> None: + """Handle a change to the nginx-route relation. + + Confirm we have the fields we expect to receive. + + Args: + event: Event triggering the relation-changed hook for the relation. + """ + # `self.unit` isn't available here, so use `self.model.unit`. + if not self._charm.model.unit.is_leader(): + return + + relation_name = event.relation.name + remote_app = event.app + if remote_app is None: + raise RuntimeError("_on_relation_changed was triggered by a broken relation.") + + if not event.relation.data[remote_app]: + logger.info( + "%s hasn't finished configuring, waiting until the relation data is populated.", + relation_name, + ) + return + + required_fields = {"service-hostname", "service-port", "service-name"} + missing_fields = sorted( + field + for field in required_fields + if event.relation.data[remote_app].get(field) is None + ) + if missing_fields: + logger.warning( + "Missing required data fields for %s relation: %s", + relation_name, + ", ".join(missing_fields), + ) + self._charm.model.unit.status = ops.model.BlockedStatus( + f"Missing fields for {relation_name}: {', '.join(missing_fields)}" + ) + return + + # Create an event that our charm can use to decide it's okay to + # configure the Kubernetes Nginx ingress resources. + self.on.nginx_route_available.emit() + + def _on_relation_broken(self, event: ops.charm.RelationBrokenEvent) -> None: + """Handle a relation-broken event in the nginx-route relation. + + Args: + event: Event triggering the relation-broken hook for the relation. + """ + if not self._charm.model.unit.is_leader(): + return + + # Create an event that our charm can use to remove the Kubernetes Nginx ingress resources. + self.on.nginx_route_broken.emit(event.relation) + + +# This is here only to maintain a reference to the instance of NginxRouteProvider created by +# the provide_nginx_route function. This is required for ops framework event handling to work. +# The provider instance will have the same lifetime as the charm that creates it. +__provider_references: weakref.WeakKeyDictionary = weakref.WeakKeyDictionary() + + +def provide_nginx_route( + charm: ops.charm.CharmBase, + on_nginx_route_available: typing.Callable, + on_nginx_route_broken: typing.Callable, + nginx_route_relation_name: str = "nginx-route", +) -> None: + """Set up nginx-route relation handlers on the provider side. + + This function must be invoked in the charm class constructor. + + Args: + charm: The charm that requires the nginx-route relation. + on_nginx_route_available: Callback function for the nginx-route-available event. + on_nginx_route_broken: Callback function for the nginx-route-broken event. + nginx_route_relation_name: Specifies the relation name of the relation handled by this + provider class. The relation must have the nginx-route interface. + """ + if __provider_references.get(charm, {}).get(nginx_route_relation_name) is not None: + raise RuntimeError( + "provide_nginx_route was invoked twice with the same nginx-route relation name" + ) + provider = _NginxRouteProvider( + charm=charm, nginx_route_relation_name=nginx_route_relation_name + ) + if charm in __provider_references: + __provider_references[charm][nginx_route_relation_name] = provider + else: + __provider_references[charm] = {nginx_route_relation_name: provider} + charm.framework.observe(provider.on.nginx_route_available, on_nginx_route_available) + charm.framework.observe(provider.on.nginx_route_broken, on_nginx_route_broken) diff --git a/metadata.yaml b/metadata.yaml index 9536a980..53797d50 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -38,3 +38,7 @@ requires: interface: ingress limit: 1 optional: true + nginx-route: + interface: nginx-route + limit: 1 + optional: true diff --git a/requirements.txt b/requirements.txt index a21971e6..7428b9d1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,3 @@ jsonschema >= 4.17 ops >= 2.2.0 -pydantic >= 1.10 +pydantic==1.10.10 diff --git a/src/charm.py b/src/charm.py index aec02b66..d1180c57 100755 --- a/src/charm.py +++ b/src/charm.py @@ -9,6 +9,7 @@ from typing import Any, Dict import ops +from charms.nginx_ingress_integrator.v0.nginx_route import require_nginx_route from charms.traefik_k8s.v1.ingress import IngressPerAppRequirer from ops.charm import ActionEvent from ops.main import main @@ -43,6 +44,15 @@ def __init__(self, *args: Any) -> None: self.model.unit.status = ops.BlockedStatus(exc.msg) return self._synapse = Synapse(charm_state=self._charm_state) + # service-hostname is a required field so we're hardcoding to the same + # value as service-name. service-hostname should be set via Nginx + # Ingress Integrator charm config. + require_nginx_route( + charm=self, + service_hostname=self.app.name, + service_name=self.app.name, + service_port=SYNAPSE_PORT, + ) self._ingress = IngressPerAppRequirer( self, port=SYNAPSE_PORT, diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index f346c63e..94fec330 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -48,23 +48,28 @@ def synapse_image_fixture(pytestconfig: Config): return synapse_image +@pytest_asyncio.fixture(scope="module", name="synapse_app_name") +def synapse_app_name_fixture() -> str: + """Get Synapse application name.""" + return "synapse" + + @pytest_asyncio.fixture(scope="module", name="synapse_app") async def synapse_app_fixture( + synapse_app_name: str, synapse_image: str, model: Model, server_name: str, synapse_charm: str, ): """Build and deploy the Synapse charm.""" - app_name = "synapse-k8s" - resources = { "synapse-image": synapse_image, } app = await model.deploy( synapse_charm, resources=resources, - application_name=app_name, + application_name=synapse_app_name, series="jammy", config={"server_name": server_name}, ) @@ -129,6 +134,28 @@ async def traefik_app_fixture( return app +@pytest.fixture(scope="module", name="nginx_integrator_app_name") +def nginx_integrator_app_name_fixture() -> str: + """Return the name of the nginx integrator application deployed for tests.""" + return "nginx-ingress-integrator" + + +@pytest_asyncio.fixture(scope="module", name="nginx_integrator_app") +async def nginx_integrator_app_fixture( + model: Model, + synapse_app, # pylint: disable=unused-argument + nginx_integrator_app_name: str, +): + """Deploy nginx-ingress-integrator.""" + app = await model.deploy( + "nginx-ingress-integrator", + application_name=nginx_integrator_app_name, + trust=True, + ) + await model.wait_for_idle(raise_on_blocked=True) + return app + + @pytest_asyncio.fixture(scope="function", name="another_synapse_app") async def another_synapse_app_fixture( model: Model, synapse_app: Application, server_name: str, another_server_name: str diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 8d9b6e7f..5681a111 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -18,6 +18,9 @@ # caused by pytest fixtures # pylint: disable=too-many-arguments +# mypy has trouble to inferred types for variables that are initialized in subclasses. +ACTIVE_STATUS_NAME = typing.cast(str, ActiveStatus.name) # type: ignore + logger = logging.getLogger(__name__) @@ -51,8 +54,7 @@ async def test_with_ingress( assert: requesting the charm through Traefik should return a correct response. """ await model.add_relation(synapse_app.name, traefik_app_name) - # mypy doesn't see that ActiveStatus has a name - await model.wait_for_idle(status=ActiveStatus.name) # type: ignore + await model.wait_for_idle(status=ACTIVE_STATUS_NAME) traefik_ip = (await get_unit_ips(traefik_app_name))[0] response = requests.get( @@ -64,6 +66,30 @@ async def test_with_ingress( assert "Welcome to the Matrix" in response.text +async def test_with_nginx_route( + model: Model, + synapse_app_name: str, + synapse_app: Application, # pylint: disable=unused-argument + nginx_integrator_app, # pylint: disable=unused-argument + nginx_integrator_app_name: str, +): + """ + arrange: build and deploy the Synapse charm, and deploy the nginx-integrator. + act: relate the nginx-integrator charm with the Synapse charm. + assert: requesting the charm through nginx-integrator should return a correct response. + """ + await model.add_relation( + f"{synapse_app_name}:nginx-route", f"{nginx_integrator_app_name}:nginx-route" + ) + await model.wait_for_idle(status=ACTIVE_STATUS_NAME) + + response = requests.get( + "http://127.0.0.1/_matrix/static/", headers={"Host": synapse_app_name}, timeout=5 + ) + assert response.status_code == 200 + assert "Welcome to the Matrix" in response.text + + async def test_server_name_changed(model: Model, another_synapse_app: Application): """ arrange: build and deploy the Synapse charm.