From 29ab1825371b7fe7324bd532a6ee9e3f0479e390 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Fri, 18 Aug 2017 15:23:38 -0400 Subject: [PATCH] elasticache_parameter_group: fix KeyError bug #24475 (#24509) * Fix KeyError bug by appending None if key doesn't exist ensure value is the expected type; if if expecting something parsed as truthy try to turn it back into the desired value - fixes result showing always changed since bool compared to str use to_text * use string_types instead of str, remove inline conditionals, abbreviate boolean logic --- .../amazon/elasticache_parameter_group.py | 56 ++++++++++++------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py b/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py index 4f4bfec9ad..9b286ca405 100644 --- a/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py +++ b/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py @@ -115,7 +115,8 @@ changed: # import module snippets from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.ec2 import boto3_conn, get_aws_connection_info, ec2_argument_spec, camel_dict_to_snake_dict -from ansible.module_utils.six import text_type +from ansible.module_utils._text import to_text +from ansible.module_utils.six import string_types import traceback try: @@ -157,8 +158,10 @@ def make_current_modifiable_param_dict(module, conn, name): modifiable_params = {} for param in parameters: - if param["IsModifiable"] and ("AllowedValues" and "ParameterValue") in param: - modifiable_params[param["ParameterName"]] = [param["AllowedValues"], param["DataType"], param["ParameterValue"]] + if param["IsModifiable"]: + modifiable_params[param["ParameterName"]] = [param.get("AllowedValues")] + modifiable_params[param["ParameterName"]].append(param["DataType"]) + modifiable_params[param["ParameterName"]].append(param.get("ParameterValue")) return modifiable_params @@ -174,21 +177,36 @@ def check_valid_modification(module, values, modifiable_params): module.fail_json(msg="%s is not a modifiable parameter. Valid parameters to modify are: %s." % (parameter, modifiable_params.keys())) # check allowed datatype for modified parameters - str_to_type = {"integer": int, "string": text_type} - if not isinstance(new_value, str_to_type[modifiable_params[parameter][1]]): - module.fail_json(msg="%s (type %s) is not an allowed value for the parameter %s. Expected a type %s." % - (new_value, type(new_value), parameter, modifiable_params[parameter][1])) + str_to_type = {"integer": int, "string": string_types} + expected_type = str_to_type[modifiable_params[parameter][1]] + if not isinstance(new_value, expected_type): + if expected_type == str: + if isinstance(new_value, bool): + values[parameter] = "yes" if new_value else "no" + else: + values[parameter] = to_text(new_value) + elif expected_type == int: + if isinstance(new_value, bool): + values[parameter] = 1 if new_value else 0 + else: + module.fail_json(msg="%s (type %s) is not an allowed value for the parameter %s. Expected a type %s." % + (new_value, type(new_value), parameter, modifiable_params[parameter][1])) + else: + module.fail_json(msg="%s (type %s) is not an allowed value for the parameter %s. Expected a type %s." % + (new_value, type(new_value), parameter, modifiable_params[parameter][1])) # check allowed values for modifiable parameters - if text_type(new_value) not in modifiable_params[parameter][0] and not isinstance(new_value, int): - module.fail_json(msg="%s is not an allowed value for the parameter %s. Valid parameters are: %s." % - (new_value, parameter, modifiable_params[parameter][0])) + choices = modifiable_params[parameter][0] + if choices: + if not (to_text(new_value) in choices or isinstance(new_value, int)): + module.fail_json(msg="%s is not an allowed value for the parameter %s. Valid parameters are: %s." % + (new_value, parameter, choices)) # check if a new value is different from current value - if text_type(new_value) != modifiable_params[parameter][2]: + if to_text(values[parameter]) != modifiable_params[parameter][2]: changed_with_update = True - return changed_with_update + return changed_with_update, values def check_changed_parameter_values(values, old_parameters, new_parameters): @@ -216,7 +234,7 @@ def modify(module, conn, name, values): # compares current group parameters with the parameters we've specified to to a value to see if this will change the group format_parameters = [] for key in values: - value = text_type(values[key]) + value = to_text(values[key]) format_parameters.append({'ParameterName': key, 'ParameterValue': value}) try: response = conn.modify_cache_parameter_group(CacheParameterGroupName=name, ParameterNameValues=format_parameters) @@ -237,7 +255,7 @@ def reset(module, conn, name, values): all_parameters = False format_parameters = [] for key in values: - value = text_type(values[key]) + value = to_text(values[key]) format_parameters.append({'ParameterName': key, 'ParameterValue': value}) else: all_parameters = True @@ -269,7 +287,7 @@ def main(): dict( group_family=dict(type='str', choices=['memcached1.4', 'redis2.6', 'redis2.8', 'redis3.2']), name=dict(required=True, type='str'), - description=dict(type='str'), + description=dict(default='', type='str'), state=dict(required=True), values=dict(type='dict'), ) @@ -297,8 +315,8 @@ def main(): exists = get_info(connection, parameter_group_name) # check that the needed requirements are available - if state == 'present' and not (exists and parameter_group_family and group_description): - module.fail_json(msg="Creating a group requires a family group and a description.") + if state == 'present' and not (exists or parameter_group_family): + module.fail_json(msg="Creating a group requires a family group.") elif state == 'reset' and not exists: module.fail_json(msg="No group %s to reset. Please create the group before using the state 'reset'." % parameter_group_name) @@ -313,14 +331,14 @@ def main(): # modify existing group else: modifiable_params = make_current_modifiable_param_dict(module, connection, parameter_group_name) - changed = check_valid_modification(module, values, modifiable_params) + changed, values = check_valid_modification(module, values, modifiable_params) response = modify(module, connection, parameter_group_name, values) # create group else: response, changed = create(module, connection, parameter_group_name, parameter_group_family, group_description) if values: modifiable_params = make_current_modifiable_param_dict(module, connection, parameter_group_name) - changed = check_valid_modification(module, values, modifiable_params) + changed, values = check_valid_modification(module, values, modifiable_params) response = modify(module, connection, parameter_group_name, values) elif state == 'absent': if exists: