testsuite for ACLs

testsuite for ACLs

[Rishabh Dave]

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

Re: testsuite for ACLs

[Darrick J. Wong]

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

Re: testsuite for ACLs

[Dave Chinner]

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

Re: testsuite for ACLs

[Rishabh Dave]

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

Re: testsuite for ACLs

[Theodore Ts'o]

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

Re: testsuite for ACLs

[Rishabh Dave]

Thanks Theodore Ts'o; your explanation was really helpful. :)

Re: testsuite for ACLs

[Rishabh Dave]

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

Re: testsuite for ACLs

[Rishabh Dave]

Hi,

Any opinions on my previous email? Should I proceed to add the new tests?

- Rishabh

Re: testsuite for ACLs

[Eryu Guan]

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