Do not rewrite server-status #1339

Merged
merged 1 commit into from Jun 25, 2015

Projects

None yet

4 participants

@clinta
Contributor
clinta commented Jun 25, 2015

Currently users cannot use the check_mk apache plugin to monitor the apache instance that is running librenms. Excluding server-status from the rewrite rules allows users to use mod_status.

@clinta clinta Do not rewrite server-status
Currently users cannot use the check_mk apache plugin to monitor the apache instance that is running librenms. Excluding server-status from the rewrite rules allows users to use mod_status.
0a54602
@f0o f0o self-assigned this Jun 25, 2015
@f0o
Member
f0o commented Jun 25, 2015

I'd prefer the user to configure Name Based VirtualHosts in apache2 (same applies for nginx's equivalent).
/server-status & co shouldnt be listening on anything other than 127.0.0.1 anyhow if you ask me..

@clinta
Contributor
clinta commented Jun 25, 2015

Restricting server-status to localhost can be done by setting the Allow directives in the Location section of the virtualhost.

The check_mk script polls by default on port 80, so if I want it to coexist with librenms, I either have to set librenms to listen on a specific address, rather than * or edit the check_mk script.

In the end the admin can choose either way they want to use server-status, keeping librenms from stomping on it just makes one of the easier methods work out of the box.

@f0o
Member
f0o commented Jun 25, 2015

Unassigning myself, I dont think it's a good idea - apache2 supports virtualhosts for a reason and from my pov the described situation is a perfect usecase for it.

However, I'm not going on a riot if somebody else from the team agrees with you and merges it ;)

@f0o f0o removed their assignment Jun 25, 2015
@laf laf merged commit c05a7cc into librenms:master Jun 25, 2015

2 checks passed

Auto-Deploy Build finished.
Details
Scrutinizer No new issues
Details
@laf
Member
laf commented Jun 25, 2015

I think the fact we ship a httpd config file to bind librenms to all interfaces says we should allow this so merged in.

@clinta clinta deleted the unknown repository branch Jun 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment