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.

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

Feature: path monitoring route #296

Merged

Conversation

kevinhuy
Copy link
Contributor

@kevinhuy kevinhuy commented Jan 2, 2021

Feature / Add ability to enable Path Monitoring in Virtual Router -> Static Route

Enabled the check box "Path Monitoring" under Virtual Router -> Static Route

This is done by adding 3 new params in the existing class StaticRoute.

Add "Path Monitoring Destination" under Virtual Router -> Static Route -> Path Monitoring

This is done by creating an new class PathMonitorDestination in network.py

Motivation and Context

Adds a new functionality.

How Has This Been Tested?

This has been tested in a physical firewall 5220 version 9.0.10 with python 3.6
The python script that i used for the test.

fw = firewall.Firewall(HOSTNAME, USERNAME, api_key=KEY)

router = fw.add(network.VirtualRouter(name='VR2'))

static_route = {
    'name': 'route2',
    'destination': '1.1.1.1/32',
    'nexthop_type': 'ip-address',
    'nexthop': '169.254.255.103',
'enable_path_monitor': True,
'failure_condition' : 'any',
'preemptive_holdtime' : 4
}

static = router.add(network.StaticRoute(**static_route))
static.create()

monitor_destination1 = {
'name': 'test',
'enable': True,
'source': '10.212.100.14/29',
'destination': '1.1.1.1'
}

monitor = static.add(network.PathMonitorDestination(**monitor_destination1))
monitor.create()

monitor_destination2 = {
    'name': 'test2',
    'enable': True,
    'source': '10.212.100.14/29',
    'destination': '2.2.2.2'
}

monitor2 = static.add(network.PathMonitorDestination(**monitor_destination2))
monitor2.create()

the result below.

              route2 {
                destination 1.1.1.1/32;
                nexthop {
                  ip-address 169.254.255.103;
                }
                metric 10;
                path-monitor {
                  enable yes;
                  failure-condition any;
                  hold-time 4;
                  monitor-destinations {
                    test {
                      enable yes;
                      source 10.212.100.14/29;
                      destination 1.1.1.1;
                      interval 3;
                      count 5;
                    }
                    test2 {
                      enable yes;
                      source 10.212.100.14/29;
                      destination 2.2.2.2;
                      interval 3;
                      count 5;
                    }
                  }
                }
              }
             }
           }

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

@welcome-to-palo-alto-networks
Copy link

🎉 Thanks for opening this pull request! We really appreciate contributors like you! 🙌

Copy link
Collaborator

@shinmog shinmog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice!

Besides the comments I've given I have three other pieces of global feedback to give:

First is that all of your path= in the params start with a forward slash - that forward slash needs to be removed.

The second is that there's lots of empty lines between params. There are typically only empty lines between the start of param specification:

params = []

...and the end:

self._params = tuple(params)

The exception to this rule is when you're going to do programatic param initialization (read: looping), so the extra space is there to bring attention to this fact.

The final piece of feedback is that the path monitoring for IPv6 static routes seems to match this exactly. So could you also add your class as a CHILDTYPES to the StaticRouteV6 as well?

Solid work all around!

panos/network.py Outdated

