classification
Title: Remove POSIX.1e ACLs in tests that rely on default permissions behavior
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.9, Python 3.8, Python 3.7, Python 2.7
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: Ivan.Pozdeev, steve.dower
Priority: normal Keywords: patch

Created on 2019-04-02 02:54 by Ivan.Pozdeev, last changed 2019-04-10 19:09 by Ivan.Pozdeev. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12655 closed Ivan.Pozdeev, 2019-04-02 02:54
Messages (3)
msg339313 - (view) Author: Ivan Pozdeev (Ivan.Pozdeev) * Date: 2019-04-02 02:54
In Linuxes with ACLs enabled, the following tests fail, as Steve Dower discovered in https://mail.python.org/pipermail/python-dev/2019-March/156929.html:

======================================================================
FAIL: test_mode (test.test_os.MakedirTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/osboxes/Documents/cpython/Lib/test/test_os.py", line 1157, in test_mode
    self.assertEqual(os.stat(parent).st_mode & 0o777, 0o775)
AssertionError: 493 != 509

======================================================================
FAIL: test_open_mode (test.test_pathlib.PosixPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/osboxes/Documents/cpython/Lib/test/test_pathlib.py", line 2104, in test_open_mode
    self.assertEqual(stat.S_IMODE(st.st_mode), 0o666)
AssertionError: 420 != 438

======================================================================
FAIL: test_touch_mode (test.test_pathlib.PosixPathTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/osboxes/Documents/cpython/Lib/test/test_pathlib.py", line 2117, in test_touch_mode
    self.assertEqual(stat.S_IMODE(st.st_mode), 0o666)
AssertionError: 420 != 438


POSIX.1e is supported by major distros even though it's officially withdrawn; see https://en.wikipedia.org/wiki/Access_control_list#Filesystem_ACLs .
msg339373 - (view) Author: Ivan Pozdeev (Ivan.Pozdeev) * Date: 2019-04-03 00:49
Seeing during PR composition how basically every mode check and every `test.support.temp_umask` use is broken by ACLs, I'm starting to doubt that fixing individual test cases is the way to go.

Though we can simply not worry about supporting everything imaginable and solve problems as they come and declare highly unusual cases unsupported.

For the record, other potential problems:

* All open(O_CREAT) and umask uses anywhere in the tests are broken by default.
    * Though only 4 test files were affected for now -- test_pathlib, test_tarfile, test_os, test_import .
* There are other overlay security frameworks that override POSIX permissions like NFSv4 and SELinux.

And possible solutions:

* Skip mode_t checks outright if overlay security is detected like it's already done in Windows
* Embed cleanup code into test.support's temp_* and such. In the POSIX.1e case, will have to create temporary dirs or change permissions for the current dir.
    * I don't think regrtest's temporary dir is a good place for this 'cuz tests are supposed to be runnable directly with `unittest`, too.
msg339883 - (view) Author: Ivan Pozdeev (Ivan.Pozdeev) * Date: 2019-04-10 19:09
Given the downsides, I think the proposed solution as it's now is too hacky to be a net improvement.

"Skip mode_t checks" looks like the only way to go (since I've no idea how to "create temporary dirs" transparently for arbitrary test logic). But skipping tests defeats the purpose of a buildbot, so it's not a solution for the problem at hand.
History
Date User Action Args
2019-04-10 19:09:52Ivan.Pozdeevsetstatus: open -> closed
resolution: rejected
messages: + msg339883

stage: patch review -> resolved
2019-04-03 03:38:56xtreaksetnosy: + steve.dower
2019-04-03 00:49:45Ivan.Pozdeevsetmessages: + msg339373
2019-04-02 02:54:38Ivan.Pozdeevsetkeywords: + patch
stage: patch review
pull_requests: + pull_request12584
2019-04-02 02:54:06Ivan.Pozdeevcreate