From 0b4b3b995aa39999c4253572f4e5491377bfc0fa Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 09:42:09 +0200 Subject: [PATCH 01/30] Adds a workflow for doing a terraform deploy --- .github/workflows/terraform_test_deploy.yml | 45 +++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 .github/workflows/terraform_test_deploy.yml diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml new file mode 100644 index 00000000..04e79821 --- /dev/null +++ b/.github/workflows/terraform_test_deploy.yml @@ -0,0 +1,45 @@ +name: Terraform Deploy + +on: + push: + branches: + - main + +jobs: + deploy: + runs-on: [self-hosted, jammy, xlarge] + + defaults: + run: + working-directory: ./terraform + + steps: + - name: Checkout code + uses: actions/checkout@v2 + + - name: Set up microk8s + uses: canonical/certification-github-workflows/.github/actions/microk8s-setup@main + + - name: Set up Terraform + uses: hashicorp/setup-terraform@v1 + + - name: Terraform Init + run: terraform init + + - name: Create juju model to deploy to + run: juju add-model test-observer-development + + - name: Terraform Apply + run: | + TF_VAR_environment=development \ + TF_VAR_nginx_ingress_integrator_charm_whitelist_source_range="" \ + terraform apply -auto-approve + + - name: Wait for deployment to complete + run: | + juju wait-for model test-observer-development \ + --timeout=1min \ + --query='life=="alive" && status=="available" && forEach(applications, app => app.status == "active")' + + + From 58fe6693d6b1ed92cb07bddff63a641fa0c47298 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 10:02:29 +0200 Subject: [PATCH 02/30] Tweaks to trigger conditions of the terraform_test_deploy workflow --- .github/workflows/terraform_test_deploy.yml | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 04e79821..f6b2e549 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -1,9 +1,21 @@ -name: Terraform Deploy - +name: Terraform deploy to a fresh microk8s model on: - push: + pull_request_review: + on: + pull_request_review: + types: + - submitted + pull_request: branches: - - main + - '**' + paths: + - 'frontend/charm/**' + - 'backend/charm/**' + - 'terraform/**' + push: + tags: + - "v*.*.*" + workflow_dispatch: jobs: deploy: From a5bca3dda28f222249de2ad9140a11aab2a70f43 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 10:06:59 +0200 Subject: [PATCH 03/30] Loosens up the filter conditions --- .github/workflows/terraform_test_deploy.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index f6b2e549..04c40e9c 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -1,17 +1,17 @@ name: Terraform deploy to a fresh microk8s model on: - pull_request_review: - on: - pull_request_review: - types: - - submitted +# pull_request_review: +# on: +# pull_request_review: +# types: +# - submitted pull_request: branches: - '**' - paths: - - 'frontend/charm/**' - - 'backend/charm/**' - - 'terraform/**' +# paths: +# - 'frontend/charm/**' +# - 'backend/charm/**' +# - 'terraform/**' push: tags: - "v*.*.*" From 24b40786a0a765b2588e3ad000104eccd842ef59 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 10:16:29 +0200 Subject: [PATCH 04/30] Install terraform dependency: unzip --- .github/workflows/terraform_test_deploy.yml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 04c40e9c..d47bbfe6 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -29,15 +29,20 @@ jobs: - name: Checkout code uses: actions/checkout@v2 - - name: Set up microk8s - uses: canonical/certification-github-workflows/.github/actions/microk8s-setup@main - + - name: Set up Terraform dependencies + run: | + sudo apt-get update + sudo apt-get install unzip + - name: Set up Terraform uses: hashicorp/setup-terraform@v1 - name: Terraform Init run: terraform init + - name: Set up microk8s + uses: canonical/certification-github-workflows/.github/actions/microk8s-setup@main + - name: Create juju model to deploy to run: juju add-model test-observer-development From e7879144fe2e9029bad1415f5d7d2479faaa9fa1 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 10:49:34 +0200 Subject: [PATCH 05/30] Remove redundant terraform apply --- .github/workflows/terraform_test_deploy.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index d47bbfe6..a12bf9be 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -34,19 +34,16 @@ jobs: sudo apt-get update sudo apt-get install unzip - - name: Set up Terraform + - name: Set up terraform uses: hashicorp/setup-terraform@v1 - - name: Terraform Init + - name: Terraform init run: terraform init - name: Set up microk8s uses: canonical/certification-github-workflows/.github/actions/microk8s-setup@main - - name: Create juju model to deploy to - run: juju add-model test-observer-development - - - name: Terraform Apply + - name: Terraform apply run: | TF_VAR_environment=development \ TF_VAR_nginx_ingress_integrator_charm_whitelist_source_range="" \ From bb4957059137bc6aff3fcf2b167bb568903c6234 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 11:02:00 +0200 Subject: [PATCH 06/30] min => m as unit --- .github/workflows/terraform_test_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index a12bf9be..6502b8d9 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -52,7 +52,7 @@ jobs: - name: Wait for deployment to complete run: | juju wait-for model test-observer-development \ - --timeout=1min \ + --timeout=10m \ --query='life=="alive" && status=="available" && forEach(applications, app => app.status == "active")' From 0c0ccf825c2b6db106a54778453d4307d48add03 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 17:09:32 +0200 Subject: [PATCH 07/30] Updates trigger conditions for tests, adds manual dispatching around --- .github/workflows/frontend_charm_analysis.yml | 4 +++- .github/workflows/publish_api.yml | 1 + .github/workflows/publish_frontend.yml | 1 + .github/workflows/test_backend.yml | 9 +++++++-- .github/workflows/test_frontend.yml | 9 +++++++-- 5 files changed, 19 insertions(+), 5 deletions(-) diff --git a/.github/workflows/frontend_charm_analysis.yml b/.github/workflows/frontend_charm_analysis.yml index b411f3a5..d0d6a1c2 100644 --- a/.github/workflows/frontend_charm_analysis.yml +++ b/.github/workflows/frontend_charm_analysis.yml @@ -2,7 +2,9 @@ name: Run charm tests on: pull_request: branches: - - main + - '**' + paths: + - 'frontend/charm/**' push: branches: ["main"] tags: ["v*.*.*"] diff --git a/.github/workflows/publish_api.yml b/.github/workflows/publish_api.yml index 1cb2982f..56b045f8 100644 --- a/.github/workflows/publish_api.yml +++ b/.github/workflows/publish_api.yml @@ -2,6 +2,7 @@ on: push: branches: ["main"] tags: ["v*.*.*"] + workflow_dispatch: env: REGISTRY: ghcr.io diff --git a/.github/workflows/publish_frontend.yml b/.github/workflows/publish_frontend.yml index 7e6ecd92..b01fdc99 100644 --- a/.github/workflows/publish_frontend.yml +++ b/.github/workflows/publish_frontend.yml @@ -2,6 +2,7 @@ on: push: branches: ["main"] tags: ["v*.*.*"] + workflow_dispatch: env: REGISTRY: ghcr.io diff --git a/.github/workflows/test_backend.yml b/.github/workflows/test_backend.yml index e3f5c7cc..d4e3a112 100644 --- a/.github/workflows/test_backend.yml +++ b/.github/workflows/test_backend.yml @@ -1,9 +1,14 @@ name: Test Backend -on: [push] -# Cancel inprogress runs if new commit pushed +on: + push: + paths: + - 'backend/**' + tags: + - "v*.*.*" concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true + jobs: test: runs-on: [self-hosted, linux, large] diff --git a/.github/workflows/test_frontend.yml b/.github/workflows/test_frontend.yml index b9559c0a..7b569c10 100644 --- a/.github/workflows/test_frontend.yml +++ b/.github/workflows/test_frontend.yml @@ -1,9 +1,14 @@ name: Test Frontend -on: [push] -# Cancel inprogress runs if new commit pushed +on: + push: + paths: + - 'frontend/**' + tags: + - "v*.*.*" concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true + jobs: test: runs-on: [self-hosted, linux, large] From 1a33fa2164f21c3a93d5a4bd80328212c92dc46f Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 17:10:18 +0200 Subject: [PATCH 08/30] Updates to the new charm format, updates charm libraries, removes stateful stuff from the charm implementation --- backend/charm/.dockerignore | 2 + backend/charm/.gitignore | 2 + backend/charm/.vscode-settings.json | 14 + backend/charm/charmcraft.yaml | 26 + .../data_platform_libs/v0/data_interfaces.py | 1443 ++++++++++++++--- .../v0/nginx_route.py | 39 +- backend/charm/metadata.yaml | 28 - backend/charm/requirements.txt | 1 + backend/charm/src/charm.py | 91 +- backend/charm/src/faults.py | 17 + 10 files changed, 1405 insertions(+), 258 deletions(-) create mode 100644 backend/charm/.dockerignore create mode 100644 backend/charm/.vscode-settings.json delete mode 100644 backend/charm/metadata.yaml create mode 100644 backend/charm/src/faults.py diff --git a/backend/charm/.dockerignore b/backend/charm/.dockerignore new file mode 100644 index 00000000..a8a6abab --- /dev/null +++ b/backend/charm/.dockerignore @@ -0,0 +1,2 @@ +prime +stage diff --git a/backend/charm/.gitignore b/backend/charm/.gitignore index 1b300007..dbf2dbb9 100644 --- a/backend/charm/.gitignore +++ b/backend/charm/.gitignore @@ -1,2 +1,4 @@ *.charm venv +prime +stage diff --git a/backend/charm/.vscode-settings.json b/backend/charm/.vscode-settings.json new file mode 100644 index 00000000..ab10e191 --- /dev/null +++ b/backend/charm/.vscode-settings.json @@ -0,0 +1,14 @@ +{ + "python.autoComplete.extraPaths": [ + "./lib" + ], + "python.analysis.extraPaths": [ + "./lib" + ], + "yaml.schemas": { + "https://json.schemastore.org/yamllint.json": [ + "./metadata.yaml" + ] + }, + "ansible.python.interpreterPath": "./venv/bin/python3" +} \ No newline at end of file diff --git a/backend/charm/charmcraft.yaml b/backend/charm/charmcraft.yaml index b77ea693..486abf2c 100644 --- a/backend/charm/charmcraft.yaml +++ b/backend/charm/charmcraft.yaml @@ -1,4 +1,9 @@ +name: test-observer-api type: charm +summary: | + API to observe the status of artifact (snaps, debs, etc) testing +description: | + API and dashboard to observe the status of artifact (snaps, debs, etc) test status bases: - build-on: - name: ubuntu @@ -6,3 +11,24 @@ bases: run-on: - name: ubuntu channel: "22.04" +assumes: + - juju >= 2.9 + - k8s-api +containers: + api: + resource: api-image +requires: + database: + interface: postgresql_client + limit: 1 + nginx-route: + interface: nginx-route +provides: + test-observer-rest-api: + interface: http + scope: global +resources: + api-image: + type: oci-image + description: OCI image from GitHub Container Repository + upstream-source: ghcr.io/canonical/test_observer/api:main diff --git a/backend/charm/lib/charms/data_platform_libs/v0/data_interfaces.py b/backend/charm/lib/charms/data_platform_libs/v0/data_interfaces.py index 86d7521a..714eace4 100644 --- a/backend/charm/lib/charms/data_platform_libs/v0/data_interfaces.py +++ b/backend/charm/lib/charms/data_platform_libs/v0/data_interfaces.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -"""Library to manage the relation for the data-platform products. +r"""Library to manage the relation for the data-platform products. This library contains the Requires and Provides classes for handling the relation between an application and multiple managed application supported by the data-team: @@ -291,22 +291,26 @@ def _on_topic_requested(self, event: TopicRequestedEvent): exchanged in the relation databag. """ +import copy import json import logging from abc import ABC, abstractmethod from collections import namedtuple from datetime import datetime -from typing import List, Optional +from enum import Enum +from typing import Callable, Dict, List, Optional, Set, Tuple, Union +from ops import JujuVersion, Secret, SecretInfo, SecretNotFoundError from ops.charm import ( CharmBase, CharmEvents, RelationChangedEvent, + RelationCreatedEvent, RelationEvent, - RelationJoinedEvent, + SecretChangedEvent, ) from ops.framework import EventSource, Object -from ops.model import Relation +from ops.model import Application, ModelError, Relation, Unit # The unique Charmhub library identifier, never change it LIBID = "6c3e6b6680d64e9c89e611d1a15f65be" @@ -316,7 +320,7 @@ def _on_topic_requested(self, event: TopicRequestedEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 12 +LIBPATCH = 24 PYDEPS = ["ops>=2.0.0"] @@ -331,7 +335,79 @@ def _on_topic_requested(self, event: TopicRequestedEvent): deleted - key that were deleted""" -def diff(event: RelationChangedEvent, bucket: str) -> Diff: +PROV_SECRET_PREFIX = "secret-" +REQ_SECRET_FIELDS = "requested-secrets" + + +class SecretGroup(Enum): + """Secret groups as constants.""" + + USER = "user" + TLS = "tls" + EXTRA = "extra" + + +# Local map to associate mappings with secrets potentially as a group +SECRET_LABEL_MAP = { + "username": SecretGroup.USER, + "password": SecretGroup.USER, + "uris": SecretGroup.USER, + "tls": SecretGroup.TLS, + "tls-ca": SecretGroup.TLS, +} + + +class DataInterfacesError(Exception): + """Common ancestor for DataInterfaces related exceptions.""" + + +class SecretError(Exception): + """Common ancestor for Secrets related exceptions.""" + + +class SecretAlreadyExistsError(SecretError): + """A secret that was to be added already exists.""" + + +class SecretsUnavailableError(SecretError): + """Secrets aren't yet available for Juju version used.""" + + +class SecretsIllegalUpdateError(SecretError): + """Secrets aren't yet available for Juju version used.""" + + +def get_encoded_dict( + relation: Relation, member: Union[Unit, Application], field: str +) -> Optional[Dict[str, str]]: + """Retrieve and decode an encoded field from relation data.""" + data = json.loads(relation.data[member].get(field, "{}")) + if isinstance(data, dict): + return data + logger.error("Unexpected datatype for %s instead of dict.", str(data)) + + +def get_encoded_list( + relation: Relation, member: Union[Unit, Application], field: str +) -> Optional[List[str]]: + """Retrieve and decode an encoded field from relation data.""" + data = json.loads(relation.data[member].get(field, "[]")) + if isinstance(data, list): + return data + logger.error("Unexpected datatype for %s instead of list.", str(data)) + + +def set_encoded_field( + relation: Relation, + member: Union[Unit, Application], + field: str, + value: Union[str, list, Dict[str, str]], +) -> None: + """Set an encoded field from relation data.""" + relation.data[member].update({field: json.dumps(value)}) + + +def diff(event: RelationChangedEvent, bucket: Union[Unit, Application]) -> Diff: """Retrieves the diff of the data in the relation changed databag. Args: @@ -343,31 +419,173 @@ def diff(event: RelationChangedEvent, bucket: str) -> Diff: keys from the event relation databag. """ # Retrieve the old data from the data key in the application relation databag. - old_data = json.loads(event.relation.data[bucket].get("data", "{}")) + old_data = get_encoded_dict(event.relation, bucket, "data") + + if not old_data: + old_data = {} + # Retrieve the new data from the event relation databag. - new_data = { - key: value for key, value in event.relation.data[event.app].items() if key != "data" - } + new_data = ( + {key: value for key, value in event.relation.data[event.app].items() if key != "data"} + if event.app + else {} + ) # These are the keys that were added to the databag and triggered this event. - added = new_data.keys() - old_data.keys() + added = new_data.keys() - old_data.keys() # pyright: ignore [reportGeneralTypeIssues] # These are the keys that were removed from the databag and triggered this event. - deleted = old_data.keys() - new_data.keys() + deleted = old_data.keys() - new_data.keys() # pyright: ignore [reportGeneralTypeIssues] # These are the keys that already existed in the databag, # but had their values changed. - changed = {key for key in old_data.keys() & new_data.keys() if old_data[key] != new_data[key]} + changed = { + key + for key in old_data.keys() & new_data.keys() # pyright: ignore [reportGeneralTypeIssues] + if old_data[key] != new_data[key] # pyright: ignore [reportGeneralTypeIssues] + } # Convert the new_data to a serializable format and save it for a next diff check. - event.relation.data[bucket].update({"data": json.dumps(new_data)}) + set_encoded_field(event.relation, bucket, "data", new_data) # Return the diff with all possible changes. return Diff(added, changed, deleted) -# Base DataProvides and DataRequires +def leader_only(f): + """Decorator to ensure that only leader can perform given operation.""" + def wrapper(self, *args, **kwargs): + if not self.local_unit.is_leader(): + logger.error( + "This operation (%s()) can only be performed by the leader unit", f.__name__ + ) + return + return f(self, *args, **kwargs) -class DataProvides(Object, ABC): - """Base provides-side of the data products relation.""" + return wrapper + + +def juju_secrets_only(f): + """Decorator to ensure that certain operations would be only executed on Juju3.""" + + def wrapper(self, *args, **kwargs): + if not self.secrets_enabled: + raise SecretsUnavailableError("Secrets unavailable on current Juju version") + return f(self, *args, **kwargs) + + return wrapper + + +class Scope(Enum): + """Peer relations scope.""" + + APP = "app" + UNIT = "unit" + + +class CachedSecret: + """Locally cache a secret. + + The data structure is precisely re-using/simulating as in the actual Secret Storage + """ + + def __init__(self, charm: CharmBase, label: str, secret_uri: Optional[str] = None): + self._secret_meta = None + self._secret_content = {} + self._secret_uri = secret_uri + self.label = label + self.charm = charm + + def add_secret(self, content: Dict[str, str], relation: Relation) -> Secret: + """Create a new secret.""" + if self._secret_uri: + raise SecretAlreadyExistsError( + "Secret is already defined with uri %s", self._secret_uri + ) + + secret = self.charm.app.add_secret(content, label=self.label) + secret.grant(relation) + self._secret_uri = secret.id + self._secret_meta = secret + return self._secret_meta + + @property + def meta(self) -> Optional[Secret]: + """Getting cached secret meta-information.""" + if not self._secret_meta: + if not (self._secret_uri or self.label): + return + try: + self._secret_meta = self.charm.model.get_secret(label=self.label) + except SecretNotFoundError: + if self._secret_uri: + self._secret_meta = self.charm.model.get_secret( + id=self._secret_uri, label=self.label + ) + return self._secret_meta + + def get_content(self) -> Dict[str, str]: + """Getting cached secret content.""" + if not self._secret_content: + if self.meta: + try: + self._secret_content = self.meta.get_content(refresh=True) + except (ValueError, ModelError) as err: + # https://bugs.launchpad.net/juju/+bug/2042596 + # Only triggered when 'refresh' is set + msg = "ERROR either URI or label should be used for getting an owned secret but not both" + if isinstance(err, ModelError) and msg not in str(err): + raise + # Due to: ValueError: Secret owner cannot use refresh=True + self._secret_content = self.meta.get_content() + return self._secret_content + + def set_content(self, content: Dict[str, str]) -> None: + """Setting cached secret content.""" + if not self.meta: + return + + if content: + self.meta.set_content(content) + self._secret_content = content + else: + self.meta.remove_all_revisions() + + def get_info(self) -> Optional[SecretInfo]: + """Wrapper function to apply the corresponding call on the Secret object within CachedSecret if any.""" + if self.meta: + return self.meta.get_info() + + +class SecretCache: + """A data structure storing CachedSecret objects.""" + + def __init__(self, charm): + self.charm = charm + self._secrets: Dict[str, CachedSecret] = {} + + def get(self, label: str, uri: Optional[str] = None) -> Optional[CachedSecret]: + """Getting a secret from Juju Secret store or cache.""" + if not self._secrets.get(label): + secret = CachedSecret(self.charm, label, uri) + if secret.meta: + self._secrets[label] = secret + return self._secrets.get(label) + + def add(self, label: str, content: Dict[str, str], relation: Relation) -> CachedSecret: + """Adding a secret to Juju Secret.""" + if self._secrets.get(label): + raise SecretAlreadyExistsError(f"Secret {label} already exists") + + secret = CachedSecret(self.charm, label) + secret.add_secret(content, relation) + self._secrets[label] = secret + return self._secrets[label] + + +# Base DataRelation + + +class DataRelation(Object, ABC): + """Base relation data mainpulation (abstract) class.""" def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) @@ -377,62 +595,612 @@ def __init__(self, charm: CharmBase, relation_name: str) -> None: self.relation_name = relation_name self.framework.observe( charm.on[relation_name].relation_changed, - self._on_relation_changed, + self._on_relation_changed_event, ) + self._jujuversion = None + self.secrets = SecretCache(self.charm) - def _diff(self, event: RelationChangedEvent) -> Diff: - """Retrieves the diff of the data in the relation changed databag. + @property + def relations(self) -> List[Relation]: + """The list of Relation instances associated with this relation_name.""" + return [ + relation + for relation in self.charm.model.relations[self.relation_name] + if self._is_relation_active(relation) + ] - Args: - event: relation changed event. + @property + def secrets_enabled(self): + """Is this Juju version allowing for Secrets usage?""" + if not self._jujuversion: + self._jujuversion = JujuVersion.from_environ() + return self._jujuversion.has_secrets - Returns: - a Diff instance containing the added, deleted and changed - keys from the event relation databag. - """ - return diff(event, self.local_app) + # Mandatory overrides for internal/helper methods @abstractmethod - def _on_relation_changed(self, event: RelationChangedEvent) -> None: + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the relation data has changed.""" raise NotImplementedError - def fetch_relation_data(self) -> dict: + @abstractmethod + def _get_relation_secret( + self, relation_id: int, group_mapping: SecretGroup, relation_name: Optional[str] = None + ) -> Optional[CachedSecret]: + """Retrieve a Juju Secret that's been stored in the relation databag.""" + raise NotImplementedError + + @abstractmethod + def _fetch_specific_relation_data( + self, relation: Relation, fields: Optional[List[str]] + ) -> Dict[str, str]: + """Fetch data available (directily or indirectly -- i.e. secrets) from the relation.""" + raise NotImplementedError + + @abstractmethod + def _fetch_my_specific_relation_data( + self, relation: Relation, fields: Optional[List[str]] + ) -> Dict[str, str]: + """Fetch data available (directily or indirectly -- i.e. secrets) from the relation for owner/this_app.""" + raise NotImplementedError + + @abstractmethod + def _update_relation_data(self, relation: Relation, data: Dict[str, str]) -> None: + """Update data available (directily or indirectly -- i.e. secrets) from the relation for owner/this_app.""" + raise NotImplementedError + + @abstractmethod + def _delete_relation_data(self, relation: Relation, fields: List[str]) -> None: + """Delete data available (directily or indirectly -- i.e. secrets) from the relation for owner/this_app.""" + raise NotImplementedError + + # Internal helper methods + + @staticmethod + def _is_relation_active(relation: Relation): + """Whether the relation is active based on contained data.""" + try: + _ = repr(relation.data) + return True + except (RuntimeError, ModelError): + return False + + @staticmethod + def _is_secret_field(field: str) -> bool: + """Is the field in question a secret reference (URI) field or not?""" + return field.startswith(PROV_SECRET_PREFIX) + + @staticmethod + def _generate_secret_label( + relation_name: str, relation_id: int, group_mapping: SecretGroup + ) -> str: + """Generate unique group_mappings for secrets within a relation context.""" + return f"{relation_name}.{relation_id}.{group_mapping.value}.secret" + + @staticmethod + def _generate_secret_field_name(group_mapping: SecretGroup) -> str: + """Generate unique group_mappings for secrets within a relation context.""" + return f"{PROV_SECRET_PREFIX}{group_mapping.value}" + + def _relation_from_secret_label(self, secret_label: str) -> Optional[Relation]: + """Retrieve the relation that belongs to a secret label.""" + contents = secret_label.split(".") + + if not (contents and len(contents) >= 3): + return + + contents.pop() # ".secret" at the end + contents.pop() # Group mapping + relation_id = contents.pop() + try: + relation_id = int(relation_id) + except ValueError: + return + + # In case '.' character appeared in relation name + relation_name = ".".join(contents) + + try: + return self.get_relation(relation_name, relation_id) + except ModelError: + return + + @staticmethod + def _group_secret_fields(secret_fields: List[str]) -> Dict[SecretGroup, List[str]]: + """Helper function to arrange secret mappings under their group. + + NOTE: All unrecognized items end up in the 'extra' secret bucket. + Make sure only secret fields are passed! + """ + secret_fieldnames_grouped = {} + for key in secret_fields: + if group := SECRET_LABEL_MAP.get(key): + secret_fieldnames_grouped.setdefault(group, []).append(key) + else: + secret_fieldnames_grouped.setdefault(SecretGroup.EXTRA, []).append(key) + return secret_fieldnames_grouped + + def _get_group_secret_contents( + self, + relation: Relation, + group: SecretGroup, + secret_fields: Optional[Union[Set[str], List[str]]] = None, + ) -> Dict[str, str]: + """Helper function to retrieve collective, requested contents of a secret.""" + if not secret_fields: + secret_fields = [] + + if (secret := self._get_relation_secret(relation.id, group)) and ( + secret_data := secret.get_content() + ): + return {k: v for k, v in secret_data.items() if k in secret_fields} + return {} + + @staticmethod + def _content_for_secret_group( + content: Dict[str, str], secret_fields: Set[str], group_mapping: SecretGroup + ) -> Dict[str, str]: + """Select : pairs from input, that belong to this particular Secret group.""" + if group_mapping == SecretGroup.EXTRA: + return { + k: v + for k, v in content.items() + if k in secret_fields and k not in SECRET_LABEL_MAP.keys() + } + + return { + k: v + for k, v in content.items() + if k in secret_fields and SECRET_LABEL_MAP.get(k) == group_mapping + } + + @juju_secrets_only + def _get_relation_secret_data( + self, relation_id: int, group_mapping: SecretGroup, relation_name: Optional[str] = None + ) -> Optional[Dict[str, str]]: + """Retrieve contents of a Juju Secret that's been stored in the relation databag.""" + secret = self._get_relation_secret(relation_id, group_mapping, relation_name) + if secret: + return secret.get_content() + + # Core operations on Relation Fields manipulations (regardless whether the field is in the databag or in a secret) + # Internal functions to be called directly from transparent public interface functions (+closely related helpers) + + def _process_secret_fields( + self, + relation: Relation, + req_secret_fields: Optional[List[str]], + impacted_rel_fields: List[str], + operation: Callable, + *args, + **kwargs, + ) -> Tuple[Dict[str, str], Set[str]]: + """Isolate target secret fields of manipulation, and execute requested operation by Secret Group.""" + result = {} + + # If the relation started on a databag, we just stay on the databag + # (Rolling upgrades may result in a relation starting on databag, getting secrets enabled on-the-fly) + # self.local_app is sufficient to check (ignored if Requires, never has secrets -- works if Provides) + fallback_to_databag = ( + req_secret_fields + and self.local_unit.is_leader() + and set(req_secret_fields) & set(relation.data[self.local_app]) + ) + + normal_fields = set(impacted_rel_fields) + if req_secret_fields and self.secrets_enabled and not fallback_to_databag: + normal_fields = normal_fields - set(req_secret_fields) + secret_fields = set(impacted_rel_fields) - set(normal_fields) + + secret_fieldnames_grouped = self._group_secret_fields(list(secret_fields)) + + for group in secret_fieldnames_grouped: + # operation() should return nothing when all goes well + if group_result := operation(relation, group, secret_fields, *args, **kwargs): + # If "meaningful" data was returned, we take it. (Some 'operation'-s only return success/failure.) + if isinstance(group_result, dict): + result.update(group_result) + else: + # If it wasn't found as a secret, let's give it a 2nd chance as "normal" field + # Needed when Juju3 Requires meets Juju2 Provider + normal_fields |= set(secret_fieldnames_grouped[group]) + return (result, normal_fields) + + def _fetch_relation_data_without_secrets( + self, app: Application, relation: Relation, fields: Optional[List[str]] + ) -> Dict[str, str]: + """Fetching databag contents when no secrets are involved. + + Since the Provider's databag is the only one holding secrest, we can apply + a simplified workflow to read the Require's side's databag. + This is used typically when the Provides side wants to read the Requires side's data, + or when the Requires side may want to read its own data. + """ + if app not in relation.data or not relation.data[app]: + return {} + + if fields: + return {k: relation.data[app][k] for k in fields if k in relation.data[app]} + else: + return dict(relation.data[app]) + + def _fetch_relation_data_with_secrets( + self, + app: Application, + req_secret_fields: Optional[List[str]], + relation: Relation, + fields: Optional[List[str]] = None, + ) -> Dict[str, str]: + """Fetching databag contents when secrets may be involved. + + This function has internal logic to resolve if a requested field may be "hidden" + within a Relation Secret, or directly available as a databag field. Typically + used to read the Provides side's databag (eigher by the Requires side, or by + Provides side itself). + """ + result = {} + normal_fields = [] + + if not fields: + if app not in relation.data or not relation.data[app]: + return {} + + all_fields = list(relation.data[app].keys()) + normal_fields = [field for field in all_fields if not self._is_secret_field(field)] + + # There must have been secrets there + if all_fields != normal_fields and req_secret_fields: + # So we assemble the full fields list (without 'secret-' fields) + fields = normal_fields + req_secret_fields + + if fields: + result, normal_fields = self._process_secret_fields( + relation, req_secret_fields, fields, self._get_group_secret_contents + ) + + # Processing "normal" fields. May include leftover from what we couldn't retrieve as a secret. + # (Typically when Juju3 Requires meets Juju2 Provides) + if normal_fields: + result.update( + self._fetch_relation_data_without_secrets(app, relation, list(normal_fields)) + ) + return result + + def _update_relation_data_without_secrets( + self, app: Application, relation: Relation, data: Dict[str, str] + ) -> None: + """Updating databag contents when no secrets are involved.""" + if app not in relation.data or relation.data[app] is None: + return + + if any(self._is_secret_field(key) for key in data.keys()): + raise SecretsIllegalUpdateError("Can't update secret {key}.") + + if relation: + relation.data[app].update(data) + + def _delete_relation_data_without_secrets( + self, app: Application, relation: Relation, fields: List[str] + ) -> None: + """Remove databag fields 'fields' from Relation.""" + if app not in relation.data or not relation.data[app]: + return + + for field in fields: + try: + relation.data[app].pop(field) + except KeyError: + logger.debug( + "Non-existing field was attempted to be removed from the databag %s, %s", + str(relation.id), + str(field), + ) + pass + + # Public interface methods + # Handling Relation Fields seamlessly, regardless if in databag or a Juju Secret + + def get_relation(self, relation_name, relation_id) -> Relation: + """Safe way of retrieving a relation.""" + relation = self.charm.model.get_relation(relation_name, relation_id) + + if not relation: + raise DataInterfacesError( + "Relation %s %s couldn't be retrieved", relation_name, relation_id + ) + + return relation + + def fetch_relation_data( + self, + relation_ids: Optional[List[int]] = None, + fields: Optional[List[str]] = None, + relation_name: Optional[str] = None, + ) -> Dict[int, Dict[str, str]]: """Retrieves data from relation. This function can be used to retrieve data from a relation in the charm code when outside an event callback. + Function cannot be used in `*-relation-broken` events and will raise an exception. Returns: a dict of the values stored in the relation data bag - for all relation instances (indexed by the relation id). + for all relation instances (indexed by the relation ID). """ + if not relation_name: + relation_name = self.relation_name + + relations = [] + if relation_ids: + relations = [ + self.get_relation(relation_name, relation_id) for relation_id in relation_ids + ] + else: + relations = self.relations + data = {} - for relation in self.relations: - data[relation.id] = { - key: value for key, value in relation.data[relation.app].items() if key != "data" - } + for relation in relations: + if not relation_ids or (relation_ids and relation.id in relation_ids): + data[relation.id] = self._fetch_specific_relation_data(relation, fields) return data - def _update_relation_data(self, relation_id: int, data: dict) -> None: - """Updates a set of key-value pairs in the relation. + def fetch_relation_field( + self, relation_id: int, field: str, relation_name: Optional[str] = None + ) -> Optional[str]: + """Get a single field from the relation data.""" + return ( + self.fetch_relation_data([relation_id], [field], relation_name) + .get(relation_id, {}) + .get(field) + ) - This function writes in the application data bag, therefore, - only the leader unit can call it. + @leader_only + def fetch_my_relation_data( + self, + relation_ids: Optional[List[int]] = None, + fields: Optional[List[str]] = None, + relation_name: Optional[str] = None, + ) -> Optional[Dict[int, Dict[str, str]]]: + """Fetch data of the 'owner' (or 'this app') side of the relation. + + NOTE: Since only the leader can read the relation's 'this_app'-side + Application databag, the functionality is limited to leaders + """ + if not relation_name: + relation_name = self.relation_name + + relations = [] + if relation_ids: + relations = [ + self.get_relation(relation_name, relation_id) for relation_id in relation_ids + ] + else: + relations = self.relations + + data = {} + for relation in relations: + if not relation_ids or relation.id in relation_ids: + data[relation.id] = self._fetch_my_specific_relation_data(relation, fields) + return data + + @leader_only + def fetch_my_relation_field( + self, relation_id: int, field: str, relation_name: Optional[str] = None + ) -> Optional[str]: + """Get a single field from the relation data -- owner side. + + NOTE: Since only the leader can read the relation's 'this_app'-side + Application databag, the functionality is limited to leaders + """ + if relation_data := self.fetch_my_relation_data([relation_id], [field], relation_name): + return relation_data.get(relation_id, {}).get(field) + + @leader_only + def update_relation_data(self, relation_id: int, data: dict) -> None: + """Update the data within the relation.""" + relation_name = self.relation_name + relation = self.get_relation(relation_name, relation_id) + return self._update_relation_data(relation, data) + + @leader_only + def delete_relation_data(self, relation_id: int, fields: List[str]) -> None: + """Remove field from the relation.""" + relation_name = self.relation_name + relation = self.get_relation(relation_name, relation_id) + return self._delete_relation_data(relation, fields) + + +# Base DataProvides and DataRequires + + +class DataProvides(DataRelation): + """Base provides-side of the data products relation.""" + + def __init__(self, charm: CharmBase, relation_name: str) -> None: + super().__init__(charm, relation_name) + + def _diff(self, event: RelationChangedEvent) -> Diff: + """Retrieves the diff of the data in the relation changed databag. Args: - relation_id: the identifier for a particular relation. - data: dict containing the key-value pairs - that should be updated in the relation. + event: relation changed event. + + Returns: + a Diff instance containing the added, deleted and changed + keys from the event relation databag. """ - if self.local_unit.is_leader(): - relation = self.charm.model.get_relation(self.relation_name, relation_id) - relation.data[self.local_app].update(data) + return diff(event, self.local_app) - @property - def relations(self) -> List[Relation]: - """The list of Relation instances associated with this relation_name.""" - return list(self.charm.model.relations[self.relation_name]) + # Private methods handling secrets + + @juju_secrets_only + def _add_relation_secret( + self, relation: Relation, content: Dict[str, str], group_mapping: SecretGroup + ) -> bool: + """Add a new Juju Secret that will be registered in the relation databag.""" + secret_field = self._generate_secret_field_name(group_mapping) + if relation.data[self.local_app].get(secret_field): + logging.error("Secret for relation %s already exists, not adding again", relation.id) + return False + + label = self._generate_secret_label(self.relation_name, relation.id, group_mapping) + secret = self.secrets.add(label, content, relation) + + # According to lint we may not have a Secret ID + if secret.meta and secret.meta.id: + relation.data[self.local_app][secret_field] = secret.meta.id + + # Return the content that was added + return True + + @juju_secrets_only + def _update_relation_secret( + self, relation: Relation, content: Dict[str, str], group_mapping: SecretGroup + ) -> bool: + """Update the contents of an existing Juju Secret, referred in the relation databag.""" + secret = self._get_relation_secret(relation.id, group_mapping) + + if not secret: + logging.error("Can't update secret for relation %s", relation.id) + return False + + old_content = secret.get_content() + full_content = copy.deepcopy(old_content) + full_content.update(content) + secret.set_content(full_content) + + # Return True on success + return True + + def _add_or_update_relation_secrets( + self, + relation: Relation, + group: SecretGroup, + secret_fields: Set[str], + data: Dict[str, str], + ) -> bool: + """Update contents for Secret group. If the Secret doesn't exist, create it.""" + secret_content = self._content_for_secret_group(data, secret_fields, group) + if self._get_relation_secret(relation.id, group): + return self._update_relation_secret(relation, secret_content, group) + else: + return self._add_relation_secret(relation, secret_content, group) + + @juju_secrets_only + def _delete_relation_secret( + self, relation: Relation, group: SecretGroup, secret_fields: List[str], fields: List[str] + ) -> bool: + """Update the contents of an existing Juju Secret, referred in the relation databag.""" + secret = self._get_relation_secret(relation.id, group) + + if not secret: + logging.error("Can't delete secret for relation %s", str(relation.id)) + return False + + old_content = secret.get_content() + new_content = copy.deepcopy(old_content) + for field in fields: + try: + new_content.pop(field) + except KeyError: + logging.error( + "Non-existing secret was attempted to be removed %s, %s", + str(relation.id), + str(field), + ) + return False + + secret.set_content(new_content) + + # Remove secret from the relation if it's fully gone + if not new_content: + field = self._generate_secret_field_name(group) + try: + relation.data[self.local_app].pop(field) + except KeyError: + pass + + # Return the content that was removed + return True + + # Mandatory internal overrides + + @juju_secrets_only + def _get_relation_secret( + self, relation_id: int, group_mapping: SecretGroup, relation_name: Optional[str] = None + ) -> Optional[CachedSecret]: + """Retrieve a Juju Secret that's been stored in the relation databag.""" + if not relation_name: + relation_name = self.relation_name + + label = self._generate_secret_label(relation_name, relation_id, group_mapping) + if secret := self.secrets.get(label): + return secret + + relation = self.charm.model.get_relation(relation_name, relation_id) + if not relation: + return + + secret_field = self._generate_secret_field_name(group_mapping) + if secret_uri := relation.data[self.local_app].get(secret_field): + return self.secrets.get(label, secret_uri) + + def _fetch_specific_relation_data( + self, relation: Relation, fields: Optional[List[str]] + ) -> Dict[str, str]: + """Fetching relation data for Provides. + + NOTE: Since all secret fields are in the Provides side of the databag, we don't need to worry about that + """ + if not relation.app: + return {} + + return self._fetch_relation_data_without_secrets(relation.app, relation, fields) + + def _fetch_my_specific_relation_data( + self, relation: Relation, fields: Optional[List[str]] + ) -> dict: + """Fetching our own relation data.""" + secret_fields = None + if relation.app: + secret_fields = get_encoded_list(relation, relation.app, REQ_SECRET_FIELDS) + + return self._fetch_relation_data_with_secrets( + self.local_app, + secret_fields, + relation, + fields, + ) + + def _update_relation_data(self, relation: Relation, data: Dict[str, str]) -> None: + """Set values for fields not caring whether it's a secret or not.""" + req_secret_fields = [] + if relation.app: + req_secret_fields = get_encoded_list(relation, relation.app, REQ_SECRET_FIELDS) + + _, normal_fields = self._process_secret_fields( + relation, + req_secret_fields, + list(data), + self._add_or_update_relation_secrets, + data=data, + ) + + normal_content = {k: v for k, v in data.items() if k in normal_fields} + self._update_relation_data_without_secrets(self.local_app, relation, normal_content) + + def _delete_relation_data(self, relation: Relation, fields: List[str]) -> None: + """Delete fields from the Relation not caring whether it's a secret or not.""" + req_secret_fields = [] + if relation.app: + req_secret_fields = get_encoded_list(relation, relation.app, REQ_SECRET_FIELDS) + + _, normal_fields = self._process_secret_fields( + relation, req_secret_fields, fields, self._delete_relation_secret, fields=fields + ) + self._delete_relation_data_without_secrets(self.local_app, relation, list(normal_fields)) + + # Public methods - "native" def set_credentials(self, relation_id: int, username: str, password: str) -> None: """Set credentials. @@ -445,13 +1213,7 @@ def set_credentials(self, relation_id: int, username: str, password: str) -> Non username: user that was created. password: password of the created user. """ - self._update_relation_data( - relation_id, - { - "username": username, - "password": password, - }, - ) + self.update_relation_data(relation_id, {"username": username, "password": password}) def set_tls(self, relation_id: int, tls: str) -> None: """Set whether TLS is enabled. @@ -460,7 +1222,7 @@ def set_tls(self, relation_id: int, tls: str) -> None: relation_id: the identifier for a particular relation. tls: whether tls is enabled (True or False). """ - self._update_relation_data(relation_id, {"tls": tls}) + self.update_relation_data(relation_id, {"tls": tls}) def set_tls_ca(self, relation_id: int, tls_ca: str) -> None: """Set the TLS CA in the application relation databag. @@ -469,108 +1231,98 @@ def set_tls_ca(self, relation_id: int, tls_ca: str) -> None: relation_id: the identifier for a particular relation. tls_ca: TLS certification authority. """ - self._update_relation_data(relation_id, {"tls-ca": tls_ca}) + self.update_relation_data(relation_id, {"tls-ca": tls_ca}) -class DataRequires(Object, ABC): +class DataRequires(DataRelation): """Requires-side of the relation.""" + SECRET_FIELDS = ["username", "password", "tls", "tls-ca", "uris"] + def __init__( self, charm, relation_name: str, - extra_user_roles: str = None, + extra_user_roles: Optional[str] = None, + additional_secret_fields: Optional[List[str]] = [], ): """Manager of base client relations.""" super().__init__(charm, relation_name) - self.charm = charm self.extra_user_roles = extra_user_roles - self.local_app = self.charm.model.app - self.local_unit = self.charm.unit - self.relation_name = relation_name + self._secret_fields = list(self.SECRET_FIELDS) + if additional_secret_fields: + self._secret_fields += additional_secret_fields + self.framework.observe( - self.charm.on[relation_name].relation_joined, self._on_relation_joined_event + self.charm.on[relation_name].relation_created, self._on_relation_created_event ) self.framework.observe( - self.charm.on[relation_name].relation_changed, self._on_relation_changed_event + charm.on.secret_changed, + self._on_secret_changed_event, ) - @abstractmethod - def _on_relation_joined_event(self, event: RelationJoinedEvent) -> None: - """Event emitted when the application joins the relation.""" - raise NotImplementedError - - @abstractmethod - def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: - raise NotImplementedError + @property + def secret_fields(self) -> Optional[List[str]]: + """Local access to secrets field, in case they are being used.""" + if self.secrets_enabled: + return self._secret_fields - def fetch_relation_data(self) -> dict: - """Retrieves data from relation. + def _diff(self, event: RelationChangedEvent) -> Diff: + """Retrieves the diff of the data in the relation changed databag. - This function can be used to retrieve data from a relation - in the charm code when outside an event callback. - Function cannot be used in `*-relation-broken` events and will raise an exception. + Args: + event: relation changed event. Returns: - a dict of the values stored in the relation data bag - for all relation instances (indexed by the relation ID). + a Diff instance containing the added, deleted and changed + keys from the event relation databag. """ - data = {} - for relation in self.relations: - data[relation.id] = { - key: value for key, value in relation.data[relation.app].items() if key != "data" - } - return data + return diff(event, self.local_unit) - def _update_relation_data(self, relation_id: int, data: dict) -> None: - """Updates a set of key-value pairs in the relation. + # Internal helper functions - This function writes in the application data bag, therefore, - only the leader unit can call it. + def _register_secret_to_relation( + self, relation_name: str, relation_id: int, secret_id: str, group: SecretGroup + ): + """Fetch secrets and apply local label on them. - Args: - relation_id: the identifier for a particular relation. - data: dict containing the key-value pairs - that should be updated in the relation. + [MAGIC HERE] + If we fetch a secret using get_secret(id=, label=), + then will be "stuck" on the Secret object, whenever it may + appear (i.e. as an event attribute, or fetched manually) on future occasions. + + This will allow us to uniquely identify the secret on Provides side (typically on + 'secret-changed' events), and map it to the corresponding relation. """ - if self.local_unit.is_leader(): - relation = self.charm.model.get_relation(self.relation_name, relation_id) - relation.data[self.local_app].update(data) + label = self._generate_secret_label(relation_name, relation_id, group) - def _diff(self, event: RelationChangedEvent) -> Diff: - """Retrieves the diff of the data in the relation changed databag. + # Fetchin the Secret's meta information ensuring that it's locally getting registered with + CachedSecret(self.charm, label, secret_id).meta - Args: - event: relation changed event. + def _register_secrets_to_relation(self, relation: Relation, params_name_list: List[str]): + """Make sure that secrets of the provided list are locally 'registered' from the databag. - Returns: - a Diff instance containing the added, deleted and changed - keys from the event relation databag. + More on 'locally registered' magic is described in _register_secret_to_relation() method """ - return diff(event, self.local_unit) + if not relation.app: + return - @property - def relations(self) -> List[Relation]: - """The list of Relation instances associated with this relation_name.""" - return [ - relation - for relation in self.charm.model.relations[self.relation_name] - if self._is_relation_active(relation) - ] + for group in SecretGroup: + secret_field = self._generate_secret_field_name(group) + if secret_field in params_name_list: + if secret_uri := relation.data[relation.app].get(secret_field): + self._register_secret_to_relation( + relation.name, relation.id, secret_uri, group + ) - @staticmethod - def _is_relation_active(relation: Relation): - try: - _ = repr(relation.data) - return True - except RuntimeError: + def _is_resource_created_for_relation(self, relation: Relation) -> bool: + if not relation.app: return False - @staticmethod - def _is_resource_created_for_relation(relation: Relation): - return ( - "username" in relation.data[relation.app] and "password" in relation.data[relation.app] + data = self.fetch_relation_data([relation.id], ["username", "password"]).get( + relation.id, {} ) + return bool(data.get("username")) and bool(data.get("password")) def is_resource_created(self, relation_id: Optional[int] = None) -> bool: """Check if the resource has been created. @@ -599,15 +1351,81 @@ def is_resource_created(self, relation_id: Optional[int] = None) -> bool: else: return ( all( - [ - self._is_resource_created_for_relation(relation) - for relation in self.relations - ] + self._is_resource_created_for_relation(relation) for relation in self.relations ) if self.relations else False ) + # Event handlers + + def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: + """Event emitted when the relation is created.""" + if not self.local_unit.is_leader(): + return + + if self.secret_fields: + set_encoded_field( + event.relation, self.charm.app, REQ_SECRET_FIELDS, self.secret_fields + ) + + @abstractmethod + def _on_secret_changed_event(self, event: RelationChangedEvent) -> None: + """Event emitted when the relation data has changed.""" + raise NotImplementedError + + # Mandatory internal overrides + + @juju_secrets_only + def _get_relation_secret( + self, relation_id: int, group: SecretGroup, relation_name: Optional[str] = None + ) -> Optional[CachedSecret]: + """Retrieve a Juju Secret that's been stored in the relation databag.""" + if not relation_name: + relation_name = self.relation_name + + label = self._generate_secret_label(relation_name, relation_id, group) + return self.secrets.get(label) + + def _fetch_specific_relation_data( + self, relation, fields: Optional[List[str]] = None + ) -> Dict[str, str]: + """Fetching Requires data -- that may include secrets.""" + if not relation.app: + return {} + return self._fetch_relation_data_with_secrets( + relation.app, self.secret_fields, relation, fields + ) + + def _fetch_my_specific_relation_data(self, relation, fields: Optional[List[str]]) -> dict: + """Fetching our own relation data.""" + return self._fetch_relation_data_without_secrets(self.local_app, relation, fields) + + def _update_relation_data(self, relation: Relation, data: dict) -> None: + """Updates a set of key-value pairs in the relation. + + This function writes in the application data bag, therefore, + only the leader unit can call it. + + Args: + relation: the particular relation. + data: dict containing the key-value pairs + that should be updated in the relation. + """ + return self._update_relation_data_without_secrets(self.local_app, relation, data) + + def _delete_relation_data(self, relation: Relation, fields: List[str]) -> None: + """Deletes a set of fields from the relation. + + This function writes in the application data bag, therefore, + only the leader unit can call it. + + Args: + relation: the particular relation. + fields: list containing the field names that should be removed from the relation. + """ + return self._delete_relation_data_without_secrets(self.local_app, relation, fields) + # General events @@ -618,30 +1436,108 @@ class ExtraRoleEvent(RelationEvent): @property def extra_user_roles(self) -> Optional[str]: """Returns the extra user roles that were requested.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("extra-user-roles") class AuthenticationEvent(RelationEvent): - """Base class for authentication fields for events.""" + """Base class for authentication fields for events. + + The amount of logic added here is not ideal -- but this was the only way to preserve + the interface when moving to Juju Secrets + """ + + @property + def _secrets(self) -> dict: + """Caching secrets to avoid fetching them each time a field is referrd. + + DON'T USE the encapsulated helper variable outside of this function + """ + if not hasattr(self, "_cached_secrets"): + self._cached_secrets = {} + return self._cached_secrets + + @property + def _jujuversion(self) -> JujuVersion: + """Caching jujuversion to avoid a Juju call on each field evaluation. + + DON'T USE the encapsulated helper variable outside of this function + """ + if not hasattr(self, "_cached_jujuversion"): + self._cached_jujuversion = None + if not self._cached_jujuversion: + self._cached_jujuversion = JujuVersion.from_environ() + return self._cached_jujuversion + + def _get_secret(self, group) -> Optional[Dict[str, str]]: + """Retrieveing secrets.""" + if not self.app: + return + if not self._secrets.get(group): + self._secrets[group] = None + secret_field = f"{PROV_SECRET_PREFIX}{group}" + if secret_uri := self.relation.data[self.app].get(secret_field): + secret = self.framework.model.get_secret(id=secret_uri) + self._secrets[group] = secret.get_content() + return self._secrets[group] + + @property + def secrets_enabled(self): + """Is this Juju version allowing for Secrets usage?""" + return self._jujuversion.has_secrets @property def username(self) -> Optional[str]: """Returns the created username.""" + if not self.relation.app: + return None + + if self.secrets_enabled: + secret = self._get_secret("user") + if secret: + return secret.get("username") + return self.relation.data[self.relation.app].get("username") @property def password(self) -> Optional[str]: """Returns the password for the created user.""" + if not self.relation.app: + return None + + if self.secrets_enabled: + secret = self._get_secret("user") + if secret: + return secret.get("password") + return self.relation.data[self.relation.app].get("password") @property def tls(self) -> Optional[str]: """Returns whether TLS is configured.""" + if not self.relation.app: + return None + + if self.secrets_enabled: + secret = self._get_secret("tls") + if secret: + return secret.get("tls") + return self.relation.data[self.relation.app].get("tls") @property def tls_ca(self) -> Optional[str]: """Returns TLS CA.""" + if not self.relation.app: + return None + + if self.secrets_enabled: + secret = self._get_secret("tls") + if secret: + return secret.get("tls-ca") + return self.relation.data[self.relation.app].get("tls-ca") @@ -654,6 +1550,9 @@ class DatabaseProvidesEvent(RelationEvent): @property def database(self) -> Optional[str]: """Returns the database that was requested.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("database") @@ -676,6 +1575,9 @@ class DatabaseRequiresEvent(RelationEvent): @property def database(self) -> Optional[str]: """Returns the database name.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("database") @property @@ -685,6 +1587,9 @@ def endpoints(self) -> Optional[str]: In VM charms, this is the primary's address. In kubernetes charms, this is the service to the primary pod. """ + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("endpoints") @property @@ -694,6 +1599,9 @@ def read_only_endpoints(self) -> Optional[str]: In VM charms, this is the address of all the secondary instances. In kubernetes charms, this is the service to all replica pod instances. """ + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("read-only-endpoints") @property @@ -702,6 +1610,9 @@ def replset(self) -> Optional[str]: MongoDB only. """ + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("replset") @property @@ -710,6 +1621,9 @@ def uris(self) -> Optional[str]: MongoDB, Redis, OpenSearch. """ + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("uris") @property @@ -718,6 +1632,9 @@ def version(self) -> Optional[str]: Version as informed by the database daemon. """ + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("version") @@ -750,24 +1667,25 @@ class DatabaseRequiresEvents(CharmEvents): class DatabaseProvides(DataProvides): """Provider-side of the database relations.""" - on = DatabaseProvidesEvents() + on = DatabaseProvidesEvents() # pyright: ignore [reportGeneralTypeIssues] def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) - def _on_relation_changed(self, event: RelationChangedEvent) -> None: + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the relation has changed.""" - # Only the leader should handle this event. + # Leader only if not self.local_unit.is_leader(): return - # Check which data has changed to emit customs events. diff = self._diff(event) # Emit a database requested event if the setup key (database name and optional # extra user roles) was added to the relation databag by the application. if "database" in diff.added: - self.on.database_requested.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "database_requested").emit( + event.relation, app=event.app, unit=event.unit + ) def set_database(self, relation_id: int, database_name: str) -> None: """Set database name. @@ -779,7 +1697,7 @@ def set_database(self, relation_id: int, database_name: str) -> None: relation_id: the identifier for a particular relation. database_name: database name. """ - self._update_relation_data(relation_id, {"database": database_name}) + self.update_relation_data(relation_id, {"database": database_name}) def set_endpoints(self, relation_id: int, connection_strings: str) -> None: """Set database primary connections. @@ -795,7 +1713,7 @@ def set_endpoints(self, relation_id: int, connection_strings: str) -> None: relation_id: the identifier for a particular relation. connection_strings: database hosts and ports comma separated list. """ - self._update_relation_data(relation_id, {"endpoints": connection_strings}) + self.update_relation_data(relation_id, {"endpoints": connection_strings}) def set_read_only_endpoints(self, relation_id: int, connection_strings: str) -> None: """Set database replicas connection strings. @@ -807,7 +1725,7 @@ def set_read_only_endpoints(self, relation_id: int, connection_strings: str) -> relation_id: the identifier for a particular relation. connection_strings: database hosts and ports comma separated list. """ - self._update_relation_data(relation_id, {"read-only-endpoints": connection_strings}) + self.update_relation_data(relation_id, {"read-only-endpoints": connection_strings}) def set_replset(self, relation_id: int, replset: str) -> None: """Set replica set name in the application relation databag. @@ -818,7 +1736,7 @@ def set_replset(self, relation_id: int, replset: str) -> None: relation_id: the identifier for a particular relation. replset: replica set name. """ - self._update_relation_data(relation_id, {"replset": replset}) + self.update_relation_data(relation_id, {"replset": replset}) def set_uris(self, relation_id: int, uris: str) -> None: """Set the database connection URIs in the application relation databag. @@ -829,7 +1747,7 @@ def set_uris(self, relation_id: int, uris: str) -> None: relation_id: the identifier for a particular relation. uris: connection URIs. """ - self._update_relation_data(relation_id, {"uris": uris}) + self.update_relation_data(relation_id, {"uris": uris}) def set_version(self, relation_id: int, version: str) -> None: """Set the database version in the application relation databag. @@ -838,24 +1756,25 @@ def set_version(self, relation_id: int, version: str) -> None: relation_id: the identifier for a particular relation. version: database version. """ - self._update_relation_data(relation_id, {"version": version}) + self.update_relation_data(relation_id, {"version": version}) class DatabaseRequires(DataRequires): """Requires-side of the database relation.""" - on = DatabaseRequiresEvents() + on = DatabaseRequiresEvents() # pyright: ignore [reportGeneralTypeIssues] def __init__( self, charm, relation_name: str, database_name: str, - extra_user_roles: str = None, - relations_aliases: List[str] = None, + extra_user_roles: Optional[str] = None, + relations_aliases: Optional[List[str]] = None, + additional_secret_fields: Optional[List[str]] = [], ): """Manager of database client relations.""" - super().__init__(charm, relation_name, extra_user_roles) + super().__init__(charm, relation_name, extra_user_roles, additional_secret_fields) self.database = database_name self.relations_aliases = relations_aliases @@ -880,6 +1799,10 @@ def __init__( DatabaseReadOnlyEndpointsChangedEvent, ) + def _on_secret_changed_event(self, event: SecretChangedEvent): + """Event notifying about a new value of a secret.""" + pass + def _assign_relation_alias(self, relation_id: int) -> None: """Assigns an alias to a relation. @@ -894,11 +1817,8 @@ def _assign_relation_alias(self, relation_id: int) -> None: # Return if an alias was already assigned to this relation # (like when there are more than one unit joining the relation). - if ( - self.charm.model.get_relation(self.relation_name, relation_id) - .data[self.local_unit] - .get("alias") - ): + relation = self.charm.model.get_relation(self.relation_name, relation_id) + if relation and relation.data[self.local_unit].get("alias"): return # Retrieve the available aliases (the ones that weren't assigned to any relation). @@ -911,7 +1831,13 @@ def _assign_relation_alias(self, relation_id: int) -> None: # Set the alias in the unit relation databag of the specific relation. relation = self.charm.model.get_relation(self.relation_name, relation_id) - relation.data[self.local_unit].update({"alias": available_aliases[0]}) + if relation: + relation.data[self.local_unit].update({"alias": available_aliases[0]}) + + # We need to set relation alias also on the application level so, + # it will be accessible in show-unit juju command, executed for a consumer application unit + if self.local_unit.is_leader(): + self.update_relation_data(relation_id, {"alias": available_aliases[0]}) def _emit_aliased_event(self, event: RelationChangedEvent, event_name: str) -> None: """Emit an aliased event to a particular relation if it has an alias. @@ -958,23 +1884,30 @@ def is_postgresql_plugin_enabled(self, plugin: str, relation_index: int = 0) -> if len(self.relations) == 0: return False - relation_data = self.fetch_relation_data()[self.relations[relation_index].id] - host = relation_data.get("endpoints") + relation_id = self.relations[relation_index].id + host = self.fetch_relation_field(relation_id, "endpoints") # Return False if there is no endpoint available. if host is None: return False host = host.split(":")[0] - user = relation_data.get("username") - password = relation_data.get("password") + + content = self.fetch_relation_data([relation_id], ["username", "password"]).get( + relation_id, {} + ) + user = content.get("username") + password = content.get("password") + connection_string = ( f"host='{host}' dbname='{self.database}' user='{user}' password='{password}'" ) try: with psycopg.connect(connection_string) as connection: with connection.cursor() as cursor: - cursor.execute(f"SELECT TRUE FROM pg_extension WHERE extname='{plugin}';") + cursor.execute( + "SELECT TRUE FROM pg_extension WHERE extname=%s::text;", (plugin,) + ) return cursor.fetchone() is not None except psycopg.Error as e: logger.exception( @@ -982,15 +1915,20 @@ def is_postgresql_plugin_enabled(self, plugin: str, relation_index: int = 0) -> ) return False - def _on_relation_joined_event(self, event: RelationJoinedEvent) -> None: - """Event emitted when the application joins the database relation.""" + def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: + """Event emitted when the database relation is created.""" + super()._on_relation_created_event(event) + # If relations aliases were provided, assign one to the relation. self._assign_relation_alias(event.relation.id) # Sets both database and extra user roles in the relation # if the roles are provided. Otherwise, sets only the database. + if not self.local_unit.is_leader(): + return + if self.extra_user_roles: - self._update_relation_data( + self.update_relation_data( event.relation.id, { "database": self.database, @@ -998,19 +1936,28 @@ def _on_relation_joined_event(self, event: RelationJoinedEvent) -> None: }, ) else: - self._update_relation_data(event.relation.id, {"database": self.database}) + self.update_relation_data(event.relation.id, {"database": self.database}) def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the database relation has changed.""" # Check which data has changed to emit customs events. diff = self._diff(event) + # Register all new secrets with their labels + if any(newval for newval in diff.added if self._is_secret_field(newval)): + self._register_secrets_to_relation(event.relation, diff.added) + # Check if the database is created # (the database charm shared the credentials). - if "username" in diff.added and "password" in diff.added: + secret_field_user = self._generate_secret_field_name(SecretGroup.USER) + if ( + "username" in diff.added and "password" in diff.added + ) or secret_field_user in diff.added: # Emit the default event (the one without an alias). logger.info("database created at %s", datetime.now()) - self.on.database_created.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "database_created").emit( + event.relation, app=event.app, unit=event.unit + ) # Emit the aliased event (if any). self._emit_aliased_event(event, "database_created") @@ -1024,7 +1971,9 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: if "endpoints" in diff.added or "endpoints" in diff.changed: # Emit the default event (the one without an alias). logger.info("endpoints changed on %s", datetime.now()) - self.on.endpoints_changed.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "endpoints_changed").emit( + event.relation, app=event.app, unit=event.unit + ) # Emit the aliased event (if any). self._emit_aliased_event(event, "endpoints_changed") @@ -1038,7 +1987,7 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: if "read-only-endpoints" in diff.added or "read-only-endpoints" in diff.changed: # Emit the default event (the one without an alias). logger.info("read-only-endpoints changed on %s", datetime.now()) - self.on.read_only_endpoints_changed.emit( + getattr(self.on, "read_only_endpoints_changed").emit( event.relation, app=event.app, unit=event.unit ) @@ -1055,11 +2004,17 @@ class KafkaProvidesEvent(RelationEvent): @property def topic(self) -> Optional[str]: """Returns the topic that was requested.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("topic") @property def consumer_group_prefix(self) -> Optional[str]: """Returns the consumer-group-prefix that was requested.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("consumer-group-prefix") @@ -1082,21 +2037,33 @@ class KafkaRequiresEvent(RelationEvent): @property def topic(self) -> Optional[str]: """Returns the topic.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("topic") @property def bootstrap_server(self) -> Optional[str]: """Returns a comma-separated list of broker uris.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("endpoints") @property def consumer_group_prefix(self) -> Optional[str]: """Returns the consumer-group-prefix.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("consumer-group-prefix") @property def zookeeper_uris(self) -> Optional[str]: """Returns a comma separated list of Zookeeper uris.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("zookeeper-uris") @@ -1124,14 +2091,14 @@ class KafkaRequiresEvents(CharmEvents): class KafkaProvides(DataProvides): """Provider-side of the Kafka relation.""" - on = KafkaProvidesEvents() + on = KafkaProvidesEvents() # pyright: ignore [reportGeneralTypeIssues] def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) - def _on_relation_changed(self, event: RelationChangedEvent) -> None: + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the relation has changed.""" - # Only the leader should handle this event. + # Leader only if not self.local_unit.is_leader(): return @@ -1141,7 +2108,9 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None: # Emit a topic requested event if the setup key (topic name and optional # extra user roles) was added to the relation databag by the application. if "topic" in diff.added: - self.on.topic_requested.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "topic_requested").emit( + event.relation, app=event.app, unit=event.unit + ) def set_topic(self, relation_id: int, topic: str) -> None: """Set topic name in the application relation databag. @@ -1150,7 +2119,7 @@ def set_topic(self, relation_id: int, topic: str) -> None: relation_id: the identifier for a particular relation. topic: the topic name. """ - self._update_relation_data(relation_id, {"topic": topic}) + self.update_relation_data(relation_id, {"topic": topic}) def set_bootstrap_server(self, relation_id: int, bootstrap_server: str) -> None: """Set the bootstrap server in the application relation databag. @@ -1159,7 +2128,7 @@ def set_bootstrap_server(self, relation_id: int, bootstrap_server: str) -> None: relation_id: the identifier for a particular relation. bootstrap_server: the bootstrap server address. """ - self._update_relation_data(relation_id, {"endpoints": bootstrap_server}) + self.update_relation_data(relation_id, {"endpoints": bootstrap_server}) def set_consumer_group_prefix(self, relation_id: int, consumer_group_prefix: str) -> None: """Set the consumer group prefix in the application relation databag. @@ -1168,7 +2137,7 @@ def set_consumer_group_prefix(self, relation_id: int, consumer_group_prefix: str relation_id: the identifier for a particular relation. consumer_group_prefix: the consumer group prefix string. """ - self._update_relation_data(relation_id, {"consumer-group-prefix": consumer_group_prefix}) + self.update_relation_data(relation_id, {"consumer-group-prefix": consumer_group_prefix}) def set_zookeeper_uris(self, relation_id: int, zookeeper_uris: str) -> None: """Set the zookeeper uris in the application relation databag. @@ -1177,13 +2146,13 @@ def set_zookeeper_uris(self, relation_id: int, zookeeper_uris: str) -> None: relation_id: the identifier for a particular relation. zookeeper_uris: comma-separated list of ZooKeeper server uris. """ - self._update_relation_data(relation_id, {"zookeeper-uris": zookeeper_uris}) + self.update_relation_data(relation_id, {"zookeeper-uris": zookeeper_uris}) class KafkaRequires(DataRequires): """Requires-side of the Kafka relation.""" - on = KafkaRequiresEvents() + on = KafkaRequiresEvents() # pyright: ignore [reportGeneralTypeIssues] def __init__( self, @@ -1192,23 +2161,45 @@ def __init__( topic: str, extra_user_roles: Optional[str] = None, consumer_group_prefix: Optional[str] = None, + additional_secret_fields: Optional[List[str]] = [], ): """Manager of Kafka client relations.""" # super().__init__(charm, relation_name) - super().__init__(charm, relation_name, extra_user_roles) + super().__init__(charm, relation_name, extra_user_roles, additional_secret_fields) self.charm = charm self.topic = topic self.consumer_group_prefix = consumer_group_prefix or "" - def _on_relation_joined_event(self, event: RelationJoinedEvent) -> None: - """Event emitted when the application joins the Kafka relation.""" + @property + def topic(self): + """Topic to use in Kafka.""" + return self._topic + + @topic.setter + def topic(self, value): + # Avoid wildcards + if value == "*": + raise ValueError(f"Error on topic '{value}', cannot be a wildcard.") + self._topic = value + + def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: + """Event emitted when the Kafka relation is created.""" + super()._on_relation_created_event(event) + + if not self.local_unit.is_leader(): + return + # Sets topic, extra user roles, and "consumer-group-prefix" in the relation relation_data = { f: getattr(self, f.replace("-", "_"), "") for f in ["consumer-group-prefix", "extra-user-roles", "topic"] } - self._update_relation_data(event.relation.id, relation_data) + self.update_relation_data(event.relation.id, relation_data) + + def _on_secret_changed_event(self, event: SecretChangedEvent): + """Event notifying about a new value of a secret.""" + pass def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the Kafka relation has changed.""" @@ -1217,10 +2208,18 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: # Check if the topic is created # (the Kafka charm shared the credentials). - if "username" in diff.added and "password" in diff.added: + + # Register all new secrets with their labels + if any(newval for newval in diff.added if self._is_secret_field(newval)): + self._register_secrets_to_relation(event.relation, diff.added) + + secret_field_user = self._generate_secret_field_name(SecretGroup.USER) + if ( + "username" in diff.added and "password" in diff.added + ) or secret_field_user in diff.added: # Emit the default event (the one without an alias). logger.info("topic created at %s", datetime.now()) - self.on.topic_created.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "topic_created").emit(event.relation, app=event.app, unit=event.unit) # To avoid unnecessary application restarts do not trigger # “endpoints_changed“ event if “topic_created“ is triggered. @@ -1231,7 +2230,7 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: if "endpoints" in diff.added or "endpoints" in diff.changed: # Emit the default event (the one without an alias). logger.info("endpoints changed on %s", datetime.now()) - self.on.bootstrap_server_changed.emit( + getattr(self.on, "bootstrap_server_changed").emit( event.relation, app=event.app, unit=event.unit ) # here check if this is the right design return @@ -1246,6 +2245,9 @@ class OpenSearchProvidesEvent(RelationEvent): @property def index(self) -> Optional[str]: """Returns the index that was requested.""" + if not self.relation.app: + return None + return self.relation.data[self.relation.app].get("index") @@ -1287,24 +2289,25 @@ class OpenSearchRequiresEvents(CharmEvents): class OpenSearchProvides(DataProvides): """Provider-side of the OpenSearch relation.""" - on = OpenSearchProvidesEvents() + on = OpenSearchProvidesEvents() # pyright: ignore[reportGeneralTypeIssues] def __init__(self, charm: CharmBase, relation_name: str) -> None: super().__init__(charm, relation_name) - def _on_relation_changed(self, event: RelationChangedEvent) -> None: + def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the relation has changed.""" - # Only the leader should handle this event. + # Leader only if not self.local_unit.is_leader(): return - # Check which data has changed to emit customs events. diff = self._diff(event) # Emit an index requested event if the setup key (index name and optional extra user roles) # have been added to the relation databag by the application. if "index" in diff.added: - self.on.index_requested.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "index_requested").emit( + event.relation, app=event.app, unit=event.unit + ) def set_index(self, relation_id: int, index: str) -> None: """Set the index in the application relation databag. @@ -1315,7 +2318,7 @@ def set_index(self, relation_id: int, index: str) -> None: requested index, and can be used to present a different index name if, for example, the requested index is invalid. """ - self._update_relation_data(relation_id, {"index": index}) + self.update_relation_data(relation_id, {"index": index}) def set_endpoints(self, relation_id: int, endpoints: str) -> None: """Set the endpoints in the application relation databag. @@ -1324,7 +2327,7 @@ def set_endpoints(self, relation_id: int, endpoints: str) -> None: relation_id: the identifier for a particular relation. endpoints: the endpoint addresses for opensearch nodes. """ - self._update_relation_data(relation_id, {"endpoints": endpoints}) + self.update_relation_data(relation_id, {"endpoints": endpoints}) def set_version(self, relation_id: int, version: str) -> None: """Set the opensearch version in the application relation databag. @@ -1333,31 +2336,66 @@ def set_version(self, relation_id: int, version: str) -> None: relation_id: the identifier for a particular relation. version: database version. """ - self._update_relation_data(relation_id, {"version": version}) + self.update_relation_data(relation_id, {"version": version}) class OpenSearchRequires(DataRequires): """Requires-side of the OpenSearch relation.""" - on = OpenSearchRequiresEvents() + on = OpenSearchRequiresEvents() # pyright: ignore[reportGeneralTypeIssues] def __init__( - self, charm, relation_name: str, index: str, extra_user_roles: Optional[str] = None + self, + charm, + relation_name: str, + index: str, + extra_user_roles: Optional[str] = None, + additional_secret_fields: Optional[List[str]] = [], ): """Manager of OpenSearch client relations.""" - super().__init__(charm, relation_name, extra_user_roles) + super().__init__(charm, relation_name, extra_user_roles, additional_secret_fields) self.charm = charm self.index = index - def _on_relation_joined_event(self, event: RelationJoinedEvent) -> None: - """Event emitted when the application joins the OpenSearch relation.""" + def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: + """Event emitted when the OpenSearch relation is created.""" + super()._on_relation_created_event(event) + + if not self.local_unit.is_leader(): + return + # Sets both index and extra user roles in the relation if the roles are provided. # Otherwise, sets only the index. data = {"index": self.index} if self.extra_user_roles: data["extra-user-roles"] = self.extra_user_roles - self._update_relation_data(event.relation.id, data) + self.update_relation_data(event.relation.id, data) + + def _on_secret_changed_event(self, event: SecretChangedEvent): + """Event notifying about a new value of a secret.""" + if not event.secret.label: + return + + relation = self._relation_from_secret_label(event.secret.label) + if not relation: + logging.info( + f"Received secret {event.secret.label} but couldn't parse, seems irrelevant" + ) + return + + if relation.app == self.charm.app: + logging.info("Secret changed event ignored for Secret Owner") + + remote_unit = None + for unit in relation.units: + if unit.app != self.charm.app: + remote_unit = unit + + logger.info("authentication updated") + getattr(self.on, "authentication_updated").emit( + relation, app=relation.app, unit=remote_unit + ) def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: """Event emitted when the OpenSearch relation has changed. @@ -1367,18 +2405,27 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: # Check which data has changed to emit customs events. diff = self._diff(event) - # Check if authentication has updated, emit event if so - updates = {"username", "password", "tls", "tls-ca"} + # Register all new secrets with their labels + if any(newval for newval in diff.added if self._is_secret_field(newval)): + self._register_secrets_to_relation(event.relation, diff.added) + + secret_field_user = self._generate_secret_field_name(SecretGroup.USER) + secret_field_tls = self._generate_secret_field_name(SecretGroup.TLS) + updates = {"username", "password", "tls", "tls-ca", secret_field_user, secret_field_tls} if len(set(diff._asdict().keys()) - updates) < len(diff): logger.info("authentication updated at: %s", datetime.now()) - self.on.authentication_updated.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "authentication_updated").emit( + event.relation, app=event.app, unit=event.unit + ) # Check if the index is created # (the OpenSearch charm shares the credentials). - if "username" in diff.added and "password" in diff.added: + if ( + "username" in diff.added and "password" in diff.added + ) or secret_field_user in diff.added: # Emit the default event (the one without an alias). logger.info("index created at: %s", datetime.now()) - self.on.index_created.emit(event.relation, app=event.app, unit=event.unit) + getattr(self.on, "index_created").emit(event.relation, app=event.app, unit=event.unit) # To avoid unnecessary application restarts do not trigger # “endpoints_changed“ event if “index_created“ is triggered. @@ -1389,7 +2436,7 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: if "endpoints" in diff.added or "endpoints" in diff.changed: # Emit the default event (the one without an alias). logger.info("endpoints changed on %s", datetime.now()) - self.on.endpoints_changed.emit( + getattr(self.on, "endpoints_changed").emit( event.relation, app=event.app, unit=event.unit ) # here check if this is the right design return diff --git a/backend/charm/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/backend/charm/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index fd2f54d6..9a7baa80 100644 --- a/backend/charm/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/backend/charm/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -1,5 +1,5 @@ # Copyright 2023 Canonical Ltd. -# Licensed under the Apache2.0, see LICENCE file in charm source for details. +# Licensed under the Apache2.0. See LICENSE file in charm source for details. """Library for the nginx-route relation. This library contains the require and provide functions for handling @@ -34,12 +34,12 @@ ```python from charms.nginx_ingress_integrator.v0.nginx_route import NginxRouteRequirer -# In your charm's `__init__` method. +# In your charm's `__init__` method (assuming your app is listening on port 8080). require_nginx_route( charm=self, - service_hostname=self.config["external_hostname"], + service_hostname=self.app.name, service_name=self.app.name, - service_port=80 + service_port=8080 ) ``` @@ -52,6 +52,23 @@ 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. + +In the example above we're setting `service_hostname` (which translates to the +external hostname for the application when related to nginx-ingress-integrator) +to `self.app.name` here. This ensures by default the charm will be available on +the name of the deployed juju application, but can be overridden in a +production deployment by setting `service-hostname` on the +nginx-ingress-integrator charm. For example: +```bash +juju deploy nginx-ingress-integrator +juju deploy my-charm +juju relate nginx-ingress-integrator my-charm:nginx-route +# The service is now reachable on the ingress IP(s) of your k8s cluster at +# 'http://my-charm'. +juju config nginx-ingress-integrator service-hostname='my-charm.example.com' +# The service is now reachable on the ingress IP(s) of your k8s cluster at +# 'http://my-charm.example.com'. +``` """ import logging import typing @@ -69,7 +86,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 2 +LIBPATCH = 4 __all__ = ["require_nginx_route", "provide_nginx_route"] @@ -153,7 +170,10 @@ def _config_reconciliation(self, _event: typing.Any = None) -> None: 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 +# C901 is ignored since the method has too many ifs but wouldn't be +# necessarily good to reduce to smaller methods. +# E501: line too long +def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,too-many-arguments # noqa: C901,E501 *, charm: ops.charm.CharmBase, service_hostname: str, @@ -303,6 +323,9 @@ def _on_relation_changed(self, event: ops.charm.RelationChangedEvent) -> None: Args: event: Event triggering the relation-changed hook for the relation. + + Raises: + RuntimeError: if _on_relation changed is triggered by a broken relation. """ # `self.unit` isn't available here, so use `self.model.unit`. if not self._charm.model.unit.is_leader(): @@ -376,6 +399,10 @@ def provide_nginx_route( 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. + + Raises: + RuntimeError: If provide_nginx_route was invoked twice with + the same nginx-route relation name """ if __provider_references.get(charm, {}).get(nginx_route_relation_name) is not None: raise RuntimeError( diff --git a/backend/charm/metadata.yaml b/backend/charm/metadata.yaml deleted file mode 100644 index 27183a88..00000000 --- a/backend/charm/metadata.yaml +++ /dev/null @@ -1,28 +0,0 @@ -name: test-observer-api -display-name: | - Test Observer API server -description: | - API and dashboard to observe the status of artifact (snaps, debs, etc) test status -summary: | - API and dashboard to observe the status of artifact (snaps, debs, etc) test status -assumes: - - juju >= 2.9 - - k8s-api -containers: - api: - resource: api-image -requires: - database: - interface: postgresql_client - limit: 1 - nginx-route: - interface: nginx-route -provides: - test-observer-rest-api: - interface: http - scope: global -resources: - api-image: - type: oci-image - description: OCI image from GitHub Container Repository - upstream-source: ghcr.io/canonical/test_observer/api:main diff --git a/backend/charm/requirements.txt b/backend/charm/requirements.txt index 60f27e45..b0681438 100644 --- a/backend/charm/requirements.txt +++ b/backend/charm/requirements.txt @@ -1,2 +1,3 @@ ops >= 2.8.0 requests==2.31.0 +types-requests==2.31.0 diff --git a/backend/charm/src/charm.py b/backend/charm/src/charm.py index e2287890..8f2d8343 100755 --- a/backend/charm/src/charm.py +++ b/backend/charm/src/charm.py @@ -2,16 +2,14 @@ import logging -from charms.data_platform_libs.v0.data_interfaces import ( - DatabaseRequires, - RelationChangedEvent, - RelationJoinedEvent, -) -from charms.nginx_ingress_integrator.v0.nginx_route import require_nginx_route +from data_platform_libs.v0.data_interfaces import DatabaseRequires +from faults import UnitReadinessFault +from nginx_ingress_integrator.v0.nginx_route import require_nginx_route +from ops import RelationChangedEvent, RelationJoinedEvent from ops.charm import CharmBase from ops.main import main from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus -from ops.pebble import Layer, ExecError +from ops.pebble import ExecError, Layer from requests import get # Log messages can be retrieved using juju debug-log @@ -63,38 +61,52 @@ def _setup_nginx(self): def _on_upgrade_charm(self, event): self._migrate_database() - def _migrate_database(self): - # only leader runs database migrations + def _attempt_database_migration(self) -> bool | UnitReadinessFault: + """Return true if migrations were attempted, false if not attempted (if unit is not the leader). + + If unit is not ready for migrations, return UnitReadinessFault. + """ if not self.unit.is_leader(): - raise SystemExit(0) + return False if not self.container.can_connect(): - self.unit.status = WaitingStatus("Waiting for Pebble for API") - raise SystemExit(0) + return UnitReadinessFault("Waiting for Pebble for API") + + pg_data = self._postgres_relation_data() + + if isinstance(pg_data, UnitReadinessFault): + return pg_data self.unit.status = MaintenanceStatus("Migrating database") process = self.container.exec( ["alembic", "upgrade", "head"], working_dir="/home/app", - environment=self._postgres_relation_data(), + environment=pg_data, ) try: stdout, _ = process.wait_output() logger.info(stdout) self.unit.status = ActiveStatus() + + return True + except ExecError as e: logger.error(e.stdout) logger.error(e.stderr) self.unit.status = BlockedStatus("Database migration failed") + return True + def _on_database_changed(self, event): - self._migrate_database() + if not self._attempt_database_migration(): + self.unit.status = WaitingStatus("Waiting for database relation") + self._update_layer_and_restart(None) def _on_database_relation_broken(self, event): - self.unit.status = WaitingStatus("Waiting for database relation") + self.unit.status = WaitingStatus("Waiting for database relation after breaking relation") raise SystemExit(0) def _on_config_changed(self, event): @@ -110,18 +122,28 @@ def _update_layer_and_restart(self, event): self.unit.status = MaintenanceStatus(f"Updating {self.pebble_service_name} layer") if self.container.can_connect(): - self.container.add_layer(self.pebble_service_name, self._pebble_layer, combine=True) + layer = self._pebble_layer + + if isinstance(layer, UnitReadinessFault): + self.unit.status = layer.__str__() + return + + self.container.add_layer(self.pebble_service_name, layer, combine=True) self.container.restart(self.pebble_service_name) + version = self.version if version: self.unit.set_workload_version(self.version) + self.unit.status = ActiveStatus() + else: self.unit.status = WaitingStatus("Waiting for Pebble for API") - def _postgres_relation_data(self) -> dict: + def _postgres_relation_data(self) -> dict | UnitReadinessFault: data = self.database.fetch_relation_data() logger.debug("Got following database relation data: %s", data) + for key, val in data.items(): if not val: continue @@ -130,16 +152,19 @@ def _postgres_relation_data(self) -> dict: db_url = f"postgresql+pg8000://{val['username']}:{val['password']}@{host}:{port}/test_observer_db" return {"DB_URL": db_url} - self.unit.status = WaitingStatus("Waiting for database relation") - raise SystemExit(0) + return UnitReadinessFault("No database relation data") @property - def _app_environment(self): - """ - This creates a dictionary of environment variables needed by the application - """ + def _app_environment(self) -> dict | UnitReadinessFault: + """Environment variables needed by the application.""" env = {"SENTRY_DSN": self.config["sentry_dsn"]} - env.update(self._postgres_relation_data()) + + db_data = self._postgres_relation_data() + + if not isinstance(db_data, dict): + return UnitReadinessFault("App environment not ready", parent=db_data) + + env.update(db_data) return env def _test_observer_rest_api_client_joined(self, event: RelationJoinedEvent) -> None: @@ -168,7 +193,12 @@ def version(self) -> str | None: return None @property - def _pebble_layer(self) -> Layer: + def _pebble_layer(self) -> Layer | UnitReadinessFault: + env = self._app_environment + + if isinstance(env, UnitReadinessFault): + return UnitReadinessFault("Pebble layer not ready", parent=env) + return Layer( { "summary": "test observer", @@ -187,7 +217,16 @@ def _pebble_layer(self) -> Layer: ] ), "startup": "enabled", - "environment": self._app_environment, + "environment": env, + } + }, + "checks": { + "online": { + "exec": { + "command": "curl --fail --silent --head http://0.0.0.0:8000/v1/version", + }, + "timeout": "5s", + "period": "5s", } }, } diff --git a/backend/charm/src/faults.py b/backend/charm/src/faults.py new file mode 100644 index 00000000..785de652 --- /dev/null +++ b/backend/charm/src/faults.py @@ -0,0 +1,17 @@ +class UnitReadinessFault: + """A ReadinessFault describes the reason for a unit not to be ready.""" + + def __init__(self, reason: str, parent: "UnitReadinessFault" | None = None): + self.reason = reason + + def __str__(self): + """Build a representation by recursively traversing the fault hierarchy in reverse order.""" + reversed_hierarchy = self.hierarchy().reverse() + return "Not ready: " + " -> ".join([fault.reason for fault in reversed_hierarchy]) + + def hierarchy(self): + """Build a list of faults by recursively traversing the fault hierarchy.""" + if self.parent is None: + return [self] + else: + return self.parent.hierarchy() + [self] From 567d6b39a8332556849a84348fa2b4f0a1ab34f3 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 17:59:32 +0200 Subject: [PATCH 09/30] Charm now happily works with multiple units --- backend/charm/.gitignore | 1 + backend/charm/src/charm.py | 9 +++++---- backend/charm/src/faults.py | 3 ++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/backend/charm/.gitignore b/backend/charm/.gitignore index dbf2dbb9..9253fe9b 100644 --- a/backend/charm/.gitignore +++ b/backend/charm/.gitignore @@ -2,3 +2,4 @@ venv prime stage +parts diff --git a/backend/charm/src/charm.py b/backend/charm/src/charm.py index 8f2d8343..b5ebd528 100755 --- a/backend/charm/src/charm.py +++ b/backend/charm/src/charm.py @@ -2,9 +2,9 @@ import logging -from data_platform_libs.v0.data_interfaces import DatabaseRequires +from charms.data_platform_libs.v0.data_interfaces import DatabaseRequires +from charms.nginx_ingress_integrator.v0.nginx_route import require_nginx_route from faults import UnitReadinessFault -from nginx_ingress_integrator.v0.nginx_route import require_nginx_route from ops import RelationChangedEvent, RelationJoinedEvent from ops.charm import CharmBase from ops.main import main @@ -59,7 +59,7 @@ def _setup_nginx(self): ) def _on_upgrade_charm(self, event): - self._migrate_database() + self._attempt_database_migration() def _attempt_database_migration(self) -> bool | UnitReadinessFault: """Return true if migrations were attempted, false if not attempted (if unit is not the leader). @@ -226,7 +226,8 @@ def _pebble_layer(self) -> Layer | UnitReadinessFault: "command": "curl --fail --silent --head http://0.0.0.0:8000/v1/version", }, "timeout": "5s", - "period": "5s", + "period": "15s", + "override": "replace", } }, } diff --git a/backend/charm/src/faults.py b/backend/charm/src/faults.py index 785de652..78e5d14d 100644 --- a/backend/charm/src/faults.py +++ b/backend/charm/src/faults.py @@ -1,8 +1,9 @@ class UnitReadinessFault: """A ReadinessFault describes the reason for a unit not to be ready.""" - def __init__(self, reason: str, parent: "UnitReadinessFault" | None = None): + def __init__(self, reason: str, parent=None): self.reason = reason + self.parent = parent def __str__(self): """Build a representation by recursively traversing the fault hierarchy in reverse order.""" From d154e5ce991c28b0a42945be370fbe51ec64ac73 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:06:22 +0200 Subject: [PATCH 10/30] Drops some stray whitespaces --- .github/workflows/terraform_test_deploy.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 6502b8d9..9e724b71 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -54,6 +54,3 @@ jobs: juju wait-for model test-observer-development \ --timeout=10m \ --query='life=="alive" && status=="available" && forEach(applications, app => app.status == "active")' - - - From 22f8fb17257dcffbd7f52f98aa4178ce517f7a76 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:07:07 +0200 Subject: [PATCH 11/30] Removes an accidentally added, redundant .dockerignore --- backend/charm/.dockerignore | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 backend/charm/.dockerignore diff --git a/backend/charm/.dockerignore b/backend/charm/.dockerignore deleted file mode 100644 index a8a6abab..00000000 --- a/backend/charm/.dockerignore +++ /dev/null @@ -1,2 +0,0 @@ -prime -stage From a23cf00ec23cf5115ea1e71650e2869e75ff0054 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:09:16 +0200 Subject: [PATCH 12/30] Drops the template for vscode-settings-default.json --- backend/charm/.vscode-settings-default.json | 12 ------------ backend/charm/.vscode-settings.json | 2 +- 2 files changed, 1 insertion(+), 13 deletions(-) delete mode 100644 backend/charm/.vscode-settings-default.json diff --git a/backend/charm/.vscode-settings-default.json b/backend/charm/.vscode-settings-default.json deleted file mode 100644 index cc1c2c4b..00000000 --- a/backend/charm/.vscode-settings-default.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "python.autoComplete.extraPaths": ["./lib"], - "python.analysis.extraPaths": [ - "./lib" - ], - "yaml.schemas": { - "https://json.schemastore.org/yamllint.json": [ - "./metadata.yaml", - ] - }, - "ansible.python.interpreterPath": "./venv/bin/python3" -} diff --git a/backend/charm/.vscode-settings.json b/backend/charm/.vscode-settings.json index ab10e191..5a243a8b 100644 --- a/backend/charm/.vscode-settings.json +++ b/backend/charm/.vscode-settings.json @@ -11,4 +11,4 @@ ] }, "ansible.python.interpreterPath": "./venv/bin/python3" -} \ No newline at end of file +} From 487370c158b75da64aed1839b593f0a4f9a26e49 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:12:51 +0200 Subject: [PATCH 13/30] Removes the bit in the README about copying in the vscode settings into place. --- .gitignore | 2 -- .vscode/settings.json | 6 ++++++ README.md | 12 ++---------- backend/charm/.vscode/settings.json | 6 ++++++ .../settings.json} | 0 5 files changed, 14 insertions(+), 12 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 backend/charm/.vscode/settings.json rename frontend/charm/{.vscode-settings-default.json => .vscode/settings.json} (100%) diff --git a/.gitignore b/.gitignore index 548b204d..e69de29b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +0,0 @@ -# Visual Studio Code configurations -.vscode diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..574541e5 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,6 @@ +{ + "python.analysis.extraPaths": [ + "./backend/charm/lib" + ], + "cmake.sourceDirectory": "/home/mz2/Developer/test_observer/frontend/linux" +} \ No newline at end of file diff --git a/README.md b/README.md index 8f875b6b..5676bc9a 100644 --- a/README.md +++ b/README.md @@ -158,17 +158,9 @@ Charms are released through GitHub actions on push to main. If however you need ## VS Code & charm libraries -VS Code fails to find (for autocompletions and code navigation purposes) the charm libraries under `lib` in each of `backend/charm` and `frontend/charm`. There is a .vscode-settings-default.json found under each of these directories which you can copy to the `.gitignore`d path `.vscode/settings.json` to make them fly. Taking the backend charm as an example: +VS Code fails to find (for autocompletions and code navigation purposes) the charm libraries under `lib` in each of `backend/charm` and `frontend/charm`. There is therafore a .vscode/settings..json found under each of the directories that will plausibly used as a VS Code workspace. -```bash -mkdir -p backend/charm/.vscode -cp backend/charm/.vscode-settings-default.json backend/charm/.vscode/settings.json - -mkdir -p frontend/charm/.vscode -cp frontend/charm/.vscode-settings-default.json frontend/charm/.vscode/settings.json -``` - -Now if you use as your project the directory `backend/charm` and `frontend/charm` respectively (which you'll want to do also for them to keep their own virtual environments), VS Code should be happy. +If you use as your project the directory `backend/charm` and `frontend/charm` respectively (which you'll want to do also for them to keep their own virtual environments), VS Code should be happy. ## Handy documentation pointers about charming diff --git a/backend/charm/.vscode/settings.json b/backend/charm/.vscode/settings.json new file mode 100644 index 00000000..6b645ee9 --- /dev/null +++ b/backend/charm/.vscode/settings.json @@ -0,0 +1,6 @@ +{ + "python.analysis.extraPaths": [ + "./lib/charms", + "./lib" + ] +} \ No newline at end of file diff --git a/frontend/charm/.vscode-settings-default.json b/frontend/charm/.vscode/settings.json similarity index 100% rename from frontend/charm/.vscode-settings-default.json rename to frontend/charm/.vscode/settings.json From 762d37a3c4a128c094115f5d9b897f05367ffa02 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:27:44 +0200 Subject: [PATCH 14/30] Build the charm locally, upgrade to it --- .github/workflows/terraform_test_deploy.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 9e724b71..f9676e84 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -19,6 +19,7 @@ on: jobs: deploy: + needs: build-api-charm runs-on: [self-hosted, jammy, xlarge] defaults: @@ -43,12 +44,20 @@ jobs: - name: Set up microk8s uses: canonical/certification-github-workflows/.github/actions/microk8s-setup@main + - name: Build charm + run: charmcraft pack + working-directory: ./backend/charm + - name: Terraform apply run: | TF_VAR_environment=development \ TF_VAR_nginx_ingress_integrator_charm_whitelist_source_range="" \ terraform apply -auto-approve + - name: Replace the API charm with the locally built one + run: | + juju refresh api --path ../backend/charm/test-observer-api_ubuntu-22.04-amd64.charm + - name: Wait for deployment to complete run: | juju wait-for model test-observer-development \ From b1392f23ce053f54c1aaa84be3c36ae4ec6b71fb Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:32:27 +0200 Subject: [PATCH 15/30] Adjusted triggers --- .github/workflows/frontend_charm_analysis.yml | 2 +- .github/workflows/terraform_test_deploy.yml | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/frontend_charm_analysis.yml b/.github/workflows/frontend_charm_analysis.yml index d0d6a1c2..71b18e6f 100644 --- a/.github/workflows/frontend_charm_analysis.yml +++ b/.github/workflows/frontend_charm_analysis.yml @@ -37,7 +37,7 @@ jobs: working-directory: ./frontend/charm unit-tests-with-coverage: - name: Unit tests + name: Frontend charm unit tests runs-on: [self-hosted, linux] steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index f9676e84..ed86fa2f 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -1,5 +1,7 @@ name: Terraform deploy to a fresh microk8s model on: + push: + branches: ["*"] # pull_request_review: # on: # pull_request_review: From 600770c4fb3b0f3882c53590980281aea52c9948 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:36:35 +0200 Subject: [PATCH 16/30] Adjusted triggers once again --- .github/workflows/terraform_test_deploy.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index ed86fa2f..8b61f0a4 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -2,6 +2,8 @@ name: Terraform deploy to a fresh microk8s model on: push: branches: ["*"] + tags: + - "v*.*.*" # pull_request_review: # on: # pull_request_review: @@ -14,9 +16,6 @@ on: # - 'frontend/charm/**' # - 'backend/charm/**' # - 'terraform/**' - push: - tags: - - "v*.*.*" workflow_dispatch: jobs: @@ -46,9 +45,13 @@ jobs: - name: Set up microk8s uses: canonical/certification-github-workflows/.github/actions/microk8s-setup@main - - name: Build charm + - name: Build API charm run: charmcraft pack working-directory: ./backend/charm + + - name: Build frontend charm + run: charmcraft pack + working-directory: ./frontend/charm - name: Terraform apply run: | From cddd434e09028527c1fbb4901936047ebc3cb60c Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:38:04 +0200 Subject: [PATCH 17/30] Adjusted triggers once again --- .github/workflows/terraform_test_deploy.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 8b61f0a4..9340f051 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -4,14 +4,17 @@ on: branches: ["*"] tags: - "v*.*.*" + pull_request: + branches: + - '**' + paths: + - 'frontend/charm/**' + - 'backend/charm/**' # pull_request_review: # on: # pull_request_review: # types: # - submitted - pull_request: - branches: - - '**' # paths: # - 'frontend/charm/**' # - 'backend/charm/**' From 79d33e68669fed842e391635a2184771cb46fe0e Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 18:43:20 +0200 Subject: [PATCH 18/30] Remove the dependency for now --- .github/workflows/terraform_test_deploy.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 9340f051..8781db23 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -23,7 +23,6 @@ on: jobs: deploy: - needs: build-api-charm runs-on: [self-hosted, jammy, xlarge] defaults: From 96ab78b5b8985a0a016b9da517157b35f1610beb Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 19:56:35 +0200 Subject: [PATCH 19/30] Adds a type hint back to faults.py --- .github/workflows/terraform_test_deploy.yml | 4 ++++ backend/charm/src/faults.py | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 8781db23..aca08032 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -47,10 +47,14 @@ jobs: - name: Set up microk8s uses: canonical/certification-github-workflows/.github/actions/microk8s-setup@main + # TODO: Separate this build step to its own action once custom VM images are available + # (when there is a charmcraft + LXD capable image available) - name: Build API charm run: charmcraft pack working-directory: ./backend/charm + # TODO: Separate this build step to its own action once custom VM images are available + # (when there is a charmcraft + LXD capable image available) - name: Build frontend charm run: charmcraft pack working-directory: ./frontend/charm diff --git a/backend/charm/src/faults.py b/backend/charm/src/faults.py index 78e5d14d..86554a68 100644 --- a/backend/charm/src/faults.py +++ b/backend/charm/src/faults.py @@ -1,7 +1,10 @@ +from typing import Optional + + class UnitReadinessFault: """A ReadinessFault describes the reason for a unit not to be ready.""" - def __init__(self, reason: str, parent=None): + def __init__(self, reason: str, parent: "Optional[UnitReadinessFault]" = None): self.reason = reason self.parent = parent From e42ef9d9cd0e7590c93735e1623f9d3c4365667e Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 20:01:21 +0200 Subject: [PATCH 20/30] Deploy also the frontend charm --- .github/workflows/terraform_test_deploy.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index aca08032..43d86d87 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -67,7 +67,11 @@ jobs: - name: Replace the API charm with the locally built one run: | - juju refresh api --path ../backend/charm/test-observer-api_ubuntu-22.04-amd64.charm + juju refresh api --path ../backend/charm/*.charm + + - name: Replace the frontend charm with the locally built one + run: | + juju refresh api --path ../frontend/charm/*.charm - name: Wait for deployment to complete run: | From 3f02329e0fd92084e097ab35924a0e781f7928e8 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 20:03:50 +0200 Subject: [PATCH 21/30] Switch the model --- .github/workflows/terraform_test_deploy.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 43d86d87..68ca60f2 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -64,6 +64,10 @@ jobs: TF_VAR_environment=development \ TF_VAR_nginx_ingress_integrator_charm_whitelist_source_range="" \ terraform apply -auto-approve + + - name: Switch model to test-observer-development + run: | + juju switch test-observer-development - name: Replace the API charm with the locally built one run: | From 5729678407228248df7855676793e76a19ebf014 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 20:08:52 +0200 Subject: [PATCH 22/30] Adjust the triggers --- .github/workflows/terraform_test_deploy.yml | 27 +++++++++++---------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 68ca60f2..dcfd0ead 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -1,28 +1,29 @@ name: Terraform deploy to a fresh microk8s model on: - push: - branches: ["*"] - tags: - - "v*.*.*" pull_request: branches: - '**' paths: - 'frontend/charm/**' - 'backend/charm/**' -# pull_request_review: -# on: -# pull_request_review: -# types: -# - submitted -# paths: -# - 'frontend/charm/**' -# - 'backend/charm/**' -# - 'terraform/**' + pull_request_review: + on: + pull_request_review: + types: + - submitted + paths: + - 'frontend/charm/**' + - 'backend/charm/**' + - 'terraform/**' + push: + branches: ["main"] + tags: ["v*.*.*"] workflow_dispatch: jobs: deploy: + name: terraform deploy + upgrade charms + if: github.event.review.state == 'APPROVED' runs-on: [self-hosted, jammy, xlarge] defaults: From c789d9c5c0c7ffb0da2cb09918ee84f61ed42a2e Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 23:35:32 +0200 Subject: [PATCH 23/30] Oops, refresh the frontend app with the frontend charm --- .github/workflows/terraform_test_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index dcfd0ead..2f665ab7 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -76,7 +76,7 @@ jobs: - name: Replace the frontend charm with the locally built one run: | - juju refresh api --path ../frontend/charm/*.charm + juju refresh frontend --path ../frontend/charm/*.charm - name: Wait for deployment to complete run: | From 075bff2ed4f6631f5ce01081962b49511ffbc4d0 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 23:45:21 +0200 Subject: [PATCH 24/30] Remove the if condition for now --- .github/workflows/terraform_test_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 2f665ab7..1f61c284 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -23,7 +23,7 @@ on: jobs: deploy: name: terraform deploy + upgrade charms - if: github.event.review.state == 'APPROVED' + # if: github.event.review.state == 'APPROVED' runs-on: [self-hosted, jammy, xlarge] defaults: From bbc885cfdf13548469c3ffce1218840f3ade894b Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Tue, 12 Dec 2023 23:45:50 +0200 Subject: [PATCH 25/30] Changes to terraform plan should also lead to a deploy --- .github/workflows/terraform_test_deploy.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 1f61c284..9298b8bd 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -6,6 +6,7 @@ on: paths: - 'frontend/charm/**' - 'backend/charm/**' + - 'terraform/**' pull_request_review: on: pull_request_review: From 80303b83f3b2fec2c3bbcc3cb7964015b0db2c99 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Wed, 13 Dec 2023 00:07:09 +0200 Subject: [PATCH 26/30] Wait for application to have deployed --- .github/workflows/terraform_test_deploy.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 9298b8bd..ac94a12c 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -73,10 +73,12 @@ jobs: - name: Replace the API charm with the locally built one run: | + juju wait-for application api --timeout=10m --query='life=="alive"' juju refresh api --path ../backend/charm/*.charm - name: Replace the frontend charm with the locally built one run: | + juju wait-for application api --timeout=10m --query='life=="alive"' juju refresh frontend --path ../frontend/charm/*.charm - name: Wait for deployment to complete From aaf38261868ac97e31b41ae536f3bca2da9d7c30 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Wed, 13 Dec 2023 00:08:48 +0200 Subject: [PATCH 27/30] Wait for _frontend_ before refreshing the frontend charm --- .github/workflows/terraform_test_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index ac94a12c..06d4a7ea 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -78,7 +78,7 @@ jobs: - name: Replace the frontend charm with the locally built one run: | - juju wait-for application api --timeout=10m --query='life=="alive"' + juju wait-for application frontend --timeout=10m --query='life=="alive"' juju refresh frontend --path ../frontend/charm/*.charm - name: Wait for deployment to complete From 99f6d8985f82ea120bdf74e66254739b926ea14a Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Wed, 13 Dec 2023 00:10:37 +0200 Subject: [PATCH 28/30] Add crashdump uploading --- .github/workflows/terraform_test_deploy.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 06d4a7ea..88443bcd 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -86,3 +86,10 @@ jobs: juju wait-for model test-observer-development \ --timeout=10m \ --query='life=="alive" && status=="available" && forEach(applications, app => app.status == "active")' + + - name: Archive juju crashdump on failure + if: failure() + uses: actions/upload-artifact@v3 + with: + name: juju-crashdump + path: ./**/juju-crashdump-*.tar.xz \ No newline at end of file From a81ba86ff6748876d283faa649ba15e1e9ae0b77 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Wed, 13 Dec 2023 01:13:23 +0200 Subject: [PATCH 29/30] Output juju status and juju crashdump --- .github/workflows/terraform_test_deploy.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/terraform_test_deploy.yml b/.github/workflows/terraform_test_deploy.yml index 88443bcd..4879efdb 100644 --- a/.github/workflows/terraform_test_deploy.yml +++ b/.github/workflows/terraform_test_deploy.yml @@ -70,6 +70,7 @@ jobs: - name: Switch model to test-observer-development run: | juju switch test-observer-development + sleep 10 # hack hack, the below wait-for condition is not actually correct, so let's wait a bit - name: Replace the API charm with the locally built one run: | @@ -87,6 +88,17 @@ jobs: --timeout=10m \ --query='life=="alive" && status=="available" && forEach(applications, app => app.status == "active")' + - name: Echo juju status on failure + if: failure() + run: | + juju status --color + + - name: Run juju crashdump on failure + if: failure() + run: | + sudo juju crashdump -o ./ -m test-observer-development --as-root -a juju-show-unit -a juju-show-status-log -a juju-show-machine -a config + sudo chown $USER ./**/juju-crashdump-*.tar.xz + - name: Archive juju crashdump on failure if: failure() uses: actions/upload-artifact@v3 From fa6a77ff016dc330f1d32b8b8360caaf6e419b33 Mon Sep 17 00:00:00 2001 From: Matias Piipari Date: Thu, 14 Dec 2023 18:50:31 +0200 Subject: [PATCH 30/30] Remove redundant SystemExit(0) --- backend/charm/src/charm.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/charm/src/charm.py b/backend/charm/src/charm.py index b5ebd528..32e99e81 100755 --- a/backend/charm/src/charm.py +++ b/backend/charm/src/charm.py @@ -107,7 +107,6 @@ def _on_database_changed(self, event): def _on_database_relation_broken(self, event): self.unit.status = WaitingStatus("Waiting for database relation after breaking relation") - raise SystemExit(0) def _on_config_changed(self, event): if self.unit.is_leader():