refactor and rename check_package_source_url

lisa/util/__init__.py

@@ -33,7 +33,7 @@
 from marshmallow import fields
 from semver import VersionInfo
 
-from lisa import secret
+from lisa import LisaException, secret
 from lisa.util import constants
 from lisa.util.perf_timer import create_timer
 
@@ -601,11 +601,21 @@ def is_valid_url(url: str, raise_error: bool = True) -> bool:
     return is_url
 
 
-def is_valid_source_code_package(
+def _raise_or_log_failure(log: Logger, raise_error: bool, failure_msg: str) -> bool:
+    if raise_error:
+        raise LisaException(failure_msg)
+    else:
+        log.info(failure_msg)
+        return False
+
+
+def check_package_source_url(
+    log: Logger,
     source_url: str,
     expected_package_name_pattern: Pattern[str],
     allowed_protocols: Optional[List[str]] = None,
     expected_domains: Optional[List[str]] = None,
+    raise_error: bool = False,
 ) -> bool:
     # avoid using a mutable default parameter
     if not allowed_protocols:
@@ -614,18 +624,23 @@ def is_valid_source_code_package(
             "sftp",
         ]
     # first, check if it's a url.
-    if not is_valid_url(url=source_url, raise_error=False):
+    failure_msg = f"{source_url} is not a valid URL, check your arguments."
+    if not (
+        is_valid_url(url=source_url, raise_error=False)
+        or _raise_or_log_failure(log, raise_error, failure_msg)
+    ):
         return False
 
     # NOTE: urllib might not work as you'd expect.
     # It doesn't throw on lots of things you wouldn't expect to be urls.
     # You must verify the parts on your own, some of them may be empty, some null.
     # check: https://docs.python.org/3/library/urllib.parse.html#url-parsing
-
+    failure_msg = f"urlparse failed to parse url {source_url}, check your arguments."
     try:
         parts = urlparse(source_url)
     except ValueError:
-        return False
+        if not _raise_or_log_failure(log, raise_error, failure_msg):
+            return False
 
     # ex: from https://www.com/path/to/file.tar
     # scheme : https
@@ -635,22 +650,47 @@ def is_valid_source_code_package(
     # get the filename from the path portion of the url
     file_path = parts.path.split("/")[-1]
     full_match = expected_package_name_pattern.match(file_path)
+    failure_msg = (
+        f"File at {source_url} did not validate with pattern "
+        "{expected_package_name_pattern.pattern}."
+    )
     if not full_match:
-        return False
+        if not _raise_or_log_failure(log, raise_error, failure_msg):
+            return False
 
     # check the expected domain is correct if present
     valid_netloc = not expected_domains or any(
         [domain.endswith(parts.netloc) for domain in expected_domains]
     )
+    failure_msg = (
+        f"net location of url {source_url} did not match "
+        f"expected domains { ','.join(expected_domains) } "
+    )
+    if not (valid_netloc or _raise_or_log_failure(log, raise_error, failure_msg)):
+        return False
 
     # optional but default is check access is via sftp/https
+    failure_msg = (
+        f"URL {source_url} uses an invalid protocol "
+        "or net location! Check url argument."
+    )
+
     valid_scheme = any([parts.scheme == x for x in allowed_protocols])
-    return (
-        valid_scheme
-        and parts.netloc != ""
-        and valid_netloc
-        and (full_match.group(0) == file_path)
+    valid_netloc_and_scheme = valid_scheme and parts.netloc != "" and valid_netloc
+    if not (
+        valid_netloc_and_scheme or _raise_or_log_failure(log, raise_error, failure_msg)
+    ):
+        return False
+
+    path_matches = full_match.group(0) == file_path
+    failure_msg = (
+        f"File at url {source_url} failed to match"
+        f" pattern {expected_package_name_pattern.pattern}."
     )
+    if not (path_matches or _raise_or_log_failure(log, raise_error, failure_msg)):
+        return False
+
+    return True
 
 
 def filter_ansi_escape(content: str) -> str:

microsoft/testsuites/dpdk/rdma_core.py

@@ -9,7 +9,7 @@
 from lisa import Node
 from lisa.operating_system import Debian, Fedora, Suse
 from lisa.tools import Git, Make, Pkgconfig, Tar, Wget
-from lisa.util import LisaException, SkippedException, is_valid_source_code_package
+from lisa.util import LisaException, SkippedException, check_package_source_url
 
 
 class RdmaCoreManager:
@@ -77,7 +77,8 @@ def _check_source_install(self) -> None:
             )
 
         # finally, validate what we have looks reasonable and cool
-        is_valid_package = is_valid_source_code_package(
+        is_valid_package = check_package_source_url(
+            self.node.log,
             source_url=self._rdma_core_source,
             expected_package_name_pattern=self._source_pattern,
             allowed_protocols=["https"],