Skip to content
This repository has been archived by the owner on Aug 22, 2023. It is now read-only.

Add Support for Device Groups #120

Merged
merged 27 commits into from
Mar 17, 2023
Merged

Add Support for Device Groups #120

merged 27 commits into from
Mar 17, 2023

Conversation

jdrew82
Copy link
Contributor

@jdrew82 jdrew82 commented Dec 14, 2022

This PR includes a few things:

  • Fixes get-device-rules fails if there are disconnected managed devices #112 by adding a try/except block around the call to get_system_info for a device.
  • Renames get_devices to get_devices_from_pano so that we can have a chat command called get-devices.
  • Refactored the get_devices_from_pano to eliminate the extraneous call to Panorama.
  • Added chat command for get-devices to return Device Inventory information to the user.
  • Added a function to get_devicegroups_from_pano to get information about DeviceGroups and their included devices.
  • Added chat command for get-devicegroups to return DeviceGroup information and their associated Devices.
  • Added unit tests for both functions to validate functionality.

@jdrew82
Copy link
Contributor Author

jdrew82 commented Mar 6, 2023

I've also gone ahead and updated the get_all_rules method to also pull in rules for a DeviceGroup/Panorama which addresses #111. I've confirmed this works when you specify an individual Firewall or a DeviceGroup and also extends to the validate-rule-exists chat command and get-device-rules chat command.

@jeffkala
Copy link

jeffkala commented Mar 8, 2023

Can you update the screenshots in the README?

return _device_dict


def get_devicegroups_from_pano(connection: Panorama) -> dict:
Copy link

Choose a reason for hiding this comment

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

Lot of overlap with get_devices_from_pano and get_devicegroups_from_pano`. Anyway we can just make this a singular function and pass what you want?

def get_from_pano(connection: Panorama, groups: bool = True, devices: bool = True) -> dict:
    <merged logic>
    return (_device_dict, _groups_dict)

Just trying to keep DRY principal in the forefront here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the device/device group output of each is different I would still most likely keep two different methods to generate each and just reference those in the get_from_pano method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeffkala I've updated the methods as requested. Is that what you were thinking?

nautobot_plugin_chatops_panorama/worker.py Outdated Show resolved Hide resolved
nautobot_plugin_chatops_panorama/worker.py Outdated Show resolved Hide resolved
@jdrew82
Copy link
Contributor Author

jdrew82 commented Mar 9, 2023

Can you update the screenshots in the README?

I can take some using Mattermost but the existing ones look like they're taken in Slack. I'm not sure how to go about getting dev environment setup to connect to Slack.

@matt852
Copy link
Contributor

matt852 commented Mar 9, 2023

Can you update the screenshots in the README?

I can take some using Mattermost but the existing ones look like they're taken in Slack. I'm not sure how to go about getting dev environment setup to connect to Slack.

I can help with getting a Slack dev environment setup. I'll reach out to you directly.

@jdrew82 jdrew82 merged commit ff00cf0 into develop Mar 17, 2023
@jdrew82 jdrew82 deleted the feat-device_groups branch March 17, 2023 15:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get-device-rules fails if there are disconnected managed devices
3 participants