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

Added CheckPoint SecurePlatform support #8000

Merged
merged 10 commits into from Jan 5, 2018

Conversation

Projects
None yet
5 participants
@ricardcc
Contributor

ricardcc commented Jan 2, 2018

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

@CLAassistant

This comment has been minimized.

CLAassistant commented Jan 2, 2018

CLA assistant check
All committers have signed the CLA.

@laf

Few comments in the code to be changed.

Can you also provide the test data from this part of the docs:

https://docs.librenms.org/#Developing/os/Test-Units/#example-workflow

It helps in ensuring we don't break things down the line.

Thanks very much for the PR :)

use LibreNMS\RRD\RrdDefinition;
$tmp_SPLAT = snmp_get_multi_oid($device, 'svnVersion.0 svnApplianceProductName.0 svnApplianceSerialNumber.0', '-OUQs', 'CHECKPOINT-MIB');

This comment has been minimized.

@laf

laf Jan 2, 2018

Member

Can you lowercase this variable please.

bgp-peers: 0
ntp: 0
services: 0
ip6-addresses: 0

This comment has been minimized.

@laf

laf Jan 2, 2018

Member

This should be ipv6-addresses but does the device not support ipv6?

This comment has been minimized.

@ricardcc

ricardcc Jan 3, 2018

Contributor

Ipv6 is supported for this platform, but it's not used. Anyway, I'll change it in order to support it

/*
* LibreNMS
*
* Copyright (c) 2014 Neil Lathwood <https://github.com/laf/ fua http://www.lathwood.co.uk>

This comment has been minimized.

@laf

laf Jan 2, 2018

Member

You need to put your own copyright here.

@@ -0,0 +1,30 @@
os: splat

This comment has been minimized.

@laf

laf Jan 2, 2018

Member

This should be secureplatform rather than splat (unless it's really called splat?). I've seen references to Gaia for the newer gen - what about that?

This comment has been minimized.

@ricardcc

ricardcc Jan 3, 2018

Contributor

Hi,
in network & security environments, checkpoint secureplatform is called splat (it's an acronism). I used this name becase it's how is known this device. But if you think is better change to secureplatform there is no problem.

I saw about CheckPoint Gaia, but for the moment I've not improved it. I saw that it has basic tests, but my company doesn't monitorize any gaia yet. I think in a mid future i will be able to improve it.
There are differences regarding to monitor these devices. Gaia contains OIDs which are not available in Gaia, for this reason I though to use a new os.

ricardcc and others added some commits Jan 4, 2018

@laf

laf approved these changes Jan 4, 2018

@laf

This comment has been minimized.

Member

laf commented Jan 4, 2018

I'm still not 100% on the os name but I'm not going to block it because of that. Anyone else @librenms/reviewers care to take a look?

@murrant

This comment has been minimized.

Member

murrant commented Jan 5, 2018

I found this:

image

Seems to indicate the OS should be secureplatform.

@ricardcc

This comment has been minimized.

Contributor

ricardcc commented Jan 5, 2018

I'm going to change the os name.

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Jan 5, 2018

The inspection completed: No new issues

@laf

This comment has been minimized.

Member

laf commented Jan 5, 2018

I found this:

image

Seems to indicate the OS should be secureplatform.

They did say splat is the well known name for it but secureplatform is more standard for us.

Merging in now as we have some test data and all checks pass.

@laf laf merged commit fc61019 into librenms:master Jan 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details
@lock

This comment has been minimized.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.