Skip to content
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

[Key Vault] Add Secret URI Parameter to Key Vault Secret Cmdlets #26222

Merged
merged 13 commits into from
Nov 11, 2024

Conversation

notyashhh
Copy link
Member

@notyashhh notyashhh commented Oct 4, 2024

Description

This PR implements a user suggested feature #23053

This PR introduces a new -Id parameter to Key Vault secret-related cmdlets, such as Get-AzKeyVaultSecret, Set-AzKeyVaultSecret, and others, allowing users to specify secrets directly by their URI. This addition improves usability by enabling direct interaction with Key Vault secrets through their unique URIs.

The changes include:
• Renaming the existing Id parameter (Parent Resource ID) to ParentResourceId with an alias for backward compatibility.
• Adding a new Id parameter (Secret URI) with an alias SecretId for specifying secrets by URI.
• Modifications in KeyVaultCmdletBase to centralize URI parsing logic for streamlined processing across all cmdlets.

Mandatory Checklist

  • SHOULD update ChangeLog.md file(s) appropriately
    • For SDK-based development mode, update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
      • A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
    • For autorest-based development mode, include the changelog in the PR description.
    • Should not change ChangeLog.md if no new release is required, such as fixing test case only.
  • SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
  • SHOULD have proper test coverage for changes in pull request.
  • SHOULD NOT adjust version of module manually in pull request

Copy link

azure-client-tools-bot-prd bot commented Oct 4, 2024

️✔️Az.Accounts
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Compute
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️ - Linux
️✔️ - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.CosmosDB
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️ - Linux
️✔️ - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.EventHub
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️ - Linux
️✔️ - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
⚠️Az.KeyVault
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Breaking Change Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
⚠️Signature Check
⚠️PowerShell Core - Windows
Type Cmdlet Description Remediation
⚠️ Get-AzKeyVaultManagedHsmRegion Get-AzKeyVaultManagedHsmRegion Changes the ConfirmImpact but does not set the SupportsShouldProcess property to true in the cmdlet attribute. Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue
⚠️ Get-AzKeyVaultManagedHsmRegion Get-AzKeyVaultManagedHsmRegion changes the confirm impact. Please ensure that the change in ConfirmImpact is justified Verify that ConfirmImpact is changed appropriately by the cmdlet. It is very rare for a cmdlet to change the ConfirmImpact.
⚠️Windows PowerShell - Windows
Type Cmdlet Description Remediation
⚠️ Get-AzKeyVaultManagedHsmRegion Get-AzKeyVaultManagedHsmRegion Changes the ConfirmImpact but does not set the SupportsShouldProcess property to true in the cmdlet attribute. Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue
⚠️ Get-AzKeyVaultManagedHsmRegion Get-AzKeyVaultManagedHsmRegion changes the confirm impact. Please ensure that the change in ConfirmImpact is justified Verify that ConfirmImpact is changed appropriately by the cmdlet. It is very rare for a cmdlet to change the ConfirmImpact.
️✔️Help Example Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Help File Existence Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️File Change Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️UX Metadata Check
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
⚠️Test
⚠️ - Linux
Type Title Current Coverage Description
⚠️ Test Coverage Less Than 50% 22.09 % Test coverage for the module cannot be lower than 50%.
⚠️ - MacOS
Type Title Current Coverage Description
⚠️ Test Coverage Less Than 50% 22.09% Test coverage for the module cannot be lower than 50%.
⚠️PowerShell Core - Windows
Type Title Current Coverage Description
⚠️ Test Coverage Less Than 50% 22.09% Test coverage for the module cannot be lower than 50%.
⚠️Windows PowerShell - Windows
Type Title Current Coverage Description
⚠️ Test Coverage Less Than 50% 22.09% Test coverage for the module cannot be lower than 50%.
️✔️Az.ManagedServiceIdentity
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Monitor
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Network
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️ - Linux
️✔️ - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.OperationalInsights
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.PrivateDns
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.Resources
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Az.ServiceBus
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
️✔️Test
️✔️ - Linux
️✔️ - MacOS
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
⚠️Az.Sql
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows
⚠️Test
⚠️ - Linux
Type Title Current Coverage Last Coverage Description
⚠️ Test Coverage Less Than 80% 62.15 % 62.22% Test coverage cannot be lower than the number of the last release.
⚠️ - MacOS
Type Title Current Coverage Last Coverage Description
⚠️ Test Coverage Less Than 80% 62.15% 62.22% Test coverage cannot be lower than the number of the last release.
⚠️PowerShell Core - Windows
Type Title Current Coverage Last Coverage Description
⚠️ Test Coverage Less Than 80% 62.15% 62.22% Test coverage cannot be lower than the number of the last release.
⚠️Windows PowerShell - Windows
Type Title Current Coverage Last Coverage Description
⚠️ Test Coverage Less Than 80% 62.15% 62.22% Test coverage cannot be lower than the number of the last release.
️✔️Az.Storage
️✔️Build
️✔️PowerShell Core - Windows
️✔️Windows PowerShell - Windows

