ddev / ddev-platformsh

Add integration with Platform.sh hosting service
Apache License 2.0
9 stars 10 forks source link

Environment variables must be set much earlier for php-fpm #89

Closed rfay closed 1 year ago

rfay commented 1 year ago

@gilzow and I were working on ddev.com site today, trying to use this add-on with it, and it turned out that the environment variables it needs to operate didn't get set early enough to be used by php-fpm.

What we do currently is create https://github.com/drud/ddev-platformsh/blob/main/homeadditions/.bashrc.d/platformsh-environment.sh, which loads environment variables in bash based on .global/environment and .environment.

Some things we could do instead:

  1. Create a .ddev/.env with these same environment variables (but that could overwrite user info) or
  2. Create a docker-compose.platformsh.yaml that set them with an environment section.

Both of these are pretty easy.

rfay commented 1 year ago

I didn't realize that .environment was a free-form file, like https://github.com/drud/ddevdotcom/blob/main/.environment

export DB_NAME=$(echo $PLATFORM_RELATIONSHIPS | base64 --decode | jq -r ".database[0].path")
export DB_HOST=$(echo $PLATFORM_RELATIONSHIPS | base64 --decode | jq -r ".database[0].host")
export DB_PORT=$(echo $PLATFORM_RELATIONSHIPS | base64 --decode | jq -r ".database[0].port")
export DB_USER=$(echo $PLATFORM_RELATIONSHIPS | base64 --decode | jq -r ".database[0].username")
export DB_PASSWORD=$(echo $PLATFORM_RELATIONSHIPS | base64 --decode | jq -r ".database[0].password")

export WP_HOME=$(echo $PLATFORM_ROUTES | base64 --decode | jq -r 'to_entries[] | select(.value.primary == true) | .key')
export WP_SITEURL="${WP_HOME}wp"
export WP_DEBUG_LOG=/var/log/app.log
export WP_ENV="${PLATFORM_ENVIRONMENT_TYPE}"
export AUTH_KEY=$PLATFORM_PROJECT_ENTROPY
export SECURE_AUTH_KEY=$PLATFORM_PROJECT_ENTROPY
export LOGGED_IN_KEY=$PLATFORM_PROJECT_ENTROPY
export NONCE_KEY=$PLATFORM_PROJECT_ENTROPY
export AUTH_SALT=$PLATFORM_PROJECT_ENTROPY
export SECURE_AUTH_SALT=$PLATFORM_PROJECT_ENTROPY
export LOGGED_IN_SALT=$PLATFORM_PROJECT_ENTROPY
export NONCE_SALT=$PLATFORM_PROJECT_ENTROPY
rfay commented 1 year ago

We already add a number of environment variables to config.platformsh.yaml, but they're static environment variables, not free-form bash stuff.

gilzow commented 1 year ago

And unfortunately, we have numerous templates that rely on the .environment file to translate PLATFORM_* environmental variables into environmental variables needed for the template to run correctly:

The Drupal templates also use an .environment file, but only to add the composer bin directory to PATH

rfay commented 1 year ago

The Drupal templates also use an .environment file, but only to add the composer bin directory to PATH

And of course composer is already provided by DDEV and is already in the $PATH

gilzow commented 1 year ago

Looks like I missed a few in my initial scan

rfay commented 1 year ago

This problem also points out the poverty of the tests. Currently I think all the tests just do a ddev start and call it good if that works out. But the reality is they need to have real databases and see if they can get real content. And that means slower tests and it means we need to figure out a way to have minimal content available.

Spun off into

rfay commented 1 year ago

A way to do this that would be general (but likely require changes to DDEV):

We might also be able to add config to supervisord that would accomplish this.