search

PaloAltoNetworks / pan-os-python

The PAN-OS SDK for Python is a package to help interact with Palo Alto Networks devices (including physical and virtualized Next-generation Firewalls and Panorama). The pan-os-python SDK is object oriented and mimics the traditional interaction with the device via the GUI or CLI/API.
https://pan-os-python.readthedocs.io
ISC License
345 stars 170 forks source link

Improve object referencing #23

Open lampwins opened 7 years ago

lampwins commented 7 years ago

As mentioned in #22 and later discussed in greater detail on Gitter, we would like the ability to have direct references to objects that make up a SecurityRule instead of just their names in string form.

@btorresgil and I agreed this functionality should not change the way the library interprets or stores the config tree. So objects will still be children of the same parents they have always been. We are simply talking about creating references to those objects in the SecurityRule at this time.

This will most likely entail a "link_references" like method that the developer will invoke after the SecurityRule and the objects have been refreshed or otherwise loaded.

My thinking is that we add additional attributes to the SecurityRule class to not interfere with the present functionality. So for example, source will still contain a list of string names, but source_obj will be added to store a list of objects.AddressObject and objects.AddressGroup objects. This way, the read/write functionality of the SecurityRule object is not changed. We could also make it so that when an object is added or removed from the source_obj list, the same thing happens to its corresponding string name in source string list.

Obviously the above would apply to all currently represented objects in the objects module that have anything to do with a SecurityRule.

btorresgil commented 7 years ago

More detail on Gitter: https://gitter.im/PaloAltoNetworks/pandevice?at=5882775a074f7be763f81813

lampwins commented 7 years ago

I am going to begin working on this.

btorresgil commented 7 years ago

I actually have this prototyped. Let me clean it up and make a PR later today. You can tell me if it will work for you.

btorresgil commented 7 years ago

Created PR #26

btorresgil commented 7 years ago

Removed 0.4.0 milestone. This feature is necessary, but requires more design consideration. Will come in a follow on release.

shinmog commented 7 years ago

@lampwins

Brian and I were discussing this issue today, I'd like to run a scenario by you for your feedback.

Let's say the user's pandevice object tree has objects for ethernet1/1 and zone L3-trust. However, the zone L3-trust is associated with both ethernet1/1 and ethernet1/2 in reality. Now the user wants to run link_references().

Seems like this should be an exception, otherwise the contents of L3-trust.interface would be [<EthernetInterface ethernet1/1>, "ethernet1/2"]...? There could be a flag that would say, "just link what you can," leaving checking if things are PanObject's or strings / lists up to the script author... Maybe that's acceptable for this functionality..? Or maybe when you link references, you limit it somehow..?

Thoughts?

lampwins commented 7 years ago

@shinmog

Brian and I actually had a chance to chat about this at Ignite. This is a tricky one given the current thought process for linking. Personally, all of my use cases involve pulling in everything and then linking. Given that, I would prefer to not have to make a call to link_references(), I would want it to just do the linking when everything is pulled in automatically.

That being said, there are valid use cases for not doing this. Notably when the config is very large. I lean on the side of leave non-linkable elements as stings. This feels more pythonic to me than raising a fatal error just because a single element in a tree of hundreds or even thousands of elements could not be linked.

I think it fair to say, not everyone is going to use this new link_references() method and those that will, are likely to be okay with some type checking (contradicting myself on what is pythonic ;) ).