| Message ID | 20190329210804.22121-4-wainersm@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/vm: Python 3, improve image caching, and misc | expand |
Wainer dos Santos Moschetta <wainersm@redhat.com> writes: > The current implementation of basevm does not check if the image > file to be downloaded has changed on server side before honouring > the cache. So any change on server-side file can go unnoticed, > keeping the cached image. > > This change implements a simple mechanism to detect the image > file changed by using the sha256sum file stored on server. It > compares with the expected checksum and then abort the execution > on mismatch. > > Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> > --- > tests/vm/basevm.py | 31 ++++++++++++++++++++++++++++++- > 1 file changed, 30 insertions(+), 1 deletion(-) > > diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py > index 083befce9f..4dfad2dc9b 100755 > --- a/tests/vm/basevm.py > +++ b/tests/vm/basevm.py > @@ -27,6 +27,7 @@ import tempfile > import shutil > import multiprocessing > import traceback > +import urllib.request > > SSH_KEY = open(os.path.join(os.path.dirname(__file__), > "..", "keys", "id_rsa")).read() > @@ -81,6 +82,18 @@ class BaseVM(object): > self._data_args = [] > > def _download_with_cache(self, url, sha256sum=None): > + > + def fetch_image_hash(url): > + fetch_url = "%s.sha256sum" % url OK this fails with the FreeBSD code as they use the form: https://download.freebsd.org/ftp/releases/ISO-IMAGES/12.0/CHECKSUM.SHA256-FreeBSD-12.0-RELEASE-amd64 I guess we need to have a method that can be overridden for this. > + try: > + with urllib.request.urlopen(fetch_url) as response: > + content = response.read() > + except urllib.error.URLError as error: > + logging.error("Failed to fetch image checksum file: %s", > + fetch_url) > + raise error > + return content.decode().strip() > + > def check_sha256sum(fname): > if not sha256sum: > return True > @@ -91,8 +104,24 @@ class BaseVM(object): > if not os.path.exists(cache_dir): > os.makedirs(cache_dir) > fname = os.path.join(cache_dir, hashlib.sha1(url.encode()).hexdigest()) > - if os.path.exists(fname) and check_sha256sum(fname): > + > + if os.path.exists(fname) and sha256sum is None: > return fname > + > + if sha256sum: > + image_checksum = fetch_image_hash(url) > + # Check the url points to a known image file. > + if image_checksum != sha256sum: > + logging.error("Image %s checksum (%s) does not match " + > + "expected (%s).", url, image_checksum, sha256sum) > + raise Exception("Image checksum failed.") > + # Check the cached image is up to date. > + if os.path.exists(fname): > + if check_sha256sum(fname): > + return fname > + logging.warning("Invalid cached image. Attempt to download " + > + "the updated one.") > + > logging.debug("Downloading %s to %s...", url, fname) > subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"], > stdout=self._stdout, stderr=self._stderr) -- Alex Bennée
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 083befce9f..4dfad2dc9b 100755 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -27,6 +27,7 @@ import tempfile import shutil import multiprocessing import traceback +import urllib.request SSH_KEY = open(os.path.join(os.path.dirname(__file__), "..", "keys", "id_rsa")).read() @@ -81,6 +82,18 @@ class BaseVM(object): self._data_args = [] def _download_with_cache(self, url, sha256sum=None): + + def fetch_image_hash(url): + fetch_url = "%s.sha256sum" % url + try: + with urllib.request.urlopen(fetch_url) as response: + content = response.read() + except urllib.error.URLError as error: + logging.error("Failed to fetch image checksum file: %s", + fetch_url) + raise error + return content.decode().strip() + def check_sha256sum(fname): if not sha256sum: return True @@ -91,8 +104,24 @@ class BaseVM(object): if not os.path.exists(cache_dir): os.makedirs(cache_dir) fname = os.path.join(cache_dir, hashlib.sha1(url.encode()).hexdigest()) - if os.path.exists(fname) and check_sha256sum(fname): + + if os.path.exists(fname) and sha256sum is None: return fname + + if sha256sum: + image_checksum = fetch_image_hash(url) + # Check the url points to a known image file. + if image_checksum != sha256sum: + logging.error("Image %s checksum (%s) does not match " + + "expected (%s).", url, image_checksum, sha256sum) + raise Exception("Image checksum failed.") + # Check the cached image is up to date. + if os.path.exists(fname): + if check_sha256sum(fname): + return fname + logging.warning("Invalid cached image. Attempt to download " + + "the updated one.") + logging.debug("Downloading %s to %s...", url, fname) subprocess.check_call(["wget", "-c", url, "-O", fname + ".download"], stdout=self._stdout, stderr=self._stderr)
The current implementation of basevm does not check if the image file to be downloaded has changed on server side before honouring the cache. So any change on server-side file can go unnoticed, keeping the cached image. This change implements a simple mechanism to detect the image file changed by using the sha256sum file stored on server. It compares with the expected checksum and then abort the execution on mismatch. Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com> --- tests/vm/basevm.py | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)