diff --git a/src/charm.py b/src/charm.py index d245948306..ee3ddcbc44 100755 --- a/src/charm.py +++ b/src/charm.py @@ -39,13 +39,14 @@ from charms.postgresql_k8s.v0.postgresql_tls import PostgreSQLTLS from charms.rolling_ops.v0.rollingops import RollingOpsManager, RunWithLock from charms.tempo_coordinator_k8s.v0.charm_tracing import trace_charm -from ops import main +from ops import JujuVersion, main from ops.charm import ( ActionEvent, HookEvent, InstallEvent, LeaderElectedEvent, RelationDepartedEvent, + SecretRemoveEvent, StartEvent, ) from ops.framework import EventBase @@ -203,6 +204,7 @@ def __init__(self, *args): self.framework.observe(self.on.set_password_action, self._on_set_password) self.framework.observe(self.on.promote_to_primary_action, self._on_promote_to_primary) self.framework.observe(self.on.update_status, self._on_update_status) + self.framework.observe(self.on.secret_remove, self._on_secret_remove) self.cluster_name = self.app.name self._member_name = self.unit.name.replace("/", "-") @@ -440,6 +442,24 @@ def get_hostname_by_unit(self, _) -> str: # the underlying provider (LXD, MAAS, etc.), the unit IP is returned. return self._unit_ip + def _on_secret_remove(self, event: SecretRemoveEvent) -> None: + if self.model.juju_version < JujuVersion("3.6.11"): + logger.warning( + "Skipping secret revision removal due to https://github.com/juju/juju/issues/20782" + ) + return + + # A secret removal (entire removal, not just a revision removal) causes + # https://github.com/juju/juju/issues/20794. This check is to avoid the + # errors that would happen if we tried to remove the revision in that case + # (in the revision removal, the label is present). + if event.secret.label is None: + logger.debug("Secret with no label cannot be removed") + return + + logger.debug(f"Removing secret with label {event.secret.label} revision {event.revision}") + event.remove_revision() + def _on_get_primary(self, event: ActionEvent) -> None: """Get primary instance.""" try: diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 217116f9dc..7bb321221f 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -183,6 +183,7 @@ async def test_postgresql_locales(ops_test: OpsTest) -> None: assert locales == SNAP_LOCALES +@pytest.mark.abort_on_fail async def test_postgresql_parameters_change(ops_test: OpsTest) -> None: """Test that's possible to change PostgreSQL parameters.""" await ops_test.model.applications[DATABASE_APP_NAME].set_config({ diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index d1a524e226..1747cf08a2 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -15,7 +15,7 @@ PostgreSQLEnableDisableExtensionError, PostgreSQLUpdateUserPasswordError, ) -from ops import Unit +from ops import JujuVersion, Unit from ops.framework import EventBase from ops.model import ( ActiveStatus, @@ -2933,3 +2933,27 @@ def test_relations_user_databases_map(harness): "replication": "all", "rewind": "all", } + + +def test_on_secret_remove(harness, only_with_juju_secrets): + with ( + patch("ops.model.Model.juju_version", new_callable=PropertyMock) as _juju_version, + ): + event = Mock() + + # New juju + _juju_version.return_value = JujuVersion("3.6.11") + harness.charm._on_secret_remove(event) + event.remove_revision.assert_called_once_with() + event.reset_mock() + + # Old juju + _juju_version.return_value = JujuVersion("3.6.9") + harness.charm._on_secret_remove(event) + assert not event.remove_revision.called + event.reset_mock() + + # No secret + event.secret.label = None + harness.charm._on_secret_remove(event) + assert not event.remove_revision.called