From 68341f4226a61641c6d2f83c063cbb5aeba58711 Mon Sep 17 00:00:00 2001 From: Alp Kose Date: Mon, 15 Jul 2024 17:31:08 +0300 Subject: [PATCH] fix: state merged for panos_security_rule * Also fixes panos_template mode xpath error on second run --- plugins/module_utils/panos.py | 82 +++++++++++++++++++---- plugins/modules/panos_security_rule.py | 93 ++++++++++++-------------- plugins/modules/panos_template.py | 5 ++ 3 files changed, 118 insertions(+), 62 deletions(-) diff --git a/plugins/module_utils/panos.py b/plugins/module_utils/panos.py index dffb513dd..25b607426 100644 --- a/plugins/module_utils/panos.py +++ b/plugins/module_utils/panos.py @@ -139,6 +139,7 @@ def __init__( self.parents = () self.sdk_params = {} self.extra_params = {} + self.preset_values = {} self.reference_operations = () self.ansible_to_sdk_param_mapping = {} self.with_uuid = False @@ -489,6 +490,9 @@ def process(self, module): ansible_param, ansible_param ) spec[sdk_param] = module.params.get(ansible_param) + if ansible_param in self.preset_values.keys(): + self.preset_values[sdk_param] = self.preset_values.pop(ansible_param) + if self.with_uuid: spec["uuid"] = module.params["uuid"] if self.with_target: @@ -672,6 +676,9 @@ def apply_state( # Apply the state. if module.params["state"] in ("present", "replaced"): + # create reference object for defaults + obj_cls = obj.__class__ + obj_default = obj_cls() # Apply the config. for item in listing: if item.uid != obj.uid: @@ -690,27 +697,47 @@ def apply_state( if not item.equal(obj, compare_children=True): result["changed"] = True obj.extend(other_children) - result["after"] = self.describe(obj) - result["diff"]["after"] = eltostr(obj) if not module.check_mode: if self.with_update_in_apply_state: - for param in obj.about().keys(): - if getattr(item, param) != getattr(obj, param): + for key, obj_value in obj.about().items(): + # checking defaults for with_update_in_apply_state doesnot have + # a use for now as template, stack and device group dont have + # defaults in the SDK + # it also breaks panos_template as SDK has `mode` attribute set + # to "normal" by default, but there is no xpath for this. + # if obj_value is None: + # default_value = getattr(obj_default, key, None) + # setattr(obj, key, default_value) + if getattr(item, key) != getattr(obj, key): try: - obj.update(param) + obj.update(key) except PanDeviceError as e: module.fail_json( msg="Failed update {0}: {1}".format( - param, e + key, e ) ) + result["after"] = self.describe(obj) + result["diff"]["after"] = eltostr(obj) else: + for key, obj_value in obj.about().items(): + if obj_value is None: + default_value = getattr(obj_default, key, None) + setattr(obj, key, default_value) + result["after"] = self.describe(obj) + result["diff"]["after"] = eltostr(obj) try: obj.apply() except PanDeviceError as e: module.fail_json(msg="Failed apply: {0}".format(e)) break else: + # NOTE alternative is to get defaults from sdk (as in merged) + for key, obj_value in obj.about().items(): + if obj_value is None: + default_value = getattr(obj_default, key, None) + setattr(obj, key, default_value) + result["changed"] = True result["before"] = None result["after"] = self.describe(obj) @@ -829,15 +856,25 @@ def apply_state( updated_params = set([]) for key, obj_value in obj.about().items(): item_value = getattr(item, key, None) - if obj_value is not None: + if obj_value: if isinstance(obj_value, list) or isinstance(item_value, list): if not item_value: item_value = [] - for elm in obj_value: - if elm not in item_value: - updated_params.add(key) - item_value.append(elm) - setattr(item, key, item_value) + # if current config or obj to create is one of the preset values + # (dropdown options in UI) then replace it with the obj value + # since values like "any" can not be in place with other values. + if ((preset_values := self.preset_values.get(key, None)) and + (set(item_value).issubset(preset_values) or + set(obj_value).issubset(preset_values))): + updated_params.add(key) + setattr(item, key, obj_value) + else: + # NOTE what happens here if obj_value is not a list? + for elm in obj_value: + if elm not in item_value: + updated_params.add(key) + item_value.append(elm) + setattr(item, key, item_value) elif item_value != obj_value: updated_params.add(key) setattr(item, key, obj_value) @@ -854,7 +891,23 @@ def apply_state( msg="Failed update {0}: {1}".format(param, e) ) break - else: + else: # create new record with merge + # TODO obj._params is not public attribute on SDK which provide default values + # put default values from sdk for null values for a newly created object + # for _param in obj._params: + # if _param.value is None: + # setattr(obj, _param.name, _param.default) + + # NOTE alternative is to create a temp object with defaults and use values + # from this temp object to fetch defaults for None values and set it for the + # object to create + obj_cls = obj.__class__ + obj_default = obj_cls() + for key, obj_value in obj.about().items(): + if obj_value is None: + default_value = getattr(obj_default, key, None) + setattr(obj, key, default_value) + result["before"] = None result["after"] = self.describe(obj) result["diff"] = { @@ -1368,6 +1421,7 @@ def get_connection( parents=None, sdk_params=None, extra_params=None, + preset_values=None, reference_operations=None, ansible_to_sdk_param_mapping=None, with_uuid=False, @@ -1745,6 +1799,8 @@ class in the package (e.g. - "VirtualRouter"). If the class is a singleton renames[k] = sdk_name spec[k] = sdk_params[k] helper.sdk_params = sdk_params + if preset_values is not None: + helper.preset_values = preset_values if with_gathered_filter: if "gathered_filter" in spec: diff --git a/plugins/modules/panos_security_rule.py b/plugins/modules/panos_security_rule.py index 6c72d8412..c7ed418a3 100644 --- a/plugins/modules/panos_security_rule.py +++ b/plugins/modules/panos_security_rule.py @@ -59,13 +59,12 @@ type: str source_zone: description: - - List of source zones. - default: ["any"] + - List of source zones. Defaults to "any" in SDK. type: list elements: str source_ip: description: - - List of source addresses. + - List of source addresses. Defaults to "any" in SDK. - This can be an IP address, an address object/group, etc. - When referencing predefined EDLs, use config names of the EDLS not their full names. The config names can be found with the CLI... @@ -74,13 +73,12 @@ panw-highrisk-ip-list panw-highrisk-ip-list panw-known-ip-list panw-known-ip-list panw-torexit-ip-list panw-torexit-ip-list - default: ["any"] type: list elements: str source_user: - description: + description: > - Use users to enforce policy for individual users or a group of users. - default: ["any"] + Defaults to "any" in SDK. type: list elements: str hip_profiles: @@ -97,13 +95,12 @@ elements: str destination_zone: description: - - List of destination zones. - default: ["any"] + - List of destination zones. Defaults to "any" in SDK. type: list elements: str destination_ip: description: - - List of destination addresses. + - List of destination addresses. Defaults to "any" in SDK. - This can be an IP address, an address object/group, etc. - When referencing predefined EDLs, use config names of the EDLS not their full names. The config names can be found with the CLI... @@ -112,34 +109,31 @@ panw-highrisk-ip-list panw-highrisk-ip-list panw-known-ip-list panw-known-ip-list panw-torexit-ip-list panw-torexit-ip-list - default: ["any"] type: list elements: str application: description: - - List of applications, application groups, and/or application filters. - default: ["any"] + - List of applications, application groups, and/or application filters. Defaults to "any" in SDK. type: list elements: str service: description: - - List of services and/or service groups. - default: ['application-default'] + - List of services and/or service groups. Defaults to "application-default" in SDK. type: list elements: str category: description: - - List of destination URL categories. + - List of destination URL categories. Defaults to "any" in SDK. - When referencing predefined EDLs, use config names of the EDLS not their full names. The config names can be found with the CLI... request system external-list show type predefined-url name panw-auth-portal-exclude-list panw-auth-portal-exclude-list - default: ["any"] type: list elements: str action: - description: - - Action to apply once rules matches. + description: > + - Action to apply to the rule. No default value in SDK, should be provided when creating + a new resource. type: str choices: - allow @@ -148,20 +142,17 @@ - reset-client - reset-server - reset-both - default: "allow" log_setting: description: - Log forwarding profile. type: str log_start: description: - - Whether to log at session start. - default: false + - Whether to log at session start. Defaults to PAN-OS behaviour. type: bool log_end: description: - - Whether to log at session end. - default: true + - Whether to log at session end. Defaults to PAN-OS behaviour. type: bool description: description: @@ -169,13 +160,12 @@ type: str rule_type: description: - - Type of security rule (version 6.1 of PanOS and above). + - Type of security rule (version 6.1 of PanOS and above). Defaults to "universal" in SDK. type: str choices: - universal - intrazone - interzone - default: 'universal' tag_name: description: - List of tags associated with the rule. @@ -183,18 +173,15 @@ elements: str negate_source: description: - - Match on the reverse of the 'source_ip' attribute - default: false + - Match on the reverse of the 'source_ip' attribute. Defaults to PAN-OS behaviour. type: bool negate_destination: description: - - Match on the reverse of the 'destination_ip' attribute - default: false + - Match on the reverse of the 'destination_ip' attribute. Defaults to PAN-OS behaviour. type: bool disabled: description: - - Disable this rule. - default: false + - Disable this rule. Defaults to PAN-OS behaviour. type: bool schedule: description: @@ -205,9 +192,9 @@ - Send 'ICMP Unreachable'. Used with 'deny', 'drop', and 'reset' actions. type: bool disable_server_response_inspection: - description: + description: > - Disables packet inspection from the server to the client. Useful under heavy server load conditions. - default: false + Defaults to PAN-OS behaviour. type: bool group_profile: description: > @@ -400,24 +387,23 @@ def main(): sdk_params=dict( rule_name=dict(required=True, sdk_param="name"), source_zone=dict( - type="list", elements="str", default=["any"], sdk_param="fromzone" + type="list", elements="str", sdk_param="fromzone" ), source_ip=dict( - type="list", elements="str", default=["any"], sdk_param="source" + type="list", elements="str", sdk_param="source" ), - source_user=dict(type="list", elements="str", default=["any"]), + source_user=dict(type="list", elements="str"), hip_profiles=dict(type="list", elements="str"), destination_zone=dict( - type="list", elements="str", default=["any"], sdk_param="tozone" + type="list", elements="str", sdk_param="tozone" ), destination_ip=dict( - type="list", elements="str", default=["any"], sdk_param="destination" + type="list", elements="str", sdk_param="destination" ), - application=dict(type="list", elements="str", default=["any"]), - service=dict(type="list", elements="str", default=["application-default"]), - category=dict(type="list", elements="str", default=["any"]), + application=dict(type="list", elements="str"), + service=dict(type="list", elements="str"), + category=dict(type="list", elements="str"), action=dict( - default="allow", choices=[ "allow", "deny", @@ -428,21 +414,20 @@ def main(): ], ), log_setting=dict(), - log_start=dict(type="bool", default=False), - log_end=dict(type="bool", default=True), + log_start=dict(type="bool"), + log_end=dict(type="bool"), description=dict(), rule_type=dict( - default="universal", choices=["universal", "intrazone", "interzone"], sdk_param="type", ), tag_name=dict(type="list", elements="str", sdk_param="tag"), - negate_source=dict(type="bool", default=False), - negate_destination=dict(type="bool", default=False), - disabled=dict(type="bool", default=False), + negate_source=dict(type="bool"), + negate_destination=dict(type="bool"), + disabled=dict(type="bool"), schedule=dict(), icmp_unreachable=dict(type="bool"), - disable_server_response_inspection=dict(type="bool", default=False), + disable_server_response_inspection=dict(type="bool"), group_profile=dict(sdk_param="group"), antivirus=dict(sdk_param="virus"), spyware=dict(), @@ -457,6 +442,16 @@ def main(): # TODO(gfreeman) - remove this in the next role release. devicegroup=dict(), ), + preset_values=dict( + source_zone=["any"], + source_ip=["any"], + source_user=["any", "pre-logon", "known-user", "unknown"], + destination_zone=["any", "multicast"], + destination_ip=["any"], + application=["any"], + service=["application-default", "any"], + category=["any"], + ), ) module = AnsibleModule( diff --git a/plugins/modules/panos_template.py b/plugins/modules/panos_template.py index 89ce4670e..e93a34103 100644 --- a/plugins/modules/panos_template.py +++ b/plugins/modules/panos_template.py @@ -60,6 +60,10 @@ - The default vsys in case of a single vsys firewall. type: str default: vsys1 + mode: + description: + - Mode for template. + type: str """ EXAMPLES = """ @@ -117,6 +121,7 @@ def main(): description=dict(), devices=dict(type="list", elements="str"), default_vsys=dict(default="vsys1"), + mode=dict(), ), )