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.

Sign up for GitHub

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

Jump to bottom

Added certificate file validity check for services (#10354) #10354

Merged
merged 5 commits into from
Nov 5, 2019
Merged

Added certificate file validity check for services (#10354) #10354

merged 5 commits into from
Nov 5, 2019

Conversation

SourceDoctor
Copy link
Member

check for certificates with following attributes

  • age in days
  • remaining validity in days

@SourceDoctor SourceDoctor changed the title snmp extend for monitoring certificate file validity monitoring certificate file validity Jun 20, 2019
@SourceDoctor SourceDoctor changed the title monitoring certificate file validity certificate file validity check Jun 29, 2019
@SourceDoctor
Copy link
Member Author

something else to do on this Extension before merging?

@laf
Copy link
Member

laf commented Jul 2, 2019

Thanks for this.

Does it not make sense to check a url for the cert rather than a file?

Would also need docs updating to show how to use this.

@Rosiak
Copy link
Member

Rosiak commented Jul 2, 2019

Why not just use the Nagios check_ssl_cert?

@SourceDoctor
Copy link
Member Author

@laf
i modified librenms-agent so it checks a URL and not more a file.
librenms/librenms-agent#239
I would love to update docs, but don't know where to find this file
@Rosiak
i prefer to do Monitoring stuff via snmp only ;-)

@SourceDoctor
Copy link
Member Author

@laf
something else to modify?

@laf
Copy link
Member

laf commented Jul 9, 2019

Docs are here for you to edit: https://github.com/SourceDoctor/librenms/blob/certificate_check/doc/Extensions/Applications.md

@SourceDoctor
Copy link
Member Author

@laf
Documentation is added

@PipoCanaja
Copy link
Contributor

@SourceDoctor There is a conflict with other changes that took place on Applications.md. Could you check it ?
Thanx

@SourceDoctor
Copy link
Member Author

@PipoCanaja
conflict solved

@PipoCanaja PipoCanaja added the Needs Testing Waiting for others to verify that the code functions properly. label Jul 10, 2019
@SourceDoctor
Copy link
Member Author

@PipoCanaja @laf
something else to change on code?

@SourceDoctor SourceDoctor mentioned this pull request Aug 11, 2019
Merged
laf
laf previously approved these changes Oct 22, 2019
View reviewed changes
Copy link
Member

@laf laf left a comment

Choose a reason for hiding this comment

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

LGTM. Needs librenms/librenms-agent#239 merging once tests pass.

@SourceDoctor
Copy link
Member Author

@laf resolved Merge Conflicts
now everything should be fine for merging :-)

@laf laf removed the Needs Testing Waiting for others to verify that the code functions properly. label Nov 5, 2019
@laf laf changed the title certificate file validity check Added certificate file validity check for services (#10354) Nov 5, 2019
laf
laf approved these changes Nov 5, 2019
View reviewed changes
Copy link
Member

@laf laf left a comment

Choose a reason for hiding this comment

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

LGTM

@laf laf merged commit 4d50fef into librenms:master Nov 5, 2019
@murrant
Copy link
Member

murrant commented Nov 25, 2019

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

https://community.librenms.org/t/v1-58-release-changelog-november-2019/10224/1

@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@laf laf laf approved these changes

Assignees
No one assigned
Labels
Applications 💾
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@SourceDoctor @laf @Rosiak @PipoCanaja @murrant @kkrumm1