So all env vars would be reset ? Without option / configuration ? Or am i reading this wrong ?

So all env vars would be reset ? Without option / configuration ? Or am i reading this wrong ?

Yes (for now), I wasn't sure how to best make it opt-in (config based). Do you think it should be opt in?

I'm all for the feature.

We could even consider this as a bugfix IMHO.

This will a have a negative impact on performances for messenger applications that uses a slow custom env-var loader or processor.

a slow custom env-var loader or processor.

env loaders/processors are used on web requests, so they have to be fast anyway.
and messenger is not on the hotpath, UX-wise.

So all env vars would be reset ? Without option / configuration ? Or am i reading this wrong ?

Yes (for now), I wasn't sure how to best make it opt-in (config based). Do you think it should be opt in?

No, but that would probably need some doc on messenger page afterward :)

If they process messages at high throughput, they are going to see a rapid impact on their infrastructure, with congested queues. I've been in that situation, I know it's the kind of change I would want to be notified about.

I'm not sure I understand your concern. As I wrote, env loaders/processors are used on web requests, so they have to be fast anyway. Fast enough for high-throughput.
Note that reading env vars from $_SERVER is already really fast so we're talking about loading them from e.g. consult or similar.
Are you talking about a real-life app where this could have impact?

I'm not sure I understand your concern. As I wrote, env loaders/processors are used on web requests, so they have to be fast anyway. Fast enough for high-throughput. Note that reading env vars from $_SERVER is already really fast so we're talking about loading them from e.g. consult or similar. Are you talking about a real-life app where this could have impact?

Yes, I'm talking about an application that fetches certain variables from Hashicorp Vault, without using caches. As these variables were not used for the web, there were no problem. I wrote an article about how I solved this issue and having resetable env var loader is a very great feature for this use-case, but I just wanted to warn that it's a change in behavior that I think is too important to be released as a bugfix.

fabbot errors are unrelated.

fabbot errors are unrelated.

fixed fabbot too

reverted as per review

This makes sense to me. Maybe we could also add a StaticEnvVarLoader that'd be a decorator of EnvVarLoaderInterface and that'd we use to decorate SodiumVault?

What would StaticEnvVarLoader do? cache the env vars of internal EnvVarLoader (SodiumVault) so that subsequent calls to loadEnvVars won't require decrypting secrets again?

Index: src/Symfony/Bundle/FrameworkBundle/Secrets/StaticEnvVarLoader.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/Symfony/Bundle/FrameworkBundle/Secrets/StaticEnvVarLoader.php b/src/Symfony/Bundle/FrameworkBundle/Secrets/StaticEnvVarLoader.php
new file mode 100644
--- /dev/null	(date 1714166958969)
+++ b/src/Symfony/Bundle/FrameworkBundle/Secrets/StaticEnvVarLoader.php	(date 1714166958969)
@@ -0,0 +1,40 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Secrets;
+
+use Symfony\Component\DependencyInjection\Attribute\AsDecorator;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
+use Symfony\Component\DependencyInjection\Attribute\AutowireDecorated;
+use Symfony\Component\DependencyInjection\EnvVarLoaderInterface;
+
+/**
+ * @internal
+ */
+#[AsDecorator('secrets.vault')]
+class StaticEnvVarLoader implements EnvVarLoaderInterface
+{
+    private array $envCache = [];
+
+    public function __construct(
+        #[AutowireDecorated]
+        private EnvVarLoaderInterface $envVarLoader,
+    ) {
+    }
+
+    public function loadEnvVars(): array
+    {
+        if ([] !== $this->envCache) {
+            return $this->envCache;
+        }
+
+        return $this->envCache = $this->envVarLoader->loadEnvVars();
+    }
+}

Like this?

Yes!

This makes sense to me. Maybe we could also add a StaticEnvVarLoader that'd be a decorator of EnvVarLoaderInterface and that'd we use to decorate SodiumVault?

Done a888820

Thank you @faizanakram99.

Hi,

Unless i'm missing something, i can't see that this works, currently?
Neither EnvVarProcessor or service_container (obviously) are added to ServiceResetter, so the env vars will never be reset when messenger tickets over.

How is this intended to work?

Thanks!

so to fix, EnvVarProcessor, we need to add here

->tag('kernel.reset', ['method' => 'reset'])

but even then, the Container still has the env vars cached so the reset EnvVarProcessor will never be called.

Any insight would be appreciated!

as far as i'm away, the container is only ever reset in KernelTestCase

Resetting env variables is service_resetter is not a good idea, as this does not reset the instantiated service and so would not take into account new value of environment variables injected in arguments.
Also, I don't think you can actually change the environment variables of an existing process, so I don't see a use case for it.

Resetting env variables is service_resetter is not a good idea, as this does not reset the instantiated service and so would not take into account new value of environment variables injected in arguments.
Also, I don't think you can actually change the environment variables of an existing process, so I don't see a use case for it.

See the linked issue for valid use cases, TLDR; messenger is unusable without this feature in multi tenant environment (single web server + multi databases)

so to fix, EnvVarProcessor, we need to add here

->tag('kernel.reset', ['method' => 'reset'])

but even then, the Container still has the env vars cached so the reset EnvVarProcessor will never be called.

Any insight would be appreciated!

@bendavies you're right, please open an issue. I think we need to clear container env vars differently as it is not reset by services_resetter, adding kernel.reset tag to env var processor won't be enough like you mentioned the cached env vars in container will take precedence.