* testsuite for ACLs @ 2019-03-25 13:18 Rishabh Dave 2019-03-25 20:20 ` Darrick J. Wong 2019-03-26 1:43 ` Dave Chinner 0 siblings, 2 replies; 9+ messages in thread From: Rishabh Dave @ 2019-03-25 13:18 UTC (permalink / raw) To: fstests Hi, I've been writing a testsuite for ACLs for CephFS[1] for some time. Since the testcases are written from a generic POV, reviewers asked me to try and get these tests merged with xfstests-dev so that it could benefit other projects as well. So far, I've got around 22 testcases. To give a brief summary, the testsuite has 5 categories of tests: tests for nondefault nonmask ACLs[2], for nondefault mask ACLs[3], for default nonmask ACLs[4], for default mask ACLs[5] and, finally, the rest of testcases that don't fall into previous categories[6] (e.g. testing effect of '-k' and '-b' option of setfacl). The second last category is neither complete nor tested, but that shouldn't be a hurdle. One important issue to address is that my testsuite is written in Python and xfstests-dev doesn't have any tests in Python. Although I don't see any guidelines in the repository instructing to not to use anything other than bash (and since I've never contributed to xfstest-dev), I think it's best to ask this explicitly: is sending a patch for tests in Python acceptable for xfstests-dev? - Rishabh [1] https://github.com/ceph/ceph/pull/26477 [2] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L270 [3] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L391 [4] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L457 [5] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L549 [6] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L609 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-03-25 13:18 testsuite for ACLs Rishabh Dave @ 2019-03-25 20:20 ` Darrick J. Wong 2019-03-26 1:43 ` Dave Chinner 1 sibling, 0 replies; 9+ messages in thread From: Darrick J. Wong @ 2019-03-25 20:20 UTC (permalink / raw) To: Rishabh Dave; +Cc: fstests On Mon, Mar 25, 2019 at 06:48:18PM +0530, Rishabh Dave wrote: > Hi, > > I've been writing a testsuite for ACLs for CephFS[1] for some time. > Since the testcases are written from a generic POV, reviewers asked me > to try and get these tests merged with xfstests-dev so that it could > benefit other projects as well. > > So far, I've got around 22 testcases. To give a brief summary, the > testsuite has 5 categories of tests: tests for nondefault nonmask > ACLs[2], for nondefault mask ACLs[3], for default nonmask ACLs[4], for > default mask ACLs[5] and, finally, the rest of testcases that don't > fall into previous categories[6] (e.g. testing effect of '-k' and '-b' > option of setfacl). The second last category is neither complete nor > tested, but that shouldn't be a hurdle. > > One important issue to address is that my testsuite is written in > Python and xfstests-dev doesn't have any tests in Python. Although I > don't see any guidelines in the repository instructing to not to use > anything other than bash (and since I've never contributed to > xfstest-dev), I think it's best to ask this explicitly: is sending a > patch for tests in Python acceptable for xfstests-dev? Which version of python? mount.py in your ceph blob repo sort of implies python 2.x ("print os.stat(...)"), which goes EOL in 9 months. --D > - Rishabh > > [1] https://github.com/ceph/ceph/pull/26477 > [2] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L270 > [3] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L391 > [4] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L457 > [5] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L549 > [6] https://github.com/rishabh-d-dave/ceph/blob/enable-acls-by-default/qa/tasks/cephfs/test_acls.py#L609 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-03-25 13:18 testsuite for ACLs Rishabh Dave 2019-03-25 20:20 ` Darrick J. Wong @ 2019-03-26 1:43 ` Dave Chinner 2019-03-29 15:59 ` Rishabh Dave 1 sibling, 1 reply; 9+ messages in thread From: Dave Chinner @ 2019-03-26 1:43 UTC (permalink / raw) To: Rishabh Dave; +Cc: fstests On Mon, Mar 25, 2019 at 06:48:18PM +0530, Rishabh Dave wrote: > Hi, > > I've been writing a testsuite for ACLs for CephFS[1] for some time. > Since the testcases are written from a generic POV, reviewers asked me > to try and get these tests merged with xfstests-dev so that it could > benefit other projects as well. > > So far, I've got around 22 testcases. To give a brief summary, the > testsuite has 5 categories of tests: tests for nondefault nonmask > ACLs[2], for nondefault mask ACLs[3], for default nonmask ACLs[4], for > default mask ACLs[5] and, finally, the rest of testcases that don't > fall into previous categories[6] (e.g. testing effect of '-k' and '-b' > option of setfacl). The second last category is neither complete nor > tested, but that shouldn't be a hurdle. > > One important issue to address is that my testsuite is written in > Python and xfstests-dev doesn't have any tests in Python. Although I > don't see any guidelines in the repository instructing to not to use > anything other than bash (and since I've never contributed to > xfstest-dev), I think it's best to ask this explicitly: is sending a > patch for tests in Python acceptable for xfstests-dev? There's no support for python in fstests. The test itself still needs the bash script to run it, set up the test environment, clean up after the test, etc. So writing the test in python really means "writing a test wrapper to execute a python script". So the first question is how many of those 22 tests are already covered by ACL tests in fstests? We've got a few tests that cover ACLs already: $ git grep -w acl tests/*/group tests/generic/group:026 acl quick auto tests/generic/group:053 acl repair auto quick tests/generic/group:077 acl attr auto enospc tests/generic/group:079 acl attr ioctl metadata auto quick tests/generic/group:099 acl auto quick tests/generic/group:105 acl auto quick tests/generic/group:237 auto quick acl tests/generic/group:307 auto quick acl tests/generic/group:318 acl attr auto quick tests/generic/group:319 acl auto quick tests/generic/group:375 auto quick acl tests/generic/group:389 auto quick acl tests/generic/group:444 auto quick acl tests/generic/group:449 auto quick acl enospc tests/nfs/group:001 auto quick nfs4_acl acl tests/xfs/group:053 attr acl repair quick auto tests/xfs/group:067 acl attr auto quick So some of the things in your test are probably already covered. FWIW, looking at that python test script, it's jsut basically running things like ls, touch, chown, chmod, setfacl, etc. It's not using python to modify and test ACLs, it's using python to run CLI programs that modify and test acls. eg: def test_acl_for_file_owner(self): self.setup() self.mount_a.run_as_root('chown %s:%s %s' % (self.currentuser, self.currentgroup, self.testdir1)) self.mount_a.run_shell('chmod u-rwx %s' % (self.testdir1)) # preliminary test self.assert_dirs_are_inaccessible(self.testdir1) self.mount_a.run_as_root(['setfacl', '-m', self.fo_acl, self.testdir1]) self.assert_acls_are_present(self.fo_acl, self.testdir1) self.assert_dirs_are_accessible(self.testdir1) which is basically: fo_acl="user::rwx" testdir1="$TEST_DIR/$seq.1" test_acl_for_file_owner() { chown $user:$group $testdir1 _runas -u $user chmod u-rwx $testdir1 check_inaccessible $user $testdir setfacl -m $fo_acl $testdir1 check_acl $fo_acl $testdir1 check_accessible $user $testdir1 } It seems to me that this is much more suited to being written as a shell script rather than in python where running CLI programs is kinda gross. Certainly from a fstests maintenance point of view it would be better as a bash script like everything else.... Cheers, Dave. -- Dave Chinner david@fromorbit.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-03-26 1:43 ` Dave Chinner @ 2019-03-29 15:59 ` Rishabh Dave 2019-03-29 22:56 ` Theodore Ts'o 2019-04-10 16:23 ` Rishabh Dave 0 siblings, 2 replies; 9+ messages in thread From: Rishabh Dave @ 2019-03-29 15:59 UTC (permalink / raw) To: Dave Chinner; +Cc: fstests Reading both replies I feel it's best to rewrite to bash. I'll go ahead and do that. On Tue, 26 Mar 2019 at 07:13, Dave Chinner <david@fromorbit.com> wrote: > So the first question is how many of those 22 tests are already > covered by ACL tests in fstests? We've got a few tests that cover > ACLs already: > > $ git grep -w acl tests/*/group > tests/generic/group:026 acl quick auto > tests/generic/group:053 acl repair auto quick > tests/generic/group:077 acl attr auto enospc > tests/generic/group:079 acl attr ioctl metadata auto quick > tests/generic/group:099 acl auto quick > tests/generic/group:105 acl auto quick > tests/generic/group:237 auto quick acl > tests/generic/group:307 auto quick acl > tests/generic/group:318 acl attr auto quick > tests/generic/group:319 acl auto quick > tests/generic/group:375 auto quick acl > tests/generic/group:389 auto quick acl > tests/generic/group:444 auto quick acl > tests/generic/group:449 auto quick acl enospc > tests/nfs/group:001 auto quick nfs4_acl acl > tests/xfs/group:053 attr acl repair quick auto > tests/xfs/group:067 acl attr auto quick Is tests/generic/group a list of titles of testcases? If so, I don't understand them. I see there are few tests for ACLs in tests/generic/099. I couldn't find them before since the tests use chacl and I was looking for setfacl and getfacl. I'll compare my testsuite to tests/generic/099, create a list of testcases not covered in xfstests-dev and start rewriting. - Rishabh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-03-29 15:59 ` Rishabh Dave @ 2019-03-29 22:56 ` Theodore Ts'o 2019-04-10 16:22 ` Rishabh Dave 2019-04-10 16:23 ` Rishabh Dave 1 sibling, 1 reply; 9+ messages in thread From: Theodore Ts'o @ 2019-03-29 22:56 UTC (permalink / raw) To: Rishabh Dave; +Cc: Dave Chinner, fstests On Fri, Mar 29, 2019 at 09:29:43PM +0530, Rishabh Dave wrote: > > Is tests/generic/group a list of titles of testcases? If so, I don't > understand them. No. In each of the various tests folders, e.g., test/generic, tests/ext4, tests/xfs, tests/btrfs, etc. there is a file named group. This file tells the xfstests infrastructure which tests belong to which groups. The line: 026 acl quick auto Says that the test generic/026 is in three groups, "acl", "quick", and "auto". The "acl" group is for tests that tests acl. The "quick" group is for a set of tests than run quickly, for people who want to be able to do a quick validation that the file system is O.K. The "auto" group is for tests that should be used as part of an automated regression testing which is much more comprehensive than just running the "quick" tests. The generic/077 test also tests acl's. It also tests extended attributes, and what happens when the disk fills up (enospc): 077 acl attr auto enospc > I see there are few tests for ACLs in tests/generic/099. I couldn't > find them before since the tests use chacl and I was looking for > setfacl and getfacl. I'll compare my testsuite to tests/generic/099, > create a list of testcases not covered in xfstests-dev and start > rewriting. You need to look at a lot more tests than just tests/generic/099. You need at all of the tests that belong to group "acl". Which you can find from looking at the files named group in the subdirectory under the tests directory. Cheers, - Ted ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-03-29 22:56 ` Theodore Ts'o @ 2019-04-10 16:22 ` Rishabh Dave 0 siblings, 0 replies; 9+ messages in thread From: Rishabh Dave @ 2019-04-10 16:22 UTC (permalink / raw) To: Theodore Ts'o; +Cc: fstests Thanks Theodore Ts'o; your explanation was really helpful. :) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-03-29 15:59 ` Rishabh Dave 2019-03-29 22:56 ` Theodore Ts'o @ 2019-04-10 16:23 ` Rishabh Dave 2019-04-18 13:26 ` Rishabh Dave 1 sibling, 1 reply; 9+ messages in thread From: Rishabh Dave @ 2019-04-10 16:23 UTC (permalink / raw) To: Dave Chinner; +Cc: fstests Hi, I went through all the tests in last couple of days. If my understanding is correct, all the tests, besides the ones in tests/generic/099, are written for some specific bug. So, the following comparison of my Python testsuite[1] is with tests/generic/099. To begin with, there is a good amount of intersection between both the testsuite. What can be added to xfstests-dev is as follows - 1. ACL inheritance is being tested only for file owner, file group and other ACLs[2]. We can add 2 more testscases here for specific users and groups (i.e cases testing d:u:user:rwx and d:g:group:rwx). 2. Mask ACLs are only being tested for ACLs for specific user[3][4] and group[5] but not for file owner, file group and other. If I am right about this, we can add 3 more testcases here. 3. There are no testcases for mask inheritance. If we wish to add them, we'll have 5 more testcases. 4. And finally I don't see a testcase where we remove only the default ACLs from a directory. There are testcases that remove all ACLs (i.e. chacl -B)[6] and all file access ACLs (i.e. chacl -R)[7]. So, there's scope for one more testcase here. Besides, all my Python testcases varied from the ones written in xfstests-dev in following 2 ways: 1. All 3 permission bits were tested for all the cases. 2. Every testcase would also remove the ACL within the testcase. This served as test if setfacl -x works for all 4 categories of ACLs. If these 2 points are not superfluous, I can go ahead modify all the testcases accordingly. - Rishabh [1] https://github.com/ceph/ceph/pull/26477/commits/3d1c11b6990befb278320d3584602aa5d0078985 [2] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/099#n227 [3] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/099#n188 [4] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/099#n193 [5] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/099#n183 [6] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/099#n254 [7] https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/tree/tests/generic/099#n253 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-04-10 16:23 ` Rishabh Dave @ 2019-04-18 13:26 ` Rishabh Dave 2019-04-19 7:24 ` Eryu Guan 0 siblings, 1 reply; 9+ messages in thread From: Rishabh Dave @ 2019-04-18 13:26 UTC (permalink / raw) To: fstests Hi, Any opinions on my previous email? Should I proceed to add the new tests? - Rishabh ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: testsuite for ACLs 2019-04-18 13:26 ` Rishabh Dave @ 2019-04-19 7:24 ` Eryu Guan 0 siblings, 0 replies; 9+ messages in thread From: Eryu Guan @ 2019-04-19 7:24 UTC (permalink / raw) To: Rishabh Dave; +Cc: fstests On Thu, Apr 18, 2019 at 06:56:17PM +0530, Rishabh Dave wrote: > Hi, > > Any opinions on my previous email? Should I proceed to add the new tests? Sounds good to me, please go ahead add new tests and we could update patches based on review comments. Thanks a lot! Eryu ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-04-19 18:40 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-03-25 13:18 testsuite for ACLs Rishabh Dave 2019-03-25 20:20 ` Darrick J. Wong 2019-03-26 1:43 ` Dave Chinner 2019-03-29 15:59 ` Rishabh Dave 2019-03-29 22:56 ` Theodore Ts'o 2019-04-10 16:22 ` Rishabh Dave 2019-04-10 16:23 ` Rishabh Dave 2019-04-18 13:26 ` Rishabh Dave 2019-04-19 7:24 ` Eryu Guan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).