Skip to content
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

[DependencyInjection] [DX] Support for JSON string environment variables #23823

Closed
Pierstoval opened this issue Aug 7, 2017 · 20 comments
Closed
Labels
DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) Feature

Comments

@Pierstoval
Copy link
Contributor

Pierstoval commented Aug 7, 2017

Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no
Symfony version master (new feature), so 3.4 or 4.1

A year ago, a proposal was made to support composite env vars (#17689), and was closed as "won't fix".

Today, I think we can take benefit from the %env(MY_VAR)% syntax to support other kind of environment vars than simple strings.

Usage in the real world

I don't know many environments and platforms, but I can say that Platform.sh defines certain env vars as serialized JSON strings, and we can use them in our applications by deserializing them with json_encode().

This gave me the idea of a new implementation for env vars as array: json strings.

Base Proposal: %json_env()%

We could add a new env var handling system with a syntax like %json_env(MY_VAR)% that would, at runtime (else, it's useless to have env vars) retrieve this env var and run json_encode(getenv('MY_VAR'), true) on it.

Currently, deserializing can be as simple as this:

$ ARRAY_VAR='{"a":1,"b":2}' php -r 'dump(json_decode(getenv("ARRAY_VAR"), true));'
array:2 [
  "a" => 1
  "b" => 2
]

Adding this to Symfony's service container doesn't seem like a big issue to me, but I'm posting an issue for advices 😄

I don't have any performance benchmark on this, but if we are looking for performances, we might as well go with PHP unserialize(), but I'm not sure this would be really DX then...

Bonus: deep parsing

As we can have control over this json_encode() call (by using a method in the container, like
$this->parseJsonEnvVar($envVarName)) we can also navigate through the array and look for other %env()% or %json_env()% strings to parse them too, still at runtime. However, it might lead to performance issues when dealing with too much nested env vars. But this is just a bonus proposal 😉

What do you think?

@greg0ire
Copy link
Contributor

greg0ire commented Aug 7, 2017

Great proposal! Only one question for now: how should invalid json be handled?

@Pierstoval
Copy link
Contributor Author

Pierstoval commented Aug 7, 2017

Great proposal! Only one question for now: how should invalid json be handled?

That's why I suggest to have something like $this->parseJsonEnvVar($envVarName) in the container: to retrieve json errors and throw an exception if the JSON is invalid.

We can even create an option for this specific feature to throw an exception on error if the option is set to true.

Edit: This is something already done in Guzzle's alternate json_decode function

@sstok
Copy link
Contributor

sstok commented Aug 8, 2017

As a small bonus parsing 12 as JSON will produce an actual integer! So this would actually solve a number of other problems.

@Pierstoval
Copy link
Contributor Author

As well as parsing {"my_param":false} will allow having real booleans in our config params :)

@javiereguiluz javiereguiluz added DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) Feature labels Aug 8, 2017
@ro0NL
Copy link
Contributor

ro0NL commented Aug 9, 2017

As well as parsing {"my_param":false} will allow having real booleans in our config params

That would give an array no?

As a small bonus parsing 12 as JSON will produce an actual integer!

Personally id prefer explicitness via either int_env(NAME) or env:int(NAME), ie support primitive scalar types first. Then again if we support all by leveraging json.. thats fine i guess :)

See also #22151

👍

@fabpot
Copy link
Member

fabpot commented Aug 9, 2017

First question: Which problem does it solve? What can you do with that new feature that you can do today?

@ro0NL
Copy link
Contributor

ro0NL commented Aug 9, 2017

For me __construct(bool $x) along with App\Service: ['%env(X)%'] where X=1 or 0.

And also for this to really work (i expected Argument 1 passed to App\Service::__construct() must be of the type boolean, string given,), the compiled container needs to declare strict_types=1, which can be a new option :)

Im skeptical about json support as a feature, but i do look primitive scalar support, something that evals to (bool) $this->getEnv('ZERO_OR_ONE').

@greg0ire
Copy link
Contributor

greg0ire commented Aug 9, 2017

First question: Which problem does it solve? What can you do with that new feature that you can do today?

@fabpot for me it solves configuring FOSElasticaBundle with several ES connections, which allows to use a round-robin strategy, in case one of the nodes goes down.

@fabpot
Copy link
Member

fabpot commented Aug 9, 2017

@greg0ire Can you elaborate? I don't see how managing several ES connections in an env var that would be a JSON string is appealing. Can't you "just" configure your connections via parameters in your container configuration?

