Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
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
53 changes: 49 additions & 4 deletions lib/charms/data_platform_libs/v1/data_interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ def _on_resource_requested(self, event: ResourceRequestedEvent) -> None:

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 3
LIBPATCH = 4

PYDEPS = ["ops>=2.0.0", "pydantic>=2.11"]

Expand Down Expand Up @@ -496,6 +496,9 @@ def store_new_data(
MtlsSecretStr = Annotated[OptionalSecretStr, Field(exclude=True, default=None), "mtls"]
ExtraSecretStr = Annotated[OptionalSecretStr, Field(exclude=True, default=None), "extra"]
EntitySecretStr = Annotated[OptionalSecretStr, Field(exclude=True, default=None), "entity"]
RequestedEntitySecretStr = Annotated[
OptionalSecretStr, Field(exclude=True, default=None), "requested-entity"
]


class Scope(Enum):
Expand Down Expand Up @@ -833,9 +836,16 @@ def extract_secrets(self, info: ValidationInfo):
if not secret_uri:
continue

secret = repository.get_secret(
secret_group, secret_uri=secret_uri, short_uuid=short_uuid
)
try:
secret = repository.get_secret(
secret_group, secret_uri=secret_uri, short_uuid=short_uuid
)
except SecretNotFoundError:
# v0 deletes the requested entity secret
if secret_group == "requested-entity":
logger.debug("Missing requested entity secret")
continue
raise
Comment on lines +839 to +848
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

v0 will delete the helper secret once the relation was established.


if not secret:
logger.info(f"No secret for group {secret_group} and short uuid {short_uuid}")
Expand Down Expand Up @@ -1002,6 +1012,13 @@ class RequirerCommonModel(CommonModel):
entity_permissions: list[EntityPermissionModel] | None = Field(default=None)
secret_mtls: SecretString | None = Field(default=None)
mtls_cert: MtlsSecretStr = Field(default=None)
secret_requested_entity: SecretString | None = Field(
default=None,
validation_alias=AliasChoices("requested-entity-secret", "secret-requested-entity"),
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should match both the old and new secret field.

)
entity_name: RequestedEntitySecretStr = Field(default=None)
entity_password: RequestedEntitySecretStr = Field(default=None, serialization_alias="password")
prefix_matching: Literal["all", "only-existing"] | None = Field(default=None)

@model_validator(mode="after")
def validate_fields(self):
Expand All @@ -1015,6 +1032,9 @@ def validate_fields(self):
if self.entity_type == "GROUP" and self.extra_user_roles:
raise ValueError("Inconsistent entity information. Use extra_group_roles instead")

if self.entity_password and not self.entity_name:
raise ValueError("Unable to set entity password without an entity name")

return self


Expand Down Expand Up @@ -1047,6 +1067,7 @@ class ResourceProviderModel(ProviderCommonModel):
entity_name: EntitySecretStr = Field(default=None)
entity_password: EntitySecretStr = Field(default=None)
version: str | None = Field(default=None)
prefix_resources: str | None = Field(default=None)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to use PlainSerializer to sort the CSV list, but it seems that the field_info.annotation in OptionalSecrets traps other annotations as well. Leaving it be for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting.
Maybe we can add just a field serializer for this one in this class ? Or that prevents it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I tried to do was:

prefix_resources: Annotated[str | None, PlainSerializer(csv_list_sorter)] = Field(default=None)

With the serialiser being:

def csv_list_sorter(el: str | None) -> str | None:
"""Serialization sorter for comma separated lists."""
if el:
return ",".join(sorted(el.split(",")))
return None

But then the field got turned into a secret. I guess that the OptionalSecretStr alias traps this, since they are both str | None, but didn't dig further, since autosorting is nice to have, but not strictly necessary.



class RequirerDataContractV0(RequirerCommonModel):
Expand Down Expand Up @@ -2090,6 +2111,12 @@ class AuthenticationUpdatedEvent(ResourceRequirerEvent[TResourceProviderModel]):
pass


class ResourcePrefixResourcesChangedEvent(ResourceRequirerEvent[TResourceProviderModel]):
"""Prefix resources have changed."""

pass


# Error Propagation Events


Expand Down Expand Up @@ -2148,6 +2175,7 @@ class ResourceRequiresEvents(CharmEvents, Generic[TResourceProviderModel]):
authentication_updated = EventSource(AuthenticationUpdatedEvent)
status_raised = EventSource(StatusRaisedEvent)
status_resolved = EventSource(StatusResolvedEvent)
prefix_resources_changed = EventSource(ResourcePrefixResourcesChangedEvent)


##############################################################################
Expand Down Expand Up @@ -2632,6 +2660,11 @@ def set_response(self, relation_id: int, response: ResourceProviderModel):
self.interface.write_model(
relation_id, response, context={"version": "v0"}
) # {"database": "database-name", "secret-user": "uri", ...}
# Set expected prefix field if present
if response.prefix_resources:
self.interface.repository(relation_id).write_field(
"prefix-databases", response.prefix_resources
)
Comment on lines +2664 to +2667
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a better way to inject this field?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is because the field is renamed ?
Then probably we can do like we do for the other resources: here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the original_field is still written in the provider response with write_field here:

old_name = request_model.original_field
request_model.request_id = None # For safety, let's ensure that we don't have a model.
self._handle_event(event, repository, request_model)
logger.info(
f"Patching databag for v0 compatibility: replacing 'resource' by '{old_name}'"
)
self.interface.repository(
event.relation.id,
).write_field(old_name, request_model.resource)

return

model = self.interface.build_model(relation_id, DataContractV1[response.__class__])
Expand Down Expand Up @@ -2875,6 +2908,10 @@ def __init__(
f"{relation_alias}_read_only_endpoints_changed",
ResourceReadOnlyEndpointsChangedEvent,
)
self.on.define_event(
f"{relation_alias}_prefix_resources_changed",
ResourcePrefixResourcesChangedEvent,
)

##############################################################################
# Extra useful functions
Expand Down Expand Up @@ -3241,3 +3278,11 @@ def _handle_event(
)
self._emit_aliased_event(event, "authentication_updated", response)
return

if "prefix-resources" in _diff.added or "prefix-resources" in _diff.changed:
logger.info(f"prefix resources updated for {response.resource} at {datetime.now()}")
getattr(self.on, "prefix_resources_changed").emit(
event.relation, app=event.app, unit=event.unit, response=response
)
self._emit_aliased_event(event, "prefix_resources_changed", response)
return
2 changes: 1 addition & 1 deletion tests/v0/integration/database-charm/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ def _on_database_pebble_ready(self, event: WorkloadEvent) -> None:
"command": "/usr/local/bin/docker-entrypoint.sh postgres",
"startup": "enabled",
"environment": {
"PGDATA": "/var/lib/postgresql/data/pgdata",
"PGDATA": "/var/lib/postgresql/data/pgdata/data",
"POSTGRES_PASSWORD": self._stored.password,
},
}
Expand Down
6 changes: 5 additions & 1 deletion tests/v1/integration/application-charm/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ requires:
interface: database_client
first-database-roles:
interface: database_client
first-database-username:
interface: database_client
second-database-db:
interface: database_client
multiple-database-clusters:
Expand All @@ -36,4 +38,6 @@ requires:
etcd-client:
interface: etcd_client
certificates:
interface: tls-certificates
interface: tls-certificates
database-with-prefix:
interface: database_client
25 changes: 25 additions & 0 deletions tests/v1/integration/application-charm/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,20 @@ def __init__(self, *args):
self._on_first_database_auth_updated,
)

