Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DPE-3737] Implement ZK client interface #142

Merged
merged 28 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
418f780
refactor: Rename interface
Batalex Apr 25, 2024
d18207e
feat: Use 'database' instead of 'chroot' in relation databag
Batalex Apr 25, 2024
9cfcd94
tests: Add interface conftest
Batalex Apr 25, 2024
e0401d3
WIP: Use repo with ZK interface
Batalex Apr 25, 2024
0518b68
WIP: Add pass test
Batalex Apr 25, 2024
25172b0
feat: Use 'ca-cert' instead of 'ca' in databag
Batalex Apr 26, 2024
56a10ad
chore: Format
Batalex Apr 26, 2024
016fda9
tests: Add interface test
Batalex Apr 26, 2024
3566fcb
chore: Format
Batalex Apr 26, 2024
49ae1cd
WIP: Interface conftest fix
Batalex Apr 29, 2024
d7485c3
feat: Rename back interface to 'zookeeper'
Batalex Apr 29, 2024
5b07c9a
chore: Format
Batalex Apr 29, 2024
5ad8c0c
feat: Use extra-user-roles instead of chroot-acl
Batalex Apr 29, 2024
9d3f68d
fix: Remove leftover 'zookeeper-client'
Batalex Apr 29, 2024
71ee026
fix: Add pytest version constraint
Batalex Apr 29, 2024
2491582
fix: Fix type check lint error
Batalex Apr 29, 2024
53c803f
Revert "fix: Add pytest version constraint"
Batalex Apr 29, 2024
7092ec2
fix: Fix conditional warning for chroot-acl
Batalex May 2, 2024
3b202b3
Revert "fix: Fix type check lint error"
Batalex May 2, 2024
cb0f9f8
fix: Lock kazoo version
Batalex May 2, 2024
d97d0e0
fix: Remove poetry plugin from project env
Batalex May 2, 2024
bb95f51
fix: Fix type check lint error
Batalex May 2, 2024
7814bc1
tests: Remove interface testing
Batalex May 2, 2024
07e5946
chore: Remove ruff warnings
Batalex May 3, 2024
b7d4df0
feat: Rename back endpoint to zookeeper
Batalex May 3, 2024
7e48a95
feat: Use deprecated function decorator
Batalex May 3, 2024
c990f81
fix: Fix decorator order
Batalex May 3, 2024
3a82fb8
chore: Cleanup a few warnings
Batalex May 3, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ peers:
interface: upgrade

provides:
zookeeper:
database:
Batalex marked this conversation as resolved.
Show resolved Hide resolved
interface: zookeeper
cos-agent:
interface: cos_agent
Expand Down
1,589 changes: 425 additions & 1,164 deletions poetry.lock

Large diffs are not rendered by default.

13 changes: 6 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,13 @@ authors = []
[tool.poetry.dependencies]
python = "^3.10"
ops = "^2.4.1"
kazoo = ">=2.8.0"
kazoo = ">=2.8.0,<2.10"
tenacity = ">=8.0.1"
pure-sasl = ">=0.6.2"
cryptography = ">42.0.0"
cosl = ">=0.0.5"
pydantic = "^1.10.7"
pyyaml = "^6.0.1"
poetry-plugin-export = ">=1.6"

[tool.poetry.group.fmt]
optional = true
Expand Down Expand Up @@ -79,8 +78,8 @@ pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workf

