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

Add gateone link #8189

Merged
merged 10 commits into from Feb 10, 2018

Conversation

Projects
None yet
5 participants
@tslytsly
Contributor

tslytsly commented Feb 2, 2018

Added ability to change SSH links to use a gateone server.

https://community.librenms.org/t/feature-request-add-gateone-terminal-emulator-integration/672
https://community.librenms.org/t/enhancement-ability-to-change-ssh-link-on-device-page/1905/6

Add the following to your config.php:

$config['gateone']['server'] = '<your_full_gateone_url>'

If this is present then instead of SSH links being ssh://'.$device['hostname'].'
they are
$config['gateone']['server'] . '?ssh=ssh://' . $device['hostname'] . '&location=' . $device['hostname'] .'

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

@davama

This comment has been minimized.

davama commented Feb 2, 2018

I attempted your pull but i get the error on the web gui

Parse error: syntax error, unexpected '$config' (T_VARIABLE) in /opt/librenms/config.php on line 85
That's where i have the gateone setting

This is what i did: (not sure is the warning is important)

cd /opt/librenms
cp -r html ~/
./scripts/github-apply 8189
<stdin>:64: trailing whitespace.
    
Checking patch html/includes/hostbox-menu.inc.php...
Checking patch html/includes/table/devices.inc.php...
Checking patch html/pages/device.inc.php...
Applied patch html/includes/hostbox-menu.inc.php cleanly.
Applied patch html/includes/table/devices.inc.php cleanly.
Applied patch html/pages/device.inc.php cleanly.
warning: 1 line adds whitespace errors.

Thank you for submitting this PR.

Any input is appreciated

Thanks

@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 2, 2018

No prob, can you show me what you have in config.php for gateone?

@davama

This comment has been minimized.

davama commented Feb 2, 2018

$config['gateone']['server'] = 'https://librenms-lab/gateone'
@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 2, 2018

try:
$config['gateone']['server'] = 'https://librenms-lab/gateone/'

@davama

This comment has been minimized.

davama commented Feb 2, 2018

No change... same output :(

@laf

This comment has been minimized.

Member

laf commented Feb 2, 2018

Missing the semicolon at the end @davama

@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 2, 2018

oh yea... good spot @laf

@davama

This comment has been minimized.

davama commented Feb 2, 2018

Missing the semicolon at the end @davama

Doh!! 🤦‍♂️
Definitely works as expected!

Thank you @laf

@davama

This comment has been minimized.

davama commented Feb 2, 2018

added new option to config.php, $config[gateone][use_librenms_user] i…
…s a boolean that inserts the logged in Librenms user to the SSH link if set to true
@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 3, 2018

Would it be possible to add the username?

@davama your wish is my command...

put $config['gateone']['use_librenms_user'] = true; in your config file.

@davama

This comment has been minimized.

davama commented Feb 3, 2018

@davama your wish is my command...

😅 hehe

@tslytsly you are awesome!! thank you

Works great!

davama added a commit to davama/librenms that referenced this pull request Feb 6, 2018

@davama

This comment has been minimized.

davama commented Feb 6, 2018

@tslytsly

I created a doc to make your life easier 😄

davama@f6a7e59
Not sure how or if it's possible to add to this PR. Hopefully i did this right.

Let me know if it's to your liking

Thanks again!

@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 7, 2018

@davama cool, I was going to do that after the PR got committed.
You will need to submit a PR for the doc separate to this one, it's easy to do.

@murrant

This comment has been minimized.

Member

murrant commented Feb 8, 2018

Please include docs, the separate PR is ok too.

@tslytsly tslytsly referenced this pull request Feb 8, 2018

Merged

Create Gateone.md #8229

1 of 1 task complete
@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 8, 2018

@murrant
PR submitted: #8229

@laf

Thanks for adding this support :)

I've added a few comments in line, it's all just avoiding code duplication really.

<a href="https://' . $device['hostname'] . '" target="_blank" rel="noopener"><i class="fa fa-globe fa-lg icon-theme" title="Launch browser https://' . $device['hostname'] . '"></i></a>
</div>
</div>';
if (isset($config['gateone']['server'])) {

This comment has been minimized.

@laf

laf Feb 8, 2018

Member

This contains a lot of duplication and you should just do the if() checks around the actual lines that need to be different.

<div class="col-xs-1"><a href="ssh://' . $device['hostname'] . '"><i class="fa fa-lock fa-lg icon-theme" title="SSH to ' . $device['hostname'] . '"></i></a></div>
<div class="col-xs-1"><a href="https://' . $device['hostname'] . '" target="_blank" rel="noopener"><i class="fa fa-globe fa-lg icon-theme" title="Launch browser https://' . $device['hostname'] . '"></i></a></div>
if (isset($config['gateone']['server'])) {

This comment has been minimized.

@laf

laf Feb 8, 2018

Member

Same with this, you could move the if() checks to avoid code duplication

<li><a href="https://'.$device['hostname'].'" target="_blank" rel="noopener"><i class="fa fa-globe fa-lg icon-theme" aria-hidden="true"></i> Web</a></li>
<li><a href="ssh://'.$device['hostname'].'" target="_blank" rel="noopener"><i class="fa fa-lock fa-lg icon-theme" aria-hidden="true"></i> SSH</a></li>
<li><a href="telnet://'.$device['hostname'].'" target="_blank" rel="noopener"><i class="fa fa-terminal fa-lg icon-theme" aria-hidden="true"></i> Telnet</a></li>';
if (isset($config['gateone']['server'])) {

This comment has been minimized.

@laf

laf Feb 8, 2018

Member

Again you can move these checks to avoid code duplication.

@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 9, 2018

Changes made @laf
Tested on my system and still working.

@laf

This comment has been minimized.

Member

laf commented Feb 10, 2018

@tslytsly All good now. However I've noticed html/includes/hostbox-menu.inc.php isn't even used anymore, can you delete it please then we can merge in.

@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Feb 10, 2018

The inspection completed: 727 Issues, 37 Patches

@tslytsly

This comment has been minimized.

Contributor

tslytsly commented Feb 10, 2018

@laf done.

@laf laf added the Feature label Feb 10, 2018

@laf laf merged commit 2f6f99a into librenms:master Feb 10, 2018

2 checks passed

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

This comment has been minimized.

Member

laf commented Feb 10, 2018

image

@tslytsly tslytsly deleted the tslytsly:add-gateone-link branch Feb 12, 2018

inetAnt added a commit to criteo-forks/librenms that referenced this pull request Mar 19, 2018

feature: Added gateone link SSH support (librenms#8189)
* adding gateone link

* Added gateone support, now checks config.php for settings

* adding gateone link

* Added gateone support, now checks config.php for settings

* fixed an incorrect url

* fixed syntax errors

* added new option to config.php, $config[gateone][use_librenms_user] is a boolean that inserts the logged in Librenms user to the SSH link if set to true

* removed code duplication

* removed hostbox-menu.inc.php
@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.