self.first_database_username = ResourceRequirerEventHandler(
self,
"first-database-username",
requests=[
RequirerCommonModel(
resource=database_name, entity_type="USER", entity_name="testuser"
)
],
response_model=ExtendedResponseModel,
)
self.framework.observe(
self.first_database.on.resource_created, self._on_first_database_created
)

# Events related to the second database that is requested
# (these events are defined in the database requires charm library).
database_name = f"{self.app.name.replace('-', '_')}_second_database_db"
Expand Down Expand Up @@ -163,6 +177,17 @@ def __init__(self, *args):
self._on_cluster_endpoints_changed,
)

self.database_prefixes = ResourceRequirerEventHandler(
charm=self,
relation_name="database-with-prefix",
requests=[
RequirerCommonModel(
resource="testdb*", extra_user_roles=EXTRA_USER_ROLES, prefix_matching="all"
)
],
response_model=ExtendedResponseModel,
)

# Multiple database clusters charm events (defined dynamically
# in the database requires charm library, using the provided cluster/relation aliases).
database_name = f"{self.app.name.replace('-', '_')}_aliased_multiple_database_clusters"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@ summary: |
requires:
backward-database:
interface: database_client
backward-prefix-database:
interface: database_client
10 changes: 10 additions & 0 deletions tests/v1/integration/backward-compatibility-charm/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ def __init__(self, *args):
self.database = DatabaseRequires(self, "backward-database", "bwclient")
self.framework.observe(self.database.on.database_created, self._on_resource_created)

