Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 11 additions & 6 deletions lib/charms/postgresql_k8s/v0/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@

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

# Groups to distinguish database permissions
PERMISSIONS_GROUP_ADMIN = "admin"

INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles"

Expand Down Expand Up @@ -187,7 +190,7 @@ def create_database(
Identifier(database)
)
)
for user_to_grant_access in [user, "admin", *self.system_users]:
for user_to_grant_access in [user, PERMISSIONS_GROUP_ADMIN, *self.system_users]:
cursor.execute(
SQL("GRANT ALL PRIVILEGES ON DATABASE {} TO {};").format(
Identifier(database), Identifier(user_to_grant_access)
Expand Down Expand Up @@ -236,15 +239,17 @@ def create_user(
roles = privileges = None
if extra_user_roles:
extra_user_roles = tuple(extra_user_roles.lower().split(","))
admin_role = "admin" in extra_user_roles
admin_role = PERMISSIONS_GROUP_ADMIN in extra_user_roles
valid_privileges, valid_roles = self.list_valid_privileges_and_roles()
roles = [
role for role in extra_user_roles if role in valid_roles and role != "admin"
role
for role in extra_user_roles
if role in valid_roles and role != PERMISSIONS_GROUP_ADMIN
]
privileges = {
extra_user_role
for extra_user_role in extra_user_roles
if extra_user_role not in roles and extra_user_role != "admin"
if extra_user_role not in roles and extra_user_role != PERMISSIONS_GROUP_ADMIN
}
invalid_privileges = [
privilege for privilege in privileges if privilege not in valid_privileges
Expand Down Expand Up @@ -566,7 +571,7 @@ def set_up_database(self) -> None:
)
)
self.create_user(
"admin",
PERMISSIONS_GROUP_ADMIN,
extra_user_roles="pg_read_all_data,pg_write_all_data",
)
cursor.execute("GRANT CONNECT ON DATABASE postgres TO admin;")
Expand Down
3 changes: 2 additions & 1 deletion src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
from constants import (
APP_SCOPE,
BACKUP_USER,
DATABASE_DEFAULT_NAME,
METRICS_PORT,
MONITORING_PASSWORD_KEY,
MONITORING_USER,
Expand Down Expand Up @@ -416,7 +417,7 @@ def postgresql(self) -> PostgreSQL:
current_host=self.endpoint,
user=USER,
password=self.get_secret(APP_SCOPE, f"{USER}-password"),
database="postgres",
database=DATABASE_DEFAULT_NAME,
system_users=SYSTEM_USERS,
)

Expand Down
1 change: 1 addition & 0 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

"""File containing constants to be used in the charm."""

DATABASE_DEFAULT_NAME = "postgres"
DATABASE_PORT = "5432"
PEER = "database-peers"
BACKUP_USER = "backup"
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
wait_fixed,
)

from constants import DATABASE_DEFAULT_NAME

CHARM_BASE = "ubuntu@22.04"
CHARM_SERIES = "jammy"
METADATA = yaml.safe_load(Path("./metadata.yaml").read_text())
Expand Down Expand Up @@ -325,7 +327,7 @@ async def execute_query_on_unit(
unit_address: str,
password: str,
query: str,
database: str = "postgres",
database: str = DATABASE_DEFAULT_NAME,
sslmode: str | None = None,
):
"""Execute given PostgreSQL query on a unit.
Expand Down
21 changes: 14 additions & 7 deletions tests/integration/new_relations/test_new_relations_1.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
from pytest_operator.plugin import OpsTest
from tenacity import Retrying, stop_after_attempt, wait_fixed

from constants import DATABASE_DEFAULT_NAME

from ..helpers import (
CHARM_BASE,
check_database_users_existence,
Expand Down Expand Up @@ -218,7 +220,10 @@ async def test_two_applications_doesnt_share_the_same_relation_data(ops_test: Op
(another_application_app_name, f"{APPLICATION_APP_NAME.replace('-', '_')}_database"),
]:
connection_string = await build_connection_string(
ops_test, application, FIRST_DATABASE_RELATION_NAME, database="postgres"
ops_test,
application,
FIRST_DATABASE_RELATION_NAME,
database=DATABASE_DEFAULT_NAME,
)
with pytest.raises(psycopg2.Error):
psycopg2.connect(connection_string)
Expand Down Expand Up @@ -448,7 +453,7 @@ async def test_admin_role(ops_test: OpsTest):

# Check that the user can access all the databases.
for database in [
"postgres",
DATABASE_DEFAULT_NAME,
f"{APPLICATION_APP_NAME.replace('-', '_')}_database",
"another_application_database",
]:
Expand All @@ -472,11 +477,11 @@ async def test_admin_role(ops_test: OpsTest):
)
assert version == data

# Write some data (it should fail in the "postgres" database).
# Write some data (it should fail in the default database name).
random_name = (
f"test_{''.join(secrets.choice(string.ascii_lowercase) for _ in range(10))}"
)
should_fail = database == "postgres"
should_fail = database == DATABASE_DEFAULT_NAME
cursor.execute(f"CREATE TABLE {random_name}(data TEXT);")
if should_fail:
assert False, (
Expand All @@ -494,7 +499,7 @@ async def test_admin_role(ops_test: OpsTest):

# Test the creation and deletion of databases.
connection_string = await build_connection_string(
ops_test, DATA_INTEGRATOR_APP_NAME, "postgresql", database="postgres"
ops_test, DATA_INTEGRATOR_APP_NAME, "postgresql", database=DATABASE_DEFAULT_NAME
)
connection = psycopg2.connect(connection_string)
connection.autocommit = True
Expand All @@ -503,8 +508,10 @@ async def test_admin_role(ops_test: OpsTest):
cursor.execute(f"CREATE DATABASE {random_name};")
cursor.execute(f"DROP DATABASE {random_name};")
try:
cursor.execute("DROP DATABASE postgres;")
assert False, "the admin extra user role was able to drop the `postgres` system database"
cursor.execute(f"DROP DATABASE {DATABASE_DEFAULT_NAME};")
assert False, (
f"the admin extra user role was able to drop the `{DATABASE_DEFAULT_NAME}` system database"
)
except psycopg2.errors.InsufficientPrivilege:
# Ignore the error, as the admin extra user role mustn't be able to drop
# the "postgres" system database.
Expand Down
8 changes: 5 additions & 3 deletions tests/integration/new_relations/test_relations_coherence.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import pytest
from pytest_operator.plugin import OpsTest

from constants import DATABASE_DEFAULT_NAME

from ..helpers import CHARM_BASE, DATABASE_APP_NAME, build_and_deploy
from .helpers import build_connection_string
from .test_new_relations_1 import DATA_INTEGRATOR_APP_NAME
Expand Down Expand Up @@ -120,14 +122,14 @@ async def test_relations(ops_test: OpsTest, charm):

for database in [
DATA_INTEGRATOR_APP_NAME.replace("-", "_"),
"postgres",
DATABASE_DEFAULT_NAME,
]:
logger.info(f"connecting to the following database: {database}")
connection_string = await build_connection_string(
ops_test, DATA_INTEGRATOR_APP_NAME, "postgresql", database=database
)
connection = None
should_fail = database == "postgres"
should_fail = database == DATABASE_DEFAULT_NAME
try:
with psycopg2.connect(
connection_string
Expand All @@ -136,7 +138,7 @@ async def test_relations(ops_test: OpsTest, charm):
data = cursor.fetchone()
assert data[0] == "some data"

# Write some data (it should fail in the "postgres" database).
# Write some data (it should fail in the default database name).
random_name = f"test_{''.join(secrets.choice(string.ascii_lowercase) for _ in range(10))}"
cursor.execute(f"CREATE TABLE {random_name}(data TEXT);")
if should_fail:
Expand Down
22 changes: 15 additions & 7 deletions tests/unit/test_postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,22 @@
import psycopg2
import pytest
from charms.postgresql_k8s.v0.postgresql import (
PERMISSIONS_GROUP_ADMIN,
PostgreSQLCreateDatabaseError,
PostgreSQLGetLastArchivedWALError,
)
from ops.testing import Harness
from psycopg2.sql import SQL, Composed, Identifier, Literal

from charm import PostgresqlOperatorCharm
from constants import PEER
from constants import (
BACKUP_USER,
MONITORING_USER,
PEER,
REPLICATION_USER,
REWIND_USER,
USER,
)


@pytest.fixture(autouse=True)
Expand Down Expand Up @@ -75,7 +83,7 @@ def test_create_database(harness):
SQL("GRANT ALL PRIVILEGES ON DATABASE "),
Identifier(database),
SQL(" TO "),
Identifier("admin"),
Identifier(PERMISSIONS_GROUP_ADMIN),
SQL(";"),
])
),
Expand All @@ -84,7 +92,7 @@ def test_create_database(harness):
SQL("GRANT ALL PRIVILEGES ON DATABASE "),
Identifier(database),
SQL(" TO "),
Identifier("backup"),
Identifier(BACKUP_USER),
SQL(";"),
])
),
Expand All @@ -93,7 +101,7 @@ def test_create_database(harness):
SQL("GRANT ALL PRIVILEGES ON DATABASE "),
Identifier(database),
SQL(" TO "),
Identifier("replication"),
Identifier(REPLICATION_USER),
SQL(";"),
])
),
Expand All @@ -102,7 +110,7 @@ def test_create_database(harness):
SQL("GRANT ALL PRIVILEGES ON DATABASE "),
Identifier(database),
SQL(" TO "),
Identifier("rewind"),
Identifier(REWIND_USER),
SQL(";"),
])
),
Expand All @@ -111,7 +119,7 @@ def test_create_database(harness):
SQL("GRANT ALL PRIVILEGES ON DATABASE "),
Identifier(database),
SQL(" TO "),
Identifier("operator"),
Identifier(USER),
SQL(";"),
])
),
Expand All @@ -120,7 +128,7 @@ def test_create_database(harness):
SQL("GRANT ALL PRIVILEGES ON DATABASE "),
Identifier(database),
SQL(" TO "),
Identifier("monitoring"),
Identifier(MONITORING_USER),
SQL(";"),
])
),
Expand Down