Defect: Incorrect expected_status and ip_list data type in method add_api_key

[janchan-hk]

Description

There are a few issues found in the method add_api_key of class Orchestrator:

1. When creating new API key with the method add_api_key, the return value is always be False even though new API key created successfully on the Orchestrator.
2. The type hint for the input argument ip_list is string. However, the actual supported data type should be list of strings.
3. Starting from Orchestrator version 9.3, the API support optional input of rbacRoles and applianceAccessGroup. However, it also take away the input argument of key.

How to reproduce

When creating new API key using method add_api_key in release of 0.16.0-a1:

from pyedgeconnect import Orchestrator

url = "<IP address or URL to an Orchestrator with 9.3 or above>"
api-key = "<API Key for non-user-based authentication>"

orch = Orchestrator(url, api-key)
result = orch.modify_preconfig(
    name="new_api_key",
    permission="net_read_write",
    expiration=1924991999,
    active=True,
    description="new API key",
    ip_list="10.1.1.1/32"
)

1. result will always be False even the new_api_key created successfully on the Orchestrator
2. The new API key would not have the IP allow list configured
3. Currently this method does not support the new features supported in the Orchestrator version 9.3 or above (RBAC roles and appliance access group)

Expected behavior:

1. The method should return a boolean value of True when the specified API key created successfully.
2. The newly created API key should have IP allow list configured as specified with the value of input argument ip_list
3. The method should allow to specify with association of RBAC roles and appliance access group for Orchestrator version 9.3 or above
4. The method should only take the input argument of key when applying on pre-9.3 version of Orchestrator

What could be the root cause:

1. When an API key successfully created by the API, it will have a response status code of 201. However, the method implemented with expected_status of 204 instead.
2. For the ip_list, the swagger document for post-9.3 version (I was testing it with Orchestrator version 9.3.2.40150) incorrectly stated with expected type of string. In the pre-9.3 version (I was testing it with Orchestrator version 9.2.6.40033), the swagger document stated with expected type of [ string ] (list of strings).
3. The implementation of the method add_api_key in release 0.16.0-a1 is lack of logic to handle request differently with different Orchestrator version.

Suggestion of code modification:

The following is the suggested code for fixing the issues, as well as supporting pre- and post- Orchestrator version 9.3:

def add_api_key(
    self,
    name: str,
    permission: str,
    expiration: int,
    active: bool,
    key: str = None,
    description: str = None,
    ip_list: list = None,
    rbac_roles: str = None,
    appliance_access_group: str = None,
) -> bool:
    """Operation to add a new API key to Orchestrator

    .. list-table::
        :header-rows: 1

        * - Swagger Section
        - Method
        - Endpoint
        * - apiKey
        - POST
        - /apiKey

    :param name: API Key Name
    :type name: str
    :param permission: API Key privileges. Allowed values are
        'net_read_write' for RW and 'net_read" for RO
    :type permission: str
    :param expiration: API Key expiration in UNIX timestamp. Key will
        automatically become inactive on expiration date.
    :type expiration: int
    :param active: API Key state is active (True) or inactive (False).
        Inactive keys cannot be used to make requests.
    :type active: bool
    :param key: API Key value, defaults to None
    :type key: str, optional
    :param description: API Key description, defaults to None
    :type description: str, optional
    :param ip_list: List of allowed IP's to make requests with this API
        Key. Leave blank to allow all IP's.
    :param rbac_roles: RBAC roles, defaults to None
    :type description: str, optional
    :param appliance_access_group: Appliance access group, defaults to None
    :type description: str, optional
    :type ip_list: str
    :return: Returns True/False based on successful call
    :rtype: bool
    """

    permission_valid = {"net_read_write", "net_read"}
    if permission not in permission_valid:
        raise ValueError("results: permission must be one of %r." % permission_valid)

    api_key_entry = {
        "name": name,
        "permission": permission,
        "expiration": expiration,
        "active": active,
    }

    if description is not None:
        api_key_entry["description"] = description
    if ip_list is not None:
        api_key_entry["ip_list"] = ip_list

    if self.orch_version >= 9.3:
        if rbac_roles is not None:
            api_key_entry["rbacRoles"] = rbac_roles
        if appliance_access_group is not None:
            api_key_entry["applianceAccessGroup"] = appliance_access_group
    else:
        if key is not None:
            api_key_entry["key"] = key

    return self._post(
        "/apiKey",
        data=api_key_entry,
        expected_status=[201],
        return_type="bool",
    )