@greg0ire
Copy link
Contributor

greg0ire commented Aug 9, 2017

@fabpot sure!
Appealing goals I want to reach:

  1. My application should have no knowledge of the number of nodes I have (number which may vary depending on the environment).
  2. I'd like to be able to add, remove or change the nodes at runtime.
  3. Id' like to store all of my machine-specific configuration in the environment. No parameters.yml or .env

If I understand correctly, ES has an auto discovery feature which makes a load balancer useless as it is supposed to do the load balancing itself. But what if a node goes down? I don't want to always ask the same node where to get my result, I want the round robin strategy. Right now I am able to do that with a parameters.yml, but ultimately I would like to be able to follow this tenet, namely "Store config in the environment", for a number of reasons that I can detail further if you wish me to.

@fabpot
Copy link
Member

fabpot commented Aug 9, 2017

@greg0ire It looks more like a discovery problem, for which there are solutions.

@greg0ire
Copy link
Contributor

greg0ire commented Aug 9, 2017

Fair enough, I'm probably not solving the issue with the right tool here.

@Pierstoval
Copy link
Contributor Author

Pierstoval commented Aug 9, 2017

@fabpot Even though it's kinda specific, there's a nice and complex use case on symfony/flex#53 about the fact that env vars are too simple for some application configurations.

JSON env vars might solve this.

Of course, we can already do something like this in a PHP config file:

// app/config/env.php
$conf = json_decode(getenv('COMPLEX_CONFIG'), true);
$container->setParameter('my_var', $conf);

Or with any parameters.yml-like file.

But this has the disadvantage of not being updated at runtime, which is the big point of env vars (else, if you don't care about runtime evaluation, let's go for parameters file, and it's done).

@ro0NL:

As well as parsing {"my_param":false} will allow having real booleans in our config params

That would give an array no?

Yes the example shows an array, but even json_decode('false') gives you false as boolean, not a string, that's what I wanted to show, but did not do it the right way 😅

@fabpot
Copy link
Member

fabpot commented Aug 9, 2017

@Pierstoval Using a parameters.yml file to configure things is still perfectly valid. Using env vars to configure emails looks wrong to me. The main advantage of env vars is that you can change their value dynamically. Doing that for a DB hostname to avoid redeploying makes sense to me.

But you should NOT use env vars to configure your project.

@Pierstoval
Copy link
Contributor Author

But you should NOT use env vars to configure your project.

I agree, but sometimes, "project configuration" can be much more complex than a bunch of environment vars.

This JSON env vars proposal is also here to simplify the workflow of more complex applications. Maybe services like CI or online services like Insight, or anything else that may need a special configuration (Heroku, etc.) could implement a simple JSON textarea for some complex confs, and text inputs for simple confs. No need for 300-lines of JSON, but this could be useful when dealing with read-only filesystems in containers, env vars imported into php scripts, etc.

But still, this is a proposal, and actually, @ro0NL's idea about %env:int(VAR)% is really interesting too, to validate env vars before using them.

@fabpot
Copy link
Member

fabpot commented Aug 9, 2017

Again, env vars are NOT for project configuration. So, the "project configuration" can be much more complex than a bunch of environment vars" part is irrelevant to me.

%env:int(VAR)% is something different indeed, but I fail to see how this is related to the current discussion.

@Pierstoval
Copy link
Contributor Author

%env:int(VAR)% is something different indeed, but I fail to see how this is related to the current discussion.

Right, maybe move this to another issue

For the rest, I don't have any more argument against the fact that env vars should not be used for project configuration, so, I'm not the one that takes the final decision 😄

Maybe this could be closed if more people agree

@fabpot
Copy link
Member

fabpot commented Aug 9, 2017

I think it's safe to close :) Let's talk about %env:int(VAR)% in the relevant issue (I think one is still open about this)

@fabpot fabpot closed this as completed Aug 9, 2017
@chalasr
Copy link
Member

chalasr commented Aug 9, 2017

#22151 is the one asking for typed env vars.

@nicolas-grekas
Copy link
Member

#22151 is about adding type at compile time to pass config, which is harder to implement than having a specific type at runtime.

For this issue, and almost all others that try to enhance what's possible with env vars, I think a revisited #20276 is the correct solution (#22151 is another story.)

fabpot added a commit that referenced this issue Sep 7, 2017
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) Feature
Projects
None yet
Development

No branches or pull requests

8 participants