Making Drupal automatically installable with default environment variables

That would be awesome!!

Great idea !

Assigning to myself for now, to see what I can hack on here at DrupalCon

Adding thoughts on the approach.

There are some prior similar issues.

I made a library to try and set settings.php for all host providers.

If the host provider doesn't set the env var, we can putenv() from the host vendor code. (Platform, ddev, etc)

Then $databases can always be set from envs.

https://github.com/operations-project/drupal-settings/blob/2.x/Settings%...

Great idea @mglaman!

12 Factor App for the win!

I vote for this change. Thank you!

I would say environment variables should be read from $_SERVER rather than getenv(), as putenv()/getenv() are generally discouraged due to not being thread-safe

as putenv()/getenv() are generally discouraged due to not being thread-safe

We can go either way - `$_ENV` or this. Honestly, I've never had a decent explanation for the thread-safe concern besides the footnote in the env library.

I feel like #2879846: Provide better support for environment variables → could solve this, but it is a bigger meta-question. So I think we can try and solve this particular problem. As long as the environment variable names do not change, the implementation can change.

This could be a duplicate of #2979749: Allow to pick up settings & DB creds from environment variables → , I think. I need to read through more. But there isn't a patch there either.

I've started working on changes to \Drupal\Core\Site\Settings::initialize and will have an MR during contribution sprints.

The tldr explanation is that if PHP is thread-safe, and you call putenv() to set environment variables from a .env file, the environment variables will be set on the whole process, including other threads where they shouldn't be. This problem shouldn't affect server environments where each request has its own process, but in any case, symfony dotenv disables the usePutenv option by default, and to use putenv() with phpdotenv, you have to call Dotenv::createUnsafeImmutable() rather than the usual Dotenv::createImmutable().

I see. Thanks, $_ENV is localized to one process, whereas putenv is for the whole process.

Opened MR. Screwed it up a bit since the issue targeted 11.0.x and not 11.x https://git.drupalcode.org/project/drupal/-/merge_requests/7970

mglaman → changed the visibility of the branch 3445816-making-drupal-automatically to hidden.

mglaman → changed the visibility of the branch 11.x to hidden.

It's ready for basic review. Test incoming

@mglaman there's also a problem with $_ENV, which is that the recommended php configs don't enable it (the recommended value for variables order is "GPCS" rather than "EGPCS", i.e. $_ENV is not registered). So, at least in my experience, $_SERVER ends up being the most universal place to read environment variables from.

Based on #13 it sounds like that's only relevant for putenv, and getenv is fine to use? The MR does not set or put any env vars, it's only reading them back.

@mstrelan the issue is that when setting environment variables, e.g. from an .env file, the app may only set them into $_SERVER and $_ENV, not by calling putenv(), so those environment variables will not be available when calling getenv()

Correct, if someone used vlucas/phpdotenv without the PutenvAdapter there would be nothing returned from getenv. There are quirks. In Symfony they use $_ENV in DotenvVault

$_ENV should be fine to use with those dotenv libraries. But, if you are setting environment variables the old-fashioned way, and using the so-called "recommended" PHP configs for development or production, $_ENV will be empty and all the environment variables will be in $_SERVER

tldr: PHP is fun

Actually, we use vlucas/phpdotenv example.settings.local.php and env

Great idea :)

Core has some protections in place to avoid these sorts of sites leaking critical information after https://www.drupal.org/sa-core-2023-004 →

But if we're making this a core feature we'll be encouraging people to do even more, a good thing imho. But since this means it expands the places and levels of experience using the feature, it does have me wondering if when adding the core feature, we should also harden it to it warns people if they have their system configured in a way that could be unsafe. Maybe a requirement check that detects environment variables would be available through core's phpinfo and this DB auto-configuration is being used it throws a warning like we do with settings directory permissions or allow_updates.

I mean even if we didn't, folks are using environment variables regardless. So the problem already exists that Drupal is providing this page. IMO it's more of a problem we don't really support APP_ENV and kill phpinfo for prod

$kernel = new DrupalKernel('prod', $autoloader);

to

$kernel = new DrupalKernel($_ENV['DRUPAL_ENV'] ?? 'prod', $autoloader);

And if prod, phpinfo page is turned off.

If there is a precedent now for guarding, I guess we can try. But it seems like a reach and bloat of scope/responsibilities.

I actually agree and disable phpinfo in production systems entirely. I think in previous discussions some people wanted phpinfo to be available in production so people could check modules and other php configuration which is what lead to the current approach and why I suggested the more target hardening. But also not trying to blow the scope, just wanted to be mindful of an impact.

@neclimdul good callouts, though. I opened ✨ Allow specifying the environment for the kernel using an environment variables Active to discuss the environment so we can do things like this.

If we gonna inject something via environment it will need policy/guards to prevent sensible information leaking, a-la contrib project/dotenv →

Personally I prefer to have kind of wrapper ✨ Use symfony/runtime for less bespoke bootstrap/compatibility with varied runtime environments Active

But this isn't about parsing or loading .env files.

This approach is used by multiple hosting providers. They all have their own ways of injecting variables and they all use their own variable names.

The proposed change would help to align the implementation across providers.

I was trying to figure out why this wasn't working for me, and after some debugging, discovered that the $_ENV array was empty.

This being because the php.ini that comes with the PHP Docker Images set variables_order without E [1], meaning $_ENV is empty.

The getenv() function is not impacted, so I'd recommend we use that instead.

[1] https://www.php.net/manual/en/ini.core.php#ini.variables-order

Here, you can see what the two default php.ini files set variables_order to:

/usr/local/etc/php $ cat php.ini-development  | grep variables_order
; variables_order
variables_order = "GPCS"
; are specified in the same manner as the variables_order directive,
; in the variables_order directive. It does not mean it will leave the super
/usr/local/etc/php $ cat php.ini-production  | grep variables_order
; variables_order
variables_order = "GPCS"
; are specified in the same manner as the variables_order directive,
; in the variables_order directive. It does not mean it will leave the super

To illustrate this, here you can see an interactive PHP terminal, showing $_ENV is empty, but getenv("DRUPAL_CONFIG_SYNC_DIR") returns a value.

/usr/local/etc/php $ php -a
Interactive shell

php > var_dump($_ENV);
array(0) {
}
php > var_dump(getenv("DRUPAL_CONFIG_SYNC_DIR"));
string(14) "../config/sync"

I'd recommend $_SERVER as I mentioned on #11 - it will be filled by default in most scenarios (using real environment variables with the recommended php configs, or using one of the dotenv libraries to set environment variables in default thread safe mode).

I do a decent amount of work with Laravel as well as Drupal, and I've always enjoyed the developer experience of their environment setup. They have a global function called `env()` that wraps `getenv()` and provides a second argument to provide a default value when no environment variable exists, as in `env('SOMEVARIABLE', 'default_value')`. I see @mfb's warnings that $_SERVER is more reliable. That _does_ seem to be the way most hosts set things, but in the wild, I've always found `getenv()` to be more reliable across the board. It seems to have the most chance of picking up the variables regardless of how they are set. That's anecdotal for sure, but just my two cents. However the feature gets implemented, better env variable support would be great!

@dustinleblanc re: laravel env(), getenv()/putenv() can be disabled for thread safety/security: https://github.com/laravel/framework/pull/28740 The library it uses under the hood to deal with environment variables is phpdotenv, which by default doesn't use putenv/getenv but does have a putenv/getenv adapter that can be enabled.