Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
There was a problem hiding this comment.
Pull request overview
Updates the Az.AppConfiguration module’s management-plane (RP) surface to use App Configuration API version 2024-06-01, including new replica-related cmdlets and related documentation/test assets.
Changes:
- Updated AutoRest configuration to target the
2024-06-01swagger and added/updated directives. - Added documentation/help/examples/tests for App Configuration replica cmdlets.
- Updated module manifest exports, changelog, solution metadata, and UX API version references.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/AppConfiguration/AppConfiguration/help/Update-AzAppConfigurationStore.md | Updates cmdlet help to reflect new/removed parameters. |
| src/AppConfiguration/AppConfiguration/help/Update-AzAppConfigurationReplica.md | Adds help page for replica update cmdlet. |
| src/AppConfiguration/AppConfiguration/help/Remove-AzAppConfigurationReplica.md | Adds help page for replica removal cmdlet. |
| src/AppConfiguration/AppConfiguration/help/New-AzAppConfigurationStore.md | Updates help to include new store creation parameters. |
| src/AppConfiguration/AppConfiguration/help/New-AzAppConfigurationReplica.md | Adds help page for replica creation cmdlet. |
| src/AppConfiguration/AppConfiguration/help/Get-AzAppConfigurationReplica.md | Adds help page for replica get/list cmdlet. |
| src/AppConfiguration/AppConfiguration/help/Az.AppConfiguration.md | Adds replica cmdlets to the module help index and refreshes descriptions. |
| src/AppConfiguration/AppConfiguration/ChangeLog.md | Adds upcoming release notes for API version upgrade, replicas, and parameter changes. |
| src/AppConfiguration/AppConfiguration/Az.AppConfiguration.psd1 | Updates exported functions and dependency version; includes replica cmdlets in exports. |
| src/AppConfiguration/AppConfiguration.sln | Updates solution project entries/IDs for the generated AppConfiguration project. |
| src/AppConfiguration/AppConfiguration.Autorest/test/Update-AzAppConfigurationReplica.Tests.ps1 | Adds Pester coverage for replica update scenarios. |
| src/AppConfiguration/AppConfiguration.Autorest/test/Remove-AzAppConfigurationReplica.Tests.ps1 | Adds Pester coverage for replica delete scenarios. |
| src/AppConfiguration/AppConfiguration.Autorest/test/New-AzAppConfigurationReplica.Tests.ps1 | Adds Pester coverage for replica create scenarios. |
| src/AppConfiguration/AppConfiguration.Autorest/test/Get-AzAppConfigurationReplica.Tests.ps1 | Adds Pester coverage for replica get/list scenarios. |
| src/AppConfiguration/AppConfiguration.Autorest/generate-info.json | Updates generation metadata identifier. |
| src/AppConfiguration/AppConfiguration.Autorest/examples/Update-AzAppConfigurationReplica.md | Adds example(s) for updating replicas. |
| src/AppConfiguration/AppConfiguration.Autorest/examples/Remove-AzAppConfigurationReplica.md | Adds example(s) for removing replicas. |
| src/AppConfiguration/AppConfiguration.Autorest/examples/New-AzAppConfigurationReplica.md | Adds example(s) for creating replicas. |
| src/AppConfiguration/AppConfiguration.Autorest/examples/Get-AzAppConfigurationReplica.md | Adds example(s) for getting/listing replicas. |
| src/AppConfiguration/AppConfiguration.Autorest/docs/Update-AzAppConfigurationStore.md | Updates reference docs to match updated parameter surface. |
| src/AppConfiguration/AppConfiguration.Autorest/docs/Update-AzAppConfigurationReplica.md | Adds reference doc for replica update cmdlet. |
| src/AppConfiguration/AppConfiguration.Autorest/docs/Remove-AzAppConfigurationReplica.md | Adds reference doc for replica removal cmdlet. |
| src/AppConfiguration/AppConfiguration.Autorest/docs/New-AzAppConfigurationStore.md | Updates reference docs to include new store creation parameters. |
| src/AppConfiguration/AppConfiguration.Autorest/docs/New-AzAppConfigurationReplica.md | Adds reference doc for replica creation cmdlet. |
| src/AppConfiguration/AppConfiguration.Autorest/docs/Get-AzAppConfigurationReplica.md | Adds reference doc for replica get/list cmdlet. |
| src/AppConfiguration/AppConfiguration.Autorest/docs/Az.AppConfiguration.md | Updates Autorest docs index with new replica cmdlets and refreshed descriptions. |
| src/AppConfiguration/AppConfiguration.Autorest/UX/Microsoft.AppConfiguration/locations-deletedConfigurationStores.json | Bumps UX API version to 2024-06-01. |
| src/AppConfiguration/AppConfiguration.Autorest/UX/Microsoft.AppConfiguration/configurationStores.json | Bumps UX API version to 2024-06-01. |
| src/AppConfiguration/AppConfiguration.Autorest/UX/Microsoft.AppConfiguration/configurationStores-replicas.json | Adds UX description for replica resource type using 2024-06-01. |
| src/AppConfiguration/AppConfiguration.Autorest/README.md | Updates spec commit/input-file and adds directives for new API surface. |
| src/AppConfiguration/AppConfiguration.Autorest/Properties/AssemblyInfo.cs | Updates assembly versioning to align with module versioning. |
src/AppConfiguration/AppConfiguration.Autorest/test/Get-AzAppConfigurationReplica.Tests.ps1
Outdated
Show resolved
Hide resolved
| BeforeAll { | ||
| $updateReplicaStoreName = "azupd" + (Get-Random -Maximum 99999) | ||
| New-AzAppConfigurationStore -Name $updateReplicaStoreName -ResourceGroupName $env.resourceGroup -Location $env.location -Sku Standard | ||
| $updateReplicaName = "westus2replica" | ||
| New-AzAppConfigurationReplica -ConfigStoreName $updateReplicaStoreName -ResourceGroupName $env.resourceGroup -Name $updateReplicaName -Location "westus2" | ||
| } |
There was a problem hiding this comment.
This test uses a per-run random store name (Get-Random) and references Update-AzAppConfigurationReplica.Recording.json, but that recording file is not present in the test folder. In playback mode the random name won’t match any recording, and the missing recording file will likely cause the mocking harness to fail. Please switch to deterministic names from env.json (or add new fixed entries) and commit the corresponding .Recording.json for this test.
src/AppConfiguration/AppConfiguration/help/Get-AzAppConfigurationReplica.md
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration/help/Update-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/New-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration/help/New-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration/help/Remove-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/Remove-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/Update-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/test/New-AzAppConfigurationReplica.Tests.ps1
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
src/AppConfiguration/AppConfiguration.Autorest/docs/Update-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/New-AzAppConfigurationReplica.md
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/Get-AzAppConfigurationReplica.md
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/Remove-AzAppConfigurationReplica.md
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/Update-AzAppConfigurationStore.md
Show resolved
Hide resolved
| Accept wildcard characters: False | ||
| ``` | ||
|
|
||
| ### -DataPlaneProxyAuthenticationMode |
There was a problem hiding this comment.
This PR introduces new store parameters in the public surface area, but the added tests focus on replica cmdlets. Consider adding Pester coverage that exercises these new New-AzAppConfigurationStore / Update-AzAppConfigurationStore parameters (at least acceptance + read-back validation where possible) to prevent regressions in parameter wiring/serialization.
src/AppConfiguration/AppConfiguration/help/New-AzAppConfigurationStore.md
Show resolved
Hide resolved
| Accept wildcard characters: False | ||
| ``` | ||
|
|
||
| ### -DefaultKeyValueRevisionRetentionPeriodInSecond |
There was a problem hiding this comment.
This PR introduces new store parameters in the public surface area, but the added tests focus on replica cmdlets. Consider adding Pester coverage that exercises these new New-AzAppConfigurationStore / Update-AzAppConfigurationStore parameters (at least acceptance + read-back validation where possible) to prevent regressions in parameter wiring/serialization.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
src/AppConfiguration/AppConfiguration/help/Update-AzAppConfigurationStore.md:1
- The help content shows
-EnableSystemAssignedIdentityas<Boolean>, but the generated docs inAppConfiguration.Autorest/docs/Update-AzAppConfigurationStore.mdshow it as nullable (System.Nullable[System.Boolean]). Please align the hand-shipped help undersrc/AppConfiguration/AppConfiguration/help/with the actual cmdlet parameter type (and the generated docs) so users don't get conflicting information about whether the parameter supports an unset/tri-state value.
src/AppConfiguration/AppConfiguration.Autorest/test/Update-AzAppConfigurationReplica.Tests.ps1
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/test/Remove-AzAppConfigurationReplica.Tests.ps1
Show resolved
Hide resolved
| -Location <String> -Sku <String> [-CreateMode <String>] [-DataPlaneProxyAuthenticationMode <String>] | ||
| [-DataPlaneProxyPrivateLinkDelegation <String>] [-DefaultKeyValueRevisionRetentionPeriodInSecond <Int64>] |
There was a problem hiding this comment.
New store parameters (DataPlaneProxyAuthenticationMode, DataPlaneProxyPrivateLinkDelegation, DefaultKeyValueRevisionRetentionPeriodInSecond) are introduced/advertised here, but this PR doesn't include Pester coverage that exercises these parameters (e.g., create/update with the parameters set and verify the resulting store properties). Adding tests for these new parameters would better protect against regressions in the new API version surface.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 37 out of 41 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (2)
src/AppConfiguration/AppConfiguration/help/Update-AzAppConfigurationStore.md:1
- Capitalize the proper service name for consistency and correctness (e.g., 'Azure App Configuration store'). This same phrasing appears in several updated help files in this PR, so aligning the capitalization across them would improve help quality.
src/AppConfiguration/AppConfiguration/help/Update-AzAppConfigurationStore.md:1 Update-AzAppConfigurationStoreshows-EnableSystemAssignedIdentity <Boolean>here, but the regenerated Autorest docs in this PR show a nullable boolean (<Boolean?>). Please regenerate/align the help so the parameter type is consistent across shipped help surfaces (or adjust generation so both reflect the actual cmdlet signature).
src/AppConfiguration/AppConfiguration.Autorest/docs/Remove-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/test/Remove-AzAppConfigurationReplica.Tests.ps1
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/test/Remove-AzAppConfigurationReplica.Tests.ps1
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/test/New-AzAppConfigurationReplica.Tests.ps1
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/test/Get-AzAppConfigurationReplica.Tests.ps1
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
There seems to be a few build issues:
@NoriZC any ideas? @jimmyca15 |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
src/AppConfiguration/AppConfiguration/help/Get-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration/help/New-AzAppConfigurationReplica.md
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration/help/New-AzAppConfigurationReplica.md
Outdated
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration.Autorest/docs/New-AzAppConfigurationStore.md
Show resolved
Hide resolved
src/AppConfiguration/AppConfiguration/help/Update-AzAppConfigurationStore.md
Show resolved
Hide resolved
| - Added `New-AzAppConfigurationReplica` cmdlet | ||
| - Added `Get-AzAppConfigurationReplica` cmdlet | ||
| - Added `Remove-AzAppConfigurationReplica` cmdlet | ||
| * Added `DataPlaneProxyAuthenticationMode`, `DataPlaneProxyPrivateLinkDelegation`, and `DefaultKeyValueRevisionRetentionPeriodInSecond` parameters to `New-AzAppConfigurationStore` and `Update-AzAppConfigurationStore` |
There was a problem hiding this comment.
ChangeLog entry has a parameter name typo: DefaultKeyValueRevisionRetentionPeriodInSecond should be DefaultKeyValueRevisionRetentionPeriodInSeconds (matches the cmdlet parameter spelling elsewhere in this PR).
| - where: | ||
| parameter-name: DefaultKeyValueRevisionRetentionPeriodInSecond | ||
| set: | ||
| parameter-name: DefaultKeyValueRevisionRetentionPeriodInSeconds |
There was a problem hiding this comment.
New directive renames DefaultKeyValueRevisionRetentionPeriodInSecond to ...InSeconds but doesn't document why. Per AutoRest README conventions, add a short comment explaining the intent (e.g., aligning the PowerShell parameter name with the underlying ...InSeconds unit / pluralization).
| - where: | ||
| parameter-name: SoftDeleteRetentionInDay | ||
| set: | ||
| parameter-name: SoftDeleteRetentionInDays |
There was a problem hiding this comment.
New directive renames SoftDeleteRetentionInDay to SoftDeleteRetentionInDays but lacks a comment explaining the rationale (unit/pluralization and user-facing breaking change). Please add a brief comment for maintainers.
There was a problem hiding this comment.
@jimmyca15 copilot when asked to fix the plural also changed this, but it results in a breaking change. But if we make the DefaultKeyValueRevisionRetentionPeriodInSeconds change it will be inconsistent with this.
| input-file: | ||
| - $(repo)/specification/appconfiguration/resource-manager/Microsoft.AppConfiguration/stable/2022-05-01/appconfiguration.json | ||
| - $(repo)/specification/appconfiguration/resource-manager/Microsoft.AppConfiguration/stable/2024-06-01/appconfiguration.json | ||
|
|
There was a problem hiding this comment.
The AutoRest config is updated to the 2024-06-01 spec, but the checked-in generated module outputs under generated/AppConfiguration/AppConfiguration.Autorest still appear to target the old API version (no 2024-06-01 references) and do not include the new Replica cmdlets. Please re-run generation and commit the updated generated outputs (or ensure the build regenerates them) so the actual shipped module matches the updated docs/manifest.
| # Write generated artifacts to the checked-in module location so the shipped output stays aligned | |
| # with the 2024-06-01 specification and includes newly added operations such as Replica cmdlets. | |
| output-folder: $(repo)/generated/AppConfiguration/AppConfiguration.Autorest | |
| # Clear previously generated files to prevent stale outputs from older API versions remaining in the module. | |
| clear-output-folder: true |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
Looks good other than our change to add more properties to the output of configuration store operations seems to have put us at risk of overflowing horizontal space. I'd like to revert that before we go in. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
Description
Updates the Azure App Configuration RP module to the latest released version of 2024-06-01
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.