diff mbox series

[v3] common/encrypt, ceph: add _require_not_encrypted test

Message ID 20211215190248.111263-1-jlayton@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v3] common/encrypt, ceph: add _require_not_encrypted test | expand

Commit Message

Jeffrey Layton Dec. 15, 2021, 7:02 p.m. UTC 
Some tests on ceph require changing the layout of new files, which is
forbidden when the files are encrypted. Add a test that touches a file
in $TEST_DIR and then tests it to see if it reports the
STATX_ATTR_ENCRYPTED flag, and does a _notrun if it's present.

Also add this requirement to three ceph tests that change the layout.

Cc: Luis Henriques <lhenriques@suse.de>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 common/encrypt | 18 ++++++++++++++++++
 tests/ceph/001 |  2 ++
 tests/ceph/002 |  2 ++
 tests/ceph/003 |  2 ++
 4 files changed, 24 insertions(+)

v2: make ceph/001 also call _require_not_encrypted

v3: move test into common/encrypt

Comments

Eric Biggers Dec. 16, 2021, 12:45 a.m. UTC | #1 
On Wed, Dec 15, 2021 at 02:02:48PM -0500, Jeff Layton wrote:
> Some tests on ceph require changing the layout of new files, which is
> forbidden when the files are encrypted. Add a test that touches a file
> in $TEST_DIR and then tests it to see if it reports the
> STATX_ATTR_ENCRYPTED flag, and does a _notrun if it's present.
> 
> Also add this requirement to three ceph tests that change the layout.
> 
> Cc: Luis Henriques <lhenriques@suse.de>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  common/encrypt | 18 ++++++++++++++++++
>  tests/ceph/001 |  2 ++
>  tests/ceph/002 |  2 ++
>  tests/ceph/003 |  2 ++
>  4 files changed, 24 insertions(+)
> 
> v2: make ceph/001 also call _require_not_encrypted
> 
> v3: move test into common/encrypt
> 
> diff --git a/common/encrypt b/common/encrypt
> index f90c4ef05a3f..156425af40a8 100644
> --- a/common/encrypt
> +++ b/common/encrypt
> @@ -940,3 +940,21 @@ _filter_nokey_filenames()
>  	# of characters that have ever been used in such names.
>  	sed "s|${dir}${dir:+/}[A-Za-z0-9+,_-]\+|${dir}${dir:+/}NOKEY_NAME|g"
>  }
> +
> +# Some tests require that encryption not be enabled.
> +_require_not_encrypted()
> +{
> +	local target=$TEST_DIR/require_not_encrypted.$$
> +	local ret=0
> +
> +	#
> +	# The top-level directory mounted with test_dummy_encryption is not
> +	# itself encrypted. Only new files and directories created under it
> +	# are.
> +	touch $target
> +	local attrs=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
> +	rm -f $target
> +
> +	# STATX_ATTR_ENCRYPTED == 0x800
> +	[ $(( attrs & 0x800 )) -eq 0 ] || _notrun "Filesystem is encrypted"
> +}

Did you consider instead skipping the test if the mount options contain
test_dummy_encryption?  That's the way that the encryption tests currently
detect that they shouldn't run (since the automatic encryption caused by
test_dummy_encryption would interfere with them); see
_require_scratch_encryption().  What you've done here will work too, but it
might make sense to check for test_dummy_encryption the same way in both places.

- Eric
Luis Henriques Dec. 16, 2021, 10:04 a.m. UTC | #2 
On Wed, Dec 15, 2021 at 02:02:48PM -0500, Jeff Layton wrote:
> Some tests on ceph require changing the layout of new files, which is
> forbidden when the files are encrypted. Add a test that touches a file
> in $TEST_DIR and then tests it to see if it reports the
> STATX_ATTR_ENCRYPTED flag, and does a _notrun if it's present.
> 
> Also add this requirement to three ceph tests that change the layout.
> 
> Cc: Luis Henriques <lhenriques@suse.de>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  common/encrypt | 18 ++++++++++++++++++
>  tests/ceph/001 |  2 ++
>  tests/ceph/002 |  2 ++
>  tests/ceph/003 |  2 ++
>  4 files changed, 24 insertions(+)
> 
> v2: make ceph/001 also call _require_not_encrypted
> 
> v3: move test into common/encrypt

Awesome, looks good to me.  Feel free to add my

Reviewed-by: Luis Henriques <lhenriques@suse.de>

