Skip to content

Commit

Permalink
Change charm database user (#21)
Browse files Browse the repository at this point in the history
* Change charm database user

* Fix unit tests

* Fix integration test call

* Fix user name in library

* Fix user

* Add default postgres user creation

* Change action name
  • Loading branch information
marceloneppel authored Aug 29, 2022
1 parent c6b438d commit 79286df
Show file tree
Hide file tree
Showing 10 changed files with 90 additions and 61 deletions.
5 changes: 3 additions & 2 deletions actions.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@

get-primary:
description: Get the unit which is the primary/leader in the replication.
get-initial-password:
description: Get the initial postgres user password for the database.
get-password:
description: Get the operator user password used by charm.
It is internal charm user, SHOULD NOT be used by applications.
4 changes: 3 additions & 1 deletion lib/charms/postgresql_k8s/v0/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ def delete_user(self, user: str) -> None:
with self._connect_to_database(
database
) as connection, connection.cursor() as cursor:
cursor.execute(sql.SQL("REASSIGN OWNED BY {} TO postgres;").format(sql.Identifier(user)))
cursor.execute(sql.SQL("REASSIGN OWNED BY {} TO {};").format(
sql.Identifier(user), sql.Identifier(self.user)
))
cursor.execute(sql.SQL("DROP OWNED BY {};").format(sql.Identifier(user)))

# Delete the user.
Expand Down
40 changes: 24 additions & 16 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from typing import List, Optional, Set

from charms.operator_libs_linux.v0 import apt
from charms.postgresql_k8s.v0.postgresql import PostgreSQL
from charms.postgresql_k8s.v0.postgresql import PostgreSQL, PostgreSQLCreateUserError
from ops.charm import (
ActionEvent,
CharmBase,
Expand All @@ -37,7 +37,7 @@
RemoveRaftMemberFailedError,
SwitchoverFailedError,
)
from constants import PEER
from constants import PEER, USER
from relations.db import DbProvides
from relations.postgresql_provider import PostgreSQLProvider
from utils import new_password
Expand All @@ -63,7 +63,7 @@ def __init__(self, *args):
self.framework.observe(self.on[PEER].relation_departed, self._on_peer_relation_departed)
self.framework.observe(self.on.pgdata_storage_detaching, self._on_pgdata_storage_detaching)
self.framework.observe(self.on.start, self._on_start)
self.framework.observe(self.on.get_initial_password_action, self._on_get_initial_password)
self.framework.observe(self.on.get_password_action, self._on_get_password)
self.framework.observe(self.on.update_status, self._on_update_status)
self._cluster_name = self.app.name
self._member_name = self.unit.name.replace("/", "-")
Expand All @@ -78,8 +78,8 @@ def postgresql(self) -> PostgreSQL:
"""Returns an instance of the object used to interact with the database."""
return PostgreSQL(
host=self.primary_endpoint,
user="postgres",
password=self._get_postgres_password(),
user=USER,
password=self._get_password(),
database="postgres",
)

Expand Down Expand Up @@ -335,7 +335,7 @@ def _patroni(self) -> Patroni:
self._member_name,
self.app.planned_units(),
self._peer_members_ips,
self._get_postgres_password(),
self._get_password(),
self._replication_password,
)

Expand Down Expand Up @@ -470,7 +470,7 @@ def _on_leader_elected(self, event: LeaderElectedEvent) -> None:
"""Handle the leader-elected event."""
data = self._peers.data[self.app]
# The leader sets the needed password on peer relation databag if they weren't set before.
data.setdefault("postgres-password", new_password())
data.setdefault("operator-password", new_password())
data.setdefault("replication-password", new_password())

# Update the list of the current PostgreSQL hosts when a new leader is elected.
Expand Down Expand Up @@ -522,11 +522,10 @@ def _on_start(self, event) -> None:
if self._has_blocked_status:
return

