From 8ef5ccfcaacfdc7cbddd63e3347d53c7f0d3e7d0 Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Fri, 12 Apr 2024 12:57:16 +0800 Subject: [PATCH 1/4] feat(Harness): support user secrets (#1176) Add support for user secrets in Harness. --- ops/testing.py | 89 +++++++++++++++++++++++++++++++++++++++++-- test/test_testing.py | 90 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 173 insertions(+), 6 deletions(-) diff --git a/ops/testing.py b/ops/testing.py index 6af417978..505088fe5 100644 --- a/ops/testing.py +++ b/ops/testing.py @@ -1537,6 +1537,54 @@ def add_model_secret(self, owner: AppUnitOrName, content: Dict[str, str]) -> str model.Secret._validate_content(content) return self._backend._secret_add(content, owner_name) + def add_user_secret(self, content: Dict[str, str]) -> str: + """Add a secret owned by the user, simulating the ``juju add-secret`` command. + + Args: + content: A key-value mapping containing the payload of the secret, + for example :code:`{"password": "foo123"}`. + + Return: + The ID of the newly-added secret. + + Example usage (the parameter ``harness`` in the test function is + a pytest fixture that does setup/teardown, see :class:`Harness`):: + + # charmcraft.yaml + config: + options: + mysec: + type: secret + description: "tell me your secrets" + + # charm.py + class MyVMCharm(ops.CharmBase): + def __init__(self, framework: ops.Framework): + super().__init__(framework) + framework.observe(self.on.config_changed, self._on_config_changed) + + def _on_config_changed(self, event: ops.ConfigChangedEvent): + mysec = self.config.get('mysec') + if mysec: + sec = self.model.get_secret(id=mysec, label="mysec") + self.config_from_secret = sec.get_content() + + # test_charm.py + def test_config_changed(harness): + secret_content = {'password': 'foo'} + secret_id = harness.add_user_secret(secret_content) + harness.grant_secret(secret_id, 'test-charm') + harness.begin() + harness.update_config({'mysec': secret_id}) + secret = harness.model.get_secret(id=secret_id).get_content() + assert harness.charm.config_from_secret == secret.get_content() + + """ + model.Secret._validate_content(content) + # Although it's named a user-owned secret in Juju, technically, the owner is the + # Model, so the secret's owner is set to `Model.uuid`. + return self._backend._secret_add(content, self.model.uuid) + def _ensure_secret(self, secret_id: str) -> '_Secret': secret = self._backend._get_secret(secret_id) if secret is None: @@ -1566,6 +1614,9 @@ def set_secret_content(self, secret_id: str, content: Dict[str, str]): def grant_secret(self, secret_id: str, observer: AppUnitOrName): """Grant read access to this secret for the given observer application or unit. + For user secrets, grant access to the application, simulating the + ``juju grant-secret`` command. + If the given application or unit has already been granted access to this secret, do nothing. @@ -1577,10 +1628,17 @@ def grant_secret(self, secret_id: str, observer: AppUnitOrName): under test must already have been created. """ secret = self._ensure_secret(secret_id) + app_or_unit_name = _get_app_or_unit_name(observer) + + # User secrets: + if secret.owner_name == self.model.uuid: + secret.user_secrets_grants.add(app_or_unit_name) + return + + # Model secrets: if secret.owner_name in [self.model.app.name, self.model.unit.name]: raise RuntimeError(f'Secret {secret_id!r} owned by the charm under test, "' f"can't call grant_secret") - app_or_unit_name = _get_app_or_unit_name(observer) relation_id = self._secret_relation_id_to(secret) if relation_id not in secret.grants: secret.grants[relation_id] = set() @@ -1600,10 +1658,18 @@ def revoke_secret(self, secret_id: str, observer: AppUnitOrName): test must have already been created. """ secret = self._ensure_secret(secret_id) + app_or_unit_name = _get_app_or_unit_name(observer) + + # User secrets: + if secret.owner_name == self.model.uuid: + secret.user_secrets_grants.discard(app_or_unit_name) + return + + # Model secrets: if secret.owner_name in [self.model.app.name, self.model.unit.name]: raise RuntimeError(f'Secret {secret_id!r} owned by the charm under test, "' f"can't call revoke_secret") - app_or_unit_name = _get_app_or_unit_name(observer) + relation_id = self._secret_relation_id_to(secret) if relation_id not in secret.grants: return @@ -1650,6 +1716,8 @@ def trigger_secret_rotation(self, secret_id: str, *, label: Optional[str] = None label is used. """ secret = self._ensure_secret(secret_id) + if secret.owner_name == self.model.uuid: + raise RuntimeError("Cannot trigger the secret-rotate event for a user secret.") if label is None: label = secret.label self.charm.on.secret_rotate.emit(secret_id, label) @@ -1690,6 +1758,8 @@ def trigger_secret_expiration(self, secret_id: str, revision: int, *, label is used. """ secret = self._ensure_secret(secret_id) + if secret.owner_name == self.model.uuid: + raise RuntimeError("Cannot trigger the secret-expired event for a user secret.") if label is None: label = secret.label self.charm.on.secret_expired.emit(secret_id, label, revision) @@ -2113,6 +2183,7 @@ class _Secret: description: Optional[str] = None tracked: int = 1 grants: Dict[int, Set[str]] = dataclasses.field(default_factory=dict) + user_secrets_grants: Set[str] = dataclasses.field(default_factory=set) @_copy_docstrings(model._ModelBackend) @@ -2535,8 +2606,14 @@ def secret_get(self, *, peek: bool = False) -> Dict[str, str]: secret = self._ensure_secret_id_or_label(id, label) - # Check that caller has permission to get this secret - if secret.owner_name not in [self.app_name, self.unit_name]: + if secret.owner_name == self.model_uuid: + # This is a user secret - charms only ever have view access. + if self.app_name not in secret.user_secrets_grants: + raise model.SecretNotFoundError( + f'Secret {id!r} not granted access to {self.app_name!r}') + elif secret.owner_name not in [self.app_name, self.unit_name]: + # This is a model secret - the model might have admin or view access. + # Check that caller has permission to get this secret # Observer is calling: does secret have a grant on relation between # this charm (the observer) and the secret owner's app? owner_app = secret.owner_name.split('/')[0] @@ -2572,6 +2649,10 @@ def _ensure_secret_owner(self, secret: _Secret): # secrets, the leader has manage permissions and other units only have # view permissions. # https://discourse.charmhub.io/t/secret-access-permissions/12627 + # For user secrets the secret owner is the model, that is, + # `secret.owner_name == self.model.uuid`, only model admins have + # manage permissions: https://juju.is/docs/juju/secret. + unit_secret = secret.owner_name == self.unit_name app_secret = secret.owner_name == self.app_name diff --git a/test/test_testing.py b/test/test_testing.py index 8521d1096..a276adb4c 100644 --- a/test/test_testing.py +++ b/test/test_testing.py @@ -1103,8 +1103,7 @@ def test_config_secret_option(self): ''') self.addCleanup(harness.cleanup) harness.begin() - # [jam] I don't think this is right, as user-secrets aren't owned by the app - secret_id = harness.add_model_secret('mycharm', {'key': 'value'}) + secret_id = harness.add_user_secret({'key': 'value'}) harness.update_config(key_values={'a': secret_id}) self.assertEqual(harness.charm.changes, [{'name': 'config-changed', 'data': {'a': secret_id}}]) @@ -5100,6 +5099,17 @@ def test_trigger_secret_rotation(self): with self.assertRaises(RuntimeError): harness.trigger_secret_rotation('nosecret') + def test_trigger_secret_rotation_on_user_secret(self): + harness = ops.testing.Harness(EventRecorder, meta='name: database') + self.addCleanup(harness.cleanup) + + secret_id = harness.add_user_secret({'foo': 'bar'}) + assert secret_id is not None + harness.begin() + + with self.assertRaises(RuntimeError): + harness.trigger_secret_rotation(secret_id) + def test_trigger_secret_removal(self): harness = ops.testing.Harness(EventRecorder, meta='name: database') self.addCleanup(harness.cleanup) @@ -5156,6 +5166,17 @@ def test_trigger_secret_expiration(self): with self.assertRaises(RuntimeError): harness.trigger_secret_removal('nosecret', 1) + def test_trigger_secret_expiration_on_user_secret(self): + harness = ops.testing.Harness(EventRecorder, meta='name: database') + self.addCleanup(harness.cleanup) + + secret_id = harness.add_user_secret({'foo': 'bar'}) + assert secret_id is not None + harness.begin() + + with self.assertRaises(RuntimeError): + harness.trigger_secret_expiration(secret_id, 1) + def test_secret_permissions_unit(self): harness = ops.testing.Harness(ops.CharmBase, meta='name: database') self.addCleanup(harness.cleanup) @@ -5202,6 +5223,71 @@ def test_secret_permissions_nonleader(self): with self.assertRaises(ops.model.SecretNotFoundError): secret.remove_all_revisions() + def test_add_user_secret(self): + harness = ops.testing.Harness(ops.CharmBase, meta=yaml.safe_dump( + {'name': 'webapp'} + )) + self.addCleanup(harness.cleanup) + harness.begin() + + secret_content = {'password': 'foo'} + secret_id = harness.add_user_secret(secret_content) + harness.grant_secret(secret_id, 'webapp') + + secret = harness.model.get_secret(id=secret_id) + self.assertEqual(secret.id, secret_id) + self.assertEqual(secret.get_content(), secret_content) + + def test_get_user_secret_without_grant(self): + harness = ops.testing.Harness(ops.CharmBase, meta=yaml.safe_dump( + {'name': 'webapp'} + )) + self.addCleanup(harness.cleanup) + harness.begin() + secret_id = harness.add_user_secret({'password': 'foo'}) + with self.assertRaises(ops.SecretNotFoundError): + harness.model.get_secret(id=secret_id) + + def test_revoke_user_secret(self): + harness = ops.testing.Harness(ops.CharmBase, meta=yaml.safe_dump( + {'name': 'webapp'} + )) + self.addCleanup(harness.cleanup) + harness.begin() + + secret_content = {'password': 'foo'} + secret_id = harness.add_user_secret(secret_content) + harness.grant_secret(secret_id, 'webapp') + harness.revoke_secret(secret_id, 'webapp') + with self.assertRaises(ops.SecretNotFoundError): + harness.model.get_secret(id=secret_id) + + def test_set_user_secret_content(self): + harness = ops.testing.Harness(EventRecorder, meta=yaml.safe_dump( + {'name': 'webapp'} + )) + self.addCleanup(harness.cleanup) + harness.begin() + secret_id = harness.add_user_secret({'password': 'foo'}) + harness.grant_secret(secret_id, 'webapp') + secret = harness.model.get_secret(id=secret_id) + self.assertEqual(secret.get_content(), {'password': 'foo'}) + harness.set_secret_content(secret_id, {'password': 'bar'}) + secret = harness.model.get_secret(id=secret_id) + self.assertEqual(secret.get_content(refresh=True), {'password': 'bar'}) + + def test_get_user_secret_info(self): + harness = ops.testing.Harness(EventRecorder, meta=yaml.safe_dump( + {'name': 'webapp'} + )) + self.addCleanup(harness.cleanup) + harness.begin() + secret_id = harness.add_user_secret({'password': 'foo'}) + harness.grant_secret(secret_id, 'webapp') + secret = harness.model.get_secret(id=secret_id) + with self.assertRaises(ops.SecretNotFoundError): + secret.get_info() + class EventRecorder(ops.CharmBase): def __init__(self, framework: ops.Framework): From 614b77b25aab2d1d0cf42e03e1639a305da80bcd Mon Sep 17 00:00:00 2001 From: Tony Meyer Date: Fri, 12 Apr 2024 16:59:58 +1200 Subject: [PATCH 2/4] chore: bump docs dependencies (#1190) As suggested in the [moderate security PR](https://github.com/canonical/operator/pull/1188#issuecomment-2050959323) for idna. Generated with Python 3.11 because that's what `.readthedocs.yaml` specifies. Freshly generated with `pip-compile`. --- docs/requirements.txt | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/docs/requirements.txt b/docs/requirements.txt index 7004242e4..7faba991a 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -1,10 +1,10 @@ # -# This file is autogenerated by pip-compile with Python 3.8 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # # pip-compile --extra=docs --output-file=docs/requirements.txt pyproject.toml # -alabaster==0.7.13 +alabaster==0.7.16 # via sphinx babel==2.14.0 # via sphinx @@ -15,7 +15,7 @@ beautifulsoup4==4.12.3 # pyspelling bracex==2.4 # via wcmatch -canonical-sphinx-extensions==0.0.19 +canonical-sphinx-extensions==0.0.20 # via ops (pyproject.toml) certifi==2024.2.2 # via requests @@ -33,14 +33,10 @@ furo==2024.1.29 # via ops (pyproject.toml) html5lib==1.1 # via pyspelling -idna==3.6 +idna==3.7 # via requests imagesize==1.4.1 # via sphinx -importlib-metadata==7.1.0 - # via - # markdown - # sphinx jinja2==3.1.3 # via # myst-parser @@ -49,7 +45,7 @@ linkify-it-py==2.0.3 # via ops (pyproject.toml) livereload==2.6.3 # via sphinx-autobuild -lxml==5.1.0 +lxml==5.2.1 # via pyspelling markdown==3.6 # via pyspelling @@ -74,8 +70,6 @@ pygments==2.17.2 # sphinx-tabs pyspelling==2.10 # via ops (pyproject.toml) -pytz==2024.1 - # via babel pyyaml==6.0.1 # via # myst-parser @@ -109,7 +103,7 @@ sphinx==6.2.1 # sphinx-tabs # sphinxcontrib-jquery # sphinxext-opengraph -sphinx-autobuild==2021.3.14 +sphinx-autobuild==2024.2.4 # via ops (pyproject.toml) sphinx-basic-ng==1.0.0b2 # via furo @@ -121,19 +115,19 @@ sphinx-notfound-page==1.0.0 # via ops (pyproject.toml) sphinx-tabs==3.4.5 # via ops (pyproject.toml) -sphinxcontrib-applehelp==1.0.4 +sphinxcontrib-applehelp==1.0.8 # via sphinx -sphinxcontrib-devhelp==1.0.2 +sphinxcontrib-devhelp==1.0.6 # via sphinx -sphinxcontrib-htmlhelp==2.0.1 +sphinxcontrib-htmlhelp==2.0.5 # via sphinx sphinxcontrib-jquery==4.1 # via ops (pyproject.toml) sphinxcontrib-jsmath==1.0.1 # via sphinx -sphinxcontrib-qthelp==1.0.3 +sphinxcontrib-qthelp==1.0.7 # via sphinx -sphinxcontrib-serializinghtml==1.1.5 +sphinxcontrib-serializinghtml==1.1.10 # via sphinx sphinxext-opengraph==0.9.1 # via ops (pyproject.toml) @@ -149,5 +143,3 @@ webencodings==0.5.1 # via html5lib websocket-client==1.7.0 # via ops (pyproject.toml) -zipp==3.18.1 - # via importlib-metadata From dbb843ebf9a91cf040ed8900e8a8b7f870501a82 Mon Sep 17 00:00:00 2001 From: Ben Hoyt Date: Fri, 12 Apr 2024 17:14:43 +1200 Subject: [PATCH 3/4] fix: revert support of change-update notice due to Juju reversion (#1189) This reverts #1170 (commit b7e5ba33e7a18022b56535a4d3d84363e1091f5b) as this feature has been reverted in Juju on the 3.5 branch, due to the issues described at this PR: https://github.com/juju/juju/pull/17191 --- ops/__init__.py | 2 -- ops/charm.py | 14 +--------- ops/pebble.py | 10 ------- ops/testing.py | 13 ++++----- test/charms/test_main/src/charm.py | 10 ------- test/test_charm.py | 33 +--------------------- test/test_main.py | 10 ------- test/test_testing.py | 45 +----------------------------- 8 files changed, 9 insertions(+), 128 deletions(-) diff --git a/ops/__init__.py b/ops/__init__.py index 77c6b82c4..60de7ad4a 100644 --- a/ops/__init__.py +++ b/ops/__init__.py @@ -66,7 +66,6 @@ 'LeaderSettingsChangedEvent', 'MetadataLinks', 'PayloadMeta', - 'PebbleChangeUpdatedEvent', 'PebbleCustomNoticeEvent', 'PebbleNoticeEvent', 'PebbleReadyEvent', @@ -209,7 +208,6 @@ LeaderSettingsChangedEvent, MetadataLinks, PayloadMeta, - PebbleChangeUpdatedEvent, PebbleCustomNoticeEvent, PebbleNoticeEvent, PebbleReadyEvent, diff --git a/ops/charm.py b/ops/charm.py index aeb97c79c..8ed3f7f16 100644 --- a/ops/charm.py +++ b/ops/charm.py @@ -34,7 +34,7 @@ cast, ) -from ops import model, pebble +from ops import model from ops._private import yaml from ops.framework import ( EventBase, @@ -809,15 +809,6 @@ class PebbleCustomNoticeEvent(PebbleNoticeEvent): """Event triggered when a Pebble notice of type "custom" is created or repeats.""" -class PebbleChangeUpdatedEvent(PebbleNoticeEvent): - """Event triggered when a Pebble notice of type "change-update" is created or repeats.""" - - def get_change(self) -> pebble.Change: - """Get the Pebble change associated with this event.""" - change_id = pebble.ChangeID(self.notice.key) - return self.workload.pebble.get_change(change_id) - - class SecretEvent(HookEvent): """Base class for all secret events.""" @@ -1197,9 +1188,6 @@ def __init__(self, framework: Framework): container_name = container_name.replace('-', '_') self.on.define_event(f"{container_name}_pebble_ready", PebbleReadyEvent) self.on.define_event(f"{container_name}_pebble_custom_notice", PebbleCustomNoticeEvent) - self.on.define_event( - f"{container_name}_pebble_change_updated", - PebbleChangeUpdatedEvent) @property def app(self) -> model.Application: diff --git a/ops/pebble.py b/ops/pebble.py index 0053bc257..638885f18 100644 --- a/ops/pebble.py +++ b/ops/pebble.py @@ -1314,17 +1314,7 @@ def __repr__(self): class NoticeType(enum.Enum): """Enum of notice types.""" - CHANGE_UPDATE = 'change-update' - """Recorded whenever a change is updated, that is, when it is first - spawned or its status was updated. The key for change-update notices is - the change ID. - """ - CUSTOM = 'custom' - """A custom notice reported via the Pebble client API or ``pebble notify``. - The key and data fields are provided by the user. The key must be in - the format ``example.com/path`` to ensure well-namespaced notice keys. - """ class NoticesUsers(enum.Enum): diff --git a/ops/testing.py b/ops/testing.py index 505088fe5..e9f2c73ca 100644 --- a/ops/testing.py +++ b/ops/testing.py @@ -1168,13 +1168,8 @@ def pebble_notify(self, container_name: str, key: str, *, id, new_or_repeated = client._notify(type, key, data=data, repeat_after=repeat_after) - if self._charm is not None and new_or_repeated: - if type == pebble.NoticeType.CUSTOM: - self.charm.on[container_name].pebble_custom_notice.emit( - container, id, type.value, key) - elif type == pebble.NoticeType.CHANGE_UPDATE: - self.charm.on[container_name].pebble_change_updated.emit( - container, id, type.value, key) + if self._charm is not None and type == pebble.NoticeType.CUSTOM and new_or_repeated: + self.charm.on[container_name].pebble_custom_notice.emit(container, id, type.value, key) return id @@ -3450,6 +3445,10 @@ def _notify(self, type: pebble.NoticeType, key: str, *, Return a tuple of (notice_id, new_or_repeated). """ + if type != pebble.NoticeType.CUSTOM: + message = f'invalid type "{type.value}" (can only add "custom" notices)' + raise self._api_error(400, message) + # The shape of the code below is taken from State.AddNotice in Pebble. now = datetime.datetime.now(tz=datetime.timezone.utc) diff --git a/test/charms/test_main/src/charm.py b/test/charms/test_main/src/charm.py index d10eb09ce..ccab5d62a 100755 --- a/test/charms/test_main/src/charm.py +++ b/test/charms/test_main/src/charm.py @@ -59,7 +59,6 @@ def __init__(self, *args: typing.Any): on_collect_metrics=[], on_test_pebble_ready=[], on_test_pebble_custom_notice=[], - on_test_pebble_change_updated=[], on_log_critical_action=[], on_log_error_action=[], @@ -93,8 +92,6 @@ def __init__(self, *args: typing.Any): self.framework.observe(self.on.test_pebble_ready, self._on_test_pebble_ready) self.framework.observe(self.on.test_pebble_custom_notice, self._on_test_pebble_custom_notice) - self.framework.observe(self.on.test_pebble_change_updated, - self._on_test_pebble_change_updated) self.framework.observe(self.on.secret_remove, self._on_secret_remove) self.framework.observe(self.on.secret_rotate, self._on_secret_rotate) @@ -200,13 +197,6 @@ def _on_test_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): self._stored.observed_event_types.append(type(event).__name__) self._stored.test_pebble_custom_notice_data = event.snapshot() - def _on_test_pebble_change_updated(self, event: ops.PebbleChangeUpdatedEvent): - assert event.workload is not None - assert isinstance(event.notice, ops.LazyNotice) - self._stored.on_test_pebble_change_updated.append(type(event).__name__) - self._stored.observed_event_types.append(type(event).__name__) - self._stored.test_pebble_change_updated_data = event.snapshot() - def _on_start_action(self, event: ops.ActionEvent): assert event.handle.kind == 'start_action', ( 'event action name cannot be different from the one being handled') diff --git a/test/test_charm.py b/test/test_charm.py index a7392ee74..b471b37f0 100644 --- a/test/test_charm.py +++ b/test/test_charm.py @@ -18,14 +18,12 @@ import tempfile import typing import unittest -import unittest.mock from pathlib import Path import yaml import ops import ops.charm -from ops import pebble from ops.model import ModelError, _ModelBackend from ops.storage import SQLiteStorage @@ -356,19 +354,7 @@ def _on_stor4_attach(self, event: ops.StorageAttachedEvent): 'StorageAttachedEvent', ]) - @unittest.mock.patch('ops.pebble.Client.get_change') - def test_workload_events(self, mock_get_change: unittest.mock.MagicMock): - def get_change(change_id: pebble.ChangeID) -> pebble.Change: - return pebble.Change.from_dict({ - 'id': pebble.ChangeID(change_id), - 'kind': 'exec', - 'ready': False, - 'spawn-time': '2021-01-28T14:37:02.247202105+13:00', - 'status': 'Doing', - 'summary': 'Exec command "foo"', - }) - - mock_get_change.side_effect = get_change + def test_workload_events(self): class MyCharm(ops.CharmBase): def __init__(self, *args: typing.Any): @@ -383,10 +369,6 @@ def __init__(self, *args: typing.Any): self.on[workload].pebble_custom_notice, self.on_any_pebble_custom_notice, ) - self.framework.observe( - self.on[workload].pebble_change_updated, - self.on_any_pebble_change_updated, - ) def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): self.seen.append(type(event).__name__) @@ -394,12 +376,6 @@ def on_any_pebble_ready(self, event: ops.PebbleReadyEvent): def on_any_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): self.seen.append(type(event).__name__) - def on_any_pebble_change_updated(self, event: ops.PebbleChangeUpdatedEvent): - self.seen.append(type(event).__name__) - change = event.get_change() - assert change.id == event.notice.key - assert change.kind == 'exec' - # language=YAML self.meta = ops.CharmMeta.from_yaml(metadata=''' name: my-charm @@ -423,18 +399,11 @@ def on_any_pebble_change_updated(self, event: ops.PebbleChangeUpdatedEvent): charm.on['containerb'].pebble_custom_notice.emit( charm.framework.model.unit.get_container('containerb'), '2', 'custom', 'y') - charm.on['container-a'].pebble_change_updated.emit( - charm.framework.model.unit.get_container('container-a'), '1', 'change-update', '42') - charm.on['containerb'].pebble_change_updated.emit( - charm.framework.model.unit.get_container('containerb'), '2', 'change-update', '42') - self.assertEqual(charm.seen, [ 'PebbleReadyEvent', 'PebbleReadyEvent', 'PebbleCustomNoticeEvent', 'PebbleCustomNoticeEvent', - 'PebbleChangeUpdatedEvent', - 'PebbleChangeUpdatedEvent', ]) def test_relations_meta(self): diff --git a/test/test_main.py b/test/test_main.py index 4747f982b..c22d0b56c 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -608,16 +608,6 @@ def test_multiple_events_handled(self): 'notice_id': '123', 'notice_type': 'custom', 'notice_key': 'example.com/a'}, - ), ( - EventSpec(ops.PebbleChangeUpdatedEvent, 'test_pebble_change_updated', - workload_name='test', - notice_id='456', - notice_type='change-update', - notice_key='42'), - {'container_name': 'test', - 'notice_id': '456', - 'notice_type': 'change-update', - 'notice_key': '42'}, ), ( EventSpec(ops.SecretChangedEvent, 'secret_changed', secret_id='secret:12345', diff --git a/test/test_testing.py b/test/test_testing.py index a276adb4c..b99863086 100644 --- a/test/test_testing.py +++ b/test/test_testing.py @@ -1106,7 +1106,7 @@ def test_config_secret_option(self): secret_id = harness.add_user_secret({'key': 'value'}) harness.update_config(key_values={'a': secret_id}) self.assertEqual(harness.charm.changes, - [{'name': 'config-changed', 'data': {'a': secret_id}}]) + [{'name': 'config-changed', 'data': {'a': secret_id}}]) def test_no_config_option_type(self): with self.assertRaises(RuntimeError): @@ -3222,8 +3222,6 @@ def observe_container_events(self, container_name: str): self.framework.observe(self.on[container_name].pebble_ready, self._on_pebble_ready) self.framework.observe(self.on[container_name].pebble_custom_notice, self._on_pebble_custom_notice) - self.framework.observe(self.on[container_name].pebble_change_updated, - self._on_pebble_change_updated) def _on_pebble_ready(self, event: ops.PebbleReadyEvent): self.changes.append({ @@ -3242,17 +3240,6 @@ def _on_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): 'notice_key': event.notice.key, }) - def _on_pebble_change_updated(self, event: ops.PebbleChangeUpdatedEvent): - type_str = (event.notice.type.value if isinstance(event.notice.type, pebble.NoticeType) - else event.notice.type) - self.changes.append({ - 'name': 'pebble-change-updated', - 'container': event.workload.name, - 'notice_id': event.notice.id, - 'notice_type': type_str, - 'notice_key': event.notice.key, - }) - def get_public_methods(obj: object): """Get the public attributes of obj to compare to another object.""" @@ -5863,36 +5850,6 @@ def _on_pebble_custom_notice(self, event: ops.PebbleCustomNoticeEvent): self.assertNotEqual(id, '') self.assertEqual(num_notices, 0) - def test_notify_change_update(self): - harness = ops.testing.Harness(ContainerEventCharm, meta=""" - name: notifier - containers: - foo: - resource: foo-image - """) - self.addCleanup(harness.cleanup) - harness.begin() - harness.charm.observe_container_events('foo') - - id1 = harness.pebble_notify('foo', '42', type=pebble.NoticeType.CHANGE_UPDATE) - id2 = harness.pebble_notify('foo', '43', type=pebble.NoticeType.CHANGE_UPDATE) - self.assertNotEqual(id1, id2) - - expected_changes = [{ - 'name': 'pebble-change-updated', - 'container': 'foo', - 'notice_id': id1, - 'notice_type': 'change-update', - 'notice_key': '42', - }, { - 'name': 'pebble-change-updated', - 'container': 'foo', - 'notice_id': id2, - 'notice_type': 'change-update', - 'notice_key': '43', - }] - self.assertEqual(harness.charm.changes, expected_changes) - class PebbleNoticesMixin: client: ops.pebble.Client From ac21f475e6de0e43a3f5eedf0d181c8b076ae8ac Mon Sep 17 00:00:00 2001 From: Tiexin Guo Date: Mon, 15 Apr 2024 12:18:36 +0800 Subject: [PATCH 4/4] docs: update docstrings with deprecated directive (#1178) Update docstrings to use sphinx `deprecated` directive. --- ops/charm.py | 13 ++++++++----- ops/lib/__init__.py | 19 +++++++++++-------- ops/model.py | 20 ++++++++++++++------ 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/ops/charm.py b/ops/charm.py index 8ed3f7f16..f4369e5d2 100644 --- a/ops/charm.py +++ b/ops/charm.py @@ -387,11 +387,12 @@ class LeaderElectedEvent(HookEvent): class LeaderSettingsChangedEvent(HookEvent): - """DEPRECATED. Event triggered when leader changes any settings. + """Event triggered when leader changes any settings. - This event has been deprecated in favor of using a Peer relation, - and having the leader set a value in the Application data bag for - that peer relation. (see :class:`RelationChangedEvent`). + .. deprecated:: 2.4.0 + This event has been deprecated in favor of using a Peer relation, + and having the leader set a value in the Application data bag for + that peer relation. (See :class:`RelationChangedEvent`.) """ @@ -1085,8 +1086,10 @@ class CharmEvents(ObjectEvents): """Triggered when a new leader has been elected (see :class:`LeaderElectedEvent`).""" leader_settings_changed = EventSource(LeaderSettingsChangedEvent) - """DEPRECATED. Triggered when leader changes any settings (see + """Triggered when leader changes any settings (see :class:`LeaderSettingsChangedEvent`). + + .. deprecated: 2.4.0 """ collect_metrics = EventSource(CollectMetricsEvent) diff --git a/ops/lib/__init__.py b/ops/lib/__init__.py index dde350159..0ea46c361 100644 --- a/ops/lib/__init__.py +++ b/ops/lib/__init__.py @@ -14,9 +14,10 @@ """Infrastructure for the opslib functionality. -DEPRECATED: The ops.lib functionality is deprecated, and is superseded by -charm libraries (https://juju.is/docs/sdk/library) and regular Python imports. -We now prefer to do version selection at build (charmcraft pack) time. +.. deprecated:: 2.1.0 + The ops.lib functionality is deprecated, and is superseded by + charm libraries (https://juju.is/docs/sdk/library) and regular Python imports. + We now prefer to do version selection at build (charmcraft pack) time. """ import logging @@ -48,9 +49,6 @@ def use(name: str, api: int, author: str) -> ModuleType: """Use a library from the ops libraries. - DEPRECATED: This function is deprecated. Prefer charm libraries instead - (https://juju.is/docs/sdk/library). - Args: name: the name of the library requested. api: the API version of the library. @@ -61,6 +59,10 @@ def use(name: str, api: int, author: str) -> ModuleType: ImportError: if the library cannot be found. TypeError: if the name, api, or author are the wrong type. ValueError: if the name, api, or author are invalid. + + .. deprecated:: 2.1.0 + This function is deprecated. Prefer charm libraries instead + (https://juju.is/docs/sdk/library). """ warnings.warn("ops.lib is deprecated, prefer charm libraries instead", category=DeprecationWarning) @@ -101,8 +103,9 @@ def autoimport(): otherwise changed in the current run, and the changes need to be seen. Otherwise libraries are found on first call of `use`. - DEPRECATED: This function is deprecated. Prefer charm libraries instead - (https://juju.is/docs/sdk/library). + .. deprecated:: 2.1.0 + This function is deprecated. Prefer charm libraries instead + (https://juju.is/docs/sdk/library). """ warnings.warn("ops.lib is deprecated, prefer charm libraries instead", category=DeprecationWarning) diff --git a/ops/model.py b/ops/model.py index 3a3d42317..2ccdc33ea 100644 --- a/ops/model.py +++ b/ops/model.py @@ -177,10 +177,11 @@ def storages(self) -> 'StorageMapping': def pod(self) -> 'Pod': """Represents the definition of a pod spec in legacy Kubernetes models. - DEPRECATED: New charms should use the sidecar pattern with Pebble. - Use :meth:`Pod.set_spec` to set the container specification for legacy Kubernetes charms. + + .. deprecated:: 2.4.0 + New charms should use the sidecar pattern with Pebble. """ return self._pod @@ -766,7 +767,12 @@ class Port: """The port number. Will be ``None`` if protocol is ``'icmp'``.""" -OpenedPort = Port # Alias for backwards compatibility. +OpenedPort = Port +"""Alias to Port for backwards compatibility. + +.. deprecated:: 2.7.0 + Use :class:`Port` instead. +""" class LazyMapping(Mapping[str, str], ABC): @@ -1904,10 +1910,12 @@ def fetch(self, name: str) -> Path: class Pod: """Represents the definition of a pod spec in legacy Kubernetes models. - DEPRECATED: New charms should use the sidecar pattern with Pebble. - Currently only supports simple access to setting the Juju pod spec via :attr:`.set_spec`. + + .. deprecated:: 2.4.0 + + New charms should use the sidecar pattern with Pebble. """ def __init__(self, backend: '_ModelBackend'): @@ -1998,7 +2006,7 @@ def index(self) -> int: @property def id(self) -> int: - """DEPRECATED. Use :attr:`Storage.index` instead.""" + """.. deprecated:: 2.4.0 Use :attr:`Storage.index` instead.""" logger.warning("model.Storage.id is being replaced - please use model.Storage.index") return self.index