8000 [DI], [Config] - Environment variables can not be used with configuration nodes of concrete types · Issue #25868 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DI], [Config] - Environment variables can not be used with configuration nodes of concrete types #25868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
edefimov opened this issue Jan 20, 2018 · 6 comments

Comments

@edefimov
Copy link
Contributor
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 4.0.3

Suppose we have defined such configuration in Configuration.php:

public function getConfigTreeBuilder()
{
    $tree = new TreeBuilder();
    $root = $tree->root('my_bundle');
    $root
        ->children()
            ->floatNode('float_value')->end()
        ->end();
    return $tree;
}

Next we want to use environment variable for this configuration, as described here:

my_bundle:
  float_value: "%env(float:MY_VALUE)%"

After that we add MY_VALUE into .env file and try to run

php bin/console cache:clear

The execution result will be an error:

Executing script cache:clear [KO]
 [KO]
Script cache:clear returned with error code 1
!!  
!!  In FloatNode.php line 34:
!!                                                                                 
!!    Invalid type for path "my_bundle.float_value"  
!!    Expected float, but got string.                                   
!!                                                                              
!!  

The command should be executed successfully.

Repository with code to reproduce this situation. The steps:

composer install
bin/console list
@gregurco
Copy link
Contributor

To add more details: floatNode will receive for validation the placeholder of parameter created in https://github.com/symfony/dependency-injection/blob/master/ParameterBag/EnvPlaceholderParameterBag.php#L51 but not the value. Evident that placeholder is string, but not float, int or whatever we expect.

A little bit more detailed description: 8p/EightPointsGuzzleBundle#184 (comment)

@chalasr
Copy link
Member
chalasr commented Jan 21, 2018

Fix in progress in #23888?

@nesl247
Copy link
nesl247 commented Jan 22, 2018

This is actually a critical issue blocking replacement of parameters with environment variables for us, as well as I'm guessing many others (working to use Docker right now).

Hopefully the fix isn't stuck for too long as the last comment suggested.

@sknuell
Copy link
sknuell commented Feb 1, 2018

Kind of critical for us too, since some third-party bundles require types, e.g. integers for port numbers: %env(int:PHP_REDIS_PORT)%.

Of course we could fork the bundle and adapt, but would be great to have this working out-of-the-box.

@ro0NL
Copy link
Contributor
ro0NL commented Feb 26, 2018

Anyone willing to try an updated version of #23888 :) I think it's working.

@edefimov
Copy link
Contributor Author

@ro0NL , It seems working! Here is the test.

nicolas-grekas added a commit that referenced this issue Mar 27, 2018
This PR was squashed before being merged into the 4.1-dev branch (closes #23888).

Discussion
----------

[DI] Validate env vars in config

| Q             | A
| ------------- | ---
| Branch?       | 4.1/master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #22151, #25868
| License       | MIT
| Doc PR        | symfony/symfony-docs#8382

This PR registers the env placeholders in `Config\BaseNode` with its default value or an empty string. It doesnt request real env vars during compilation,

What it does is if a config value exactly matches a env placeholder, we validate/normalize the default value/empty string but we keep returning the env placeholder as usual. If a placeholder occurs in the middle of a string it also proceeds as usual.

The latter to me is OK as you need to expect any string value during runtime anyway,  including the empty string.

Commits
-------

2c74fbc [DI] Validate env vars in config
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants
0