Merge "Make default mount point prefix configurable, and allow empty prefix"

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")

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]:

manila/tests/share/test_api.py

@@ -976,10 +976,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'
@@ -989,6 +1012,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': {},

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.