Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
4a0399e
Change charm database user
marceloneppel Aug 16, 2022
37c0639
Fix unit tests
marceloneppel Aug 16, 2022
d05d991
Fix integration test call
marceloneppel Aug 16, 2022
52226c7
Merge branch 'main' into new-user
marceloneppel Aug 16, 2022
13c5aa9
Merge branch 'main' into new-user
marceloneppel Aug 17, 2022
aa61f60
Fix user name in library
marceloneppel Aug 17, 2022
991de5f
Fix user
marceloneppel Aug 17, 2022
db18ba6
Add default postgres user creation
marceloneppel Aug 25, 2022
61946da
Change action name
marceloneppel Aug 25, 2022
5f01c6e
Rework secrets management
marceloneppel Aug 25, 2022
aa14718
Add password rotation logic
marceloneppel Aug 26, 2022
9e7fdfd
Add user to the action parameters
marceloneppel Aug 26, 2022
12a4a7b
Add separate environments for different tests
marceloneppel Aug 27, 2022
bc723d1
Add all dependencies
marceloneppel Aug 27, 2022
3e22732
Merge branch 'password-rotation' into parallelize-tests
marceloneppel Aug 27, 2022
2d90e03
Add pytest marks
marceloneppel Aug 27, 2022
3fab9c6
Merge branch 'password-rotation' into parallelize-tests
marceloneppel Aug 27, 2022
82a9a68
Merge branch 'main' into password-rotation
marceloneppel Sep 1, 2022
3f259d5
Fix action description
marceloneppel Sep 2, 2022
56ff42a
Fix method docstring
marceloneppel Sep 2, 2022
62cc604
Fix pytest mark
marceloneppel Sep 2, 2022
63004b4
Fix pytest markers
marceloneppel Sep 2, 2022
e188259
Merge branch 'password-rotation' into parallelize-tests
marceloneppel Sep 2, 2022
42a5b13
Register pytest markers
marceloneppel Sep 3, 2022
79d885b
Merge branch 'main' into parallelize-tests
marceloneppel Sep 5, 2022
3b04265
Import files
marceloneppel Sep 6, 2022
1560781
Import files
marceloneppel Sep 6, 2022
ee43a39
Update libraries
marceloneppel Sep 7, 2022
7764cb0
Merge branch 'tls-relations' into tls-implementation
marceloneppel Sep 7, 2022
7c1c74e
Add TLS implementation
marceloneppel Sep 7, 2022
0bce570
Delete file
marceloneppel Sep 7, 2022
5121002
Add integration test
marceloneppel Sep 7, 2022
9d761b7
Update library
marceloneppel Sep 8, 2022
00bd1e8
Fix PostgreSQL library
marceloneppel Sep 8, 2022
f25f7b0
Merge branch 'tls-relations' into tls-implementation
marceloneppel Sep 8, 2022
dbfb7a6
Merge branch 'tls-implementation' into tls-integration-test
marceloneppel Sep 8, 2022
79b1447
Merge branch 'main' into tls-relations
marceloneppel Sep 9, 2022
35a83dd
Merge branch 'tls-relations' into tls-implementation
marceloneppel Sep 9, 2022
30a1484
Add relation broken test
marceloneppel Sep 9, 2022
03cc4f3
Merge branch 'tls-implementation' into tls-integration-test
marceloneppel Sep 9, 2022
91a1f26
Add jsonschema as a binary dependency
marceloneppel Sep 12, 2022
c472047
Merge branch 'tls-relations' into tls-implementation
marceloneppel Sep 12, 2022
a5c9226
Merge branch 'tls-implementation' into tls-integration-test
marceloneppel Sep 12, 2022
154df25
Change hostname to unit ip
marceloneppel Sep 12, 2022
1a8c7a2
Merge branch 'tls-implementation' into tls-integration-test
marceloneppel Sep 12, 2022
24ac60e
Add unit test dependency
marceloneppel Sep 12, 2022
abc324f
Merge branch 'tls-implementation' into tls-integration-test
marceloneppel Sep 12, 2022
d9ec362
Add workaround for Patroni REST API TLS
marceloneppel Sep 12, 2022
bdca5d4
Improve TLS status retrieval
marceloneppel Sep 13, 2022
5bc6ae1
Merge branch 'main' into tls-patroni-rest-api
marceloneppel Sep 20, 2022
7c860b7
Readd checks
marceloneppel Sep 20, 2022
334a50d
Add additional check
marceloneppel Sep 21, 2022
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
10 changes: 5 additions & 5 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ def _patroni(self) -> Patroni:
self._peer_members_ips,
self._get_password(),
self._replication_password,
bool(self.unit_peer_data.get("tls")),
)

