Handling case insensitivity for tenant access allow-create option (#968)

* Handling case insensitivity for tenant access allow-create option

1. Accepting case insensitive true/false values for allow-create option to
   set access for tenants. But throwing error for invalid values for allow-create option

Testing:
Added test cases for case insensitive true/false values as well as invalid value
Manual testing done

Resolves: #918

* Validating the tenant access set allow-create in auth_api

1. Moved the validatin logic to auth_api to be applied when invoked from
both Admin CLI and VMODL

2. Modifying test cases to be hit by vmdkops_admin tenant_access_set api

* 1. Adding allow_create validation to tenant_access_add

2. Correcting vmodl tests to allow optional allow_create parameter

3. Code cleaning

* Code cleaning and reverting vmdk_ops_test

* 1. Removing redundant test code and modifying comments
2. Updating help for admin tenant access set --allow-create option

* Proper message check for invalid input to tenant access set allow_create option

esx_service/cli/vmdkops_admin.py

@@ -362,6 +362,7 @@ def commands():
                                 '--allow-create': {
                                     'help': 
                                     'Allow create and delete on datastore if set to True; disallow create and delete on datastore if set to False',
+                                    'metavar': 'Value{True|False} - e.g. True'
                                 },
                                 '--volume-maxsize': {
                                     'help': 'Maximum size of the volume that can be created',
@@ -923,7 +924,6 @@ def tenant_vm_ls(args):
     print(cli_table.create(header, rows))
 
 
-
 def tenant_access_add(args):
     """ Handle tenant access command """
     volume_maxsize_in_MB = None
@@ -957,7 +957,7 @@ def tenant_access_set(args):
 
     error_info = auth_api._tenant_access_set(name=args.name, 
                                              datastore=args.datastore,
-                                             allow_create=args.allow_create, 
+                                             allow_create=args.allow_create,
                                              volume_maxsize_in_MB=volume_maxsize_in_MB, 
                                              volume_totalsize_in_MB=volume_totalsize_in_MB)
 

esx_service/cli/vmdkops_admin_test.py

@@ -735,30 +735,54 @@ def test_tenant_access(self):
         # change total_volume size to 2GB
         volume_maxsize_in_MB = convert.convert_to_MB("1000MB")
         volume_totalsize_in_MB = convert.convert_to_MB("3GB")
-        error_info = auth_api._tenant_access_set(name=self.tenant1_name,
-                                                 datastore=self.datastore_name,
-                                                 allow_create="True",
-                                                 volume_maxsize_in_MB=volume_maxsize_in_MB,
-                                                 volume_totalsize_in_MB=volume_totalsize_in_MB
-                                             )
-        self.assertEqual(None, error_info)
 
-        error_info, privileges = auth_api._tenant_access_ls(self.tenant1_name)
-        self.assertEqual(None, error_info)
+        self.parser = vmdkops_admin.create_parser()
 
-        rows = vmdkops_admin.generate_tenant_access_ls_rows(privileges)
-        self.assertEqual(len(rows), 1)
+        privilege_test_info = [
+            ["False", "False"],
+            ["FALSE", "False"],
+            ["false", "False"],
+            ["True", "True"],
+            ["TRUE", "True"],
+            ["true", "True"],
+        ]
+
+        for val in privilege_test_info:
+            command = ("tenant access set --name={0} ".format(self.tenant1_name))
+            command += ("--datastore={0} ".format(self.datastore_name))
+            command += ("--allow-create={0} ".format(val[0]))
+            command += ("--volume-maxsize=500MB --volume-totalsize=1GB")
+
+            args = self.parser.parse_args(command.split())
+            error_info = vmdkops_admin.tenant_access_set(args)
+            self.assertEqual(None, error_info)
 
+            error_info, privileges = auth_api._tenant_access_ls(self.tenant1_name)
+            self.assertEqual(None, error_info)
 
-        expected_output = [self.datastore_name,
-                           "True",
-                           "1000.00MB",
-                           "3.00GB"]
-        actual_output = [rows[0][0],
-                         rows[0][1],
-                         rows[0][2],
-                         rows[0][3]]
-        self.assertEqual(expected_output, actual_output)
+            rows = vmdkops_admin.generate_tenant_access_ls_rows(privileges)
+            self.assertEqual(len(rows), 1)
+
+            expected_output = [self.datastore_name,
+                            val[1],
+                            "500.00MB",
+                            "1.00GB"]
+            actual_output = [rows[0][0],
+                            rows[0][1],
+                            rows[0][2],
+                            rows[0][3]]
+            self.assertEqual(expected_output, actual_output)
+
+        for val in ["INVALID", ""]:
+            command = ("tenant access set --name={0} ".format(self.tenant1_name))
+            command += ("--datastore={0} ".format(self.datastore_name))
+            command += ("--allow-create={0} ".format(val))
+            command += ("--volume-maxsize=500MB --volume-totalsize=1GB")
+
+            args = self.parser.parse_args(command.split())
+            error_info = vmdkops_admin.tenant_access_set(args)
+            expected_message = "Invalid value {0} for allow-create option".format(val)
+            self.assertEqual(expected_message, error_info)
 
         if self.datastore1_name:
             # second datastore is available, can test tenant access add with --default_datastore

esx_service/utils/auth_api.py

@@ -167,6 +167,30 @@ def set_privileges(allow_create, privileges, value):
     privileges[auth_data_const.COL_ALLOW_CREATE] = value
     return privileges
 
+
+def validate_string_to_bool(allow_create):
+    """
+    Validating case insensitive true, false strings
+    Return boolean value of the arguement if it is valid,
+    else return original value. Also return status if arguement is valid or not
+    """
+    is_valid = True
+
+    # If already bool, return
+    if type(allow_create) is bool:
+        return allow_create, is_valid
+
+    allow_create = str(allow_create).lower()
+
+    if allow_create == "true":
+        return True, is_valid
+    elif allow_create == "false":
+        return False, is_valid
+    else:
+        is_valid = False
+        return allow_create, is_valid
+
+
 def generate_privileges(datastore, allow_create, volume_maxsize_in_MB, volume_totalsize_in_MB):
     """ Generate privileges based on input params """
     logging.debug("generate_privileges: datastore=%s allow_create=%s"
@@ -180,7 +204,7 @@ def generate_privileges(datastore, allow_create, volume_maxsize_in_MB, volume_to
     privileges[auth_data_const.COL_DATASTORE_URL] = datastore_url
 
     if allow_create is True:
-        set_privileges(allow_create, privileges, 1)
+        privileges = set_privileges(allow_create, privileges, 1)
 
     if volume_maxsize_in_MB:
         privileges[auth_data_const.COL_MAX_VOLUME_SIZE] = volume_maxsize_in_MB
@@ -195,8 +219,12 @@ def modify_privileges(privileges, allow_create, volume_maxsize_in_MB, volume_tot
     """ Modify privileges based on input params """
     logging.debug("modify_privileges: allow_create=%s, volume_maxsize_in_MB=%s, volume_totalsize_in_MB=%s",
                   allow_create, volume_maxsize_in_MB, volume_totalsize_in_MB)
+
+    # If None, don't change the privilege
+    # If not None, change accordingly
     if allow_create is not None:
-        if allow_create == "True":
+        # allow_create has been validated. It is either True or False
+        if allow_create is True:
             privileges = set_privileges(allow_create, privileges, 1)
         else:
             privileges = set_privileges(allow_create, privileges, 0)
@@ -575,6 +603,21 @@ def _tenant_access_add(name, datastore, allow_create=None, default_datastore=Fal
         error_info = error_code.generate_error_info(ErrorCode.PRIVILEGE_ALREADY_EXIST, name, datastore)
         return error_info
 
+    # Possible value:
+    # None -  no change required
+    # True/False (boolean or string) - change to corresponding True/False
+    if allow_create is not None:
+        # validate to boolean value if it is a string
+        allow_create_val, valid = validate_string_to_bool(allow_create)
+
+        if not valid:
+            err_code = ErrorCode.PRIVILEGE_INVALID_ALLOW_CREATE_VALUE
+            err_msg = error_code.error_code_to_message[err_code].format(allow_create)
+            logging.error(err_msg)
+            return ErrorInfo(err_code, err_msg)
+
+        allow_create = allow_create_val
+
     privileges = generate_privileges(datastore=datastore,
                                      allow_create=allow_create,
                                      volume_maxsize_in_MB=volume_maxsize_in_MB,
@@ -652,6 +695,17 @@ def _tenant_access_set(name, datastore, allow_create=None, volume_maxsize_in_MB=
         error_info = ErrorInfo(err_code, err_msg)
         return error_info
 
+    if allow_create is not None:
+        allow_create_val, valid = validate_string_to_bool(allow_create)
+
+        if not valid:
+            err_code = ErrorCode.PRIVILEGE_INVALID_ALLOW_CREATE_VALUE
+            err_msg = error_code.error_code_to_message[err_code].format(allow_create)
+            logging.error(err_msg)
+            return ErrorInfo(err_code, err_msg)
+
+        allow_create = allow_create_val
+
     privileges_dict = generate_privileges_dict(privileges[0])
     logging.debug("_tenant_access_set: originial privileges_dict=%s", privileges_dict)
     privileges_dict = modify_privileges(privileges=privileges_dict,

esx_service/utils/error_code.py

@@ -36,6 +36,7 @@ class ErrorCode:
     PRIVILEGE_NOT_FOUND = 201
     PRIVILEGE_ALREADY_EXIST = 202
     PRIVILEGE_INVALID_VOLUME_SIZE = 203
+    PRIVILEGE_INVALID_ALLOW_CREATE_VALUE = 204
     # Privilege related error code end
 
     # DATASTORE related error code start
@@ -72,6 +73,7 @@ class ErrorCode:
     ErrorCode.PRIVILEGE_NOT_FOUND : "No privilege exists for ({0}, {1})",
     ErrorCode.PRIVILEGE_ALREADY_EXIST : "Privilege for ({0}, {1}) already exists",
     ErrorCode.PRIVILEGE_INVALID_VOLUME_SIZE : "Volume max size {0}MB exceeds the total size {1}MB",
+    ErrorCode.PRIVILEGE_INVALID_ALLOW_CREATE_VALUE : "Invalid value {0} for allow-create option",
 
     ErrorCode.DEFAULT_DS_NOT_SET : "Default datastore is not set",
     ErrorCode.DS_NOT_EXIST : "Datastore {0} does not exist",