Skip to content

Commit

Permalink
fix: object_handling method for overriding defaults
Browse files Browse the repository at this point in the history
Allows overriding values for newly created/replaced objects
for mitigating issues on passing `None` value to sdk for params
with default values.

With this change pan-os-python does not need to accept 'none' value
for the nexthop type in panos_static_route.
  • Loading branch information
alperenkose committed Nov 5, 2024
1 parent d067d91 commit 253b44d
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 16 deletions.
31 changes: 18 additions & 13 deletions plugins/module_utils/panos.py
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,20 @@ def spec_handling(self, spec, module):
"""
pass

def object_handling(self, obj, module):
"""Override to provide custom functionality for newly created/replaced objects.
This method is run for newly created objects with merged state or
created/replaced objects with present state.
By default it will handle default values for objects.
It's advised to call `super().object_handling(obj, module)` if overriden
in the modules.
"""
for key, obj_value in obj.about().items():
if obj_value is None:
setattr(obj, key, self._get_default_value(obj, key))

def pre_state_handling(self, obj, result, module):
"""Override to provide custom pre-state handling functionality."""
pass
Expand Down Expand Up @@ -694,6 +708,8 @@ def apply_state(
continue
other_children.append(x)
item.remove(x)
# object_handling need to be before equal comparison for evaluating defaults
self.object_handling(obj, module)
if not item.equal(obj, compare_children=True):
result["changed"] = True
obj.extend(other_children)
Expand All @@ -703,10 +719,6 @@ def apply_state(
# NOTE 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:
# setattr(obj, key, self._get_default_value(obj, key))
if getattr(item, key) != getattr(obj, key):
try:
obj.update(key)
Expand All @@ -717,9 +729,6 @@ def apply_state(
result["after"] = self.describe(obj)
result["diff"]["after"] = eltostr(obj)
else:
for key, obj_value in obj.about().items():
if obj_value is None:
setattr(obj, key, self._get_default_value(obj, key))
result["after"] = self.describe(obj)
result["diff"]["after"] = eltostr(obj)
try:
Expand All @@ -728,9 +737,7 @@ def apply_state(
module.fail_json(msg="Failed apply: {0}".format(e))
break
else:
for key, obj_value in obj.about().items():
if obj_value is None:
setattr(obj, key, self._get_default_value(obj, key))
self.object_handling(obj, module)
result["changed"] = True
result["before"] = None
result["after"] = self.describe(obj)
Expand Down Expand Up @@ -889,9 +896,7 @@ def apply_state(
)
break
else: # create new record with merge
for key, obj_value in obj.about().items():
if obj_value is None:
setattr(obj, key, self._get_default_value(obj, key))
self.object_handling(obj, module)
result["before"] = None
result["after"] = self.describe(obj)
result["diff"] = {
Expand Down
13 changes: 10 additions & 3 deletions plugins/modules/panos_static_route.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,13 @@
type: str
nexthop_type:
description:
- Type of next hop.
- Type of next hop. Defaults to I("ip-address").
type: str
choices:
- ip-address
- discard
- none
- next-vr
default: 'ip-address'
nexthop:
description:
- Next hop IP address. Required if I(state=present).
Expand Down Expand Up @@ -165,7 +164,11 @@ def spec_handling(self, spec, module):
# default to ip-address when nexthop is set in merged state
# we dont know if object exists or not in merged state, and we dont set default values in module invocation
# in order to avoid unintended updates to non-provided params, but if nexthop is given, type must be ip-address
if module.params["state"] == "merged" and spec["nexthop_type"] is None and spec["nexthop"] is not None:
if (
module.params["state"] == "merged"
and spec["nexthop_type"] is None
and spec["nexthop"] is not None
):
spec["nexthop_type"] = "ip-address"

# NOTE merged state have a lot of API issues for updating nexthop we will let the API return it..
Expand All @@ -181,6 +184,10 @@ def spec_handling(self, spec, module):
]
module.fail_json(msg=" ".join(msg))

def object_handling(self, obj, module):
super().object_handling(obj, module)
if module.params.get("nexthop_type") == "none":
setattr(obj, "nexthop_type", None)


def main():
Expand Down
8 changes: 8 additions & 0 deletions plugins/modules/panos_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ def pre_state_handling(self, obj, result, module):
vsys = to_sdk_cls("device", "Vsys")(module.params["default_vsys"])
obj.add(vsys)

def object_handling(self, obj, module):
super().object_handling(obj, module)
# override 'mode' param sdk default to None if it's not set explicitly in invocation.
# SDK has `mode` attribute set to "normal" by default, but there is no xpath for this
# resulting in xpath schema error if default is used.
if module.params.get("mode") is None:
setattr(obj, "mode", None)


def main():
helper = get_connection(
Expand Down

0 comments on commit 253b44d

Please sign in to comment.