On Tue, Jan 16, 2024 at 10:19:36AM +0100, Richard Weinberger wrote:
> Dave,
>
> ----- Ursprüngliche Mail -----
> > Von: "Dave Chinner" <david@xxxxxxxxxxxxx>
> > 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?
Thanks
Zorro
>
> I think _require_user_exists() fits better, because _require_user()
> tests whether the user can run commands via su.
>
> Are we all happy with something like that?
> diff --git a/common/rc b/common/rc
> index a9e0ba7e..f1eabf3c 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2461,13 +2461,16 @@ _cat_group()
> cat /etc/group
> }
>
> -# check if a user exists in the system
> +# check if a user exists in the system and has a user group
> #
> _require_user_exists()
> {
> local user=$1
> _cat_passwd | grep -q "^$user:"
> [ "$?" == "0" ] || _notrun "$user user not defined."
> +
> + id -n -G $user | grep -w -q $user
> + [ "$?" == "0" ] || _notrun "$user not in group $user."
> }
>
> # check if a user exists and is able to execute commands.
>
> Thanks,
> //richard
>
