Ddev: Always create settings.ddev.php, Add conditional include to user's settings.php

[rfay]

What happened (or feature request):

Currently, we generate a settings.php or wp-config.php if one doesn't exist, and we have some explicit support for settings.local.php. However, we don't handle all cases, and we don't communicate to the user what the situation for their particular site.

I'll speak just in Drupal terms here but we need to document and notify the user what is happening if:

• They have a settings.php that does include settings.local.php
• They have a settings.php that does not include settings.local.php
• They have no settings.php file (and we generate it for them)

Also we need to be clear about ownership of the settings.php and settings.local.php; we may want to have a settings.ddev.php as well for ddev-only settings.

And our generated settings.php should include settings.local.php. It does not currently.

In general: Try for flexible and clearly communicated management of settings files, even though different sites manage these differently and different platforms like Pantheon manage them differently.

What you expected to happen:

How to reproduce this:

Anything else do we need to know:

Related source links or issues:

#407 - Provide a warning if settings.local.php not included

[ultimike]

Big +1 to a settings.ddev.php file for all the stuff that DDEV generates and currently puts in settings.local.php.

I _always_ put my own local environment settings in settings.local.php including settings for:

• Stage File Proxy
• Environment Indicator
• All of the stuff from Drupal 8's example.settings.local.php

When using DDEV, I have to create a settings.local2.php for all of my stuff, otherwise DDEV will overwrite it.

[ultimike]

Another other option would be for DDEV to not overwrite the _entire_ settings.local.php file - just its own section.

[rickmanelius]

@ultimike Technically if you remove the #ddev-generated label in settings.local.php, it will no longer attempt to overwrite it if you re-run config. This will be a change in v0.16.0 as a result of https://github.com/drud/ddev/issues/692, which creates the file during config instead of an import-db. We'll leave this open and link because this will be something that will block the v0.16.0 from a docs perspective.

[ultimike]

After reading #692 I'm still not 100% sure that will solve the issue for everyone. I can envision and edge case where someone re-runs ddev config (either on purpose or by accident) and blows away any customizations they have in settings.local.php.

I'm still of the mind that DDEV should only rewrite its own section of settings.local.php on ddev config and not the entire file.

[rfay]

@ultimike ddev will never ever touch a settings file except one that it generated and that has its own signature in it. There's a big warning about that in every generated file.

/**
 #ddev-generated: Automatically generated Drupal settings.php file.
 ddev manages this file and may delete or overwrite the file unless this comment is removed.
 */

[rickmanelius]

With #752 in place, I believe we can close this out for now and modify based on end-user feedback if this continues to be an issue. @ultimike I know you mentioned that this might not solve the issue, but if the #ddev-generated comment is removed, I'm curious what your experience is after that.

[ultimike]

Sorry - been busy the last couple of days updating client sites 😄

I'll play with this a bit (I just updated to 0.16.0) and report back.

[ultimike]

