From 4a86948ccd204242d759e6fe92bafab016f15d9d Mon Sep 17 00:00:00 2001 From: Sylvan Le Deunff Date: Fri, 8 Nov 2024 11:43:48 +0100 Subject: [PATCH] Make default mount point prefix configurable, and allow empty prefix Closes-Bug: #2086767 Change-Id: I669584ea93fd410c705b8e5d7476c79a61c96053 --- manila/share/api.py | 16 ++++++-- manila/share/share_types.py | 6 +-- manila/tests/share/test_api.py | 38 ++++++++++++++++++- ...064907-allow-empty-mount-point-prefix.yaml | 6 +++ 4 files changed, 58 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/bug-2064907-allow-empty-mount-point-prefix.yaml diff --git a/manila/share/api.py b/manila/share/api.py index 7566b81ed9..7fc5c7d1f0 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -58,6 +58,11 @@ share_api_opts = [ 'CreateFromSnapshotFilter is enabled and to have hosts ' 'reporting replication_domain option.' ), + cfg.StrOpt('default_mount_point_prefix', + default='{project_id}_', + help='Default prefix that will be used if none is provided' + 'through share_type extra specs. Prefix will only be' + 'used if share_type support mount_point_name.'), cfg.BoolOpt('is_deferred_deletion_enabled', default=False, help='Whether to delete shares and share snapshots in a ' @@ -1192,13 +1197,16 @@ class API(base.Base): mount_point_name=None): prefix = share_type.get('extra_specs').get( constants.ExtraSpecs.PROVISIONING_MOUNT_POINT_PREFIX) - prefix = prefix or context.project_id - prefix = prefix.format(context.to_dict()) - mount_point_name = f"{prefix}_{mount_point_name}" + if prefix is None: + prefix = CONF.default_mount_point_prefix + + mount_point_name_template = f"{prefix}{mount_point_name}" + mount_point_name = mount_point_name_template.format( + **context.to_dict()) if mount_point_name and ( not re.match( - r'^[a-zA-Z0-9_]*$', mount_point_name) + r'^[a-zA-Z0-9_-]*$', mount_point_name) or len(mount_point_name) > 255 ): msg = _("Invalid mount_point_name: %s") diff --git a/manila/share/share_types.py b/manila/share/share_types.py index 891ad93afe..55bb6c62f6 100644 --- a/manila/share/share_types.py +++ b/manila/share/share_types.py @@ -330,8 +330,8 @@ def is_valid_csv(extra_spec_value): return all([v.strip() for v in values]) -def is_valid_string(v): - return isinstance(v, str) and len(v) in range(1, 256) +def is_valid_string(v, min_length=1, max_length=256): + return isinstance(v, str) and min_length <= len(v) and len(v) < max_length def sanitize_csv(csv_string): @@ -365,7 +365,7 @@ def is_valid_optional_extra_spec(key, value): elif key == constants.ExtraSpecs.AVAILABILITY_ZONES: return is_valid_csv(value) elif key == constants.ExtraSpecs.PROVISIONING_MOUNT_POINT_PREFIX: - return is_valid_string(value) + return is_valid_string(value, min_length=0) elif key in [constants.ExtraSpecs.PROVISIONING_MAX_SHARE_SIZE, constants.ExtraSpecs.PROVISIONING_MIN_SHARE_SIZE, constants.ExtraSpecs.PROVISIONING_MAX_SHARE_EXTEND_SIZE]: diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index d664051bfc..1ae55c9b66 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -971,10 +971,33 @@ class ShareAPITestCase(test.TestCase): availability_zones=az, mount_point_name='fake_mp') + def test_configure_default_prefix(self): + share_type = {'extra_specs': {}} + conf = dict(DEFAULT=dict(default_mount_point_prefix="manila_")) + with test_utils.create_temp_config_with_opts(conf): + self.context.project_id = 'project_id' + mount_point_name = 'mount_point' + result = self.api._prefix_mount_point_name( + share_type, self.context, mount_point_name + ) + self.assertEqual(result, 'manila_mount_point') + + def test_configure_empty_default_prefix(self): + share_type = {'extra_specs': {}} + conf = dict(DEFAULT=dict(default_mount_point_prefix="")) + with test_utils.create_temp_config_with_opts(conf): + self.context.project_id = 'project_id' + mount_point_name = 'mount_point' + result = self.api._prefix_mount_point_name( + share_type, self.context, mount_point_name + ) + self.assertEqual(result, 'mount_point') + def test_prefix_with_valid_mount_point_name(self): share_type = { 'extra_specs': { - constants.ExtraSpecs.PROVISIONING_MOUNT_POINT_PREFIX: 'prefix', + constants.ExtraSpecs.PROVISIONING_MOUNT_POINT_PREFIX: + 'prefix_', } } self.context.project_id = 'project_id' @@ -984,6 +1007,19 @@ class ShareAPITestCase(test.TestCase): ) self.assertEqual(result, 'prefix_mount_point') + def test_empty_prefix_with_valid_mount_point_name(self): + share_type = { + 'extra_specs': { + constants.ExtraSpecs.PROVISIONING_MOUNT_POINT_PREFIX: '', + } + } + self.context.project_id = 'project_id' + mount_point_name = 'mount_point' + result = self.api._prefix_mount_point_name( + share_type, self.context, mount_point_name + ) + self.assertEqual(result, 'mount_point') + def test_prefix_with_valid_missing_extra_spec_mount_point_name(self): share_type = { 'extra_specs': {}, diff --git a/releasenotes/notes/bug-2064907-allow-empty-mount-point-prefix.yaml b/releasenotes/notes/bug-2064907-allow-empty-mount-point-prefix.yaml new file mode 100644 index 0000000000..de4c54bdcb --- /dev/null +++ b/releasenotes/notes/bug-2064907-allow-empty-mount-point-prefix.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Allow using an empty prefix for shares created with mount_point_name. + - | + Allow configuring default mount_point_name prefix through an option.