akeneo / pim-community-dev

[Community Development Repository] The open source Product Information Management (PIM)
http://www.akeneo.com
Other
954 stars 514 forks source link

Please remove redundant problematic variables in docker-compose.yml #14305

Open adriancuadrado opened 3 years ago

adriancuadrado commented 3 years ago

:bug: I'm reporting a Bug :bug:

It's not really a bug, but I think it would be much better if you fix it: When I edit the APP_ENV and XDEBUG_MODE environment variables in my .env.local and I create a new extra variable as follows:

APP_ENV=dev
XDEBUG_MODE=debug
MY_ENV_VARIABLE=a_value

And then run a Symfony command that I create to print the values of those variables...

docker-compose run -u www-data --rm php php bin/console app:my-command

I get the following values:

APP_ENV=prod
XDEBUG_MODE=off
MY_ENV_VARIABLE=a_value

The APP_ENV and XDEBUG_MODE variables are ignored, but not the MY_ENV_VARIABLE. This is so because the exported environment variables have precedence over those defined in .env and in .env.local. If you take a look at the docker-compose.yml file, you will see this:

services:
  php:
    image: 'akeneo/pim-php-dev:5.0'
    environment:
      APP_ENV: '${APP_ENV:-prod}'
      COMPOSER_HOME: '/var/www/.composer'
      PHP_IDE_CONFIG: 'serverName=pim-docker-cli'
      XDEBUG_MODE: '${XDEBUG_MODE:-off}'
      XDEBUG_CONFIG: 'client_host=172.17.0.1'
      BLACKFIRE_CLIENT_ID: '${BLACKFIRE_CLIENT_ID:-client_id}'
      BLACKFIRE_CLIENT_TOKEN: '${BLACKFIRE_CLIENT_TOKEN:-client_token}'

docker-compose only reads .env and not .env.local and then executes the command passing the environment defined under environment:. Once the script is executed, because APP_ENV=prod and XDEBUG_MODE=off are defined in the environment that docker-compose provided to the command, they end up overriding the .env.local values (because those variables, while executing the command, are not read from .env or .env.local, but from the environment).

My suggestion: Please remove the APP_ENV and XDEBUG_MODE environment variables from docker-compose. They are already defined in .env and is redundant and they will only cause problems like the one I mentioned above.

adriancuadrado commented 3 years ago

I just realized that maybe it wouldn't be worth it to remove the XDEBUG_MODE because that variable should be present before any php code gets executed to activate the debug mode. If you set it in the .env variable it wouldn't make sense because that won't tell php to use XDEBUG in debug mode. However, everything holds true for APP_ENV.

Thijzer commented 3 years ago

I can concur,

For me it only happens on Akeneo 5.0 during a migration.

I had issues trying to set APP_ENV to dev an nothing happens, to problem sadly is one of many layers.

First problem, much as you described

docker-compose sets APP_ENV at start but you can't switch it anymore during, which is a shame and unneeded during development. APP_ENV should be a more flexible env variable to test more environments without the need to restart your docker container.

Also docker only reads .env during up while symfony reads .env and .env.local or .env.local.php as overrides, these are ignored by docker. Also the SetEnv is used here https://github.com/akeneo/pim-community-dev/blob/master/docker/akeneo.conf#L12 which is a best practice for production machines, but maybe not required and redundant for development containers.

Second related problem

composer update overrides your .env The file is created and overridden by Symfony, not by flex, flex is not found and the composer.json is standard. In a Symfony setup this is fine as the .env file has all the default values that keeps your app from breaking, .env.local has your overrides. So docker uses and override .env file where the default is prod.

Thijzer commented 3 years ago

I would like to add the following solution.

/docker/akeneo.conf

# remove
SetEnv APP_ENV ${APP_ENV}

This hard sets the APP_ENV as a variable in the Apache container which is not required for docker development.

Just, Remove the entry, restart your container, and use .env.local to overload your global variables.

/.env

# DON'T EDIT THIS FILE !
# IT COULD BE OVERWRITTEN DURING THE LAST UPDATE
# You must use .env.local to overload ENV VARs

APP_ENV=prod
APP_DEBUG=0
....

/.env.local

APP_ENV=dev
APP_DEBUG=1