add new cloudflare ruleset commands#44533
Conversation
content-bot
added
Contribution
content-bot
changed the base branch from
master
to
contrib/andrew-paloalto_cloudflare-ruleset-list
|
Thank you for your contribution. Your generosity and caring are unrivaled! Make sure to register your contribution by filling the Contribution Registration form, so our content wizard @kamalq97 will know the proposed changes are ready to be reviewed. |
|
Hi @andrew-paloalto, thanks for contributing to the XSOAR marketplace. To receive credit for your generous contribution please follow this link. |
🤖 AI-Powered Code Review AvailableHi @kamalq97, you can leverage AI-powered code review to assist with this PR! Available Commands:
|
content-bot
added
Community
Contribution Form Filled
TheL0L
added
the
ready-for-ai-review
marketplace-ai-reviewer
removed
the
ready-for-ai-review
|
🤖 Analysis started. Please wait for results... |
🤖 AI Review DisclaimerThis review was generated by an AI-powered tool and may contain inaccuracies. Please be advised, and we extend our sincere apologies for any inconvenience this may cause. |
marketplace-ai-reviewer
left a comment
marketplace-ai-reviewer
left a comment
There was a problem hiding this comment.
Hi! Thanks for your contribution adding new ruleset commands to the Cloudflare WAF integration. I've left a few notes to help polish the PR, mostly focusing on standardizing descriptions and improving code robustness.
Please ensure you use .get() when accessing command arguments, add error handling for JSON parsing, and standardize the YAML descriptions (starting with "The" and ending with a period). Also, remember to update the README with the new commands and adjust the release notes version bump to reflect the new features rather than just a patch.
Thanks for your hard work!
Additionally, please address the following file-level notes:
Packs/CloudflareWAF/Integrations/CloudflareWAF/README.md: The new ruleset management commands are missing from the README.md.Packs/CloudflareWAF/Integrations/CloudflareWAF/CloudflareWAF.yml: The first key defined at the root level of the.ymlfile must always benameordisplay. Currently, the file starts withsectionorder. Please reorder the keys so thatnameordisplayis at the top.
Additionally, please ensure that the README.md file is updated to include documentation for the newly added commands (cloudflare-waf-ruleset-list, cloudflare-waf-ruleset-get, cloudflare-waf-ruleset-create, cloudflare-waf-ruleset-update, cloudflare-waf-ruleset-delete).
Packs/CloudflareWAF/pack_metadata.json: - The mandatory keycreatedis missing from the pack metadata.- The vendor name 'Cloudflare' is missing from the
keywordsarray. Packs/CloudflareWAF/ReleaseNotes/1_0_38.md: The version bump type (patch) does not match the nature of the changes (new features).
@TheL0L please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.
|
|
||
| return self._http_request(method="PUT", url_suffix=url_suffix, json_data=params) | ||
|
|
||
| def cloudflare_waf_ruleset_delete_request(self, ruleset_id: str, zone_id: str = None) -> dict[str, Any]: |
There was a problem hiding this comment.
Incorrect return type hint for resp_type="response"
| description = args.get("description") | ||
| rules_json = args.get("rules") | ||
|
|
||
| rules = json.loads(rules_json) if rules_json else None |
There was a problem hiding this comment.
Consider adding error handling for JSON parsing and validating the type.
| description = args.get("description") | ||
| rules_json = args.get("rules") | ||
|
|
||
| rules = json.loads(rules_json) if rules_json else None |
There was a problem hiding this comment.
Consider adding error handling for JSON parsing and validating the type.
| description: The list ID. | ||
| type: String | ||
| - arguments: | ||
| - description: Zone identifier. If provided, lists zone-level rulesets. If not provided, lists account-level rulesets. The value set in the instance configuration is used by default. |
There was a problem hiding this comment.
Descriptions for parameters, arguments, and context paths must start with "The " and end with a period.
| - contextPath: CloudflareWAF.Ruleset.phase | ||
| description: The phase of the ruleset. | ||
| type: String | ||
| - contextPath: CloudflareWAF.Ruleset.description |
There was a problem hiding this comment.
Descriptions for parameters, arguments, and context paths must start with "The " and end with a period.
| name: name | ||
| - description: A description of the ruleset. | ||
| name: description | ||
| - description: 'A JSON array of rule objects to set in the ruleset. This replaces all existing rules. For example: [{"action": "block", "expression": "(ip.src eq 192.0.2.1)", "description": "Block bad IP"}].' |
There was a problem hiding this comment.
Descriptions for parameters, arguments, and context paths must start with "The " and end with a period.
| name: description | ||
| - description: 'A JSON array of rule objects to set in the ruleset. This replaces all existing rules. For example: [{"action": "block", "expression": "(ip.src eq 192.0.2.1)", "description": "Block bad IP"}].' | ||
| name: rules | ||
| - description: Zone identifier. If provided, updates a zone-level ruleset. If not provided, updates an account-level ruleset. The value set in the instance configuration is used by default. |
There was a problem hiding this comment.
Descriptions for parameters, arguments, and context paths must start with "The " and end with a period.
| - contextPath: CloudflareWAF.Ruleset.phase | ||
| description: The phase of the ruleset. | ||
| type: String | ||
| - contextPath: CloudflareWAF.Ruleset.description |
There was a problem hiding this comment.
Descriptions for parameters, arguments, and context paths must start with "The " and end with a period.
| - contextPath: CloudflareWAF.Ruleset.version | ||
| description: The version of the ruleset. | ||
| type: String | ||
| - contextPath: CloudflareWAF.Ruleset.last_updated |
There was a problem hiding this comment.
Time/date formats must include examples in their description.
| - description: The ruleset identifier. | ||
| name: ruleset_id | ||
| required: true | ||
| - description: Zone identifier. If provided, deletes a zone-level ruleset. If not provided, deletes an account-level ruleset. The value set in the instance configuration is used by default. |
There was a problem hiding this comment.
Descriptions for parameters, arguments, and context paths must start with "The " and end with a period.
TheL0L
left a comment
TheL0L
left a comment
There was a problem hiding this comment.
Thank you @andrew-paloalto for your contribution!
Aside from refactoring the two for-loops mentioned in the comments above, everything looks great.
Please address the remaining feedback from @marketplace-ai-reviewer so we can get this merged.
| rulesets = [] | ||
| for ruleset in output: | ||
| rulesets.append( | ||
| { | ||
| "id": ruleset.get("id"), | ||
| "name": ruleset.get("name"), | ||
| "kind": ruleset.get("kind"), | ||
| "phase": ruleset.get("phase"), | ||
| "description": ruleset.get("description"), | ||
| "version": ruleset.get("version"), | ||
| "last_updated": ruleset.get("last_updated"), | ||
| } | ||
| ) |
There was a problem hiding this comment.
Is this for-loop necessary? Consider using the already extracted results list.
| rules = output.get("rules", []) | ||
| rules_table = [] | ||
| for rule in rules: | ||
| rules_table.append( | ||
| { | ||
| "id": rule.get("id"), | ||
| "action": rule.get("action"), | ||
| "expression": rule.get("expression"), | ||
| "description": rule.get("description"), | ||
| "enabled": rule.get("enabled"), | ||
| "version": rule.get("version"), | ||
| "ref": rule.get("ref"), | ||
| } | ||
| ) |
There was a problem hiding this comment.
Is this for-loop necessary? Consider using the already extracted rules list.
TheL0L
added
the
pending-contributor
5 participants
Contributing to Cortex XSOAR Content
Make sure to register your contribution by filling the contribution registration form
The Pull Request will be reviewed only after the contribution registration form is filled.
Status
Related Issues