From 164f79da9f1e82e3a4688811e9835369c9752db4 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Fri, 2 Feb 2024 13:18:53 -0500 Subject: [PATCH 1/3] feat: Add support for devmode confinement --- lib/charms/operator_libs_linux/v2/snap.py | 53 +++++++++++++++++++---- tests/unit/test_snap.py | 31 +++++++++++++ 2 files changed, 75 insertions(+), 9 deletions(-) diff --git a/lib/charms/operator_libs_linux/v2/snap.py b/lib/charms/operator_libs_linux/v2/snap.py index 38c88cf..dc674cd 100644 --- a/lib/charms/operator_libs_linux/v2/snap.py +++ b/lib/charms/operator_libs_linux/v2/snap.py @@ -83,7 +83,7 @@ # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 3 +LIBPATCH = 4 # Regex to locate 7-bit C1 ANSI sequences @@ -214,7 +214,7 @@ class Snap(object): - state: a `SnapState` representation of its install status - channel: "stable", "candidate", "beta", and "edge" are common - revision: a string representing the snap's revision - - confinement: "classic" or "strict" + - confinement: "classic", "strict", or "devmode" """ def __init__( @@ -475,6 +475,8 @@ def _install( args = [] if self.confinement == "classic": args.append("--classic") + if self.confinement == "devmode": + args.append("--devmode") if channel: args.append('--channel="{}"'.format(channel)) if revision: @@ -489,6 +491,7 @@ def _refresh( channel: Optional[str] = "", cohort: Optional[str] = "", revision: Optional[str] = None, + devmode: Optional[bool] = False, leave_cohort: Optional[bool] = False, ) -> None: """Refresh a snap. @@ -497,6 +500,7 @@ def _refresh( channel: the channel to install from cohort: optionally, specify a cohort. revision: optionally, specify the revision of the snap to refresh + devmode: optionally, specify devmode confinement leave_cohort: leave the current cohort. """ args = [] @@ -506,6 +510,9 @@ def _refresh( if revision: args.append('--revision="{}"'.format(revision)) + if devmode: + args.append("--devmode") + if not cohort: cohort = self._cohort @@ -530,6 +537,7 @@ def ensure( self, state: SnapState, classic: Optional[bool] = False, + devmode: Optional[bool] = False, channel: Optional[str] = "", cohort: Optional[str] = "", revision: Optional[str] = None, @@ -539,6 +547,7 @@ def ensure( Args: state: a `SnapState` to reconcile to. classic: an (Optional) boolean indicating whether classic confinement should be used + devmode: an (Optional) boolean indicating whether devmode confinement should be used channel: the channel to install from cohort: optional. Specify the key of a snap cohort. revision: optional. the revision of the snap to install/refresh @@ -549,7 +558,15 @@ def ensure( Raises: SnapError if an error is encountered """ - self._confinement = "classic" if classic or self._confinement == "classic" else "" + if classic and devmode: + raise ValueError("Cannot set both classic and devmode confinement") + + if classic or self._confinement == "classic": + self._confinement = "classic" + elif devmode or self._confinement == "devmode": + self._confinement = "devmode" + else: + self._confinement = "" if state not in (SnapState.Present, SnapState.Latest): # We are attempting to remove this snap. @@ -566,7 +583,7 @@ def ensure( self._install(channel, cohort, revision) else: # The snap is installed, but we are changing it (e.g., switching channels). - self._refresh(channel, cohort, revision) + self._refresh(channel, cohort, revision, devmode) self._update_snap_apps() self._state = state @@ -892,6 +909,7 @@ def add( state: Union[str, SnapState] = SnapState.Latest, channel: Optional[str] = "", classic: Optional[bool] = False, + devmode: Optional[bool] = False, cohort: Optional[str] = "", revision: Optional[str] = None, ) -> Union[Snap, List[Snap]]: @@ -904,6 +922,8 @@ def add( channel: an (Optional) channel as a string. Defaults to 'latest' classic: an (Optional) boolean specifying whether it should be added with classic confinement. Default `False` + devmode: an (Optional) boolean specifying whether it should be added with devmode + confinement. Default `False` cohort: an (Optional) string specifying the snap cohort to use revision: an (Optional) string specifying the snap revision to use @@ -920,7 +940,7 @@ def add( if isinstance(state, str): state = SnapState(state) - return _wrap_snap_operations(snap_names, state, channel, classic, cohort, revision) + return _wrap_snap_operations(snap_names, state, channel, classic, devmode, cohort, revision) @_cache_init @@ -937,7 +957,7 @@ def remove(snap_names: Union[str, List[str]]) -> Union[Snap, List[Snap]]: if not snap_names: raise TypeError("Expected at least one snap to add, received zero!") - return _wrap_snap_operations(snap_names, SnapState.Absent, "", False) + return _wrap_snap_operations(snap_names, SnapState.Absent, "", False, False) @_cache_init @@ -946,6 +966,7 @@ def ensure( state: str, channel: Optional[str] = "", classic: Optional[bool] = False, + devmode: Optional[bool] = False, cohort: Optional[str] = "", revision: Optional[int] = None, ) -> Union[Snap, List[Snap]]: @@ -957,6 +978,8 @@ def ensure( channel: an (Optional) channel as a string. Defaults to 'latest' classic: an (Optional) boolean specifying whether it should be added with classic confinement. Default `False` + devmode: an (Optional) boolean specifying whether it should be added with devmode + confinement. Default `False` cohort: an (Optional) string specifying the snap cohort to use revision: an (Optional) integer specifying the snap revision to use @@ -970,7 +993,7 @@ def ensure( channel = "latest" if state in ("present", "latest") or revision: - return add(snap_names, SnapState(state), channel, classic, cohort, revision) + return add(snap_names, SnapState(state), channel, classic, devmode, cohort, revision) else: return remove(snap_names) @@ -980,6 +1003,7 @@ def _wrap_snap_operations( state: SnapState, channel: str, classic: bool, + devmode: bool, cohort: Optional[str] = "", revision: Optional[str] = None, ) -> Union[Snap, List[Snap]]: @@ -995,7 +1019,12 @@ def _wrap_snap_operations( snap.ensure(state=SnapState.Absent) else: snap.ensure( - state=state, classic=classic, channel=channel, cohort=cohort, revision=revision + state=state, + classic=classic, + devmode=devmode, + channel=channel, + cohort=cohort, + revision=revision, ) snaps["success"].append(snap) except SnapError as e: @@ -1014,13 +1043,17 @@ def _wrap_snap_operations( def install_local( - filename: str, classic: Optional[bool] = False, dangerous: Optional[bool] = False + filename: str, + classic: Optional[bool] = False, + devmode: Optional[bool] = False, + dangerous: Optional[bool] = False, ) -> Snap: """Perform a snap operation. Args: filename: the path to a local .snap file to install classic: whether to use classic confinement + devmode: whether to use devmode confinement dangerous: whether --dangerous should be passed to install snaps without a signature Raises: @@ -1033,6 +1066,8 @@ def install_local( ] if classic: args.append("--classic") + if devmode: + args.append("--devmode") if dangerous: args.append("--dangerous") try: diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index eca7ac9..accc9f0 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -290,6 +290,37 @@ def test_can_run_snap_commands(self, mock_subprocess): ["snap", "install", "foo", "--classic", '--revision="123"'], universal_newlines=True ) + @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") + def test_can_run_snap_commands_devmode(self, mock_subprocess): + mock_subprocess.return_value = 0 + foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "devmode") + self.assertEqual(foo.present, True) + + foo.ensure(snap.SnapState.Absent) + mock_subprocess.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) + + foo.ensure(snap.SnapState.Latest, devmode=True, channel="latest/edge") + + mock_subprocess.assert_called_with( + [ + "snap", + "install", + "foo", + "--devmode", + '--channel="latest/edge"', + ], + universal_newlines=True, + ) + self.assertEqual(foo.latest, True) + + foo.state = snap.SnapState.Absent + mock_subprocess.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) + + foo.ensure(snap.SnapState.Latest, revision=123) + mock_subprocess.assert_called_with( + ["snap", "install", "foo", "--devmode", '--revision="123"'], universal_newlines=True + ) + @patch("charms.operator_libs_linux.v2.snap.subprocess.run") def test_can_run_snap_daemon_commands(self, mock_subprocess): mock_subprocess.return_value = MagicMock() From aa6180be44f4390fa10eb8f4ee27d1c0acc97535 Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Tue, 6 Feb 2024 15:01:45 -0500 Subject: [PATCH 2/3] chore: Use bool instead of optional[bool] --- lib/charms/operator_libs_linux/v2/snap.py | 29 ++++++++++++++++------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/charms/operator_libs_linux/v2/snap.py b/lib/charms/operator_libs_linux/v2/snap.py index dc674cd..871ff5d 100644 --- a/lib/charms/operator_libs_linux/v2/snap.py +++ b/lib/charms/operator_libs_linux/v2/snap.py @@ -491,7 +491,7 @@ def _refresh( channel: Optional[str] = "", cohort: Optional[str] = "", revision: Optional[str] = None, - devmode: Optional[bool] = False, + devmode: bool = False, leave_cohort: Optional[bool] = False, ) -> None: """Refresh a snap. @@ -537,7 +537,7 @@ def ensure( self, state: SnapState, classic: Optional[bool] = False, - devmode: Optional[bool] = False, + devmode: bool = False, channel: Optional[str] = "", cohort: Optional[str] = "", revision: Optional[str] = None, @@ -583,7 +583,7 @@ def ensure( self._install(channel, cohort, revision) else: # The snap is installed, but we are changing it (e.g., switching channels). - self._refresh(channel, cohort, revision, devmode) + self._refresh(channel=channel, cohort=cohort, revision=revision, devmode=devmode) self._update_snap_apps() self._state = state @@ -909,7 +909,7 @@ def add( state: Union[str, SnapState] = SnapState.Latest, channel: Optional[str] = "", classic: Optional[bool] = False, - devmode: Optional[bool] = False, + devmode: bool = False, cohort: Optional[str] = "", revision: Optional[str] = None, ) -> Union[Snap, List[Snap]]: @@ -956,8 +956,13 @@ def remove(snap_names: Union[str, List[str]]) -> Union[Snap, List[Snap]]: snap_names = [snap_names] if isinstance(snap_names, str) else snap_names if not snap_names: raise TypeError("Expected at least one snap to add, received zero!") - - return _wrap_snap_operations(snap_names, SnapState.Absent, "", False, False) + return _wrap_snap_operations( + snap_names=snap_names, + state=SnapState.Absent, + channel="", + classic=False, + devmode=False, + ) @_cache_init @@ -966,7 +971,7 @@ def ensure( state: str, channel: Optional[str] = "", classic: Optional[bool] = False, - devmode: Optional[bool] = False, + devmode: bool = False, cohort: Optional[str] = "", revision: Optional[int] = None, ) -> Union[Snap, List[Snap]]: @@ -993,7 +998,15 @@ def ensure( channel = "latest" if state in ("present", "latest") or revision: - return add(snap_names, SnapState(state), channel, classic, devmode, cohort, revision) + return add( + snap_names=snap_names, + state=SnapState(state), + channel=channel, + classic=classic, + devmode=devmode, + cohort=cohort, + revision=revision, + ) else: return remove(snap_names) From 1e5cb23406c643bee2f9aed687ad90e244aef4eb Mon Sep 17 00:00:00 2001 From: Guillaume Belanger Date: Tue, 6 Feb 2024 15:02:39 -0500 Subject: [PATCH 3/3] chore: Fix test name --- tests/unit/test_snap.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_snap.py b/tests/unit/test_snap.py index accc9f0..e1e1a82 100644 --- a/tests/unit/test_snap.py +++ b/tests/unit/test_snap.py @@ -291,17 +291,17 @@ def test_can_run_snap_commands(self, mock_subprocess): ) @patch("charms.operator_libs_linux.v2.snap.subprocess.check_output") - def test_can_run_snap_commands_devmode(self, mock_subprocess): - mock_subprocess.return_value = 0 + def test_can_run_snap_commands_devmode(self, mock_check_output): + mock_check_output.return_value = 0 foo = snap.Snap("foo", snap.SnapState.Present, "stable", "1", "devmode") self.assertEqual(foo.present, True) foo.ensure(snap.SnapState.Absent) - mock_subprocess.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) + mock_check_output.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) foo.ensure(snap.SnapState.Latest, devmode=True, channel="latest/edge") - mock_subprocess.assert_called_with( + mock_check_output.assert_called_with( [ "snap", "install", @@ -314,10 +314,10 @@ def test_can_run_snap_commands_devmode(self, mock_subprocess): self.assertEqual(foo.latest, True) foo.state = snap.SnapState.Absent - mock_subprocess.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) + mock_check_output.assert_called_with(["snap", "remove", "foo"], universal_newlines=True) foo.ensure(snap.SnapState.Latest, revision=123) - mock_subprocess.assert_called_with( + mock_check_output.assert_called_with( ["snap", "install", "foo", "--devmode", '--revision="123"'], universal_newlines=True )