From 718501f0fc870ffd52bef09b6b024db76cd8f2c4 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 9 Sep 2024 11:18:04 -0300 Subject: [PATCH 1/9] Add pgAudit Signed-off-by: Marcelo Henrique Neppel --- config.yaml | 4 ++++ lib/charms/postgresql_k8s/v0/postgresql.py | 12 +++++++++++- metadata.yaml | 2 +- src/charm.py | 4 ++-- src/config.py | 1 + src/constants.py | 1 + src/relations/db.py | 8 +++++++- src/relations/postgresql_provider.py | 3 ++- templates/patroni.yml.j2 | 2 +- tests/integration/test_plugins.py | 9 +++++++++ 10 files changed, 39 insertions(+), 7 deletions(-) diff --git a/config.yaml b/config.yaml index 2a3cad8c17..0df7ed8dfc 100644 --- a/config.yaml +++ b/config.yaml @@ -303,6 +303,10 @@ options: default: false type: boolean description: Enable timescaledb extension + plugin_audit_enable: + default: false + type: boolean + description: Enable pgAudit extension profile: description: | Profile representing the scope of deployment, and used to tune resource allocation. diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index c3412d36df..9a72c8c5c7 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -36,7 +36,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 34 +LIBPATCH = 35 INVALID_EXTRA_USER_ROLE_BLOCKING_MESSAGE = "invalid role(s) for extra user roles" @@ -114,6 +114,15 @@ def __init__( self.database = database self.system_users = system_users + def _configure_pgaudit(self, database: str, enable: bool) -> None: + with self._connect_to_database() as connection, connection.cursor() as cursor: + if enable: + cursor.execute( + f"ALTER DATABASE {database} SET pgaudit.log to 'ROLE,DDL,MISC,MISC_SET';" + ) + else: + cursor.execute(f"ALTER DATABASE {database} RESET pgaudit.log;") + def _connect_to_database( self, database: str = None, database_host: str = None ) -> psycopg2.extensions.connection: @@ -325,6 +334,7 @@ def enable_disable_extensions(self, extensions: Dict[str, bool], database: str = if enable else f"DROP EXTENSION IF EXISTS {extension};" ) + self._configure_pgaudit(database, ordered_extensions["pgaudit"]) except psycopg2.errors.UniqueViolation: pass except psycopg2.errors.DependentObjectsStillExist: diff --git a/metadata.yaml b/metadata.yaml index a7311d0a7f..cb3f49b6e4 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -28,7 +28,7 @@ resources: postgresql-image: type: oci-image description: OCI image for PostgreSQL - upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:2c066876e80d60058d79835c8b5d18090963b3a0f84261385afa1fc652477605 # renovate: oci-image tag: 14.12-22.04_edge + upstream-source: ghcr.io/canonical/charmed-postgresql@sha256:3abdbc00413b065fbfea8c6a3aaad8e137790ebb3e7bf5e1f42e19cbc1861926 # renovate: oci-image tag: 14.13-22.04_edge peers: database-peers: diff --git a/src/charm.py b/src/charm.py index 6a0ccb31b1..440f5aad9e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -90,6 +90,7 @@ MONITORING_USER, PATRONI_PASSWORD_KEY, PEER, + PLUGIN_OVERRIDES, POSTGRES_LOG_FILES, REPLICATION_PASSWORD_KEY, REPLICATION_USER, @@ -654,7 +655,6 @@ def enable_disable_extensions(self, database: str = None) -> None: logger.debug("Early exit enable_disable_extensions: standby cluster") return spi_module = ["refint", "autoinc", "insert_username", "moddatetime"] - plugins_exception = {"uuid_ossp": '"uuid-ossp"'} original_status = self.unit.status extensions = {} # collect extensions @@ -667,7 +667,7 @@ def enable_disable_extensions(self, database: str = None) -> None: for ext in spi_module: extensions[ext] = enable continue - extension = plugins_exception.get(extension, extension) + extension = PLUGIN_OVERRIDES.get(extension, extension) if self._check_extension_dependencies(extension, enable): self.unit.status = BlockedStatus(EXTENSIONS_DEPENDENCY_MESSAGE) return diff --git a/src/config.py b/src/config.py index f39920b20e..b5f41ec5b4 100644 --- a/src/config.py +++ b/src/config.py @@ -34,6 +34,7 @@ class CharmConfig(BaseConfigModel): optimizer_join_collapse_limit: Optional[int] profile: str profile_limit_memory: Optional[int] + plugin_audit_enable: bool plugin_citext_enable: bool plugin_debversion_enable: bool plugin_hstore_enable: bool diff --git a/src/constants.py b/src/constants.py index d56611ba1a..5f6a581bb1 100644 --- a/src/constants.py +++ b/src/constants.py @@ -42,6 +42,7 @@ SECRET_KEY_OVERRIDES = {"ca": "cauth"} BACKUP_TYPE_OVERRIDES = {"full": "full", "differential": "diff", "incremental": "incr"} +PLUGIN_OVERRIDES = {"audit": "pgaudit", "uuid_ossp": '"uuid-ossp"'} TRACING_RELATION_NAME = "tracing" TRACING_PROTOCOL = "otlp_http" diff --git a/src/relations/db.py b/src/relations/db.py index 290528d251..3862f6fb10 100644 --- a/src/relations/db.py +++ b/src/relations/db.py @@ -22,7 +22,12 @@ from ops.model import ActiveStatus, BlockedStatus, Relation, Unit from pgconnstr import ConnectionString -from constants import ALL_LEGACY_RELATIONS, DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE +from constants import ( + ALL_LEGACY_RELATIONS, + DATABASE_PORT, + ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, + PLUGIN_OVERRIDES, +) from utils import new_password logger = logging.getLogger(__name__) @@ -181,6 +186,7 @@ def set_up_relation(self, relation: Relation) -> bool: for plugin in self.charm.config.plugin_keys() if self.charm.config[plugin] ] + plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins] self.charm.postgresql.create_database( database, user, plugins=plugins, client_relations=self.charm.client_relations diff --git a/src/relations/postgresql_provider.py b/src/relations/postgresql_provider.py index 625d5e9c22..7f5193065a 100644 --- a/src/relations/postgresql_provider.py +++ b/src/relations/postgresql_provider.py @@ -20,7 +20,7 @@ from ops.framework import Object from ops.model import ActiveStatus, BlockedStatus, Relation -from constants import DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE +from constants import DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, PLUGIN_OVERRIDES from utils import new_password logger = logging.getLogger(__name__) @@ -92,6 +92,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: for plugin in self.charm.config.plugin_keys() if self.charm.config[plugin] ] + plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins] self.charm.postgresql.create_database( database, user, plugins=plugins, client_relations=self.charm.client_relations diff --git a/templates/patroni.yml.j2 b/templates/patroni.yml.j2 index 96fa3844d1..ccac3ee1e5 100644 --- a/templates/patroni.yml.j2 +++ b/templates/patroni.yml.j2 @@ -40,7 +40,7 @@ bootstrap: log_truncate_on_rotation: 'on' logging_collector: 'on' wal_level: logical - shared_preload_libraries: 'timescaledb' + shared_preload_libraries: 'timescaledb,pgaudit' {%- if pg_parameters %} {%- for key, value in pg_parameters.items() %} {{key}}: {{value}} diff --git a/tests/integration/test_plugins.py b/tests/integration/test_plugins.py index 834a599ae2..202dd38671 100644 --- a/tests/integration/test_plugins.py +++ b/tests/integration/test_plugins.py @@ -253,3 +253,12 @@ async def test_plugin_objects(ops_test: OpsTest) -> None: logger.info("Waiting for status to resolve again") async with ops_test.fast_forward(fast_interval="60s"): await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") + + +@pytest.mark.group(1) +async def test_audit_plugin(ops_test: OpsTest) -> None: + """Test the audit plugin.""" + await ops_test.model.applications[DATABASE_APP_NAME].set_config({ + "plugin_audit_enable": "True" + }) + await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") From 065ce5ca68192a25e6a75b404e9bdda7356ee208 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 9 Sep 2024 12:26:22 -0300 Subject: [PATCH 2/9] Fix extensions enablement on database request Signed-off-by: Marcelo Henrique Neppel --- src/charm.py | 4 ++-- src/constants.py | 2 ++ src/relations/db.py | 5 +++++ src/relations/postgresql_provider.py | 6 +++++- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index 440f5aad9e..33bd3d95fa 100755 --- a/src/charm.py +++ b/src/charm.py @@ -98,6 +98,7 @@ SECRET_DELETED_LABEL, SECRET_INTERNAL_LABEL, SECRET_KEY_OVERRIDES, + SPI_MODULE, SYSTEM_USERS, TLS_CA_FILE, TLS_CERT_FILE, @@ -654,7 +655,6 @@ def enable_disable_extensions(self, database: str = None) -> None: if self._patroni.get_primary() is None: logger.debug("Early exit enable_disable_extensions: standby cluster") return - spi_module = ["refint", "autoinc", "insert_username", "moddatetime"] original_status = self.unit.status extensions = {} # collect extensions @@ -664,7 +664,7 @@ def enable_disable_extensions(self, database: str = None) -> None: # Enable or disable the plugin/extension. extension = "_".join(plugin.split("_")[1:-1]) if extension == "spi": - for ext in spi_module: + for ext in SPI_MODULE: extensions[ext] = enable continue extension = PLUGIN_OVERRIDES.get(extension, extension) diff --git a/src/constants.py b/src/constants.py index 5f6a581bb1..7fb157e507 100644 --- a/src/constants.py +++ b/src/constants.py @@ -44,6 +44,8 @@ BACKUP_TYPE_OVERRIDES = {"full": "full", "differential": "diff", "incremental": "incr"} PLUGIN_OVERRIDES = {"audit": "pgaudit", "uuid_ossp": '"uuid-ossp"'} +SPI_MODULE = ["refint", "autoinc", "insert_username", "moddatetime"] + TRACING_RELATION_NAME = "tracing" TRACING_PROTOCOL = "otlp_http" diff --git a/src/relations/db.py b/src/relations/db.py index 3862f6fb10..63deeed687 100644 --- a/src/relations/db.py +++ b/src/relations/db.py @@ -27,6 +27,7 @@ DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, PLUGIN_OVERRIDES, + SPI_MODULE, ) from utils import new_password @@ -187,6 +188,10 @@ def set_up_relation(self, relation: Relation) -> bool: if self.charm.config[plugin] ] plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins] + if "spi" in plugins: + plugins.remove("spi") + for ext in SPI_MODULE: + plugins.append(ext) self.charm.postgresql.create_database( database, user, plugins=plugins, client_relations=self.charm.client_relations diff --git a/src/relations/postgresql_provider.py b/src/relations/postgresql_provider.py index 7f5193065a..ac86474371 100644 --- a/src/relations/postgresql_provider.py +++ b/src/relations/postgresql_provider.py @@ -20,7 +20,7 @@ from ops.framework import Object from ops.model import ActiveStatus, BlockedStatus, Relation -from constants import DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, PLUGIN_OVERRIDES +from constants import DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, PLUGIN_OVERRIDES, SPI_MODULE from utils import new_password logger = logging.getLogger(__name__) @@ -93,6 +93,10 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: if self.charm.config[plugin] ] plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins] + if "spi" in plugins: + plugins.remove("spi") + for ext in SPI_MODULE: + plugins.append(ext) self.charm.postgresql.create_database( database, user, plugins=plugins, client_relations=self.charm.client_relations From 4998dd336781b6d9b309b638044ddfe800a45b82 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 10 Sep 2024 17:19:10 -0300 Subject: [PATCH 3/9] Make pgAudit work and add integration test Signed-off-by: Marcelo Henrique Neppel --- lib/charms/postgresql_k8s/v0/postgresql.py | 28 ++++++--- src/charm.py | 14 +++++ src/relations/db.py | 13 +---- src/relations/postgresql_provider.py | 16 ++--- tests/integration/test_plugins.py | 68 ++++++++++++++++++++++ 5 files changed, 107 insertions(+), 32 deletions(-) diff --git a/lib/charms/postgresql_k8s/v0/postgresql.py b/lib/charms/postgresql_k8s/v0/postgresql.py index 9a72c8c5c7..0a13a765de 100644 --- a/lib/charms/postgresql_k8s/v0/postgresql.py +++ b/lib/charms/postgresql_k8s/v0/postgresql.py @@ -114,14 +114,24 @@ def __init__( self.database = database self.system_users = system_users - def _configure_pgaudit(self, database: str, enable: bool) -> None: - with self._connect_to_database() as connection, connection.cursor() as cursor: - if enable: - cursor.execute( - f"ALTER DATABASE {database} SET pgaudit.log to 'ROLE,DDL,MISC,MISC_SET';" - ) - else: - cursor.execute(f"ALTER DATABASE {database} RESET pgaudit.log;") + def _configure_pgaudit(self, enable: bool) -> None: + connection = None + try: + connection = self._connect_to_database() + connection.autocommit = True + with connection.cursor() as cursor: + if enable: + cursor.execute("ALTER SYSTEM SET pgaudit.log = 'ROLE,DDL,MISC,MISC_SET';") + cursor.execute("ALTER SYSTEM SET pgaudit.log_client TO off;") + cursor.execute("ALTER SYSTEM SET pgaudit.log_parameter TO off") + else: + cursor.execute("ALTER SYSTEM RESET pgaudit.log;") + cursor.execute("ALTER SYSTEM RESET pgaudit.log_client;") + cursor.execute("ALTER SYSTEM RESET pgaudit.log_parameter;") + cursor.execute("SELECT pg_reload_conf();") + finally: + if connection is not None: + connection.close() def _connect_to_database( self, database: str = None, database_host: str = None @@ -334,7 +344,7 @@ def enable_disable_extensions(self, extensions: Dict[str, bool], database: str = if enable else f"DROP EXTENSION IF EXISTS {extension};" ) - self._configure_pgaudit(database, ordered_extensions["pgaudit"]) + self._configure_pgaudit(ordered_extensions.get("pgaudit", False)) except psycopg2.errors.UniqueViolation: pass except psycopg2.errors.DependentObjectsStillExist: diff --git a/src/charm.py b/src/charm.py index 33bd3d95fa..35d26602b4 100755 --- a/src/charm.py +++ b/src/charm.py @@ -2091,6 +2091,20 @@ def log_pitr_last_transaction_time(self) -> None: else: logger.error("Can't tell last completed transaction time") + def get_plugins(self) -> List[str]: + """Return a list of installed plugins.""" + plugins = [ + "_".join(plugin.split("_")[1:-1]) + for plugin in self.config.plugin_keys() + if self.config[plugin] + ] + plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins] + if "spi" in plugins: + plugins.remove("spi") + for ext in SPI_MODULE: + plugins.append(ext) + return plugins + if __name__ == "__main__": main(PostgresqlOperatorCharm, use_juju_for_storage=True) diff --git a/src/relations/db.py b/src/relations/db.py index 63deeed687..38af8c08b6 100644 --- a/src/relations/db.py +++ b/src/relations/db.py @@ -26,8 +26,6 @@ ALL_LEGACY_RELATIONS, DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, - PLUGIN_OVERRIDES, - SPI_MODULE, ) from utils import new_password @@ -182,16 +180,7 @@ def set_up_relation(self, relation: Relation) -> bool: user = f"relation_id_{relation.id}" password = unit_relation_databag.get("password", new_password()) self.charm.postgresql.create_user(user, password, self.admin) - plugins = [ - "_".join(plugin.split("_")[1:-1]) - for plugin in self.charm.config.plugin_keys() - if self.charm.config[plugin] - ] - plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins] - if "spi" in plugins: - plugins.remove("spi") - for ext in SPI_MODULE: - plugins.append(ext) + plugins = self.charm.get_plugins() self.charm.postgresql.create_database( database, user, plugins=plugins, client_relations=self.charm.client_relations diff --git a/src/relations/postgresql_provider.py b/src/relations/postgresql_provider.py index ac86474371..6018501278 100644 --- a/src/relations/postgresql_provider.py +++ b/src/relations/postgresql_provider.py @@ -20,7 +20,10 @@ from ops.framework import Object from ops.model import ActiveStatus, BlockedStatus, Relation -from constants import DATABASE_PORT, ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, PLUGIN_OVERRIDES, SPI_MODULE +from constants import ( + DATABASE_PORT, + ENDPOINT_SIMULTANEOUSLY_BLOCKING_MESSAGE, +) from utils import new_password logger = logging.getLogger(__name__) @@ -87,16 +90,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: user = f"relation_id_{event.relation.id}" password = new_password() self.charm.postgresql.create_user(user, password, extra_user_roles=extra_user_roles) - plugins = [ - "_".join(plugin.split("_")[1:-1]) - for plugin in self.charm.config.plugin_keys() - if self.charm.config[plugin] - ] - plugins = [PLUGIN_OVERRIDES.get(plugin, plugin) for plugin in plugins] - if "spi" in plugins: - plugins.remove("spi") - for ext in SPI_MODULE: - plugins.append(ext) + plugins = self.charm.get_plugins() self.charm.postgresql.create_database( database, user, plugins=plugins, client_relations=self.charm.client_relations diff --git a/tests/integration/test_plugins.py b/tests/integration/test_plugins.py index 202dd38671..ac5798723a 100644 --- a/tests/integration/test_plugins.py +++ b/tests/integration/test_plugins.py @@ -6,15 +6,19 @@ import psycopg2 as psycopg2 import pytest as pytest from pytest_operator.plugin import OpsTest +from tenacity import Retrying, stop_after_delay, wait_fixed from .helpers import ( + APPLICATION_NAME, DATABASE_APP_NAME, build_and_deploy, db_connect, get_password, get_primary, get_unit_address, + run_command_on_unit, ) +from .new_relations.helpers import build_connection_string logger = logging.getLogger(__name__) @@ -86,6 +90,7 @@ "CREATE TABLE vector_test (id bigserial PRIMARY KEY, embedding vector(3));" ) TIMESCALEDB_EXTENSION_STATEMENT = "CREATE TABLE test_timescaledb (time TIMESTAMPTZ NOT NULL); SELECT create_hypertable('test_timescaledb', 'time');" +RELATION_ENDPOINT = "database" @pytest.mark.group(1) @@ -258,7 +263,70 @@ async def test_plugin_objects(ops_test: OpsTest) -> None: @pytest.mark.group(1) async def test_audit_plugin(ops_test: OpsTest) -> None: """Test the audit plugin.""" + await ops_test.model.deploy(APPLICATION_NAME) + await ops_test.model.relate(f"{APPLICATION_NAME}:{RELATION_ENDPOINT}", DATABASE_APP_NAME) + async with ops_test.fast_forward(): + await ops_test.model.wait_for_idle( + apps=[APPLICATION_NAME, DATABASE_APP_NAME], status="active" + ) + + # Check that the audit plugin is disabled. + connection_string = await build_connection_string( + ops_test, APPLICATION_NAME, RELATION_ENDPOINT + ) + connection = None + try: + connection = psycopg2.connect(connection_string) + with connection.cursor() as cursor: + cursor.execute("CREATE TABLE test1(value TEXT);") + cursor.execute("GRANT SELECT ON test1 TO PUBLIC;") + cursor.execute("SET TIME ZONE 'Europe/Rome';") + except Exception: + if connection is not None: + connection.close() + try: + primary = await get_primary(ops_test) + logs = await run_command_on_unit( + ops_test, + primary, + "grep AUDIT /var/log/postgresql/postgresql-*.log", + ) + except Exception: + pass + else: + logger.info(f"Logs: {logs}") + assert False, "Audit logs were found when the plugin is disabled." + + # Enable the audit plugin. await ops_test.model.applications[DATABASE_APP_NAME].set_config({ "plugin_audit_enable": "True" }) await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") + + # Check that the audit plugin is enabled. + try: + connection = psycopg2.connect(connection_string) + with connection.cursor() as cursor: + cursor.execute("CREATE TABLE test2(value TEXT);") + cursor.execute("GRANT SELECT ON test2 TO PUBLIC;") + cursor.execute("SET TIME ZONE 'Europe/Rome';") + except Exception: + if connection is not None: + connection.close() + for attempt in Retrying(stop=stop_after_delay(90), wait=wait_fixed(10), reraise=True): + with attempt: + try: + primary = await get_primary(ops_test) + logs = await run_command_on_unit( + ops_test, + primary, + "grep AUDIT /var/log/postgresql/postgresql-*.log", + ) + assert "MISC,BEGIN,,,BEGIN" in logs + assert ( + "DDL,CREATE TABLE,TABLE,public.test2,CREATE TABLE test2(value TEXT);" in logs + ) + assert "ROLE,GRANT,TABLE,,GRANT SELECT ON test2 TO PUBLIC;" in logs + assert "MISC,SET,,,SET TIME ZONE 'Europe/Rome';" in logs + except Exception: + assert False, "Audit logs were not found when the plugin is enabled." From 167f9546acd767e47aec20f62476e9f3dfc90eb0 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Tue, 10 Sep 2024 17:38:00 -0300 Subject: [PATCH 4/9] Split integration tests Signed-off-by: Marcelo Henrique Neppel --- tests/integration/test_audit.py | 96 +++++++++++++++++++++++++++++++ tests/integration/test_plugins.py | 77 ------------------------- 2 files changed, 96 insertions(+), 77 deletions(-) create mode 100644 tests/integration/test_audit.py diff --git a/tests/integration/test_audit.py b/tests/integration/test_audit.py new file mode 100644 index 0000000000..445cbb7dd3 --- /dev/null +++ b/tests/integration/test_audit.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. +import asyncio +import logging + +import psycopg2 as psycopg2 +import pytest as pytest +from pytest_operator.plugin import OpsTest +from tenacity import Retrying, stop_after_delay, wait_fixed + +from .helpers import ( + APPLICATION_NAME, + DATABASE_APP_NAME, + build_and_deploy, + get_primary, + run_command_on_unit, +) +from .new_relations.helpers import build_connection_string + +logger = logging.getLogger(__name__) + +RELATION_ENDPOINT = "database" + + +@pytest.mark.group(1) +@pytest.mark.abort_on_fail +async def test_audit_plugin(ops_test: OpsTest) -> None: + """Test the audit plugin.""" + await asyncio.gather(build_and_deploy(ops_test, 1), ops_test.model.deploy(APPLICATION_NAME)) + await ops_test.model.relate(f"{APPLICATION_NAME}:{RELATION_ENDPOINT}", DATABASE_APP_NAME) + async with ops_test.fast_forward(): + await ops_test.model.wait_for_idle( + apps=[APPLICATION_NAME, DATABASE_APP_NAME], status="active" + ) + + logger.info("Checking that the audit plugin is disabled") + connection_string = await build_connection_string( + ops_test, APPLICATION_NAME, RELATION_ENDPOINT + ) + connection = None + try: + connection = psycopg2.connect(connection_string) + with connection.cursor() as cursor: + cursor.execute("CREATE TABLE test1(value TEXT);") + cursor.execute("GRANT SELECT ON test1 TO PUBLIC;") + cursor.execute("SET TIME ZONE 'Europe/Rome';") + except Exception: + if connection is not None: + connection.close() + try: + primary = await get_primary(ops_test) + logs = await run_command_on_unit( + ops_test, + primary, + "grep AUDIT /var/log/postgresql/postgresql-*.log", + ) + except Exception: + pass + else: + logger.info(f"Logs: {logs}") + assert False, "Audit logs were found when the plugin is disabled." + + logger.info("Enabling the audit plugin") + await ops_test.model.applications[DATABASE_APP_NAME].set_config({ + "plugin_audit_enable": "True" + }) + await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") + + logger.info("Checking that the audit plugin is enabled") + try: + connection = psycopg2.connect(connection_string) + with connection.cursor() as cursor: + cursor.execute("CREATE TABLE test2(value TEXT);") + cursor.execute("GRANT SELECT ON test2 TO PUBLIC;") + cursor.execute("SET TIME ZONE 'Europe/Rome';") + except Exception: + if connection is not None: + connection.close() + for attempt in Retrying(stop=stop_after_delay(90), wait=wait_fixed(10), reraise=True): + with attempt: + try: + primary = await get_primary(ops_test) + logs = await run_command_on_unit( + ops_test, + primary, + "grep AUDIT /var/log/postgresql/postgresql-*.log", + ) + assert "MISC,BEGIN,,,BEGIN" in logs + assert ( + "DDL,CREATE TABLE,TABLE,public.test2,CREATE TABLE test2(value TEXT);" in logs + ) + assert "ROLE,GRANT,TABLE,,GRANT SELECT ON test2 TO PUBLIC;" in logs + assert "MISC,SET,,,SET TIME ZONE 'Europe/Rome';" in logs + except Exception: + assert False, "Audit logs were not found when the plugin is enabled." diff --git a/tests/integration/test_plugins.py b/tests/integration/test_plugins.py index ac5798723a..834a599ae2 100644 --- a/tests/integration/test_plugins.py +++ b/tests/integration/test_plugins.py @@ -6,19 +6,15 @@ import psycopg2 as psycopg2 import pytest as pytest from pytest_operator.plugin import OpsTest -from tenacity import Retrying, stop_after_delay, wait_fixed from .helpers import ( - APPLICATION_NAME, DATABASE_APP_NAME, build_and_deploy, db_connect, get_password, get_primary, get_unit_address, - run_command_on_unit, ) -from .new_relations.helpers import build_connection_string logger = logging.getLogger(__name__) @@ -90,7 +86,6 @@ "CREATE TABLE vector_test (id bigserial PRIMARY KEY, embedding vector(3));" ) TIMESCALEDB_EXTENSION_STATEMENT = "CREATE TABLE test_timescaledb (time TIMESTAMPTZ NOT NULL); SELECT create_hypertable('test_timescaledb', 'time');" -RELATION_ENDPOINT = "database" @pytest.mark.group(1) @@ -258,75 +253,3 @@ async def test_plugin_objects(ops_test: OpsTest) -> None: logger.info("Waiting for status to resolve again") async with ops_test.fast_forward(fast_interval="60s"): await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") - - -@pytest.mark.group(1) -async def test_audit_plugin(ops_test: OpsTest) -> None: - """Test the audit plugin.""" - await ops_test.model.deploy(APPLICATION_NAME) - await ops_test.model.relate(f"{APPLICATION_NAME}:{RELATION_ENDPOINT}", DATABASE_APP_NAME) - async with ops_test.fast_forward(): - await ops_test.model.wait_for_idle( - apps=[APPLICATION_NAME, DATABASE_APP_NAME], status="active" - ) - - # Check that the audit plugin is disabled. - connection_string = await build_connection_string( - ops_test, APPLICATION_NAME, RELATION_ENDPOINT - ) - connection = None - try: - connection = psycopg2.connect(connection_string) - with connection.cursor() as cursor: - cursor.execute("CREATE TABLE test1(value TEXT);") - cursor.execute("GRANT SELECT ON test1 TO PUBLIC;") - cursor.execute("SET TIME ZONE 'Europe/Rome';") - except Exception: - if connection is not None: - connection.close() - try: - primary = await get_primary(ops_test) - logs = await run_command_on_unit( - ops_test, - primary, - "grep AUDIT /var/log/postgresql/postgresql-*.log", - ) - except Exception: - pass - else: - logger.info(f"Logs: {logs}") - assert False, "Audit logs were found when the plugin is disabled." - - # Enable the audit plugin. - await ops_test.model.applications[DATABASE_APP_NAME].set_config({ - "plugin_audit_enable": "True" - }) - await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") - - # Check that the audit plugin is enabled. - try: - connection = psycopg2.connect(connection_string) - with connection.cursor() as cursor: - cursor.execute("CREATE TABLE test2(value TEXT);") - cursor.execute("GRANT SELECT ON test2 TO PUBLIC;") - cursor.execute("SET TIME ZONE 'Europe/Rome';") - except Exception: - if connection is not None: - connection.close() - for attempt in Retrying(stop=stop_after_delay(90), wait=wait_fixed(10), reraise=True): - with attempt: - try: - primary = await get_primary(ops_test) - logs = await run_command_on_unit( - ops_test, - primary, - "grep AUDIT /var/log/postgresql/postgresql-*.log", - ) - assert "MISC,BEGIN,,,BEGIN" in logs - assert ( - "DDL,CREATE TABLE,TABLE,public.test2,CREATE TABLE test2(value TEXT);" in logs - ) - assert "ROLE,GRANT,TABLE,,GRANT SELECT ON test2 TO PUBLIC;" in logs - assert "MISC,SET,,,SET TIME ZONE 'Europe/Rome';" in logs - except Exception: - assert False, "Audit logs were not found when the plugin is enabled." From 6cf142b23017e94bfce33c2d5096612756c27262 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 11 Sep 2024 11:50:58 -0300 Subject: [PATCH 5/9] Fix integration test Signed-off-by: Marcelo Henrique Neppel --- tests/integration/test_audit.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/integration/test_audit.py b/tests/integration/test_audit.py index 445cbb7dd3..d1298b192e 100644 --- a/tests/integration/test_audit.py +++ b/tests/integration/test_audit.py @@ -13,7 +13,6 @@ APPLICATION_NAME, DATABASE_APP_NAME, build_and_deploy, - get_primary, run_command_on_unit, ) from .new_relations.helpers import build_connection_string @@ -45,14 +44,13 @@ async def test_audit_plugin(ops_test: OpsTest) -> None: cursor.execute("CREATE TABLE test1(value TEXT);") cursor.execute("GRANT SELECT ON test1 TO PUBLIC;") cursor.execute("SET TIME ZONE 'Europe/Rome';") - except Exception: + finally: if connection is not None: connection.close() try: - primary = await get_primary(ops_test) logs = await run_command_on_unit( ops_test, - primary, + "postgresql-k8s/0", "grep AUDIT /var/log/postgresql/postgresql-*.log", ) except Exception: @@ -74,16 +72,15 @@ async def test_audit_plugin(ops_test: OpsTest) -> None: cursor.execute("CREATE TABLE test2(value TEXT);") cursor.execute("GRANT SELECT ON test2 TO PUBLIC;") cursor.execute("SET TIME ZONE 'Europe/Rome';") - except Exception: + finally: if connection is not None: connection.close() for attempt in Retrying(stop=stop_after_delay(90), wait=wait_fixed(10), reraise=True): with attempt: try: - primary = await get_primary(ops_test) logs = await run_command_on_unit( ops_test, - primary, + "postgresql-k8s/0", "grep AUDIT /var/log/postgresql/postgresql-*.log", ) assert "MISC,BEGIN,,,BEGIN" in logs From 97128b35779f260143c407712feb0e38d2c54621 Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 11 Sep 2024 12:40:33 -0300 Subject: [PATCH 6/9] Add unit tests Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_charm.py | 21 +++++++++++++++++++++ tests/unit/test_postgresql.py | 27 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index f8b7a154c2..2351bbbb84 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1778,3 +1778,24 @@ def test_create_pgdata(harness): "postgres:postgres", "/var/lib/postgresql/data", ]) + + +def test_get_plugins(harness): + with patch("charm.PostgresqlOperatorCharm._on_config_changed"): + # Test when the charm has no plugins enabled. + assert harness.charm.get_plugins() == [] + + # Test when the charm has some plugins enabled. + harness.update_config({ + "plugin_audit_enable": True, + "plugin_citext_enable": True, + "plugin_spi_enable": True, + }) + assert harness.charm.get_plugins() == [ + "pgaudit", + "citext", + "refint", + "autoinc", + "insert_username", + "moddatetime", + ] diff --git a/tests/unit/test_postgresql.py b/tests/unit/test_postgresql.py index dd07a6eff5..a4c30467d8 100644 --- a/tests/unit/test_postgresql.py +++ b/tests/unit/test_postgresql.py @@ -324,3 +324,30 @@ def test_build_postgresql_parameters(harness): parameters = harness.charm.postgresql.build_postgresql_parameters(config_options, 1000000000) assert "shared_buffers" not in parameters assert "effective_cache_size" not in parameters + + +def test_configure_pgaudit(harness): + with patch( + "charms.postgresql_k8s.v0.postgresql.PostgreSQL._connect_to_database" + ) as _connect_to_database: + # Test when pgAudit is enabled. + execute = ( + _connect_to_database.return_value.cursor.return_value.__enter__.return_value.execute + ) + harness.charm.postgresql._configure_pgaudit(True) + execute.assert_has_calls([ + call("ALTER SYSTEM SET pgaudit.log = 'ROLE,DDL,MISC,MISC_SET';"), + call("ALTER SYSTEM SET pgaudit.log_client TO off;"), + call("ALTER SYSTEM SET pgaudit.log_parameter TO off"), + call("SELECT pg_reload_conf();"), + ]) + + # Test when pgAudit is disabled. + execute.reset_mock() + harness.charm.postgresql._configure_pgaudit(False) + execute.assert_has_calls([ + call("ALTER SYSTEM RESET pgaudit.log;"), + call("ALTER SYSTEM RESET pgaudit.log_client;"), + call("ALTER SYSTEM RESET pgaudit.log_parameter;"), + call("SELECT pg_reload_conf();"), + ]) From 32437181df919b938509ed6e6081ee54e2f1f59c Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Mon, 16 Sep 2024 15:31:41 -0300 Subject: [PATCH 7/9] Enable pgAudit by default Signed-off-by: Marcelo Henrique Neppel --- config.yaml | 2 +- tests/integration/test_audit.py | 64 ++++++++++++++------------ tests/unit/test_charm.py | 2 +- tests/unit/test_db.py | 4 +- tests/unit/test_postgresql_provider.py | 2 +- 5 files changed, 39 insertions(+), 35 deletions(-) diff --git a/config.yaml b/config.yaml index 0df7ed8dfc..4e83398437 100644 --- a/config.yaml +++ b/config.yaml @@ -304,7 +304,7 @@ options: type: boolean description: Enable timescaledb extension plugin_audit_enable: - default: false + default: true type: boolean description: Enable pgAudit extension profile: diff --git a/tests/integration/test_audit.py b/tests/integration/test_audit.py index d1298b192e..cd61538715 100644 --- a/tests/integration/test_audit.py +++ b/tests/integration/test_audit.py @@ -33,39 +33,11 @@ async def test_audit_plugin(ops_test: OpsTest) -> None: apps=[APPLICATION_NAME, DATABASE_APP_NAME], status="active" ) - logger.info("Checking that the audit plugin is disabled") + logger.info("Checking that the audit plugin is enabled") connection_string = await build_connection_string( ops_test, APPLICATION_NAME, RELATION_ENDPOINT ) connection = None - try: - connection = psycopg2.connect(connection_string) - with connection.cursor() as cursor: - cursor.execute("CREATE TABLE test1(value TEXT);") - cursor.execute("GRANT SELECT ON test1 TO PUBLIC;") - cursor.execute("SET TIME ZONE 'Europe/Rome';") - finally: - if connection is not None: - connection.close() - try: - logs = await run_command_on_unit( - ops_test, - "postgresql-k8s/0", - "grep AUDIT /var/log/postgresql/postgresql-*.log", - ) - except Exception: - pass - else: - logger.info(f"Logs: {logs}") - assert False, "Audit logs were found when the plugin is disabled." - - logger.info("Enabling the audit plugin") - await ops_test.model.applications[DATABASE_APP_NAME].set_config({ - "plugin_audit_enable": "True" - }) - await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") - - logger.info("Checking that the audit plugin is enabled") try: connection = psycopg2.connect(connection_string) with connection.cursor() as cursor: @@ -75,12 +47,13 @@ async def test_audit_plugin(ops_test: OpsTest) -> None: finally: if connection is not None: connection.close() + unit_name = f"{DATABASE_APP_NAME}/0" for attempt in Retrying(stop=stop_after_delay(90), wait=wait_fixed(10), reraise=True): with attempt: try: logs = await run_command_on_unit( ops_test, - "postgresql-k8s/0", + unit_name, "grep AUDIT /var/log/postgresql/postgresql-*.log", ) assert "MISC,BEGIN,,,BEGIN" in logs @@ -91,3 +64,34 @@ async def test_audit_plugin(ops_test: OpsTest) -> None: assert "MISC,SET,,,SET TIME ZONE 'Europe/Rome';" in logs except Exception: assert False, "Audit logs were not found when the plugin is enabled." + + logger.info("Disabling the audit plugin") + await ops_test.model.applications[DATABASE_APP_NAME].set_config({ + "plugin_audit_enable": "False" + }) + await ops_test.model.wait_for_idle(apps=[DATABASE_APP_NAME], status="active") + + logger.info("Removing the previous logs") + await run_command_on_unit(ops_test, unit_name, "rm /var/log/postgresql/postgresql-*.log") + + logger.info("Checking that the audit plugin is disabled") + try: + connection = psycopg2.connect(connection_string) + with connection.cursor() as cursor: + cursor.execute("CREATE TABLE test1(value TEXT);") + cursor.execute("GRANT SELECT ON test1 TO PUBLIC;") + cursor.execute("SET TIME ZONE 'Europe/Rome';") + finally: + if connection is not None: + connection.close() + try: + logs = await run_command_on_unit( + ops_test, + unit_name, + "grep AUDIT /var/log/postgresql/postgresql-*.log", + ) + except Exception: + pass + else: + logger.info(f"Logs: {logs}") + assert False, "Audit logs were found when the plugin is disabled." diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 2351bbbb84..4077bfe44e 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1783,7 +1783,7 @@ def test_create_pgdata(harness): def test_get_plugins(harness): with patch("charm.PostgresqlOperatorCharm._on_config_changed"): # Test when the charm has no plugins enabled. - assert harness.charm.get_plugins() == [] + assert harness.charm.get_plugins() == ["pgaudit"] # Test when the charm has some plugins enabled. harness.update_config({ diff --git a/tests/unit/test_db.py b/tests/unit/test_db.py index 2e262428f1..ddcbec8390 100644 --- a/tests/unit/test_db.py +++ b/tests/unit/test_db.py @@ -234,7 +234,7 @@ def test_set_up_relation(harness): user = f"relation_id_{rel_id}" postgresql_mock.create_user.assert_called_once_with(user, "test-password", False) postgresql_mock.create_database.assert_called_once_with( - DATABASE, user, plugins=[], client_relations=[relation] + DATABASE, user, plugins=["pgaudit"], client_relations=[relation] ) assert postgresql_mock.get_postgresql_version.call_count == 1 _update_unit_status.assert_called_once() @@ -279,7 +279,7 @@ def test_set_up_relation(harness): assert harness.charm.legacy_db_relation.set_up_relation(relation) postgresql_mock.create_user.assert_called_once_with(user, "test-password", False) postgresql_mock.create_database.assert_called_once_with( - DATABASE, user, plugins=[], client_relations=[relation] + DATABASE, user, plugins=["pgaudit"], client_relations=[relation] ) assert postgresql_mock.get_postgresql_version.call_count == 1 _update_unit_status.assert_called_once() diff --git a/tests/unit/test_postgresql_provider.py b/tests/unit/test_postgresql_provider.py index 95fb20dfed..e441697599 100644 --- a/tests/unit/test_postgresql_provider.py +++ b/tests/unit/test_postgresql_provider.py @@ -113,7 +113,7 @@ def test_on_database_requested(harness): database_relation = harness.model.get_relation(RELATION_NAME) client_relations = [database_relation] postgresql_mock.create_database.assert_called_once_with( - DATABASE, user, plugins=[], client_relations=client_relations + DATABASE, user, plugins=["pgaudit"], client_relations=client_relations ) postgresql_mock.get_postgresql_version.assert_called_once() From 93dd1aceea0db27d299fd41f68415f058133f9fb Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 18 Sep 2024 15:22:19 -0300 Subject: [PATCH 8/9] Fix test_no_password_exposed_on_logs Signed-off-by: Marcelo Henrique Neppel --- tests/integration/test_password_rotation.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_password_rotation.py b/tests/integration/test_password_rotation.py index 618502054b..b2d4be63af 100644 --- a/tests/integration/test_password_rotation.py +++ b/tests/integration/test_password_rotation.py @@ -2,6 +2,7 @@ # Copyright 2022 Canonical Ltd. # See LICENSE file for licensing details. import json +import re import time import psycopg2 @@ -180,4 +181,8 @@ async def test_no_password_exposed_on_logs(ops_test: OpsTest) -> None: ) except Exception: continue - assert len(logs) == 0, f"Sensitive information detected on {unit.name} logs" + regex = re.compile("(PASSWORD )(?!)") + logs_without_false_positives = regex.findall(logs) + assert ( + len(logs_without_false_positives) == 0 + ), f"Sensitive information detected on {unit.name} logs" From 7d4e11ce70d0e4be670adf021379a89ed779f72e Mon Sep 17 00:00:00 2001 From: Marcelo Henrique Neppel Date: Wed, 18 Sep 2024 15:24:46 -0300 Subject: [PATCH 9/9] Enable/disable the plugin at the unit test Signed-off-by: Marcelo Henrique Neppel --- tests/unit/test_charm.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 4077bfe44e..e3b4ee514b 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -1799,3 +1799,13 @@ def test_get_plugins(harness): "insert_username", "moddatetime", ] + + # Test when the charm has the pgAudit plugin disabled. + harness.update_config({"plugin_audit_enable": False}) + assert harness.charm.get_plugins() == [ + "citext", + "refint", + "autoinc", + "insert_username", + "moddatetime", + ]