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

Rewrite service checks #14071

Draft
wants to merge 69 commits into
base: master
Choose a base branch
from
Draft

Conversation

murrant
Copy link
Member

@murrant murrant commented Jun 26, 2022

Rewrite services to improve security
Adds a services polling module, but still polls through the separate check-services.php or lnms services:poll scripts by default.
Attempts to parse help text into valid options.
Default values for options such as -H.

TODO:

  • Improve help parsing and tests
  • Test polling
  • Test all the webui
  • Convert all old check scripts.

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 <pr_id>, i.e ./scripts/github-apply 5926
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.

@moschroe
Copy link

As suggested on discord, here my proposed extensions for service plugins:

  1. It ought to be possible to keep the original name of perf metrics for the graph and use the canonicalised form only for interfacing with RRD, allowing spaces and punctuation in the legend.
  2. Also, there should be an option available to pass SNMP credentials (+ protocol, sec level, etc.) via envvar to the service script. This would allow using existing SNMP facilities (extend scripts, forwarded ports, firewall rules) while not increasing exposed surface.

I guess the second point would be more straightforward to implement. I might be able to contribute there, let me know whether that would be desirable or I should open a separate PR (probably waiting on this one then)

@murrant
Copy link
Member Author

murrant commented Jul 27, 2022

@moschroe Thanks for the input. If you would like to help out with this, you can send a pull request directly to my branch. https://github.com/murrant/librenms/tree/rewrite_services

Also, creating services is quite broken right now in this PR.

@murrant murrant marked this pull request as draft July 27, 2022 06:47
@murrant
Copy link
Member Author

murrant commented Nov 6, 2023

Needs rework due to not including arguments, only options.

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

Successfully merging this pull request may close these issues.

None yet

2 participants