self.database_prefixes = DatabaseRequires(
charm=self,
relation_name="backward-prefix-database",
database_name="testdb*",
requested_entity_name="testuser",
)
self.framework.observe(
self.database_prefixes.on.database_created, self._on_resource_created
)

def _on_start(self, _) -> None:
"""Only sets an active status."""
self.unit.status = ActiveStatus("Backward compatibility charm ready!")
Expand Down
33 changes: 22 additions & 11 deletions tests/v1/integration/database-charm/src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ def _on_database_pebble_ready(self, event: WorkloadEvent) -> None:
"command": "/usr/local/bin/docker-entrypoint.sh postgres",
"startup": "enabled",
"environment": {
"PGDATA": "/var/lib/postgresql/data/pgdata",
"PGDATA": "/var/lib/postgresql/data/pgdata/data",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Canonical k8s compatibility (non-empty volume is provided).

"POSTGRES_PASSWORD": self._stored.password,
},
}
Expand All @@ -228,23 +228,30 @@ def _on_resource_requested(self, event: ResourceRequestedEvent) -> None:

resource = request.resource
extra_user_roles = request.extra_user_roles
username = request.entity_name
password = request.entity_password

username = f"relation_{relation_id}_{request.request_id}"
password = self._new_password()
if resource[-1] == "*":
resources = [f"{resource[:-1]}1", f"{resource[:-1]}2"]
else:
resources = [resource]
username = username or f"relation_{relation_id}_{request.request_id}"
password = password or self._new_password()
connection_string = (
"dbname='postgres' user='postgres' host='localhost' "
f"password='{self._stored.password}' connect_timeout=10"
)
connection = psycopg2.connect(connection_string)
connection.autocommit = True
cursor = connection.cursor()
# Create the database, user and password. Also gives the user access to the database.
cursor.execute(f"CREATE DATABASE {resource};")
cursor.execute(f"CREATE USER {username} WITH ENCRYPTED PASSWORD '{password}';")
cursor.execute(f"GRANT ALL PRIVILEGES ON DATABASE {resource} TO {username};")
# Add the roles to the user.
if extra_user_roles:
cursor.execute(f"ALTER USER {username} {extra_user_roles};")
# Create the database, user and password. Also gives the user access to the database.
for resource in resources:
cursor.execute(f"CREATE DATABASE {resource};")
cursor.execute(f"GRANT ALL PRIVILEGES ON DATABASE {resource} TO {username};")
# Add the roles to the user.
if extra_user_roles:
cursor.execute(f"ALTER USER {username} {extra_user_roles};")
# Get the database version.
cursor.execute("SELECT version();")
version = cursor.fetchone()[0]
Expand Down Expand Up @@ -272,6 +279,7 @@ def _on_resource_requested(self, event: ResourceRequestedEvent) -> None:
username=username,
endpoints=f"{self.model.get_binding('database').network.bind_address}:5432",
version=version,
prefix_resources=",".join(resources) if len(resources) > 1 else None,
)
self.database.set_response(event.relation.id, response)
self.unit.status = ActiveStatus()
Expand All @@ -294,8 +302,11 @@ def _on_resource_entity_requested(self, event: ResourceEntityRequestedEvent) ->
entity_type = request.entity_type

# Generate a entity-name and a entity-password for the application.
rolename = self._new_rolename()
password = self._new_password()
rolename = request.entity_name
password = request.entity_password

rolename = rolename or self._new_rolename()
password = password or self._new_password()

