From b4001966a6a55dfd4c1d6c0d91be5ceae42989fe Mon Sep 17 00:00:00 2001 From: John Mulligan Date: Wed, 8 May 2024 10:45:05 -0400 Subject: [PATCH] mgr/smb: move a handful of small functions to utils.py Includes updating the files that were using them and even a few test functions. Amusingly, the `one` function was being tested as part of test_handler.py demonstrating the confusion that not having a utils.py and corresponding test file brought. Signed-off-by: John Mulligan --- src/pybind/mgr/smb/handler.py | 21 ++--------- src/pybind/mgr/smb/internal.py | 2 +- src/pybind/mgr/smb/module.py | 15 ++++++-- src/pybind/mgr/smb/proto.py | 25 ------------- src/pybind/mgr/smb/resources.py | 3 +- src/pybind/mgr/smb/results.py | 3 +- src/pybind/mgr/smb/tests/test_handler.py | 25 ------------- src/pybind/mgr/smb/tests/test_utils.py | 28 +++++++++++++++ src/pybind/mgr/smb/utils.py | 46 ++++++++++++++++++++++++ 9 files changed, 94 insertions(+), 74 deletions(-) create mode 100644 src/pybind/mgr/smb/tests/test_utils.py create mode 100644 src/pybind/mgr/smb/utils.py diff --git a/src/pybind/mgr/smb/handler.py b/src/pybind/mgr/smb/handler.py index cc799517ba6..84702e72f78 100644 --- a/src/pybind/mgr/smb/handler.py +++ b/src/pybind/mgr/smb/handler.py @@ -13,8 +13,6 @@ from typing import ( ) import logging -import random -import string import time from ceph.deployment.service_spec import SMBSpec @@ -46,10 +44,10 @@ from .proto import ( OrchSubmitter, PathResolver, Simplified, - checked, ) from .resources import SMBResource from .results import ErrorResult, Result, ResultGroup +from .utils import checked, ynbool ClusterRef = Union[resources.Cluster, resources.RemovedCluster] ShareRef = Union[resources.Share, resources.RemovedShare] @@ -956,11 +954,6 @@ def _ug_refs(cluster: resources.Cluster) -> Collection[str]: } -def _ynbool(value: bool) -> str: - """Convert a bool to an smb.conf compatible string.""" - return 'Yes' if value else 'No' - - def _generate_share( share: resources.Share, resolver: PathResolver, cephx_entity: str ) -> Dict[str, Dict[str, str]]: @@ -988,8 +981,8 @@ def _generate_share( 'ceph:config_file': '/etc/ceph/ceph.conf', 'ceph:filesystem': share.cephfs.volume, 'ceph:user_id': cephx_entity, - 'read only': _ynbool(share.readonly), - 'browseable': _ynbool(share.browseable), + 'read only': ynbool(share.readonly), + 'browseable': ynbool(share.browseable), 'kernel share modes': 'no', 'x:ceph:id': f'{share.cluster_id}.{share.share_id}', } @@ -1251,11 +1244,3 @@ def _cephx_data_entity(cluster_id: str) -> str: use for data access. """ return f'client.smb.fs.cluster.{cluster_id}' - - -def rand_name(prefix: str, max_len: int = 18, suffix_len: int = 8) -> str: - trunc = prefix[: (max_len - suffix_len)] - suffix = ''.join( - random.choice(string.ascii_lowercase) for _ in range(suffix_len) - ) - return f'{trunc}{suffix}' diff --git a/src/pybind/mgr/smb/internal.py b/src/pybind/mgr/smb/internal.py index 1b2554317ec..3571ed44400 100644 --- a/src/pybind/mgr/smb/internal.py +++ b/src/pybind/mgr/smb/internal.py @@ -12,10 +12,10 @@ from .proto import ( EntryKey, Self, Simplifiable, - one, ) from .resources import SMBResource from .results import ErrorResult +from .utils import one T = TypeVar('T') diff --git a/src/pybind/mgr/smb/module.py b/src/pybind/mgr/smb/module.py index d8133a59c47..70c97d1a975 100644 --- a/src/pybind/mgr/smb/module.py +++ b/src/pybind/mgr/smb/module.py @@ -6,7 +6,16 @@ import orchestrator from ceph.deployment.service_spec import PlacementSpec, SMBSpec from mgr_module import MgrModule, Option -from . import cli, fs, handler, mon_store, rados_store, resources, results +from . import ( + cli, + fs, + handler, + mon_store, + rados_store, + resources, + results, + utils, +) from .enums import AuthMode, JoinSourceType, UserGroupSourceType from .proto import AccessAuthorizer, Simplified @@ -116,7 +125,7 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): 'a domain join username & password value' ' must contain a "%" separator' ) - rname = handler.rand_name(cluster_id) + rname = utils.rand_name(cluster_id) join_sources.append( resources.JoinSource( source_type=JoinSourceType.RESOURCE, @@ -156,7 +165,7 @@ class Module(orchestrator.OrchestratorClientMixin, MgrModule): for unpw in define_user_pass or []: username, password = unpw.split('%', 1) users.append({'name': username, 'password': password}) - rname = handler.rand_name(cluster_id) + rname = utils.rand_name(cluster_id) user_group_settings.append( resources.UserGroupSource( source_type=UserGroupSourceType.RESOURCE, ref=rname diff --git a/src/pybind/mgr/smb/proto.py b/src/pybind/mgr/smb/proto.py index 6af80866cff..ffcc647a48e 100644 --- a/src/pybind/mgr/smb/proto.py +++ b/src/pybind/mgr/smb/proto.py @@ -9,7 +9,6 @@ from typing import ( List, Optional, Tuple, - TypeVar, ) import sys @@ -155,27 +154,3 @@ class AccessAuthorizer(Protocol): self, volume: str, entity: str, caps: str = '' ) -> None: ... # pragma: no cover - - -T = TypeVar('T') - - -# TODO: move to a utils.py -def one(lst: List[T]) -> T: - if len(lst) != 1: - raise ValueError("list does not contain exactly one element") - return lst[0] - - -class IsNoneError(ValueError): - pass - - -def checked(v: Optional[T]) -> T: - """Ensures the provided value is not a None or raises a IsNoneError. - Intended use is similar to an `assert v is not None` but more usable in - one-liners and list/dict/etc comprehensions. - """ - if v is None: - raise IsNoneError('value is None') - return v diff --git a/src/pybind/mgr/smb/resources.py b/src/pybind/mgr/smb/resources.py index 290758c578e..842b4387855 100644 --- a/src/pybind/mgr/smb/resources.py +++ b/src/pybind/mgr/smb/resources.py @@ -16,7 +16,8 @@ from .enums import ( LoginCategory, UserGroupSourceType, ) -from .proto import Self, Simplified, checked +from .proto import Self, Simplified +from .utils import checked def _get_intent(data: Simplified) -> Intent: diff --git a/src/pybind/mgr/smb/results.py b/src/pybind/mgr/smb/results.py index 77bda1e6a29..b62d6e66377 100644 --- a/src/pybind/mgr/smb/results.py +++ b/src/pybind/mgr/smb/results.py @@ -2,8 +2,9 @@ from typing import Iterable, Iterator, List, Optional import errno -from .proto import Simplified, one +from .proto import Simplified from .resources import SMBResource +from .utils import one _DOMAIN = 'domain' diff --git a/src/pybind/mgr/smb/tests/test_handler.py b/src/pybind/mgr/smb/tests/test_handler.py index 2eab2a1cd91..93395c75225 100644 --- a/src/pybind/mgr/smb/tests/test_handler.py +++ b/src/pybind/mgr/smb/tests/test_handler.py @@ -574,14 +574,6 @@ def test_apply_no_matching_cluster_error(thandler): assert not rg.success -def test_one(): - assert smb.proto.one(['a']) == 'a' - with pytest.raises(ValueError): - smb.proto.one([]) - with pytest.raises(ValueError): - smb.proto.one(['a', 'b']) - - def test_apply_full_cluster_create(thandler): to_apply = [ smb.resources.JoinAuth( @@ -1259,23 +1251,6 @@ def test_apply_cluster_bad_linked_auth(thandler): assert rs['results'][1]['msg'] == 'join auth linked to different cluster' -def test_rand_name(): - name = smb.handler.rand_name('bob') - assert name.startswith('bob') - assert len(name) == 11 - name = smb.handler.rand_name('carla') - assert name.startswith('carla') - assert len(name) == 13 - name = smb.handler.rand_name('dangeresque') - assert name.startswith('dangeresqu') - assert len(name) == 18 - name = smb.handler.rand_name('fhqwhgadsfhqwhgadsfhqwhgads') - assert name.startswith('fhqwhgadsf') - assert len(name) == 18 - name = smb.handler.rand_name('') - assert len(name) == 8 - - def test_apply_with_create_only(thandler): test_apply_full_cluster_create(thandler) diff --git a/src/pybind/mgr/smb/tests/test_utils.py b/src/pybind/mgr/smb/tests/test_utils.py new file mode 100644 index 00000000000..5017990e9b1 --- /dev/null +++ b/src/pybind/mgr/smb/tests/test_utils.py @@ -0,0 +1,28 @@ +import pytest + +import smb.utils + + +def test_one(): + assert smb.utils.one(['a']) == 'a' + with pytest.raises(ValueError): + smb.utils.one([]) + with pytest.raises(ValueError): + smb.utils.one(['a', 'b']) + + +def test_rand_name(): + name = smb.utils.rand_name('bob') + assert name.startswith('bob') + assert len(name) == 11 + name = smb.utils.rand_name('carla') + assert name.startswith('carla') + assert len(name) == 13 + name = smb.utils.rand_name('dangeresque') + assert name.startswith('dangeresqu') + assert len(name) == 18 + name = smb.utils.rand_name('fhqwhgadsfhqwhgadsfhqwhgads') + assert name.startswith('fhqwhgadsf') + assert len(name) == 18 + name = smb.utils.rand_name('') + assert len(name) == 8 diff --git a/src/pybind/mgr/smb/utils.py b/src/pybind/mgr/smb/utils.py new file mode 100644 index 00000000000..2646815f112 --- /dev/null +++ b/src/pybind/mgr/smb/utils.py @@ -0,0 +1,46 @@ +"""Assorted utility functions for smb mgr module.""" +from typing import List, Optional, TypeVar + +import random +import string + +T = TypeVar('T') + + +def one(lst: List[T]) -> T: + """Given a list, ensure that the list contains exactly one item and return + it. A ValueError will be raised in the case that the list does not contain + exactly one item. + """ + if len(lst) != 1: + raise ValueError("list does not contain exactly one element") + return lst[0] + + +class IsNoneError(ValueError): + """A ValueError subclass raised by ``checked`` function.""" + + pass + + +def checked(v: Optional[T]) -> T: + """Ensures the provided value is not a None or raises a IsNoneError. + Intended use is similar to an `assert v is not None` but more usable in + one-liners and list/dict/etc comprehensions. + """ + if v is None: + raise IsNoneError('value is None') + return v + + +def ynbool(value: bool) -> str: + """Convert a bool to an smb.conf-style boolean string.""" + return 'Yes' if value else 'No' + + +def rand_name(prefix: str, max_len: int = 18, suffix_len: int = 8) -> str: + trunc = prefix[: (max_len - suffix_len)] + suffix = ''.join( + random.choice(string.ascii_lowercase) for _ in range(suffix_len) + ) + return f'{trunc}{suffix}' -- 2.39.5