[SOAR-17745] Duo admin Improve handling of 429s in task code #3048
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lcwiklinski-r7
changed the title
lcwiklinski-r7
requested review from
cmcnally-r7,
igorski-r7,
joneill-r7 and
rmurray-r7
| @@ -64,7 +64,9 @@ class MonitorLogsOutput(insightconnect_plugin_runtime.Output): | |||
| "type": "array", | |||
| "title": "Logs", | |||
| "description": "List of administrator, authentication and trust monitor event logs within the specified time range", | |||
| "items": {}, | |||
| "items": { | |||
| "$ref": {} | |||
There was a problem hiding this comment.
This needs to be reverted :( Sdk refresh does this automatically
rmurray-r7
reviewed
Jan 14, 2025
plugins/duo_admin/komand_duo_admin/tasks/monitor_logs/schema.py
Outdated
Show resolved
Hide resolved
rmurray-r7
reviewed
Jan 14, 2025
cmcnally-r7
requested changes
Jan 14, 2025
rmurray-r7
reviewed
Jan 14, 2025
rmurray-r7
reviewed
Jan 14, 2025
rmurray-r7
approved these changes
Jan 14, 2025
igorski-r7
reviewed
Jan 14, 2025
| @@ -103,7 +108,46 @@ def get_parameters_for_query( | |||
| self.logger.info(f"Retrieve data from {mintime} to {maxtime}. Get next page is set to {get_next_page}") | |||
| return mintime, maxtime, get_next_page | |||
|
|
|||
| def check_rate_limit(self, state: Dict): | |||
There was a problem hiding this comment.
Suggested change
| def check_rate_limit(self, state: Dict): | |
| def check_rate_limit(self, state: Dict[str, Any]) -> Union[PluginException, None]: |
joneill-r7
reviewed
Jan 14, 2025
| @@ -285,7 +330,7 @@ def add_log_type_field(logs: list, value: str) -> list: | |||
|
|
|||
| @staticmethod | |||
| def sha1(log: dict) -> str: | |||
| hash_ = sha1() # nosec B303 | |||
| hash_ = sha1(usedforsecurity=False) # nosec B303 | |||
There was a problem hiding this comment.
we've got a hashing method we can call from the SDK we can use now, if you want to swap out to use that
| @@ -37,6 +37,12 @@ def setUpClass(cls) -> None: | |||
| "lookback": "2023-04-30T08:34:46.000Z", | |||
| }, | |||
| ], | |||
| [ | |||
| "with_rate_limit", | |||
There was a problem hiding this comment.
do you mind adding a test that has the whole flow of:
- happy state -> API returns 429 -> task returns 200 with rate limit in state -> second task call returns 429 as task status code
and maybe one showing the rate limit time has passed and we resume task? just i know these can be a bit tricky to test locally!
rmurray-r7
reviewed
Jan 16, 2025
plugins/whois/.CHECKSUM
Outdated
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "spec": "4d67f7597ca0c9b43ff85bc3d161836e", | |||
| "spec": "bd19980a894fe29ad2516339a3668832", | |||
There was a problem hiding this comment.
Why is there Whois files in here?
rmurray-r7
reviewed
Jan 16, 2025
plugins/whois/plugin.spec.yaml
Outdated
| @@ -32,7 +32,8 @@ hub_tags: | |||
| key_features: ["Perform a WHOIS lookup for a provided IP address or domain to gain information on who is responsible for a domain or IP"] | |||
| references: ["[WHOIS](https://en.wikipedia.org/wiki/WHOIS)"] | |||
| links: ["[WHOIS](https://en.wikipedia.org/wiki/WHOIS)"] | |||
| troubleshooting: "Multiple records can be returned by the server, this plugin currently only returns the first unique records found." | |||
| troubleshooting: | |||
There was a problem hiding this comment.
Yeah these Whois files shouldn't be in the PR.
lcwiklinski-r7
force-pushed
the
SOAR-17745_duo_admin_Improve-handling-of-429s-in-task-code
branch
from
2bb8a2c to
54c0ec4
Compare
rmurray-r7
approved these changes
Jan 16, 2025
igorski-r7
approved these changes
Jan 16, 2025
ekelly-r7
reviewed
Jan 17, 2025
cmcnally-r7
approved these changes
Jan 17, 2025
rmurray-r7
force-pushed
the
SOAR-17745_duo_admin_Improve-handling-of-429s-in-task-code
branch
2 times, most recently
from
360f870 to
86f9f03
Compare
rmurray-r7
approved these changes
Jan 21, 2025
igorski-r7
approved these changes
Jan 21, 2025
ekelly-r7
approved these changes
Jan 21, 2025
cmcnally-r7
approved these changes
Jan 21, 2025
lcwiklinski-r7
force-pushed
the
SOAR-17745_duo_admin_Improve-handling-of-429s-in-task-code
branch
from
083267f to
7791e66
Compare
lcwiklinski-r7
deleted the
SOAR-17745_duo_admin_Improve-handling-of-429s-in-task-code
branch
igorski-r7
pushed a commit
that referenced
this pull request
Jan 27, 2025
* Updating plugin.spec (#3047) * Duo admin Improve handling of 429s in task code * Linting the code * Version bump * Adjusting styling * Resolving static code analysis * Resolving plugin's validation * CR fixes * Additional test case * Bumping the SDK and linting the code --------- Co-authored-by: rmurray-r7 <ryanj_murray@rapid7.com>
lcwiklinski-r7
added a commit
that referenced
this pull request
Jan 29, 2025
* Updating plugin.spec (#3047) * Duo admin Improve handling of 429s in task code * Linting the code * Version bump * Adjusting styling * Resolving static code analysis * Resolving plugin's validation * CR fixes * Additional test case * Bumping the SDK and linting the code --------- Co-authored-by: rmurray-r7 <ryanj_murray@rapid7.com>
lcwiklinski-r7
added a commit
that referenced
this pull request
Jan 29, 2025
…3067) * Updating plugin.spec (#3047) * Duo admin Improve handling of 429s in task code * Linting the code * Version bump * Adjusting styling * Resolving static code analysis * Resolving plugin's validation * CR fixes * Additional test case * Bumping the SDK and linting the code --------- Co-authored-by: rmurray-r7 <ryanj_murray@rapid7.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://rapid7.atlassian.net/browse/SOAR-17745
Unit Tests
Review our documentation on generating and writing plugin unit tests