# Connect to the database.
connection_string = (
Expand Down
36 changes: 36 additions & 0 deletions tests/v1/integration/test_backward_compatibility_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,39 @@ async def test_backward_relation_with_charm_libraries_secrets(ops_test: OpsTest)
assert len(password) == 16
assert endpoints
assert database == "bwclient"


@pytest.mark.abort_on_fail
async def test_backward_relation_with_prefix_and_username(ops_test: OpsTest):
prefix_relation = "backward-prefix-database"
# Relate the charms and wait for them exchanging some connection data.
await ops_test.model.add_relation(
DATABASE_APP_NAME, f"{APPLICATION_APP_NAME}:{prefix_relation}"
)
await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active")

# check unit message to check if the topic_created_event is triggered
for unit in ops_test.model.applications[APPLICATION_APP_NAME].units:
assert unit.workload_status_message == "backward_database_created"

# Get the requests
secret_uri = (
await get_application_relation_data(
ops_test, APPLICATION_APP_NAME, prefix_relation, f"{PROV_SECRET_PREFIX}user"
)
or ""
)
secret_data = await get_juju_secret(ops_test, secret_uri)
username = secret_data["username"]
password = secret_data["password"]
endpoints = await get_application_relation_data(
ops_test, APPLICATION_APP_NAME, prefix_relation, "endpoints"
)
prefix_databases = await get_application_relation_data(
ops_test, APPLICATION_APP_NAME, prefix_relation, "prefix-databases"
)

assert username == "testuser"
assert len(password) == 16
assert endpoints
assert prefix_databases == "testdb1,testdb2"
56 changes: 56 additions & 0 deletions tests/v1/integration/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@
DB_FIRST_DATABASE_RELATION_NAME = "first-database-db"
DB_SECOND_DATABASE_RELATION_NAME = "second-database-db"
ROLES_FIRST_DATABASE_RELATION_NAME = "first-database-roles"
USERNAME_FIRST_DATABASE_RELATION_NAME = "first-database-username"
PREFIX_DATABASE_RELATION_NAME = "database-with-prefix"

MULTIPLE_DATABASE_CLUSTERS_RELATION_NAME = "multiple-database-clusters"
ALIASED_MULTIPLE_DATABASE_CLUSTERS_RELATION_NAME = "aliased-multiple-database-clusters"
Expand Down Expand Up @@ -765,6 +767,33 @@ async def test_database_roles_relation_with_charm_libraries_secrets(ops_test: Op
assert entity_pass is not None


@pytest.mark.abort_on_fail
@pytest.mark.usefixtures("only_with_juju_secrets")
async def test_database_username(ops_test: OpsTest):
"""Test basic functionality of database-roles relation interface."""
# Relate the charms and wait for them exchanging some connection data.

pytest.first_database_relation = await ops_test.model.add_relation(
f"{APPLICATION_APP_NAME}:{USERNAME_FIRST_DATABASE_RELATION_NAME}", DATABASE_APP_NAME
)
await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active")

requests = json.loads(
await get_application_relation_data(
ops_test, APPLICATION_APP_NAME, USERNAME_FIRST_DATABASE_RELATION_NAME, "requests"
)
or "[]"
)
request = requests[0]
secret_uri = request.get(f"{SECRET_REF_PREFIX}entity")
secret_content = await get_juju_secret(ops_test, secret_uri)
entity_name = secret_content["entity-name"]
entity_pass = secret_content["entity-password"]

assert entity_name == "testuser"
assert entity_pass is not None


async def test_an_application_can_connect_to_multiple_database_clusters(
ops_test: OpsTest, database_charm
):
Expand Down Expand Up @@ -1370,3 +1399,30 @@ async def test_scaling_requires_can_access_shared_secrets(ops_test):
await action.wait()
new_password = action.results.get("value")
assert new_password == orig_password


@pytest.mark.abort_on_fail
async def test_database_prefix(ops_test: OpsTest):
"""Test basic functionality of database-roles relation interface."""
# Relate the charms and wait for them exchanging some connection data.

pytest.first_database_relation = await ops_test.model.add_relation(
f"{APPLICATION_APP_NAME}:{PREFIX_DATABASE_RELATION_NAME}", DATABASE_APP_NAME
)
await ops_test.model.wait_for_idle(apps=APP_NAMES, status="active")

requests = json.loads(
await get_application_relation_data(
ops_test, APPLICATION_APP_NAME, PREFIX_DATABASE_RELATION_NAME, "requests"
)
or "[]"
)
request = requests[0]

assert request["prefix-resources"] == "testdb1,testdb2"
data = json.loads(
await get_application_relation_data(
ops_test, APPLICATION_APP_NAME, PREFIX_DATABASE_RELATION_NAME, "data"
)
)
assert data[request["request-id"]]["prefix-matching"] == "all"
Loading
Loading