Proposal: Replace all BOA control files used in global.inc with a per-platform and per-site, well documented control.ini file [#2052703]

My feeling is that the use of control files to alter site behaviour not ideal practice within the PHP world, and very poor DX. I commented on this over at #2015553: Force user_register policy unless modules/disable_user_register_protection.info exists #8:

I suppose what this boils down to is that I believe control files are great for many things outside Drupal (in Bash/Perl), but since we're already in PHP at this point a standardised config file and some nice clean functions might be more appropriate as the system grows. And, if those settings were editable from Aegir or the site itself then that would be really great -- not to mention better UX/DX since the help would be right there by the settings.

A related discussion continued on #2034391: Document all control files and make it easy to find. There Omega8cc said:

I would rather create a single file, like sites/all/modules/CONTROL.info and sites/foo.com/modules/CONTROL.info with all control files explained inside - or even a link to docs on the web, so we don't need to update this file. This is still not enough, because you can't expect that people will expect and find docs like this. This still needs to be documented properly on the web, in the project own docs/ and clearly linked in every welcome and upgrade e-mail.

That's pretty much what I'm arguing for, BUT instead of that CONTROL.info explaining the other files, it actually CONTAINS these settings and therefore replaces the need for any further files... I'm proposing:

Each CONTROL.info file (or whatever name it gets) is structured in the INI file format, much like normal Drupal module/theme .info files. For example:

;; BOA Control options file
;; ====================
;;
;; NOTE: This file overrides the defaults in /data/conf/global.inc for the current platform or
;; site (dependent on location of this file).
;;
;; See http://omega8.cc/wherever-the-docs-are
;; ------------
;; When set to TRUE allows anonymous users to add content. Best practice and default is FALSE.
allow_anon_node_add = FALSE

;; Controls how long the NginX speed cache holds copies served pages.
;; Values are: "Day", "Hour", "Quarter", and "Disable" to turn off page caching. The default is "Hour".
nginx_cache_duration = "Hour"

;; This is an example future option, remove the starting semicolon on the line below to use this feature.
; allow_self_awareness = TRUE
These can either be read with a copy of drupal_parse_info_file(), or the simpler and faster would be to use PHP's parse_ini_file() - my vote is for this, especially as that allows for the 'defaults' to be defined (e.g. define('NGINX_CACHE_HOUR', 'Hour');) and be used inside the info files, which could help with documenation, consistency and readability.
The load order is:
1. Default settings are held as associative array in the top of global.inc, called $_BOA_SETTINGS (for example)
2. If sites/all/modules/CONTROL.info exists, parse_ini_file() it and array_merge() the resultant array with $_BOA_SETTINGS.
3. If sites/[domain.com]/modules/CONTROL.info exists, parse_ini_file() it and array_merge() with $_BOA_SETTINGS.
4. global.inc now does its thing as before, but uses the values in $_BOA_SETTINGS rather than looking for the (now obsolete) control files. E.g.
```
if (!$_BOA_SETTINGS['allow_anon_node_add'] && !$has_session) {
  // Don't allow anonymous to add nodes.
  // Do "/node/*/add" and "admin/*" path checks and send 403 if match.
}
```
5. At the end of global.inc we clear up with unset($_BOA_SETTINGS) -- though it might be nice to not clear this on 'dev' domains to help with debugging.
[Later, optional but pretty cool] global.inc is refactored to be more functional, and allow some individual functions to be overridden/changed. This opens up the chance to put the handler function name for certain behaviours/checks into the settings array and allow it to be overridden in the .info file, meaning the core of global.inc can be shrunk and the special cases moved elsewhere, making it less 'one size fits all' and more able to handle specifics without hacking 'core' BOA.

For developers, site builders and hosts the above could be a really big boost for clarity and management IMHO. It also opens up the chance of the settings file being looked for by an BOA Aegir module to list overrides for platforms and sites, again removing doubt and avoiding the need to look at the (undocumented at point of use) filesystem listings to know how a site/platform behaves -- this could later be controlled/updated from within Aegir but still be documented and clear at the file level.

Sure, there would be a small overhead to loading the files, but I'd have thought OS caching would largely mitigate that and it pales into insignificance compared to the hundreds/thousands Drupal loads. Plus, the INI file format is standard, human readable and Perl and Bash can use values from there very easily. I suppose the files could also be JSON, YAML or any other format used in the Drupal world, though .info files are essentially INI files so the momentum is in that direction presently. They could be PHP .inc files too, if that better leverages APC or the Zend OpCache and gains speed.

Fast forward a few years... when there will be a need to support D5, 6, 7, 8 and 9, not to mention hundreds more users of BOA with their own edge cases, libraries, services, etc... I think the current design will struggle. Hopefully the above is the start of a path to something more clean, easy and robust. It also makes the Drupal side of BOA feel less like a set of Sys Admin scripts and more a part of the Drupal/PHP ecosystem.

Obviously the devil is in the details, but what do we all think?

Comments

Comment #0.0

Jim Kirkpatrick CreditAttribution: Jim Kirkpatrick commented 29 July 2013 at 10:43

Issue summary:

View changes

Added rest of Omega8cc's comment

Comment #1

omega8cc CreditAttribution: omega8cc commented 29 July 2013 at 12:14

Thank you for the suggestions. It is a good start!

I think that we should try to keep things even more simple/standard and introduce site and platform level early control overrides as we have them already available globally, using cached .inc (PHP) files. I don't think we need .ini files parsed, config arrays merged etc, but correct me if you see any reason to use them -- I just don't see why we would want these settings to be available at higher bootstrap levels -- they are intended to affect the early settings.php bootstrap level only.

We have now this high level global *pre* override available in adddition to *post* /data/conf/override.global.inc, and to avoid confusion with files like local.settings.php we could probably use there:

/**
 * Global level controls
 *
 */
if (file_exists('/data/conf/control.global.inc')) {
  require_once "/data/conf/control.global.inc";
}

Also, we should probably drop the non-standard location inside the modules directory, and instead follow the local.settings.php example, so I imagine we could do this like this:

/**
 * Platform level controls
 *
 */
if (file_exists('./control.platform.inc')) {
  require_once "./control.platform.inc";
}

/**
 * Site level controls
 *
 */
if (file_exists('sites/'. $_SERVER['SERVER_NAME'] .'/control.site.inc')) {
  require_once 'sites/'. $_SERVER['SERVER_NAME'] .'/control.site.inc';
}

Both site and platform level files should be created on platform or site verify if don't exist, and should include defaults with comments.

We should also check all those files on octopus upgrade (maybe also on barracuda upgrade) to add new controls or apply comments updates.

The questions is, how to handle these files permissions, because in contrast to empty control files, they are potentially dangerous, so we may need to handle them similarly to local.settings.php

What do you think?

Comment #2

Jim Kirkpatrick CreditAttribution: Jim Kirkpatrick commented 31 July 2013 at 16:09

I do actually prefer the 'like local.settings.inc' approach since it avoids the need for array merges as you say, and also keeps things PHP rather than a 'foreign' format... In my proposal I was trying to keep things in line with what we had (.info files), but actually going the whole hog and replacing them with PHP .inc files is the best move IMHO.

The standards we use are then 'be like settings.php' as far as documentation and config changes are concerned, which fits with Drupal's documentation and standards perfectly.

The high-level /data/conf/settings.global.inc is good for cross-server stuff. If we then add
control.platform.inc and control.site.inc files then we have all the flexibility we need without costing any extra work beyond migration tasks as you state. Also, these includes could contain the overriding functions I alluded to in my last bullet point, so here we can have a 'example_com_boa_session_check()' function that can be referred to by the settings array to allow certain behaviours to be overridden as needed, whilst keeping all the goodies in global.inc standard.

As for protecting the file new control.*.inc files, they should be protected per local.settings.inc as you say, perhaps also with a couple of lines at the top like this (assuming $_BOA_SETTINGS is our setting array):

// Bail if no $_BOA_SETTINGS available to secure this file further.
if (!$_BOA_SETTINGS) {
  exit;
}
... rest of file ...

Finally, I'm not sure these files need creating on sites/platforms because we'd have to always load them if so, which if they're just 'default' then is a waste of a load. Perhaps on migration of BOA to new version, they are created as needed if control files exist, otherwise we could put a stub/example file in place like Drupal does with default.settings.php? E.g. default.control.site.inc could contain:

/** @file
 * default.control.site.inc 
 * Holds example controls for controlling BOA behaviour of a site.
 * 
 * NOTE: To have specific site controls enabled for this site:
 *   1. Rename this file to "control.site.inc" (without the 'default.' at the front).
 *   2. Alter the settings for this site as required below.
 *   3. Refer to [LINK TO DOCS] for new settings or advanced features.
 */

/** When set to TRUE allows anonymous users to add content. Best practice and the
 * default is 'FALSE'.
 */
$_BOA_SETTINGS['allow_anon_node_add'] = FALSE;

/** Another example config setting... Remove # to use. Controls how long the NginX
 * speed cache holds copies served pages. Values are: "Day", "Hour", "Quarter", and
 * "Disable" to turn off page caching. The default is "Hour".
 */
#$_BOA_SETTINGS['nginx_cache_duration'] = "Hour"

These default.control.*.inc files can then be auto-updated with new settings/docs every BOA upgrade, and the control.*.inc files -- if they exist -- can then be updated programmatically or manually depending on the nature of the change.

Comment #3

omega8cc CreditAttribution: omega8cc commented 29 July 2013 at 13:29

Title:

Proposal: Replace all .info files used by PHP (global.inc) with a single, well documented CONTROL.info file

» Proposal: Replace all .info files used by PHP (global.inc) with a per-platform and per-site, well documented control.inc file

There are more problems with this approach:

1. There is no FTP/SSH access to the platform level directory in any built-in platform
2. There is no FTP/SSH write access to the site directory.

This is why we are using control files inside sites/all/modules and sites/foo.com/modules directories.

However, it would open a security hole if we allow PHP code there.

Even if we will use current, writable by default locations to make these files accessible, but with protection control similar to that used for local.settings.php we will multiply the risk of locking the site in the DoS state if there is a PHP error made, and in fact the whole protection would be false, because the parent "modules" directories are writable by the group anyway, in contrast to the sites/foo.com directory.

As you can see, there are a few reasons we have decided to use control files and not PHP includes for this purpose.

Any ideas?

Comment #4

Jim Kirkpatrick CreditAttribution: Jim Kirkpatrick commented 29 July 2013 at 13:31

Re #2: To be clear, the logic around [default.]control.*.inc files I was suggesting is:

Global.inc only ever looks for an appropriate control.*.inc file, this is then loaded as needed brining in the new settings and extra functions that might be needed.
default.control.*.inc are ignored by global.inc
Put default.control.*.inc in correct places for every site/platform and ALWAYS update them to the latest when Barracuda or Octopus updates if there has been a change. This ensures the control/settings documentation and examples are ready to use and up to date.
Never auto-update active control.*.inc files unless there's been a change that requires such action -- these files belong to the user. If such changes are made, email the user.
[one day perhaps] Aegir can compare the default.control.*.inc and control.*.inc to show what overrides are in place for a given site or platform within the Aegir interface. Again, one day perhaps these settings can be tweaked via the Aegir UI. Perhaps

Comment #5

Jim Kirkpatrick CreditAttribution: Jim Kirkpatrick commented 29 July 2013 at 14:01

RE #3: Aha... I see. Could we put these files in /sites/[all|sitename]/modules/boa? And then treat them like normal module files?

Then maybe you could lock/disallow changes the /sites/all/modules/boa platform control on built-in platforms, forcing site-level settings only for those? And allow custom platforms control files to be overridden by their owners. Or am I missing something?

I guess treating these files like the local.settings.inc file is overkill because they are not able to open up database or other sensitive data here -- they are a set of changes to an array only that has no knowledge of anything sensitive at this point before bootstrap.

PHP errors will take a site down, sure -- that's life! I don't think we can protect against that kind of thing easily due to limitations with PHP's include error handling.

So we're either in need of a good way to secure these control.inc files, or left having to use the INI approach (or leave the existing flag control files approach in place).

Comment #6

jeremyr CreditAttribution: jeremyr commented 29 July 2013 at 14:46

For what it's worth: Having a platform control file in sites/[domain.com]/modules (or /sites/[domain.com]) as well as a platform specific control file in sites/all/modules/ would be fantastic and ideally writable by the o1.ftp user.

I also like the idea of the control file existing by default... This would make the process of overriding things faster on a platform or per-domain basis quicker. Plus, I think largely people just aren't aware of what some of these config files do, so if it exists by default and tells you how to use it (documentation) then it will likely reduce the number of support issues.

Comment #7

omega8cc CreditAttribution: omega8cc commented 4 August 2013 at 14:16

Title:

Proposal: Replace all .info files used by PHP (global.inc) with a per-platform and per-site, well documented control.inc file

» Proposal: Replace all .info files used by PHP (global.inc) with a per-platform and per-site, well documented control.ini file

We have had control files inside pseudo-modules, but it was too confusing, so the current standard is to have them in the sites/foo.com/modules and sites/all/modules directly.

Locking/unlocking the write access via Aegir tasks only adds complexity and risk to lock the site in the WSOD state. We do this only for local.settings.php because it is *included* with include_once() and expected to have PHP code which may affect the site with possibly broken PHP syntax directly.

On the other side, adding a little overhead with parsing custom ini-like files to keep all controls in the single file per site and per platform to avoid PHP code and permissions drama, sounds like the only viable alternative to the current method. Maybe that overhead is even smaller than (cached by PHP, but still not elegant) control files on-disk-existence check.

I would suggest to use the .ini extension to signal the syntax expected in this file. We should parse them after checking that they exist, so we should provide them initially as default.control.ini and keep these example files updated via /var/xdrago/daily.sh script.

To avoid permissions related issues and issues related to (no) access to the platform root in the built-in platforms, we should use already known "control" locations, so it would be sites/foo.com/modules/control.ini and sites/all/modules/control.ini -- both control files will be group writable by default.

Of course we should auto-generate these files (only once, on the first upgrade) in every site and platform where any previously used control files exist.

~Greg

Comment #8

Jim Kirkpatrick CreditAttribution: Jim Kirkpatrick commented 6 August 2013 at 20:10

Title:

Proposal: Replace all .info files used by PHP (global.inc) with a per-platform and per-site, well documented control.ini file

» Proposal: Replace all BOA control files used in global.inc with a per-platform and per-site, well documented control.ini file

Thanks for the update, Greg. What you say makes a lot of sense.

I suppose we won't know if the single .ini vs multiple control files approach is quicker until we can do some actual tests... However, I doubt we're describing much overhead in either case.

I agree about the file location too... Though maybe a nicer, more explicit name than 'control.ini' is better... Per the comments #1, #2 and #4 I would suggest:

Platform control file, boa-platform-control.ini, is located at sites/all/modules/boa-platform-control.ini so as to be clear what is controling, and that it's part of BOA.
Site control file, boa-site-control.ini, is at sites/foo.com/modules/boa-site-control.ini, again because it's explicit what it does.
The control file docs/defaults above are added in all relevant locations every update as you outlined, whilst the actual control file do not need to be made for each site/platform unless there are existing control .info files present. This way the checks Omega8cc outlined in #1 mean we only ever load overrides, not just everything by default which is pointless.
The settings array is made explicit at the top of the global.inc, ideally in its own include file that can be overridden in the same way -- i.e. default.global-settings.inc is the 'factory settings' defined by you guys, and it's loaded on every request unless there's a global-settings.inc file present, in which case that it loaded instead. OR maybe put the default settings array directly in global.inc, and wrap in a file exists check for global-settings.inc to allow system-wide tweaks? What do you think?

Comment #9

Jim Kirkpatrick CreditAttribution: Jim Kirkpatrick commented 8 August 2013 at 11:11

Example use cases (assuming BOA is installed, developer/host starts adding platforms OR BOA is updated, system updates platforms)

Developer/site builder/host with filesystem access (henceforth 'user') decides some settings need overriding...

There is a system-wide tweak needed, so the user (as a system owner) copies the $_BOA_SETTINGS array into a new PHP include at /data/disk/global-settings.inc. The user edits this file (CAREFULLY!!!!) and sets the system wide-setting accordingly. They save and the settings array comes from the global-settings.inc file, not the defaults at the head of global.inc.
This change is not ideal for one particular platform, so they go to /data/disk/[Octopus Instance]/[static|platforms|distro]/foo and find the sites/all/default.boa-platform-control.ini file. The user copies it to sites/all/boa-platform-control.ini per the instructions in the file and online, then uncomments and alters the appropriate setting as needed. They save and it magically gets included and starts working.
A site needs a few tweaks too... So - as in 2) above - the user copies sites/foo.com/default.boa-site-control.ini to sites/foo.com/boa-site-control.ini, uncomments the appropriate settings, alters them, saves, and it starts working (magically!)
A deep sense of wellbeing is felt by all involved.

If people have comments or issues on the above, then what issue number step is the issue... I expect 1) will need a little more discussion as the system-wide settings override process is not the same as the platform and site ones. I'd like to hear people's preferences there.

Finally I want to make a related suggestion (I've mentioned this elsewhere in the issue): If there was a new BOA Aegir module that could extend the UI to make these settings visible -- and ideally editable -- from with the control panel, this would gain a couple more scoops of awesome IMHO... Actually this approach could resolve the risk around using PHP .inc files since they could be locked by the system and parsed, checked and read/written to from the same module as needed. This would remove the risk associated with the files and also bring these hosting settings into the Aegir world. I suppose .ini files would be fine too, but since the system can control and check them, we can use PHP more safely which has its own performance benefits.

This 'custom Aegir module' approach might be best used only for platform and site overrides. And of course it's a secondary goal -- the proposed single control file approach ideally needs to in place first - though the ultimate choice of .ini vs .inc might be affected by this goal, should it be decided it's a good one to aim for. I would be interested in helping write such a module if we like the idea - and it could start out being read only for documentation/clarity purposes anyway.