@notyashhh notyashhh linked an issue Oct 4, 2024 that may be closed by this pull request
Copy link
Contributor

@BethanyZhou BethanyZhou left a comment

Choose a reason for hiding this comment

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

Generally looks good with minor modifications required

  1. Please refresh help docs when command's syntax change
  2. Add test cases if applicable
  3. Resolve suggestions if applicable

Copy link

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

@notyashhh notyashhh requested a review from BethanyZhou October 23, 2024 06:01
Copy link
Contributor

@BethanyZhou BethanyZhou left a comment

Choose a reason for hiding this comment

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

Please add an example in each cmdlet's help doc if a new parameter set is introduced

src/KeyVault/KeyVault/Models/KeyVaultCmdletBase.cs Outdated Show resolved Hide resolved
src/KeyVault/KeyVault/Models/Secret/KeyVaultSecretUri.cs Outdated Show resolved Hide resolved
Copy link

This PR was labeled "needs-revision" because it has unresolved review comments or CI failures.
Please resolve all open review comments and make sure all CI checks are green. Refer to our guide to troubleshoot common CI failures.

@Nickcandy Nickcandy changed the base branch from main to release-2024-11-11 October 29, 2024 07:54
@notyashhh notyashhh requested a review from BethanyZhou October 29, 2024 08:00
@isra-fel
Copy link
Member

isra-fel commented Nov 8, 2024

release-2024-11-11 is for Az 12.5. This PR should target 13.0. I'm updating the base branch to main.

@isra-fel isra-fel changed the base branch from release-2024-11-11 to main November 8, 2024 07:18
@BethanyZhou
Copy link
Contributor

/azp run license/cla

Copy link
Contributor

No pipelines are associated with this pull request.

@BethanyZhou BethanyZhou reopened this Nov 11, 2024
@BethanyZhou BethanyZhou merged commit 04ad4f4 into Azure:main Nov 11, 2024
12 checks passed
@ishepherd
Copy link

Thanks @BethanyZhou !

Nickcandy added a commit that referenced this pull request Nov 21, 2024
* update readme and generate sdk

* modifications to match generated code

* update polling method

* testbasic cluster sfmc

* more test recordings

* apptype test recordings

* add managed service test

* adding managedapptypeversion recording

* add manaegdApp recording

* add managedAppType recording

* update readme

* undo breaking changes

* undo breaking change

* Fix Readme.md

* generate code

* update commands

* undo generated code changes

* remove unnecessary cast

* remove unnecessary casting

* update iptag

* revert iptag change

* unchanged sdk

* updates

* update sdk

* generate sfrp sdk

* update test recordings

* update resource values

* update all tests

* updated sdk

* update sfmc sdk

* Update README.md

* Update README.md

* add breaking.csv file

* update breakinchanges.csv

* more updates

* target Pr

* Update ChangeLog.md