params.append(
VersionedParamPath(
"preemptive_holdtime", vartype="int", path="/path-monitor/hold-time"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change preemptive_holdtime to preemptive_hold_time (and in the docstring as well).

panos/network.py Outdated

Args:
name (str): Name of Path Monitor Destination
enabled (bool): Enable Path Monitor Destination
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be enable not enabled down below, just need to tweak this docstring.

panos/network.py Outdated
@@ -1742,10 +1742,14 @@ class StaticRoute(VersionedPanObject):
interface (str): Next hop interface
admin_dist (str): Administrative distance
metric (int): Metric (Default: 10)

enable_path_monitor (bool): Enable Path Monitor
failure_condition: Path Monitor failure condition set 'any' or 'all'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just missing the (str) type in this docstring.

@kevinhuy
Copy link
Contributor Author

kevinhuy commented Jan 8, 2021

@shinmog thanks a lots for your comment and tips, i will check and make the appropriate change 👍 have a good night :)

@kevinhuy
Copy link
Contributor Author

Hi / Bonjour

@shinmog can you review the last change ? thanks

Copy link
Collaborator

@shinmog shinmog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@shinmog shinmog merged commit 8484b4f into PaloAltoNetworks:develop Jan 14, 2021
@welcome-to-palo-alto-networks

🎉 Congrats on getting your first pull request merged! We here at Palo Alto Networks are so grateful! ❤️

shinmog pushed a commit that referenced this pull request May 6, 2021
This adds static route path monitoring to both the static route class and the IPv6 static route class.  This
also adds a few new parameters to the static route classes themselves to control the path monitoring.

PR #296
github-actions bot pushed a commit that referenced this pull request May 6, 2021
## [1.1.0](v1.0.2...v1.1.0) (2021-05-06)

### Features

* Add `PanDevice.plugins()` ([fa1e4a6](fa1e4a6)), closes [#263](#263)
* Add audit comment support for rules ([#323](#323)) ([350840f](350840f)), closes [#272](#272) [#209](#209)
* Add Authentication profile/sequance ([#286](#286)) ([a66a01d](a66a01d))
* Add device group hierarchy support ([#321](#321)) ([ef90979](ef90979))
* Add DHCP relay support ([#319](#319)) ([fde1fe4](fde1fe4)), closes [#251](#251) [#259](#259)
* Add hit count support (opstate) ([#310](#310)) ([ba1f4d5](ba1f4d5)), closes [#239](#239)
* Add PanDevice.whoami() ([#318](#318)) ([f00d587](f00d587)), closes [#261](#261)
* Add static route path monitoring ([7662496](7662496)), closes [#296](#296)
* Add Zone 8.0+ support ([21f7026](21f7026)), closes [#158](#158)
* Content version by refresh_system_info() ([00f982f](00f982f))
* Save versions during device refresh ([7d7a7f9](7d7a7f9))

### Bug Fixes

* Add missing tag colors ([d021922](d021922)), closes [#267](#267)
* Anti-replay specified twice ([#274](#274)) ([aa30205](aa30205))
* AuthenticationSequence class name ([9632c93](9632c93))
* Correct `clock()` parsing ([48faab5](48faab5)), closes [#278](#278)
* correct user-id tag_user / untag_user ([#299](#299)) ([1de69f8](1de69f8)), closes [#287](#287)
* Correcting profile xpaths ([#333](#333)) ([c1ac9c4](c1ac9c4)), closes [#266](#266)
* Fix placement of default PAN-OS version const ([6fd6ae3](6fd6ae3))
* Fix show_system_resource parsing ([83ab35d](83ab35d)), closes [#280](#280)
btorresgil pushed a commit that referenced this pull request May 10, 2021
## [1.1.0](v1.0.2...v1.1.0) (2021-05-06)

### Features

* Add `PanDevice.plugins()` ([fa1e4a6](fa1e4a6)), closes [#263](#263)
* Add audit comment support for rules ([#323](#323)) ([350840f](350840f)), closes [#272](#272) [#209](#209)
* Add Authentication profile/sequance ([#286](#286)) ([a66a01d](a66a01d))
* Add device group hierarchy support ([#321](#321)) ([ef90979](ef90979))
* Add DHCP relay support ([#319](#319)) ([fde1fe4](fde1fe4)), closes [#251](#251) [#259](#259)
* Add hit count support (opstate) ([#310](#310)) ([ba1f4d5](ba1f4d5)), closes [#239](#239)
* Add PanDevice.whoami() ([#318](#318)) ([f00d587](f00d587)), closes [#261](#261)
* Add static route path monitoring ([7662496](7662496)), closes [#296](#296)
* Add Zone 8.0+ support ([21f7026](21f7026)), closes [#158](#158)
* Content version by refresh_system_info() ([00f982f](00f982f))
* Save versions during device refresh ([7d7a7f9](7d7a7f9))

### Bug Fixes

* Add missing tag colors ([d021922](d021922)), closes [#267](#267)
* Anti-replay specified twice ([#274](#274)) ([aa30205](aa30205))
* AuthenticationSequence class name ([9632c93](9632c93))
* Correct `clock()` parsing ([48faab5](48faab5)), closes [#278](#278)
* correct user-id tag_user / untag_user ([#299](#299)) ([1de69f8](1de69f8)), closes [#287](#287)
* Correcting profile xpaths ([#333](#333)) ([c1ac9c4](c1ac9c4)), closes [#266](#266)
* Fix placement of default PAN-OS version const ([6fd6ae3](6fd6ae3))
* Fix show_system_resource parsing ([83ab35d](83ab35d)), closes [#280](#280)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants