It would be handy to be able to specify paths that would *not* be redirected. For example, I monitor our sites with Nagios, which uses a particular path on the site. Currently, using Maintenance Mode Redirect, when the site is in maintenance mode, the nagios path is redirected, which errors Nagios reporting. If it was possible to specify paths to be excluded from redirection, we could use this module without disabling Nagios reporting when in Maintenance Mode.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

  • 3086569-add-ability-to Comparechanges, plain diff MR !5
  • 1 hidden branch
  • 2.x Comparecompare

Comments

Ben Coleman created an issue. See original summary.

shrop’s picture

That is a good suggestion for sure. It makes sense because a normal maintenance mode in Drupal provides a different HTTP code of 503.

I think a good next step is to have someone test what the http response is from Drupal and the subsequent redirect using curl or other methods. Also, what does the Nagios check look like? Is it looking for a HTTP 200 response?

ben coleman’s picture

The usual path for Nagios checks is /nagios, potentially with some parameters, though this can be changed via the Nagios Monitoring module configuration. Nagios expects a 200 response.

Note that it might be good to not redirect when the the beginning of the incoming path matches a user-specified "don't redirect path", so parameters don't affect this.

ben coleman’s picture

Looking at the 7.x code a bit(just because the specific situation I'm looking at is in 7.x), letting the user specify specific paths to be excluded from being redirected should be relatively easy. Just append the paths the user entered to the $allowed_paths array, and it should work.

For the situation where you want to excluded from redirection paths that start with a particular path (which would be needed, for example in my nagios example, as the path will include parameters) , you could do something similar could be set up - perhaps an $allowed_start_paths array, and a function to determine if a passed current path starts with one of the paths in $allowed_start_paths.

A glance at the D8 code suggests that something similar should work there.

What I'm wondering about is the UI. Do we want one list of paths, with a 'starts with' checkbox for each, or two lists, one for specific paths, and one for 'starts with' paths.

ben coleman’s picture

Version: 7.x-1.x-dev » 2.x-dev

I'd still like to see this done, but by this time, it needs to go into 2.0.x. Looks to me like you want to be able to add more paths from an additional configure page field to the $allowed_path array in checkForRedirection and also possibility change the logic so those paths can match the beginning of $current_path, instead of being a full match.

aaron.ferris’s picture

Assigned: Unassigned » aaron.ferris

I'll look at implementing something for this, I may need some help with the Nagios example specifically but we can start with a custom form element with a list of allowed URLs.

aaron.ferris’s picture

Initial code change for this is in the MR, flat paths at the moment - need to add an option for any 'Path starts with'.

Also had some issues with fatals in this module, namely:

Error: Class "Url" not found in maintenance_mode_redirect_help() (line 17 of /var/www/html/web/modules/contrib/maintenance_mode_redirect-3086569/maintenance_mode_redirect.module).

TypeError: Drupal\maintenance_mode_redirect\EventSubscriber\MaintenanceModeRedirectSubscriber::checkForRedirection(): Argument #1 ($event) must be of type Drupal\maintenance_mode_redirect\EventSubscriber\GetResponseEvent, Symfony\Component\HttpKernel\Event\RequestEvent given in Drupal\maintenance_mode_redirect\EventSubscriber\MaintenanceModeRedirectSubscriber->checkForRedirection() (line 32 of /var/www/html/web/modules/contrib/maintenance_mode_redirect-3086569/src/EventSubscriber/MaintenanceModeRedirectSubscriber.php).

Both should be fixed in the attached MR.

aaron.ferris’s picture

Assigned: aaron.ferris » Unassigned
Status: Active » Needs review
avpaderno’s picture

Status: Needs review » Reviewed & tested by the community

I reviewed the code and I didn't find anything wrong.
I noticed that the configuration values have names that are prefixed by the module name, but that should be fixed in a different issue.

apaderno changed the visibility of the branch 2.x to hidden.

avpaderno’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.