Add DDoS custom policy association to Public IP address cmdlets - #29925
Add DDoS custom policy association to Public IP address cmdlets#29925venkatasaimaram7 wants to merge 11 commits into
Conversation
Adds support to associate a DDoS custom policy (DCP) with a Public IP address (including instance-level Public IPs), mirroring the existing LB frontend IP association (PR Azure#29409). - New-AzPublicIpAddress: added -DdosCustomPolicyId parameter. - Set-AzPublicIpAddress: added -DdosCustomPolicyId parameter and -RemoveDdosCustomPolicy switch (mutually exclusive) to attach/detach. - Updated ChangeLog.md, markdown help, and added a scenario test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fa69d393-8fac-4e81-a80d-d51a123bf30d
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Adds support in the Az.Network Public IP address cmdlets to associate/detach a DDoS custom policy (DCP) with a Public IP address, aligning behavior with the existing Load Balancer frontend IP association support.
Changes:
- Added
-DdosCustomPolicyIdsupport toNew-AzPublicIpAddressandSet-AzPublicIpAddress. - Added
-RemoveDdosCustomPolicytoSet-AzPublicIpAddressfor detaching an existing association. - Updated Network module changelog, cmdlet markdown help, and added a scenario test covering attach/detach/reattach flows.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Network/Network/PublicIpAddress/SetAzurePublicIpAddressCommand.cs | Adds parameters and runtime validation to attach/detach DDoS custom policy on update. |
| src/Network/Network/PublicIpAddress/NewAzurePublicIpAddressCommand.cs | Adds parameter and creation-time validation to attach DDoS custom policy on create. |
| src/Network/Network/help/Set-AzPublicIpAddress.md | Documents new -DdosCustomPolicyId / -RemoveDdosCustomPolicy parameters. |
| src/Network/Network/help/New-AzPublicIpAddress.md | Documents new -DdosCustomPolicyId parameter. |
| src/Network/Network/ChangeLog.md | Adds Upcoming Release entry describing the new Public IP DDoS custom policy association support. |
| src/Network/Network.Test/ScenarioTests/PublicIpAddressTests.ps1 | Adds scenario test for creating a Public IP with DCP and then detaching/reattaching via Set. |
Comments suppressed due to low confidence (1)
src/Network/Network/PublicIpAddress/NewAzurePublicIpAddressCommand.cs:277
- Indentation in the Tier block is inconsistent (extra indentation on the
ifline) andif(publicIp.Sku == null)is missing a space, which reduces readability and makes the block look mis-nested.
if (!string.IsNullOrEmpty(this.Tier))
{
if(publicIp.Sku == null)
{
publicIp.Sku = new PSPublicIpAddressSku();
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2d89d7-0339-446a-bb1e-c55b085eb052
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/Network/Network/PublicIpAddress/NewAzurePublicIpAddressCommand.cs:276
- The
Tierblock has inconsistent indentation and spacing (e.g.,if(publicIp.Sku == null)), which makes the control flow harder to read and deviates from the surrounding style in this method.
if (!string.IsNullOrEmpty(this.Tier))
{
if(publicIp.Sku == null)
{
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2d89d7-0339-446a-bb1e-c55b085eb052
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/Network/Network/help/Set-AzPublicIpAddress.md:150
- Parameter description mentions "instance-level" and requires NIC attachment, but the cmdlet only validates that DDoS protection mode is
Enabled. Update the parameter description to reflect the enforced requirement (or add corresponding validation in code).
### -DdosCustomPolicyId
The DDoS custom policy ID to associate with an instance-level public IP address.
The public IP address must be attached to a network interface and have its DDoS protection mode set to `Enabled`.
src/Network/Network/help/Set-AzPublicIpAddress.md:89
- The Example 4 narrative says the public IP "must be attached to a network interface" and is "instance-level", but the cmdlet implementation only enforces that
DdosSettings.ProtectionModeisEnabled(no NIC/instance-level validation). Please align the help text with the actual behavior (or add validation in the cmdlet if NIC attachment is truly required).
This issue also appears on line 148 of the same file.
The first command gets a Standard public IP address that is attached to a network interface and
has DDoS protection mode set to `Enabled`. The second command associates the DDoS custom policy
with that instance-level public IP address.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2d89d7-0339-446a-bb1e-c55b085eb052
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/Network/Network/PublicIpAddress/SetAzurePublicIpAddressCommand.cs:62
- The cmdlet performs a network GET via IsPublicIpAddressPresent() before checking that -DdosCustomPolicyId and -RemoveDdosCustomPolicy are mutually exclusive. When both are supplied, this makes an unnecessary service call (and changes failure mode to a remote not-found check rather than immediate parameter validation). Consider validating the mutual exclusivity before calling IsPublicIpAddressPresent().
if (this.RemoveDdosCustomPolicy.IsPresent && !string.IsNullOrEmpty(this.DdosCustomPolicyId))
{
throw new ArgumentException("Specify either DdosCustomPolicyId or RemoveDdosCustomPolicy, but not both.");
}
src/Network/Network/ChangeLog.md:24
- PR description states that New-AzPublicIpAddress adds -DdosCustomPolicyId, but no corresponding change appears in the codebase (the only new Public IP cmdlet parameter added is on Set-AzPublicIpAddress). Please either update the PR description to match the actual scope, or add the missing New-AzPublicIpAddress support before merging.
* Added support to associate a DDoS custom policy (DCP) with a supported Public IP address attachment.
- Added the `-DdosCustomPolicyId` parameter to `Set-AzPublicIpAddress`.
- Added the `-RemoveDdosCustomPolicy` switch to remove an existing association.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2d89d7-0339-446a-bb1e-c55b085eb052
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/Network/Network/PublicIpAddress/SetAzurePublicIpAddressCommand.cs:44
- Parameter help message is missing a trailing period, while similar cmdlets end HelpMessage sentences with punctuation (for example SetAzureRmLoadBalancerFrontendIpConfigCommand.cs:133).
HelpMessage = "Removes the DDoS custom policy association from the Public IP address")]
src/Network/Network/PublicIpAddress/SetAzurePublicIpAddressCommand.cs:37
- Parameter help message uses "id" instead of the more consistent "ID" (for example, the related Load Balancer cmdlet uses "resource ID" in SetAzureRmLoadBalancerFrontendIpConfigCommand.cs:126). Consider updating for consistency across cmdlets.
This issue also appears on line 44 of the same file.
HelpMessage = "The DDoS custom policy id to attach to the Public IP address. Requires the Public IP address ProtectionMode to be 'Enabled'.")]
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2d89d7-0339-446a-bb1e-c55b085eb052
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/Network/Network/PublicIpAddress/SetAzurePublicIpAddressCommand.cs:70
- This ArgumentException is user-facing but doesn’t include the parameter name; using the overload with paramName improves the PowerShell error record and matches common patterns in this module.
throw new ArgumentException("DdosCustomPolicyId can only be set when the Public IP address has ProtectionMode set to 'Enabled'.");
src/Network/Network/help/Set-AzPublicIpAddress.md:196
- To align with the cmdlet behavior, call out that -RemoveDdosCustomPolicy can’t be used together with -DdosCustomPolicyId in the parameter description.
Removes the DDoS custom policy association from the Public IP address.
src/Network/Network/PublicIpAddress/SetAzurePublicIpAddressCommand.cs:57
- The mutually exclusive-parameter validation throws an ArgumentException without a parameter name, which results in less actionable PowerShell errors. Other cmdlets in this repo commonly include the parameter name (e.g., src/Network/Network/AzureFirewallPolicy/AzureFirewallPolicyNetworkRuleProtocolHelper.cs:41). Consider using the ArgumentException overload that includes a paramName so the error record points users at the right input.
This issue also appears on line 70 of the same file.
if (this.RemoveDdosCustomPolicy.IsPresent && !string.IsNullOrEmpty(this.DdosCustomPolicyId))
{
throw new ArgumentException("Specify either DdosCustomPolicyId or RemoveDdosCustomPolicy, but not both.");
}
src/Network/Network/help/Set-AzPublicIpAddress.md:151
- The cmdlet throws if both -DdosCustomPolicyId and -RemoveDdosCustomPolicy are provided, but the parameter help text doesn’t mention that they’re mutually exclusive. Adding a short note here would prevent confusion and reduce trial-and-error.
This issue also appears on line 196 of the same file.
The DDoS custom policy ID to associate with a supported public IP address.
The public IP address must have its DDoS protection mode set to `Enabled`.
The service validates whether the public IP address attachment supports a DDoS custom policy.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2d89d7-0339-446a-bb1e-c55b085eb052
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/Network/Network/PublicIpAddress/SetAzurePublicIpAddressCommand.cs:58
- The mutual-exclusion validation throws an ArgumentException that is attributed to
DdosCustomPolicyIdvia theparamNameargument, but the error is really about the invalid combination ofDdosCustomPolicyIdandRemoveDdosCustomPolicy. OmittingparamNameavoids PowerShell highlighting the wrong single parameter.
throw new ArgumentException(
"Specify either DdosCustomPolicyId or RemoveDdosCustomPolicy, but not both.",
nameof(DdosCustomPolicyId));
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c2d89d7-0339-446a-bb1e-c55b085eb052
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b56be592-c5d7-45cf-bead-f5980b7da8f3
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
-DdosCustomPolicyIdparameter toSet-AzPublicIpAddress.-RemoveDdosCustomPolicyswitch to remove an existing association.Description
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.