Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(panos_security_rule): state merged with existing values #570

Merged
merged 6 commits into from
Aug 29, 2024
Merged

fix(panos_security_rule): state merged with existing values #570

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
68341f4
fix: state merged for panos_security_rule
alperenkose Jul 15, 2024
c3fc902
fix: state merged for panos_security_rule
alperenkose Aug 7, 2024
6070430
chore: apply black code formatting
alperenkose Aug 7, 2024
e51e6cf
fix: invocation param as str while config is a member type
alperenkose Aug 8, 2024
ab49672
docs(panos_security_rule): move defaults to description
alperenkose Aug 8, 2024
03eb9e8
Merge branch 'develop' into 564-state-merged-merges-default-values-wi…
alperenkose Aug 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 69 additions & 13 deletions plugins/module_utils/panos.py
View file
Open in desktop
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
Open in desktop
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
Loading