Skip to content

Commit

Permalink
fix (tests): ensure that the leader is managing secrets (#356)
Browse files Browse the repository at this point in the history
## 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 <bendrissmehdi@gmail.com>
  • Loading branch information
tonyandrewmeyer and Mehdi-Bendriss committed Apr 11, 2024
1 parent ee9f23c commit 9a262da
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 6 deletions.
4 changes: 2 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
parameterized==0.9.0
47 changes: 43 additions & 4 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
Expand All @@ -730,14 +736,24 @@ 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")
assert re.match(f"mongodb.{scope}", secret_id)

@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"
Expand All @@ -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):
Expand All @@ -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")
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 9a262da

Please sign in to comment.