-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Environment Variables lack type - break built-in Symfony configuration #22151
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
Comments
An environment variables is a string by design. |
That is correct. However, Symfony configuration can be type aware. Therefore, there is a mismatch between what Symfony expects as a configuration parameter, and what Symfony provides as a configuration parameter through |
Not really. Environment variables fits specific use cases. The example you give is not a valid use case IMHO. Why would you want to change the cache value via an env var? Even if it was possible, that wouldn't work. As changing an evn var value does not triggers a container recompilation. |
Symfony's stated methodology appears to disagree with you, as per this blog post which references environment variables as part of the twelve-factor app methodology. http://symfony.com/blog/new-in-symfony-3-2-runtime-environment-variables
The concept here is that I want to be able to switch debugging on and off for any given environment easily and quickly. However, my default for "prod" is going to be different from my default for "dev", where caching can be a nuisance. I do not want to rewrite my parameters.yml between environments - that is a big part of the point, and it's a big part of the point of why As I write this, I realize that another solution is to add a parameters_dev.yml that holds cache: false. This is a good temporary measure. If you don't like my example, though, please refer to literally any other place in Symfony where a boolean value is expected. Passing via If you truly believe that there is no situation where an environment variable can or should be a boolean in Symfony's eyes, I ask you to reconsider carefully... This is an arbitrary limitation. |
Keep in mind that using env vars does not replace parameters. An env var is something that can change without the container being recompiled. Your example would not work even if booleans were supported. |
I am not sure I am following you. I have been easily able to make this work in Symfony by creating a PHP configuration and validating the boolean manually myself. Laravel uses PHPDotEnv with their I can certainly understand why there would be exceptions, or even valid reasons not to do this, but I do think it is a bug, especially for those of us who have used other frameworks. Symfony is incredible, but I do not believe that this should be the standard behavior. |
This is key; parameters:
foo: %env(HOME)%
services:
foobar:
class: stdClass
properties:
a: %foo%
b: %env(HOME)% Produces; <parameter key="foo">%env(HOME)%</parameter>
<service id="foobar" class="stdClass">
<property name="a">%env(HOME)%</property>
<property name="b">%env(HOME)%</property>
</service> $this->services['foobar'] = $instance = new \stdClass();
$instance->a = $this->getEnv('HOME');
$instance->b = $this->getEnv('HOME'); Parameters are resolved at compile time, env parameters at runtime. This is good! https://symfony.com/blog/new-in-symfony-3-2-runtime-environment-variables also states that
This is what's happening here. However, i do understand the confusion; as it highly looks like normal parameters. But your solution of moving things to Not sure SF should do anything here, other then improving docs. Maybe it's needs a different syntax to make it less confusing with parameters. |
@fabpot what do you think of the idea to support built-in type casts for env's?
Looks like it fits a need; note this is not about |
@ro0NL I really do get the difference between compile-time and runtime. I am probably doing a poor job expressing this issue and my proposed fix. For me, the issue that leads to unnecessary confusion is this:
Configuration for FooBundle:
This results in an error. I am not suggesting that you force environment variables to be type coerced throughout Symfony - sorry if that was not clear. If you need that in your PHP files, you can simply use Proposed new behavior:
In my–possibly flawed–view, any time you call
I hope this is clearer. Also: Note that if you run
This is built into PHP by design, and will prevent the breaking of config files that validate for a boolean value. Maybe I'm missing something (probably), but it does feel like validating for boolean should validate the intention and PHP standards, rather than the actual type, given these mismatches between environment variables and the way YML itself determines type. Requiring your cache to be named "web/false" seems like an extreme edge case. Additionally, if someone really wanted to do that, depending on how this was implemented it could wind up being doable like this:
I'm still relatively new to the Symfony universe, and I do truly find it a joy to use. If I'm suggesting something that flies in the face of the Symfony philosophy, I understand and humbly apologize. However, I want to make sure my point is at least understood as it was intended. I do think simply updating the documentation to describe environment file handling would be a big help. I am sure there are other solutions as well, such as, in a Capistrano deploy for example, moving parameters.yml to the Basically, it can be a bit of a struggle when you want more dynamic and fluid environment variables, and you don't want to make your application aware of and have to handle |
I get your point, and i think it's a serious DX issue (at least the confusion with parameters). So here to help :) First thing;
So what it evaluates to really depends on the type of the referenced value. And for env's this is always a string. If i understand correctly you're proposing to automagically filter a parameter var, if it references a env var, right? Not sure about that, why exactly should we assume it's a boolean? I think the conversion needs to be explicit, hence my proposal of doing it thru syntax;
This way BOOL=false could actually be supported using This should also makes things compatible with PHP7 typehints for instance. |
I think that makes a lot of sense! I am all for that solution. |
Reclassified as a feature request asking for typed env vars, which are not supported yet as explained by @fabpot |
In case types are finally adopted for env vars, could we use Python's dictionaries syntax instead of the proposed syntax? # Current Proposal
Strings: %env(...)%
Booleans: %env:bool(...)%
Integers: %env:int(...)%
Floats: %env:float(...)%
# Proposal based on Python's syntax
Strings: %env(...)s% (%env(...)% works too)
Booleans: %env(...)b%
Integers: %env(...)i%
Floats: %env(...)f% |
We should move toward a solution because right now we can't use dynamic config based on runtime for simple configuration (boolean) Ex: swiftmailer:
disable_delivery: "%env(DISABLE_EMAIL_DELIVERY)%" parameters:
env(DISABLE_EMAIL_DELIVERY): true
To me, it's unfortunate that 3.3 doesn't ship type hiting & conf validation :( |
Also, could be related BUT in the following example, the bundle validates the
It results when configuration being validated in :
|
Hi, environment Variables don't work only in case
I'm getting an error
Symfony version 3.3.2 docker-compose.yml
parameters.yml
|
@Chorss Which version of SwiftmailerBundle do you use? |
@Chorss this is unrelated to the current issue, but to SwiftmailerBundle. See symfony/swiftmailer-bundle#180 |
Is there anyway to work around environment variable data type? I want to pass some boolean value. |
And #23888 :) The workaround is to not use env params, but regular params for now. Or adjust your config constraints. |
This PR was merged into the 3.4 branch. Discussion ---------- [DI] Allow processing env vars | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | see description | License | MIT | Doc PR | - This PR is an updated version of #20276 ~~(it embeds #23899 for now.)~~ It superscedes/closes: - [DI] Add support for secrets #23621 ping @dunglas - Runtime container parameter not found event filter #23669 ping @marfillaster - [DependencyInjection] [DX] Support for JSON string environment variables #23823 ping @Pierstoval - add support for composite environment variables #17689 ping @greg0ire - [DI] ENV parameters at runtime with PHP 7 strict types not working properly #20434 ping @sandrokeil - Issue when using a SQLite database and the DATABASE_URL env var #23527 ping @javiereguiluz #22151 is another story, so not fixed here. The way it works is via `%env(foo:BAR)%` prefixes, where "foo" can be bound to any services you'd like. By default, the following prefixes are supported: - `bool`, `int`, `float`, `string`, `base64` - `const` (for referencing PHP constants) - `json` (supporting only json **arrays** for type predictability) - `file` (eg for access to secrets stored in files.) - `resolve` (for processing parameters inside env vars.) New prefixes can be added by implementing the new `EnvProviderInterface`, and tagging with `container.env_provider` (see `Rot13EnvProvider` in tests.) Prefixes can be combined to chain processing, eg. `%env(json:base64:file:FOO)%` will be roughly equivalent to `json_decode(base64_decode(file_get_content(getenv('FOO'))))`. Commits ------- 1f92e45 [DI] Allow processing env vars
Would anyone like to try #23888? It might solve this issue! |
Hey everyone, I'm in a need to implement environment variables set through parameters configuration to be run through a Configuration.php # config.yml
parameters:
locale: en
env(MY_VAR): true
bundleconfigurator:
toplevelconfig:
my_first_var:
active: "%env(MY_VAR)%" // DependencyInjection/Configuration.php
$treeBuilder = new TreeBuilder();
$rootNode = $treeBuilder->root('bundleconfigurator');
$rootNode
->children()
->arrayNode('toplevelconfig')
->useAttributeAsKey('name')
->prototype('array')
->children()
->booleanNode('active')
->beforeNormalization()
->ifString()
->then(function($v) {
if (substr($v, 0, 4) == "env_") {
// environment variable comes in as string like "env_MY_VAR_2bb0ccb650f8cf0aa754dd8396513258"
// I don't mind parsing this here to get the value, a bit hacky sure but I need it
// I just don't know what to query to get the value of the registered env variable
die(dump($v));
return $v;
}
return boolval($v);
})
->end()
->defaultFalse() What can I do here exactly? |
@RedactedProfile your work around is more or less what #23888 is aiming for out-of-the-box. With the exception you'd use
We are open for suggestions basically :-) |
I am sharing my hack here. If you really want to get your expected type from env, here is what I do: parameters:
env(MY_BOOL): "b:1;"
any_service:
trigger: !php/object env(MY_BOOL) but I agree with @fabpot after I read his design, I updated my code and did not need this use-case anymore. |
I'm closing this one as we do have support for casting values now. |
I still believe this is an issue, for example, in a fresh installation of symfony/skeleton if we edit our The This means that there is actually no way of disabling debug with the env variable Its true that normally we would not want to disable debug in dev env but I still think its an error to be able to set |
@hakebi APP_DEBUG is one of those very early used env var that won't ever have any kind of preprocessing. Just use |
@hakebi @nicolas-grekas it's not exactly the same problem. the problem is in index.php
if you declare APP_DEBUG in environment variable, $debug become a string and kernel wait a boolean... code would be :
|
Fixed in #23888 |
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
Related to #20434, if you try to pull data such as CACHE from an environment variable and pass it to Twig, Twig's Bundle reads the type literally, as "false" or "true" - as it is passed from
getenv
.How to Replicate
Add this environment variable:
In config.yml:
This results in a cache being created under
web/false/
, which is certainly not the intended result.Note that this has other unintended consequences throughout Symfony, where a boolean is expected but not validated.
Solutions?
A simple solution could be to use the
env()
library created by Laravel as it provides for type coercion out of the box.The text was updated successfully, but these errors were encountered: