Skip to content

Added graphing of DDoS-protection related OIDs for PAN-OS#14847

Merged
murrant merged 1 commit intolibrenms:masterfrom
bakerds:master
Apr 7, 2023
Merged

Added graphing of DDoS-protection related OIDs for PAN-OS#14847
murrant merged 1 commit intolibrenms:masterfrom
bakerds:master

Conversation

@bakerds
Copy link
Copy Markdown
Contributor

@bakerds bakerds commented Feb 15, 2023

Please give a short description what your pull request is for

DO NOT DELETE THE UNDERLYING TEXT

Please note

Please read this information carefully. You can run ./lnms dev:check to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.
  • If my Pull Request makes discovery/polling/yaml changes, I have added/updated test data.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply 14847
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@bakerds
Copy link
Copy Markdown
Contributor Author

bakerds commented Feb 15, 2023

I had difficulty generating tests/data/panos.json with the new OIDs, however the data is present in the snmprec file included in this PR.

@Jellyfrog
Copy link
Copy Markdown
Member

Jellyfrog commented Feb 28, 2023

Youre missing actually polling the data in includes/definitions/discovery/panos.yaml, and therefor no test data is updated.

@Jellyfrog Jellyfrog added Device 🖥️ New or added device support Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/ labels Feb 28, 2023
@github-actions
Copy link
Copy Markdown

Please add test data so we can ensure your change is not broken in the future.
Read the docs to find out how: https://docs.librenms.org/Developing/os/Test-Units

@bakerds
Copy link
Copy Markdown
Contributor Author

bakerds commented Mar 1, 2023

Hi @Jellyfrog
It works though, and the other special firewall graphs that already exist aren't defined in includes/definitions/discovery/panos.yaml

Edit -- there is no test data in tests/data/panos.json for the other graphs either, actually. For example, there's a pre-existing custom graph for panSessionActiveICMP, which is not polled anywhere else, and does not appear in tests/data/panos.json

@murrant thoughts on this?

@bakerds
Copy link
Copy Markdown
Contributor Author

bakerds commented Apr 5, 2023

Can someone review this for merging? As far as I can tell, the unit tests don't work for this type of graph/data.

@murrant murrant merged commit a340672 into librenms:master Apr 7, 2023
@librenms-bot
Copy link
Copy Markdown

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/23-5-0-changelog/21481/1

TheMysteriousX pushed a commit to TheMysteriousX/librenms that referenced this pull request Aug 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Device 🖥️ New or added device support Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants