[PATCH] test: make cached assets for functional tests read-only

[PATCH] test: make cached assets for functional tests read-only

[Daniel P. Berrangé]

This ensures that if a functional test runs QEMU with a writable
disk pointing to a cached asset, an error will be reported, rather
than silently modifying the cache file.

As an example, tweaking test_sbsaref.py to set snapshot=off,
results in a clear error:

  Command: ./build/qemu-system-aarch64 ...snip... -drive file=/var/home/berrange/.cache/qemu/download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461,format=raw,snapshot=off -cpu max,pauth=off
  Output: qemu-system-aarch64: Could not open '/var/home/berrange/.cache/qemu/download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461': Permission denied

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/asset.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index e47bfac035..f126cd5863 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -8,6 +8,7 @@
 import hashlib
 import logging
 import os
+import stat
 import subprocess
 import sys
 import unittest
@@ -143,6 +144,8 @@ def fetch(self):
             raise Exception("Hash of %s does not match %s" %
                             (self.url, self.hash))
         tmp_cache_file.replace(self.cache_file)
+        # Remove write perms to stop tests accidentally modifying them
+        os.chmod(self.cache_file, stat.S_IRUSR | stat.S_IRGRP)
 
         self.log.info("Cached %s at %s" % (self.url, self.cache_file))
         return str(self.cache_file)
-- 
2.46.0

Re: [PATCH] test: make cached assets for functional tests read-only

[Philippe Mathieu-Daudé]

On 21/10/24 09:36, Daniel P. Berrangé wrote:
> This ensures that if a functional test runs QEMU with a writable
> disk pointing to a cached asset, an error will be reported, rather
> than silently modifying the cache file.
> 
> As an example, tweaking test_sbsaref.py to set snapshot=off,
> results in a clear error:
> 
>    Command: ./build/qemu-system-aarch64 ...snip... -drive file=/var/home/berrange/.cache/qemu/download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461,format=raw,snapshot=off -cpu max,pauth=off
>    Output: qemu-system-aarch64: Could not open '/var/home/berrange/.cache/qemu/download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461': Permission denied
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/asset.py | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] test: make cached assets for functional tests read-only

[Thomas Huth]

On 21/10/2024 14.36, Daniel P. Berrangé wrote:
> This ensures that if a functional test runs QEMU with a writable
> disk pointing to a cached asset, an error will be reported, rather
> than silently modifying the cache file.
> 
> As an example, tweaking test_sbsaref.py to set snapshot=off,
> results in a clear error:
> 
>    Command: ./build/qemu-system-aarch64 ...snip... -drive file=/var/home/berrange/.cache/qemu/download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461,format=raw,snapshot=off -cpu max,pauth=off
>    Output: qemu-system-aarch64: Could not open '/var/home/berrange/.cache/qemu/download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461': Permission denied
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/asset.py | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>
Tested-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH] test: make cached assets for functional tests read-only

[Thomas Huth]

On 21/10/2024 17.02, Thomas Huth wrote:
> On 21/10/2024 14.36, Daniel P. Berrangé wrote:
>> This ensures that if a functional test runs QEMU with a writable
>> disk pointing to a cached asset, an error will be reported, rather
>> than silently modifying the cache file.
>>
>> As an example, tweaking test_sbsaref.py to set snapshot=off,
>> results in a clear error:
>>
>>    Command: ./build/qemu-system-aarch64 ...snip... -drive file=/var/home/ 
>> berrange/.cache/qemu/ 
>> download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461,format=raw,snapshot=off -cpu max,pauth=off
>>    Output: qemu-system-aarch64: Could not open '/var/home/berrange/.cache/ 
>> qemu/ 
>> download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461': Permission denied
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>   tests/functional/qemu_test/asset.py | 3 +++
>>   1 file changed, 3 insertions(+)
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Tested-by: Thomas Huth <thuth@redhat.com>

Actually, if you start with a clean cache, or if you manually do:

  chmod a-w ~/.cache/qemu/download/*

a couple of tests are failing now. Could you please have a look?

  Thanks
   Thomas

Re: [PATCH] test: make cached assets for functional tests read-only

[Daniel P. Berrangé]

On Fri, Oct 25, 2024 at 09:47:07AM +0200, Thomas Huth wrote:
> On 21/10/2024 17.02, Thomas Huth wrote:
> > On 21/10/2024 14.36, Daniel P. Berrangé wrote:
> > > This ensures that if a functional test runs QEMU with a writable
> > > disk pointing to a cached asset, an error will be reported, rather
> > > than silently modifying the cache file.
> > > 
> > > As an example, tweaking test_sbsaref.py to set snapshot=off,
> > > results in a clear error:
> > > 
> > >    Command: ./build/qemu-system-aarch64 ...snip... -drive
> > > file=/var/home/ berrange/.cache/qemu/ download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461,format=raw,snapshot=off
> > > -cpu max,pauth=off
> > >    Output: qemu-system-aarch64: Could not open
> > > '/var/home/berrange/.cache/ qemu/ download/44cdbae275ef1bb6dab1d5fbb59473d4f741e1c8ea8a80fd9e906b531d6ad461':
> > > Permission denied
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > >   tests/functional/qemu_test/asset.py | 3 +++
> > >   1 file changed, 3 insertions(+)
> > 
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Tested-by: Thomas Huth <thuth@redhat.com>
> 
> Actually, if you start with a clean cache, or if you manually do:
> 
>  chmod a-w ~/.cache/qemu/download/*
> 
> a couple of tests are failing now. Could you please have a look?

Oooh, I thought I tested with all targets enabled, but I guess I must
have missed something.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|