-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[KeyVault] Support private endpoint connections #12073
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
Changes from 20 commits
320ad5a
b2ee5aa
3f03c0e
037d321
dd5b3a4
f53c836
0c4908a
1536780
e16e266
cc53d1f
28a3474
58bcd54
0ba10ec
abfa412
0c35744
e816aa9
eb910a9
b2d332d
06412f6
2a8f43c
d56da79
8ac1614
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -144,6 +144,31 @@ | |||
| short-summary: Manage vault network ACLs. | ||||
| """ | ||||
|
|
||||
| helps['keyvault private-endpoint'] = """ | ||||
| type: group | ||||
| short-summary: Manage vault private endpoint connections. | ||||
| """ | ||||
|
|
||||
| helps['keyvault private-endpoint delete'] = """ | ||||
| type: command | ||||
| short-summary: Delete the specified private endpoint connection associated with a Key Vault. | ||||
| """ | ||||
|
|
||||
| helps['keyvault private-endpoint show'] = """ | ||||
| type: command | ||||
| short-summary: Show details of a private endpoint connection associated with a Key Vault. | ||||
| """ | ||||
|
|
||||
| helps['keyvault private-link-resource'] = """ | ||||
| type: group | ||||
| short-summary: Manage vault private link resources. | ||||
| """ | ||||
|
|
||||
| helps['keyvault private-link-resource show'] = """ | ||||
| type: command | ||||
| short-summary: Show the private link resources supported for a Key Vault. | ||||
|
Comment on lines
+167
to
+169
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The |
||||
| """ | ||||
|
|
||||
| helps['keyvault recover'] = """ | ||||
| type: command | ||||
| short-summary: Recover a key vault. | ||||
|
|
||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -166,6 +166,27 @@ def validate_policy_permissions(ns): | |
| '--certificate-permissions --storage-permissions') | ||
|
|
||
|
|
||
| def validate_private_endpoint_connection_id(cmd, ns): | ||
| connection_id = ns.connection_id | ||
| connection_name = ns.private_endpoint_connection_name | ||
| vault_name = ns.vault_name | ||
|
|
||
| if not connection_id: | ||
| if not all([connection_name, vault_name]): | ||
| raise argparse.ArgumentError( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on my knowledge, usually we would |
||
| None, 'specify both: --connection-name/-n and --vault-name') | ||
| ns.resource_group_name = _get_resource_group_from_vault_name(cmd.cli_ctx, vault_name) | ||
| else: | ||
| if any([connection_name, vault_name]): | ||
| raise argparse.ArgumentError( | ||
| None, 'you don\'t need to specify --connection-name/-n or --vault-name if --connection-id is specified') | ||
|
|
||
| id_parts = connection_id.split('/') | ||
| ns.private_endpoint_connection_name = id_parts[-1] | ||
| ns.vault_name = id_parts[-3] | ||
| ns.resource_group_name = id_parts[-7] | ||
|
|
||
|
|
||
| def validate_principal(ns): | ||
| num_set = sum(1 for p in [ns.object_id, ns.spn, ns.upn] if p) | ||
| if num_set != 1: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,10 +8,11 @@ | |
|
|
||
|
|
||
| from ._client_factory import ( | ||
| keyvault_client_vaults_factory, keyvault_data_plane_factory) | ||
| keyvault_client_vaults_factory, keyvault_client_private_endpoint_connections_factory, | ||
| keyvault_client_private_link_resources_factory, keyvault_data_plane_factory) | ||
|
|
||
| from ._validators import ( | ||
| process_secret_set_namespace, process_certificate_cancel_namespace) | ||
| process_secret_set_namespace, process_certificate_cancel_namespace, validate_private_endpoint_connection_id) | ||
|
|
||
|
|
||
| # pylint: disable=too-many-locals, too-many-statements | ||
|
|
@@ -34,6 +35,18 @@ def load_command_table(self, _): | |
| resource_type=ResourceType.MGMT_KEYVAULT | ||
| ) | ||
|
|
||
| kv_private_endpoint_connections_sdk = CliCommandType( | ||
| operations_tmpl='azure.mgmt.keyvault.operations#PrivateEndpointConnectionsOperations.{}', | ||
| client_factory=keyvault_client_private_endpoint_connections_factory, | ||
| resource_type=ResourceType.MGMT_KEYVAULT | ||
| ) | ||
|
|
||
| kv_private_link_resources_sdk = CliCommandType( | ||
| operations_tmpl='azure.mgmt.keyvault.operations#PrivateLinkResourcesOperations.{}', | ||
| client_factory=keyvault_client_private_link_resources_factory, | ||
| resource_type=ResourceType.MGMT_KEYVAULT | ||
| ) | ||
|
|
||
| kv_data_sdk = CliCommandType( | ||
| operations_tmpl='azure.keyvault.key_vault_client#KeyVaultClient.{}', | ||
| client_factory=keyvault_data_plane_factory, | ||
|
|
@@ -64,6 +77,25 @@ def load_command_table(self, _): | |
| g.custom_command('remove', 'remove_network_rule') | ||
| g.custom_command('list', 'list_network_rules') | ||
|
|
||
| with self.command_group('keyvault private-endpoint', | ||
| kv_private_endpoint_connections_sdk, | ||
| min_api='2018-02-14', | ||
| client_factory=keyvault_client_private_endpoint_connections_factory, | ||
| is_preview=True) as g: | ||
| g.custom_command('approve', 'approve_private_endpoint_connection', | ||
| validator=validate_private_endpoint_connection_id) | ||
| g.custom_command('reject', 'reject_private_endpoint_connection', | ||
| validator=validate_private_endpoint_connection_id) | ||
|
Comment on lines
+85
to
+88
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is not very conventional for PowerShell to design corresponding commands as
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jiasli Yes, this was confirmed with Key Vault team, will forward you the email later.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here. I think this design is good. |
||
| g.command('delete', 'delete', validator=validate_private_endpoint_connection_id) | ||
| g.show_command('show', 'get', validator=validate_private_endpoint_connection_id) | ||
|
|
||
| with self.command_group('keyvault private-link-resource', | ||
| kv_private_link_resources_sdk, | ||
| min_api='2018-02-14', | ||
| client_factory=keyvault_client_private_link_resources_factory, | ||
| is_preview=True) as g: | ||
| g.show_command('show', 'list_by_vault') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should it support id? |
||
|
|
||
| # Data Plane Commands | ||
| with self.command_group('keyvault key', kv_data_sdk) as g: | ||
| g.keyvault_command('list', 'get_keys') | ||
|
|
||
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.
On second thought, we'd better name it as
private-endpoint-connectionas it corresponds toprivateEndpointConnectionsfromaz keyvault show.privateEndpointstands for another resource typeMicrosoft.Network/privateEndpoints. I think we shouldn't mix these two concepts.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.
@jiasli Thanks for this, for the convenience, I shortened the group name as
private-endpoint(in fact, I copied the group name fromnetwork private-endpoint), but as you mentioned, this name stands for another resource type and may cause confusion, I will modify this later.