(Although you may send another revision with Eric's suggestion...)

Cheers,
--
Luís


> diff --git a/common/encrypt b/common/encrypt
> index f90c4ef05a3f..156425af40a8 100644
> --- a/common/encrypt
> +++ b/common/encrypt
> @@ -940,3 +940,21 @@ _filter_nokey_filenames()
>  	# of characters that have ever been used in such names.
>  	sed "s|${dir}${dir:+/}[A-Za-z0-9+,_-]\+|${dir}${dir:+/}NOKEY_NAME|g"
>  }
> +
> +# Some tests require that encryption not be enabled.
> +_require_not_encrypted()
> +{
> +	local target=$TEST_DIR/require_not_encrypted.$$
> +	local ret=0
> +
> +	#
> +	# The top-level directory mounted with test_dummy_encryption is not
> +	# itself encrypted. Only new files and directories created under it
> +	# are.
> +	touch $target
> +	local attrs=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
> +	rm -f $target
> +
> +	# STATX_ATTR_ENCRYPTED == 0x800
> +	[ $(( attrs & 0x800 )) -eq 0 ] || _notrun "Filesystem is encrypted"
> +}
> diff --git a/tests/ceph/001 b/tests/ceph/001
> index c00de308fd95..d6ded026a6dd 100755
> --- a/tests/ceph/001
> +++ b/tests/ceph/001
> @@ -17,11 +17,13 @@ _begin_fstest auto quick copy_range
>  . common/filter
>  . common/attr
>  . common/reflink
> +. common/encrypt
>  
>  # real QA test starts here
>  _supported_fs ceph
>  
>  _require_xfs_io_command "copy_range"
> +_require_not_encrypted
>  _require_attrs
>  _require_test
>  
> diff --git a/tests/ceph/002 b/tests/ceph/002
> index 9bc728fd2e18..2232b75e6dfa 100755
> --- a/tests/ceph/002
> +++ b/tests/ceph/002
> @@ -25,11 +25,13 @@ _begin_fstest auto quick copy_range
>  # get standard environment
>  . common/filter
>  . common/attr
> +. common/encrypt
>  
>  # real QA test starts here
>  _supported_fs ceph
>  
>  _require_xfs_io_command "copy_range"
> +_require_not_encrypted
>  _require_attrs
>  _require_test
>  
> diff --git a/tests/ceph/003 b/tests/ceph/003
> index faedb48cfeea..aa130ae807f9 100755
> --- a/tests/ceph/003
> +++ b/tests/ceph/003
> @@ -13,11 +13,13 @@ _begin_fstest auto quick copy_range
>  . common/filter
>  . common/attr
>  . common/reflink
> +. common/encrypt
>  
>  # real QA test starts here
>  _supported_fs ceph
>  
>  _require_xfs_io_command "copy_range"
> +_require_not_encrypted
>  _require_attrs
>  _require_test
>  
> -- 
> 2.33.1
>
Eryu Guan Dec. 19, 2021, 1:20 p.m. UTC | #3 
On Wed, Dec 15, 2021 at 02:02:48PM -0500, Jeff Layton wrote:
> Some tests on ceph require changing the layout of new files, which is
> forbidden when the files are encrypted. Add a test that touches a file
> in $TEST_DIR and then tests it to see if it reports the
> STATX_ATTR_ENCRYPTED flag, and does a _notrun if it's present.
> 
> Also add this requirement to three ceph tests that change the layout.
> 
> Cc: Luis Henriques <lhenriques@suse.de>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  common/encrypt | 18 ++++++++++++++++++
>  tests/ceph/001 |  2 ++
>  tests/ceph/002 |  2 ++
>  tests/ceph/003 |  2 ++
>  4 files changed, 24 insertions(+)
> 
> v2: make ceph/001 also call _require_not_encrypted
> 
> v3: move test into common/encrypt
> 
> diff --git a/common/encrypt b/common/encrypt
> index f90c4ef05a3f..156425af40a8 100644
> --- a/common/encrypt
> +++ b/common/encrypt
> @@ -940,3 +940,21 @@ _filter_nokey_filenames()
>  	# of characters that have ever been used in such names.
>  	sed "s|${dir}${dir:+/}[A-Za-z0-9+,_-]\+|${dir}${dir:+/}NOKEY_NAME|g"
>  }
> +
> +# Some tests require that encryption not be enabled.
> +_require_not_encrypted()

We have some other helpers that require something not enabled or present,
and they're all in the format as:

_require_no_compress()
_require_no_realtime()
_require_no_xfs_debug()

So how about name it as _require_no_encryption() to be consistent with
other functions?

