-
Notifications
You must be signed in to change notification settings - Fork 786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
2004 scan target configuration #2133
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2133 +/- ##
===========================================
+ Coverage 56.64% 56.89% +0.24%
===========================================
Files 486 489 +3
Lines 13226 13278 +52
===========================================
+ Hits 7492 7554 +62
+ Misses 5734 5724 -10
Continue to review full report at Codecov.
|
monkey/tests/unit_tests/common/agent_configuration/validators/test_ip_ranges.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're not going to be using the IP range validators anywhere else, move them to be inside the schema class, as done with CustomPBAConfigurationSchema
. Or move that validation out to a separate file. The former makes more sense to me but in any case, this needs to be consistent.
We can use them on any hostname, ip or ip range input. Moving them into schema would require some work and I'm not sure we even gain anything. I would rather move the windows filename validation away into validators. It makes the schema smaller, separates validation responsibility and once we need to upload more files we won't need to rework the schema |
if match and match.group() == hostname: | ||
return | ||
else: | ||
raise ValidationError(f"Invalid hostname {hostname}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if match and match.group() == hostname: | |
return | |
else: | |
raise ValidationError(f"Invalid hostname {hostname}") | |
if not (match and match.group() == hostname): | |
raise ValidationError(f"Invalid hostname {hostname}") |
try: | ||
ip_range = ip_range.replace(" ", "") | ||
ips = ip_range.split("-") | ||
validate_ip(ips[0]) | ||
validate_ip(ips[1]) | ||
if len(ips) != 2: | ||
raise ValidationError(f"Invalid IP range {ip_range}") | ||
except (AddressValueError, IndexError): | ||
raise ValidationError(f"Invalid IP range {ip_range}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do the length check first you don't need to handle IndexError
.
try: | |
ip_range = ip_range.replace(" ", "") | |
ips = ip_range.split("-") | |
validate_ip(ips[0]) | |
validate_ip(ips[1]) | |
if len(ips) != 2: | |
raise ValidationError(f"Invalid IP range {ip_range}") | |
except (AddressValueError, IndexError): | |
raise ValidationError(f"Invalid IP range {ip_range}") | |
ip_range = ip_range.replace(" ", "") | |
ips = ip_range.split("-") | |
if len(ips) != 2: | |
raise ValidationError(f"Invalid IP range {ip_range}") | |
try: | |
validate_ip(ips[0]) | |
validate_ip(ips[1]) | |
except AddressValueError: | |
raise ValidationError(f"Invalid IP range {ip_range}") |
c8e5d53
to
a1760a8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UTs missing for ScanTargetConfiguration
in tests/unit_tests/common/configuration/test_agent_configuration.py
and for common/agent_configuration/validators/filenames.py
def validate_windows_filename(windows_filename: str): | ||
validate_windows_filename_not_reserved(windows_filename) | ||
if not re.match(_valid_windows_filename_regex, windows_filename): | ||
raise ValidationError(f"Invalid Windows filename {windows_filename}: illegal characters") | ||
|
||
|
||
def validate_windows_filename_not_reserved(windows_filename: str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def validate_windows_filename(windows_filename: str): | |
validate_windows_filename_not_reserved(windows_filename) | |
if not re.match(_valid_windows_filename_regex, windows_filename): | |
raise ValidationError(f"Invalid Windows filename {windows_filename}: illegal characters") | |
def validate_windows_filename_not_reserved(windows_filename: str): | |
def validate_windows_filename(windows_filename: str): | |
_validate_windows_filename_not_reserved(windows_filename) | |
if not re.match(_valid_windows_filename_regex, windows_filename): | |
raise ValidationError(f"Invalid Windows filename {windows_filename}: illegal characters") | |
def _validate_windows_filename_not_reserved(windows_filename: str): |
|
||
def validate_windows_filename_not_reserved(windows_filename: str): | ||
# filename shouldn't start with any of these and be followed by a period | ||
if PureWindowsPath(windows_filename).is_reserved(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic!
What does this PR do?
Fixes network target configuration part of #2004
Add any further explanations here.
PR Checklist
Testing Checklist