Fix parsing of permissions with special bits

kolla_set_configs currently fails to parse permissions
such as 2775 because only legacy values like 0755 are
converted to Python octal notation.

Accept standard UNIX permission notation for
permissions with special mode bits such as SGID, SUID,
and sticky bit.

Change-Id: I5937a17b0d70f9c1db4b2617854d0180c37fe2c0
Closes-Bug: #2154269
Needed-By: https://review.opendev.org/c/openstack/kolla-ansible/+/989850
Signed-off-by: Michal Arbet <michal.arbet@ultimum.io>

3 files changed, 84 insertions, 5 deletions

diff --git a/docker/base/set_configs.py b/docker/base/set_configs.py
index 542537e4c..c0a4af76c 100644
--- a/docker/base/set_configs.py
+++ b/docker/base/set_configs.py
@@ -410,11 +410,20 @@ def handle_permissions(config):
                               path, uid, gid)
 
             if perm:
-                # NOTE(Jeffrey4l): py3 need '0oXXX' format for octal literals,
-                # and py2 support such format too.
-                if len(perm) == 4 and perm[1] != 'o':
-                    perm = ''.join([perm[:1], 'o', perm[1:]])
-                perm = int(perm, base=0)
+                # Accept valid UNIX permission notation
+                # (e.g. 644, 0600, 1777, 2755)
+                # or equivalent Python octal literals
+                # (e.g. 0o644, 0o1777, 0o2755).
+                if re.fullmatch(r'[0-7]{3,4}', perm):
+                    perm = int(perm, 8)
+                elif re.fullmatch(r'0o[0-7]{3,4}', perm):
+                    perm = int(perm, 0)
+                else:
+                    raise ValueError(
+                        f"Invalid permission '{perm}'. Expected a valid UNIX "
+                        "permission (e.g. 644, 0600, 2755) or a Python octal "
+                        "literal (e.g. 0o644, 0o0600, 0o2755)."
+                    )
 
                 # Ensure execute bit on directory if read bit is set
                 if os.path.isdir(path):
diff --git a/releasenotes/notes/bug-2154269-0b79adc81dab29a8.yaml b/releasenotes/notes/bug-2154269-0b79adc81dab29a8.yaml
new file mode 100644
index 000000000..ed2ec4021
--- /dev/null
+++ b/releasenotes/notes/bug-2154269-0b79adc81dab29a8.yaml
@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes ``kolla_set_configs`` failing to parse traditional UNIX
+    permissions with special bits.
+    `LP#2154269 <https://launchpad.net/bugs/2154269>`__
diff --git a/tests/test_set_config.py b/tests/test_set_config.py
index 89b71fca8..45f71bdd4 100644
--- a/tests/test_set_config.py
+++ b/tests/test_set_config.py
@@ -1104,3 +1104,67 @@ class ExecuteConfigCheckStateMismatchTest(base.BaseTestCase):
         self.assertRaises(set_configs.StateMismatch,
                           set_configs.execute_config_check,
                           config)
+
+
+class HandlePermissionsPermParsingTest(base.BaseTestCase):
+
+    def _assert_permission(self, input_perm, expected_perm, is_dir=False):
+        path = '/var/log/kolla/glance'
+        config = {
+            'permissions': [
+                {
+                    'path': path,
+                    'owner': 'glance:glance',
+                    'perm': input_perm,
+                }
+            ]
+        }
+
+        with mock.patch('pwd.getpwnam') as mock_getpwnam, \
+                mock.patch('grp.getgrnam') as mock_getgrnam, \
+                mock.patch('glob.glob', return_value=[path]) as mock_glob, \
+                mock.patch('os.path.exists',
+                           return_value=True) as mock_exists, \
+                mock.patch('os.path.isdir',
+                           return_value=is_dir) as mock_isdir, \
+                mock.patch('os.chown') as mock_chown, \
+                mock.patch('os.chmod') as mock_chmod:
+            mock_getpwnam.return_value.pw_uid = 4242
+            mock_getgrnam.return_value.gr_gid = 4243
+
+            set_configs.handle_permissions(config)
+
+            mock_getpwnam.assert_called_once_with('glance')
+            mock_getgrnam.assert_called_once_with('glance')
+            mock_glob.assert_called_once_with(path)
+            mock_exists.assert_called_once_with(path)
+            mock_isdir.assert_called_once_with(path)
+            mock_chown.assert_called_once_with(path, 4242, 4243)
+            mock_chmod.assert_called_once_with(path, expected_perm)
+
+    def test_handle_permissions_accepts_valid_permission_notations(self):
+        """Test valid permission notations."""
+        permissions = (
+            ('644', 0o644),
+            ('755', 0o755),
+            ('0644', 0o644),
+            ('0755', 0o755),
+            ('0o644', 0o644),
+            ('0o755', 0o755),
+            ('1777', 0o1777),
+            ('2775', 0o2775),
+            ('0o2775', 0o2775),
+            ('4755', 0o4755),
+        )
+
+        for input_perm, expected_perm in permissions:
+            with self.subTest(input_perm=input_perm):
+                self._assert_permission(input_perm, expected_perm)
+
+    def test_handle_permissions_accepts_python_octal_notation(self):
+        """Test Python octal permission notation."""
+        self._assert_permission('0o2775', 0o2775)
+
+    def test_handle_permissions_adds_execute_bits_for_directories(self):
+        """Test execute bits are added to readable directories."""
+        self._assert_permission('0644', 0o755, is_dir=True)