search

magento / magento2

Prior to making any Submission(s), you must sign an Adobe Contributor License Agreement, available here at: https://opensource.adobe.com/cla.html. All Submissions you make to Adobe Inc. and its affiliates, assigns and subsidiaries (collectively “Adobe”) are subject to the terms of the Adobe Contributor License Agreement.
http://www.magento.com
Open Software License 3.0
11.47k stars 9.29k forks source link

Using MAGENTO_DC_{path} environment variables is only partly implemented #39026

Closed LeanderFS closed 6 days ago

LeanderFS commented 1 month ago

Summary

The comments inside vendor/magento/framework/App/DeploymentConfig.php seem to suggest that it should be possible to set deployment configuration using environment variables only. After testing this it seems that this only works partly, as the environment variable is only set in the $this->flatData array and not in the $this->data array. Resulting in conflicting configuration and key collisions.

Examples

Snippet of the method responsible for loading the deployment configuration seems to suggest deployment configuration can be set using environment variables only. Note that the there are 2 arrays being created, $this->data and $this->flatData. The value of the environment variable is correctly added in $this->flatData, but is absent in $this->data. Both arrays are being used to load deployment configuration.

\Magento\Framework\App\DeploymentConfig::load

private function load()
{
    if (empty($this->data)) {
        $this->data = array_replace(
            $this->reader->load(),
            $this->overrideData ?? [],
            $this->getEnvOverride()
        );
        // flatten data for config retrieval using get()
        $this->flatData = $this->flattenParams($this->data);

        // allow reading values from env variables by convention
        // MAGENTO_DC_{path}, like db/connection/default/host =>
        // can be overwritten by MAGENTO_DC_DB__CONNECTION__DEFAULT__HOST
        foreach (getenv() as $key => $value) {
            if (false !== \strpos($key, self::MAGENTO_ENV_PREFIX)
                && $key !== self::OVERRIDE_KEY
            ) {
                // convert MAGENTO_DC_DB__CONNECTION__DEFAULT__HOST into db/connection/default/host
                $flatKey = strtolower(str_replace([self::MAGENTO_ENV_PREFIX, '__'], ['', '/'], $key));
                $this->flatData[$flatKey] = $value;
            }
        }
    }
}

\Magento\Framework\App\DeploymentConfig::getConfigData returns deployment configuration from the $this->data array.

public function getConfigData($key = null)
{
    $this->load();

    if ($key !== null && !isset($this->data[$key])) {
        return null;
    }

    return $this->data[$key] ?? $this->data;
}

\Magento\Framework\App\DeploymentConfig::get returns deployment configuration from the $this->flatData array.

public function get($key = null, $defaultValue = null)
{
    $this->load();
    if ($key === null) {
        return $this->flatData;
    }

    if (array_key_exists($key, $this->flatData) && $this->flatData[$key] === null) {
        return '';
    }

    return $this->flatData[$key] ?? $defaultValue;
}

Proposed solution

If this method of setting deployment configuration is supported (as suggested by the comments), make sure the environment variable values are being propagated to both the $this->data and the $this->flatData array. On a related note, documenting this somewhere in the devDocs would be an amazing addition and would make the lives of a lot of devops engineers a lot easier working with Magento 2.

Release note

No response

Triage and priority

m2-assistant[bot] commented 1 month ago

Hi @LeanderFS. Thank you for your report. To speed up processing of this issue, make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:

engcom-Hotel commented 1 month ago

Hello @LeanderFS,

Thanks for the report and collaboration!

We have gone through the issue description and also the file mentioned in the description https://github.com/magento/magento2/blob/6f4805f82bb7511f72935daa493d48ebda3d9039/lib/internal/Magento/Framework/App/DeploymentConfig.php#L7

But we are not able to get the comment which says the below:

seem to suggest that it should be possible to set deployment configuration using environment variables only.

Can you please elaborate more on this?

Thanks

m2-assistant[bot] commented 6 days ago

Hi @engcom-Hotel. Thank you for working on this issue. In order to make sure that issue has enough information and ready for development, please read and check the following instruction: :point_down:

engcom-Hotel commented 6 days ago

Dear @LeanderFS,

We've observed that there haven't been any updates on this issue for quite some time. Therefore, we assume it's been resolved and will close it. Feel free to open a new ticket or reopen this one if you need more help.

Regards