make_read_only_recursively: don't touch symlink targets

This fixes a problem visible when installing openssl 1.1.1d from
component/openssl. At the end of install step, some symlinks are created
inside openssl install directory for all certificates from
ca-certificates part, then the "make read only" step occurs, which
recursively chmod on files and folder in openssl installed parts, but
since this was following symlinks, it was also trying to make readonly
in ca-certificates parts, which in my case was belonging to another
unix user, because it was a shared part folder from outer level slapos.

/reviewed-on nexedi/slapos.recipe.build!10

slapos/recipe/build/tests.py

@@ -13,6 +13,8 @@ from functools import wraps
 from subprocess import check_call, check_output, CalledProcessError
 from slapos.recipe.gitclone import GIT_CLONE_ERROR_MESSAGE, \
     GIT_CLONE_CACHE_ERROR_MESSAGE
+from slapos.recipe.downloadunpacked import make_read_only_recursively
+
 
 optionflags = (doctest.ELLIPSIS | doctest.NORMALIZE_WHITESPACE)
 
@@ -490,6 +492,48 @@ repository = %s
   def test_ignore_cloning_submodules(self):
     self.test_clone_submodules_by_default(ignore_cloning_submodules=True)
 
+
+class MakeReadOnlyTests(unittest.TestCase):
+
+  def setUp(self):
+    self.tmp_dir = tempfile.mkdtemp()
+    os.mkdir(os.path.join(self.tmp_dir, 'folder'))
+    with open(os.path.join(self.tmp_dir, 'folder', 'file'), 'w') as f:
+      f.write('content')
+
+  def tearDown(self):
+    for path in (
+        ('folder', 'file', ),
+        ('folder', 'symlink', ),
+        ('folder', 'broken_symlink', ),
+        ('folder', 'symlink_to_file_of_another_owner', ),
+        ('folder', ),
+        ()):
+      full_path = os.path.join(self.tmp_dir, *path)
+      if os.path.exists(full_path) and not os.path.islink(full_path):
+        os.chmod(full_path, 0o700)
+    shutil.rmtree(self.tmp_dir)
+
+  def test_make_read_only_recursive(self):
+    make_read_only_recursively(self.tmp_dir)
+    self.assertRaises(IOError, open, os.path.join(self.tmp_dir, 'folder', 'file'), 'w')
+    self.assertRaises(IOError, open, os.path.join(self.tmp_dir, 'folder', 'another_file'), 'w')
+    self.assertRaises(OSError, os.mkdir, os.path.join(self.tmp_dir, 'another_folder'))
+
+  def test_make_read_only_recursive_symlink(self):
+    os.symlink(
+        os.path.join(self.tmp_dir, 'folder', 'file'),
+        os.path.join(self.tmp_dir, 'folder', 'symlink'))
+    os.symlink(
+        os.path.join(self.tmp_dir, 'folder', 'not_exist'),
+        os.path.join(self.tmp_dir, 'folder', 'broken_symlink'))
+    os.symlink(
+        os.devnull,
+        os.path.join(self.tmp_dir, 'folder', 'symlink_to_file_of_another_owner'))
+    make_read_only_recursively(self.tmp_dir)
+    self.assertRaises(IOError, open, os.path.join(self.tmp_dir, 'folder', 'symlink'), 'w')
+
+
 def test_suite():
   suite = unittest.TestSuite((
       doctest.DocFileSuite(
@@ -507,6 +551,7 @@ def test_suite():
               ]),
           ),
       unittest.makeSuite(GitCloneNonInformativeTests),
+      unittest.makeSuite(MakeReadOnlyTests),
       ))
   return suite
 

slapos/recipe/downloadunpacked.py

@@ -32,7 +32,6 @@ import tarfile
 import zc.buildout
 import tempfile
 import setuptools.archive_util
-import stat
 from hashlib import md5
 
 is_true = ('false', 'true').index
@@ -222,15 +221,15 @@ def unpatch_archive_util():
     setuptools.archive_util.unpack_tarfile,
   )
 
-_read_only_mask = 0o777 ^ (stat.S_IWRITE | stat.S_IWGRP | stat.S_IWOTH)
 def make_read_only(path):
-  os.chmod(path, os.stat(path).st_mode & _read_only_mask)
+  if not os.path.islink(path):
+    os.chmod(path, os.stat(path).st_mode & 0o555)
 
 def make_read_only_recursively(path):
   make_read_only(path)
   for root, dir_list, file_list in os.walk(path):
-    for dir in dir_list:
-      make_read_only(os.path.join(root, dir))
+    for dir_ in dir_list:
+      make_read_only(os.path.join(root, dir_))
     for file_ in file_list:
       make_read_only(os.path.join(root, file_))