So, creating a brand new local Drupal 8 site (based on https://github.com/drupal-composer/drupal-project/) using DDEV leads to a web/sites/default/settings.local.php file with _only_ the DDEV-injected stuff in it.

This is _okay_, but I'm going to be picky and always want a bit more. One issue with how things work in 0.16.0 is that while the settings.local.php file is created with the database connection information, by default the conditional include at the bottom of settings.php is commented out by default, so the user is still prompted for the database credentials during install 😢

In my ideal situation, the following would happen during/after ddev config:

1. The web/sites/example.settings.local.php file would be copied/renamed to web/sites/default/settings.local.php
2. A new web/sites/default/settings.ddev.php file would be created with the DDEV-injected stuff.
3. A conditional PHP include would be added to either the settings.php or settings.local.php that brings in the settings.ddev.php file.

This way, the DDEV stuff is off in its own little sandbox with a file namespaced as such and developers can utilize the settings.local.php file normally without worrying about messing up the DDEV stuff.

-mike

[ultimike]

I just repeated the process, here's the steps I took to achieve what I proposed in my previous comment after running ddev config:

mv web/sites/default/settings.local.php web/sites/default/settings.ddev.php
cp web/sites/example.settings.local.php web/sites/default/settings.local.php

Edit settings.php and uncomment:

if (file_exists($app_root . '/' . $site_path . '/settings.local.php')) {
  include $app_root . '/' . $site_path . '/settings.local.php';
}

and then add the following:

if (file_exists($app_root . '/' . $site_path . '/settings.ddev.php')) {
  include $app_root . '/' . $site_path . '/settings.ddev.php';
}

That last bit can be added to the bottom of either settings.local.php or settings.php - I'm not sure if there's any pros/cons of doing it either way.

-mike

[ultimike]

The other alternative would be to do all this stuff in an imaginary (but future?) post-config DDEV hook...

[rickmanelius]

Going to look at this again in the upcoming sprint.

[jenlampton]

I'd also prefer the ddev specific settings live in a settings.ddev.php file and allow me to keep all my other things in the settings.local.php file, as usual. For Backdrop, the inclusion of settings.local.php is already uncommented, so an additional section to include settings.ddev.php could always be written to the end of the file in the same way.

[rfay]

After team meeting today Rick has given his go ahead with this as I think it's been outlined here:

1. If not settings.php, create a settings.php that does nothing but (conditionally) include the settings.ddev.php
2. If there is a settings.php, detect if it already has our conditional settings.ddev.php include in it. If it does not have the conditional settings.ddev.php include, add that to the bottom of the settings.php (even though that's touching the user's file)
3. Always create a settings.ddev.php (unless the user has one that we're not managing). This will have the contents that we've used in the past in settings.local.php
4. settings.ddev.php should be able to detect if it's already been included and exit in that case.
5. Add to sites/default a .gitignore that ignores settings.ddev.php (UNLESS there's already a .gitignore in that directory)
6. Wordpress and Backdrop done the same.
7. TYPO3 we think we don't have to do anything, but it would be worth asking the powers that be and running this by them

We know that there's some risk here, and this might have to be improved over time.

It will be lovely if we can get reviews on this PR from some of the people who have chimed in here and in d.o slack.

[ultimike]

@rfay For step 1 above, I'd suggest, "If not settings.php, create a settings.php from default.settings.php that does nothing but (conditionally) include the settings.ddev.php"

I can live with having to manually copy example.settings.local.php to settings.local.php :)

Glad this is getting some attention!

-mike

[alkymst]

@rfay I want to throw out there that for WordPress and the use of Constants for it's config values. We would want to add the WP DDEV config before any values are set, if we need to override any default settings that already exist in the wp-config.php.

It's pretty standard to have a conditional check in your production wp-config.php in WP, so you can over-write the value for local or staging.

/** The name of the database for WordPress */
if (!defined('DB_NAME'))
    define( 'DB_NAME', 'db' );

If this issue is purely for additional settings then adding it the end works for WP as planned.

Environment Based WP Config Examples:
https://abandon.ie/notebook/wordpress-configuration-for-multiple-environments
https://github.com/studio24/wordpress-multi-env-config

[alkymst]

It relates to what I was doing over here - https://github.com/drud/ddev/pull/932

[rfay]

@alkymst so what we'd prefer to do is be able to override anything (trusted host patterns, whatever) that might normally be set in a prod environment. On Drupal settings, you can do that with an include at the bottom. You're saying that's not possible in WP? In general, we want to make only one surgical change to the user's settings file, and put all our needed stuff in settings.ddev.php or equivalent. When we generate a settings file, we're in control, and we don't ever want them to edit it, so we don't need conditionals or the like. When we do that here, we'll be generating a settings file that does nothing but include settings.ddev.php

[alkymst]

@rfay I'm saying WP made a really bad design decision that the community has had to build weird workarounds that are not really standard. So Yes, it's not possible in the same way in WP. The closest we will get is env vars written to .env which has become the best way to handle it for WP, much like the TYPO3 way of handling it. Laravel does the same thing, but by default.

[alkymst]

<?php

/** @var string Directory containing all of the site's files */
$root_dir = dirname(__DIR__);

/** @var string Document Root */
$webroot_dir = $root_dir . '/web';

/**
 * Expose global env() function from oscarotero/env
 */
Env::init();

/**
 * Use Dotenv to set required environment variables and load .env file in root
 */
$dotenv = new Dotenv\Dotenv($root_dir);
if (file_exists($root_dir . '/.env')) {
    $dotenv->load();
    $dotenv->required(['DB_NAME', 'DB_USER', 'DB_PASSWORD', 'WP_HOME', 'WP_SITEURL']);
}

/**
 * Set up our global environment constant and load its config first
 * Default: production
 */
define('WP_ENV', env('WP_ENV') ?: 'production');

$env_config = __DIR__ . '/environments/' . WP_ENV . '.php';

if (file_exists($env_config)) {
    require_once $env_config;
}

/**
 * URLs
 */
define('WP_HOME', env('WP_HOME'));
define('WP_SITEURL', env('WP_SITEURL'));

/**
 * Custom Content Directory
 */
define('CONTENT_DIR', '/app');
define('WP_CONTENT_DIR', $webroot_dir . CONTENT_DIR);
define('WP_CONTENT_URL', WP_HOME . CONTENT_DIR);

/**
 * DB settings
 */
define('DB_NAME', env('DB_NAME'));
define('DB_USER', env('DB_USER'));
define('DB_PASSWORD', env('DB_PASSWORD'));
define('DB_HOST', env('DB_HOST') ?: 'localhost');
define('DB_CHARSET', 'utf8mb4');
define('DB_COLLATE', '');
$table_prefix = env('DB_PREFIX') ?: 'wp_';

/**
 * Authentication Unique Keys and Salts
 */
define('AUTH_KEY', env('AUTH_KEY'));
define('SECURE_AUTH_KEY', env('SECURE_AUTH_KEY'));
define('LOGGED_IN_KEY', env('LOGGED_IN_KEY'));
define('NONCE_KEY', env('NONCE_KEY'));
define('AUTH_SALT', env('AUTH_SALT'));
define('SECURE_AUTH_SALT', env('SECURE_AUTH_SALT'));
define('LOGGED_IN_SALT', env('LOGGED_IN_SALT'));
define('NONCE_SALT', env('NONCE_SALT'));

/**
 * Custom Settings
 */
define('AUTOMATIC_UPDATER_DISABLED', true);
define('DISABLE_WP_CRON', env('DISABLE_WP_CRON') ?: false);
define('DISALLOW_FILE_EDIT', true);

/**
 * Bootstrap WordPress
 */
if (!defined('ABSPATH')) {
    define('ABSPATH', $webroot_dir . '/wp/');
}

[jeffsheltren]

+1 on this approach from the Drupal perspective. Thanks!

[rickmanelius]

@rfay I believe this one is complete as per Drupal/Backdrop behavior and we have other open PRs and issues to address WP. If you'd like to keep open, feel free. I believe this one is complete as specified.

[rfay]

This is complete! It was released with no drama in v1.0.0