> +{
> +	local target=$TEST_DIR/require_not_encrypted.$$
> +	local ret=0
> +
> +	#
> +	# The top-level directory mounted with test_dummy_encryption is not
> +	# itself encrypted. Only new files and directories created under it
> +	# are.
> +	touch $target
> +	local attrs=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')

I think we may need to check if statx is supported as well

_require_xfs_io_command "statx" "-r"

Thanks,
Eryu

> +	rm -f $target
> +
> +	# STATX_ATTR_ENCRYPTED == 0x800
> +	[ $(( attrs & 0x800 )) -eq 0 ] || _notrun "Filesystem is encrypted"
> +}
> diff --git a/tests/ceph/001 b/tests/ceph/001
> index c00de308fd95..d6ded026a6dd 100755
> --- a/tests/ceph/001
> +++ b/tests/ceph/001
> @@ -17,11 +17,13 @@ _begin_fstest auto quick copy_range
>  . common/filter
>  . common/attr
>  . common/reflink
> +. common/encrypt
>  
>  # real QA test starts here
>  _supported_fs ceph
>  
>  _require_xfs_io_command "copy_range"
> +_require_not_encrypted
>  _require_attrs
>  _require_test
>  
> diff --git a/tests/ceph/002 b/tests/ceph/002
> index 9bc728fd2e18..2232b75e6dfa 100755
> --- a/tests/ceph/002
> +++ b/tests/ceph/002
> @@ -25,11 +25,13 @@ _begin_fstest auto quick copy_range
>  # get standard environment
>  . common/filter
>  . common/attr
> +. common/encrypt
>  
>  # real QA test starts here
>  _supported_fs ceph
>  
>  _require_xfs_io_command "copy_range"
> +_require_not_encrypted
>  _require_attrs
>  _require_test
>  
> diff --git a/tests/ceph/003 b/tests/ceph/003
> index faedb48cfeea..aa130ae807f9 100755
> --- a/tests/ceph/003
> +++ b/tests/ceph/003
> @@ -13,11 +13,13 @@ _begin_fstest auto quick copy_range
>  . common/filter
>  . common/attr
>  . common/reflink
> +. common/encrypt
>  
>  # real QA test starts here
>  _supported_fs ceph
>  
>  _require_xfs_io_command "copy_range"
> +_require_not_encrypted
>  _require_attrs
>  _require_test
>  
> -- 
> 2.33.1
diff mbox series

Patch

diff --git a/common/encrypt b/common/encrypt
index f90c4ef05a3f..156425af40a8 100644
--- a/common/encrypt
+++ b/common/encrypt
@@ -940,3 +940,21 @@  _filter_nokey_filenames()
 	# of characters that have ever been used in such names.
 	sed "s|${dir}${dir:+/}[A-Za-z0-9+,_-]\+|${dir}${dir:+/}NOKEY_NAME|g"
 }
+
+# Some tests require that encryption not be enabled.
+_require_not_encrypted()
+{
+	local target=$TEST_DIR/require_not_encrypted.$$
+	local ret=0
+
+	#
+	# The top-level directory mounted with test_dummy_encryption is not
+	# itself encrypted. Only new files and directories created under it
+	# are.
+	touch $target
+	local attrs=$($XFS_IO_PROG -c 'statx -r' $target | awk '/stat.attributes / { print $3 }')
+	rm -f $target
+
+	# STATX_ATTR_ENCRYPTED == 0x800
+	[ $(( attrs & 0x800 )) -eq 0 ] || _notrun "Filesystem is encrypted"
+}
diff --git a/tests/ceph/001 b/tests/ceph/001
index c00de308fd95..d6ded026a6dd 100755
--- a/tests/ceph/001
+++ b/tests/ceph/001
@@ -17,11 +17,13 @@  _begin_fstest auto quick copy_range
 . common/filter
 . common/attr
 . common/reflink
+. common/encrypt
 
 # real QA test starts here
 _supported_fs ceph
 
 _require_xfs_io_command "copy_range"
+_require_not_encrypted
 _require_attrs
 _require_test
 
diff --git a/tests/ceph/002 b/tests/ceph/002
index 9bc728fd2e18..2232b75e6dfa 100755
--- a/tests/ceph/002
+++ b/tests/ceph/002
@@ -25,11 +25,13 @@  _begin_fstest auto quick copy_range
 # get standard environment
 . common/filter
 . common/attr
+. common/encrypt
 
 # real QA test starts here
 _supported_fs ceph
 
 _require_xfs_io_command "copy_range"
+_require_not_encrypted
 _require_attrs
 _require_test
 
diff --git a/tests/ceph/003 b/tests/ceph/003
index faedb48cfeea..aa130ae807f9 100755
--- a/tests/ceph/003
+++ b/tests/ceph/003
@@ -13,11 +13,13 @@  _begin_fstest auto quick copy_range
 . common/filter
 . common/attr
 . common/reflink
+. common/encrypt
 
 # real QA test starts here
 _supported_fs ceph
 
 _require_xfs_io_command "copy_range"
+_require_not_encrypted
 _require_attrs
 _require_test