* Az.service fabric preview (#25124)

* update readme and generate sdk

* modifications to match generated code

* update polling method

* testbasic cluster sfmc

* more test recordings

* apptype test recordings

* add managed service test

* adding managedapptypeversion recording

* add manaegdApp recording

* add managedAppType recording

* update readme

* undo breaking changes

* undo breaking change

* Fix Readme.md

* generate code

* update commands

* undo generated code changes

* remove unnecessary cast

* remove unnecessary casting

* update iptag

* revert iptag change

* unchanged sdk

* updates

* update sdk

* generate sfrp sdk

* update test recordings

* update resource values

* update all tests

* updated sdk

* update sfmc sdk

* Update README.md

* Update README.md

* add breaking.csv file

* update breakinchanges.csv

* more updates

* target Pr

* Update ChangeLog.md

---------

Co-authored-by: Lei Jin <leijin@microsoft.com>
Co-authored-by: NoriZC <110961157+NoriZC@users.noreply.github.com>

* upgrade servicefabric to 4.0.0-preview

* sanitize password in example

* resolve bad merge

* Adding examples for constrained role delegation

Adding examples to showcase how constrained role delegation can be done with PowerShell.

* Remove GitHub Token From CI Pipeline (#26602)

Remove GitHub Token From CI Pipeline

* Migrate ConnectedKubernetes from generation to main (#26630)

* Move ConnectedKubernetes to main

* Update ChangeLog.md

* Update ChangeLog.md

* Update ChangeLog.md

---------

Co-authored-by: azure-powershell-bot <65331932+azure-powershell-bot@users.noreply.github.com>
Co-authored-by: NoriZC <110961157+NoriZC@users.noreply.github.com>

* add new sdk and test recordings

* Migrate DevCenter from generation to main (#26626)

* Move DevCenter to main

* Add change log and suppress breaking change

---------

Co-authored-by: azure-powershell-bot <65331932+azure-powershell-bot@users.noreply.github.com>
Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* Add support for creating cross-subscription Geo-replicated databases (#26598)

* add cross subscription support for NewAzSqlDatabaseSecondary cmdlet

* run replication tests to validate cross-sub change

* update changelog

* fix typo on resource group assert

* update help document with new param

* Sanitize data

---------

Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* Update a string value in Get PR Changed Files Script (#26637)

* Update get-pr-changed-files.yml

* Update get-pr-changed-files.yml

* [Storage] Mirgate Storage file cmdlets to Track2 SDK  (#26575) (#26594)

* [Storage] Mirgate Storage file cmdlets to Track2 SDK  (#26575)

* REsolve merge conflicts

* Resolve merge conflicts

* remove breaking change warnings

* Update changelog

* Add breaking change exception

* Remove unused file InvalidCLoudFileShare.cs

* Update RemoveFilePathTrailingDot helper function per comments

---------

Co-authored-by: yifanz7 <131133995+yifanz7@users.noreply.github.com>
Co-authored-by: Yifan Zhang <yifanzhang@microsoft.com>

* Downgrade the servity of didn't find defined parameter in path to 2 (#26639)

* Migrate Astro from generation to main (#26643)

* Move Astro to main

* Update ChangeLog.md

---------

Co-authored-by: azure-powershell-bot <65331932+azure-powershell-bot@users.noreply.github.com>
Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* update chagelog

* Fix JSON format issue (#26646)

* Migrate App from generation to main (#26644)

* Move App to main

* update changelog and BreakingChangeIssues.csv

---------

Co-authored-by: azure-powershell-bot <65331932+azure-powershell-bot@users.noreply.github.com>
Co-authored-by: Jinpei Li <v-jinpel@microsoft.com>

* Migrate NeonPostgres from generation to main (#26647)

* Move NeonPostgres to main

* Update Az.NeonPostgres.psd1

---------

Co-authored-by: azure-powershell-bot <65331932+azure-powershell-bot@users.noreply.github.com>
Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* [Key Vault] Add Secret URI Parameter to Key Vault Secret Cmdlets (#26222)

* Added secretUri support for all the 'secret' cmdlets

* Updated Changelog

* Added ResourceId aliases for backwards compatibility

* Added Tests, Secret Data Class

* Updated Help Docs

* Use Typed varaibles

* Add example usages in help docs

* Error Suppression

* Change Data Class Accessibility

* Move Split Logic to Constructor

* Added uri format to help docs

* [Accounts] Add Long-Running Operation (LRO) Support to Invoke-AzRestMethod (#26225)

* Refactored existing code

* Added general LRO support

* Removed excess log statements

* Updated Changelog

* Added comments

* Removed log statements

* Regression Compatiblility

* Removed debug logs

* Updated the help docs

* Update param name, Add use case example

* Updated help docs

* Updated description

* Update to add ArgumentCompleter

* Delay the Get-AzAccessToken Breaking Change (#26625)

* Delay the Get-AzAccessToken breaking change

* Update the doc of Get-AzAccessToken

* Address review comments

* Address review comments

---------

Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* Update azure-powershell-modules.md (#26650)

* Update azure-powershell-modules.md

* Update azure-powershell-modules.md

* Update azure-powershell-modules.md

* Bump up version for ConnectedKubernetes (#26635)

* remove secrets

* Migrate ConnectedMachine from generation to main (#26657)

* Move ConnectedMachine to main

* Update ChangeLog.md

* Fix signature issues.

---------

Co-authored-by: azure-powershell-bot <65331932+azure-powershell-bot@users.noreply.github.com>
Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* Remove ProgressAction and tweak the change log (#26661)

* [Devtestlabs] Migrated DevTestLabs SDK to generated SDK (#26648)

* generate DevTestLabs by autorest v2

* add DevTestLabs.Management.Sdk

* use 2016-05-15 api vesion

* migrate DevTestLabs to autorest.powershell

* update and record the test

* Update ChangeLog.md

* Revert "Update ChangeLog.md"

This reverts commit 4788e93.

* Update ChangeLog.md

---------

Co-authored-by: Yabo Hu <yabhu@microsoft.com>

* Changing to two simple roles for examples

The built-in roles have the same GUID in every tenant.

* Remove get-github-token-steps.yml (#26660)

* Update code-gen.yml

* Delete .azure-pipelines/util/get-github-token-steps.yml

* Add engineer docs for example analyzer (#26663)

* Create example-analyzer.md

* refine docs

* debug way

* Migrate DesktopVirtualization from generation to release-2024-11-19 (#26668)

* Move DesktopVirtualization to release-2024-11-19

* Remove ProgressAction

---------

Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* Remove alias for cmdlet Resolve-AzError (#26666)

* Remove alias for cmdlet Resolve-AzError

* Suppress breaking change

* Update ChangeLog.md

---------

Co-authored-by: Yabo Hu <yabhu@microsoft.com>

* Fixed overriding of Bicep parameters in Deployment Stack cmdlets to support SecureString parameters. (#26670)

* Fixed overriding of Bicep parameters in Deployment Stack cmdlets to support SecureString parameters.

* Added session record.

---------

Co-authored-by: Dante DG <test>

* Migrate ConnectedKubernetes from generation to main (#26675)

* Move ConnectedKubernetes to main

* Update ChangeLog.md

---------

Co-authored-by: Yabo Hu <yabhu@microsoft.com>

* Migrate DnsResolver from generation to release-2024-11-19 (#26676)

* Move DnsResolver to release-2024-11-19

* Update change log and suppress breaking changes

* Updated change log

---------

Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* Updated verbose status log in deployment stacks deployment. (#26681)

* Updated verbose status log in deployment stacks deployment.

* Update ChangeLog.md

---------

Co-authored-by: Dante DG <test>
Co-authored-by: Yabo Hu <yabhu@microsoft.com>

* Migrate ContainerInstance from generation to release-2024-11-19 (#26690)

* Move ContainerInstance to release-2024-11-19

* Remove ProgressAction and update change log

* Reset recording

---------

Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>

* Bump up version for ConnectedKubernetes (#26686)

* [DataFactory]Powershell Release (#26702)

* release-2024-11-19 (#26706)

* bump minor versions at least to distinguish with LTS

* Bump a major version for Az.Compute

* Remove BreakingChangeIssues.csv for bumping major version

* Bump Version for Az 13.0.0

* add community contributors

* SyntaxChangeLog-Az13.md

* add community contributors in changelog.md

* Update Start-AzNetAppFilesPeerExternalCluster.md (#26684)

* Resolve the solutions with dependencies (#26710)

* Update model creation parameter location or order

* Add migration guide

* Update documentation/migration-guides/Az.13.0.0-migration-guide.md

Co-authored-by: yifanz7 <131133995+yifanz7@users.noreply.github.com>

* Update documentation/migration-guides/Az.13.0.0-migration-guide.md

Co-authored-by: yifanz7 <131133995+yifanz7@users.noreply.github.com>

* Update documentation/migration-guides/Az.13.0.0-migration-guide.md

Co-authored-by: yifanz7 <131133995+yifanz7@users.noreply.github.com>

* Update ChangeLog.md

* Update documentation/migration-guides/Az.13.0.0-migration-guide.md

Co-authored-by: Wei Wei <weiwei@microsoft.com>

* Update ExampleIssues.csv

* remove secrets

* Updating migration guide typos for Az 13.0.0 (#26722)

* Fixing migration guide for SQL DAG.

* Chaning string to lowercase.

---------

Co-authored-by: jovancevic123 <jojovancevic@microsoft.com>

* Update migration guide. (#26723)

* update file

* update

* update

* remove secrets

* remove more secrets

* remove more secrests

* remove passwords

* Update CredScanSuppressions.json

* Update CredScanSuppressions.json

---------

Co-authored-by: Lei Jin <leijin@microsoft.com>
Co-authored-by: NoriZC <110961157+NoriZC@users.noreply.github.com>
Co-authored-by: Yabo Hu <yabhu@microsoft.com>
Co-authored-by: Sebastian Claesson <34689432+SebastianClaesson@users.noreply.github.com>
Co-authored-by: Yan Xu <yanxu1@microsoft.com>
Co-authored-by: azure-pipelines[bot] <36771401+azure-pipelines[bot]@users.noreply.github.com>
Co-authored-by: azure-powershell-bot <65331932+azure-powershell-bot@users.noreply.github.com>
Co-authored-by: Vincent Dai <23257217+vidai-msft@users.noreply.github.com>
Co-authored-by: alecbain <70780720+alecbain@users.noreply.github.com>
Co-authored-by: yifanz7 <131133995+yifanz7@users.noreply.github.com>
Co-authored-by: Yifan Zhang <yifanzhang@microsoft.com>
Co-authored-by: Yunchi Wang <54880216+wyunchi-ms@users.noreply.github.com>
Co-authored-by: Jinpei Li <v-jinpel@microsoft.com>
Co-authored-by: Yash <55773468+notyashhh@users.noreply.github.com>
Co-authored-by: Jin Lei <54836179+msJinLei@users.noreply.github.com>
Co-authored-by: NanxiangLiu <33285578+Nickcandy@users.noreply.github.com>
Co-authored-by: JoyerJin <116236375+JoyerJin@users.noreply.github.com>
Co-authored-by: Beisi Zhou <zhoubeisi@gmail.com>
Co-authored-by: Dante <danted@microsoft.com>
Co-authored-by: Jingshu918 <138486531+Jingshu918@users.noreply.github.com>
Co-authored-by: Wei Wei <weiwei@microsoft.com>
Co-authored-by: jovancevic123 <62032416+jovancevic123@users.noreply.github.com>
Co-authored-by: jovancevic123 <jojovancevic@microsoft.com>
Co-authored-by: lijinpei2008 <31384087+lijinpei2008@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Get-AzKeyVaultSecret should allow to fetch by ID
4 participants