postgres_password = self._get_postgres_password()
replication_password = self._get_postgres_password()
postgres_password = self._get_password()
# If the leader was not elected (and the needed passwords were not generated yet),
# the cluster cannot be bootstrapped yet.
if not postgres_password or not replication_password:
if not postgres_password or not self._replication_password:
logger.info("leader not elected and/or passwords not yet generated")
self.unit.status = WaitingStatus("awaiting passwords generation")
event.defer()
Expand Down Expand Up @@ -554,15 +553,24 @@ def _on_start(self, event) -> None:
event.defer()
return

# Create the default postgres database user that is needed for some
# applications (not charms) like Landscape Server.
try:
self.postgresql.create_user("postgres", new_password(), admin=True)
except PostgreSQLCreateUserError as e:
logger.exception(e)
self.unit.status = BlockedStatus("Failed to create postgres user")
return

self.postgresql_client_relation.oversee_users()

# Set the flag to enable the replicas to start the Patroni service.
self._peers.data[self.app]["cluster_initialised"] = "True"
self.unit.status = ActiveStatus()

def _on_get_initial_password(self, event: ActionEvent) -> None:
"""Returns the password for the postgres user as an action response."""
event.set_results({"postgres-password": self._get_postgres_password()})
def _on_get_password(self, event: ActionEvent) -> None:
"""Returns the password for the operator user as an action response."""
event.set_results({"operator-password": self._get_password()})

def _on_update_status(self, _) -> None:
"""Update endpoints of the postgres client relation and update users list."""
Expand All @@ -576,15 +584,15 @@ def _has_blocked_status(self) -> bool:
"""Returns whether the unit is in a blocked state."""
return isinstance(self.unit.status, BlockedStatus)

