fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] generic/079 test fails if 'nobody' and 'daemon' don't exist
@ 2021-09-23 10:41 Luis Henriques
  2021-09-23 10:41 ` [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists Luis Henriques
  2021-09-23 10:41 ` [PATCH 2/2] generic/079: make sure users 'nobody' and 'daemon' exist Luis Henriques
  0 siblings, 2 replies; 6+ messages in thread
From: Luis Henriques @ 2021-09-23 10:41 UTC (permalink / raw)
  To: fstests; +Cc: Eryu Guan, Luis Chamberlain, Luis Henriques

Hi!

Test generic/079 currently fails if for some reason the 'nobody' or
'daemon' users don't exist:

*** starting up
add_acl(/mnt/scratch/079/immutable.f) did not set errno == EPERM
add_acl(/mnt/scratch/079/immutable.d) did not set errno == EPERM
add_acl(/mnt/scratch/079/append-only.f) did not set errno == EPERM
add_acl(/mnt/scratch/079/append-only.d) did not set errno == EPERM
testing immutable...FAILED! (2 tests failed)
testing append-only...FAILED! (2 tests failed)
*** cleaning up

The first patch that follows adds a new function _require_user_exists()
that will verify a user exists in the system (and refactors existent
_require_user() to use the new function).

The second patch is the actual test fix.

Luis Henriques (2):
  common/rc: add _require_user_exists() to check if a user exists
  generic/079: make sure users 'nobody' and 'daemon' exist

 common/rc         | 27 ++++++++++++++++++---------
 tests/generic/079 |  2 ++
 2 files changed, 20 insertions(+), 9 deletions(-)


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists
  2021-09-23 10:41 [PATCH 0/2] generic/079 test fails if 'nobody' and 'daemon' don't exist Luis Henriques
@ 2021-09-23 10:41 ` Luis Henriques
  2021-09-23 19:47   ` Darrick J. Wong
  2021-09-23 19:53   ` Eric Biggers
  2021-09-23 10:41 ` [PATCH 2/2] generic/079: make sure users 'nobody' and 'daemon' exist Luis Henriques
  1 sibling, 2 replies; 6+ messages in thread
From: Luis Henriques @ 2021-09-23 10:41 UTC (permalink / raw)
  To: fstests; +Cc: Eryu Guan, Luis Chamberlain, Luis Henriques

Function _require_user() does check if a user exists *and* if it is able
to execute commands.  Add a new function to simply check if a user exists.

Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
 common/rc | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/common/rc b/common/rc
index 154bc2dd7e94..c7e77c0e26dc 100644
--- a/common/rc
+++ b/common/rc
@@ -2289,18 +2289,27 @@ _cat_group()
 	cat /etc/group
 }
 
