Re: [PATCH] generic/193: Ensure user in expected group

[Richard Weinberger <richard@nod.at>]

----- Ursprüngliche Mail -----
> Von: "Zorro Lang" <zlang@redhat.com>
> An: "richard" <richard@nod.at>
> CC: "Dave Chinner" <david@fromorbit.com>, "fstests" <fstests@vger.kernel.org>
> Gesendet: Dienstag, 16. Januar 2024 18:10:55
> Betreff: Re: [PATCH] generic/193: Ensure user in expected group

> On Tue, Jan 16, 2024 at 10:19:36AM +0100, Richard Weinberger wrote:
>> Dave,
>> 
>> ----- Ursprüngliche Mail -----
>> > Von: "Dave Chinner" <david@fromorbit.com>
>> > Just do this check in _require_user(). The user needs to be set up
>> > correctly for all tests - if the user and group is not set up
>> > correctly, don't run any of the tests that require that user.
>> > 
>> > This means we don't have to play whack-a-mole with "user has no
>> > group" every time someone assumes that a user is created with a
>> > group by default.
> 
> I think Dave is right. Due to not only the g/193, lots of cases base
> on "fsgqa* user is in fsgqa* group". If you hope to check that by
> the single function _require_user_in_group(), it might need to call
> it in most of cases which use _require_user.
> 
> But yes, as you said, it's weird to do it in _require_user, that makes
> the _require_group calling is a bit redundant.
> 
> If I have to choose one way, I think we can keep the _require_user_in_group
> as an independent function, but call it in _require_user, e.g.
> 
> _require_user()
> {
>        qa_user=fsgqa
>        if [ -n "$1" ];then
>                qa_user=$1
>        fi
>        _require_user_exists $qa_user
> +	# fstests needs each test user is in its own group name, it only need
> +	# a user exist, please use _require_user_exists directly.
> +	_require_user_in_group $qa_user $qa_user
>        echo /bin/true | su $qa_user
>        [ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
> }
> 
> What do both of you think?

Fine by me. :-)

Thanks,
//richard