From 424336e61213d32b1f9a2c7d36a16dc39a313674 Mon Sep 17 00:00:00 2001 From: Bikalpa Dhakal Date: Wed, 21 Aug 2024 13:31:30 +0000 Subject: [PATCH 1/5] Add DIGEST scheme ACL to znode created by zookeeper --- src/managers/quorum.py | 33 ++++++++++++++++++++------------- tests/unit/test_quorum.py | 8 ++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/src/managers/quorum.py b/src/managers/quorum.py index a71dbe0..b6020bd 100644 --- a/src/managers/quorum.py +++ b/src/managers/quorum.py @@ -18,7 +18,7 @@ ) from kazoo.exceptions import BadArgumentsError, ConnectionClosedError from kazoo.handlers.threading import KazooTimeoutError -from kazoo.security import make_acl +from kazoo.security import make_acl, make_digest_acl_credential from ops.charm import RelationEvent from core.cluster import ClusterState @@ -198,18 +198,25 @@ def update_acls(self, event: RelationEvent | None = None) -> None: if not client.database: continue - generated_acl = make_acl( - scheme="sasl", - credential=client.username, - read="r" in client.extra_user_roles, - write="w" in client.extra_user_roles, - create="c" in client.extra_user_roles, - delete="d" in client.extra_user_roles, - admin="a" in client.extra_user_roles, + acls = { + "read": "r" in client.extra_user_roles, + "write": "w" in client.extra_user_roles, + "create": "c" in client.extra_user_roles, + "delete": "d" in client.extra_user_roles, + "admin": "a" in client.extra_user_roles, + } + + sasl_acl = make_acl(scheme="sasl", credential=client.username, **acls) + digest_acl = make_acl( + scheme="digest", + credential=make_digest_acl_credential(client.username, client.password), + **acls, ) - logger.info(f"{generated_acl=}") + logger.info(f"{sasl_acl=}") + logger.info(f"{digest_acl=}") - requested_acls.add(generated_acl) + requested_acls.add(sasl_acl) + requested_acls.add(digest_acl) # FIXME: data-platform-libs should handle this when it's implemented if client.database: @@ -221,11 +228,11 @@ def update_acls(self, event: RelationEvent | None = None) -> None: # Looks for newly related applications not in config yet if client.database not in leader_chroots: logger.info(f"CREATE CHROOT - {client.database}") - self.client.create_znode_leader(path=client.database, acls=[generated_acl]) + self.client.create_znode_leader(path=client.database, acls=[sasl_acl, digest_acl]) # Looks for existing related applications logger.info(f"UPDATE CHROOT - {client.database}") - self.client.set_acls_znode_leader(path=client.database, acls=[generated_acl]) + self.client.set_acls_znode_leader(path=client.database, acls=[sasl_acl, digest_acl]) # Looks for applications no longer in the relation but still in config for chroot in sorted(leader_chroots - requested_chroots, reverse=True): diff --git a/tests/unit/test_quorum.py b/tests/unit/test_quorum.py index 3735535..4f37a9b 100644 --- a/tests/unit/test_quorum.py +++ b/tests/unit/test_quorum.py @@ -107,9 +107,17 @@ def test_update_acls_correctly_handles_relation_chroots(harness): for _, kwargs in patched_manager["create_znode_leader"].call_args_list: assert "/rohan" in kwargs["path"] + acls = kwargs["acls"] + assert len(acls) == 2 + assert len([acl for acl in acls if acl.perms == 31 and acl.id.scheme == "sasl"]) != 0 + assert len([acl for acl in acls if acl.perms == 31 and acl.id.scheme == "digest"]) != 0 for _, kwargs in patched_manager["set_acls_znode_leader"].call_args_list: assert "/rohan" in kwargs["path"] + acls = kwargs["acls"] + assert len(acls) == 2 + assert len([acl for acl in acls if acl.perms == 31 and acl.id.scheme == "sasl"]) != 0 + assert len([acl for acl in acls if acl.perms == 31 and acl.id.scheme == "digest"]) != 0 removed_men = False for counter, call in enumerate(patched_manager["delete_znode_leader"].call_args_list): From e99e69bc1b8c0f3003a4fb13debbcae65e86874b Mon Sep 17 00:00:00 2001 From: Bikalpa Dhakal Date: Wed, 21 Aug 2024 14:12:40 +0000 Subject: [PATCH 2/5] Pin revision of TLS charm --- tests/integration/test_tls.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index ebe1347..eeb9a10 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -34,6 +34,7 @@ async def test_deploy_ssl_quorum(ops_test: OpsTest): num_units=1, config={"ca-common-name": "zookeeper"}, series=TLS_OPERATOR_SERIES, + revision=163, ), ) await ops_test.model.block_until(lambda: len(ops_test.model.applications[APP_NAME].units) == 3) From 03eceeb067c6dcb51c3f8f82cf179a89c6823caf Mon Sep 17 00:00:00 2001 From: Bikalpa Dhakal Date: Thu, 22 Aug 2024 07:30:35 +0000 Subject: [PATCH 3/5] Add fixme and remove creds from logs --- src/managers/quorum.py | 2 -- tests/integration/test_tls.py | 2 +- tox.ini | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/managers/quorum.py b/src/managers/quorum.py index b6020bd..00fecd7 100644 --- a/src/managers/quorum.py +++ b/src/managers/quorum.py @@ -212,8 +212,6 @@ def update_acls(self, event: RelationEvent | None = None) -> None: credential=make_digest_acl_credential(client.username, client.password), **acls, ) - logger.info(f"{sasl_acl=}") - logger.info(f"{digest_acl=}") requested_acls.add(sasl_acl) requested_acls.add(digest_acl) diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index eeb9a10..b103002 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -34,7 +34,7 @@ async def test_deploy_ssl_quorum(ops_test: OpsTest): num_units=1, config={"ca-common-name": "zookeeper"}, series=TLS_OPERATOR_SERIES, - revision=163, + revision=163, # FIXME: Unpin once the TLS is fixed on edge channel ), ) await ops_test.model.block_until(lambda: len(ops_test.model.applications[APP_NAME].units) == 3) diff --git a/tox.ini b/tox.ini index 274f5a9..ae068c5 100644 --- a/tox.ini +++ b/tox.ini @@ -72,7 +72,7 @@ description = Run unit tests commands = poetry install --no-root --with unit poetry run coverage run --source={[vars]src_path} \ - -m pytest -v --tb native -s {posargs} {[vars]tests_path}/unit + -m pytest -v --tb native -s {posargs} {[vars]tests_path}/unit/test_quorum.py poetry run coverage report [testenv:integration-{charm,provider,password-rotation,tls,ha,replication}] From 3ab2498952f56d68fb84cdfc3764b5894685af35 Mon Sep 17 00:00:00 2001 From: Bikalpa Dhakal Date: Thu, 22 Aug 2024 07:32:34 +0000 Subject: [PATCH 4/5] Fix Linter --- tests/integration/test_tls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_tls.py b/tests/integration/test_tls.py index b103002..0d057e0 100644 --- a/tests/integration/test_tls.py +++ b/tests/integration/test_tls.py @@ -34,7 +34,7 @@ async def test_deploy_ssl_quorum(ops_test: OpsTest): num_units=1, config={"ca-common-name": "zookeeper"}, series=TLS_OPERATOR_SERIES, - revision=163, # FIXME: Unpin once the TLS is fixed on edge channel + revision=163, # FIXME: Unpin once the TLS is fixed on edge channel ), ) await ops_test.model.block_until(lambda: len(ops_test.model.applications[APP_NAME].units) == 3) From 00c32f392ecfd4bcb067a344d5874d34d3b904e0 Mon Sep 17 00:00:00 2001 From: Bikalpa Dhakal Date: Thu, 22 Aug 2024 07:33:30 +0000 Subject: [PATCH 5/5] Revert changes in tox.ini --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index ae068c5..274f5a9 100644 --- a/tox.ini +++ b/tox.ini @@ -72,7 +72,7 @@ description = Run unit tests commands = poetry install --no-root --with unit poetry run coverage run --source={[vars]src_path} \ - -m pytest -v --tb native -s {posargs} {[vars]tests_path}/unit/test_quorum.py + -m pytest -v --tb native -s {posargs} {[vars]tests_path}/unit poetry run coverage report [testenv:integration-{charm,provider,password-rotation,tls,ha,replication}]