@property
Expand Down Expand Up @@ -820,12 +821,10 @@ def push_tls_files_to_workload(self) -> None:
self.update_config()

def _restart(self, _) -> None:
"""Restart PostgreSQL."""
try:
self._patroni.restart_postgresql()
except RetryError as e:
"""Restart Patroni and PostgreSQL."""
if not self._patroni.restart_patroni():
logger.exception("failed to restart PostgreSQL")
self.unit.status = BlockedStatus(f"failed to restart PostgreSQL with error {e}")
self.unit.status = BlockedStatus("failed to restart Patroni and PostgreSQL")

def update_config(self) -> None:
"""Updates Patroni config file based on the existence of the TLS files."""
Expand All @@ -838,6 +837,7 @@ def update_config(self) -> None:

restart_postgresql = enable_tls != self.postgresql.is_tls_enabled()
self._patroni.reload_patroni_configuration()
self.unit_peer_data.update({"tls": "enabled" if enable_tls else ""})

# Restart PostgreSQL if TLS configuration has changed
# (so the both old and new connections use the configuration).
Expand Down
43 changes: 34 additions & 9 deletions src/cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from charms.operator_libs_linux.v0.apt import DebianPackage
from charms.operator_libs_linux.v1.systemd import (
daemon_reload,
service_restart,
service_running,
service_start,
)
Expand All @@ -28,7 +29,7 @@
wait_fixed,
)

