From dd8a4c566d17f8d308e7d6518f6dfd729773155c Mon Sep 17 00:00:00 2001 From: MiaAltieri Date: Mon, 11 Mar 2024 08:02:05 +0000 Subject: [PATCH] PR feedback --- lib/charms/mongodb/v1/shards_interface.py | 44 +++++++++++------------ 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/lib/charms/mongodb/v1/shards_interface.py b/lib/charms/mongodb/v1/shards_interface.py index 0ccd9c9fd..97c1e7d02 100644 --- a/lib/charms/mongodb/v1/shards_interface.py +++ b/lib/charms/mongodb/v1/shards_interface.py @@ -31,7 +31,7 @@ ShardNotPlannedForRemovalError, ) from charms.mongodb.v1.users import BackupUser, MongoDBUser, OperatorUser -from ops.charm import CharmBase, EventBase, RelationBrokenEvent +from ops.charm import CharmBase, EventBase, RelationBrokenEvent, RelationChangedEvent from ops.framework import Object from ops.model import ( ActiveStatus, @@ -464,7 +464,9 @@ def cluster_password_synced(self) -> bool: return False raise except ServerSelectionTimeoutError as e: - if e.code == 111: # Connection refused, - i.e. TLS certs not in sync + # Connection refused, - this occurs when internal membership via TLS is not in sync + # across the cluster. + if e.code == 111: return False raise @@ -573,18 +575,18 @@ def update_member_auth( self, event: RelationChangedEvent, membership_auth: Tuple[bool, bool] ) -> None: """Updates the shard to have the same membership auth as the config-server.""" - is_key_file_contents_auth, is_tls_auth = membership_auth + cluster_auth_keyfile, cluster_auth_tls = membership_auth tls_integrated = self.charm.model.get_relation(Config.TLS.TLS_PEER_RELATION) # Edge case: shard has TLS enabled before having connected to the config-server. For TLS in # sharded MongoDB clusters it is necessary that the subject and organisation name are the # same in their CSRs. Re-requesting a cert after integrated with the config-server # regenerates the cert with the appropriate configurations needed for sharding. - if cluster_tls_ca and tls_integrated and not self.has_requested_cluster_certs(): + if cluster_auth_tls and tls_integrated and not self._should_request_new_certs(): logger.info("Cluster implements internal membership auth via certificates") self.charm.tls.request_certificate(param=None, internal=True) self.charm.tls.request_certificate(param=None, internal=False) - elif key_file_contents and not cluster_tls_ca and not tls_integrated: + elif cluster_auth_keyfile and not cluster_auth_tls and not tls_integrated: logger.info("Cluster implements internal membership auth via keyFile") # Copy over keyfile regardless of whether the cluster uses TLS or or KeyFile for internal @@ -598,7 +600,9 @@ def update_member_auth( # Future PR - status updating for inconsistencies with TLS (i.e. shard has TLS but # config-server does not and vice versa or CA-mismatch) - def get_cluster_passwords(self, event: RelationChangedEvent) -> Tuple[Optional[str], Optional[str]]: + def get_cluster_passwords( + self, event: RelationChangedEvent + ) -> Tuple[Optional[str], Optional[str]]: """Retrieves shared cluster passwords.""" operator_password = self.database_requires.fetch_relation_field( event.relation.id, OPERATOR_PASSWORD_KEY @@ -608,7 +612,7 @@ def get_cluster_passwords(self, event: RelationChangedEvent) -> Tuple[Optional[s ) return (operator_password, backup_password) - def update_cluster_passwords( + def sync_cluster_passwords( self, event: RelationChangedEvent, operator_password: str, backup_password: str ) -> None: """Updates shared cluster passwords.""" @@ -620,7 +624,7 @@ def update_cluster_passwords( except RetryError: self.charm.unit.status = BlockedStatus("Shard not added to config-server") logger.error( - "Shard could not be added to config server, failed to set operator password." + "Failed to sync cluster passwords from config-server to shard. Shard cannot be added to config-server, deferring event and retrying." ) event.defer() @@ -671,7 +675,7 @@ def _on_relation_changed(self, event): self.charm.unit.status = WaitingStatus("Waiting for secrets from config-server") return - self.update_cluster_passwords(event, operator_password, backup_password) + self.sync_cluster_passwords(event, operator_password, backup_password) # after updating the password of the backup user, restart pbm with correct password self.charm._connect_pbm_agent() @@ -971,7 +975,9 @@ def cluster_password_synced(self) -> bool: return False raise except ServerSelectionTimeoutError as e: - if e.code == 111: # Connection refused, - i.e. TLS certs not in sync + # Connection refused, - this occurs when internal membership via TLS is not in sync + # across the cluster. + if e.code == 111: return False raise @@ -1019,18 +1025,8 @@ def get_mongos_hosts(self) -> List[str]: return json.loads(config_server_relation.data[config_server_relation.app].get(HOSTS_KEY)) - def has_requested_cluster_certs(self) -> bool: + def _should_request_new_certs(self) -> bool: """Returns if the shard has already requested the certificates for internal-membership.""" - if ( - "int_certs_subject" not in self.charm.unit_peer_data - or "ext_certs_subject" not in self.charm.unit_peer_data - ): - logger.info("Unit hasn't requested necessary all certs for cluster TLS.") - return False - - int_subject = json.loads(self.charm.unit_peer_data["int_certs_subject"]) - ext_subject = json.loads(self.charm.unit_peer_data["ext_certs_subject"]) - - requested_new_int_cert = int_subject == self.get_config_server_name() - requested_new_ext_cert = ext_subject == self.get_config_server_name() - return requested_new_int_cert and requested_new_ext_cert + int_subject = self.charm.unit_peer_data.get("int_certs_subject", None) + ext_subject = self.charm.unit_peer_data.get("ext_certs_subject", None) + return {int_subject, ext_subject} != {self.get_config_server_name()}