Making possible to define configuration by environment

[jalogut]

Description

Add new file config_env.php into the config pool:

• config_env.php can be in VCS and add specific configurations by environment. This file updates values in config.php but env.php has still the highest priority

Add CONFIG_ENV_MODE in env.php to be able to specify the system environment:

• CONFIG_ENV_MODE can be set into the env.php to specify the config environment mode to load the config settings.

Fixed Issues

The new deployment pipeline added on 2.2 makes possible to propagate configuration to a live server. However, that introduces new issues as it is not possible to specify such configuration by environment. See following example:

Situation

• On config.php we specify dev/js/merge_files = 1 and dev/css/merge_css_files = 1
• We need that on config.php because is the only configuration file under VCS and same configuration is needed on the build system.
• That works great on our build and PROD environments as they need same configuration

Problem

• If I install the project locally, I do not want the files to be merged.
• As this configuration is now present on config.php the only way to modify that on my local is by manually adding the corresponding configuration on env.php.
• Imho it is a big limitation that the only way to do that is by manually editing the env.php (or executing command config:set --lock path value)
  • env.php is not in VCS, so you need to manually edit the env.php every time you install a project locally
  • Working on a team, if someone adds changes on config.php for production and build server. He has no way to add the corresponding for local env to ensure that those changes will not break the local installations of his colleagues. For example enabling varnish cache on config.php. Next time the other developers checkout the latest version of the project, it will break their local installation as they do not have Varnish.

Alternatives Solutions that do not work
Discussing that in Slack, Anton Kril suggested to use condition statements inside config.php that return different configurations depending on env variables. Although that works, these statements are gone as soon as you execute any command that edits the config.php. That is the case for app:config:dump or module:disable/enable. Because of that, it is not possible to properly maintain those condition statements inside config.php.

Manual testing scenarios

These changes allow keeping configuration by environment like that:

Possible to define in env.php type of environment for config settings

<?php
# etc/env.php
return array(
//...  
'CONFIG_ENV_MODE' => 'LOCAL',
//...
);

Use environment type inside config_env.php to overwrite default settings in config.php

<?php
# etc/config_env.php
$config = [];

if (strtoupper(getenv('CONFIG_ENV_MODE')) == 'LOCAL') {
    $configLocal = array(
        'system' =>
            array(
                'default' =>
                    array(
                        'dev' =>
                            array(
                                'js' =>
                                    array(
                                        'merge_files' => '0',
                                    ),
                                'css' =>
                                    array(
                                        'merge_css_files' => '0',
                                    ),
                                'static' =>
                                    array(
                                        'sign' => '0',
                                    ),
                            ),
                        # Disable Varnish
                        'system' =>
                            array(
                                'full_page_cache' =>
                                    array(
                                        'caching_application' => '1',
                                    ),
                            ),
                    ),
            ),
    );
    $config = array_replace_recursive($config, $configLocal);
}

if (in_array(strtoupper(getenv('CONFIG_ENV_MODE')), ['LOCAL', 'JENKINS'])) {
    $configLocal = array(
        'modules' =>
            array (
                'TddWizard_Fixtures' => 1,
            ),
    );
    $config = array_replace_recursive($config, $configLocal);
}

return $config;

As you can see in this example, thanks to that, we can assure that all developers will have merge_files = 0 and TddWizard_Fixtures module enabled on their local environments. That modifies the config.php which is meant for PROD and BUILD systems but it is shared among all environments.

<?php
# etc/config_php
return array(
    'modules' =>
        array(
          //....
           'TddWizard_Fixtures' => 0,
          //...
         ),
     'system' =>
            array(
                'default' =>
                    array(
                        'dev' =>
                            array(
                                'js' =>
                                    array(
                                        'merge_files' => '1',
                                    ),
                                'css' =>
                                    array(
                                        'merge_css_files' => '1',
                                    ),
                                'static' =>
                                    array(
                                        'sign' => '1',
                                    ),
                            ),
                    ),
            ),
);

Contribution checklist

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[mzeis]

@jalogut Thank you for this nice developer feature contribution! Please can you fix the failing unit test? Thanks!

[jalogut]

@mzeis thanks for noticing. All green now.

[fooman]

@jalogut thanks for this pull request - just cross referencing this pull request here
#12178 as it is proposing changes in the same area.

[andrewhowdencom]

After much discussion in the ENGCOM, we concluded this would also allow addressing some of the issues outlined in #4955 -- it would provide an "untouchable" configuration pool, from which things can be extended.

Because it doesn't override env.php, configuration would need to be omitted form that to be merged in with this file.

Edited: With more thinking / discussion, I think the ideal naming for this is env.php, and rename the other file runtime.php. This more accurately reflects their proposed responsibilities --

runtime.php -- application managed configuration, and
env.php -- externally managed configuration.

I understand this is a breaking change, and I am not suggesting this make it into this PR. However, I would consider it for refactoring between 2.2 and 2.3

[paveq]

Hi,

Vaimo has implemented internally similar feature that updates core_config_data table contents when setup:upgrade is called (from recurring setup). One of the issue in this proposal is that config_env.php is a PHP array, which is not easy to maintain or read. Perhaps there are some room to combine these efforts?

Our YAML format as an example:

"*": # Applies to all environments, including local and jenkins build
  - path: [ dev/js/merge_files, dev/js/minify_files, dev/css/merge_css_files, dev/css/minify_files ]
    value: 1
    scope: default
    scope_id: 0

