[DPE-3737] Add a zookeeper_client interface

[Batalex]

This PR adds a zookeeper interface.

Notes:

• The interface is zookeeper instead of zookeeper_client to keep compatibility with existing charms
• Tests are not implemented. I wrote a recap of stuff that don't work out in the following PR: [DPE-3737] Implement ZK client interface zookeeper-operator#142

[marcoppenheimer]

Left some nitpicks and a couple suggestions! I haven't had a chance to go through with a fine-comb yet, but will do when the PR is open.

Overall it looks good! I haven't seen the Pydantic use for client-relation-interfaces yet, and I'm less familiar with what actually is expected for secret-relations, so I'll defer my opinions to those more in the know.

Oh, and I suggest you just open a branch in the repo, rather than using forks. Makes it easier whenever other people from the team want to make changes to a PR branch!

[marcoppenheimer]

todo: Likewise, adding kafka and kafka-k8s here.
@deusebio - Are there any charms 'close' to using ZK that would be relevant here?

[deusebio]

kyuubi most likely once we will attack HA, but right now we are just charming a first draft (with no HA). Should we be exhaustive when listing things here? Meaning, should we keep this list updates as new charm implement the relation from the requirer side? @gruyaume

[marcoppenheimer]

question: Right now, we pass a fluffy extra field tls=""|"enabled". Am I correct in assuming that once we implement this, we should be doing something like this?

if not tls-ca:
    do_non_tls_stuff()

[Batalex]

We are not compelled to do so because the extra field can still be a part of the secret. We could do it, though

[deusebio]

I would keep UX consistent with other charms. If the TLS is enabled condition can be checked with the existence of tls-ca on the secrets, I wsuppose we are good

[marcoppenheimer]

suggestion: Perhaps here it might be OK to loosen the typing from URI to just be string.

Then, we can add a validator in the class itself, something like:

from pydantic import validator
from ipaddress

class ZookeeperProviderAppData(BaseModel):
    endpoints: string = Field(
        description="A comma-seperated list of ZooKeeper server uris, and zNode",
        examples=["10.141.78.133:2181,10.141.78.50:2181,10.141.78.45:2181/myappB"],
        title="ZooKeeper URIs",
    )

	@validator("endpoints")
    @classmethod
    def is_comma_list_of_ips(cls, value: str) -> str | None:
        try:
            ips = [address.split(":") for address in value.split(",")]
            valid_ips = [ipaddress.ip_address(ip) for ip in ips]
        except Exception as e:
            raise ValueError(
                f"Value not a valid IP address: {e}"
            )
	
    @validator("endpoints")
    @classmethod
    def ports_are_the_same(cls, value: str) -> str | None:
        try:
            non_chroot = value.split("/")[0]
            ports = {int(address.split(":")[1]) for address in non_chroot.split(",")}
            if not len(ports) == 1:
                raise ValueError(
                    "Value does not contain the same port for every address"
                )
        except Exception as e:
            raise ValueError(
                f"Value does not contain valid ports: {e}"
            )

[Batalex]

Using Annotated, this is equivalent, but I might have missed your point

[Batalex]

Thanks for the review!

Oh, and I suggest you just open a branch in the repo, rather than using forks. Makes it easier whenever other people from the team want to make changes to a PR branch!

Unfortunately I don't have permission :(

[gruyaume]

ingress? Is this a mistake?

[Batalex]

This comes from the template for new interfaces, tests are not implemented yet