Browse Source

[ozone] Fix edge cases when volume name start with service_id in path (#3893)

- Improve path traversal by checking for cases when volume name can be similar to service_id.
- Also handle an edge case explicitly, when volume name is equal to service_id in which we introduced a flag to explicitly add `ofs://<service_id>` for such paths at all directory levels.
Harsh Gupta 1 year ago
parent
commit
1675c50ebc

+ 22 - 7
desktop/core/src/desktop/lib/fs/ozone/__init__.py

@@ -19,7 +19,6 @@ import posixpath
 
 
 from hadoop.fs import normpath as fs_normpath
 from hadoop.fs import normpath as fs_normpath
 
 
-
 OFS_ROOT = 'ofs://'
 OFS_ROOT = 'ofs://'
 OFS_PATH_RE = re.compile('^/*[oO][fF][sS]?://([^/]+)(/(.*?([^/]+)?/?))?$')
 OFS_PATH_RE = re.compile('^/*[oO][fF][sS]?://([^/]+)(/(.*?([^/]+)?/?))?$')
 
 
@@ -40,7 +39,7 @@ def normpath(path):
     if is_root(path):
     if is_root(path):
       normalized = path
       normalized = path
     else:
     else:
-      normalized = '%s%s' % (OFS_ROOT, fs_normpath(path[len(OFS_ROOT):]))
+      normalized = '%s%s' % (OFS_ROOT, fs_normpath(path[len(OFS_ROOT) :]))
   else:
   else:
     normalized = fs_normpath(path)
     normalized = fs_normpath(path)
   return normalized
   return normalized
@@ -66,17 +65,33 @@ def join(*comp_list):
       return '/%s/%s' % parse_uri(uri)[:2]
       return '/%s/%s' % parse_uri(uri)[:2]
     except ValueError:
     except ValueError:
       return '/' if is_root(uri) else uri
       return '/' if is_root(uri) else uri
+
   joined = posixpath.join(*list(map(_prep, comp_list)))
   joined = posixpath.join(*list(map(_prep, comp_list)))
   return joined
   return joined
 
 
-def _serviceid_join(path, ofs_serviceid):
-  if path and (path == '/' or path.startswith('/' + ofs_serviceid)):
+
+def _serviceid_join(path, ofs_serviceid, is_vol_serviceid_equal=False):
+  """
+  Modify the provided path based on the service ID and a flag indicating
+  if the volume service ID is equal to the service ID.
+
+  Args:
+    path (str): The path to be joined with ofs_serviceid.
+    ofs_serviceid (str): The ofs_serviceid to be joined with the path.
+    is_vol_serviceid_equal (bool, optional): Flag to indicate if ofs_serviceid is equal to the volume service ID. Defaults to False.
+
+  Returns:
+    str: The joined path.
+  """
+
+  if path and (path == '/' or path.startswith('/' + ofs_serviceid + '/') or path == '/' + ofs_serviceid) and not is_vol_serviceid_equal:
     path = 'ofs:/' + path
     path = 'ofs:/' + path
   elif path and not path.startswith(OFS_ROOT + ofs_serviceid + '/'):
   elif path and not path.startswith(OFS_ROOT + ofs_serviceid + '/'):
     path = OFS_ROOT + ofs_serviceid + '/' + path.lstrip('/')
     path = OFS_ROOT + ofs_serviceid + '/' + path.lstrip('/')
-  
+
   return path
   return path
 
 
+
 def _append_separator(path):
 def _append_separator(path):
   if path and not path.endswith('/'):
   if path and not path.endswith('/'):
     path += '/'
     path += '/'
@@ -87,8 +102,8 @@ def parse_uri(uri):
   """
   """
   Returns tuple (service_id, key_name, key_basename).
   Returns tuple (service_id, key_name, key_basename).
   Raises ValueError if invalid OFS URI is passed.
   Raises ValueError if invalid OFS URI is passed.
-  
-  ofs://ozone1/volume1/bucket1/key1/key2 -> 
+
+  ofs://ozone1/volume1/bucket1/key1/key2 ->
   group1 -> ozone1
   group1 -> ozone1
   group2 -> /volume1/bucket1/key1/key2
   group2 -> /volume1/bucket1/key1/key2
   group3 -> volume1/bucket1/key1/key2
   group3 -> volume1/bucket1/key1/key2

+ 6 - 5
desktop/core/src/desktop/lib/fs/ozone/ofsstat.py

@@ -16,15 +16,12 @@
 # limitations under the License.
 # limitations under the License.
 
 
 from builtins import oct
 from builtins import oct
-import math
-import stat
 
 
 from django.utils.encoding import smart_str
 from django.utils.encoding import smart_str
 
 
-from hadoop.fs.hadoopfs import decode_fs_path
+from desktop.lib.fs.ozone import _serviceid_join, join as ofs_join
 from hadoop.fs.webhdfs_types import WebHdfsStat
 from hadoop.fs.webhdfs_types import WebHdfsStat
 
 
-from desktop.lib.fs.ozone import _serviceid_join, join as ofs_join
 
 
 class OzoneFSStat(WebHdfsStat):
 class OzoneFSStat(WebHdfsStat):
   """
   """
@@ -35,7 +32,11 @@ class OzoneFSStat(WebHdfsStat):
 
 
   def __init__(self, file_status, parent_path, ofs_serviceid=''):
   def __init__(self, file_status, parent_path, ofs_serviceid=''):
     super(OzoneFSStat, self).__init__(file_status, parent_path)
     super(OzoneFSStat, self).__init__(file_status, parent_path)
-    self.path = _serviceid_join(ofs_join(parent_path, self.name), ofs_serviceid)
+
+    # Check for edge case when volume name is equal to service_id,
+    # then forcefully append ofs://<service_id> in current path so that further directory level paths are consistent.
+    is_vol_serviceid_equal = parent_path.startswith(f'/{ofs_serviceid}')
+    self.path = _serviceid_join(ofs_join(parent_path, self.name), ofs_serviceid, is_vol_serviceid_equal)
 
 
   def __unicode__(self):
   def __unicode__(self):
     return "[OzoneFSStat] %7s %8s %8s %12s %s%s" % (oct(self.mode), self.user, self.group, self.size, self.path, self.isDir and '/' or "")
     return "[OzoneFSStat] %7s %8s %8s %12s %s%s" % (oct(self.mode), self.user, self.group, self.size, self.path, self.isDir and '/' or "")

+ 297 - 14
desktop/core/src/desktop/lib/fs/ozone/ofsstat_test.py

@@ -19,21 +19,208 @@
 from desktop.lib.fs.ozone.ofsstat import OzoneFSStat
 from desktop.lib.fs.ozone.ofsstat import OzoneFSStat
 
 
 
 
-class TestOzoneFSStat(object):
-  def setup_method(self):
+class TestOzoneFSStat:
+  def test_stat_normal_file_path(self):
     test_file_status = {
     test_file_status = {
-      'pathSuffix': 'testfile.csv', 'type': 'FILE', 'length': 32, 'owner': 'hueadmin', 'group': 'huegroup',
-      'permission': '666', 'accessTime': 1677914460588, 'modificationTime': 1677914460588, 'blockSize': 268435456, 'replication': 3}
+      'pathSuffix': 'testfile.csv',
+      'type': 'FILE',
+      'length': 32,
+      'owner': 'hueadmin',
+      'group': 'huegroup',
+      'permission': '666',
+      'accessTime': 1677914460588,
+      'modificationTime': 1677914460588,
+      'blockSize': 268435456,
+      'replication': 3,
+    }
 
 
-    test_parent_path = '/ozone1/gethue/'
+    test_parent_path = '/gethue/buck'
 
 
-    self.stat = OzoneFSStat(test_file_status, test_parent_path)
+    self.stat = OzoneFSStat(test_file_status, test_parent_path, 'ozone1')
 
 
+    assert self.stat.name == 'testfile.csv'
+    assert self.stat.path == 'ofs://ozone1/gethue/buck/testfile.csv'
+    assert self.stat.isDir is False
+    assert self.stat.type == 'FILE'
+    assert self.stat.atime == 1677914460
+    assert self.stat.mtime == 1677914460
+    assert self.stat.user == 'hueadmin'
+    assert self.stat.group == 'huegroup'
+    assert self.stat.size == 32
+    assert self.stat.blockSize == 268435456
+    assert self.stat.replication == 3
+    assert self.stat.aclBit is None
+    assert self.stat.fileId is None
+    assert self.stat.mode == 33206
+
+    expected_json_dict = {
+      'path': 'ofs://ozone1/gethue/buck/testfile.csv',
+      'size': 32,
+      'atime': 1677914460,
+      'mtime': 1677914460,
+      'mode': 33206,
+      'user': 'hueadmin',
+      'group': 'huegroup',
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+
+    assert self.stat.to_json_dict() == expected_json_dict
+
+  def test_stat_root_path(self):
+    test_path_status = {
+      'pathSuffix': 'ozone1',
+      'type': 'FILE',
+      'length': 32,
+      'owner': 'hueadmin',
+      'group': 'huegroup',
+      'permission': '666',
+      'accessTime': 1677914460588,
+      'modificationTime': 1677914460588,
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+    test_parent_path = 'ofs://'
+    self.stat = OzoneFSStat(test_path_status, test_parent_path, 'ozone1')
+
+    assert self.stat.name == 'ozone1'
+    assert self.stat.path == 'ofs://ozone1'
+    assert self.stat.isDir is False
+    assert self.stat.type == 'FILE'
+    assert self.stat.atime == 1677914460
+    assert self.stat.mtime == 1677914460
+    assert self.stat.user == 'hueadmin'
+    assert self.stat.group == 'huegroup'
+    assert self.stat.size == 32
+    assert self.stat.blockSize == 268435456
+    assert self.stat.replication == 3
+    assert self.stat.aclBit is None
+    assert self.stat.fileId is None
+    assert self.stat.mode == 33206
+
+    expected_json_dict = {
+      'path': 'ofs://ozone1',
+      'size': 32,
+      'atime': 1677914460,
+      'mtime': 1677914460,
+      'mode': 33206,
+      'user': 'hueadmin',
+      'group': 'huegroup',
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+
+    assert self.stat.to_json_dict() == expected_json_dict
+
+  def test_stat_normal_volume_path(self):
+    test_path_status = {
+      'pathSuffix': 'gethue',
+      'type': 'FILE',
+      'length': 32,
+      'owner': 'hueadmin',
+      'group': 'huegroup',
+      'permission': '666',
+      'accessTime': 1677914460588,
+      'modificationTime': 1677914460588,
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+    test_parent_path = ''
+    self.stat = OzoneFSStat(test_path_status, test_parent_path, 'ozone1')
+
+    assert self.stat.name == 'gethue'
+    assert self.stat.path == 'ofs://ozone1/gethue'
+    assert self.stat.isDir is False
+    assert self.stat.type == 'FILE'
+    assert self.stat.atime == 1677914460
+    assert self.stat.mtime == 1677914460
+    assert self.stat.user == 'hueadmin'
+    assert self.stat.group == 'huegroup'
+    assert self.stat.size == 32
+    assert self.stat.blockSize == 268435456
+    assert self.stat.replication == 3
+    assert self.stat.aclBit is None
+    assert self.stat.fileId is None
+    assert self.stat.mode == 33206
+
+    expected_json_dict = {
+      'path': 'ofs://ozone1/gethue',
+      'size': 32,
+      'atime': 1677914460,
+      'mtime': 1677914460,
+      'mode': 33206,
+      'user': 'hueadmin',
+      'group': 'huegroup',
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+
+    assert self.stat.to_json_dict() == expected_json_dict
+
+  def test_stat_volume_path_startswith_serviceid(self):
+    test_path_status = {
+      'pathSuffix': 'ozone1-gethue',
+      'type': 'FILE',
+      'length': 32,
+      'owner': 'hueadmin',
+      'group': 'huegroup',
+      'permission': '666',
+      'accessTime': 1677914460588,
+      'modificationTime': 1677914460588,
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+    test_parent_path = ''
+    self.stat = OzoneFSStat(test_path_status, test_parent_path, 'ozone1')
+
+    assert self.stat.name == 'ozone1-gethue'
+    assert self.stat.path == 'ofs://ozone1/ozone1-gethue'
+    assert self.stat.isDir is False
+    assert self.stat.type == 'FILE'
+    assert self.stat.atime == 1677914460
+    assert self.stat.mtime == 1677914460
+    assert self.stat.user == 'hueadmin'
+    assert self.stat.group == 'huegroup'
+    assert self.stat.size == 32
+    assert self.stat.blockSize == 268435456
+    assert self.stat.replication == 3
+    assert self.stat.aclBit is None
+    assert self.stat.fileId is None
+    assert self.stat.mode == 33206
+
+    expected_json_dict = {
+      'path': 'ofs://ozone1/ozone1-gethue',
+      'size': 32,
+      'atime': 1677914460,
+      'mtime': 1677914460,
+      'mode': 33206,
+      'user': 'hueadmin',
+      'group': 'huegroup',
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+
+    assert self.stat.to_json_dict() == expected_json_dict
+
+  def test_stat_file_path_when_volume_name_startswith_serviceid(self):
+    test_path_status = {
+      'pathSuffix': 'testfile.csv',
+      'type': 'FILE',
+      'length': 32,
+      'owner': 'hueadmin',
+      'group': 'huegroup',
+      'permission': '666',
+      'accessTime': 1677914460588,
+      'modificationTime': 1677914460588,
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+    test_parent_path = '/ozone1-gethue/buck'
+    self.stat = OzoneFSStat(test_path_status, test_parent_path, 'ozone1')
 
 
-  def test_stat_attributes(self):
     assert self.stat.name == 'testfile.csv'
     assert self.stat.name == 'testfile.csv'
-    assert self.stat.path == 'ofs://ozone1/gethue/testfile.csv'
-    assert self.stat.isDir == False
+    assert self.stat.path == 'ofs://ozone1/ozone1-gethue/buck/testfile.csv'
+    assert self.stat.isDir is False
     assert self.stat.type == 'FILE'
     assert self.stat.type == 'FILE'
     assert self.stat.atime == 1677914460
     assert self.stat.atime == 1677914460
     assert self.stat.mtime == 1677914460
     assert self.stat.mtime == 1677914460
@@ -42,14 +229,110 @@ class TestOzoneFSStat(object):
     assert self.stat.size == 32
     assert self.stat.size == 32
     assert self.stat.blockSize == 268435456
     assert self.stat.blockSize == 268435456
     assert self.stat.replication == 3
     assert self.stat.replication == 3
-    assert self.stat.aclBit == None
-    assert self.stat.fileId == None
+    assert self.stat.aclBit is None
+    assert self.stat.fileId is None
     assert self.stat.mode == 33206
     assert self.stat.mode == 33206
 
 
+    expected_json_dict = {
+      'path': 'ofs://ozone1/ozone1-gethue/buck/testfile.csv',
+      'size': 32,
+      'atime': 1677914460,
+      'mtime': 1677914460,
+      'mode': 33206,
+      'user': 'hueadmin',
+      'group': 'huegroup',
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+
+    assert self.stat.to_json_dict() == expected_json_dict
+
+  def test_stat_volume_path_equals_serviceid(self):
+    test_path_status = {
+      'pathSuffix': 'ozone1',
+      'type': 'FILE',
+      'length': 32,
+      'owner': 'hueadmin',
+      'group': 'huegroup',
+      'permission': '666',
+      'accessTime': 1677914460588,
+      'modificationTime': 1677914460588,
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+    test_parent_path = ''
+    self.stat = OzoneFSStat(test_path_status, test_parent_path, 'ozone1')
+
+    assert self.stat.name == 'ozone1'
+    assert self.stat.path == 'ofs://ozone1/ozone1'
+    assert self.stat.isDir is False
+    assert self.stat.type == 'FILE'
+    assert self.stat.atime == 1677914460
+    assert self.stat.mtime == 1677914460
+    assert self.stat.user == 'hueadmin'
+    assert self.stat.group == 'huegroup'
+    assert self.stat.size == 32
+    assert self.stat.blockSize == 268435456
+    assert self.stat.replication == 3
+    assert self.stat.aclBit is None
+    assert self.stat.fileId is None
+    assert self.stat.mode == 33206
+
+    expected_json_dict = {
+      'path': 'ofs://ozone1/ozone1',
+      'size': 32,
+      'atime': 1677914460,
+      'mtime': 1677914460,
+      'mode': 33206,
+      'user': 'hueadmin',
+      'group': 'huegroup',
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+
+    assert self.stat.to_json_dict() == expected_json_dict
+
+  def test_stat_file_path_when_volume_name_equals_serviceid(self):
+    test_path_status = {
+      'pathSuffix': 'testfile.csv',
+      'type': 'FILE',
+      'length': 32,
+      'owner': 'hueadmin',
+      'group': 'huegroup',
+      'permission': '666',
+      'accessTime': 1677914460588,
+      'modificationTime': 1677914460588,
+      'blockSize': 268435456,
+      'replication': 3,
+    }
+    test_parent_path = '/ozone1/buck'
+    self.stat = OzoneFSStat(test_path_status, test_parent_path, 'ozone1')
+
+    assert self.stat.name == 'testfile.csv'
+    assert self.stat.path == 'ofs://ozone1/ozone1/buck/testfile.csv'
+    assert self.stat.isDir is False
+    assert self.stat.type == 'FILE'
+    assert self.stat.atime == 1677914460
+    assert self.stat.mtime == 1677914460
+    assert self.stat.user == 'hueadmin'
+    assert self.stat.group == 'huegroup'
+    assert self.stat.size == 32
+    assert self.stat.blockSize == 268435456
+    assert self.stat.replication == 3
+    assert self.stat.aclBit is None
+    assert self.stat.fileId is None
+    assert self.stat.mode == 33206
 
 
-  def test_to_json_dict(self):
     expected_json_dict = {
     expected_json_dict = {
-      'path': 'ofs://ozone1/gethue/testfile.csv', 'size': 32, 'atime': 1677914460, 'mtime': 1677914460, 'mode': 33206, 'user': 'hueadmin',
-      'group': 'huegroup', 'blockSize': 268435456, 'replication': 3}
+      'path': 'ofs://ozone1/ozone1/buck/testfile.csv',
+      'size': 32,
+      'atime': 1677914460,
+      'mtime': 1677914460,
+      'mode': 33206,
+      'user': 'hueadmin',
+      'group': 'huegroup',
+      'blockSize': 268435456,
+      'replication': 3,
+    }
 
 
     assert self.stat.to_json_dict() == expected_json_dict
     assert self.stat.to_json_dict() == expected_json_dict