Skip to content

Commit

Permalink
fix: state merged for panos_security_rule
Browse files Browse the repository at this point in the history 
* Also fixes panos_template mode xpath error on second run
  • Loading branch information
@alperenkose
alperenkose committed Jul 15, 2024
1 parent c64cd79 commit 68341f4
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 62 deletions.
82 changes: 69 additions & 13 deletions plugins/module_utils/panos.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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"] = {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down
93 changes: 44 additions & 49 deletions plugins/modules/panos_security_rule.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...
Expand All @@ -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:
Expand All @@ -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...
Expand All @@ -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 <tab>
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
Expand All @@ -148,53 +142,46 @@
- 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:
- Description of the security rule.
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.
type: list
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:
Expand All @@ -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: >
Expand Down Expand Up @@ -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",
Expand All @@ -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(),
Expand All @@ -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(
Expand Down
Loading

0 comments on commit 68341f4

Please sign in to comment.