From ebb4f5bdf9c75870cae9b87fb1e9817150f9afd1 Mon Sep 17 00:00:00 2001 From: Amber Charitos Date: Wed, 29 May 2024 13:22:02 +0100 Subject: [PATCH 1/7] allow ldaps for ldap url --- src/structured_config.py | 2 +- tests/integration/test_policy.py | 3 +-- tests/unit/test_structured_config.py | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/structured_config.py b/src/structured_config.py index 5f04f4a..a482985 100644 --- a/src/structured_config.py +++ b/src/structured_config.py @@ -109,7 +109,7 @@ def sync_ldap_url_validator(cls, value: str) -> Optional[str]: Raises: ValueError: in the case when the value incorrectly formatted. """ - ldap_url_pattern = r"^ldap://.*:\d+$" + ldap_url_pattern = r"^ldaps?://.*:\d+$" if re.match(ldap_url_pattern, value) is not None: return value raise ValueError("Value incorrectly formatted.") diff --git a/tests/integration/test_policy.py b/tests/integration/test_policy.py index 17c44dd..5e7b71c 100644 --- a/tests/integration/test_policy.py +++ b/tests/integration/test_policy.py @@ -61,5 +61,4 @@ async def test_create_service(self, ops_test: OpsTest): new_service = ranger.get_service(TRINO_SERVICE) logger.info(f"service: {new_service}") name = new_service.get("name") - service_id = new_service.get("id") - assert name == TRINO_SERVICE and service_id == 1 + assert TRINO_SERVICE in name diff --git a/tests/unit/test_structured_config.py b/tests/unit/test_structured_config.py index 23cac66..808691c 100644 --- a/tests/unit/test_structured_config.py +++ b/tests/unit/test_structured_config.py @@ -45,7 +45,7 @@ def test_string_values(_harness) -> None: # sync-ldap-url check_invalid_values(_harness, "sync-ldap-url", erroneus_values) - accepted_values = ["ldap://ldap-k8s:3893", "ldap://example-host:636"] + accepted_values = ["ldap://ldap-k8s:3893", "ldaps://example-host:636"] check_valid_values(_harness, "sync-ldap-url", accepted_values) From db41772db27235ee145e958a861b0c3f8ee0998d Mon Sep 17 00:00:00 2001 From: Amber Charitos Date: Wed, 29 May 2024 13:27:00 +0100 Subject: [PATCH 2/7] linting for variables command and context --- src/charm.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/charm.py b/src/charm.py index 4a0c5fe..f765dc4 100755 --- a/src/charm.py +++ b/src/charm.py @@ -303,12 +303,11 @@ def update(self, event): charm_function = self.config["charm-function"].value logger.info("configuring ranger %s", charm_function) - if charm_function == "admin": - self.model.unit.open_port(port=APPLICATION_PORT, protocol="tcp") - command, context = self._configure_ranger_admin(container) - if charm_function == "usersync": command, context = self._configure_ranger_usersync(container) + else: + self.model.unit.open_port(port=APPLICATION_PORT, protocol="tcp") + command, context = self._configure_ranger_admin(container) logger.info("planning ranger %s execution", charm_function) pebble_layer = { From a3fe3ce778074d8532aff006fcf2519ab5acf7b0 Mon Sep 17 00:00:00 2001 From: Amber Charitos Date: Wed, 29 May 2024 13:31:25 +0100 Subject: [PATCH 3/7] update libs --- .../data_platform_libs/v0/data_interfaces.py | 303 ++++++++++++------ .../v0/nginx_route.py | 17 +- .../prometheus_k8s/v0/prometheus_scrape.py | 10 +- 3 files changed, 214 insertions(+), 116 deletions(-) diff --git a/lib/charms/data_platform_libs/v0/data_interfaces.py b/lib/charms/data_platform_libs/v0/data_interfaces.py index 4a2ee5a..b331bdc 100644 --- a/lib/charms/data_platform_libs/v0/data_interfaces.py +++ b/lib/charms/data_platform_libs/v0/data_interfaces.py @@ -331,7 +331,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 = 33 +LIBPATCH = 36 PYDEPS = ["ops>=2.0.0"] @@ -493,6 +493,7 @@ def wrapper(self, *args, **kwargs): return return f(self, *args, **kwargs) + wrapper.leader_only = True return wrapper @@ -559,6 +560,7 @@ def __init__( component: Union[Application, Unit], label: str, secret_uri: Optional[str] = None, + legacy_labels: List[str] = [], ): self._secret_meta = None self._secret_content = {} @@ -566,16 +568,25 @@ def __init__( self.label = label self._model = model self.component = component + self.legacy_labels = legacy_labels + self.current_label = None - def add_secret(self, content: Dict[str, str], relation: Relation) -> Secret: + def add_secret( + self, + content: Dict[str, str], + relation: Optional[Relation] = None, + label: Optional[str] = None, + ) -> Secret: """Create a new secret.""" if self._secret_uri: raise SecretAlreadyExistsError( "Secret is already defined with uri %s", self._secret_uri ) - secret = self.component.add_secret(content, label=self.label) - if relation.app != self._model.app: + label = self.label if not label else label + + secret = self.component.add_secret(content, label=label) + if relation and relation.app != self._model.app: # If it's not a peer relation, grant is to be applied secret.grant(relation) self._secret_uri = secret.id @@ -588,13 +599,20 @@ def meta(self) -> Optional[Secret]: if not self._secret_meta: if not (self._secret_uri or self.label): return - try: - self._secret_meta = self._model.get_secret(label=self.label) - except SecretNotFoundError: - if self._secret_uri: - self._secret_meta = self._model.get_secret( - id=self._secret_uri, label=self.label - ) + + for label in [self.label] + self.legacy_labels: + try: + self._secret_meta = self._model.get_secret(label=label) + except SecretNotFoundError: + pass + else: + if label != self.label: + self.current_label = label + break + + # If still not found, to be checked by URI, to be labelled with the proposed label + if not self._secret_meta and self._secret_uri: + self._secret_meta = self._model.get_secret(id=self._secret_uri, label=self.label) return self._secret_meta def get_content(self) -> Dict[str, str]: @@ -618,12 +636,30 @@ def get_content(self) -> Dict[str, str]: self._secret_content = self.meta.get_content() return self._secret_content + def _move_to_new_label_if_needed(self): + """Helper function to re-create the secret with a different label.""" + if not self.current_label or not (self.meta and self._secret_meta): + return + + # Create a new secret with the new label + content = self._secret_meta.get_content() + self._secret_uri = None + + # I wish we could just check if we are the owners of the secret... + try: + self._secret_meta = self.add_secret(content, label=self.label) + except ModelError as err: + if "this unit is not the leader" not in str(err): + raise + self.current_label = None + def set_content(self, content: Dict[str, str]) -> None: """Setting cached secret content.""" if not self.meta: return if content: + self._move_to_new_label_if_needed() self.meta.set_content(content) self._secret_content = content else: @@ -655,10 +691,14 @@ def __init__(self, model: Model, component: Union[Application, Unit]): self.component = component self._secrets: Dict[str, CachedSecret] = {} - def get(self, label: str, uri: Optional[str] = None) -> Optional[CachedSecret]: + def get( + self, label: str, uri: Optional[str] = None, legacy_labels: List[str] = [] + ) -> Optional[CachedSecret]: """Getting a secret from Juju Secret store or cache.""" if not self._secrets.get(label): - secret = CachedSecret(self._model, self.component, label, uri) + secret = CachedSecret( + self._model, self.component, label, uri, legacy_labels=legacy_labels + ) if secret.meta: self._secrets[label] = secret return self._secrets.get(label) @@ -676,10 +716,14 @@ def add(self, label: str, content: Dict[str, str], relation: Relation) -> Cached def remove(self, label: str) -> None: """Remove a secret from the cache.""" if secret := self.get(label): - secret.remove() - self._secrets.pop(label) - else: - logging.error("Non-existing Juju Secret was attempted to be removed %s", label) + try: + secret.remove() + self._secrets.pop(label) + except (SecretsUnavailableError, KeyError): + pass + else: + return + logging.debug("Non-existing Juju Secret was attempted to be removed %s", label) ################################################################################ @@ -716,11 +760,21 @@ def __setitem__(self, key: str, item: str) -> None: def __getitem__(self, key: str) -> str: """Get an item of the Abstract Relation Data dictionary.""" result = None - if not (result := self.relation_data.fetch_my_relation_field(self.relation_id, key)): + + # Avoiding "leader_only" error when cross-charm non-leader unit, not to report useless error + if ( + not hasattr(self.relation_data.fetch_my_relation_field, "leader_only") + or self.relation_data.component != self.relation_data.local_app + or self.relation_data.local_unit.is_leader() + ): + result = self.relation_data.fetch_my_relation_field(self.relation_id, key) + + if not result: try: result = self.relation_data.fetch_relation_field(self.relation_id, key) except NotImplementedError: pass + if not result: raise KeyError return result @@ -1095,7 +1149,7 @@ def _delete_relation_data_without_secrets( try: relation.data[component].pop(field) except KeyError: - logger.error( + logger.debug( "Non-existing field '%s' was attempted to be removed from the databag (relation ID: %s)", str(field), str(relation.id), @@ -1351,7 +1405,7 @@ def _delete_relation_secret( try: new_content.pop(field) except KeyError: - logging.error( + logging.debug( "Non-existing secret was attempted to be removed %s, %s", str(relation.id), str(field), @@ -1532,7 +1586,7 @@ def _register_secret_to_relation( """ label = self._generate_secret_label(relation_name, relation_id, group) - # Fetchin the Secret's meta information ensuring that it's locally getting registered with + # Fetching the Secret's meta information ensuring that it's locally getting registered with CachedSecret(self._model, self.component, label, secret_id).meta def _register_secrets_to_relation(self, relation: Relation, params_name_list: List[str]): @@ -1723,6 +1777,7 @@ def __init__( self._secret_label_map = {} # Secrets that are being dynamically added within the scope of this event handler run self._new_secrets = [] + self._additional_secret_group_mapping = additional_secret_group_mapping for group, fields in additional_secret_group_mapping.items(): if group not in SECRET_GROUPS.groups(): @@ -1769,12 +1824,15 @@ def current_secret_fields(self) -> List[str]: relation = self._model.relations[self.relation_name][0] fields = [] + + ignores = [SECRET_GROUPS.get_group("user"), SECRET_GROUPS.get_group("tls")] for group in SECRET_GROUPS.groups(): + if group in ignores: + continue if content := self._get_group_secret_contents(relation, group): - fields += [self._field_to_internal_name(field, group) for field in content] + fields += list(content.keys()) return list(set(fields) | set(self._new_secrets)) - @juju_secrets_only @dynamic_secrets_only def set_secret( self, @@ -1792,13 +1850,13 @@ def set_secret( group_mapping: The name of the "secret group", in case the field is to be added to an existing secret """ full_field = self._field_to_internal_name(field, group_mapping) - if full_field not in self.current_secret_fields: + if self.secrets_enabled and full_field not in self.current_secret_fields: self._new_secrets.append(full_field) - self.update_relation_data(relation_id, {full_field: value}) + if self._no_group_with_databag(field, full_field): + self.update_relation_data(relation_id, {full_field: value}) # Unlike for set_secret(), there's no harm using this operation with static secrets # The restricion is only added to keep the concept clear - @juju_secrets_only @dynamic_secrets_only def get_secret( self, @@ -1808,13 +1866,15 @@ def get_secret( ) -> Optional[str]: """Public interface method to fetch secrets only.""" full_field = self._field_to_internal_name(field, group_mapping) - if full_field not in self.current_secret_fields: - raise SecretsUnavailableError( - f"Secret {field} from group {group_mapping} was not found" - ) - return self.fetch_my_relation_field(relation_id, full_field) + if ( + self.secrets_enabled + and full_field not in self.current_secret_fields + and field not in self.current_secret_fields + ): + return + if self._no_group_with_databag(field, full_field): + return self.fetch_my_relation_field(relation_id, full_field) - @juju_secrets_only @dynamic_secrets_only def delete_secret( self, @@ -1824,9 +1884,11 @@ def delete_secret( ) -> Optional[str]: """Public interface method to delete secrets only.""" full_field = self._field_to_internal_name(field, group_mapping) - if full_field not in self.current_secret_fields: + if self.secrets_enabled and full_field not in self.current_secret_fields: logger.warning(f"Secret {field} from group {group_mapping} was not found") - self.delete_relation_data(relation_id, [full_field]) + return + if self._no_group_with_databag(field, full_field): + self.delete_relation_data(relation_id, [full_field]) # Helpers @@ -1870,6 +1932,73 @@ def _content_for_secret_group( if k in self.secret_fields } + # Backwards compatibility + + def _check_deleted_label(self, relation, fields) -> None: + """Helper function for legacy behavior.""" + current_data = self.fetch_my_relation_data([relation.id], fields) + if current_data is not None: + # Check if the secret we wanna delete actually exists + # Given the "deleted label", here we can't rely on the default mechanism (i.e. 'key not found') + if non_existent := (set(fields) & set(self.secret_fields)) - set( + current_data.get(relation.id, []) + ): + logger.debug( + "Non-existing secret %s was attempted to be removed.", + ", ".join(non_existent), + ) + + def _remove_secret_from_databag(self, relation, fields: List[str]) -> None: + """For Rolling Upgrades -- when moving from databag to secrets usage. + + Practically what happens here is to remove stuff from the databag that is + to be stored in secrets. + """ + if not self.secret_fields: + return + + secret_fields_passed = set(self.secret_fields) & set(fields) + for field in secret_fields_passed: + if self._fetch_relation_data_without_secrets(self.component, relation, [field]): + self._delete_relation_data_without_secrets(self.component, relation, [field]) + + def _remove_secret_field_name_from_databag(self, relation) -> None: + """Making sure that the old databag URI is gone. + + This action should not be executed more than once. + """ + # Nothing to do if 'internal-secret' is not in the databag + if not (relation.data[self.component].get(self._generate_secret_field_name())): + return + + # Making sure that the secret receives its label + # (This should have happened by the time we get here, rather an extra security measure.) + secret = self._get_relation_secret(relation.id) + + # Either app scope secret with leader executing, or unit scope secret + leader_or_unit_scope = self.component != self.local_app or self.local_unit.is_leader() + if secret and leader_or_unit_scope: + # Databag reference to the secret URI can be removed, now that it's labelled + relation.data[self.component].pop(self._generate_secret_field_name(), None) + + def _previous_labels(self) -> List[str]: + """Generator for legacy secret label names, for backwards compatibility.""" + result = [] + members = [self._model.app.name] + if self.scope: + members.append(self.scope.value) + result.append(f"{'.'.join(members)}") + return result + + def _no_group_with_databag(self, field: str, full_field: str) -> bool: + """Check that no secret group is attempted to be used together with databag.""" + if not self.secrets_enabled and full_field != field: + logger.error( + f"Can't access {full_field}: no secrets available (i.e. no secret groups either)." + ) + return False + return True + # Event handlers def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: @@ -1885,7 +2014,7 @@ def _on_secret_changed_event(self, event: SecretChangedEvent) -> None: def _generate_secret_label( self, relation_name: str, relation_id: int, group_mapping: SecretGroup ) -> str: - members = [self._model.app.name] + members = [relation_name, self._model.app.name] if self.scope: members.append(self.scope.value) if group_mapping != SECRET_GROUPS.EXTRA: @@ -1919,16 +2048,12 @@ def _get_relation_secret( label = self._generate_secret_label(relation_name, relation_id, group_mapping) secret_uri = relation.data[self.component].get(self._generate_secret_field_name(), None) - # Fetching the secret with fallback to URI (in case label is not yet known) - # Label would we "stuck" on the secret in case it is found - secret = self.secrets.get(label, secret_uri) - - # Either app scope secret with leader executing, or unit scope secret - leader_or_unit_scope = self.component != self.local_app or self.local_unit.is_leader() - if secret_uri and secret and leader_or_unit_scope: - # Databag reference to the secret URI can be removed, now that it's labelled - relation.data[self.component].pop(self._generate_secret_field_name(), None) - return secret + # URI or legacy label is only to applied when moving single legacy secret to a (new) label + if group_mapping == SECRET_GROUPS.EXTRA: + # Fetching the secret with fallback to URI (in case label is not yet known) + # Label would we "stuck" on the secret in case it is found + return self.secrets.get(label, secret_uri, legacy_labels=self._previous_labels()) + return self.secrets.get(label) def _get_group_secret_contents( self, @@ -1939,27 +2064,11 @@ def _get_group_secret_contents( """Helper function to retrieve collective, requested contents of a secret.""" secret_fields = [self._internal_name_to_field(k)[0] for k in secret_fields] result = super()._get_group_secret_contents(relation, group, secret_fields) - if not self.deleted_label: - return result - return { - self._field_to_internal_name(key, group): result[key] - for key in result - if result[key] != self.deleted_label - } - - def _remove_secret_from_databag(self, relation, fields: List[str]) -> None: - """For Rolling Upgrades -- when moving from databag to secrets usage. - - Practically what happens here is to remove stuff from the databag that is - to be stored in secrets. - """ - if not self.secret_fields: - return - - secret_fields_passed = set(self.secret_fields) & set(fields) - for field in secret_fields_passed: - if self._fetch_relation_data_without_secrets(self.component, relation, [field]): - self._delete_relation_data_without_secrets(self.component, relation, [field]) + if self.deleted_label: + result = {key: result[key] for key in result if result[key] != self.deleted_label} + if self._additional_secret_group_mapping: + return {self._field_to_internal_name(key, group): result[key] for key in result} + return result @either_static_or_dynamic_secrets def _fetch_my_specific_relation_data( @@ -1982,6 +2091,7 @@ def _update_relation_data(self, relation: Relation, data: Dict[str, str]) -> Non data=data, uri_to_databag=False, ) + self._remove_secret_field_name_from_databag(relation) normal_content = {k: v for k, v in data.items() if k in normal_fields} self._update_relation_data_without_secrets(self.component, relation, normal_content) @@ -1990,17 +2100,8 @@ def _update_relation_data(self, relation: Relation, data: Dict[str, str]) -> Non 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.""" if self.secret_fields and self.deleted_label: - current_data = self.fetch_my_relation_data([relation.id], fields) - if current_data is not None: - # Check if the secret we wanna delete actually exists - # Given the "deleted label", here we can't rely on the default mechanism (i.e. 'key not found') - if non_existent := (set(fields) & set(self.secret_fields)) - set( - current_data.get(relation.id, []) - ): - logger.error( - "Non-existing secret %s was attempted to be removed.", - ", ".join(non_existent), - ) + # Legacy, backwards compatibility + self._check_deleted_label(relation, fields) _, normal_fields = self._process_secret_fields( relation, @@ -2036,19 +2137,10 @@ def fetch_relation_field( "fetch_my_relation_data() and fetch_my_relation_field()" ) - 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. - - Re-implementing the inherited function due to field@group conversion - """ - if relation_data := self.fetch_my_relation_data([relation_id], [field], relation_name): - return relation_data.get(relation_id, {}).get(self._internal_name_to_field(field)[0]) - # Public functions -- inherited fetch_my_relation_data = Data.fetch_my_relation_data + fetch_my_relation_field = Data.fetch_my_relation_field class DataPeerEventHandlers(RequirerEventHandlers): @@ -2217,7 +2309,7 @@ def _secrets(self) -> dict: return self._cached_secrets def _get_secret(self, group) -> Optional[Dict[str, str]]: - """Retrieveing secrets.""" + """Retrieving secrets.""" if not self.app: return if not self._secrets.get(group): @@ -2924,7 +3016,7 @@ class KafkaRequiresEvents(CharmEvents): # Kafka Provides and Requires -class KafkaProvidesData(ProviderData): +class KafkaProviderData(ProviderData): """Provider-side of the Kafka relation.""" def __init__(self, model: Model, relation_name: str) -> None: @@ -2967,12 +3059,12 @@ def set_zookeeper_uris(self, relation_id: int, zookeeper_uris: str) -> None: self.update_relation_data(relation_id, {"zookeeper-uris": zookeeper_uris}) -class KafkaProvidesEventHandlers(EventHandlers): +class KafkaProviderEventHandlers(EventHandlers): """Provider-side of the Kafka relation.""" on = KafkaProvidesEvents() # pyright: ignore [reportAssignmentType] - def __init__(self, charm: CharmBase, relation_data: KafkaProvidesData) -> None: + def __init__(self, charm: CharmBase, relation_data: KafkaProviderData) -> None: super().__init__(charm, relation_data) # Just to keep lint quiet, can't resolve inheritance. The same happened in super().__init__() above self.relation_data = relation_data @@ -2994,15 +3086,15 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: ) -class KafkaProvides(KafkaProvidesData, KafkaProvidesEventHandlers): +class KafkaProvides(KafkaProviderData, KafkaProviderEventHandlers): """Provider-side of the Kafka relation.""" def __init__(self, charm: CharmBase, relation_name: str) -> None: - KafkaProvidesData.__init__(self, charm.model, relation_name) - KafkaProvidesEventHandlers.__init__(self, charm, self) + KafkaProviderData.__init__(self, charm.model, relation_name) + KafkaProviderEventHandlers.__init__(self, charm, self) -class KafkaRequiresData(RequirerData): +class KafkaRequirerData(RequirerData): """Requirer-side of the Kafka relation.""" def __init__( @@ -3032,12 +3124,12 @@ def topic(self, value): self._topic = value -class KafkaRequiresEventHandlers(RequirerEventHandlers): +class KafkaRequirerEventHandlers(RequirerEventHandlers): """Requires-side of the Kafka relation.""" on = KafkaRequiresEvents() # pyright: ignore [reportAssignmentType] - def __init__(self, charm: CharmBase, relation_data: KafkaRequiresData) -> None: + def __init__(self, charm: CharmBase, relation_data: KafkaRequirerData) -> None: super().__init__(charm, relation_data) # Just to keep lint quiet, can't resolve inheritance. The same happened in super().__init__() above self.relation_data = relation_data @@ -3050,10 +3142,13 @@ def _on_relation_created_event(self, event: RelationCreatedEvent) -> None: 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"] - } + relation_data = {"topic": self.relation_data.topic} + + if self.relation_data.extra_user_roles: + relation_data["extra-user-roles"] = self.relation_data.extra_user_roles + + if self.relation_data.consumer_group_prefix: + relation_data["consumer-group-prefix"] = self.relation_data.consumer_group_prefix self.relation_data.update_relation_data(event.relation.id, relation_data) @@ -3096,7 +3191,7 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: return -class KafkaRequires(KafkaRequiresData, KafkaRequiresEventHandlers): +class KafkaRequires(KafkaRequirerData, KafkaRequirerEventHandlers): """Provider-side of the Kafka relation.""" def __init__( @@ -3108,7 +3203,7 @@ def __init__( consumer_group_prefix: Optional[str] = None, additional_secret_fields: Optional[List[str]] = [], ) -> None: - KafkaRequiresData.__init__( + KafkaRequirerData.__init__( self, charm.model, relation_name, @@ -3117,7 +3212,7 @@ def __init__( consumer_group_prefix, additional_secret_fields, ) - KafkaRequiresEventHandlers.__init__(self, charm, self) + KafkaRequirerEventHandlers.__init__(self, charm, self) # Opensearch related events diff --git a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py index c9a22dd..a2ec38e 100644 --- a/lib/charms/nginx_ingress_integrator/v0/nginx_route.py +++ b/lib/charms/nginx_ingress_integrator/v0/nginx_route.py @@ -86,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 = 6 +LIBPATCH = 7 __all__ = ["require_nginx_route", "provide_nginx_route"] @@ -119,7 +119,7 @@ class _NginxRouteCharmEvents(ops.charm.CharmEvents): nginx_route_broken = ops.framework.EventSource(_NginxRouteBrokenEvent) -class _NginxRouteRequirer(ops.framework.Object): +class NginxRouteRequirer(ops.framework.Object): """This class defines the functionality for the 'requires' side of the 'nginx-route' relation. Hook events observed: @@ -148,7 +148,7 @@ def __init__( self._config_reconciliation, ) # Set default values. - self._config: typing.Dict[str, typing.Union[str, int, bool]] = { + self.config: typing.Dict[str, typing.Union[str, int, bool]] = { "service-namespace": self._charm.model.name, **config, } @@ -163,11 +163,11 @@ def _config_reconciliation(self, _event: typing.Any = None) -> None: delete_keys = { relation_field for relation_field in relation_app_data - if relation_field not in self._config + if relation_field not in self.config } for delete_key in delete_keys: del relation_app_data[delete_key] - relation_app_data.update({k: str(v) for k, v in self._config.items()}) + relation_app_data.update({k: str(v) for k, v in self.config.items()}) # C901 is ignored since the method has too many ifs but wouldn't be @@ -195,7 +195,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to session_cookie_max_age: typing.Optional[int] = None, tls_secret_name: typing.Optional[str] = None, nginx_route_relation_name: str = "nginx-route", -) -> None: +) -> NginxRouteRequirer: """Set up nginx-route relation handlers on the requirer side. This function must be invoked in the charm class constructor. @@ -242,6 +242,9 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to nginx_route_relation_name: Specifies the relation name of the relation handled by this requirer class. The relation must have the nginx-route interface. + + Returns: + the NginxRouteRequirer. """ config: typing.Dict[str, typing.Union[str, int, bool]] = {} if service_hostname is not None: @@ -281,7 +284,7 @@ def require_nginx_route( # pylint: disable=too-many-locals,too-many-branches,to if tls_secret_name is not None: config["tls-secret-name"] = tls_secret_name - _NginxRouteRequirer( + return NginxRouteRequirer( charm=charm, config=config, nginx_route_relation_name=nginx_route_relation_name ) diff --git a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py index 72c3fe7..e3d35c6 100644 --- a/lib/charms/prometheus_k8s/v0/prometheus_scrape.py +++ b/lib/charms/prometheus_k8s/v0/prometheus_scrape.py @@ -178,7 +178,7 @@ def __init__(self, *args): - `scrape_timeout` - `proxy_url` - `relabel_configs` -- `metrics_relabel_configs` +- `metric_relabel_configs` - `sample_limit` - `label_limit` - `label_name_length_limit` @@ -362,7 +362,7 @@ def _on_scrape_targets_changed(self, event): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 45 +LIBPATCH = 47 PYDEPS = ["cosl"] @@ -377,7 +377,7 @@ def _on_scrape_targets_changed(self, event): "scrape_timeout", "proxy_url", "relabel_configs", - "metrics_relabel_configs", + "metric_relabel_configs", "sample_limit", "label_limit", "label_name_length_limit", @@ -521,8 +521,8 @@ def expand_wildcard_targets_into_individual_jobs( # for such a target. Therefore labeling with Juju topology, excluding the # unit name. non_wildcard_static_config["labels"] = { - **non_wildcard_static_config.get("labels", {}), **topology.label_matcher_dict, + **non_wildcard_static_config.get("labels", {}), } non_wildcard_static_configs.append(non_wildcard_static_config) @@ -547,9 +547,9 @@ def expand_wildcard_targets_into_individual_jobs( if topology: # Add topology labels modified_static_config["labels"] = { - **modified_static_config.get("labels", {}), **topology.label_matcher_dict, **{"juju_unit": unit_name}, + **modified_static_config.get("labels", {}), } # Instance relabeling for topology should be last in order. From b814cb20f7061215a805d613c5c3d47bcad5e362 Mon Sep 17 00:00:00 2001 From: Amber Charitos Date: Wed, 29 May 2024 14:44:31 +0100 Subject: [PATCH 4/7] add an else for failure to include charm_function logic --- src/charm.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index f765dc4..169051a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -305,9 +305,13 @@ def update(self, event): if charm_function == "usersync": command, context = self._configure_ranger_usersync(container) - else: + elif charm_function == "admin": self.model.unit.open_port(port=APPLICATION_PORT, protocol="tcp") command, context = self._configure_ranger_admin(container) + else: + assert ( + False + ), "Programmer error, please add your 'charm_function' to the logic." logger.info("planning ranger %s execution", charm_function) pebble_layer = { From c70e5ed4d479e49185387a0dc14c71b3e39b0f2e Mon Sep 17 00:00:00 2001 From: Amber Charitos Date: Wed, 29 May 2024 14:50:22 +0100 Subject: [PATCH 5/7] remove assert --- src/charm.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 169051a..f9da258 100755 --- a/src/charm.py +++ b/src/charm.py @@ -309,9 +309,10 @@ def update(self, event): self.model.unit.open_port(port=APPLICATION_PORT, protocol="tcp") command, context = self._configure_ranger_admin(container) else: - assert ( - False - ), "Programmer error, please add your 'charm_function' to the logic." + logger.error( + "Programmer error, please add your 'charm_function' to the logic." + ) + return logger.info("planning ranger %s execution", charm_function) pebble_layer = { From 4448be8a9a4e0d57442ec901b22a5035df8598c0 Mon Sep 17 00:00:00 2001 From: Amber Charitos Date: Thu, 30 May 2024 10:21:01 +0100 Subject: [PATCH 6/7] add blocked status and close usersync port --- src/charm.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index f9da258..5d7f4fa 100755 --- a/src/charm.py +++ b/src/charm.py @@ -305,13 +305,12 @@ def update(self, event): if charm_function == "usersync": command, context = self._configure_ranger_usersync(container) + self.model.unit.close_port(port=APPLICATION_PORT, protocol="tcp") elif charm_function == "admin": self.model.unit.open_port(port=APPLICATION_PORT, protocol="tcp") command, context = self._configure_ranger_admin(container) else: - logger.error( - "Programmer error, please add your 'charm_function' to the logic." - ) + self.unit.status = BlockedStatus("Missing charm-function.") return logger.info("planning ranger %s execution", charm_function) From df290695fc5218d4962e57803577b3d5e2ea8734 Mon Sep 17 00:00:00 2001 From: Amber Charitos Date: Thu, 30 May 2024 10:30:57 +0100 Subject: [PATCH 7/7] close port by default --- src/charm.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 5d7f4fa..678a0a6 100755 --- a/src/charm.py +++ b/src/charm.py @@ -303,9 +303,10 @@ def update(self, event): charm_function = self.config["charm-function"].value logger.info("configuring ranger %s", charm_function) + self.model.unit.close_port(port=APPLICATION_PORT, protocol="tcp") + if charm_function == "usersync": command, context = self._configure_ranger_usersync(container) - self.model.unit.close_port(port=APPLICATION_PORT, protocol="tcp") elif charm_function == "admin": self.model.unit.open_port(port=APPLICATION_PORT, protocol="tcp") command, context = self._configure_ranger_admin(container)