Skip to content

Aggregate command block-domain - #45619

Draft
nbensalm-palo wants to merge 25 commits into
masterfrom
aggregate-command-block-domain
Draft

Aggregate command block-domain#45619
nbensalm-palo wants to merge 25 commits into
masterfrom
aggregate-command-block-domain

Conversation

@nbensalm-palo

Copy link
Copy Markdown
Contributor

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

https://jira-dc.paloaltonetworks.com/browse/CRTX-250429

Description

Added the block-domain script, which blocks one or more domains across your configured security products.

Must have

  • Tests
  • Documentation

@nbensalm-palo nbensalm-palo added ready-for-pipeline-running Whether the pr is ready for running the whole pipeline, including testing on SAAS machines ready-for-ai-review The PR is ready for reviewing the PR with the AI Reviewer. labels Aug 23, 2026
@content-bot

Copy link
Copy Markdown
Contributor

🤖 AI-Powered Code Review Available

You can leverage AI-powered code review to assist with this PR!

Available Commands:

  • @marketplace-ai-reviewer start review - Initiate a full AI code review
  • @marketplace-ai-reviewer re-review - Incremental review for new commits

@github-actions

github-actions Bot commented Aug 23, 2026

Copy link
Copy Markdown

Coverage

Coverage Report
FileStmtsMissCoverMissing
Packs/AggregatedScripts/Scripts/BlockDomain
   BlockDomain.py2945381%189–190, 233, 239, 466, 566, 647–651, 827–833, 836, 838–839, 841, 843, 849–850, 869–871, 874–878, 887–892, 913–921, 926, 928–929, 931–932
TOTAL2945381% 

Tests Skipped Failures Errors Time
45 0 💤 0 ❌ 0 🔥 3.135s ⏱️

@marketplace-ai-reviewer marketplace-ai-reviewer removed the ready-for-ai-review The PR is ready for reviewing the PR with the AI Reviewer. label Aug 23, 2026
@marketplace-ai-reviewer

Copy link
Copy Markdown
Contributor

🤖 Analysis started. Please wait for results...

Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Dismissed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Fixed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Fixed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Fixed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Fixed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Fixed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py Dismissed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py Fixed
@marketplace-ai-reviewer

Copy link
Copy Markdown
Contributor
🤖 AI Review Disclaimer

This 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 marketplace-ai-reviewer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks for your work on the BlockDomain script.

I've left a few notes to help polish the contribution, mainly around standardizing the YAML context paths (e.g., BlockDomain.Result.*), formatting the argument descriptions, and syncing the README. For the Python code, please add the missing type hints, ensure full tracebacks are logged during exceptions, and address a potential AttributeError.

Thanks again for the great work!

Additionally, please address the following file-level notes:

  • Packs/AggregatedScripts/pack_metadata.json: The vendor name (e.g., 'Cortex' or 'Palo Alto Networks') is missing from the keywords list. Please add it to improve searchability.

@nbensalm-palo please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.

Comment thread Packs/AggregatedScripts/ReleaseNotes/1_4_0.md
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py Outdated
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py Outdated
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml Outdated
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml Outdated
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml Outdated
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/README.md Outdated
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/README.md Outdated
@content-bot

This comment has been minimized.

Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Dismissed
Comment thread Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py Dismissed
@content-bot

This comment has been minimized.

@content-bot

This comment has been minimized.

@content-bot

This comment has been minimized.

@content-bot

Copy link
Copy Markdown
Contributor

Validate summary
The following errors were reported as warnings: BC117.
BC117 validation requires PM review due to changes to supportedModules.
The following errors were thrown as a part of this pr: PA124.
The following errors can be ignored: PA124.
If the AG100 validation in the pre-commit GitHub Action fails, the pull request cannot be force-merged.

Verdict: PR can be force merged from validate perspective? ❌

@Shellyber Shellyber left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't look at the code yet, will do after the demo + changes.
Let's add doc review as well


##### New: block-domain

- Added the **block-domain** script, which blocks one or more domains across your configured security products.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's mention that it currently supports only panorama

timeout: 20m0s
type: python
subtype: python3
compliantpolicies:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this for now and ignore the validation.
We will probably add block domain policy arorund Q2

@content-bot

Copy link
Copy Markdown
Contributor

🔍 AI Triage Report Available

An automated triage report has been generated for this pipeline.

Status: failed
Report ID: 5518fe8ba416109c

📋 Triage Report
💡 Resolutions are available in the full report.

⚠️ AI-generated triage. Validate before acting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-pipeline-running Whether the pr is ready for running the whole pipeline, including testing on SAAS machines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants