-
Notifications
You must be signed in to change notification settings - Fork 55
[SOAR-20236] Rapid7 InsightIDR - Triggers: Get New Alerts - Improved error handling | Updated SDK to the latest version (6.3.10) #3622
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
base: rapid7_insightidr-12.0.2-release
Are you sure you want to change the base?
Conversation
… handling | Updated SDK to the latest version (6.3.10)
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Version history states this updates the Get New Alerts trigger, but it appears the Get New Investigations trigger is being updated. |
python-dateutil==2.9.0 | ||
validators==0.34.0 | ||
aiohttp==3.12.14 | ||
python-dateutil==2.9.0.post0 |
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.
should we just use the full version (2.9.0) or is there a reason for using the post release version?
try: | ||
investigations = self.get_investigations(search, last_poll_time, current_time) | ||
except Exception as error: | ||
self.logger.error(error) |
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.
Would be better to prepend this with log a message indicating that get_investigations
has failed? (instead of logging the exception alone)
self.logger.error(error) | ||
self.logger.info(f"The request will be retried after {frequency} seconds...") | ||
time.sleep(frequency) | ||
continue |
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.
From the ticket I understand that the raised PluginException
is causing the container to restart - is the intention to avoid raising exceptions and retry indefinitely?
Just wondering what would happen here in the case of bad config (or some other error that will never resolve without intervention from the customer)
|
||
# Version History | ||
|
||
* 12.0.2 - Triggers: `Get New Alerts` - Improved error handling | Updated SDK to the latest version (6.3.10) |
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.
Typo - should be Investigations
# Flag to track the first execution | ||
first_execution = True | ||
# Initialize the trigger starting point | ||
last_poll_time = datetime.now(UTC) - timedelta(minutes=INITIAL_LOOKBACK_MINUTES) |
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.
do we want to change this default lookback from 20 minutes to 5?
for page_index in range(1, total_pages): | ||
self.logger.info(f"Pulling data from page - ({page_index + 1}/{total_pages})") | ||
response = self._call_search_api( | ||
request, endpoint, "POST", payload, {"size": TOTAL_SIZE, "index": page_index} |
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.
I haven't seen the API docs but before this seemed to pass 100, but I'm guessing that was a bug, this does look more accurate
params: Dict[str, Any] = None, | ||
) -> Dict[str, Any]: | ||
response = resource_helper.resource_request(endpoint, method, payload=payload, params=params) | ||
return json.loads(response.get("resource", "{}")) |
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.
should we still catch if any invalid jSON is returned or are we happy maybe just having the generic try/except catch it?
|
||
# Back off before next iteration (sleep for 15 seconds) | ||
# Update last poll time for next iteration | ||
last_poll_time = current_time |
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 a 500 isn't thrown, is this not incorrectly moving the time forward and then we could miss investigations?
except Exception as error: | ||
self.logger.error(error) | ||
self.logger.info(f"The request will be retried after {frequency} seconds...") | ||
time.sleep(frequency) |
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.
do we need this time.sleep() we've got one already at the end of the trigger?
Proposed Changes
Description
Describe the proposed changes:
Get New Alerts
- Improved error handlingPR Requirements
Developers, verify you have completed the following items by checking them off:
Testing
Unit Tests
Review our documentation on generating and writing plugin unit tests
In-Product Tests
If you are an InsightConnect customer or have access to an InsightConnect instance, the following in-product tests should be done:
Style
Review the style guide
USER nobody
in theDockerfile
when possiblerapid7/insightconnect-python-3-38-slim-plugin:{sdk-version-num}
andrapid7/insightconnect-python-3-38-plugin:{sdk-version-num}
insight-plugin validate
which callsicon_validate
to linthelp.md
Functional Checklist
tests/
directory created withinsight-plugin samples
tests/$action_bad.json
insight-plugin run -T tests/example.json --debug --jq
insight-plugin run -T all --debug --jq
(use PR format at end)insight-plugin run -R tests/example.json --debug --jq
insight-plugin run --debug --jq
(use PR format at end)Assessment
You must validate your work to reviewers:
insight-plugin validate
and make sure everything passesinsight-plugin run -A
. For single action validation:insight-plugin run tests/{file}.json -A
insight-plugin ... | pbcopy
) and paste the output in a new post on this PR