Bug fixes for GCP modules (#54256)

This commit is contained in:
The Magician 2019-03-22 12:37:17 -07:00 committed by ansibot
parent 5fb416ae34
commit f7177006f5
3 changed files with 85 additions and 69 deletions

View file

@ -237,12 +237,16 @@ class GcpModule(AnsibleModule):
return new
# This class takes in two dictionaries `a` and `b`.
# These are dictionaries of arbitrary depth, but made up of standard Python
# types only.
# This differ will compare all values in `a` to those in `b`.
# Note: Only keys in `a` will be compared. Extra keys in `b` will be ignored.
# Note: On all lists, order does matter.
# This class does difference checking according to a set of GCP-specific rules.
# This will be primarily used for checking dictionaries.
# In an equivalence check, the left-hand dictionary will be the request and
# the right-hand side will be the response.
# Rules:
# Extra keys in response will be ignored.
# Ordering of lists does not matter.
# - exception: lists of dictionaries are
# assumed to be in sorted order.
class GcpRequest(object):
def __init__(self, request):
self.request = request
@ -253,31 +257,48 @@ class GcpRequest(object):
def __ne__(self, other):
return not self.__eq__(other)
# Returns the difference between `self.request` and `b`
def difference(self, b):
return self._compare_dicts(self.request, b.request)
# Returns the difference between a request + response.
# While this is used under the hood for __eq__ and __ne__,
# it is useful for debugging.
def difference(self, response):
return self._compare_value(self.request, response.request)
def _compare_dicts(self, dict1, dict2):
def _compare_dicts(self, req_dict, resp_dict):
difference = {}
for key in dict1:
difference[key] = self._compare_value(dict1.get(key), dict2.get(key))
for key in req_dict:
if resp_dict.get(key):
difference[key] = self._compare_value(req_dict.get(key), resp_dict.get(key))
# Remove all empty values from difference.
difference2 = {}
sanitized_difference = {}
for key in difference:
if difference[key]:
difference2[key] = difference[key]
sanitized_difference[key] = difference[key]
return difference2
return sanitized_difference
# Takes in two lists and compares them.
def _compare_lists(self, list1, list2):
# All things in the list should be identical (even if a dictionary)
def _compare_lists(self, req_list, resp_list):
# Have to convert each thing over to unicode.
# Python doesn't handle equality checks between unicode + non-unicode well.
difference = []
for index in range(len(list1)):
value1 = list1[index]
if index < len(list2):
value2 = list2[index]
difference.append(self._compare_value(value1, value2))
new_req_list = self._convert_value(req_list)
new_resp_list = self._convert_value(resp_list)
# We have to compare each thing in the request to every other thing
# in the response.
# This is because the request value will be a subset of the response value.
# The assumption is that these lists will be small enough that it won't
# be a performance burden.
for req_item in new_req_list:
found_item = False
for resp_item in new_resp_list:
# Looking for a None value here.
if not self._compare_value(req_item, resp_item):
found_item = True
if not found_item:
difference.append(req_item)
difference2 = []
for value in difference:
@ -286,24 +307,25 @@ class GcpRequest(object):
return difference2
def _compare_value(self, value1, value2):
# Compare two values of arbitrary types.
def _compare_value(self, req_value, resp_value):
diff = None
# If a None is found, a difference does not exist.
# Only differing values matter.
if not value2:
if not resp_value:
return None
# Can assume non-None types at this point.
try:
if isinstance(value1, list):
diff = self._compare_lists(value1, value2)
elif isinstance(value2, dict):
diff = self._compare_dicts(value1, value2)
elif isinstance(value1, bool):
diff = self._compare_boolean(value1, value2)
if isinstance(req_value, list):
diff = self._compare_lists(req_value, resp_value)
elif isinstance(req_value, dict):
diff = self._compare_dicts(req_value, resp_value)
elif isinstance(req_value, bool):
diff = self._compare_boolean(req_value, resp_value)
# Always use to_text values to avoid unicode issues.
elif to_text(value1) != to_text(value2):
diff = value1
elif to_text(req_value) != to_text(resp_value):
diff = req_value
# to_text may throw UnicodeErrors.
# These errors shouldn't crash Ansible and should be hidden.
except UnicodeError:
@ -311,24 +333,43 @@ class GcpRequest(object):
return diff
def _compare_boolean(self, value1, value2):
# Compare two boolean values.
def _compare_boolean(self, req_value, resp_value):
try:
# Both True
if value1 and isinstance(value2, bool) and value2:
if req_value and isinstance(resp_value, bool) and resp_value:
return None
# Value1 True, value2 'true'
elif value1 and to_text(value2) == 'true':
# Value1 True, resp_value 'true'
elif req_value and to_text(resp_value) == 'true':
return None
# Both False
elif not value1 and isinstance(value2, bool) and not value2:
elif not req_value and isinstance(resp_value, bool) and not resp_value:
return None
# Value1 False, value2 'false'
elif not value1 and to_text(value2) == 'false':
# Value1 False, resp_value 'false'
elif not req_value and to_text(resp_value) == 'false':
return None
else:
return value2
return resp_value
# to_text may throw UnicodeErrors.
# These errors shouldn't crash Ansible and should be hidden.
except UnicodeError:
return None
# Python (2 esp.) doesn't do comparisons between unicode + non-unicode well.
# This leads to a lot of false positives when diffing values.
# The Ansible to_text() function is meant to get all strings
# into a standard format.
def _convert_value(self, value):
if isinstance(value, list):
new_list = []
for item in value:
new_list.append(self._convert_value(item))
return new_list
elif isinstance(value, dict):
new_dict = {}
for key in value:
new_dict[key] = self._convert_value(value[key])
return new_dict
else:
return to_text(value)

View file

@ -852,10 +852,10 @@ class InstanceFailoverreplica(object):
self.request = {}
def to_request(self):
return remove_nones_from_dict({u'available': self.request.get('available'), u'name': self.request.get('name')})
return remove_nones_from_dict({u'name': self.request.get('name')})
def from_response(self):
return remove_nones_from_dict({u'available': self.request.get(u'available'), u'name': self.request.get(u'name')})
return remove_nones_from_dict({u'name': self.request.get(u'name')})
class InstanceIpaddressesArray(object):
@ -972,7 +972,6 @@ class InstanceSettings(object):
u'tier': self.request.get('tier'),
u'availabilityType': self.request.get('availability_type'),
u'backupConfiguration': InstanceBackupconfiguration(self.request.get('backup_configuration', {}), self.module).to_request(),
u'settingsVersion': self.request.get('settings_version'),
}
)
@ -983,7 +982,6 @@ class InstanceSettings(object):
u'tier': self.request.get(u'tier'),
u'availabilityType': self.request.get(u'availabilityType'),
u'backupConfiguration': InstanceBackupconfiguration(self.request.get(u'backupConfiguration', {}), self.module).from_response(),
u'settingsVersion': self.request.get(u'settingsVersion'),
}
)