def _get_postgres_password(self) -> str:
"""Get postgres user password.
def _get_password(self) -> str:
"""Get operator user password.
Returns:
The password from the peer relation or None if the
password has not yet been set by the leader.
"""
data = self._peers.data[self.app]
return data.get("postgres-password")
return data.get("operator-password")

@property
def _replication_password(self) -> str:
Expand Down
5 changes: 4 additions & 1 deletion src/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
wait_fixed,
)

from constants import USER

logger = logging.getLogger(__name__)

PATRONI_SERVICE = "patroni"
Expand Down Expand Up @@ -70,7 +72,7 @@ def __init__(
member_name: name of the member inside the cluster
peers_ips: IP addresses of the peer units
planned_units: number of units planned for the cluster
superuser_password: password for the postgres user
superuser_password: password for the operator user
replication_password: password for the user used in the replication
"""
self.unit_ip = unit_ip
Expand Down Expand Up @@ -259,6 +261,7 @@ def _render_patroni_yml_file(self, replica: bool = False) -> None:
scope=self.cluster_name,
self_ip=self.unit_ip,
replica=replica,
superuser=USER,
superuser_password=self.superuser_password,
replication_password=self.replication_password,
version=self._get_postgresql_version(),
Expand Down
1 change: 1 addition & 0 deletions src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@
LEGACY_DB_ADMIN = "db-admin"
PEER = "database-peers"
ALL_CLIENT_RELATIONS = [DATABASE, LEGACY_DB, LEGACY_DB_ADMIN]
USER = "operator"
2 changes: 1 addition & 1 deletion templates/patroni.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,5 @@ postgresql:
username: replication
password: {{ replication_password }}
superuser:
username: postgres
username: {{ superuser }}
password: {{ superuser_password }}
22 changes: 11 additions & 11 deletions tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ async def check_database_users_existence(
"""
unit = ops_test.model.applications[DATABASE_APP_NAME].units[0]
unit_address = await unit.get_public_address()
password = await get_postgres_password(ops_test, unit.name)
password = await get_password(ops_test, unit.name)

# Retrieve all users in the database.
users_in_db = await execute_query_on_unit(
Expand All @@ -94,7 +94,7 @@ async def check_databases_creation(ops_test: OpsTest, databases: List[str]) -> N
databases: List of database names that should have been created
"""
unit = ops_test.model.applications[DATABASE_APP_NAME].units[0]
password = await get_postgres_password(ops_test, unit.name)
password = await get_password(ops_test, unit.name)

for unit in ops_test.model.applications[DATABASE_APP_NAME].units:
unit_address = await unit.get_public_address()
Expand Down Expand Up @@ -160,13 +160,13 @@ def db_connect(host: str, password: str) -> psycopg2.extensions.connection:
Args:
host: the IP of the postgres host
password: postgres password
password: operator user password
Returns:
psycopg2 connection object linked to postgres db, under "postgres" user.
psycopg2 connection object linked to postgres db, under "operator" user.
"""
return psycopg2.connect(
f"dbname='postgres' user='postgres' host='{host}' password='{password}' connect_timeout=10"
f"dbname='postgres' user='operator' host='{host}' password='{password}' connect_timeout=10"
)


Expand Down Expand Up @@ -290,7 +290,7 @@ async def execute_query_on_unit(
A list of rows that were potentially returned from the query.
"""
with psycopg2.connect(
f"dbname='{database}' user='postgres' host='{unit_address}' password='{password}' connect_timeout=10"
f"dbname='{database}' user='operator' host='{unit_address}' password='{password}' connect_timeout=10"
) as connection, connection.cursor() as cursor:
cursor.execute(query)
output = list(itertools.chain(*cursor.fetchall()))
Expand Down Expand Up @@ -344,20 +344,20 @@ def get_application_units_ips(ops_test: OpsTest, application_name: str) -> List[
return [unit.public_address for unit in ops_test.model.applications[application_name].units]


async def get_postgres_password(ops_test: OpsTest, unit_name: str) -> str:
"""Retrieve the postgres user password using the action.
async def get_password(ops_test: OpsTest, unit_name: str) -> str:
"""Retrieve the operator user password using the action.
Args:
ops_test: ops_test instance.
unit_name: the name of the unit.
Returns:
the postgres user password.
the operator user password.
"""
unit = ops_test.model.units.get(unit_name)
action = await unit.run_action("get-initial-password")
action = await unit.run_action("get-password")
result = await action.wait()
return result.results["postgres-password"]
return result.results["operator-password"]


async def get_primary(ops_test: OpsTest, unit_name: str) -> str:
Expand Down
13 changes: 5 additions & 8 deletions tests/integration/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
convert_records_to_dict,
db_connect,
find_unit,
get_postgres_password,
get_password,
get_primary,
get_unit_address,
scale_application,
Expand Down Expand Up @@ -78,12 +78,9 @@ async def test_settings_are_correct(ops_test: OpsTest, series: str, unit_id: int
# Set a composite application name in order to test in more than one series at the same time.
application_name = build_application_name(series)

# Retrieving the postgres user password using the action.
action = await ops_test.model.units.get(f"{application_name}/{unit_id}").run_action(
"get-initial-password"
)
action = await action.wait()
password = action.results["postgres-password"]
# Retrieving the operator user password using the action.
any_unit_name = ops_test.model.applications[application_name].units[0].name
password = await get_password(ops_test, any_unit_name)

# Connect to PostgreSQL.
host = get_unit_address(ops_test, f"{application_name}/{unit_id}")
Expand Down Expand Up @@ -215,7 +212,7 @@ async def test_persist_data_through_primary_deletion(ops_test: OpsTest, series:
application_name = build_application_name(series)
any_unit_name = ops_test.model.applications[application_name].units[0].name
primary = await get_primary(ops_test, any_unit_name)
password = await get_postgres_password(ops_test, primary)
password = await get_password(ops_test, primary)

# Write data to primary IP.
host = get_unit_address(ops_test, primary)
Expand Down
Loading

0 comments on commit 79286df

Please sign in to comment.