DPDK: Refactor rdma-core source build, allow arbitrary package build #3110
Conversation
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
from
27fd8c7 to
cfe22b9
Compare
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
from
6bd48ba to
dc03e6b
Compare
lisa/util/__init__.py
Outdated
| @@ -47,7 +48,7 @@ | |||
| # source - | |||
| # https://github.com/django/django/blob/stable/1.3.x/django/core/validators.py#L45 | |||
| __url_pattern = re.compile( | |||
| r"^(?:http|ftp)s?://" # http:// or https:// | |||
| r"^(?:http|s?ftp)s?://" # http:// or https:// | |||
There was a problem hiding this comment.
- Update the comments to make it consistent with the patterns.
- Expand the patterns make it easy to read, and avoid invalid pattern like
sftps,ftps.(?:http|https|sftp|ftp)
There was a problem hiding this comment.
Either remove the comments or make it consistent with patterns.
lisa/util/__init__.py
Outdated
| if raise_error: | ||
| raise LisaException(failure_msg) | ||
| else: | ||
| log.info(failure_msg) |
There was a problem hiding this comment.
It should be debug level by default, because it's not worth to raise an exception. It means it's ignorable error.
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
from
dc03e6b to
bd77550
Compare
lisa/util/__init__.py
Outdated
| def check_url( | ||
| log: Logger, | ||
| source_url: str, | ||
| expected_package_name_pattern: Optional[Pattern[str]] = None, |
There was a problem hiding this comment.
I want to emphasize it's just the filename portion, I'll make it expected_filename_pattern
There was a problem hiding this comment.
If it's a regexp pattern, you can just check the end parts. If it'a name like filename, it limits usages to relative/full path validation.
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
from
bd77550 to
494ffb3
Compare
lisa/util/__init__.py
Outdated
| source_url: str, | ||
| expected_package_name_pattern: Optional[Pattern[str]] = None, | ||
| allowed_protocols: Optional[List[str]] = None, | ||
| expected_domains: Optional[List[str]] = None, |
There was a problem hiding this comment.
expected_domain_pattern. One pattern should be enough to validate all.
There was a problem hiding this comment.
I started with just a regex match but it was a pain with annoying edge cases, so I started writing this version. My goal is making it easier for the caller, this way you can pass a couple of domains, a couple of protocols, and a filename pattern (or just a string) and restrict future users from downloading whatever tarball from wherever later with little room for error.
There was a problem hiding this comment.
long story short I'm writing it for me so I don't mess it up next time I go to do this 😅
There was a problem hiding this comment.
You can test regexp at someplace like https://regex101.com. if it's string comparison, it will meet more problems actually. Like how to allow/disallow subdomains, or attack like "invalidmicrosoft.com"?
There was a problem hiding this comment.
Honestly, wish I hadn't even tried to handle this at this point.
There was a problem hiding this comment.
okedokey, moved to a pattern for domains,
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
3 times, most recently
from
1917e3c to
82a4aa5
Compare
Fulfilling an ask to be able to test multiple rdma-core versions to allow development and regression tests. Setting rdma_core_source to either a git or tar.gz link allows you to build from source. For git either set rdma_core_ref or it will pick the latest version sorted tag.
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
5 times, most recently
from
4b47ec6 to
bb107e1
Compare
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
2 times, most recently
from
98c48e5 to
00c47a2
Compare
mcgov
force-pushed
the
mcgov/rdma-core-source
branch
from
00c47a2 to
642a523
Compare
Fulfilling an ask to be able to test multiple rdma-core versions to allow development and regression tests. Refactors the rdma-core bits into their own own class.
Setting rdma_core_source to either a git or tar.gz link allows you to build from source. For git either set rdma_core_ref or it will pick the latest version sorted tag.