-
Notifications
You must be signed in to change notification settings - Fork 4.1k
Fixed default parameter set for migration #29049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Ptnan7
wants to merge
3
commits into
Azure:main
Choose a base branch
from
Ptnan7:fix-migration
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -75,8 +75,7 @@ function Start-AzFrontDoorCdnProfilePrepareMigration { | |||||
| # Name of the pricing tier. | ||||||
| ${SkuName}, | ||||||
|
|
||||||
| [Parameter(ParameterSetName='MigrateExpanded')] | ||||||
| [AllowEmptyCollection()] | ||||||
| [Parameter(ParameterSetName='MigrateExpanded', Mandatory)] | ||||||
| [Microsoft.Azure.PowerShell.Cmdlets.Cdn.Category('Body')] | ||||||
| [Microsoft.Azure.PowerShell.Cmdlets.Cdn.Models.IMigrationWebApplicationFirewallMapping[]] | ||||||
| # Waf mapping for the migrated profile | ||||||
|
|
@@ -235,8 +234,18 @@ function Start-AzFrontDoorCdnProfilePrepareMigration { | |||||
| Write-Debug("WAF linked to the frontdoor: $allPoliciesWithWAF") | ||||||
| Write-Debug("Key vault name used for the frontdoor: $allPoliciesWithVault") | ||||||
|
|
||||||
| if (${MigrationWebApplicationFirewallMapping}.count -ne $allPoliciesWithWAF.count) { | ||||||
| throw "MigrationWebApplicationFirewallMapping parameter instance should be equal to the number of WAF policy instance in the profile." | ||||||
| # Validate WAF mapping based on parameter set | ||||||
| if ($PSCmdlet.ParameterSetName -eq 'MigrateExpanded') { | ||||||
| # In MigrateExpanded, MigrationWebApplicationFirewallMapping is mandatory | ||||||
| # Validate the count matches | ||||||
| if (${MigrationWebApplicationFirewallMapping}.count -ne $allPoliciesWithWAF.count) { | ||||||
| throw "MigrationWebApplicationFirewallMapping parameter instance should be equal to the number of WAF policy instance in the profile. Expected: $($allPoliciesWithWAF.count), Provided: $($MigrationWebApplicationFirewallMapping.count)" | ||||||
| } | ||||||
| } else { | ||||||
| # In CreateExpanded, if Front Door has WAF policies, user must use MigrateExpanded parameter set | ||||||
| if ($allPoliciesWithWAF.count -gt 0) { | ||||||
| throw "The Front Door has $($allPoliciesWithWAF.count) WAF policy/policies associated. Please provide the -MigrationWebApplicationFirewallMapping parameter with WAF policy mappings." | ||||||
|
||||||
| throw "The Front Door has $($allPoliciesWithWAF.count) WAF policy/policies associated. Please provide the -MigrationWebApplicationFirewallMapping parameter with WAF policy mappings." | |
| throw "The Front Door has $($allPoliciesWithWAF.count) associated WAF $(if ($allPoliciesWithWAF.count -eq 1) { 'policy' } else { 'policies' }). Please provide the -MigrationWebApplicationFirewallMapping parameter with WAF policy mappings." |
Comment on lines
+443
to
448
Copilot
AI
Jan 11, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ValidateWafPolicies function has redundant logic. Lines 436-439 check if MigrationWebApplicationFirewallMapping is null or empty and return early, but then line 441 checks the same condition again. The inner if statement on line 441 will always be true when reached since we've already returned early if count is 0. Consider removing the redundant check on line 441.
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| { | ||
| "generate_Id": "b87540a6-bd8f-4e14-bb97-83bb4cf9d31e" | ||
| "generate_Id": "81ba469a-66c9-4ffb-8535-6c7ada28141a" | ||
| } |
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |
| --> | ||
|
|
||
| ## Upcoming Release | ||
| * Fixed default parameter set issue. | ||
|
||
|
|
||
| ## Version 6.0.0 | ||
| * Added support for edge action | ||
|
|
||
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message enhancement provides better clarity by showing both expected and provided counts. However, consider using consistent terminology. The message says "parameter instance" which might be unclear to users. Consider rewording to: "The number of WAF mapping entries must match the number of WAF policies in the profile. Expected: $($allPoliciesWithWAF.count), Provided: $ ($MigrationWebApplicationFirewallMapping.count)"