from constants import USER
from constants import TLS_CA_FILE, USER

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -62,6 +63,7 @@ def __init__(
peers_ips: Set[str],
superuser_password: str,
replication_password: str,
tls_enabled: bool,
):
"""Initialize the Patroni class.

Expand All @@ -74,6 +76,7 @@ def __init__(
planned_units: number of units planned for the cluster
superuser_password: password for the operator user
replication_password: password for the user used in the replication
tls_enabled: whether TLS is enabled
Copy link
Contributor

Choose a reason for hiding this comment

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

nit add verify

Copy link
Member Author

Choose a reason for hiding this comment

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

This specific parameter is related to whether TLS is already enabled or not, different from the tests, where we have parameters related to verify whether TLS is enabled.

"""
self.unit_ip = unit_ip
self.storage_path = storage_path
Expand All @@ -83,6 +86,16 @@ def __init__(
self.peers_ips = peers_ips
self.superuser_password = superuser_password
self.replication_password = replication_password
self.tls_enabled = tls_enabled
# Variable mapping to requests library verify parameter.
# The CA bundle file is used to validate the server certificate when
# TLS is enabled, otherwise True is set because it's the default value.
self.verify = f"{self.storage_path}/{TLS_CA_FILE}" if tls_enabled else True

@property
def _patroni_url(self) -> str:
"""Patroni REST API URL."""
return f"{'https' if self.tls_enabled else 'http'}://{self.unit_ip}:8008"

def bootstrap_cluster(self) -> bool:
"""Bootstrap a PostgreSQL cluster using Patroni."""
Expand Down Expand Up @@ -117,7 +130,7 @@ def _change_owner(self, path: str) -> None:
def cluster_members(self) -> set:
"""Get the current cluster members."""
# Request info from cluster endpoint (which returns all members of the cluster).
cluster_status = requests.get(f"http://{self.unit_ip}:8008/cluster")
cluster_status = requests.get(f"{self._patroni_url}/cluster", verify=self.verify)
return set([member["name"] for member in cluster_status.json()["members"]])

def _create_directory(self, path: str, mode: int) -> None:
Expand Down Expand Up @@ -150,7 +163,7 @@ def get_member_ip(self, member_name: str) -> str:
"""
ip = None
# Request info from cluster endpoint (which returns all members of the cluster).
cluster_status = requests.get(f"http://{self.unit_ip}:8008/cluster")
cluster_status = requests.get(f"{self._patroni_url}/cluster", verify=self.verify)
for member in cluster_status.json()["members"]:
if member["name"] == member_name:
ip = member["host"]
Expand All @@ -168,7 +181,7 @@ def get_primary(self, unit_name_pattern=False) -> str:
"""
primary = None
# Request info from cluster endpoint (which returns all members of the cluster).
cluster_status = requests.get(f"http://{self.unit_ip}:8008/cluster")
cluster_status = requests.get(f"{self._patroni_url}/cluster", verify=self.verify)
for member in cluster_status.json()["members"]:
if member["role"] == "leader":
primary = member["name"]
Expand All @@ -190,7 +203,9 @@ def are_all_members_ready(self) -> bool:
try:
for attempt in Retrying(stop=stop_after_delay(10), wait=wait_fixed(3)):
with attempt:
cluster_status = requests.get(f"http://{self.unit_ip}:8008/cluster")
cluster_status = requests.get(
f"{self._patroni_url}/cluster", verify=self.verify
)
except RetryError:
return False

Expand All @@ -212,7 +227,7 @@ def member_started(self) -> bool:
try:
for attempt in Retrying(stop=stop_after_delay(60), wait=wait_fixed(3)):
with attempt:
r = requests.get(f"http://{self.unit_ip}:8008/health")
r = requests.get(f"{self._patroni_url}/health", verify=self.verify)
except RetryError:
return False

Expand Down Expand Up @@ -300,8 +315,9 @@ def switchover(self) -> None:
with attempt:
current_primary = self.get_primary()
r = requests.post(
f"http://{self.unit_ip}:8008/switchover",
f"{self._patroni_url}/switchover",
json={"leader": current_primary},
verify=self.verify,
)

# Check whether the switchover was unsuccessful.
Expand Down Expand Up @@ -348,9 +364,18 @@ def remove_raft_member(self, member_ip: str) -> None:
@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=2, max=10))
def reload_patroni_configuration(self):
"""Reload Patroni configuration after it was changed."""
requests.post(f"http://{self.unit_ip}:8008/reload")
requests.post(f"{self._patroni_url}/reload", verify=self.verify)

def restart_patroni(self) -> bool:
"""Restart Patroni.

Returns:
Whether the service restarted successfully.
"""
service_restart(PATRONI_SERVICE)
return service_running(PATRONI_SERVICE)

@retry(stop=stop_after_attempt(3), wait=wait_exponential(multiplier=1, min=2, max=10))
def restart_postgresql(self) -> None:
"""Restart PostgreSQL."""
requests.post(f"http://{self.unit_ip}:8008/restart")
requests.post(f"{self._patroni_url}/restart", verify=self.verify)
12 changes: 12 additions & 0 deletions templates/patroni.yml.j2
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,18 @@ log:
restapi:
listen: '{{ self_ip }}:8008'
connect_address: '{{ self_ip }}:8008'
{%- if enable_tls %}
cafile: {{ conf_path }}/ca.pem
certfile: {{ conf_path }}/cert.pem
keyfile: {{ conf_path }}/key.pem
{%- endif %}

{%- if enable_tls %}
ctl:
cacert: {{ conf_path }}/ca.pem
certfile: {{ conf_path }}/cert.pem
keyfile: {{ conf_path }}/key.pem
{%- endif %}

raft:
data_dir: {{ conf_path }}/raft
Expand Down
68 changes: 68 additions & 0 deletions tests/integration/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Copyright 2022 Canonical Ltd.
# See LICENSE file for licensing details.
import itertools
import json
import tempfile
import zipfile
from datetime import datetime
Expand Down Expand Up @@ -412,6 +413,32 @@ async def get_primary(ops_test: OpsTest, unit_name: str) -> str:
return action.results["primary"]