[tool.ruff]
line-length = 99
select = ["E", "W", "F", "C", "N", "D", "I001"]
extend-ignore = [
lint.select = ["E", "W", "F", "C", "N", "D", "I001"]
lint.extend-ignore = [
"D203",
"D204",
"D213",
Expand All @@ -94,13 +93,13 @@ extend-ignore = [
"D409",
"D413",
]
ignore = ["E501", "D107"]
lint.ignore = ["E501", "D107"]
extend-exclude = ["__pycache__", "*.egg_info", "tests/integration/app-charm/lib"]
per-file-ignores = {"tests/*" = ["D100","D101","D102","D103","D104", "E999"], "src/literals.py" = ["D101"]}
lint.per-file-ignores = {"tests/*" = ["D100","D101","D102","D103","D104", "E999"], "src/literals.py" = ["D101"]}
target-version="py310"
src = ["src", "tests"]

[tool.ruff.mccabe]
[tool.ruff.lint.mccabe]
max-complexity = 10

[tool.pyright]
Expand Down
55 changes: 7 additions & 48 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,54 +1,13 @@
build==1.0.3 ; python_version >= "3.10" and python_version < "4.0"
cachecontrol[filecache]==0.13.1 ; python_version >= "3.10" and python_version < "4.0"
certifi==2023.11.17 ; python_version >= "3.10" and python_version < "4.0"
cffi==1.16.0 ; python_version >= "3.10" and python_version < "4.0" and (platform_python_implementation != "PyPy" or sys_platform == "darwin")
charset-normalizer==3.3.2 ; python_version >= "3.10" and python_version < "4.0"
cleo==2.1.0 ; python_version >= "3.10" and python_version < "4.0"
colorama==0.4.6 ; python_version >= "3.10" and python_version < "4.0" and os_name == "nt"
cosl==0.0.7 ; python_version >= "3.10" and python_version < "4.0"
crashtest==0.4.1 ; python_version >= "3.10" and python_version < "4.0"
cffi==1.16.0 ; python_version >= "3.10" and python_version < "4.0" and platform_python_implementation != "PyPy"
cosl==0.0.11 ; python_version >= "3.10" and python_version < "4.0"
cryptography==42.0.5 ; python_version >= "3.10" and python_version < "4.0"
distlib==0.3.8 ; python_version >= "3.10" and python_version < "4.0"
dulwich==0.21.7 ; python_version >= "3.10" and python_version < "4.0"
fastjsonschema==2.19.1 ; python_version >= "3.10" and python_version < "4.0"
filelock==3.13.1 ; python_version >= "3.10" and python_version < "4.0"
idna==3.6 ; python_version >= "3.10" and python_version < "4.0"
importlib-metadata==7.0.1 ; python_version >= "3.10" and python_version < "3.12"
installer==0.7.0 ; python_version >= "3.10" and python_version < "4.0"
jaraco-classes==3.3.0 ; python_version >= "3.10" and python_version < "4.0"
jeepney==0.8.0 ; python_version >= "3.10" and python_version < "4.0" and sys_platform == "linux"
kazoo==2.9.0 ; python_version >= "3.10" and python_version < "4.0"
keyring==24.3.0 ; python_version >= "3.10" and python_version < "4.0"
more-itertools==10.2.0 ; python_version >= "3.10" and python_version < "4.0"
msgpack==1.0.7 ; python_version >= "3.10" and python_version < "4.0"
ops==2.9.0 ; python_version >= "3.10" and python_version < "4.0"
packaging==23.2 ; python_version >= "3.10" and python_version < "4.0"
pexpect==4.9.0 ; python_version >= "3.10" and python_version < "4.0"
pkginfo==1.9.6 ; python_version >= "3.10" and python_version < "4.0"
platformdirs==3.11.0 ; python_version >= "3.10" and python_version < "4.0"
poetry-core==1.8.1 ; python_version >= "3.10" and python_version < "4.0"
poetry-plugin-export==1.6.0 ; python_version >= "3.10" and python_version < "4.0"
poetry==1.7.1 ; python_version >= "3.10" and python_version < "4.0"
ptyprocess==0.7.0 ; python_version >= "3.10" and python_version < "4.0"
ops==2.13.0 ; python_version >= "3.10" and python_version < "4.0"
pure-sasl==0.6.2 ; python_version >= "3.10" and python_version < "4.0"
pycparser==2.21 ; python_version >= "3.10" and python_version < "4.0" and (platform_python_implementation != "PyPy" or sys_platform == "darwin")
pydantic==1.10.13 ; python_version >= "3.10" and python_version < "4.0"
pyproject-hooks==1.0.0 ; python_version >= "3.10" and python_version < "4.0"
pywin32-ctypes==0.2.2 ; python_version >= "3.10" and python_version < "4.0" and sys_platform == "win32"
pycparser==2.22 ; python_version >= "3.10" and python_version < "4.0" and platform_python_implementation != "PyPy"
pydantic==1.10.15 ; python_version >= "3.10" and python_version < "4.0"
pyyaml==6.0.1 ; python_version >= "3.10" and python_version < "4.0"
rapidfuzz==3.6.1 ; python_version >= "3.10" and python_version < "4.0"
requests-toolbelt==1.0.0 ; python_version >= "3.10" and python_version < "4.0"
requests==2.31.0 ; python_version >= "3.10" and python_version < "4.0"
secretstorage==3.3.3 ; python_version >= "3.10" and python_version < "4.0" and sys_platform == "linux"
shellingham==1.5.4 ; python_version >= "3.10" and python_version < "4.0"
six==1.16.0 ; python_version >= "3.10" and python_version < "4.0"
tenacity==8.2.3 ; python_version >= "3.10" and python_version < "4.0"
tomli==2.0.1 ; python_version >= "3.10" and python_version < "3.11"
tomlkit==0.12.3 ; python_version >= "3.10" and python_version < "4.0"
trove-classifiers==2024.1.8 ; python_version >= "3.10" and python_version < "4.0"
typing-extensions==4.9.0 ; python_version >= "3.10" and python_version < "4.0"
urllib3==2.1.0 ; python_version >= "3.10" and python_version < "4.0"
virtualenv==20.25.0 ; python_version >= "3.10" and python_version < "4.0"
websocket-client==1.7.0 ; python_version >= "3.10" and python_version < "4.0"
xattr==0.10.1 ; python_version >= "3.10" and python_version < "4.0" and sys_platform == "darwin"
zipp==3.17.0 ; python_version >= "3.10" and python_version < "3.12"
typing-extensions==4.11.0 ; python_version >= "3.10" and python_version < "4.0"
websocket-client==1.8.0 ; python_version >= "3.10" and python_version < "4.0"
11 changes: 7 additions & 4 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def __init__(self, *args):
self,
substrate=SUBSTRATE,
dependency_model=ZooKeeperDependencyModel(
**DEPENDENCIES # pyright: ignore[reportGeneralTypeIssues]
**DEPENDENCIES # pyright: ignore[reportArgumentType]
),
)

Expand Down Expand Up @@ -318,7 +318,7 @@ def init_server(self):
if (
self.state.cluster.tls
and self.state.unit_server.certificate
and self.state.unit_server.ca
and self.state.unit_server.ca_cert
): # TLS is probably completed
self.tls_manager.set_private_key()
self.tls_manager.set_ca()
Expand Down Expand Up @@ -422,12 +422,15 @@ def update_client_data(self) -> None:

client.update(
{
"uris": client.uris,
"endpoints": client.endpoints,
"tls": client.tls,
"username": client.username,
"password": client.password,
"chroot": client.chroot,
"database": client.database,
# Duplicated for compatibility with older requirers
# TODO (zkclient): Remove these entries
"chroot": client.database,
"uris": client.uris,
}
)

Expand Down
63 changes: 60 additions & 3 deletions src/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

"""Collection of state objects for the ZooKeeper relations, apps and units."""
import logging
import warnings
from collections.abc import MutableMapping
from typing import Literal

Expand Down Expand Up @@ -104,7 +105,12 @@ def endpoints(self) -> str:
@property
def uris(self) -> str:
"""The ZooKeeper connection uris for the client application to connect with."""
return self._uris + self.chroot if self._uris else ""
# TODO (zkclient): Remove this property
warnings.warn(
Batalex marked this conversation as resolved.
Show resolved Hide resolved
"Using 'uris' in the databag is deprecated, use 'endpoints' instead",
DeprecationWarning,
)
return self._uris + self.database if self._uris else ""

@property
def tls(self) -> str:
Expand All @@ -126,12 +132,51 @@ def chroot_acl(self) -> str:
- 'w' - write
- 'a' - append
"""
return self.relation_data.get("chroot-acl", "cdrwa")
# TODO (zkclient): Remove this property and replace by "cdrwa" in self.extra_user_roles
acl = self.relation_data.get("chroot-acl")
if acl is not None:
warnings.warn(
"Using 'chroot-acl' in the databag is deprecated, use 'extra-user-roles' instead",
DeprecationWarning,
)

else:
acl = "cdrwa"

return acl

@property
def extra_user_roles(self) -> str:
"""The client defined ACLs for their requested ACL.

Contains:
- 'c' - create
- 'd' - delete
- 'r' - read
- 'w' - write
- 'a' - append
"""
return self.relation_data.get("extra-user-roles", self.chroot_acl)

@property
def chroot(self) -> str:
"""The client requested root zNode path value."""
# TODO (zkclient): Remove this property and replace by "" in self.database
chroot = self.relation_data.get("chroot", "")
if chroot:
warnings.warn(
"Using 'chroot' in the databag is deprecated, use 'database' instead",
DeprecationWarning,
)
if chroot and not chroot.startswith("/"):
chroot = f"/{chroot}"

return chroot

@property
def database(self) -> str:
Batalex marked this conversation as resolved.
Show resolved Hide resolved
"""The client requested root zNode path value."""
chroot = self.relation_data.get("database", self.chroot)
if chroot and not chroot.startswith("/"):
chroot = f"/{chroot}"

Expand Down Expand Up @@ -366,7 +411,19 @@ def certificate(self) -> str:
def ca(self) -> str:
"""The root CA contents for the unit to use for TLS."""
# Backwards compatibility
return self.relation_data.get("ca-cert", self.relation_data.get("ca", ""))
# TODO (zkclient): Remove this property and replace by "" in self.ca_cert
ca = self.relation_data.get("ca", "")
if ca:
warnings.warn(
"Using 'ca' in the databag is deprecated, use 'ca_cert' instead",
DeprecationWarning,
)
return ca

@property
def ca_cert(self) -> str:
"""The root CA contents for the unit to use for TLS."""
return self.relation_data.get("ca-cert", self.ca)

@property
def sans(self) -> dict[str, list[str]]:
Expand Down
2 changes: 1 addition & 1 deletion src/events/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def _on_client_relation_updated(self, event: RelationEvent) -> None:
# FIXME: should be data-platform-libs
# setting password for new users to relation data
for client in self.charm.state.clients:
if not client.chroot:
if not client.database:
continue

if (
Expand Down
2 changes: 1 addition & 1 deletion src/literals.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
CHARM_KEY = "zookeeper"

PEER = "cluster"
REL_NAME = "zookeeper"
REL_NAME = "database"
CONTAINER = "zookeeper"
CHARM_USERS = ["super", "sync"]
CERTS_REL_NAME = "certificates"
Expand Down
26 changes: 13 additions & 13 deletions src/managers/quorum.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,37 +195,37 @@ def update_acls(self, event: RelationEvent | None = None) -> None:
requested_chroots = set()

for client in self.state.clients:
if not client.chroot:
if not client.database:
continue

generated_acl = make_acl(
scheme="sasl",
credential=client.username,
read="r" in client.chroot_acl,
write="w" in client.chroot_acl,
create="c" in client.chroot_acl,
delete="d" in client.chroot_acl,
admin="a" in client.chroot_acl,
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,
)
logger.info(f"{generated_acl=}")

requested_acls.add(generated_acl)

# FIXME: data-platform-libs should handle this when it's implemented
if client.chroot:
if client.database:
if event and client.relation and client.relation.id == event.relation.id:
continue # skip broken chroots, so they're removed
else:
requested_chroots.add(client.chroot)
requested_chroots.add(client.database)

# Looks for newly related applications not in config yet
if client.chroot not in leader_chroots:
logger.info(f"CREATE CHROOT - {client.chroot}")
self.client.create_znode_leader(path=client.chroot, acls=[generated_acl])
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])

# Looks for existing related applications
logger.info(f"UPDATE CHROOT - {client.chroot}")
self.client.set_acls_znode_leader(path=client.chroot, acls=[generated_acl])
logger.info(f"UPDATE CHROOT - {client.database}")
self.client.set_acls_znode_leader(path=client.database, acls=[generated_acl])

# Looks for applications no longer in the relation but still in config
for chroot in sorted(leader_chroots - requested_chroots, reverse=True):
Expand Down
4 changes: 2 additions & 2 deletions src/managers/tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def set_private_key(self) -> None:

def set_ca(self) -> None:
"""Sets the unit CA."""
if not self.state.unit_server.ca:
if not self.state.unit_server.ca_cert:
logger.error("Can't set CA to unit, missing CA in relation data")
return

self.workload.write(content=self.state.unit_server.ca, path=self.workload.paths.ca)
self.workload.write(content=self.state.unit_server.ca_cert, path=self.workload.paths.ca)

def set_certificate(self) -> None:
"""Sets the unit certificate."""
Expand Down
Loading
Loading