-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[Storage] Add support for private link resource #12383
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 14 commits
95f3327
11726e2
795c4a2
1673947
6bab031
c0d2082
0f2bcdf
6ddc2e1
cbaed8a
50b30ea
a07666c
5741b61
83aef9b
5c54ca1
4ab1b1c
149f769
8dd3d5b
3e796e8
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 |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |
| validate_azcopy_remove_arguments, as_user_validator, parse_storage_account) | ||
|
|
||
|
|
||
| def load_arguments(self, _): # pylint: disable=too-many-locals, too-many-statements | ||
| def load_arguments(self, _): # pylint: disable=too-many-locals, too-many-statements, too-many-lines | ||
| from argcomplete.completers import FilesCompleter | ||
| from six import u as unicode_string | ||
|
|
||
|
|
@@ -204,6 +204,27 @@ def load_arguments(self, _): # pylint: disable=too-many-locals, too-many-statem | |
| c.argument('publish_microsoft_endpoints', publish_microsoft_endpoints_type) | ||
| c.argument('publish_internet_endpoints', publish_internet_endpoints_type) | ||
|
|
||
| with self.argument_context('storage account private-endpoint-connection', | ||
| resource_type=ResourceType.MGMT_STORAGE) as c: | ||
| c.argument('private_endpoint_connection_name', options_list=['--name', '-n'], | ||
| help='The name of the private endpoint connection associated with the Storage Account.') | ||
| for item in ['approve', 'reject', 'show', 'delete']: | ||
| with self.argument_context('storage account private-endpoint-connection {}'.format(item), | ||
| resource_type=ResourceType.MGMT_STORAGE) as c: | ||
| c.argument('private_endpoint_connection_name', options_list=['--name', '-n'], required=False, | ||
| help='The name of the private endpoint connection associated with the Storage Account.') | ||
| c.extra('connection_id', options_list=['--id'], | ||
|
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. 😄 |
||
| help='The ID of the private endpoint connection associated with the Storage Account. You can get ' | ||
| 'it using `az storage account show`.') | ||
| c.argument('account_name', help='The storage account name.', required=False) | ||
| c.argument('resource_group_name', help='The resource group name of specified storage account.', | ||
| required=False) | ||
| c.argument('description', help='Comments for {} operation.'.format(item)) | ||
|
|
||
| with self.argument_context('storage account private-link-resource', | ||
| resource_type=ResourceType.MGMT_STORAGE) as c: | ||
| c.argument('account_name', acct_name_type) | ||
|
|
||
| with self.argument_context('storage account update', resource_type=ResourceType.MGMT_STORAGE) as c: | ||
| c.register_common_storage_account_options() | ||
| c.argument('custom_domain', | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -366,6 +366,50 @@ def remove_network_rule(cmd, client, resource_group_name, account_name, ip_addre | |
| return client.update(resource_group_name, account_name, params) | ||
|
|
||
|
|
||
| def _update_private_endpoint_connection_status(cmd, client, resource_group_name, account_name, | ||
| private_endpoint_connection_name, is_approved=True, description=None): # pylint: disable=unused-argument | ||
|
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.
remove?
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. sure |
||
|
|
||
| PrivateEndpointServiceConnectionStatus, ErrorResponseException = \ | ||
| cmd.get_models('PrivateEndpointServiceConnectionStatus', 'ErrorResponseException') | ||
|
|
||
| private_endpoint_connection = client.get(resource_group_name=resource_group_name, account_name=account_name, | ||
| private_endpoint_connection_name=private_endpoint_connection_name) | ||
|
|
||
| old_status = private_endpoint_connection.private_link_service_connection_state.status | ||
| new_status = PrivateEndpointServiceConnectionStatus.approved \ | ||
| if is_approved else PrivateEndpointServiceConnectionStatus.rejected | ||
| private_endpoint_connection.private_link_service_connection_state.status = new_status | ||
| private_endpoint_connection.private_link_service_connection_state.description = description | ||
| try: | ||
| return client.put(resource_group_name=resource_group_name, | ||
| account_name=account_name, | ||
| private_endpoint_connection_name=private_endpoint_connection_name, | ||
| properties=private_endpoint_connection) | ||
| except ErrorResponseException as ex: | ||
| if ex.response.status_code == 400: | ||
|
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. do you need raise the exception again after all?
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. good catch! Thanks! |
||
| from msrestazure.azure_exceptions import CloudError | ||
| if new_status == "Approved" and old_status == "Rejected": | ||
| raise CloudError(ex.response, "You cannot approve the connection request after rejection. " | ||
| "Please create a new connection for approval.") | ||
|
|
||
|
|
||
| def approve_private_endpoint_connection(cmd, client, resource_group_name, account_name, | ||
| private_endpoint_connection_name, description=None): | ||
|
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. @bim-msft I think it make senses to use
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.
Agree with you, I will modify this as well. |
||
|
|
||
| return _update_private_endpoint_connection_status( | ||
| cmd, client, resource_group_name=resource_group_name, account_name=account_name, is_approved=True, | ||
| private_endpoint_connection_name=private_endpoint_connection_name, description=description | ||
| ) | ||
|
|
||
|
|
||
| def reject_private_endpoint_connection(cmd, client, resource_group_name, account_name, private_endpoint_connection_name, | ||
| description=None): | ||
| return _update_private_endpoint_connection_status( | ||
| cmd, client, resource_group_name=resource_group_name, account_name=account_name, is_approved=False, | ||
| private_endpoint_connection_name=private_endpoint_connection_name, description=description | ||
| ) | ||
|
|
||
|
|
||
| def create_management_policies(client, resource_group_name, account_name, policy=None): | ||
| if policy: | ||
| if os.path.exists(policy): | ||
|
|
||
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.
this is a question, what is the default value for required? from your sample, it seems we can use connection_id or connection_name + account_name +rpName, why are the later parameters marked as required=false
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.
requiredmeans it has to be provided when using the command.As you know, we have two options to manage the connection. With the two options, connection_id can be none if you use option2, and connection_name+account_name+rgName also can be none if you use option1.
In this way, no one is always required for the command.
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.
why connection_id does not have required=false marked? I suppose required is false by default?