live: # Applies to live environment
  - path: dev/js/minify_files
    value: 1
    # When using scope default and scope_id  0, they can be omitted

qa: 
  - path: [ web/unsecure/base_url, web/secure/base_url ]
    value: https://qa-base-url.example.com/

LOCAL: # Applies to local development installations only
  - path: [ dev/js/merge_files, dev/js/minify_files, dev/css/merge_css_files, dev/css/minify_files ]
    value: 0

  - path: klevu_search/general/enabled
    value: 0

  - path: integration/general/api_key
    delete: true # will cause config option with this path to be removed (only from default scope!)

[jalogut]

Hi @pavec,

We also use .yaml to update core_config_data table in Magento 2.1.x versions. In Magento 2.2 things have improved and data can be maintained into config.php. As this file has a higher priority than DB, a file like config_env.php is needed to define configuration by environment for those settings existing in config.php

[shiftedreality]

Hi @jalogut

There is another way to set these values per environment - using environment variables:
http://devdocs.magento.com/guides/v2.2/config-guide/prod/config-reference-var-name.html

In your case, you need to pass next variables into your $_ENV:

CONFIG__DEFAULT__DEV__JS__MERGE__FILES to 0
CONFIG__DEFAULT__DEV__CSS__MERGE__CSS__FILES to 0

These are absolutely top priority configs which will overwrite values in env.php and config.php.

[jalogut]

Hi @shiftedreality ,

I did not know that, thank your for pointing it. That would be exactly what I was looking for 😀

[antonkril]

@jalogut, I thought you wanted to keep your env-specific values in git? Anyway, good to see that built-in solution works for you.

[jalogut]

@antonkril, yes, I still want that and I’d very happy if the PR gets merged. However, if it doesn’t, it is nice to know that there is an alternative.

Only drowback with the $_ENV variables is to edit the index.php and also php.ini configuration. Env-specific values can be in git and insert them in index.php like:

if(file_exists('env_config.php')) {
    include 'env_config.php';
}

Including that by default in index.php could be also a good PR, what do you think?

[shiftedreality]

@jalogut I would like to add more thoughts on this item:

So the reason, why do we have 2 config files now - is separating environment and shared configuration. Your case is environment configuration, which should be in env.php, but you would like to share it with a team.

Proper solution
This case would be good to cover with a smart writer/reader, which will not overwrite your custom code. But this solution has 2 main cons:

1. It's not trivial to implement
2. It won't work, in the case if config files format will change in future

Environment solution
For cases, when you have a small team or does not really need to share these configs in git, it would be enough to use $_ENV configs at all.

Current solution
Current PR looks good for me in terms of development configuration. The only points I would add:

• Naming should be changed to something more specific, like dev.php or dev_config.php
• It MUST be in .gitignore, cause it still environment configuration and may be shared only in teams, and even there very carefully.
• It should be tested well because there is app:config:import mechanism which checks configs for changes and imports them
• Verify the priority of configs, in my understanding dev.php must have higher priority than env.php

CONFIG_ENV
I would not add this mechanism to the code for now.
You can just add

#app/etc/dev.php
$envConfig = file_exists(__DIR__ . '/env.php') ? require(__DIR__ . '/env.php') : [];
$envMode = isset($envConfig['CONFIG_ENV_MODE']) ? $envConfig['CONFIG_ENV_MODE'] : [];

[andrewhowdencom]

Hola @shiftedreality;

For me at least, the problem with both pools (env.php and config.php) is they're both application managed -- one cannot stub third party credential management, such as vault etc; the file is overwritten periodically. This PR provides a "sacred" file, which no one touches.

Stubbing it in the environment is usually possible (I also didn't know about this feature -- cool!) but perhaps more difficult? I'm unsure.

[jalogut]

Hi, any updates on this PR? Do you think that would be merged?

[sidolov]

Hi @jalogut , generally solution looks good for me, but I have several concerns regarding you implementation:

1. Making changes in the global scope: putenv(self::CONFIG_ENV_MODE . "=" . $configEnvMode);. Btw, this implementation depends on server configuration: safe_mode_allowed_env_vars and safe_mode_protected_env_vars settings
2. File name config_env.php little bit confusing, we already have config.php and env.php name from your implementation looks like something merged both default configs.
3. Not obvious files load order

As an alternative solution, please, consider to refactor logic by using two folders:

• conf.d contains any number of configuration files that should be loaded after config.php file
• env.d contains any number of configuration files that should be loaded after env.php file

This solution similar to Linux files configurations and will be more familiar for people.

[andrewhowdencom]

conf.d

This is a lovely API.

[sidolov]

Hi @jalogut , are you going to continue progress with PR?

[jalogut]

Hi @sidolov unfortunately I do not have much time atm. So I would be glad if someone takes over.

[jalogut]

Just noticed that it is possible to define environment specific config settings in di.xml files:

• https://devdocs.magento.com/guides/v2.2/extension-dev-guide/build/di-xml-file.html#ext-di-sens

An alternative solution to this PR could be to have modules with different settings in their di.xml files. For example Vendor_LocalSettings and Vendor_DevSettings. Then with a custom script, just enable or disable theses modules depending on the environment.

I still like more the solution of this PR but just wanted to leave that here as documentation of other possible options.

[sidolov]

Hi @jalogut , I'm closing this PR due to inactivity.
Please, reopen it if you wish to continue!
Thank you for the collaboration!