Disallow renaming the default tenant. #1099
Conversation
|
Fixes #1092 |
esx_service/utils/auth_api.py
Outdated
| @@ -347,6 +348,10 @@ def _tenant_create(name, description="", vm_list=None, privileges=None): | |||
|
|
|||
| def _tenant_update(name, new_name=None, description=None, default_datastore=None): | |||
| """ API to update a tenant """ | |||
| if name == auth_data_const.DEFAULT_TENANT: | |||
| error_info = error_code.generate_error_info(ErrorCode.TENANT_NAME_INVALID, name, VALID_TENANT_NAMES) | |||
There was a problem hiding this comment.
If I understand this code path correctly, this change stops updating description, default datastore etc for the _DEFAULT tenant where as our intention is only to stop renaming _DEFAULT tenant, please correct me otherwise.
There was a problem hiding this comment.
Correct. I think this check should be moved after the if check on line 368
| @@ -28,6 +28,7 @@ | |||
|
|
|||
| # regex for valid tenant name | |||
| VALID_TENANT_NAME_REGEXP = "[a-zA-Z0-9_][a-zA-Z0-9_.-]*" | |||
| VALID_TENANT_NAMES = 'rename of vm-groups other than _DEFAULT' | |||
There was a problem hiding this comment.
nit: can you please improve the error message?
There was a problem hiding this comment.
The error msg comes out like this,
Vm-group name _DEFAULT is invalid, only _rename of vm-groups other than DEFAULT is allowed
There was a problem hiding this comment.
I would like to suggest something like renaming of _DEFAULT vm-group is not allowed
There was a problem hiding this comment.
In the message the highlited parts are fixed for the error string define for TENANT_NAME_INVALID
Vm-group name _DEFAULT is invalid, _only rename of vm-groups other than DEFAULT is allowed
There was a problem hiding this comment.
Thanks for the clarification.
|
Will move the check. Can you say what change needed to the error message.
It is part of another error message defined for invalid group name
…On Mar 29, 2017 7:46 AM, "Shahzeb Patel" ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In esx_service/utils/auth_api.py
<#1099 (comment)>
:
> @@ -347,6 +348,10 @@ def _tenant_create(name, description="", vm_list=None, privileges=None):
def _tenant_update(name, new_name=None, description=None, default_datastore=None):
""" API to update a tenant """
+ if name == auth_data_const.DEFAULT_TENANT:
+ error_info = error_code.generate_error_info(ErrorCode.TENANT_NAME_INVALID, name, VALID_TENANT_NAMES)
Correct. I think this check should be moved after the if check on line 368
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1099 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/APHseKEZaQ9_vdyTC5q29cs3iNweq88Dks5rqb77gaJpZM4Mrcpx>
.
|
|
@govint Proposed changes looks OK to me. It would be great if you can add unit test to cover proposed change. |
| @@ -28,6 +28,7 @@ | |||
|
|
|||
| # regex for valid tenant name | |||
| VALID_TENANT_NAME_REGEXP = "[a-zA-Z0-9_][a-zA-Z0-9_.-]*" | |||
| VALID_TENANT_NAMES = 'rename of vm-groups other than _DEFAULT' | |||
There was a problem hiding this comment.
Thanks for the clarification.
Verified as below,
/usr/lib/vmware/vmdkops/bin/vmdkops_admin.py vm-group update --name=_DEFAULT --descr
iption="Renaming default tenant to vm-group1" --new-name=new-vm-group1
Vm-group name _DEFAULT is invalid, only rename of vm-groups other than _DEFAULT is allowed