View file

@ -979,11 +979,8 @@ class BucketAclArray(object):
return remove_nones_from_dict(
{
u'bucket': replace_resource_dict(item.get(u'bucket', {}), 'name'),
u'domain': item.get('domain'),
u'email': item.get('email'),
u'entity': item.get('entity'),
u'entityId': item.get('entity_id'),
u'id': item.get('id'),
u'projectTeam': BucketProjectteam(item.get('project_team', {}), self.module).to_request(),
u'role': item.get('role'),
}
@ -993,11 +990,8 @@ class BucketAclArray(object):
return remove_nones_from_dict(
{
u'bucket': item.get(u'bucket'),
u'domain': item.get(u'domain'),
u'email': item.get(u'email'),
u'entity': item.get(u'entity'),
u'entityId': item.get(u'entityId'),
u'id': item.get(u'id'),
u'projectTeam': BucketProjectteam(item.get(u'projectTeam', {}), self.module).from_response(),
u'role': item.get(u'role'),
}
@ -1084,32 +1078,15 @@ class BucketDefaultobjectaclArray(object):
return remove_nones_from_dict(
{
u'bucket': replace_resource_dict(item.get(u'bucket', {}), 'name'),
u'domain': item.get('domain'),
u'email': item.get('email'),
u'entity': item.get('entity'),
u'entityId': item.get('entity_id'),
u'generation': item.get('generation'),
u'id': item.get('id'),
u'object': item.get('object'),
u'projectTeam': BucketProjectteam(item.get('project_team', {}), self.module).to_request(),
u'role': item.get('role'),
}
)
def _response_from_item(self, item):
return remove_nones_from_dict(
{
u'bucket': item.get(u'bucket'),
u'domain': item.get(u'domain'),
u'email': item.get(u'email'),
u'entity': item.get(u'entity'),
u'entityId': item.get(u'entityId'),
u'generation': item.get(u'generation'),
u'id': item.get(u'id'),
u'object': item.get(u'object'),
u'projectTeam': BucketProjectteam(item.get(u'projectTeam', {}), self.module).from_response(),
u'role': item.get(u'role'),
}
{u'bucket': item.get(u'bucket'), u'entity': item.get(u'entity'), u'object': item.get(u'object'), u'role': item.get(u'role')}
)
@ -1250,10 +1227,10 @@ class BucketOwner(object):
self.request = {}
def to_request(self):
return remove_nones_from_dict({u'entity': self.request.get('entity'), u'entityId': self.request.get('entity_id')})
return remove_nones_from_dict({u'entity': self.request.get('entity')})
def from_response(self):
return remove_nones_from_dict({u'entity': self.request.get(u'entity'), u'entityId': self.request.get(u'entityId')})
return remove_nones_from_dict({u'entity': self.request.get(u'entity')})
class BucketVersioning(object):