{Network} Add DDoS Protection feature: DDoS Policy Customization #32673
Conversation
william051200
requested review from
kairu-ms,
necusjz and
wangzelin007
as code owners
️✔️AzureCLI-FullTest
|
|
Hi @william051200, |
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| network ddos-custom-policy | sub group network ddos-custom-policy added |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
There was a problem hiding this comment.
Pull request overview
This PR adds support for DDoS Policy Customization by introducing new commands to manage DDoS custom policies in Azure CLI. The implementation includes commands for creating, updating, deleting, and showing DDoS custom policies with detection rules.
Changes:
- Added new command group
az network ddos-custom-policywith create, update, delete, show, and wait commands - Implemented custom create logic to support adding detection rules to existing policies
- Added comprehensive test coverage for the new functionality
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test_network_commands.py | Added comprehensive scenario tests for DDoS custom policy CRUD operations |
| commands.py | Registered new ddos-custom-policy command group |
| custom.py | Implemented custom create logic with policy merging and format conversion |
| _template_builder.py | Added builder function for constructing DDoS custom policy objects |
| _params.py | Defined command parameters for create operation |
| _help.py | Added help documentation with examples |
| AAZ generated files | Auto-generated command implementations for create, update, delete, show, and wait |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # region DdosCustomPolicy | ||
| with self.command_group('network ddos-custom-policy') as g: | ||
| g.custom_show_command('create', 'create_ddos_custom_policy', supports_no_wait=True) |
There was a problem hiding this comment.
The command is registered using 'custom_show_command' which is incorrect. The 'create' command should be registered using 'custom_command' instead, as it's not a show/read operation. Using 'custom_show_command' for a create operation is semantically incorrect and could lead to unexpected behavior or confusion.
| g.custom_show_command('create', 'create_ddos_custom_policy', supports_no_wait=True) | |
| g.custom_command('create', 'create_ddos_custom_policy', supports_no_wait=True) |
|
|
||
|
|
||
| def build_ddos_custom_policy(cmd, ddos_custom_policy_name, location=None, tags=None, detection_rule_name=None, | ||
| detection_mode=None, packets_per_second=None, traffic_type=False, ip_config_id=None): |
There was a problem hiding this comment.
The parameter 'traffic_type' has a default value of 'False' (boolean) when it should likely be 'None' or not have a default value. This parameter is supposed to be a string representing traffic type (Tcp, Udp, TcpSyn), not a boolean. This will cause incorrect behavior when the parameter is not provided.
| detection_mode=None, packets_per_second=None, traffic_type=False, ip_config_id=None): | |
| detection_mode=None, packets_per_second=None, traffic_type=None, ip_config_id=None): |
| except Exception as err: | ||
| pass |
There was a problem hiding this comment.
The exception handling uses a bare 'except Exception' with 'pass', silently ignoring all exceptions. This could hide important errors. Consider catching specific exceptions (e.g., ResourceNotFoundError) or at minimum logging the exception for debugging purposes.
| except Exception as err: | |
| pass | |
| except ResourceNotFoundError: | |
| # No existing DDoS custom policy; proceed with creation. | |
| logger.debug("DDoS custom policy '%s' not found in resource group '%s'.", | |
| ddos_custom_policy_name, resource_group_name) | |
| except Exception as err: # pylint: disable=broad-except | |
| # Log unexpected errors while preserving previous behavior of not failing the command. | |
| logger.warning("Failed to retrieve existing DDoS custom policy '%s' in resource group '%s': %s", | |
| ddos_custom_policy_name, resource_group_name, err) |
| az network ddos-custom-policy create --resource-group rg1 --ddos-custom-policy-name test- | ||
| ddos-custom-policy --location centraluseuap --detection-rule-name detectionRuleTcp | ||
| --detection-mode TrafficThreshold --traffic-type Tcp --packets-per-second 1000000 |
There was a problem hiding this comment.
The help text for the create command example is split across lines 6254-6256 in an awkward way that will result in improper formatting. The command should be written on a single line or use proper line continuation to ensure it displays correctly in the help output.
| az network ddos-custom-policy create --resource-group rg1 --ddos-custom-policy-name test- | |
| ddos-custom-policy --location centraluseuap --detection-rule-name detectionRuleTcp | |
| --detection-mode TrafficThreshold --traffic-type Tcp --packets-per-second 1000000 | |
| az network ddos-custom-policy create --resource-group rg1 --ddos-custom-policy-name test-ddos-custom-policy \\ | |
| --location centraluseuap --detection-rule-name detectionRuleTcp \\ | |
| --detection-mode TrafficThreshold --traffic-type Tcp --packets-per-second 1000000 |
3 participants
Related command
az network ddos-custom-policy createaz network ddos-custom-policy updateaz network ddos-custom-policy deleteaz network ddos-custom-policy showDescription
Resolve #32529
aaz Azure/aaz#923
Testing Guide
Create custom policy with rule
az network ddos-custom-policy create --detection-rule-name --detection-mode --traffic-type --packets-per-secondShow and validate created custom policy
az network ddos-custom-policy showCreate a second rule within the same policy
az network ddos-custom-policy create --detection-rule-name --detection-mode --traffic-type --packets-per-secondUpdate first rule name and make sure the second rule stays the same
az network ddos-custom-policy update --set detectionRules[0].name=<name>Remove first rule and make sure it only have 1 rule left and it is the second rule
az network ddos-custom-policy update --remove detectionRules 0Delete the policy and make sure it is deleted
az network ddos-custom-policy deleteHistory Notes
[Network]
az network ddos-custom-policy: Support DDoS Policy CustomizationThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.