Closed (fixed)
Project:
Maintenance Mode Redirect
Version:
2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
8 Oct 2019 at 16:52 UTC
Updated:
23 Jun 2024 at 08:34 UTC
Jump to comment: Most recent
Comments
Comment #2
shrop commentedThat 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?
Comment #3
ben coleman commentedThe 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.
Comment #4
ben coleman commentedLooking 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.
Comment #5
ben coleman commentedI'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.
Comment #6
aaron.ferris commentedI'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.
Comment #8
aaron.ferris commentedInitial 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.
Comment #9
aaron.ferris commentedComment #10
avpadernoI 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.
Comment #13
avpaderno