8000 [DI] Allow injecting ENV parameters at runtime using %env(MY_ENV_VAR)% by nicolas-grekas · Pull Request #19681 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DI] Allow injecting ENV parameters at runtime using %env(MY_ENV_VAR)% #19681

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

Merged
merged 1 commit into from
Sep 15, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions src/Symfony/Component/DependencyInjection/Compiler/Compiler.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Exception\EnvParameterException;

/**
* This class is used to remove circular dependencies between individual passes.
Expand Down Expand Up @@ -108,8 +109,29 @@ public function getLog()
*/
public function compile(ContainerBuilder $container)
{
foreach ($this->passConfig->getPasses() as $pass) {
$pass->process($container);
try {
foreach ($this->passConfig->getPasses() as $pass) {
$pass->process($container);
}
} catch (\Exception $e) {
$usedEnvs = array();
$prev = $e;

do {
$msg = $prev->getMessage();

if ($msg !== $resolvedMsg = $container->resolveEnvPlaceholders($msg, null, $usedEnvs)) {
$r = new \ReflectionProperty($prev, 'message');
$r->setAccessible(true);
$r->setValue($prev, $resolvedMsg);
}
} while ($prev = $prev->getPrevious());

if ($usedEnvs) {
$e = new EnvParameterException($usedEnvs, $e);
}

throw $e;
}
}
}
33 changes: 31 additions & 2 deletions src/Symfony/Component/DependencyInjection/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@

namespace Symfony\Component\DependencyInjection;

use Symfony\Component\DependencyInjection\Exception\EnvNotFoundException;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;

/**
Expand Down Expand Up @@ -70,13 +71,14 @@ class Container implements ResettableContainerInterface
protected $loading = array();

private $underscoreMap = array('_' => '', '.' => '_', '\\' => '_');
private $envCache = array();

/**
* @param ParameterBagInterface $parameterBag A ParameterBagInterface instance
*/
public function __construct(ParameterBagInterface $parameterBag = null)
{
$this->parameterBag = $parameterBag ?: new ParameterBag();
$this->parameterBag = $parameterBag ?: new EnvPlaceholderParameterBag();
}

/**
Expand Down Expand Up @@ -372,6 +374,33 @@ public static function underscore($id)
return strtolower(preg_replace(array('/([A-Z]+)([A-Z][a-z])/', '/([a-z\d])([A-Z])/'), array('\\1_\\2', '\\1_\\2'), str_replace('_', '.', $id)));
}

/**
* Fetches a variable from the environment.
*
* @param string The name of the environment variable
*
* @return scalar The value to use for the provided environment variable name
*
* @throws EnvNotFoundException When the environment variable is not found and has no default value
*/
protected function getEnv($name)
Copy link
Contributor
@theofidry theofidry Nov 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicolas-grekas dumb question but why not having the default parameter value as a second argument here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for what purpose?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks more elegant and natural than doing something like:

parameters:
    env(FOO): bar
    foo: "%env(FOO)%"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but I don't understand. If you're suggesting to use %env(FOO, default)%, then again, this is a parameter name, not a DSL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suggesting to be able to do:

parameters:
    foo: "%env(FOO, 'bar')%"

instead of the workaround above, although the gain might be limited by the difficulty of parsing the default value. I guess I'm just being a bit to envious on Laravel way of doing things there:

return [
    'foo' => env('FOO', 'bar'),
];

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, fully agree. It's really nice that parameters.yml(.dist) can declare defaults. See e.g. symfony/demo@21fcb92

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my current project I dropped parameters.yml completely. I think its just unnecessary when using environment variables. For development I am using dotenv to bring the default values…

Copy link
Contributor
@theofidry theofidry Nov 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah actually I want to get rid of app_dev.php as well so using dotenv, although doable it's a bit annoying (dotenv should not be used in production).

But the answers that have been given are quite helpful, thanks for it :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but you shouldn't have unset environment variables in production either.
I think .env for development is a pretty good solution.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. The .env file will only be loaded if we are in debug mode and environment is not prod. So in prod, I specify every variable in my container configuration e.g. docker-compose.yml. For running unit tests or a composer install while building the container I do an export on the .env.example file which is similar to the previously parameters.yml.dist

