From 9a262da361020f52a5819c1ef2e55d9ede851c0a Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Thu, 11 Apr 2024 20:43:18 +1200 Subject: [PATCH] fix (tests): ensure that the leader is managing secrets (#356) ## Issue As of ops 2.10+ `Harness` applies the same access controls to secrets as Juju does in production. This causes tests to fail that were ignoring these access restrictions (deliberately - they have docstrings that call it out). ## Solution Set the unit to be leader in each of the applicable tests. It might be that you'd rather have tests for both cases, leader and (failing appropriately) non-leader - I can adjust to do that if you prefer. --------- Co-authored-by: Mehdi Bendriss --- requirements.txt | 4 ++-- tests/unit/test_charm.py | 47 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/requirements.txt b/requirements.txt index a4a2f819c..3f9e92f32 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ cosl==0.0.7 importlib-resources==5.10.2 tenacity==8.1.0 pymongo==4.3.3 -ops==2.4.1 +ops==2.12 jsonschema==4.17.3 cryptography==38.0.4 pure-sasl==0.6.2 @@ -16,4 +16,4 @@ pyyaml==6.0.1 zipp==3.11.0 pyOpenSSL==22.1.0 typing-extensions==4.5.0 -parameterized==0.9.0 \ No newline at end of file +parameterized==0.9.0 diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index ea61ffb6a..eb915952a 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -9,7 +9,13 @@ import pytest from charms.operator_libs_linux.v1 import snap -from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus +from ops.model import ( + ActiveStatus, + BlockedStatus, + MaintenanceStatus, + ModelError, + WaitingStatus, +) from ops.testing import Harness from parameterized import parameterized from pymongo.errors import ConfigurationError, ConnectionFailure, OperationFailure @@ -720,8 +726,8 @@ def test_get_password(self): self.harness.charm.get_secret("unit", "non-existing-secret") is None def test_set_reset_existing_password_app(self): - """NOTE: currently ops.testing seems to allow for non-leader to set secrets too!""" self._setup_secrets() + self.harness.set_leader(True) # Getting current password self.harness.charm.set_secret("app", "monitor-password", "bla") @@ -730,6 +736,14 @@ def test_set_reset_existing_password_app(self): self.harness.charm.set_secret("app", "monitor-password", "blablabla") assert self.harness.charm.get_secret("app", "monitor-password") == "blablabla" + def test_set_reset_existing_password_app_nonleader(self): + self._setup_secrets() + self.harness.set_leader(False) + + # Getting current password + with self.assertRaises(ModelError): + self.harness.charm.set_secret("app", "monitor-password", "bla") + @parameterized.expand([("app"), ("unit")]) def test_set_secret_returning_secret_id(self, scope): secret_id = self.harness.charm.set_secret(scope, "somekey", "bla") @@ -737,7 +751,9 @@ def test_set_secret_returning_secret_id(self, scope): @parameterized.expand([("app"), ("unit")]) def test_set_reset_new_secret(self, scope): - """NOTE: currently ops.testing seems to allow for non-leader to set secrets too!""" + if scope == "app": + self.harness.set_leader(True) + # Getting current password self.harness.charm.set_secret(scope, "new-secret", "bla") assert self.harness.charm.get_secret(scope, "new-secret") == "bla" @@ -750,6 +766,22 @@ def test_set_reset_new_secret(self, scope): self.harness.charm.set_secret(scope, "new-secret2", "blablabla") assert self.harness.charm.get_secret(scope, "new-secret2") == "blablabla" + def test_set_reset_new_secret_non_leader(self): + self.harness.set_leader(True) + + # Getting current password + self.harness.charm.set_secret("app", "new-secret", "bla") + assert self.harness.charm.get_secret("app", "new-secret") == "bla" + + # Reset new secret + self.harness.set_leader(False) + with self.assertRaises(ModelError): + self.harness.charm.set_secret("app", "new-secret", "blablabla") + + # Set another new secret + with self.assertRaises(ModelError): + self.harness.charm.set_secret("app", "new-secret2", "blablabla") + @parameterized.expand([("app"), ("unit")]) def test_invalid_secret(self, scope): with self.assertRaises(TypeError): @@ -760,8 +792,8 @@ def test_invalid_secret(self, scope): @pytest.mark.usefixtures("use_caplog") def test_delete_password(self): - """NOTE: currently ops.testing seems to allow for non-leader to remove secrets too!""" self._setup_secrets() + self.harness.set_leader(True) assert self.harness.charm.get_secret("app", "monitor-password") self.harness.charm.remove_secret("app", "monitor-password") @@ -796,6 +828,13 @@ def test_delete_password(self): in self._caplog.text ) + def test_delete_password_non_leader(self): + self._setup_secrets() + self.harness.set_leader(False) + assert self.harness.charm.get_secret("app", "monitor-password") + with self.assertRaises(ModelError): + self.harness.charm.remove_secret("app", "monitor-password") + @parameterized.expand([("app"), ("unit")]) @patch("charm.MongodbOperatorCharm._connect_mongodb_exporter") def test_on_secret_changed(self, scope, connect_exporter):