[DependencyInjection] Make Container::getEnv public

[malarzm]

Q	A
Branch?	6.2
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #34173, #45450 (comment)
License	MIT
Doc PR	to be added if accepted

In our app we're controlling certain features' availability with feature switches defined in envs (to prevent non-tech folks from turning them on without our assitance). We'd like to show whether those switches are on or off in the administration panel. Sadly, there is no kosher way to get those values during runtime other than injecting every variable we want to show. By making getEnv public we're exposing not only raw values one could get with getenv but more importantly processors. By the way we're also ironing out two not-so-great hacks to get env variables in Symfony itself (included in PR).

Code will need finishing if we agree this is good idea:

• I'll deal with new interface method according to the rules
• Documentation will be written

I just don't want to waste time on these in case the PR will be shot down :)

[raziel057]

@malarzm Could it help for the following topics?
lexik/LexikTranslationBundle#420
#33358

Currently it seems complicated to resolve environment variable on loading extensions.

[malarzm]

Currently it seems complicated to resolve environment variable on loading extensions.

No, this PR barely exposes accessing env variables to the outside world and does not change any parameters vs envs vs extensions.

[stof]

@raziel057 not reading environment variables inside extensions is not a bug. It is a feature. The whole promise of env placeholders in the DependencyInjection is that new values of the environment variable will be taken into account without requiring to clear the cache. So if a container extension uses a setting to change the structure of the container instead of injecting into a service, it cannot use an env placeholder.

[raziel057]

@stof Thanks for your answer. The issue is that often when you deploy an app in a container for example you don't want to embed a config file for static parameters that can change if you deploy the same image in different environment. It would be just nice / easier to be able to specify if the ENV should be resolved statically or not.

I have seen lots of people simply define static ENV vars in docker compose files or Kubernetes Management Platforms like Rancher for example.

I just seen this have been already discussed here: #40794

[stof]

@raziel057 for such case, I'm still using https://github.com/Incenteev/ParameterHandler to create non-env-based parameters during composer install, based either on env variables or an interactive prompt.

[nicolas-grekas]

I'm 👎 for the reasons given by @stof. Closing therefore, thanks for suggesting.

[malarzm]

I'm sorry but I'm missing the reasons? To me it looks like discussion was unrelated to the proposal itself as it was about how envs are behaving.

[ro0NL]

would this already be achievable using either the ParameterBagInterface or ContainerBagInterface?

[malarzm]

would this already be achievable using either the ParameterBagInterface or ContainerBagInterface?

It wouldn't expose processors that way as its logic is defined by \Symfony\Component\DependencyInjection\Container::getEnv itself. For instance in our example in our envs we have: SOME_FEATURE='2022-07-11 13:00:00'; and we have dedicated processor %env(feature_switch:SOME_FEATURE)% in services definitions (the processor treats date-ish strings as bool:'s true). Now if we'd be relying on stuff defined in parameters I would be forbidden to call %env(bool:SOME_FEATURE)% somewhere down the line (continuing my example, in a controller showing status of a feature) as that parameter does not exist (nor its fallback as we're defining it with feature_switch:).

[ro0NL]

IMHO the way forward would be $this->parameterBag->get('env(csv:APP_FOO)')

which currently only works using a hack like

parameters:
    env(csv:APP_FOO): '%env(csv:APP_FOO)%'

making it work out-of-the-box would be more elegant though.

[nicolas-grekas]

Rereading this again I'm 👎 here: exposing new APIs is increasing the maintenance cost, and here, ppl will jump into using the method and shooting themself in the foot (see comments from @raziel057 and @stof)

IMHO the way forward would be $this->parameterBag->get('env(csv:APP_FOO)')

Yes, that's a good way forward (it works with any parameter name, not only env(...))`

[nicolas-grekas]

Closing as explained, thanks for proposing.