-# check for a user on the machine, fsgqa as default
+# check if a user exists in the system
+#
+_require_user_exists()
+{
+	user=$1
+	_cat_passwd | grep -q $user
+	[ "$?" == "0" ] || _notrun "$user user not defined."
+}
+
+# check if a user exists and is able to execute commands.
+# Uses 'fsgqa' user as default.
 #
 _require_user()
 {
-    qa_user=fsgqa
-    if [ -n "$1" ];then
-        qa_user=$1
-    fi
-    _cat_passwd | grep -q $qa_user
-    [ "$?" == "0" ] || _notrun "$qa_user user not defined."
-    echo /bin/true | su $qa_user
-    [ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
+	qa_user=fsgqa
+	if [ -n "$1" ];then
+		qa_user=$1
+	fi
+	_require_user_exists $qa_user
+	echo /bin/true | su $qa_user
+	[ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
 }
 
 # check for a chown support

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] generic/079: make sure users 'nobody' and 'daemon' exist
  2021-09-23 10:41 [PATCH 0/2] generic/079 test fails if 'nobody' and 'daemon' don't exist Luis Henriques
  2021-09-23 10:41 ` [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists Luis Henriques
@ 2021-09-23 10:41 ` Luis Henriques
  2021-09-23 19:48   ` Darrick J. Wong
  1 sibling, 1 reply; 6+ messages in thread
From: Luis Henriques @ 2021-09-23 10:41 UTC (permalink / raw)
  To: fstests; +Cc: Eryu Guan, Luis Chamberlain, Luis Henriques

Test 'src/t_immutable' assumes the existence of users 'nobody' and
'daemon' on the system.  If any of these users don't exist, the test will
fail with:

  add_acl(/media/scratch/079/immutable.f) did not set errno == EPERM
  ...

because it tries to set ACLs such as:

  u::rwx,g::rwx,o::rwx,u:daemon:rwx,m::rwx

Use the new _require_user_exists() function to ensure these users exist.

Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
 tests/generic/079 | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/generic/079 b/tests/generic/079
index 829275db8cdf..9e7ccd31c8e2 100755
--- a/tests/generic/079
+++ b/tests/generic/079
@@ -26,6 +26,8 @@ _cleanup()
 _supported_fs generic
 
 _require_chattr ia
+_require_user_exists "nobody"
+_require_user_exists "daemon"
 _require_test_program "t_immutable"
 _require_scratch
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists
  2021-09-23 10:41 ` [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists Luis Henriques
@ 2021-09-23 19:47   ` Darrick J. Wong
  2021-09-23 19:53   ` Eric Biggers
  1 sibling, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-09-23 19:47 UTC (permalink / raw)
  To: Luis Henriques; +Cc: fstests, Eryu Guan, Luis Chamberlain

On Thu, Sep 23, 2021 at 11:41:22AM +0100, Luis Henriques wrote:
> Function _require_user() does check if a user exists *and* if it is able
> to execute commands.  Add a new function to simply check if a user exists.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.de>

Looks like a simple enough hoist.

You /could/ streamline the bash constructs too, but ... fmeh.  This is
easy to review, at least.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  common/rc | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 154bc2dd7e94..c7e77c0e26dc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2289,18 +2289,27 @@ _cat_group()
>  	cat /etc/group
>  }
>  
> -# check for a user on the machine, fsgqa as default
> +# check if a user exists in the system
> +#
> +_require_user_exists()
> +{
> +	user=$1
> +	_cat_passwd | grep -q $user
> +	[ "$?" == "0" ] || _notrun "$user user not defined."
> +}
> +
> +# check if a user exists and is able to execute commands.
> +# Uses 'fsgqa' user as default.
>  #
>  _require_user()
>  {
> -    qa_user=fsgqa
> -    if [ -n "$1" ];then
> -        qa_user=$1
> -    fi
> -    _cat_passwd | grep -q $qa_user
> -    [ "$?" == "0" ] || _notrun "$qa_user user not defined."
> -    echo /bin/true | su $qa_user
> -    [ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
> +	qa_user=fsgqa
> +	if [ -n "$1" ];then
> +		qa_user=$1
> +	fi
> +	_require_user_exists $qa_user
> +	echo /bin/true | su $qa_user
> +	[ "$?" == "0" ] || _notrun "$qa_user cannot execute commands."
>  }
>  
>  # check for a chown support

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] generic/079: make sure users 'nobody' and 'daemon' exist
  2021-09-23 10:41 ` [PATCH 2/2] generic/079: make sure users 'nobody' and 'daemon' exist Luis Henriques
@ 2021-09-23 19:48   ` Darrick J. Wong
  0 siblings, 0 replies; 6+ messages in thread
From: Darrick J. Wong @ 2021-09-23 19:48 UTC (permalink / raw)
  To: Luis Henriques; +Cc: fstests, Eryu Guan, Luis Chamberlain

On Thu, Sep 23, 2021 at 11:41:23AM +0100, Luis Henriques wrote:
> Test 'src/t_immutable' assumes the existence of users 'nobody' and
> 'daemon' on the system.  If any of these users don't exist, the test will
> fail with:
> 
>   add_acl(/media/scratch/079/immutable.f) did not set errno == EPERM
>   ...
> 
> because it tries to set ACLs such as:
> 
>   u::rwx,g::rwx,o::rwx,u:daemon:rwx,m::rwx
> 
> Use the new _require_user_exists() function to ensure these users exist.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.de>

Looks correct.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  tests/generic/079 | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/generic/079 b/tests/generic/079
> index 829275db8cdf..9e7ccd31c8e2 100755
> --- a/tests/generic/079
> +++ b/tests/generic/079
> @@ -26,6 +26,8 @@ _cleanup()
>  _supported_fs generic
>  
>  _require_chattr ia
> +_require_user_exists "nobody"
> +_require_user_exists "daemon"
>  _require_test_program "t_immutable"
>  _require_scratch
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists
  2021-09-23 10:41 ` [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists Luis Henriques
  2021-09-23 19:47   ` Darrick J. Wong
@ 2021-09-23 19:53   ` Eric Biggers
  1 sibling, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2021-09-23 19:53 UTC (permalink / raw)
  To: Luis Henriques; +Cc: fstests, Eryu Guan, Luis Chamberlain

On Thu, Sep 23, 2021 at 11:41:22AM +0100, Luis Henriques wrote:
> Function _require_user() does check if a user exists *and* if it is able
> to execute commands.  Add a new function to simply check if a user exists.
> 
> Signed-off-by: Luis Henriques <lhenriques@suse.de>
> ---
>  common/rc | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 154bc2dd7e94..c7e77c0e26dc 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2289,18 +2289,27 @@ _cat_group()
>  	cat /etc/group
>  }
>  
> -# check for a user on the machine, fsgqa as default
> +# check if a user exists in the system
> +#
> +_require_user_exists()
> +{
> +	user=$1
> +	_cat_passwd | grep -q $user
> +	[ "$?" == "0" ] || _notrun "$user user not defined."
> +}

Please make "user" a local variable so that it doesn't overwrite another
variable with the same name.

- Eric

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-09-23 19:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23 10:41 [PATCH 0/2] generic/079 test fails if 'nobody' and 'daemon' don't exist Luis Henriques
2021-09-23 10:41 ` [PATCH 1/2] common/rc: add _require_user_exists() to check if a user exists Luis Henriques
2021-09-23 19:47   ` Darrick J. Wong
2021-09-23 19:53   ` Eric Biggers
2021-09-23 10:41 ` [PATCH 2/2] generic/079: make sure users 'nobody' and 'daemon' exist Luis Henriques
2021-09-23 19:48   ` Darrick J. Wong

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).