From 8fc134fcc81a406e0b025da4d0bfa24321a5e3ab Mon Sep 17 00:00:00 2001 From: Guillaume Boutry Date: Tue, 26 Sep 2023 17:31:02 +0200 Subject: [PATCH] Migrate traefik ingress to v2 + bind charm rename Current ingress relation only routes unit to requirer's leader. Ingress V2 fixes that issue. bind9-k8s has been renamed to designate-bind-k8s, update the libraries (and follow up). Change-Id: I692acfde39e7f545bb57e8a160fe4f6162c2b41e --- charms/designate-k8s/.zuul.yaml | 2 +- charms/designate-k8s/charmcraft.yaml | 1 + charms/designate-k8s/fetch-libs.sh | 4 +- .../v0/bind_rndc.py | 92 ++-- .../charms/traefik_k8s/{v1 => v2}/ingress.py | 477 ++++++++++++------ charms/designate-k8s/requirements.txt | 1 + charms/designate-k8s/src/charm.py | 67 ++- charms/designate-k8s/tests/bundles/smoke.yaml | 10 +- charms/designate-k8s/tests/tests.yaml | 2 +- .../tests/unit/test_designate_charm.py | 10 +- 10 files changed, 442 insertions(+), 224 deletions(-) rename charms/designate-k8s/lib/charms/{bind9_k8s => designate_bind_k8s}/v0/bind_rndc.py (83%) rename charms/designate-k8s/lib/charms/traefik_k8s/{v1 => v2}/ingress.py (53%) diff --git a/charms/designate-k8s/.zuul.yaml b/charms/designate-k8s/.zuul.yaml index c72c87f9..26a82ce0 100644 --- a/charms/designate-k8s/.zuul.yaml +++ b/charms/designate-k8s/.zuul.yaml @@ -7,5 +7,5 @@ charm_build_name: designate-k8s juju_channel: 3.1/stable juju_classic_mode: false - microk8s_channel: 1.26-strict/stable + microk8s_channel: 1.28-strict/stable microk8s_classic_mode: false diff --git a/charms/designate-k8s/charmcraft.yaml b/charms/designate-k8s/charmcraft.yaml index ac49568b..2fdc318d 100644 --- a/charms/designate-k8s/charmcraft.yaml +++ b/charms/designate-k8s/charmcraft.yaml @@ -26,5 +26,6 @@ parts: charm-binary-python-packages: - cryptography - jsonschema + - pydantic<2.0 - jinja2 - git+https://opendev.org/openstack/charm-ops-sunbeam#egg=ops_sunbeam diff --git a/charms/designate-k8s/fetch-libs.sh b/charms/designate-k8s/fetch-libs.sh index 4e7ebe85..5cdf501d 100755 --- a/charms/designate-k8s/fetch-libs.sh +++ b/charms/designate-k8s/fetch-libs.sh @@ -4,6 +4,6 @@ echo "INFO: Fetching libs from charmhub." charmcraft fetch-lib charms.data_platform_libs.v0.database_requires charmcraft fetch-lib charms.keystone_k8s.v1.identity_service charmcraft fetch-lib charms.rabbitmq_k8s.v0.rabbitmq -charmcraft fetch-lib charms.traefik_k8s.v1.ingress -charmcraft fetch-lib charms.bind9_k8s.v0.bind_rndc +charmcraft fetch-lib charms.traefik_k8s.v2.ingress +charmcraft fetch-lib charms.designate_bind_k8s.v0.bind_rndc diff --git a/charms/designate-k8s/lib/charms/bind9_k8s/v0/bind_rndc.py b/charms/designate-k8s/lib/charms/designate_bind_k8s/v0/bind_rndc.py similarity index 83% rename from charms/designate-k8s/lib/charms/bind9_k8s/v0/bind_rndc.py rename to charms/designate-k8s/lib/charms/designate_bind_k8s/v0/bind_rndc.py index f0fa68db..2bb2f93e 100644 --- a/charms/designate-k8s/lib/charms/bind9_k8s/v0/bind_rndc.py +++ b/charms/designate-k8s/lib/charms/designate_bind_k8s/v0/bind_rndc.py @@ -13,7 +13,7 @@ Two events are also available to respond to: - goneaway A basic example showing the usage of this relation follows: ``` -from charms.bind9_k8s.v0.bind_rndc import ( +from charms.designate_bind_k8s.v0.bind_rndc import ( BindRndcRequires ) class BindRndcClientCharm(CharmBase): @@ -31,6 +31,13 @@ class BindRndcClientCharm(CharmBase): self.bind_rndc.on.goneaway, self._on_bind_rndc_goneaway ) + def _on_bind_rndc_connected(self, event): + '''React to the Bind Rndc Connected event. + This event happens when BindRndc relation is added to the + model. + ''' + # Request the rndc key from the Bind Rndc relation. + self.bind_rndc.request_rndc_key("generated nonce") def _on_bind_rndc_ready(self, event): '''React to the Bind Rndc Ready event. This event happens when BindRndc relation is added to the @@ -49,7 +56,6 @@ class BindRndcClientCharm(CharmBase): import json import logging -import secrets from typing import ( Any, Dict, @@ -63,14 +69,41 @@ import ops logger = logging.getLogger(__name__) # The unique Charmhub library identifier, never change it -LIBID = "0fb2f64f2a1344feb80044cee22ef3a8" +LIBID = "1cb766c981874e7383d17cf54148b3d4" # 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 = 3 +LIBPATCH = 1 + + +class BindRndcConnectedEvent(ops.EventBase): + """Bind rndc connected event.""" + + def __init__( + self, + handle: ops.Handle, + relation_id: int, + relation_name: str, + ): + super().__init__(handle) + self.relation_id = relation_id + self.relation_name = relation_name + + def snapshot(self) -> dict: + """Return snapshot data that should be persisted.""" + return { + "relation_id": self.relation_id, + "relation_name": self.relation_name, + } + + def restore(self, snapshot: Dict[str, Any]): + """Restore the value state from a given snapshot.""" + super().restore(snapshot) + self.relation_id = snapshot["relation_id"] + self.relation_name = snapshot["relation_name"] class BindRndcReadyEvent(ops.EventBase): @@ -81,22 +114,16 @@ class BindRndcReadyEvent(ops.EventBase): handle: ops.Handle, relation_id: int, relation_name: str, - algorithm: str, - secret: str, ): super().__init__(handle) self.relation_id = relation_id self.relation_name = relation_name - self.algorithm = algorithm - self.secret = secret def snapshot(self) -> dict: """Return snapshot data that should be persisted.""" return { "relation_id": self.relation_id, "relation_name": self.relation_name, - "algorithm": self.algorithm, - "secret": self.secret, } def restore(self, snapshot: Dict[str, Any]): @@ -104,8 +131,6 @@ class BindRndcReadyEvent(ops.EventBase): super().restore(snapshot) self.relation_id = snapshot["relation_id"] self.relation_name = snapshot["relation_name"] - self.algorithm = snapshot["algorithm"] - self.secret = snapshot["secret"] class BindRndcGoneAwayEvent(ops.EventBase): @@ -117,7 +142,8 @@ class BindRndcGoneAwayEvent(ops.EventBase): class BindRndcRequirerEvents(ops.ObjectEvents): """List of events that the BindRndc requires charm can leverage.""" - bind_rndc_ready = ops.EventSource(BindRndcReadyEvent) + connected = ops.EventSource(BindRndcConnectedEvent) + ready = ops.EventSource(BindRndcReadyEvent) goneaway = ops.EventSource(BindRndcGoneAwayEvent) @@ -125,13 +151,11 @@ class BindRndcRequires(ops.Object): """Class to be instantiated by the requiring side of the relation.""" on = BindRndcRequirerEvents() - _stored = ops.StoredState() def __init__(self, charm: ops.CharmBase, relation_name: str): super().__init__(charm, relation_name) self.charm = charm self.relation_name = relation_name - self._stored.set_default(nonce="") self.framework.observe( self.charm.on[relation_name].relation_joined, self._on_relation_joined, @@ -147,24 +171,20 @@ class BindRndcRequires(ops.Object): def _on_relation_joined(self, event: ops.RelationJoinedEvent): """Handle relation joined event.""" - self._request_rndc_key(event.relation) + self.on.connected.emit( + event.relation.id, + event.relation.name, + ) def _on_relation_changed(self, event: ops.RelationJoinedEvent): """Handle relation changed event.""" host = self.host(event.relation) rndc_key = self.get_rndc_key(event.relation) - if rndc_key is None: - self._request_rndc_key(event.relation) - return - if host is not None: - algorithm = rndc_key["algorithm"] - secret = rndc_key["secret"] - self.on.bind_rndc_ready.emit( + if all((host, rndc_key)): + self.on.ready.emit( event.relation.id, event.relation.name, - algorithm, - secret, ) def _on_relation_broken(self, event: ops.RelationBrokenEvent): @@ -177,33 +197,25 @@ class BindRndcRequires(ops.Object): return None return relation.data[relation.app].get("host") - def nonce(self) -> str: - """Return nonce from stored state.""" - return self._stored.nonce + def nonce(self, relation: ops.Relation) -> Optional[str]: + """Return nonce from relation.""" + return relation.data[self.charm.unit].get("nonce") def get_rndc_key(self, relation: ops.Relation) -> Optional[dict]: """Get rndc keys.""" if relation.app is None: return None - if self._stored.nonce == "": + if self.nonce(relation) is None: logger.debug("No nonce set for unit yet") return None return json.loads( relation.data[relation.app].get("rndc_keys", "{}") - ).get(self._stored.nonce) + ).get(self.nonce(relation)) - def _request_rndc_key(self, relation: ops.Relation): + def request_rndc_key(self, relation: ops.Relation, nonce: str): """Request rndc key over the relation.""" - if self._stored.nonce == "": - self._stored.nonce = secrets.token_hex(16) - relation.data[self.charm.unit]["nonce"] = self._stored.nonce - - def reconcile_rndc_key(self, relation: ops.Relation): - """Reconcile rndc key over the relation.""" - if self._stored.nonce != relation.data[self.charm.unit].get("nonce"): - self._stored.nonce = secrets.token_hex(16) - relation.data[self.charm.unit]["nonce"] = self._stored.nonce + relation.data[self.charm.unit]["nonce"] = nonce class NewBindClientAttachedEvent(ops.EventBase): diff --git a/charms/designate-k8s/lib/charms/traefik_k8s/v1/ingress.py b/charms/designate-k8s/lib/charms/traefik_k8s/v2/ingress.py similarity index 53% rename from charms/designate-k8s/lib/charms/traefik_k8s/v1/ingress.py rename to charms/designate-k8s/lib/charms/traefik_k8s/v2/ingress.py index e393fb52..0364c8ab 100644 --- a/charms/designate-k8s/lib/charms/traefik_k8s/v1/ingress.py +++ b/charms/designate-k8s/lib/charms/traefik_k8s/v2/ingress.py @@ -1,4 +1,4 @@ -# Copyright 2022 Canonical Ltd. +# Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. r"""# Interface Library for ingress. @@ -28,7 +28,7 @@ requires: Then, to initialise the library: ```python -from charms.traefik_k8s.v1.ingress import (IngressPerAppRequirer, +from charms.traefik_k8s.v2.ingress import (IngressPerAppRequirer, IngressPerAppReadyEvent, IngressPerAppRevokedEvent) class SomeCharm(CharmBase): @@ -50,105 +50,181 @@ class SomeCharm(CharmBase): def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent): logger.info("This app no longer has ingress") """ - +import json import logging import socket import typing -from typing import Any, Dict, Optional, Tuple, Union +from dataclasses import dataclass +from typing import ( + Any, + Dict, + List, + MutableMapping, + Optional, + Sequence, + Tuple, +) -import yaml +import pydantic from ops.charm import CharmBase, RelationBrokenEvent, RelationEvent from ops.framework import EventSource, Object, ObjectEvents, StoredState -from ops.model import ModelError, Relation +from ops.model import ModelError, Relation, Unit +from pydantic import AnyHttpUrl, BaseModel, Field, validator # The unique Charmhub library identifier, never change it LIBID = "e6de2a5cd5b34422a204668f3b8f90d2" # Increment this major API version when introducing breaking changes -LIBAPI = 1 +LIBAPI = 2 # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 15 +LIBPATCH = 6 + +PYDEPS = ["pydantic<2.0"] DEFAULT_RELATION_NAME = "ingress" RELATION_INTERFACE = "ingress" log = logging.getLogger(__name__) +BUILTIN_JUJU_KEYS = {"ingress-address", "private-address", "egress-subnets"} -try: - import jsonschema - DO_VALIDATION = True -except ModuleNotFoundError: - log.warning( - "The `ingress` library needs the `jsonschema` package to be able " - "to do runtime data validation; without it, it will still work but validation " - "will be disabled. \n" - "It is recommended to add `jsonschema` to the 'requirements.txt' of your charm, " - "which will enable this feature." +class DatabagModel(BaseModel): + """Base databag model.""" + + class Config: + """Pydantic config.""" + + allow_population_by_field_name = True + """Allow instantiating this class by field name (instead of forcing alias).""" + + _NEST_UNDER = None + + @classmethod + def load(cls, databag: MutableMapping): + """Load this model from a Juju databag.""" + if cls._NEST_UNDER: + return cls.parse_obj(json.loads(databag[cls._NEST_UNDER])) + + try: + data = {k: json.loads(v) for k, v in databag.items() if k not in BUILTIN_JUJU_KEYS} + except json.JSONDecodeError as e: + msg = f"invalid databag contents: expecting json. {databag}" + log.error(msg) + raise DataValidationError(msg) from e + + try: + return cls.parse_raw(json.dumps(data)) # type: ignore + except pydantic.ValidationError as e: + msg = f"failed to validate databag: {databag}" + log.error(msg, exc_info=True) + raise DataValidationError(msg) from e + + def dump(self, databag: Optional[MutableMapping] = None, clear: bool = True): + """Write the contents of this model to Juju databag. + + :param databag: the databag to write the data to. + :param clear: ensure the databag is cleared before writing it. + """ + if clear and databag: + databag.clear() + + if databag is None: + databag = {} + + if self._NEST_UNDER: + databag[self._NEST_UNDER] = self.json() + + dct = self.dict() + for key, field in self.__fields__.items(): # type: ignore + value = dct[key] + databag[field.alias or key] = json.dumps(value) + + return databag + + +# todo: import these models from charm-relation-interfaces/ingress/v2 instead of redeclaring them +class IngressUrl(BaseModel): + """Ingress url schema.""" + + url: AnyHttpUrl + + +class IngressProviderAppData(DatabagModel): + """Ingress application databag schema.""" + + ingress: IngressUrl + + +class ProviderSchema(BaseModel): + """Provider schema for Ingress.""" + + app: IngressProviderAppData + + +class IngressRequirerAppData(DatabagModel): + """Ingress requirer application databag model.""" + + model: str = Field(description="The model the application is in.") + name: str = Field(description="the name of the app requesting ingress.") + port: int = Field(description="The port the app wishes to be exposed.") + + # fields on top of vanilla 'ingress' interface: + strip_prefix: Optional[bool] = Field( + description="Whether to strip the prefix from the ingress url.", alias="strip-prefix" + ) + redirect_https: Optional[bool] = Field( + description="Whether to redirect http traffic to https.", alias="redirect-https" ) - DO_VALIDATION = False -INGRESS_REQUIRES_APP_SCHEMA = { - "type": "object", - "properties": { - "model": {"type": "string"}, - "name": {"type": "string"}, - "host": {"type": "string"}, - "port": {"type": "string"}, - "strip-prefix": {"type": "string"}, - "redirect-https": {"type": "string"}, - }, - "required": ["model", "name", "host", "port"], -} + scheme: Optional[str] = Field( + default="http", description="What scheme to use in the generated ingress url" + ) -INGRESS_PROVIDES_APP_SCHEMA = { - "type": "object", - "properties": { - "ingress": {"type": "object", "properties": {"url": {"type": "string"}}}, - }, - "required": ["ingress"], -} + @validator("scheme", pre=True) + def validate_scheme(cls, scheme): # noqa: N805 # pydantic wants 'cls' as first arg + """Validate scheme arg.""" + if scheme not in {"http", "https", "h2c"}: + raise ValueError("invalid scheme: should be one of `http|https|h2c`") + return scheme -try: - from typing import TypedDict -except ImportError: - from typing_extensions import TypedDict # py35 compatibility - -# Model of the data a unit implementing the requirer will need to provide. -RequirerData = TypedDict( - "RequirerData", - { - "model": str, - "name": str, - "host": str, - "port": int, - "strip-prefix": bool, - "redirect-https": bool, - }, - total=False, -) -# Provider ingress data model. -ProviderIngressData = TypedDict("ProviderIngressData", {"url": str}) -# Provider application databag model. -ProviderApplicationData = TypedDict("ProviderApplicationData", {"ingress": ProviderIngressData}) # type: ignore + @validator("port", pre=True) + def validate_port(cls, port): # noqa: N805 # pydantic wants 'cls' as first arg + """Validate port.""" + assert isinstance(port, int), type(port) + assert 0 < port < 65535, "port out of TCP range" + return port -def _validate_data(data, schema): - """Checks whether `data` matches `schema`. +class IngressRequirerUnitData(DatabagModel): + """Ingress requirer unit databag model.""" - Will raise DataValidationError if the data is not valid, else return None. - """ - if not DO_VALIDATION: - return - try: - jsonschema.validate(instance=data, schema=schema) - except jsonschema.ValidationError as e: - raise DataValidationError(data, schema) from e + host: str = Field(description="Hostname the unit wishes to be exposed.") + + @validator("host", pre=True) + def validate_host(cls, host): # noqa: N805 # pydantic wants 'cls' as first arg + """Validate host.""" + assert isinstance(host, str), type(host) + return host -class DataValidationError(RuntimeError): +class RequirerSchema(BaseModel): + """Requirer schema for Ingress.""" + + app: IngressRequirerAppData + unit: IngressRequirerUnitData + + +class IngressError(RuntimeError): + """Base class for custom errors raised by this library.""" + + +class NotReadyError(IngressError): + """Raised when a relation is not ready.""" + + +class DataValidationError(IngressError): """Raised when data validation fails on IPU relation data.""" @@ -156,7 +232,7 @@ class _IngressPerAppBase(Object): """Base class for IngressPerUnit interface classes.""" def __init__(self, charm: CharmBase, relation_name: str = DEFAULT_RELATION_NAME): - super().__init__(charm, relation_name + "_V1") + super().__init__(charm, relation_name) self.charm: CharmBase = charm self.relation_name = relation_name @@ -234,13 +310,13 @@ class _IPAEvent(RelationEvent): class IngressPerAppDataProvidedEvent(_IPAEvent): """Event representing that ingress data has been provided for an app.""" - __args__ = ("name", "model", "port", "host", "strip_prefix", "redirect_https") + __args__ = ("name", "model", "hosts", "strip_prefix", "redirect_https") if typing.TYPE_CHECKING: name: Optional[str] = None model: Optional[str] = None - port: Optional[str] = None - host: Optional[str] = None + # sequence of hostname, port dicts + hosts: Sequence["IngressRequirerUnitData"] = () strip_prefix: bool = False redirect_https: bool = False @@ -256,12 +332,32 @@ class IngressPerAppProviderEvents(ObjectEvents): data_removed = EventSource(IngressPerAppDataRemovedEvent) +@dataclass +class IngressRequirerData: + """Data exposed by the ingress requirer to the provider.""" + + app: "IngressRequirerAppData" + units: List["IngressRequirerUnitData"] + + +class TlsProviderType(typing.Protocol): + """Placeholder.""" + + @property + def enabled(self) -> bool: # type: ignore + """Placeholder.""" + + class IngressPerAppProvider(_IngressPerAppBase): """Implementation of the provider of ingress.""" on = IngressPerAppProviderEvents() # type: ignore - def __init__(self, charm: CharmBase, relation_name: str = DEFAULT_RELATION_NAME): + def __init__( + self, + charm: CharmBase, + relation_name: str = DEFAULT_RELATION_NAME, + ): """Constructor for IngressPerAppProvider. Args: @@ -275,15 +371,14 @@ class IngressPerAppProvider(_IngressPerAppBase): # created, joined or changed: if remote side has sent the required data: # notify listeners. if self.is_ready(event.relation): - data = self._get_requirer_data(event.relation) + data = self.get_data(event.relation) self.on.data_provided.emit( # type: ignore event.relation, - data["name"], - data["model"], - data["port"], - data["host"], - data.get("strip-prefix", False), - data.get("redirect-https", False), + data.app.name, + data.app.model, + [unit.dict() for unit in data.units], + data.app.strip_prefix or False, + data.app.redirect_https or False, ) def _handle_relation_broken(self, event): @@ -303,32 +398,39 @@ class IngressPerAppProvider(_IngressPerAppBase): return del relation.data[self.app]["ingress"] - def _get_requirer_data(self, relation: Relation) -> RequirerData: # type: ignore - """Fetch and validate the requirer's app databag. + def _get_requirer_units_data(self, relation: Relation) -> List["IngressRequirerUnitData"]: + """Fetch and validate the requirer's app databag.""" + out: List["IngressRequirerUnitData"] = [] - For convenience, we convert 'port' to integer. - """ - if not relation.app or not relation.app.name: # type: ignore - # Handle edge case where remote app name can be missing, e.g., - # relation_broken events. - # FIXME https://github.com/canonical/traefik-k8s-operator/issues/34 - return {} + unit: Unit + for unit in relation.units: + databag = relation.data[unit] + try: + data = IngressRequirerUnitData.load(databag) + out.append(data) + except pydantic.ValidationError: + log.info(f"failed to validate remote unit data for {unit}") + raise + return out - databag = relation.data[relation.app] - remote_data: Dict[str, Union[int, str]] = {} - for k in ("port", "host", "model", "name", "mode", "strip-prefix", "redirect-https"): - v = databag.get(k) - if v is not None: - remote_data[k] = v - _validate_data(remote_data, INGRESS_REQUIRES_APP_SCHEMA) - remote_data["port"] = int(remote_data["port"]) - remote_data["strip-prefix"] = bool(remote_data.get("strip-prefix", "false") == "true") - remote_data["redirect-https"] = bool(remote_data.get("redirect-https", "false") == "true") - return typing.cast(RequirerData, remote_data) + @staticmethod + def _get_requirer_app_data(relation: Relation) -> "IngressRequirerAppData": + """Fetch and validate the requirer's app databag.""" + app = relation.app + if app is None: + raise NotReadyError(relation) - def get_data(self, relation: Relation) -> RequirerData: # type: ignore - """Fetch the remote app's databag, i.e. the requirer data.""" - return self._get_requirer_data(relation) + databag = relation.data[app] + return IngressRequirerAppData.load(databag) + + def get_data(self, relation: Relation) -> IngressRequirerData: + """Fetch the remote (requirer) app and units' databags.""" + try: + return IngressRequirerData( + self._get_requirer_app_data(relation), self._get_requirer_units_data(relation) + ) + except (pydantic.ValidationError, DataValidationError) as e: + raise DataValidationError("failed to validate ingress requirer data") from e def is_ready(self, relation: Optional[Relation] = None): """The Provider is ready if the requirer has sent valid data.""" @@ -336,38 +438,35 @@ class IngressPerAppProvider(_IngressPerAppBase): return any(map(self.is_ready, self.relations)) try: - return bool(self._get_requirer_data(relation)) - except DataValidationError as e: - log.warning("Requirer not ready; validation error encountered: %s" % str(e)) + self.get_data(relation) + except (DataValidationError, NotReadyError) as e: + log.debug("Provider not ready; validation error encountered: %s" % str(e)) return False + return True - def _provided_url(self, relation: Relation) -> ProviderIngressData: # type: ignore + def _published_url(self, relation: Relation) -> Optional["IngressProviderAppData"]: """Fetch and validate this app databag; return the ingress url.""" - if not relation.app or not relation.app.name or not self.unit.is_leader(): # type: ignore + if not self.is_ready(relation) or not self.unit.is_leader(): # Handle edge case where remote app name can be missing, e.g., # relation_broken events. # Also, only leader units can read own app databags. # FIXME https://github.com/canonical/traefik-k8s-operator/issues/34 - return typing.cast(ProviderIngressData, {}) # noqa + return None # fetch the provider's app databag - raw_data = relation.data[self.app].get("ingress") - if not raw_data: - raise RuntimeError("This application did not `publish_url` yet.") + databag = relation.data[self.app] + if not databag.get("ingress"): + raise NotReadyError("This application did not `publish_url` yet.") - ingress: ProviderIngressData = yaml.safe_load(raw_data) - _validate_data({"ingress": ingress}, INGRESS_PROVIDES_APP_SCHEMA) - return ingress + return IngressProviderAppData.load(databag) def publish_url(self, relation: Relation, url: str): """Publish to the app databag the ingress url.""" - ingress = {"url": url} - ingress_data = {"ingress": ingress} - _validate_data(ingress_data, INGRESS_PROVIDES_APP_SCHEMA) - relation.data[self.app]["ingress"] = yaml.safe_dump(ingress) + ingress_url = {"url": url} + IngressProviderAppData.parse_obj({"ingress": ingress_url}).dump(relation.data[self.app]) @property - def proxied_endpoints(self): + def proxied_endpoints(self) -> Dict[str, str]: """Returns the ingress settings provided to applications by this IngressPerAppProvider. For example, when this IngressPerAppProvider has provided the @@ -385,11 +484,25 @@ class IngressPerAppProvider(_IngressPerAppBase): results = {} for ingress_relation in self.relations: - assert ( - ingress_relation.app - ), "no app in relation (shouldn't happen)" # for type checker - results[ingress_relation.app.name] = self._provided_url(ingress_relation) + if not ingress_relation.app: + log.warning( + f"no app in relation {ingress_relation} when fetching proxied endpoints: skipping" + ) + continue + try: + ingress_data = self._published_url(ingress_relation) + except NotReadyError: + log.warning( + f"no published url found in {ingress_relation}: " + f"traefik didn't publish_url yet to this relation." + ) + continue + if not ingress_data: + log.warning(f"relation {ingress_relation} not ready yet: try again in some time.") + continue + + results[ingress_relation.app.name] = ingress_data.ingress.dict() return results @@ -430,6 +543,9 @@ class IngressPerAppRequirer(_IngressPerAppBase): port: Optional[int] = None, strip_prefix: bool = False, redirect_https: bool = False, + # fixme: this is horrible UX. + # shall we switch to manually calling provide_ingress_requirements with all args when ready? + scheme: typing.Callable[[], str] = lambda: "http", ): """Constructor for IngressRequirer. @@ -445,7 +561,8 @@ class IngressPerAppRequirer(_IngressPerAppBase): host: Hostname to be used by the ingress provider to address the requiring application; if unspecified, the default Kubernetes service name will be used. strip_prefix: configure Traefik to strip the path prefix. - redirect_https: redirect incoming requests to the HTTPS. + redirect_https: redirect incoming requests to HTTPS. + scheme: callable returning the scheme to use when constructing the ingress url. Request Args: port: the port of the service @@ -455,6 +572,7 @@ class IngressPerAppRequirer(_IngressPerAppBase): self.relation_name = relation_name self._strip_prefix = strip_prefix self._redirect_https = redirect_https + self._get_scheme = scheme self._stored.set_default(current_url=None) # type: ignore @@ -467,7 +585,7 @@ class IngressPerAppRequirer(_IngressPerAppBase): def _handle_relation(self, event): # created, joined or changed: if we have auto data: publish it - self._publish_auto_data(event.relation) + self._publish_auto_data() if self.is_ready(): # Avoid spurious events, emit only when there is a NEW URL available @@ -486,56 +604,93 @@ class IngressPerAppRequirer(_IngressPerAppBase): def _handle_upgrade_or_leader(self, event): """On upgrade/leadership change: ensure we publish the data we have.""" - for relation in self.relations: - self._publish_auto_data(relation) + self._publish_auto_data() def is_ready(self): """The Requirer is ready if the Provider has sent valid data.""" try: return bool(self._get_url_from_relation_data()) except DataValidationError as e: - log.warning("Requirer not ready; validation error encountered: %s" % str(e)) + log.debug("Requirer not ready; validation error encountered: %s" % str(e)) return False - def _publish_auto_data(self, relation: Relation): - if self._auto_data and self.unit.is_leader(): + def _publish_auto_data(self): + if self._auto_data: host, port = self._auto_data self.provide_ingress_requirements(host=host, port=port) - def provide_ingress_requirements(self, *, host: Optional[str] = None, port: int): + def provide_ingress_requirements( + self, + *, + scheme: Optional[str] = None, + host: Optional[str] = None, + port: int, + ): """Publishes the data that Traefik needs to provide ingress. - NB only the leader unit is supposed to do this. - Args: + scheme: Scheme to be used; if unspecified, use the one used by __init__. host: Hostname to be used by the ingress provider to address the requirer unit; if unspecified, FQDN will be used instead port: the port of the service (required) """ - # get only the leader to publish the data since we only - # require one unit to publish it -- it will not differ between units, - # unlike in ingress-per-unit. - assert self.unit.is_leader(), "only leaders should do this." - assert self.relation, "no relation" + for relation in self.relations: + self._provide_ingress_requirements(scheme, host, port, relation) + def _provide_ingress_requirements( + self, + scheme: Optional[str], + host: Optional[str], + port: int, + relation: Relation, + ): + if self.unit.is_leader(): + self._publish_app_data(scheme, port, relation) + + self._publish_unit_data(host, relation) + + def _publish_unit_data( + self, + host: Optional[str], + relation: Relation, + ): if not host: host = socket.getfqdn() - data = { - "model": self.model.name, - "name": self.app.name, - "host": host, - "port": str(port), - } + unit_databag = relation.data[self.unit] + try: + IngressRequirerUnitData(host=host).dump(unit_databag) + except pydantic.ValidationError as e: + msg = "failed to validate unit data" + log.info(msg, exc_info=True) # log to INFO because this might be expected + raise DataValidationError(msg) from e - if self._strip_prefix: - data["strip-prefix"] = "true" + def _publish_app_data( + self, + scheme: Optional[str], + port: int, + relation: Relation, + ): + # assumes leadership! + app_databag = relation.data[self.app] - if self._redirect_https: - data["redirect-https"] = "true" + if not scheme: + # If scheme was not provided, use the one given to the constructor. + scheme = self._get_scheme() - _validate_data(data, INGRESS_REQUIRES_APP_SCHEMA) - self.relation.data[self.app].update(data) + try: + IngressRequirerAppData( # type: ignore # pyright does not like aliases + model=self.model.name, + name=self.app.name, + scheme=scheme, + port=port, + strip_prefix=self._strip_prefix, # type: ignore # pyright does not like aliases + redirect_https=self._redirect_https, # type: ignore # pyright does not like aliases + ).dump(app_databag) + except pydantic.ValidationError as e: + msg = "failed to validate app data" + log.info(msg, exc_info=True) # log to INFO because this might be expected + raise DataValidationError(msg) from e @property def relation(self): @@ -553,7 +708,7 @@ class IngressPerAppRequirer(_IngressPerAppBase): # fetch the provider's app databag try: - raw = relation.data.get(relation.app, {}).get("ingress") + databag = relation.data[relation.app] except ModelError as e: log.debug( f"Error {e} attempting to read remote app data; " @@ -561,12 +716,10 @@ class IngressPerAppRequirer(_IngressPerAppBase): ) return None - if not raw: + if not databag: # not ready yet return None - ingress: ProviderIngressData = yaml.safe_load(raw) - _validate_data({"ingress": ingress}, INGRESS_PROVIDES_APP_SCHEMA) - return ingress["url"] + return str(IngressProviderAppData.load(databag).ingress.url) @property def url(self) -> Optional[str]: @@ -574,6 +727,8 @@ class IngressPerAppRequirer(_IngressPerAppBase): Returns None if the URL isn't available yet. """ - data = self._stored.current_url or self._get_url_from_relation_data() # type: ignore - assert isinstance(data, (str, type(None))) # for static checker + data = ( + typing.cast(Optional[str], self._stored.current_url) # type: ignore + or self._get_url_from_relation_data() + ) return data diff --git a/charms/designate-k8s/requirements.txt b/charms/designate-k8s/requirements.txt index ff55d0c8..db1e74a8 100644 --- a/charms/designate-k8s/requirements.txt +++ b/charms/designate-k8s/requirements.txt @@ -1,5 +1,6 @@ ops jsonschema +pydantic<2.0 jinja2 git+https://github.com/openstack/charm-ops-sunbeam#egg=ops_sunbeam lightkube diff --git a/charms/designate-k8s/src/charm.py b/charms/designate-k8s/src/charm.py index ebb5f01a..f6f3aa5e 100755 --- a/charms/designate-k8s/src/charm.py +++ b/charms/designate-k8s/src/charm.py @@ -23,13 +23,15 @@ This charm provide Designate services as part of an OpenStack deployment """ import logging +import secrets from typing import ( Callable, List, Mapping, + Optional, ) -import charms.bind9_k8s.v0.bind_rndc as bind_rndc +import charms.designate_bind_k8s.v0.bind_rndc as bind_rndc import ops import ops.charm import ops_sunbeam.charm as sunbeam_charm @@ -47,6 +49,13 @@ logger = logging.getLogger(__name__) DESIGNATE_CONTAINER = "designate" BIND_RNDC_RELATION = "dns-backend" RNDC_SECRET_PREFIX = "rndc_" +NONCE_SECRET_LABEL = "nonce-rndc" + + +class NoRelationError(Exception): + """No relation found.""" + + pass class DesignatePebbleHandler(sunbeam_chandlers.WSGIPebbleHandler): @@ -175,15 +184,33 @@ class BindRndcRequiresRelationHandler(sunbeam_rhandlers.RelationHandler): """Setup event handler for the relation.""" interface = bind_rndc.BindRndcRequires(self.charm, BIND_RNDC_RELATION) self.framework.observe( - interface.on.bind_rndc_ready, + interface.on.connected, + self._on_bind_rndc_connected, + ) + self.framework.observe( + interface.on.ready, self._on_bind_rndc_ready, ) self.framework.observe( interface.on.goneaway, self._on_bind_rndc_goneaway, ) + + try: + self.request_rndc_key(interface, self._relation) + except NoRelationError: + pass + return interface + def _on_bind_rndc_connected(self, event: bind_rndc.BindRndcConnectedEvent): + """Handle bind rndc connected event.""" + relation = self.model.get_relation( + event.relation_name, event.relation_id + ) + if relation is not None: + self.request_rndc_key(self.interface, relation) + def _on_bind_rndc_ready(self, event: bind_rndc.BindRndcReadyEvent): """Handle bind rndc ready event.""" self.callback_f(event) @@ -192,12 +219,21 @@ class BindRndcRequiresRelationHandler(sunbeam_rhandlers.RelationHandler): """Handle bind rndc goneaway event.""" self.callback_f(event) + def request_rndc_key( + self, interface: bind_rndc.BindRndcRequires, relation: ops.Relation + ): + """Request credentials from vault-kv relation.""" + nonce = self.charm.get_nonce() + if nonce is None: + return + interface.request_rndc_key(relation, nonce) + @property def _relation(self) -> ops.Relation: """Get relation.""" relation = self.framework.model.get_relation(self.relation_name) if relation is None: - raise Exception("Relation not found") + raise NoRelationError("Relation not found") return relation @property @@ -205,7 +241,6 @@ class BindRndcRequiresRelationHandler(sunbeam_rhandlers.RelationHandler): """Ready when a key is available for current unit.""" try: relation = self._relation - self.interface.reconcile_rndc_key(relation) return self.interface.get_rndc_key(relation) is not None except Exception: return False @@ -224,7 +259,7 @@ class BindRndcRequiresRelationHandler(sunbeam_rhandlers.RelationHandler): ) secret_value = secret.get_content()["secret"] rndc_key["secret"] = secret_value - rndc_key["name"] = self.interface.nonce() + rndc_key["name"] = self.interface.nonce(self._relation) return rndc_key @@ -284,7 +319,19 @@ class DesignateOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): BIND_RNDC_RELATION, } - def configure_unit(self, event: ops.framework.EventBase) -> None: + def __init__(self, *args): + super().__init__(*args) + self.framework.observe(self.on.install, self._on_install) + + def _on_install(self, event: ops.EventBase) -> None: + """Handle install event.""" + self.unit.add_secret( + {"nonce": secrets.token_hex(16)}, + label=NONCE_SECRET_LABEL, + description="nonce for bind-rndc relation", + ) + + def configure_unit(self, event: ops.EventBase) -> None: """Run configuration on this unit.""" self.check_leader_ready() self.check_relation_handlers_ready() @@ -420,6 +467,14 @@ class DesignateOperatorCharm(sunbeam_charm.OSBaseOperatorAPICharm): "Updating pools failed" ) + def get_nonce(self) -> Optional[str]: + """Return nonce stored in secret.""" + try: + secret = self.model.get_secret(label=NONCE_SECRET_LABEL) + return secret.get_content()["nonce"] + except ops.SecretNotFoundError: + return None + if __name__ == "__main__": main(DesignateOperatorCharm) diff --git a/charms/designate-k8s/tests/bundles/smoke.yaml b/charms/designate-k8s/tests/bundles/smoke.yaml index 662020b6..96298810 100644 --- a/charms/designate-k8s/tests/bundles/smoke.yaml +++ b/charms/designate-k8s/tests/bundles/smoke.yaml @@ -10,7 +10,7 @@ applications: # If this isn't present, the units will hang at "installing agent". traefik: charm: ch:traefik-k8s - channel: 1.0/stable + channel: 1.0/candidate scale: 1 trust: true options: @@ -35,9 +35,9 @@ applications: fernet-keys: 5M credential-keys: 5M - bind9: - charm: ch:bind9-k8s - channel: latest/edge + designate-bind: + charm: ch:designate-bind-k8s + channel: 9/edge scale: 1 trust: false @@ -67,5 +67,5 @@ relations: - designate:ingress-internal - - traefik:ingress - designate:ingress-public -- - bind9:dns-backend +- - designate-bind:dns-backend - designate:dns-backend diff --git a/charms/designate-k8s/tests/tests.yaml b/charms/designate-k8s/tests/tests.yaml index b796c255..5acffeae 100644 --- a/charms/designate-k8s/tests/tests.yaml +++ b/charms/designate-k8s/tests/tests.yaml @@ -30,7 +30,7 @@ target_deploy_status: mysql: workload-status: active workload-status-message-regex: '^.*$' - bind9: + designate-bind: workload-status: active workload-status-message-regex: '^.*$' designate: diff --git a/charms/designate-k8s/tests/unit/test_designate_charm.py b/charms/designate-k8s/tests/unit/test_designate_charm.py index f8144b81..85a55ac3 100644 --- a/charms/designate-k8s/tests/unit/test_designate_charm.py +++ b/charms/designate-k8s/tests/unit/test_designate_charm.py @@ -17,9 +17,6 @@ """Unit tests for Designate operator.""" import json -from unittest.mock import ( - patch, -) import ops_sunbeam.test_utils as test_utils from ops.testing import ( @@ -86,12 +83,9 @@ class TestDesignateOperatorCharm(test_utils.CharmTestCase): test_utils.set_all_pebbles_ready(self.harness) self.assertEqual(len(self.harness.charm.seen_events), 1) - @patch( - "secrets.token_hex", - ) - def test_all_relations(self, token_hex): + def test_all_relations(self): """Test all integrations for operator.""" - token_hex.return_value = "abfcdfea12" + self.harness.charm.on.install.emit() self.harness.set_leader() test_utils.set_all_pebbles_ready(self.harness) # this adds all the default/common relations