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

feat: add rip support #330

Merged
merged 22 commits into from
Jul 2, 2021

Conversation

markharden817
Copy link
Contributor

@markharden817 markharden817 commented Apr 23, 2021

Description

Adding RIP configuration objects to be added to panos.network.VirtualRouter instance.

The following classes have been added to enable this functionality:

  • Rip
  • RipInterface
  • RipAuthProfile
  • RipAuthProfileMd5
  • RipExportRules

Added Unit tests to cover the additional classes

  • TestRip
  • TestRipAuthProfile
  • TestRipAuthProfileMd5
  • TestRipInterface
  • TestRipExportRules

Feature enhancement #329

Motivation and Context

Current network design requires RIP routing configuration on VirtualRouter objects.

How Has This Been Tested?

pylint pass
flake8 pass
pytest live tests pass

image

Functionality tested with the following driver script:

import os

from panos.firewall import Firewall
from panos.network import (
    RedistributionProfile,
    Rip,
    RipAuthProfile,
    RipAuthProfileMd5,
    RipExportRules,
    RipInterface,
    VirtualRouter,
)

HOSTNAME = os.environ["PAN_HOSTNAME"]
USERNAME = os.environ["PAN_USERNAME"]
PASSWORD = os.environ["PAN_PASSWORD"]

VR_NAME = "vr_1"
REDIST_NAME = "redist_1"
VR_INTERFACES = ["ethernet1/1"]
REDIST_INTERFACE = "ethernet1/1"


fw = Firewall(HOSTNAME, USERNAME, PASSWORD)

# find or create a virtual router
vr = fw.find_or_create(VR_NAME, VirtualRouter, interface=VR_INTERFACES)

# create redist profile
redist_profile = RedistributionProfile(
    name=REDIST_NAME, priority=1, action="redist"
)
vr.add(redist_profile)

rip_spec = {
    "enable": True,
    "reject_default_route": True,
    "allow_redist_default_route": True,
    "delete_intervals": 121,
    "expire_intervals": 181,
    "interval_seconds": 2,
    "update_intervals": 31,
}
rip = Rip(**rip_spec)

# add rip auth (password)
rip_auth = RipAuthProfile(
    name="rip_profile_1", auth_type="password", password="#Password1"
)
rip.add(rip_auth)

# add rip auth (md5)
rip_auth = RipAuthProfile(name="rip_profile_2", auth_type="md5")
md5 = RipAuthProfileMd5(keyid=1, key="#Password1", preferred=True)
rip_auth.add(md5)
rip.add(rip_auth)

# add rip export rules
rip_export = RipExportRule(name=REDIST_NAME, metric=10)
rip.add(rip_export)

# add rip interfaces
rip_interface_spec = {
    "name": REDIST_INTERFACE,
    "enable": True,
    "advertise_default_route": "advertise",
    "metric": 11,
    "auth_profile": "rip_profile_1",
    "mode": "passive",
}
rip_interface = RipInterface(**rip_interface_spec)
rip.add(rip_interface)

# add rip config to virtual router and apply changes
vr.add(rip)
vr.apply()

Result

image
image
image
image

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

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

@markharden817 markharden817 changed the title Feat/add rip support feat: add rip support Apr 23, 2021
@markharden817
Copy link
Contributor Author

Hello @btorresgil @shinmog

Would you please review my PR and comment if any changes are required.

Thank 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.

This is confusing and something that we'll need to enhance tests to check for, but when specifying the xpath of an object it needs to start with a /, however when specifying the xpath for a param it shouldn't. I saw that this got you in a few places and have called them out.

panos/network.py Outdated Show resolved Hide resolved
panos/network.py Outdated Show resolved Hide resolved
panos/network.py Outdated Show resolved Hide resolved
panos/network.py Outdated Show resolved Hide resolved
panos/network.py Outdated Show resolved Hide resolved
panos/network.py Show resolved Hide resolved
panos/network.py Outdated Show resolved Hide resolved
panos/network.py Show resolved Hide resolved
panos/network.py Outdated Show resolved Hide resolved
panos/network.py Outdated Show resolved Hide resolved
@shinmog
Copy link
Collaborator

shinmog commented Jun 24, 2021

As an aside, I appreciate the addition of tests.

@markharden817
Copy link
Contributor Author

As an aside, I appreciate the addition of tests.

I really like your testing framework. It made easy to onboard additional test cases.

@markharden817 markharden817 requested a review from shinmog June 28, 2021 16:52
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
Copy link
Collaborator

shinmog commented Jun 30, 2021

@markharden817

Looks like you need to run make format still.

@shinmog
Copy link
Collaborator

shinmog commented Jun 30, 2021

@markharden817

Some formatting is still off - make format fixes the formatting.

@markharden817
Copy link
Contributor Author

@shinmog
I am not getting any output from make format. I did find more black and isort format changes changes.

@markharden817
Copy link
Contributor Author

@shinmog

Ok, I have resolved the issue with make format. Having issues with poetry on Windows but it is working in WSL.

WSL - mark: pan-os-python on feat/add-rip-support [3.8.10] ❯ poetry run make format
isort --recursive --atomic panos
black .
reformatted /mnt/c/Users/Mark/Documents/projects/pan-os-python/tests/live/test_network.py
reformatted /mnt/c/Users/Mark/Documents/projects/pan-os-python/panos/network.py
All done! ✨ 🍰 ✨

@shinmog shinmog merged commit 7b697fe into PaloAltoNetworks:develop Jul 2, 2021
@welcome-to-palo-alto-networks
Copy link

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

github-actions bot pushed a commit that referenced this pull request Jul 23, 2021
## [1.4.0](v1.3.0...v1.4.0) (2021-07-23)

### Features

* Add new HIP options to security rules ([#355](#355)) ([bab35d1](bab35d1)), closes [#345](#345)
* Add RIP support ([#330](#330)) ([7b697fe](7b697fe))
* Audit registered IPs for a specific tag ([5d13e05](5d13e05)), closes [#322](#322)

### Bug Fixes

* Legacy HA interface stubs removed ([8d44951](8d44951)), closes [#350](#350)
@github-actions
Copy link

🎉 This PR is included in version 1.4.0 🎉

The release is available on PyPI and GitHub release

Posted by semantic-release bot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants