Environment variables broken > rc-21

[prineshaz]

This is an issue i assumed was part of drupal-console repo, but its due to the way environment vars are handled by drupal-console-core.

[console] Environment variables in chain file not working

Problem

Chaining a file with environment variables (via custom install yml file) seems to broken in version > 1.0.0-rc21 . We have a jenkins job which pulls in latest drupal/console & installs drupal 8 via a custom install yml chain file. This implements the env variables approach defined here.

My Steps to reproduce:

1. Export env variables

export DATABASE_USER=root
export DATABASE_PASSWORD=root
export DATABASE_NAME=drupal_8
export DATABASE_PORT=3306
export DATABASE_HOST=localhost

2. Run install Using this custom site-install-new.yml file with code below:

commands:
# Install Drupal
  - command: site:install
    options:
      langcode: en
      db-type: mysql
      db-host: '${{DATABASE_HOST}}'
      db-name: '${{DATABASE_NAME}}'
      db-user: '${{DATABASE_USER}}'
      db-pass: '${{DATABASE_PASSWORD}}'
      db-port: '${{DATABASE_PORT}}'
      site-name: 'Drupal 8'
      site-mail: admin@example.org # default email
      account-name: root # default account
      account-mail: admin@example.org # default email
      account-pass: root # default pass
    arguments:
      profile: standard

3. Execute

./vendor/bin/drupal chain --file=../console/chain/site-install-new.yml

I get the following error:

[ERROR] commands.chain.messages.missing-environment-placeholders-default                                               

 commands.chain.messages.set-environment-placeholders-custom

 export 0=0_VALUE                                                                                                       
                                                                                                                        
 export 1=1_VALUE                                                                                                       
                                                                                                                        
 export 2=2_VALUE                                                                                                       
                                                                                                                        
 export 3=3_VALUE                                                                                                       
                                                                                                                        
 export 4=4_VALUE

Details to include:

• Drupal 8.3
• Console version: 1.0.0-rc21
• Console Launcher version : Not using launcher

[prineshaz]

Issue seems to be down to the way the Drupal\Console\Core\Command\Chain\ChainCommand::execute() is looping through $environmentPlaceHolders array on line 279 in drupal-console-core .

The foreach loop on line 279 was updated to this commit which tries to loop through an assoc array, even though the returned array is a regular indexed array.

The error output i posted in my initial post highlights that, instead of returning a error message of environment placeholders, the error message displays the index's.

commands.chain.messages.set-environment-placeholders-custom

 export 0=0_VALUE                                                                                                       
                                                                                                                        
 export 1=1_VALUE                                                                                                       
                                                                                                                        
 export 2=2_VALUE                                                                                                       
                                                                                                                        
 export 3=3_VALUE                                                                                                       
                                                                                                                        
 export 4=4_VALUE

Problem below, conditionif (!getenv($envPlaceHolder)) { rightfully fails.

foreach ($environmentPlaceHolders as $envPlaceHolder => $envPlaceHolderValue) {
  if (!getenv($envPlaceHolder)) {
    $missingEnvironmentPlaceHolders[$envPlaceHolder] = sprintf(
      'export %s=%s_VALUE',
      $envPlaceHolder,
      strtoupper($envPlaceHolder)
    );
    continue;
  }
  $envPlaceHolderMap[$envPlaceHolder] = getenv($envPlaceHolder);
}

To fix this, revert back to looping thought indexed array:

foreach ($environmentPlaceHolders as $envPlaceHolder ) {
  if (!getenv($envPlaceHolder)) {
    $missingEnvironmentPlaceHolders[$envPlaceHolder] = sprintf(
      'export %s=%s_VALUE',
      $envPlaceHolder,
      strtoupper($envPlaceHolder)
    );
    continue;
  }
  $envPlaceHolderMap[$envPlaceHolder] = getenv($envPlaceHolder);
}

By modifying it with the above change it seems to be working now.

[jmolivas]

@prineshaz Are you planning to send a PR to have this fixed?

[prineshaz]

Hi @jmolivas , id love to! I didn't know if i was allowed to. I shall raise a pull request shortly. Thanks!

[prineshaz]

Hi @jmolivas , apologies if I've made a mess of the PR comments, I realised now i should've added my comments in the PR comment section.

Thank you

[jmolivas]

@prineshaz The PR #236 was merged and thanks for your contribution. This will be included in the next release.

[prineshaz]

Great thanks!

[oukjweather]

Issue still present in version 1.0.2.

[prineshaz]

Hi @oukjweather i believe version 1.0.2 was cut before the above was merged. Its in master.

https://github.com/hechoendrupal/drupal-console-core/blame/master/src/Command/Chain/ChainCommand.php#L279

Cheers

[jmolivas]

@oukjweather @prineshaz Exactly the code was committed, but no release tagged yet. ETA this week on Wednesday.

[prineshaz]

Thanks for the update @jmolivas