rockstor / rockstor-core

Linux/BTRFS based Network Attached Storage(NAS)
http://rockstor.com/docs/contribute_section.html
GNU General Public License v3.0
548 stars 139 forks source link

Leap 15.6: SFTP share error - library paths changed #2856

Open Hooverdan96 opened 1 week ago

Hooverdan96 commented 1 week ago

Thanks to forum user mpidala, the following issue was reported during sftp share creation on a Leap 15.6 Rockstor instance (https://forum.rockstor.com/t/error-adding-sftp-share/9548/): the following error message is thrown when trying to create an sftp share:

            Traceback (most recent call last):
  File "/opt/rockstor/src/rockstor/rest_framework_custom/generic_view.py", line 41, in _handle_exception
    yield
  File "/opt/rockstor/src/rockstor/storageadmin/views/sftp.py", line 82, in post
    rsync_for_sftp(chroot_loc)
  File "/opt/rockstor/src/rockstor/system/ssh.py", line 303, in rsync_for_sftp
    copy(l, "{}{}".format(chroot_loc, l))
  File "/usr/lib64/python3.11/shutil.py", line 431, in copy
    copyfile(src, dst, follow_symlinks=follow_symlinks)
  File "/usr/lib64/python3.11/shutil.py", line 256, in copyfile
    with open(src, 'rb') as fsrc:
         ^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/lib64/libz.so.1'

As @FroggyFlox already identified, some of the shared libraries that are symbolically linked during the sftp process can't be found because their physical location has changed with the Leap 15.6 release. Within the Rockstor code base they are maintained here:

https://github.com/rockstor/rockstor-core/blob/3f8d02b61f38c0a906423b6a371d17c837f52299/src/rockstor/system/ssh.py#L273-L285

the one that triggered the above error is assumed to be located in: /lib64/libz.so.1

In Leap 15.6 the location has changed to: /usr/lib64/libz.so.1

This likely has to be updated. The decision here is whether to remain backwards compatible to at least Leap 15.5 or not.

If no, then an update of the paths in the above code snippet should be sufficient. If yes, we will need an additional way of identifying which Leap release is currently being used during the execution of the program.

Unfortunately, in the copying process only the distribution ID (OS_DISTRO_ID) from the settings file is used to validate:

https://github.com/rockstor/rockstor-core/blob/3f8d02b61f38c0a906423b6a371d17c837f52299/src/rockstor/system/ssh.py#L302-L304

As one can see, in the settings file the values for OS_DISTRO_ID do not distinguish by distribution release

https://github.com/rockstor/rockstor-core/blob/3f8d02b61f38c0a906423b6a371d17c837f52299/src/rockstor/settings.py#L466-L473

Unfortunately, just replacing the OS_DISTRO_ID with 'OS_DISTRO_VERSION' would likely break the "rolling" Tumbleweed version in this case. Since the version under Tumbleweed changes with every release which are very frequent (daily/weekly). Updating Rockstor that frequently is neither practical nor sustainable.

One approach could be to add another condition that essentially checks whether the Rockstor instance runs on Leap, and if so, check whether it's version 15.6 or earlier. If earlier the current definition shown above would be used, and if 15.6 (and presumably later) select a newly created section that contains the altered paths to create symbolic links from. If 15.6 or higher, concatenate ID and Version and the section the copy process refers would also have the concatenated version (e.g. opensuse-leap15.6+ and for other leap flavors it remains what it was, opensuse-leap). That way, other Leap flavors are not impacted. Once support for 15.5 is dropped, that section can be dropped, and it can be similarly managed if a change comes with a 15.7 (if ever released) version ...

Or a completely different approach, of course.

Finally, I am surprised that the testing library would not have caught this, but I did not take a closer look at the unit tests. When fixing this, the unit test should probably be corrected, or if it doesn't exist should be created, since I suspect this will not be the last time that the location of these types of files will change. For reference:

https://github.com/rockstor/rockstor-core/blob/testing/src/rockstor/storageadmin/tests/test_sftp.py

happy path (which should be unhappy on 15.6 in my opinion): https://github.com/rockstor/rockstor-core/blob/3f8d02b61f38c0a906423b6a371d17c837f52299/src/rockstor/storageadmin/tests/test_sftp.py#L158-L165

Hooverdan96 commented 1 week ago

I can confirm that the above file, /lib64/libz.so.1, seems to be the only one with a new path (/usr/lib64/libz.so.1). Furthermore, in a proof-of-concept I updated the new (no other code) in the leap section of the above code manually and executed systemctl restart rockstor.

Et voila, the sftp share could be created (non root-user owned). I just wanted to confirm that there aren't any follow-on errors during the sftp export creation, and it seem that there aren't.

phillxnet commented 1 week ago

Currently working on this issue, and I have begun a minor modernisation of sorts. Way back we just grabbed these library dependencies from ldd bash and ldd rsync to enable both within the chroot we setup when establishing the sftp access. So I'm preparing a PR that dynamically retrieves the required dependencies per host OS and architecture. We likely also have some ARM issue also now. Plus the TW list is now out-of-date as well. Test of ldd wrapper used to be submitted with the associated PR. Nearly there now.

Hooverdan96 commented 1 week ago

way more elegant, actually :).