async def get_tls_ca(
ops_test: OpsTest,
unit_name: str,
) -> str:
"""Returns the TLS CA used by the unit.

Args:
ops_test: The ops test framework instance
unit_name: The name of the unit

Returns:
TLS CA or an empty string if there is no CA.
"""
raw_data = (await ops_test.juju("show-unit", unit_name))[1]
if not raw_data:
raise ValueError(f"no unit info could be grabbed for {unit_name}")
data = yaml.safe_load(raw_data)
# Filter the data based on the relation name.
relation_data = [
v for v in data[unit_name]["relation-info"] if v["endpoint"] == "certificates"
]
if len(relation_data) == 0:
return ""
return json.loads(relation_data[0]["application-data"]["certificates"])[0].get("ca")


def get_unit_address(ops_test: OpsTest, unit_name: str) -> str:
"""Get unit IP address.

Expand Down Expand Up @@ -490,6 +517,47 @@ async def check_tls(ops_test: OpsTest, unit_name: str, enabled: bool) -> bool:
return False


async def check_tls_patroni_api(ops_test: OpsTest, unit_name: str, enabled: bool) -> bool:
"""Returns whether TLS is enabled on Patroni REST API.

Args:
ops_test: The ops test framework instance.
unit_name: The name of the unit where Patroni is running.
enabled: check if TLS is enabled/disabled

Returns:
Whether TLS is enabled/disabled on Patroni REST API.
"""
unit_address = get_unit_address(ops_test, unit_name)
tls_ca = await get_tls_ca(ops_test, unit_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you return False here if not tls_ca?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'll add it.

Copy link
Member Author

@marceloneppel marceloneppel Sep 21, 2022

Choose a reason for hiding this comment

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

Added on 334a50d.


# If there is no TLS CA in the relation, something is wrong in
# the relation between the TLS Certificates Operator and PostgreSQL.
if enabled and not tls_ca:
return False

try:
for attempt in Retrying(
stop=stop_after_attempt(10), wait=wait_exponential(multiplier=1, min=2, max=30)
):
with attempt, tempfile.NamedTemporaryFile() as temp_ca_file:
# Write the TLS CA to a temporary file to use it in a request.
temp_ca_file.write(tls_ca.encode("utf-8"))
temp_ca_file.seek(0)

# The CA bundle file is used to validate the server certificate when
# TLS is enabled, otherwise True is set because it's the default value
# for the verify parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

excellent commenting

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks Mia!

health_info = requests.get(
f"{'https' if enabled else 'http'}://{unit_address}:8008/health",
verify=temp_ca_file.name if enabled else True,
)
return health_info.status_code == 200
except RetryError:
return False
return False


async def scale_application(ops_test: OpsTest, application_name: str, count: int) -> None:
"""Scale a given application to a specific unit count.

Expand Down
8 changes: 7 additions & 1 deletion tests/integration/test_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
from pytest_operator.plugin import OpsTest

from tests.helpers import METADATA
from tests.integration.helpers import DATABASE_APP_NAME, check_tls
from tests.integration.helpers import (
DATABASE_APP_NAME,
check_tls,
check_tls_patroni_api,
)

APP_NAME = METADATA["name"]
TLS_CERTIFICATES_APP_NAME = "tls-certificates-operator"
Expand Down Expand Up @@ -43,6 +47,7 @@ async def test_tls_enabled(ops_test: OpsTest) -> None:
# Wait for all units enabling TLS.
for unit in ops_test.model.applications[DATABASE_APP_NAME].units:
assert await check_tls(ops_test, unit.name, enabled=True)
assert await check_tls_patroni_api(ops_test, unit.name, enabled=True)

# Remove the relation.
await ops_test.model.applications[DATABASE_APP_NAME].remove_relation(
Expand All @@ -53,3 +58,4 @@ async def test_tls_enabled(ops_test: OpsTest) -> None:
# Wait for all units disabling TLS.
for unit in ops_test.model.applications[DATABASE_APP_NAME].units:
assert await check_tls(ops_test, unit.name, enabled=False)
assert await check_tls_patroni_api(ops_test, unit.name, enabled=False)
1 change: 1 addition & 0 deletions tests/unit/test_cluster.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ def setUp(self):
peers_ips,
"fake-superuser-password",
"fake-replication-password",
False,
)

@patch("charms.operator_libs_linux.v0.apt.DebianPackage.from_system")
Expand Down