From 484f9307bc4a9bee74bec070fed87f30b20628b6 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Fri, 27 Sep 2024 11:35:59 +0000 Subject: [PATCH 01/14] store the data of peer-cluster relation in a secret --- .../opensearch/v0/opensearch_peer_clusters.py | 5 +- .../v0/opensearch_relation_peer_cluster.py | 175 +++++++++++++++++- .../opensearch/v0/opensearch_secrets.py | 7 +- 3 files changed, 174 insertions(+), 13 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_peer_clusters.py b/lib/charms/opensearch/v0/opensearch_peer_clusters.py index 76969701f..691a95e1c 100644 --- a/lib/charms/opensearch/v0/opensearch_peer_clusters.py +++ b/lib/charms/opensearch/v0/opensearch_peer_clusters.py @@ -39,6 +39,7 @@ ) from charms.opensearch.v0.opensearch_exceptions import OpenSearchError from charms.opensearch.v0.opensearch_internal_data import Scope +from charms.opensearch.v0.opensearch_relation_peer_cluster import rel_data_from_secret from ops import BlockedStatus from shortuuid import ShortUUID @@ -522,10 +523,10 @@ def rel_data(self) -> Optional[PeerClusterRelData]: rel = self._charm.model.get_relation( PeerClusterOrchestratorRelationName, orchestrators.main_rel_id ) - if not (data := rel.data[rel.app].get("data")): + if not (secret_id := rel.data[rel.app].get("data")): return None - return PeerClusterRelData.from_str(data) + return rel_data_from_secret(self._charm, secret_id) def _pre_validate_roles_change(self, new_roles: List[str], prev_roles: List[str]): """Validate that the config changes of roles are allowed to happen.""" diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index 39193d694..f32e4acb3 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -45,6 +45,7 @@ RelationDepartedEvent, RelationEvent, RelationJoinedEvent, + Secret, WaitingStatus, ) from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed @@ -260,6 +261,19 @@ def refresh_relation_data( # compute the data that needs to be broadcast to all related clusters (success or error) rel_data = self._rel_data(deployment_desc, orchestrators) + # if rel_data is an error, prepare to broadcast it to all related clusters + if isinstance(rel_data, PeerClusterRelData): + rel_data_secret_content = self._rel_data_secret_content(rel_data) + + # check if a secret already exists for this orchestrator's relations + # and update it if so + rel_data_secret = self._update_or_create_rel_data_secret( + rel_data_secret_content, all_relation_ids + ) + + # grant the secrets inside the rel_data to all the related clusters + self._grant_rel_data_secrets(rel_data_secret_content, all_relation_ids) + # exit if current cluster should not have been considered a provider if self._notify_if_wrong_integration(rel_data, all_relation_ids) and event_rel_id: self.delete_from_rel("trigger", rel_id=event_rel_id) @@ -281,9 +295,9 @@ def refresh_relation_data( orchestrators[f"{cluster_type}_app"] = deployment_desc.app.to_dict() self.charm.peers_data.put_object(Scope.APP, "orchestrators", orchestrators) - peer_rel_data_key, should_defer = "data", False + should_defer = False if isinstance(rel_data, PeerClusterRelErrorData): - peer_rel_data_key, should_defer = "error_data", rel_data.should_wait + should_defer = rel_data.should_wait # save the orchestrators of this fleet for rel_id in all_relation_ids: @@ -299,9 +313,11 @@ def refresh_relation_data( # there is no error to broadcast - we clear any previously broadcasted error if isinstance(rel_data, PeerClusterRelData): self.delete_from_rel("error_data", rel_id=rel_id) + self.put_in_rel(data={"data": rel_data_secret.id}, rel_id=rel_id) + rel_data_secret.grant(self.get_rel(rel_id=rel_id)) - # are we potentially overriding stuff here? - self.put_in_rel(data={peer_rel_data_key: rel_data.to_str()}, rel_id=rel_id) + else: + self.put_in_rel(data={"error_data": rel_data.to_str()}, rel_id=rel_id) if can_defer and should_defer: event.defer() @@ -519,6 +535,73 @@ def _fetch_local_cm_nodes(self, deployment_desc: DeploymentDescription) -> List[ if node.is_cm_eligible() and node.app.id == deployment_desc.app.id ] + def _rel_data_secret_content(self, rel_data: PeerClusterRelData) -> dict[str, str]: + """Convert the secret data to a dict for storage in a secret.""" + # If we use the values of the secrets then the size of the secret + # would be too large for juju we store the secret ids instead and + # fetch the secrets when needed in the requirer side + secrets = self.charm.secrets + + credentials = { + "admin-username": AdminUser, + "admin-password": secrets.get_secret_id(Scope.APP, secrets.password_key(AdminUser)), + "admin-password-hash": secrets.get_secret_id(Scope.APP, secrets.hash_key(AdminUser)), + "kibana-password": secrets.get_secret_id( + Scope.APP, secrets.password_key(KibanaserverUser) + ), + "kibana-password-hash": secrets.get_secret_id( + Scope.APP, secrets.hash_key(KibanaserverUser) + ), + } + + if monitor_password := secrets.get_secret_id(Scope.APP, secrets.password_key(COSUser)): + credentials["monitor-password"] = monitor_password + if admin_tls := secrets.get_secret_id(Scope.APP, CertType.APP_ADMIN.val): + credentials["admin-tls"] = admin_tls + + if s3_creds := rel_data.credentials.s3: + credentials["s3"] = { + "access-key": s3_creds.access_key, + "secret-key": s3_creds.secret_key, + } + + return { + "cluster-name": rel_data.cluster_name, + "cm-nodes": json.dumps([node.to_dict() for node in rel_data.cm_nodes]), + "credentials": json.dumps(credentials), + "deployment-desc": json.dumps(rel_data.deployment_desc.to_dict()), + } + + def _update_or_create_rel_data_secret( + self, rel_data_secret_content: dict[str, str], all_rel_ids: list[int] + ) -> Secret: + """Update or create the secret for the peer cluster relation data.""" + for rel_id in all_rel_ids: + rel_data_secret_id = self.get_from_rel("data", rel_id=rel_id) + if rel_data_secret_id: + rel_data_secret = self.model.get_secret(id=rel_data_secret_id) + if rel_data_secret_content != rel_data_secret.get_content(): + rel_data_secret.set_content(rel_data_secret_content) + + return rel_data_secret + + return self.model.app.add_secret(rel_data_secret_content) + + def _grant_rel_data_secrets( + self, rel_data_secret_content: dict[str, str], all_rel_ids: list[int] + ): + """Grant the secrets to all the related apps.""" + credentials = json.loads(rel_data_secret_content["credentials"]) + for key, secret_id in credentials.items(): + # s3 and admin-username are not secrets + if key == "s3" or key == "admin-username": + continue + + secret = self.model.get_secret(id=secret_id) + for rel_id in all_rel_ids: + if relation := self.get_rel(rel_id=rel_id): + secret.grant(relation) + class OpenSearchPeerClusterRequirer(OpenSearchPeerClusterRelation): """Peer cluster relation requirer class.""" @@ -573,8 +656,7 @@ def _on_peer_cluster_relation_changed(self, event: RelationChangedEvent): # noq return # fetch the success data - data = PeerClusterRelData.from_str(data["data"]) - + data = rel_data_from_secret(self.charm, data["data"]) # check errors that can only be figured out from the requirer side if self._error_set_from_requirer(orchestrators, deployment_desc, data, event.relation.id): return @@ -806,11 +888,11 @@ def _cm_nodes(self, orchestrators: PeerClusterOrchestrators) -> List[Node]: if rel_id == -1: continue - data = self.get_obj_from_rel(key="data", rel_id=rel_id) - if not data: # not ready yet + secret_id = self.get_from_rel(key="data", rel_id=rel_id, remote_app=True) + if not secret_id: # not ready yet continue - data = PeerClusterRelData.from_dict(data) + data = rel_data_from_secret(self.charm, secret_id) cm_nodes = {**cm_nodes, **{node.name: node for node in data.cm_nodes}} # attempt to have an opensearch reported list of CMs - the response @@ -847,7 +929,7 @@ def _error_set_from_providers( error = None for rel_id in orchestrator_rel_ids: - data = self.get_obj_from_rel("data", rel_id=rel_id) + data = self.get_from_rel("data", rel_id=rel_id, remote_app=True) error_data = self.get_obj_from_rel("error_data", rel_id=rel_id) if not data and not error_data: # relation data still incomplete return True @@ -960,3 +1042,76 @@ def _clear_errors(self, *error_labels: str): error = self.charm.peers_data.get(Scope.APP, error_label, "") self.charm.status.clear(error, app=True) self.charm.peers_data.delete(Scope.APP, error_label) + + +def rel_data_from_secret( + charm: "OpenSearchBaseCharm", secret_id: str +) -> PeerClusterRelData | None: + """Construct the peer cluster rel data from the secret data.""" + secret = charm.model.get_secret(id=secret_id) + if not secret: + return None + + content = secret.get_content() + credentials = json.loads(content["credentials"]) + + secrets = charm.secrets + + admin_password = ( + charm.model.get_secret(id=credentials["admin-password"]) + .get_content() + .get(secrets.password_key(AdminUser)) + ) + + admin_password_hash = ( + charm.model.get_secret(id=credentials["admin-password-hash"]) + .get_content() + .get(secrets.hash_key(AdminUser)) + ) + + kibana_password = ( + charm.model.get_secret(id=credentials["kibana-password"]) + .get_content() + .get(secrets.password_key(KibanaserverUser)) + ) + + kibana_password_hash = ( + charm.model.get_secret(id=credentials["kibana-password-hash"]) + .get_content() + .get(secrets.hash_key(KibanaserverUser)) + ) + + monitor_password = None + if "monitor-password" in credentials: + monitor_password = ( + charm.model.get_secret(id=credentials["monitor-password"]) + .get_content() + .get(secrets.password_key(COSUser)) + ) + + admin_tls = None + if "admin-tls" in credentials: + admin_tls = charm.model.get_secret(id=credentials["admin-tls"]).get_content() + + s3 = None + if "s3" in credentials: + s3 = S3RelDataCredentials( + access_key=credentials["s3"]["access-key"], + secret_key=credentials["s3"]["secret-key"], + ) + + return PeerClusterRelData( + cluster_name=content["cluster-name"], + cm_nodes=[Node.from_dict(node) for node in json.loads(content["cm-nodes"])], + credentials=PeerClusterRelDataCredentials( + admin_username=AdminUser, + admin_password=admin_password, + admin_password_hash=admin_password_hash, + kibana_password=kibana_password, + kibana_password_hash=kibana_password_hash, + monitor_password=monitor_password, + admin_tls=admin_tls, + s3=s3, + ), + deployment_desc=DeploymentDescription.from_dict(json.loads(content["deployment-desc"])), + ) diff --git a/lib/charms/opensearch/v0/opensearch_secrets.py b/lib/charms/opensearch/v0/opensearch_secrets.py index c36e7b9f6..680a24cad 100644 --- a/lib/charms/opensearch/v0/opensearch_secrets.py +++ b/lib/charms/opensearch/v0/opensearch_secrets.py @@ -40,7 +40,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 if TYPE_CHECKING: @@ -368,3 +368,8 @@ def delete(self, scope: Scope, key: str) -> None: self._remove_juju_secret(scope, key) logging.debug(f"Deleted secret {scope}:{key}") + + def get_secret_id(self, scope: Scope, key: str) -> Optional[str]: + """Get the secret ID from the cache.""" + label = self.label(scope, key) + return self._charm.peers_data.get(scope, label) From 52d9410f4331636c42d71b03b26268972c51c76e Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Mon, 30 Sep 2024 07:21:48 +0000 Subject: [PATCH 02/14] incremented libpatch and added unit test for get_secret_id --- .../v0/opensearch_relation_peer_cluster.py | 2 +- tests/unit/lib/test_opensearch_secrets.py | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index f32e4acb3..b6426e214 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -65,7 +65,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 class OpenSearchPeerClusterRelation(Object): diff --git a/tests/unit/lib/test_opensearch_secrets.py b/tests/unit/lib/test_opensearch_secrets.py index d84b559df..402686f0e 100644 --- a/tests/unit/lib/test_opensearch_secrets.py +++ b/tests/unit/lib/test_opensearch_secrets.py @@ -185,3 +185,15 @@ def test_bad_label(self): @parameterized.expand([Scope.APP, Scope.UNIT]) def test_put_and_get_complex_obj(self, scope): return + + def test_get_secret_id(self): + # add a secret to the store + content = {"secret": "value"} + self.store.put(Scope.APP, "super-secret-key", content) + # get the secret id + secret_id = self.store.get_secret_id(Scope.APP, "super-secret-key") + self.assertIsNotNone(secret_id) + # check the secret content + secret = self.charm.model.get_secret(id=secret_id) + secret_content = secret.get_content() + self.assertDictEqual(secret_content, {"super-secret-key": str(content)}) From 64612e58a861097336110f74f76d6f0d0b1a6259 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Wed, 2 Oct 2024 08:44:32 +0000 Subject: [PATCH 03/14] added hash to databag --- .../v0/opensearch_relation_peer_cluster.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index b6426e214..d90ec4863 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -4,6 +4,7 @@ """Peer clusters relation related classes for OpenSearch.""" import json import logging +from hashlib import sha1 from typing import TYPE_CHECKING, Any, Dict, List, MutableMapping, Optional, Union from charms.opensearch.v0.constants_charm import ( @@ -313,7 +314,17 @@ def refresh_relation_data( # there is no error to broadcast - we clear any previously broadcasted error if isinstance(rel_data, PeerClusterRelData): self.delete_from_rel("error_data", rel_id=rel_id) - self.put_in_rel(data={"data": rel_data_secret.id}, rel_id=rel_id) + # we add the hash of the rel_data to only emit a change event + # if the data has actually changed + self.put_in_rel( + data={ + "data": rel_data_secret.id, + "rel_data_hash": sha1( + json.dumps(rel_data.to_dict(), sort_keys=True).encode() + ).hexdigest(), + }, + rel_id=rel_id, + ) rel_data_secret.grant(self.get_rel(rel_id=rel_id)) else: From 267dd2acd5b18a435757562c4bf1c98c309cfe4e Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Thu, 17 Oct 2024 11:24:40 +0000 Subject: [PATCH 04/14] store clear data in databag except for secrets --- .../opensearch/v0/opensearch_peer_clusters.py | 8 +- .../v0/opensearch_relation_peer_cluster.py | 87 ++++++------------- 2 files changed, 33 insertions(+), 62 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_peer_clusters.py b/lib/charms/opensearch/v0/opensearch_peer_clusters.py index 691a95e1c..a44bd10e7 100644 --- a/lib/charms/opensearch/v0/opensearch_peer_clusters.py +++ b/lib/charms/opensearch/v0/opensearch_peer_clusters.py @@ -39,7 +39,9 @@ ) from charms.opensearch.v0.opensearch_exceptions import OpenSearchError from charms.opensearch.v0.opensearch_internal_data import Scope -from charms.opensearch.v0.opensearch_relation_peer_cluster import rel_data_from_secret +from charms.opensearch.v0.opensearch_relation_peer_cluster import ( + rel_data_from_redacted_str, +) from ops import BlockedStatus from shortuuid import ShortUUID @@ -523,10 +525,10 @@ def rel_data(self) -> Optional[PeerClusterRelData]: rel = self._charm.model.get_relation( PeerClusterOrchestratorRelationName, orchestrators.main_rel_id ) - if not (secret_id := rel.data[rel.app].get("data")): + if not (redacted_dict_str := rel.data[rel.app].get("data")): return None - return rel_data_from_secret(self._charm, secret_id) + return rel_data_from_redacted_str(self._charm, redacted_dict_str) def _pre_validate_roles_change(self, new_roles: List[str], prev_roles: List[str]): """Validate that the config changes of roles are allowed to happen.""" diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index d90ec4863..d3add502d 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -46,7 +46,6 @@ RelationDepartedEvent, RelationEvent, RelationJoinedEvent, - Secret, WaitingStatus, ) from tenacity import RetryError, Retrying, stop_after_attempt, wait_fixed @@ -264,16 +263,10 @@ def refresh_relation_data( # if rel_data is an error, prepare to broadcast it to all related clusters if isinstance(rel_data, PeerClusterRelData): - rel_data_secret_content = self._rel_data_secret_content(rel_data) - - # check if a secret already exists for this orchestrator's relations - # and update it if so - rel_data_secret = self._update_or_create_rel_data_secret( - rel_data_secret_content, all_relation_ids - ) + rel_data_redacted_dict = self._rel_data_redacted_dict(rel_data) # grant the secrets inside the rel_data to all the related clusters - self._grant_rel_data_secrets(rel_data_secret_content, all_relation_ids) + self._grant_rel_data_secrets(rel_data_redacted_dict, all_relation_ids) # exit if current cluster should not have been considered a provider if self._notify_if_wrong_integration(rel_data, all_relation_ids) and event_rel_id: @@ -318,15 +311,13 @@ def refresh_relation_data( # if the data has actually changed self.put_in_rel( data={ - "data": rel_data_secret.id, + "data": json.dumps(rel_data_redacted_dict), "rel_data_hash": sha1( json.dumps(rel_data.to_dict(), sort_keys=True).encode() ).hexdigest(), }, rel_id=rel_id, ) - rel_data_secret.grant(self.get_rel(rel_id=rel_id)) - else: self.put_in_rel(data={"error_data": rel_data.to_str()}, rel_id=rel_id) @@ -546,14 +537,16 @@ def _fetch_local_cm_nodes(self, deployment_desc: DeploymentDescription) -> List[ if node.is_cm_eligible() and node.app.id == deployment_desc.app.id ] - def _rel_data_secret_content(self, rel_data: PeerClusterRelData) -> dict[str, str]: + def _rel_data_redacted_dict(self, rel_data: PeerClusterRelData) -> dict[str, Any]: """Convert the secret data to a dict for storage in a secret.""" # If we use the values of the secrets then the size of the secret # would be too large for juju we store the secret ids instead and # fetch the secrets when needed in the requirer side secrets = self.charm.secrets - credentials = { + redacted_dict = rel_data.to_dict() + + redacted_dict["credentials"] = { "admin-username": AdminUser, "admin-password": secrets.get_secret_id(Scope.APP, secrets.password_key(AdminUser)), "admin-password-hash": secrets.get_secret_id(Scope.APP, secrets.hash_key(AdminUser)), @@ -566,46 +559,26 @@ def _rel_data_secret_content(self, rel_data: PeerClusterRelData) -> dict[str, st } if monitor_password := secrets.get_secret_id(Scope.APP, secrets.password_key(COSUser)): - credentials["monitor-password"] = monitor_password + redacted_dict["credentials"]["monitor-password"] = monitor_password if admin_tls := secrets.get_secret_id(Scope.APP, CertType.APP_ADMIN.val): - credentials["admin-tls"] = admin_tls + redacted_dict["credentials"]["admin-tls"] = admin_tls - if s3_creds := rel_data.credentials.s3: - credentials["s3"] = { - "access-key": s3_creds.access_key, - "secret-key": s3_creds.secret_key, + if rel_data.credentials.s3: + redacted_dict["credentials"]["s3"] = { + "access-key": secrets.get_secret_id(Scope.APP, "access-key"), + "secret-key": secrets.get_secret_id(Scope.APP, "access-key"), } - return { - "cluster-name": rel_data.cluster_name, - "cm-nodes": json.dumps([node.to_dict() for node in rel_data.cm_nodes]), - "credentials": json.dumps(credentials), - "deployment-desc": json.dumps(rel_data.deployment_desc.to_dict()), - } - - def _update_or_create_rel_data_secret( - self, rel_data_secret_content: dict[str, str], all_rel_ids: list[int] - ) -> Secret: - """Update or create the secret for the peer cluster relation data.""" - for rel_id in all_rel_ids: - rel_data_secret_id = self.get_from_rel("data", rel_id=rel_id) - if rel_data_secret_id: - rel_data_secret = self.model.get_secret(id=rel_data_secret_id) - if rel_data_secret_content != rel_data_secret.get_content(): - rel_data_secret.set_content(rel_data_secret_content) - - return rel_data_secret - - return self.model.app.add_secret(rel_data_secret_content) + return redacted_dict def _grant_rel_data_secrets( - self, rel_data_secret_content: dict[str, str], all_rel_ids: list[int] + self, rel_data_secret_content: dict[str, Any], all_rel_ids: list[int] ): """Grant the secrets to all the related apps.""" - credentials = json.loads(rel_data_secret_content["credentials"]) + credentials = rel_data_secret_content["credentials"] for key, secret_id in credentials.items(): # s3 and admin-username are not secrets - if key == "s3" or key == "admin-username": + if key == "admin-username": continue secret = self.model.get_secret(id=secret_id) @@ -667,7 +640,7 @@ def _on_peer_cluster_relation_changed(self, event: RelationChangedEvent): # noq return # fetch the success data - data = rel_data_from_secret(self.charm, data["data"]) + data = rel_data_from_redacted_str(self.charm, data["data"]) # check errors that can only be figured out from the requirer side if self._error_set_from_requirer(orchestrators, deployment_desc, data, event.relation.id): return @@ -899,11 +872,11 @@ def _cm_nodes(self, orchestrators: PeerClusterOrchestrators) -> List[Node]: if rel_id == -1: continue - secret_id = self.get_from_rel(key="data", rel_id=rel_id, remote_app=True) - if not secret_id: # not ready yet + redacted_dict_str = self.get_from_rel(key="data", rel_id=rel_id, remote_app=True) + if not redacted_dict_str: # not ready yet continue - data = rel_data_from_secret(self.charm, secret_id) + data = rel_data_from_redacted_str(self.charm, redacted_dict_str) cm_nodes = {**cm_nodes, **{node.name: node for node in data.cm_nodes}} # attempt to have an opensearch reported list of CMs - the response @@ -1055,16 +1028,12 @@ def _clear_errors(self, *error_labels: str): self.charm.peers_data.delete(Scope.APP, error_label) -def rel_data_from_secret( - charm: "OpenSearchBaseCharm", secret_id: str +def rel_data_from_redacted_str( + charm: "OpenSearchBaseCharm", redacted_dict_str: str ) -> PeerClusterRelData | None: """Construct the peer cluster rel data from the secret data.""" - secret = charm.model.get_secret(id=secret_id) - if not secret: - return None - - content = secret.get_content() - credentials = json.loads(content["credentials"]) + content = json.loads(redacted_dict_str) + credentials = content["credentials"] secrets = charm.secrets @@ -1112,8 +1081,8 @@ def rel_data_from_secret( ) return PeerClusterRelData( - cluster_name=content["cluster-name"], - cm_nodes=[Node.from_dict(node) for node in json.loads(content["cm-nodes"])], + cluster_name=content["cluster_name"], + cm_nodes=[Node.from_dict(node) for node in content["cm_nodes"]], credentials=PeerClusterRelDataCredentials( admin_username=AdminUser, admin_password=admin_password, @@ -1124,5 +1093,5 @@ def rel_data_from_secret( admin_tls=admin_tls, s3=s3, ), - deployment_desc=DeploymentDescription.from_dict(json.loads(content["deployment-desc"])), + deployment_desc=DeploymentDescription.from_dict(content["deployment_desc"]), ) From 14dcd4a456ee8238a046566e2e0c85dfd6938fed Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Fri, 18 Oct 2024 13:24:11 +0000 Subject: [PATCH 05/14] implemented rene feedback --- .../opensearch/v0/opensearch_peer_clusters.py | 59 ++++++- .../v0/opensearch_relation_peer_cluster.py | 162 ++++++------------ .../opensearch/v0/opensearch_secrets.py | 2 +- 3 files changed, 113 insertions(+), 110 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_peer_clusters.py b/lib/charms/opensearch/v0/opensearch_peer_clusters.py index a44bd10e7..55c6f258a 100644 --- a/lib/charms/opensearch/v0/opensearch_peer_clusters.py +++ b/lib/charms/opensearch/v0/opensearch_peer_clusters.py @@ -2,14 +2,18 @@ # See LICENSE file for licensing details. """Class for Managing simple or large deployments and configuration related changes.""" +import json import logging from datetime import datetime from typing import TYPE_CHECKING, List, Literal, Optional from charms.opensearch.v0.constants_charm import ( + AdminUser, CMRoleRemovalForbidden, CmVoRolesProvidedInvalid, + COSUser, DataRoleRemovalForbidden, + KibanaserverUser, PClusterNoRelation, PClusterWrongNodesCountForQuorum, PClusterWrongRelation, @@ -34,14 +38,12 @@ PeerClusterConfig, PeerClusterOrchestrators, PeerClusterRelData, + S3RelDataCredentials, StartMode, State, ) from charms.opensearch.v0.opensearch_exceptions import OpenSearchError from charms.opensearch.v0.opensearch_internal_data import Scope -from charms.opensearch.v0.opensearch_relation_peer_cluster import ( - rel_data_from_redacted_str, -) from ops import BlockedStatus from shortuuid import ShortUUID @@ -528,7 +530,7 @@ def rel_data(self) -> Optional[PeerClusterRelData]: if not (redacted_dict_str := rel.data[rel.app].get("data")): return None - return rel_data_from_redacted_str(self._charm, redacted_dict_str) + return self.rel_data_from_redacted_str(redacted_dict_str) def _pre_validate_roles_change(self, new_roles: List[str], prev_roles: List[str]): """Validate that the config changes of roles are allowed to happen.""" @@ -590,3 +592,52 @@ def _deployment_type(config: PeerClusterConfig, start_mode: StartMode) -> Deploy if not config.init_hold else DeploymentType.FAILOVER_ORCHESTRATOR ) + + def rel_data_from_redacted_str(self, redacted_dict_str: str) -> PeerClusterRelData | None: + """Construct the peer cluster rel data from the secret data.""" + content = json.loads(redacted_dict_str) + credentials = content["credentials"] + + credentials["admin_password"] = ( + self._charm.model.get_secret(id=credentials["admin_password"]) + .get_content() + .get(self._charm.secrets.password_key(AdminUser)) + ) + + credentials["admin_password_hash"] = ( + self._charm.model.get_secret(id=credentials["admin_password_hash"]) + .get_content() + .get(self._charm.secrets.hash_key(AdminUser)) + ) + + credentials["kibana_password"] = ( + self._charm.model.get_secret(id=credentials["kibana_password"]) + .get_content() + .get(self._charm.secrets.password_key(KibanaserverUser)) + ) + + credentials["kibana_password_hash"] = ( + self._charm.model.get_secret(id=credentials["kibana_password_hash"]) + .get_content() + .get(self._charm.secrets.hash_key(KibanaserverUser)) + ) + + if "monitor_password" in credentials: + credentials["monitor_password"] = ( + self._charm.model.get_secret(id=credentials["monitor_password"]) + .get_content() + .get(self._charm.secrets.password_key(COSUser)) + ) + + if "admin_tls" in credentials: + credentials["admin_tls"] = self._charm.model.get_secret( + id=credentials["admin_tls"] + ).get_content() + + if "s3" in credentials: + credentials["s3"] = S3RelDataCredentials( + access_key=credentials["s3"]["access_key"], + secret_key=credentials["s3"]["secret_key"], + ) + + return PeerClusterRelData.from_dict(content) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index d3add502d..b435f4ffd 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -65,7 +65,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 = 1 class OpenSearchPeerClusterRelation(Object): @@ -76,6 +76,7 @@ def __init__(self, charm: "OpenSearchBaseCharm", relation_name: str): self.relation_name = relation_name self.charm = charm self.peer_cm = charm.opensearch_peer_cm + self.secrets = self.charm.secrets def get_from_rel( self, key: str, rel_id: int = None, remote_app: bool = False @@ -419,20 +420,27 @@ def _rel_data( # peer rel data for requirers to show a blocked status until it's fully # ready (will receive a subsequent try: - secrets = self.charm.secrets return PeerClusterRelData( cluster_name=deployment_desc.config.cluster_name, cm_nodes=self._fetch_local_cm_nodes(deployment_desc), credentials=PeerClusterRelDataCredentials( admin_username=AdminUser, - admin_password=secrets.get(Scope.APP, secrets.password_key(AdminUser)), - admin_password_hash=secrets.get(Scope.APP, secrets.hash_key(AdminUser)), - kibana_password=secrets.get(Scope.APP, secrets.password_key(KibanaserverUser)), - kibana_password_hash=secrets.get( - Scope.APP, secrets.hash_key(KibanaserverUser) + admin_password=self.secrets.get( + Scope.APP, self.secrets.password_key(AdminUser) ), - monitor_password=secrets.get(Scope.APP, secrets.password_key(COSUser)), - admin_tls=secrets.get_object(Scope.APP, CertType.APP_ADMIN.val), + admin_password_hash=self.secrets.get( + Scope.APP, self.secrets.hash_key(AdminUser) + ), + kibana_password=self.secrets.get( + Scope.APP, self.secrets.password_key(KibanaserverUser) + ), + kibana_password_hash=self.secrets.get( + Scope.APP, self.secrets.hash_key(KibanaserverUser) + ), + monitor_password=self.secrets.get( + Scope.APP, self.secrets.password_key(COSUser) + ), + admin_tls=self.secrets.get_object(Scope.APP, CertType.APP_ADMIN.val), s3=self._s3_credentials(deployment_desc), ), deployment_desc=deployment_desc, @@ -542,31 +550,35 @@ def _rel_data_redacted_dict(self, rel_data: PeerClusterRelData) -> dict[str, Any # If we use the values of the secrets then the size of the secret # would be too large for juju we store the secret ids instead and # fetch the secrets when needed in the requirer side - secrets = self.charm.secrets - redacted_dict = rel_data.to_dict() redacted_dict["credentials"] = { - "admin-username": AdminUser, - "admin-password": secrets.get_secret_id(Scope.APP, secrets.password_key(AdminUser)), - "admin-password-hash": secrets.get_secret_id(Scope.APP, secrets.hash_key(AdminUser)), - "kibana-password": secrets.get_secret_id( - Scope.APP, secrets.password_key(KibanaserverUser) + "admin_username": AdminUser, + "admin_password": self.secrets.get_secret_id( + Scope.APP, self.secrets.password_key(AdminUser) ), - "kibana-password-hash": secrets.get_secret_id( - Scope.APP, secrets.hash_key(KibanaserverUser) + "admin_password_hash": self.secrets.get_secret_id( + Scope.APP, self.secrets.hash_key(AdminUser) + ), + "kibana_password": self.secrets.get_secret_id( + Scope.APP, self.secrets.password_key(KibanaserverUser) + ), + "kibana_password_hash": self.secrets.get_secret_id( + Scope.APP, self.secrets.hash_key(KibanaserverUser) ), } - if monitor_password := secrets.get_secret_id(Scope.APP, secrets.password_key(COSUser)): - redacted_dict["credentials"]["monitor-password"] = monitor_password - if admin_tls := secrets.get_secret_id(Scope.APP, CertType.APP_ADMIN.val): - redacted_dict["credentials"]["admin-tls"] = admin_tls + if monitor_password := self.secrets.get_secret_id( + Scope.APP, self.secrets.password_key(COSUser) + ): + redacted_dict["credentials"]["monitor_password"] = monitor_password + if admin_tls := self.secrets.get_secret_id(Scope.APP, CertType.APP_ADMIN.val): + redacted_dict["credentials"]["admin_tls"] = admin_tls if rel_data.credentials.s3: redacted_dict["credentials"]["s3"] = { - "access-key": secrets.get_secret_id(Scope.APP, "access-key"), - "secret-key": secrets.get_secret_id(Scope.APP, "access-key"), + "access_key": self.secrets.get_secret_id(Scope.APP, "access-key"), + "secret_key": self.secrets.get_secret_id(Scope.APP, "access-key"), } return redacted_dict @@ -578,7 +590,7 @@ def _grant_rel_data_secrets( credentials = rel_data_secret_content["credentials"] for key, secret_id in credentials.items(): # s3 and admin-username are not secrets - if key == "admin-username": + if key == "admin_username": continue secret = self.model.get_secret(id=secret_id) @@ -640,7 +652,7 @@ def _on_peer_cluster_relation_changed(self, event: RelationChangedEvent): # noq return # fetch the success data - data = rel_data_from_redacted_str(self.charm, data["data"]) + data = self.peer_cm.rel_data_from_redacted_str(data["data"]) # check errors that can only be figured out from the requirer side if self._error_set_from_requirer(orchestrators, deployment_desc, data, event.relation.id): return @@ -685,18 +697,27 @@ def _on_peer_cluster_relation_changed(self, event: RelationChangedEvent): # noq def _set_security_conf(self, data: PeerClusterRelData) -> None: """Store security related config.""" # set admin secrets - secrets = self.charm.secrets - secrets.put(Scope.APP, secrets.password_key(AdminUser), data.credentials.admin_password) - secrets.put(Scope.APP, secrets.hash_key(AdminUser), data.credentials.admin_password_hash) - secrets.put( - Scope.APP, secrets.password_key(KibanaserverUser), data.credentials.kibana_password + self.secrets.put( + Scope.APP, self.secrets.password_key(AdminUser), data.credentials.admin_password + ) + self.secrets.put( + Scope.APP, self.secrets.hash_key(AdminUser), data.credentials.admin_password_hash ) - secrets.put( - Scope.APP, secrets.hash_key(KibanaserverUser), data.credentials.kibana_password_hash + self.secrets.put( + Scope.APP, + self.secrets.password_key(KibanaserverUser), + data.credentials.kibana_password, + ) + self.secrets.put( + Scope.APP, + self.secrets.hash_key(KibanaserverUser), + data.credentials.kibana_password_hash, + ) + self.secrets.put( + Scope.APP, self.secrets.password_key(COSUser), data.credentials.monitor_password ) - secrets.put(Scope.APP, secrets.password_key(COSUser), data.credentials.monitor_password) - secrets.put_object(Scope.APP, CertType.APP_ADMIN.val, data.credentials.admin_tls) + self.secrets.put_object(Scope.APP, CertType.APP_ADMIN.val, data.credentials.admin_tls) # store the app admin TLS resources if not stored self.charm.tls.store_new_tls_resources(CertType.APP_ADMIN, data.credentials.admin_tls) @@ -876,7 +897,7 @@ def _cm_nodes(self, orchestrators: PeerClusterOrchestrators) -> List[Node]: if not redacted_dict_str: # not ready yet continue - data = rel_data_from_redacted_str(self.charm, redacted_dict_str) + data = self.peer_cm.rel_data_from_redacted_str(redacted_dict_str) cm_nodes = {**cm_nodes, **{node.name: node for node in data.cm_nodes}} # attempt to have an opensearch reported list of CMs - the response @@ -1026,72 +1047,3 @@ def _clear_errors(self, *error_labels: str): error = self.charm.peers_data.get(Scope.APP, error_label, "") self.charm.status.clear(error, app=True) self.charm.peers_data.delete(Scope.APP, error_label) - - -def rel_data_from_redacted_str( - charm: "OpenSearchBaseCharm", redacted_dict_str: str -) -> PeerClusterRelData | None: - """Construct the peer cluster rel data from the secret data.""" - content = json.loads(redacted_dict_str) - credentials = content["credentials"] - - secrets = charm.secrets - - admin_password = ( - charm.model.get_secret(id=credentials["admin-password"]) - .get_content() - .get(secrets.password_key(AdminUser)) - ) - - admin_password_hash = ( - charm.model.get_secret(id=credentials["admin-password-hash"]) - .get_content() - .get(secrets.hash_key(AdminUser)) - ) - - kibana_password = ( - charm.model.get_secret(id=credentials["kibana-password"]) - .get_content() - .get(secrets.password_key(KibanaserverUser)) - ) - - kibana_password_hash = ( - charm.model.get_secret(id=credentials["kibana-password-hash"]) - .get_content() - .get(secrets.hash_key(KibanaserverUser)) - ) - - monitor_password = None - if "monitor-password" in credentials: - monitor_password = ( - charm.model.get_secret(id=credentials["monitor-password"]) - .get_content() - .get(secrets.password_key(COSUser)) - ) - - admin_tls = None - if "admin-tls" in credentials: - admin_tls = charm.model.get_secret(id=credentials["admin-tls"]).get_content() - - s3 = None - if "s3" in credentials: - s3 = S3RelDataCredentials( - access_key=credentials["s3"]["access-key"], - secret_key=credentials["s3"]["secret-key"], - ) - - return PeerClusterRelData( - cluster_name=content["cluster_name"], - cm_nodes=[Node.from_dict(node) for node in content["cm_nodes"]], - credentials=PeerClusterRelDataCredentials( - admin_username=AdminUser, - admin_password=admin_password, - admin_password_hash=admin_password_hash, - kibana_password=kibana_password, - kibana_password_hash=kibana_password_hash, - monitor_password=monitor_password, - admin_tls=admin_tls, - s3=s3, - ), - deployment_desc=DeploymentDescription.from_dict(content["deployment_desc"]), - ) diff --git a/lib/charms/opensearch/v0/opensearch_secrets.py b/lib/charms/opensearch/v0/opensearch_secrets.py index 680a24cad..ac5f2e63a 100644 --- a/lib/charms/opensearch/v0/opensearch_secrets.py +++ b/lib/charms/opensearch/v0/opensearch_secrets.py @@ -40,7 +40,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 = 1 if TYPE_CHECKING: From e4d222984b8a5ba7fdc60ed136c699acc606ffb4 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Mon, 21 Oct 2024 07:40:46 +0000 Subject: [PATCH 06/14] moved granting secrets to opensearch_secrets --- .../opensearch/v0/opensearch_relation_peer_cluster.py | 5 ++--- lib/charms/opensearch/v0/opensearch_secrets.py | 7 ++++++- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index b435f4ffd..3f68d828f 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -589,14 +589,13 @@ def _grant_rel_data_secrets( """Grant the secrets to all the related apps.""" credentials = rel_data_secret_content["credentials"] for key, secret_id in credentials.items(): - # s3 and admin-username are not secrets + # admin-username is not secrets if key == "admin_username": continue - secret = self.model.get_secret(id=secret_id) for rel_id in all_rel_ids: if relation := self.get_rel(rel_id=rel_id): - secret.grant(relation) + self.secrets.grant_secret_to_relation(secret_id, relation) class OpenSearchPeerClusterRequirer(OpenSearchPeerClusterRelation): diff --git a/lib/charms/opensearch/v0/opensearch_secrets.py b/lib/charms/opensearch/v0/opensearch_secrets.py index ac5f2e63a..d65895238 100644 --- a/lib/charms/opensearch/v0/opensearch_secrets.py +++ b/lib/charms/opensearch/v0/opensearch_secrets.py @@ -27,7 +27,7 @@ Scope, SecretCache, ) -from ops import JujuVersion, Secret, SecretNotFoundError +from ops import JujuVersion, Relation, Secret, SecretNotFoundError from ops.charm import SecretChangedEvent from ops.framework import Object from overrides import override @@ -373,3 +373,8 @@ def get_secret_id(self, scope: Scope, key: str) -> Optional[str]: """Get the secret ID from the cache.""" label = self.label(scope, key) return self._charm.peers_data.get(scope, label) + + def grant_secret_to_relation(self, secret_id: int, relation: Relation): + """Grant a secret to a relation.""" + secret = self._charm.model.get_secret(id=secret_id) + secret.grant(relation) From 94408f85e2493ed2dc099767d33949d020032906 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Sat, 26 Oct 2024 07:12:24 +0000 Subject: [PATCH 07/14] fix for s3 secret ids --- .../opensearch/v0/opensearch_relation_peer_cluster.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index 3f68d828f..6f74f4229 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -595,7 +595,11 @@ def _grant_rel_data_secrets( for rel_id in all_rel_ids: if relation := self.get_rel(rel_id=rel_id): - self.secrets.grant_secret_to_relation(secret_id, relation) + if key == "s3": + self.secrets.grant_secret_to_relation(secret_id["access_key"], relation) + self.secrets.grant_secret_to_relation(secret_id["secret_key"], relation) + else: + self.secrets.grant_secret_to_relation(secret_id, relation) class OpenSearchPeerClusterRequirer(OpenSearchPeerClusterRelation): From a894458b736579e104c34c6f5e9f4f71f14909cf Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Tue, 29 Oct 2024 11:08:36 +0000 Subject: [PATCH 08/14] add checks for s3 credentials before granting secrets --- .../opensearch/v0/opensearch_relation_peer_cluster.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index 6f74f4229..10a02f836 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -596,8 +596,14 @@ def _grant_rel_data_secrets( for rel_id in all_rel_ids: if relation := self.get_rel(rel_id=rel_id): if key == "s3": - self.secrets.grant_secret_to_relation(secret_id["access_key"], relation) - self.secrets.grant_secret_to_relation(secret_id["secret_key"], relation) + if secret_id["access_key"]: + self.secrets.grant_secret_to_relation( + secret_id["access_key"], relation + ) + if secret_id["secret_key"]: + self.secrets.grant_secret_to_relation( + secret_id["secret_key"], relation + ) else: self.secrets.grant_secret_to_relation(secret_id, relation) From f95739e3b637b01618fbdbddb290fd867aca4748 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Tue, 29 Oct 2024 14:12:28 +0000 Subject: [PATCH 09/14] add conditions on s3 credentials being none --- .../opensearch/v0/opensearch_relation_peer_cluster.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index 10a02f836..f282290e7 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -575,7 +575,11 @@ def _rel_data_redacted_dict(self, rel_data: PeerClusterRelData) -> dict[str, Any if admin_tls := self.secrets.get_secret_id(Scope.APP, CertType.APP_ADMIN.val): redacted_dict["credentials"]["admin_tls"] = admin_tls - if rel_data.credentials.s3: + if ( + rel_data.credentials.s3 + and rel_data.credentials.s3.access_key + and rel_data.credentials.s3.secret_key + ): redacted_dict["credentials"]["s3"] = { "access_key": self.secrets.get_secret_id(Scope.APP, "access-key"), "secret_key": self.secrets.get_secret_id(Scope.APP, "access-key"), From ec8227b840a11669dc2cbe9ca5ef73c8d550efe6 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Tue, 29 Oct 2024 15:18:17 +0000 Subject: [PATCH 10/14] added conditions on empty acces_key and secret_key --- lib/charms/opensearch/v0/opensearch_peer_clusters.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/charms/opensearch/v0/opensearch_peer_clusters.py b/lib/charms/opensearch/v0/opensearch_peer_clusters.py index 55c6f258a..6f20c0ba9 100644 --- a/lib/charms/opensearch/v0/opensearch_peer_clusters.py +++ b/lib/charms/opensearch/v0/opensearch_peer_clusters.py @@ -634,7 +634,11 @@ def rel_data_from_redacted_str(self, redacted_dict_str: str) -> PeerClusterRelDa id=credentials["admin_tls"] ).get_content() - if "s3" in credentials: + if ( + "s3" in credentials + and credentials["s3"].get("access_key") + and credentials["s3"].get("secret_key") + ): credentials["s3"] = S3RelDataCredentials( access_key=credentials["s3"]["access_key"], secret_key=credentials["s3"]["secret_key"], From 6ee55961369bb59bc1b3b659ec9ac7f8371a94c9 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Wed, 30 Oct 2024 14:38:35 +0000 Subject: [PATCH 11/14] standardized the underscore and dash for credentials --- .../opensearch/v0/opensearch_peer_clusters.py | 17 +++++++++-------- .../v0/opensearch_relation_peer_cluster.py | 4 ++-- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_peer_clusters.py b/lib/charms/opensearch/v0/opensearch_peer_clusters.py index 6f20c0ba9..6891f4d3e 100644 --- a/lib/charms/opensearch/v0/opensearch_peer_clusters.py +++ b/lib/charms/opensearch/v0/opensearch_peer_clusters.py @@ -38,7 +38,6 @@ PeerClusterConfig, PeerClusterOrchestrators, PeerClusterRelData, - S3RelDataCredentials, StartMode, State, ) @@ -636,12 +635,14 @@ def rel_data_from_redacted_str(self, redacted_dict_str: str) -> PeerClusterRelDa if ( "s3" in credentials - and credentials["s3"].get("access_key") - and credentials["s3"].get("secret_key") + and credentials["s3"].get("access-key") + and credentials["s3"].get("secret-key") ): - credentials["s3"] = S3RelDataCredentials( - access_key=credentials["s3"]["access_key"], - secret_key=credentials["s3"]["secret_key"], - ) - + credentials["s3"]["access-key"] = self._charm.model.get_secret( + id=credentials["s3"]["access-key"] + ).get_content() + credentials["s3"]["secret-key"] = self._charm.model.get_secret( + id=credentials["s3"]["secret-key"] + ).get_content() + logging.log(logging.DEBUG, f"PeerClusterRelData.from_dict: {content}") return PeerClusterRelData.from_dict(content) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index f282290e7..25a92ad16 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -581,8 +581,8 @@ def _rel_data_redacted_dict(self, rel_data: PeerClusterRelData) -> dict[str, Any and rel_data.credentials.s3.secret_key ): redacted_dict["credentials"]["s3"] = { - "access_key": self.secrets.get_secret_id(Scope.APP, "access-key"), - "secret_key": self.secrets.get_secret_id(Scope.APP, "access-key"), + "access-key": self.secrets.get_secret_id(Scope.APP, "access-key"), + "secret-key": self.secrets.get_secret_id(Scope.APP, "access-key"), } return redacted_dict From 6c662521d130c7b966a5503ddb5bf2d4031443a5 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Tue, 5 Nov 2024 08:33:27 +0000 Subject: [PATCH 12/14] renamed keys of s3 secrets and added them to the secrets --- .../opensearch/v0/opensearch_peer_clusters.py | 17 ++++++----- .../v0/opensearch_relation_peer_cluster.py | 29 ++++++++++++------- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_peer_clusters.py b/lib/charms/opensearch/v0/opensearch_peer_clusters.py index 6891f4d3e..4e784ae02 100644 --- a/lib/charms/opensearch/v0/opensearch_peer_clusters.py +++ b/lib/charms/opensearch/v0/opensearch_peer_clusters.py @@ -638,11 +638,14 @@ def rel_data_from_redacted_str(self, redacted_dict_str: str) -> PeerClusterRelDa and credentials["s3"].get("access-key") and credentials["s3"].get("secret-key") ): - credentials["s3"]["access-key"] = self._charm.model.get_secret( - id=credentials["s3"]["access-key"] - ).get_content() - credentials["s3"]["secret-key"] = self._charm.model.get_secret( - id=credentials["s3"]["secret-key"] - ).get_content() - logging.log(logging.DEBUG, f"PeerClusterRelData.from_dict: {content}") + credentials["s3"]["access-key"] = ( + self._charm.model.get_secret(id=credentials["s3"]["access-key"]) + .get_content() + .get("s3-access-key") + ) + credentials["s3"]["secret-key"] = ( + self._charm.model.get_secret(id=credentials["s3"]["secret-key"]) + .get_content() + .get("s3-secret-key") + ) return PeerClusterRelData.from_dict(content) diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index 25a92ad16..1a21622e5 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -395,18 +395,25 @@ def _s3_credentials( return None # As the main orchestrator, this application must set the S3 information. + access_key = self.charm.backup.s3_client.get_s3_connection_info().get("access-key") + secret_key = self.charm.backup.s3_client.get_s3_connection_info().get("secret-key") + + # set the secrets in the charm + self.charm.secrets.put(Scope.APP, "s3-access-key", access_key) + self.charm.secrets.put(Scope.APP, "s3-secret-key", secret_key) + return S3RelDataCredentials( - access_key=self.charm.backup.s3_client.get_s3_connection_info().get("access-key"), - secret_key=self.charm.backup.s3_client.get_s3_connection_info().get("secret-key"), + access_key=access_key, + secret_key=secret_key, ) - if not self.charm.secrets.get(Scope.APP, "access-key"): + if not self.charm.secrets.get(Scope.APP, "s3-access-key"): return None # Return what we have received from the peer relation return S3RelDataCredentials( - access_key=self.charm.secrets.get(Scope.APP, "access-key"), - secret_key=self.charm.secrets.get(Scope.APP, "secret-key"), + access_key=self.charm.secrets.get(Scope.APP, "s3-access-key"), + secret_key=self.charm.secrets.get(Scope.APP, "s3-secret-key"), ) def _rel_data( @@ -581,8 +588,8 @@ def _rel_data_redacted_dict(self, rel_data: PeerClusterRelData) -> dict[str, Any and rel_data.credentials.s3.secret_key ): redacted_dict["credentials"]["s3"] = { - "access-key": self.secrets.get_secret_id(Scope.APP, "access-key"), - "secret-key": self.secrets.get_secret_id(Scope.APP, "access-key"), + "access-key": self.secrets.get_secret_id(Scope.APP, "s3-access-key"), + "secret-key": self.secrets.get_secret_id(Scope.APP, "s3-secret-key"), } return redacted_dict @@ -600,13 +607,13 @@ def _grant_rel_data_secrets( for rel_id in all_rel_ids: if relation := self.get_rel(rel_id=rel_id): if key == "s3": - if secret_id["access_key"]: + if secret_id["access-key"]: self.secrets.grant_secret_to_relation( - secret_id["access_key"], relation + secret_id["access-key"], relation ) - if secret_id["secret_key"]: + if secret_id["secret-key"]: self.secrets.grant_secret_to_relation( - secret_id["secret_key"], relation + secret_id["secret-key"], relation ) else: self.secrets.grant_secret_to_relation(secret_id, relation) From 545436f86cf9ee54fd5f5624284cd572072af200 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Tue, 5 Nov 2024 08:44:16 +0000 Subject: [PATCH 13/14] updated ci workflow versions --- .github/workflows/ci.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index bd5cda6eb..b1bed3a62 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -16,7 +16,7 @@ on: jobs: lint: name: Lint - uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v21.0.1 + uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v23.0.4 unit-test: name: Unit test charm @@ -24,7 +24,7 @@ jobs: timeout-minutes: 10 steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install tox & poetry run: | pipx install tox @@ -39,7 +39,7 @@ jobs: timeout-minutes: 5 steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 with: fetch-depth: 0 - run: | From 0c256ddc8960512455d78e94f3b1aee95793f416 Mon Sep 17 00:00:00 2001 From: Smail KOURTA Date: Thu, 21 Nov 2024 12:53:54 +0000 Subject: [PATCH 14/14] code refactoring and comment enhancement --- .../opensearch/v0/opensearch_peer_clusters.py | 6 +++--- .../v0/opensearch_relation_peer_cluster.py | 21 +++++++++++-------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/charms/opensearch/v0/opensearch_peer_clusters.py b/lib/charms/opensearch/v0/opensearch_peer_clusters.py index c6632011a..7f6ac9443 100644 --- a/lib/charms/opensearch/v0/opensearch_peer_clusters.py +++ b/lib/charms/opensearch/v0/opensearch_peer_clusters.py @@ -529,10 +529,10 @@ def rel_data(self) -> Optional[PeerClusterRelData]: rel = self._charm.model.get_relation( PeerClusterOrchestratorRelationName, orchestrators.main_rel_id ) - if not (redacted_dict_str := rel.data[rel.app].get("data")): + if not (data := rel.data[rel.app].get("data")): return None - return self.rel_data_from_redacted_str(redacted_dict_str) + return self.rel_data_from_str(data) def _pre_validate_roles_change(self, new_roles: List[str], prev_roles: List[str]): """Validate that the config changes of roles are allowed to happen.""" @@ -595,7 +595,7 @@ def _deployment_type(config: PeerClusterConfig, start_mode: StartMode) -> Deploy else DeploymentType.FAILOVER_ORCHESTRATOR ) - def rel_data_from_redacted_str(self, redacted_dict_str: str) -> PeerClusterRelData | None: + def rel_data_from_str(self, redacted_dict_str: str) -> PeerClusterRelData: """Construct the peer cluster rel data from the secret data.""" content = json.loads(redacted_dict_str) credentials = content["credentials"] diff --git a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py index 1a21622e5..cd45df636 100644 --- a/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py +++ b/lib/charms/opensearch/v0/opensearch_relation_peer_cluster.py @@ -260,9 +260,11 @@ def refresh_relation_data( ) # compute the data that needs to be broadcast to all related clusters (success or error) + # if rel_data is an error, prepare to broadcast it to all related clusters rel_data = self._rel_data(deployment_desc, orchestrators) - # if rel_data is an error, prepare to broadcast it to all related clusters + # if rel_data is NOT an error, we will replace the plaintext credentials in + # the object, with their corresponding secret IDs if isinstance(rel_data, PeerClusterRelData): rel_data_redacted_dict = self._rel_data_redacted_dict(rel_data) @@ -399,6 +401,7 @@ def _s3_credentials( secret_key = self.charm.backup.s3_client.get_s3_connection_info().get("secret-key") # set the secrets in the charm + # TODO Move this to s3 relation and include both in one secret self.charm.secrets.put(Scope.APP, "s3-access-key", access_key) self.charm.secrets.put(Scope.APP, "s3-secret-key", secret_key) @@ -553,10 +556,9 @@ def _fetch_local_cm_nodes(self, deployment_desc: DeploymentDescription) -> List[ ] def _rel_data_redacted_dict(self, rel_data: PeerClusterRelData) -> dict[str, Any]: - """Convert the secret data to a dict for storage in a secret.""" - # If we use the values of the secrets then the size of the secret - # would be too large for juju we store the secret ids instead and - # fetch the secrets when needed in the requirer side + """Replace the secrets' plain text content in the rel data by their IDs.""" + # hide the secrets and instead pass their ids so that + # they can be fetched when needed in the requirer side redacted_dict = rel_data.to_dict() redacted_dict["credentials"] = { @@ -587,6 +589,7 @@ def _rel_data_redacted_dict(self, rel_data: PeerClusterRelData) -> dict[str, Any and rel_data.credentials.s3.access_key and rel_data.credentials.s3.secret_key ): + # TODO Move this to s3 relation and include both in one secret redacted_dict["credentials"]["s3"] = { "access-key": self.secrets.get_secret_id(Scope.APP, "s3-access-key"), "secret-key": self.secrets.get_secret_id(Scope.APP, "s3-secret-key"), @@ -672,7 +675,7 @@ def _on_peer_cluster_relation_changed(self, event: RelationChangedEvent): # noq return # fetch the success data - data = self.peer_cm.rel_data_from_redacted_str(data["data"]) + data = self.peer_cm.rel_data_from_str(data["data"]) # check errors that can only be figured out from the requirer side if self._error_set_from_requirer(orchestrators, deployment_desc, data, event.relation.id): return @@ -913,11 +916,11 @@ def _cm_nodes(self, orchestrators: PeerClusterOrchestrators) -> List[Node]: if rel_id == -1: continue - redacted_dict_str = self.get_from_rel(key="data", rel_id=rel_id, remote_app=True) - if not redacted_dict_str: # not ready yet + data = self.get_from_rel(key="data", rel_id=rel_id, remote_app=True) + if not data: # not ready yet continue - data = self.peer_cm.rel_data_from_redacted_str(redacted_dict_str) + data = self.peer_cm.rel_data_from_str(data) cm_nodes = {**cm_nodes, **{node.name: node for node in data.cm_nodes}} # attempt to have an opensearch reported list of CMs - the response