{
if (isset($this->envCache[$name]) || array_key_exists($name, $this->envCache)) {
return $this->envCache[$name];
}
if (isset($_ENV[$name])) {
return $this->envCache[$name] = $_ENV[$name];
}
if (false !== $env = getenv($name)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to define "providers" instead of calling getenv directly ?
This way, we could use key/value store (like consul, etcd, ...) or even files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good idea! I suggest to focus on env vars in this PR and open for more env-like providers next.

return $this->envCache[$name] = $env;
}
if (!$this->hasParameter("env($name)")) {
throw new EnvNotFoundException($name);
}

return $this->envCache[$name] = $this->getParameter("env($name)");
}

private function __clone()
{
}
Expand Down
76 changes: 76 additions & 0 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
use Symfony\Component\Config\Resource\FileResource;
use Symfony\Component\Config\Resource\ResourceInterface;
use Symfony\Component\DependencyInjection\LazyProxy\Instantiator\InstantiatorInterface;
Expand Down Expand Up @@ -89,6 +90,16 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
*/
private $usedTags = array();

/**
* @var string[][] A map of env var names to their placeholders
*/
private $envPlaceholders = array();

/**
* @var int[] A map of env vars to their resolution counter.
*/
private $envCounters = array();

private $compiled = false;

/**
Expand Down Expand Up @@ -481,6 +492,18 @@ public function merge(ContainerBuilder $container)

$this->extensionConfigs[$name] = array_merge($this->extensionConfigs[$name], $container->getExtensionConfig($name));
}

if ($this->getParameterBag() instanceof EnvPlaceholderParameterBag && $container->getParameterBag() instanceof EnvPlaceholderParameterBag) {
$this->getParameterBag()->mergeEnvPlaceholders($container->getParameterBag());
}

foreach ($container->envCounters as $env => $count) {
if (!isset($this->envCounters[$env])) {
$this->envCounters[$env] = $count;
} else {
$this->envCounters[$env] += $count;
}
}
}

/**
Expand Down Expand Up @@ -551,8 +574,11 @@ public function compile()
}

$this->extensionConfigs = array();
$bag = $this->getParameterBag();

parent::compile();

$this->envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : array();
}

/**
Expand Down Expand Up @@ -995,6 +1021,56 @@ public function getExpressionLanguageProviders()
return $this->expressionLanguageProviders;
}

/**
* Resolves env parameter placeholders in a string.
*
* @param string $string The string to resolve
* @param string|null $format A sprintf() format to use as replacement for env placeholders or null to use the default parameter format
* @param array &$usedEnvs Env vars found while resolving are added to this array
*
* @return string The string with env parameters resolved
*/
public function resolveEnvPlaceholders($string, $format = null, array &$usedEnvs = null)
{
$bag = $this->getParameterBag();
$envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : $this->envPlaceholders;

if (null === $format) {
$format = '%%env(%s)%%';
}

foreach ($envPlaceholders as $env => $placeholders) {
foreach ($placeholders as $placeholder) {
if (false !== stripos($string, $placeholder)) {
$string = str_ireplace($placeholder, sprintf($format, $env), $string);
$usedEnvs[$env] = $env;
$this->envCounters[$env] = isset($this->envCounters[$env]) ? 1 + $this->envCounters[$env] : 1;
}
}
}

return $string;
}

/**
* Get statistics about env usage.
*
* @return int[] The number of time each env vars has been resolved
*/
public function getEnvCounters()
{
$bag = $this->getParameterBag();
$envPlaceholders = $bag instanceof EnvPlaceholderParameterBag ? $bag->getEnvPlaceholders() : $this->envPlaceholders;

foreach ($envPlaceholders as $env => $placeholders) {
if (!isset($this->envCounters[$env])) {
$this->envCounters[$env] = 0;
}
}

return $this->envCounters;
}

/**
* Returns the Service Conditionals.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public function dump(array $options = array())
}
}

return $this->startDot().$this->addNodes().$this->addEdges().$this->endDot();
return $this->container->resolveEnvPlaceholders($this->startDot().$this->addNodes().$this->addEdges().$this->endDot(), '__ENV_%s__');
}

/**
Expand Down
Loading
0