diff --git a/src/charm.py b/src/charm.py index def71d972a..ff5bbfeda9 100755 --- a/src/charm.py +++ b/src/charm.py @@ -1753,7 +1753,7 @@ def _generate_ldap_service(self) -> dict: ldap_base_dn = ldap_params["ldapbasedn"] ldap_bind_username = ldap_params["ldapbinddn"] - ldap_bing_password = ldap_params["ldapbindpasswd"] + ldap_bind_password = ldap_params["ldapbindpasswd"] ldap_group_mappings = self.postgresql.build_postgresql_group_map(self.config.ldap_map) return { @@ -1766,7 +1766,7 @@ def _generate_ldap_service(self) -> dict: "LDAP_PORT": ldap_port, "LDAP_BASE_DN": ldap_base_dn, "LDAP_BIND_USERNAME": ldap_bind_username, - "LDAP_BIND_PASSWORD": ldap_bing_password, + "LDAP_BIND_PASSWORD": ldap_bind_password, "LDAP_GROUP_IDENTITY": json.dumps(ACCESS_GROUP_IDENTITY), "LDAP_GROUP_MAPPINGS": json.dumps(ldap_group_mappings), "POSTGRES_HOST": "127.0.0.1", @@ -1984,7 +1984,7 @@ def _restart_ldap_sync_service(self) -> None: if not self.is_primary and sync_service[0].is_running(): logger.debug("Stopping LDAP sync service. It must only run in the primary") - container.stop(self.pg_ldap_sync_service) + container.stop(self.ldap_sync_service) if self.is_primary and not self.is_ldap_enabled: logger.debug("Stopping LDAP sync service") diff --git a/src/ldap.py b/src/ldap.py index 7d2b52c4ad..ae737494d1 100644 --- a/src/ldap.py +++ b/src/ldap.py @@ -11,12 +11,9 @@ LdapRequirer, LdapUnavailableEvent, ) -from charms.postgresql_k8s.v0.postgresql_tls import ( - TLS_TRANSFER_RELATION, -) from ops import Relation from ops.framework import Object -from ops.model import ActiveStatus, BlockedStatus +from ops.model import ActiveStatus logger = logging.getLogger(__name__) @@ -35,29 +32,13 @@ def __init__(self, charm, relation_name: str): self.framework.observe(self.ldap.on.ldap_ready, self._on_ldap_ready) self.framework.observe(self.ldap.on.ldap_unavailable, self._on_ldap_unavailable) - @property - def ca_transferred(self) -> bool: - """Return whether the CA certificate has been transferred.""" - ca_transferred_relations = self.model.relations[TLS_TRANSFER_RELATION] - - for relation in ca_transferred_relations: - if relation.app.name == self._relation.app.name: - return True - - return False - @property def _relation(self) -> Relation: """Return the relation object.""" return self.model.get_relation(self.relation_name) - def _on_ldap_ready(self, event: LdapReadyEvent) -> None: + def _on_ldap_ready(self, _: LdapReadyEvent) -> None: """Handler for the LDAP ready event.""" - if not self.ca_transferred: - self.charm.unit.status = BlockedStatus("LDAP insecure. Send LDAP server certificate") - event.defer() - return - logger.debug("Enabling LDAP connection") if self.charm.unit.is_leader(): self.charm.app_peer_data.update({"ldap_enabled": "True"}) diff --git a/src/relations/postgresql_provider.py b/src/relations/postgresql_provider.py index ebb77d4b19..a828d046ae 100644 --- a/src/relations/postgresql_provider.py +++ b/src/relations/postgresql_provider.py @@ -265,8 +265,9 @@ def update_tls_flag(self, tls: str) -> None: ca = "" for relation in relations: - self.database_provides.set_tls(relation.id, tls) - self.database_provides.set_tls_ca(relation.id, ca) + if self.database_provides.fetch_relation_field(relation.id, "database"): + self.database_provides.set_tls(relation.id, tls) + self.database_provides.set_tls_ca(relation.id, ca) def _check_multiple_endpoints(self) -> bool: """Checks if there are relations with other endpoints.""" diff --git a/tests/integration/ha_tests/test_upgrade.py b/tests/integration/ha_tests/test_upgrade.py index c8a31a4db9..1958e2ef17 100644 --- a/tests/integration/ha_tests/test_upgrade.py +++ b/tests/integration/ha_tests/test_upgrade.py @@ -59,7 +59,6 @@ async def test_deploy_latest(ops_test: OpsTest) -> None: await ops_test.model.wait_for_idle( apps=[DATABASE_APP_NAME, APPLICATION_NAME], status="active", - raise_on_error=False, timeout=1000, ) assert len(ops_test.model.applications[DATABASE_APP_NAME].units) == 3 diff --git a/tests/integration/ha_tests/test_upgrade_from_stable.py b/tests/integration/ha_tests/test_upgrade_from_stable.py index 0b2fe125c7..11bed81f63 100644 --- a/tests/integration/ha_tests/test_upgrade_from_stable.py +++ b/tests/integration/ha_tests/test_upgrade_from_stable.py @@ -54,7 +54,7 @@ async def test_deploy_stable(ops_test: OpsTest) -> None: logger.info("Wait for applications to become active") async with ops_test.fast_forward(): await ops_test.model.wait_for_idle( - apps=[DATABASE_APP_NAME, APPLICATION_NAME], status="active", raise_on_error=False + apps=[DATABASE_APP_NAME, APPLICATION_NAME], status="active" ) assert len(ops_test.model.applications[DATABASE_APP_NAME].units) == 3 diff --git a/tests/integration/helpers.py b/tests/integration/helpers.py index 0b28e18901..66b8d5f47d 100644 --- a/tests/integration/helpers.py +++ b/tests/integration/helpers.py @@ -115,7 +115,6 @@ async def build_and_deploy( apps=[database_app_name], status=status, raise_on_blocked=True, - raise_on_error=False, timeout=1000, wait_for_exact_units=num_units, ) diff --git a/tests/unit/test_ldap.py b/tests/unit/test_ldap.py index eedd35d7ea..83fa1c4dbf 100644 --- a/tests/unit/test_ldap.py +++ b/tests/unit/test_ldap.py @@ -3,7 +3,6 @@ from unittest.mock import ( MagicMock, - PropertyMock, patch, ) @@ -29,14 +28,10 @@ def harness(): harness.cleanup() -def test_on_ldap_ready_with_certificate(harness): +def test_on_ldap_ready(harness): mock_event = MagicMock() - with ( - patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, - patch("charm.PostgreSQLLDAP.ca_transferred", new_callable=PropertyMock) as _ca_transferred, - ): - _ca_transferred.return_value = True + with patch("charm.PostgresqlOperatorCharm.update_config") as _update_config: harness.charm.ldap._on_ldap_ready(mock_event) _update_config.assert_called_once() @@ -45,22 +40,6 @@ def test_on_ldap_ready_with_certificate(harness): assert "ldap_enabled" in app_databag -def test_on_ldap_ready_without_certificate(harness): - mock_event = MagicMock() - - with ( - patch("charm.PostgresqlOperatorCharm.update_config") as _update_config, - patch("charm.PostgreSQLLDAP.ca_transferred", new_callable=PropertyMock) as _ca_transferred, - ): - _ca_transferred.return_value = False - harness.charm.ldap._on_ldap_ready(mock_event) - _update_config.assert_not_called() - - peer_rel_id = harness.model.get_relation(PEER).id - app_databag = harness.get_relation_data(peer_rel_id, harness.charm.app) - assert "ldap_enabled" not in app_databag - - def test_on_ldap_unavailable(harness): mock_event = MagicMock() diff --git a/tests/unit/test_postgresql_provider.py b/tests/unit/test_postgresql_provider.py index 4de278c950..1c1e1bd017 100644 --- a/tests/unit/test_postgresql_provider.py +++ b/tests/unit/test_postgresql_provider.py @@ -1,7 +1,7 @@ # Copyright 2022 Canonical Ltd. # See LICENSE file for licensing details. -from unittest.mock import Mock, PropertyMock, patch +from unittest.mock import Mock, PropertyMock, patch, sentinel import pytest from charms.postgresql_k8s.v0.postgresql import ( @@ -216,3 +216,27 @@ def test_on_relation_broken(harness): ) harness.charm.postgresql_client_relation._on_relation_broken(event) postgresql_mock.delete_user.assert_not_called() + + +def test_update_tls_flag(harness): + with ( + patch("charm.PostgreSQLTLS.get_tls_files", return_value=(None, sentinel.ca, None)), + patch( + "relations.postgresql_provider.new_password", return_value="test-password" + ) as _new_password, + patch( + "relations.postgresql_provider.DatabaseProvides.fetch_relation_field", + side_effect=[None, "db"], + ), + patch( + "relations.postgresql_provider.DatabaseProvides.set_tls", + ) as _set_tls, + patch( + "relations.postgresql_provider.DatabaseProvides.set_tls_ca", + ) as _set_tls_ca, + ): + with harness.hooks_disabled(): + second_rel = harness.add_relation(RELATION_NAME, "second_app") + harness.charm.postgresql_client_relation.update_tls_flag("True") + _set_tls.assert_called_once_with(second_rel, "True") + _set_tls_ca.assert_called_once_with(second_rel, sentinel.ca)