From 091ab12692f2c44ce9cac6439c022b32dedc1cb6 Mon Sep 17 00:00:00 2001
From: Bill Dodd <billdodd@gmail.com>
Date: Tue, 3 Sep 2019 20:44:33 -0500
Subject: [PATCH] Fix issues and limitations in account mgmt commands (#58441)
* fix issues and limitations in account mgmt commands
* fix pep8 and example yaml errors
* remove new option and update option aliases
* find next empty slot when adding user via PATCH
---
lib/ansible/module_utils/redfish_utils.py | 249 +++++++++++++++---
.../redfish/redfish_command.py | 103 ++++++--
2 files changed, 297 insertions(+), 55 deletions(-)
diff --git a/lib/ansible/module_utils/redfish_utils.py b/lib/ansible/module_utils/redfish_utils.py
index cf80172b770..b79f1df8769 100644
--- a/lib/ansible/module_utils/redfish_utils.py
+++ b/lib/ansible/module_utils/redfish_utils.py
@@ -43,7 +43,8 @@ class RedfishUtils(object):
msg = self._get_extended_message(e)
return {'ret': False,
'msg': "HTTP Error %s on GET request to '%s', extended message: '%s'"
- % (e.code, uri, msg)}
+ % (e.code, uri, msg),
+ 'status': e.code}
except URLError as e:
return {'ret': False, 'msg': "URL Error on GET request to '%s': '%s'"
% (uri, e.reason)}
@@ -66,7 +67,8 @@ class RedfishUtils(object):
msg = self._get_extended_message(e)
return {'ret': False,
'msg': "HTTP Error %s on POST request to '%s', extended message: '%s'"
- % (e.code, uri, msg)}
+ % (e.code, uri, msg),
+ 'status': e.code}
except URLError as e:
return {'ret': False, 'msg': "URL Error on POST request to '%s': '%s'"
% (uri, e.reason)}
@@ -100,7 +102,8 @@ class RedfishUtils(object):
msg = self._get_extended_message(e)
return {'ret': False,
'msg': "HTTP Error %s on PATCH request to '%s', extended message: '%s'"
- % (e.code, uri, msg)}
+ % (e.code, uri, msg),
+ 'status': e.code}
except URLError as e:
return {'ret': False, 'msg': "URL Error on PATCH request to '%s': '%s'"
% (uri, e.reason)}
@@ -110,9 +113,10 @@ class RedfishUtils(object):
'msg': "Failed PATCH request to '%s': '%s'" % (uri, to_text(e))}
return {'ret': True, 'resp': resp}
- def delete_request(self, uri, pyld):
+ def delete_request(self, uri, pyld=None):
try:
- resp = open_url(uri, data=json.dumps(pyld),
+ data = json.dumps(pyld) if pyld else None
+ resp = open_url(uri, data=data,
headers=DELETE_HEADERS, method="DELETE",
url_username=self.creds['user'],
url_password=self.creds['pswd'],
@@ -123,7 +127,8 @@ class RedfishUtils(object):
msg = self._get_extended_message(e)
return {'ret': False,
'msg': "HTTP Error %s on DELETE request to '%s', extended message: '%s'"
- % (e.code, uri, msg)}
+ % (e.code, uri, msg),
+ 'status': e.code}
except URLError as e:
return {'ret': False, 'msg': "URL Error on DELETE request to '%s': '%s'"
% (uri, e.reason)}
@@ -670,6 +675,60 @@ class RedfishUtils(object):
return response
return {'ret': True, 'changed': True}
+ def _find_account_uri(self, username=None, acct_id=None):
+ if not any((username, acct_id)):
+ return {'ret': False, 'msg':
+ 'Must provide either account_id or account_username'}
+
+ response = self.get_request(self.root_uri + self.accounts_uri)
+ if response['ret'] is False:
+ return response
+ data = response['data']
+
+ uris = [a.get('@odata.id') for a in data.get('Members', []) if
+ a.get('@odata.id')]
+ for uri in uris:
+ response = self.get_request(self.root_uri + uri)
+ if response['ret'] is False:
+ continue
+ data = response['data']
+ headers = response['headers']
+ if username:
+ if username == data.get('UserName'):
+ return {'ret': True, 'data': data,
+ 'headers': headers, 'uri': uri}
+ if acct_id:
+ if acct_id == data.get('Id'):
+ return {'ret': True, 'data': data,
+ 'headers': headers, 'uri': uri}
+
+ return {'ret': False, 'no_match': True, 'msg':
+ 'No account with the given account_id or account_username found'}
+
+ def _find_empty_account_slot(self):
+ response = self.get_request(self.root_uri + self.accounts_uri)
+ if response['ret'] is False:
+ return response
+ data = response['data']
+
+ uris = [a.get('@odata.id') for a in data.get('Members', []) if
+ a.get('@odata.id')]
+ if uris:
+ # first slot may be reserved, so move to end of list
+ uris += [uris.pop(0)]
+ for uri in uris:
+ response = self.get_request(self.root_uri + uri)
+ if response['ret'] is False:
+ continue
+ data = response['data']
+ headers = response['headers']
+ if data.get('UserName') == "" and not data.get('Enabled', True):
+ return {'ret': True, 'data': data,
+ 'headers': headers, 'uri': uri}
+
+ return {'ret': False, 'no_match': True, 'msg':
+ 'No empty account slot found'}
+
def list_users(self):
result = {}
# listing all users has always been slower than other operations, why?
@@ -684,7 +743,7 @@ class RedfishUtils(object):
result['ret'] = True
data = response['data']
- for users in data[u'Members']:
+ for users in data.get('Members', []):
user_list.append(users[u'@odata.id']) # user_list[] are URIs
# for each user, get details
@@ -703,54 +762,184 @@ class RedfishUtils(object):
result["entries"] = users_results
return result
+ def add_user_via_patch(self, user):
+ if user.get('account_id'):
+ # If Id slot specified, use it
+ response = self._find_account_uri(acct_id=user.get('account_id'))
+ else:
+ # Otherwise find first empty slot
+ response = self._find_empty_account_slot()
+
+ if not response['ret']:
+ return response
+ uri = response['uri']
+ payload = {}
+ if user.get('account_username'):
+ payload['UserName'] = user.get('account_username')
+ if user.get('account_password'):
+ payload['Password'] = user.get('account_password')
+ if user.get('account_roleid'):
+ payload['RoleId'] = user.get('account_roleid')
+ response = self.patch_request(self.root_uri + uri, payload)
+ if response['ret'] is False:
+ return response
+ return {'ret': True}
+
def add_user(self, user):
- uri = self.root_uri + self.accounts_uri + "/" + user['userid']
- username = {'UserName': user['username']}
- pswd = {'Password': user['userpswd']}
- roleid = {'RoleId': user['userrole']}
- enabled = {'Enabled': True}
- for payload in username, pswd, roleid, enabled:
- response = self.patch_request(uri, payload)
- if response['ret'] is False:
+ if not user.get('account_username'):
+ return {'ret': False, 'msg':
+ 'Must provide account_username for AddUser command'}
+
+ response = self._find_account_uri(username=user.get('account_username'))
+ if response['ret']:
+ # account_username already exists, nothing to do
+ return {'ret': True, 'changed': False}
+
+ response = self.get_request(self.root_uri + self.accounts_uri)
+ if not response['ret']:
+ return response
+ headers = response['headers']
+
+ if 'allow' in headers:
+ methods = [m.strip() for m in headers.get('allow').split(',')]
+ if 'POST' not in methods:
+ # if Allow header present and POST not listed, add via PATCH
+ return self.add_user_via_patch(user)
+
+ payload = {}
+ if user.get('account_username'):
+ payload['UserName'] = user.get('account_username')
+ if user.get('account_password'):
+ payload['Password'] = user.get('account_password')
+ if user.get('account_roleid'):
+ payload['RoleId'] = user.get('account_roleid')
+
+ response = self.post_request(self.root_uri + self.accounts_uri, payload)
+ if not response['ret']:
+ if response.get('status') == 405:
+ # if POST returned a 405, try to add via PATCH
+ return self.add_user_via_patch(user)
+ else:
return response
return {'ret': True}
def enable_user(self, user):
- uri = self.root_uri + self.accounts_uri + "/" + user['userid']
+ response = self._find_account_uri(username=user.get('account_username'),
+ acct_id=user.get('account_id'))
+ if not response['ret']:
+ return response
+ uri = response['uri']
+ data = response['data']
+
+ if data.get('Enabled'):
+ # account already enabled, nothing to do
+ return {'ret': True, 'changed': False}
+
payload = {'Enabled': True}
- response = self.patch_request(uri, payload)
+ response = self.patch_request(self.root_uri + uri, payload)
+ if response['ret'] is False:
+ return response
+ return {'ret': True}
+
+ def delete_user_via_patch(self, user, uri=None, data=None):
+ if not uri:
+ response = self._find_account_uri(username=user.get('account_username'),
+ acct_id=user.get('account_id'))
+ if not response['ret']:
+ return response
+ uri = response['uri']
+ data = response['data']
+
+ if data and data.get('UserName') == '' and not data.get('Enabled', False):
+ # account UserName already cleared, nothing to do
+ return {'ret': True, 'changed': False}
+
+ payload = {'UserName': ''}
+ if 'Enabled' in data:
+ payload['Enabled'] = False
+ response = self.patch_request(self.root_uri + uri, payload)
if response['ret'] is False:
return response
return {'ret': True}
def delete_user(self, user):
- uri = self.root_uri + self.accounts_uri + "/" + user['userid']
- payload = {'UserName': ""}
- response = self.patch_request(uri, payload)
- if response['ret'] is False:
- return response
+ response = self._find_account_uri(username=user.get('account_username'),
+ acct_id=user.get('account_id'))
+ if not response['ret']:
+ if response.get('no_match'):
+ # account does not exist, nothing to do
+ return {'ret': True, 'changed': False}
+ else:
+ # some error encountered
+ return response
+
+ uri = response['uri']
+ headers = response['headers']
+ data = response['data']
+
+ if 'allow' in headers:
+ methods = [m.strip() for m in headers.get('allow').split(',')]
+ if 'DELETE' not in methods:
+ # if Allow header present and DELETE not listed, del via PATCH
+ return self.delete_user_via_patch(user, uri=uri, data=data)
+
+ response = self.delete_request(self.root_uri + uri)
+ if not response['ret']:
+ if response.get('status') == 405:
+ # if DELETE returned a 405, try to delete via PATCH
+ return self.delete_user_via_patch(user, uri=uri, data=data)
+ else:
+ return response
return {'ret': True}
def disable_user(self, user):
- uri = self.root_uri + self.accounts_uri + "/" + user['userid']
+ response = self._find_account_uri(username=user.get('account_username'),
+ acct_id=user.get('account_id'))
+ if not response['ret']:
+ return response
+ uri = response['uri']
+ data = response['data']
+
+ if not data.get('Enabled'):
+ # account already disabled, nothing to do
+ return {'ret': True, 'changed': False}
+
payload = {'Enabled': False}
- response = self.patch_request(uri, payload)
+ response = self.patch_request(self.root_uri + uri, payload)
if response['ret'] is False:
return response
return {'ret': True}
def update_user_role(self, user):
- uri = self.root_uri + self.accounts_uri + "/" + user['userid']
- payload = {'RoleId': user['userrole']}
- response = self.patch_request(uri, payload)
+ if not user.get('account_roleid'):
+ return {'ret': False, 'msg':
+ 'Must provide account_roleid for UpdateUserRole command'}
+
+ response = self._find_account_uri(username=user.get('account_username'),
+ acct_id=user.get('account_id'))
+ if not response['ret']:
+ return response
+ uri = response['uri']
+ data = response['data']
+
+ if data.get('RoleId') == user.get('account_roleid'):
+ # account already has RoleId , nothing to do
+ return {'ret': True, 'changed': False}
+
+ payload = {'RoleId': user.get('account_roleid')}
+ response = self.patch_request(self.root_uri + uri, payload)
if response['ret'] is False:
return response
return {'ret': True}
def update_user_password(self, user):
- uri = self.root_uri + self.accounts_uri + "/" + user['userid']
- payload = {'Password': user['userpswd']}
- response = self.patch_request(uri, payload)
+ response = self._find_account_uri(username=user.get('account_username'),
+ acct_id=user.get('account_id'))
+ if not response['ret']:
+ return response
+ uri = response['uri']
+ payload = {'Password': user['account_password']}
+ response = self.patch_request(self.root_uri + uri, payload)
if response['ret'] is False:
return response
return {'ret': True}
diff --git a/lib/ansible/modules/remote_management/redfish/redfish_command.py b/lib/ansible/modules/remote_management/redfish/redfish_command.py
index bc06f621e60..f16e486edb0 100644
--- a/lib/ansible/modules/remote_management/redfish/redfish_command.py
+++ b/lib/ansible/modules/remote_management/redfish/redfish_command.py
@@ -41,7 +41,7 @@ options:
username:
required: true
description:
- - User for authentication with OOB controller
+ - Username for authentication with OOB controller
type: str
version_added: "2.8"
password:
@@ -51,26 +51,30 @@ options:
type: str
id:
required: false
+ aliases: [ account_id ]
description:
- - ID of user to add/delete/modify
+ - ID of account to delete/modify
type: str
version_added: "2.8"
new_username:
required: false
+ aliases: [ account_username ]
description:
- - name of user to add/delete/modify
+ - Username of account to add/delete/modify
type: str
version_added: "2.8"
new_password:
required: false
+ aliases: [ account_password ]
description:
- - password of user to add/delete/modify
+ - New password of account to add/modify
type: str
version_added: "2.8"
roleid:
required: false
+ aliases: [ account_roleid ]
description:
- - role of user to add/delete/modify
+ - Role of account to add/modify
type: str
version_added: "2.8"
bootdevice:
@@ -146,6 +150,55 @@ EXAMPLES = '''
username: "{{ username }}"
password: "{{ password }}"
+ - name: Add user
+ redfish_command:
+ category: Accounts
+ command: AddUser
+ baseuri: "{{ baseuri }}"
+ username: "{{ username }}"
+ password: "{{ password }}"
+ new_username: "{{ new_username }}"
+ new_password: "{{ new_password }}"
+ roleid: "{{ roleid }}"
+
+ - name: Add user using new option aliases
+ redfish_command:
+ category: Accounts
+ command: AddUser
+ baseuri: "{{ baseuri }}"
+ username: "{{ username }}"
+ password: "{{ password }}"
+ account_username: "{{ account_username }}"
+ account_password: "{{ account_password }}"
+ account_roleid: "{{ account_roleid }}"
+
+ - name: Delete user
+ redfish_command:
+ category: Accounts
+ command: DeleteUser
+ baseuri: "{{ baseuri }}"
+ username: "{{ username }}"
+ password: "{{ password }}"
+ account_username: "{{ account_username }}"
+
+ - name: Disable user
+ redfish_command:
+ category: Accounts
+ command: DisableUser
+ baseuri: "{{ baseuri }}"
+ username: "{{ username }}"
+ password: "{{ password }}"
+ account_username: "{{ account_username }}"
+
+ - name: Enable user
+ redfish_command:
+ category: Accounts
+ command: EnableUser
+ baseuri: "{{ baseuri }}"
+ username: "{{ username }}"
+ password: "{{ password }}"
+ account_username: "{{ account_username }}"
+
- name: Add and enable user
redfish_command:
category: Accounts
@@ -153,20 +206,10 @@ EXAMPLES = '''
baseuri: "{{ baseuri }}"
username: "{{ username }}"
password: "{{ password }}"
- id: "{{ id }}"
new_username: "{{ new_username }}"
new_password: "{{ new_password }}"
roleid: "{{ roleid }}"
- - name: Disable and delete user
- redfish_command:
- category: Accounts
- command: ["DisableUser", "DeleteUser"]
- baseuri: "{{ baseuri }}"
- username: "{{ username }}"
- password: "{{ password }}"
- id: "{{ id }}"
-
- name: Update user password
redfish_command:
category: Accounts
@@ -174,8 +217,18 @@ EXAMPLES = '''
baseuri: "{{ baseuri }}"
username: "{{ username }}"
password: "{{ password }}"
- id: "{{ id }}"
- new_password: "{{ new_password }}"
+ account_username: "{{ account_username }}"
+ account_password: "{{ account_password }}"
+
+ - name: Update user role
+ redfish_command:
+ category: Accounts
+ command: UpdateUserRole
+ baseuri: "{{ baseuri }}"
+ username: "{{ username }}"
+ password: "{{ password }}"
+ account_username: "{{ account_username }}"
+ roleid: "{{ roleid }}"
- name: Clear Manager Logs with a timeout of 20 seconds
redfish_command:
@@ -220,10 +273,10 @@ def main():
baseuri=dict(required=True),
username=dict(required=True),
password=dict(required=True, no_log=True),
- id=dict(),
- new_username=dict(),
- new_password=dict(no_log=True),
- roleid=dict(),
+ id=dict(aliases=["account_id"]),
+ new_username=dict(aliases=["account_username"]),
+ new_password=dict(aliases=["account_password"], no_log=True),
+ roleid=dict(aliases=["account_roleid"]),
bootdevice=dict(),
timeout=dict(type='int', default=10),
uefi_target=dict(),
@@ -240,10 +293,10 @@ def main():
'pswd': module.params['password']}
# user to add/modify/delete
- user = {'userid': module.params['id'],
- 'username': module.params['new_username'],
- 'userpswd': module.params['new_password'],
- 'userrole': module.params['roleid']}
+ user = {'account_id': module.params['id'],
+ 'account_username': module.params['new_username'],
+ 'account_password': module.params['new_password'],
+ 'account_roleid': module.params['roleid']}
# timeout
timeout = module.params['timeout']