Add tests for getfacl(1)

[shivansh]

Summary

Meta information:
atf_check -s exit:0 -o match:'acls' echo "$(mount | head -n1)" is a workaround for atf_check -s exit:0 -o match:'acls' mount | head -n1 (I cannot figure out why the second version doesn't work, it seems to ignore the command after pipe).

Test Plan

• Run make install from src/bin/cat/tests.
• Run kyua test from /usr/tests/bin/cat. All 4 test cases should succeed.

[shivansh]

Remove nop call for setting umask (shodowed by setfacl(1))

[shivansh]

• Refactor test-case names
• Update commit message

[asomers]

You're right that atf_check -s exit:0 -o match:'acls' mount | head -n1 ignores the part after the pipe. It's an order of operations problem. That command line takes the output of atf_check -s exit:0 -o match:'acls' mount and pipes it to head -n. Perhaps you meant to do atf_check -s exit:0 -o match:'acls' -x "mount | head -n1" ?

[asomers]

Don't forget to update bin/getfacl/Makefile

[asomers]

bin/getfacl/tests/getfacl_test.sh

33

This isn't reliable. There's no guarantee on the order of filesystems returned by mount. If you want to check the root partition, you should try something like mount | awk '$3 == "/"'

Also, you seem to be using this as a gate for whether to run tests. In that case, it's inappropriate to ues atf_check, because that will fail the test if ACLs aren't enabled. Instead, you should skip the test if ACLs aren't enabled.

Finally, Kyua doesn't use / for temporary files. Instead, it uses $TMPDIR. But you shouldn't rely on that. Instead, you should check the filesystem where you happen to be executing. Try something like this:

fs=`df . | awk '$NF ~ "/" {print $1}'`
if mount | awk -v fs=$fs '$1 == fs' | grep -q acls; then
    # ok
else
   atf_skip "ACLs not enabled"
fi

[asomers]

bin/getfacl/tests/getfacl_test.sh

72

This will only work for POSIX.1e ACLs. FreeBSD supports two incompatible ACL formats: POSIX.1e and NFSv4. I think the best thing to do would be to write two sets of tests: one for each ACL format. On any one run, one set would be skipped, depending on what filesystem Kyua is using. Or, you could write tests just for POSIX.1e ACLs, because that's what FreeBSD's CI system uses. In that case, you should modify check_acl to exclude filesystems with NFSv4 ACLs.

[shivansh]

• Update "check_acl()" to confirm if POSIX.1e ACLs are enabled
• atf_set "require.user" "root" added to enable execution of tunefs(8)

[shivansh]

• Remove redundant check

[shivansh]

• Improve the check for confirming POSIX.1e ACLs (sorry for the spam! :sigh:)

[asomers]

As written, the tests only work for POSIX 1.e ACLs and only work for UFS. It would be best if they supported both ACL formats and both filesystems. But if they don't, then they should explicitly say so, probably in the test case names. And don't forget to update bin/getfacl/Makefile

bin/getfacl/tests/getfacl_test.sh

34

tunefs will only work for UFS. Can you write something that works for ZFS too? Try getconf TRUSTEDBSD_ACL_NFS4 . and getconf TRUSTEDBSD_ACL_EXTENDED . .

[ngie]

bin/getfacl/tests/getfacl_test.sh

31–38

1. Using getconf(1) is better (cuts to the chase and is not reliant on how the filesystem information is presented):

_PC_ACL_EXTENDED
        Returns 1 if an Access Control List (ACL) can be set on the
        specified file, otherwise 0.

_PC_ACL_NFS4
        Returns 1 if an NFSv4 ACLs can be set on the specified file,
        otherwise 0.

For example:

# UFS (without POSIX ACLs enabled)
$ getconf ACL_EXTENDED /mnt/tmp/
0
# UFS (with POSIX ACLs enabled)
# XXX: had to unmount -- mount -ru /mnt/tmp updated the superblock, but
#      the value returned via *pathconf(2) didn't change.
# $ sudo mount -ru /mnt/tmp
$ sudo umount /mnt/tmp
$ sudo tunefs -a enable /dev/md0
$ sudo mount /dev/md0 /mnt/tmp
$ getconf ACL_EXTENDED /mnt/tmp
1
# ZFS
$ getconf ACL_EXTENDED /
0

If you want to be really slick and get full coverage, I'd create a temporary UFS file system as I showed above and cleanup at test completion. I need to code this up generically though--I have some common patterns in contrib/netbsd-tests and elsewhere that illustrates how one can do this.

2. It might be a good idea to check sysctl kern.features.ufs_acl to make sure it's available and skip the UFS tests if it's not available -- otherwise the filesystem won't support it and you'll get confusing errors with newfs/tunefs.

[ngie]

bin/getfacl/tests/getfacl_test.sh

34

Don't use TRUSTEDBSD_ACL_EXTENDED -- that isn't documented as well as ACL_EXTENDED is (I should really remove that in another CR I send out to rwatson@ -- it's an old backwards compat knob that doesn't make much sense IMHO).

[asomers]

@shivansh Do you still plan to update this review?

[shivansh]

Oh no! I totally missed this diff, sorry ; Will update it today itself!

[shivansh]

bin/getfacl/tests/getfacl_test.sh

31–38

@ngie I found out src/contrib/netbsd-tests/fs/tmpfs/h_funcs.subr in which there are custom functions to create mount points and mount/unmount them (test_mount() and test_unmount()).

I have some common patterns in contrib/netbsd-tests and elsewhere that illustrates how one can do this.

Is this the pattern that you previously mentioned ?

In case this is the pattern which you mentioned above, is it advisable for me to use this file itself via . $(atf_get_srcdir)/$(traverse to appropriate location)/h_funcs.subr or should I make a similar file inside src/bin/getfacl/tests.