[Bug] DSN is not interpolated at runtime with environment variable

[B-Galati]

Hello,

I just upgraded from 2.0.6 to 2.1.0 and my app broke because the DSN was not updated.

It looks like the issue is similar to #302.

I checked my cache and the value of the config is hard-coded instead of using the env variable.

Here is my config :

snc_redis:
    clients:
        default:
            type: predis
            alias: default
            dsn: "%env(REDIS_DSN)%"
    session:
        client: default
        prefix: session

Am I missing something ?

In the meantime I moved back to 2.0.6 and everything is back to normal.

Let me know for anything and thanks for your help.

[curry684]

@Majkl578 could this be related to #393? Think that's the only possibly relevant change between 2.0.6 and 2.1.0.

[curry684]

It's reproducible but no time right now to find the cause. Will happily draft 2.1.1 release after we fix this as it's quite fundamental (the Symfony recipe generates this suggested code).

[Majkl578]

Unlikely, it only added env support for profile, support for DSN was already there and imho was resolving env during compile time before.

[Majkl578]

If anything, I'd suggest to dig into #378, that PR changed how DSN is being resolved.

[curry684]

Hum yes given #361 (comment) that seems plausible hehe.

[curry684]

It's reproducible

Scrap that, I reproduced it once this afternoon in a project right after upgrading 2.0.6 to 2.1.0, but not since.

@B-Galati doesn't cache clearing fix this for you? Perhaps a hard rm -rf var/cache/*?

[B-Galati]

@curry684 No it's not fixing the issue. The value of the env variable stays hardcoded in the cache instead of having the env name resolved at runtime.

[curry684]

I'm sorry but I've just tried reproducing the issue in every possible way, with both phpredis and Predis, in a Symfony 4 project with SncRedisBundle 2.1.0, and it's all just working as it should. This still indicates the only reason I saw it once could've been because of lingering cache.

I'm marking this as not reproducible until I can debug a failing case.

[B-Galati]

@curry684 thank you for your help.

I set up a new SF4 project and I have the same problem :

❯ grep -ri "REDIS_URL" var
// nothing
❯ grep -ri "localhost" var
var/cache/dev/srcDevDebugProjectContainer.xml:    <parameter key="router.request_context.host">localhost</parameter>
var/cache/dev/srcDevDebugProjectContainer.xml:      <argument>redis://localhost</argument>
var/cache/dev/srcDevDebugProjectContainer.xml:      <argument>memcached://localhost</argument>
var/cache/dev/srcDevDebugProjectContainer.xml:      <argument>localhost</argument>
var/cache/dev/srcDevDebugProjectContainer.xml:        <argument key="host">localhost</argument>
var/cache/dev/ContainerFncx1D4/getSncRedis_DefaultService.php:return $this->services['snc_redis.default'] = new \Predis\Client(new \Predis\Connection\Parameters(array('read_write_timeout' => NULL, 'iterable_multibulk' => false, 'profile' => 'default', 'prefix' => NULL, 'service' => NULL, 'async_connect' => false, 'timeout' => 5, 'persistent' => false, 'exceptions' => true, 'logging' => true, 'alias' => 'default', 'scheme' => 'tcp', 'host' => 'localhost', 'port' => 6379, 'password' => NULL, 'weight' => NULL)), new \Predis\Configuration\Options(array('read_write_timeout' => NULL, 'iterable_multibulk' => false, 'profile' => $a, 'prefix' => NULL, 'service' => NULL, 'async_connect' => false, 'timeout' => 5, 'persistent' => false, 'exceptions' => true, 'connections' => $b)));
var/cache/dev/ContainerFncx1D4/srcDevDebugProjectContainer.php:        return $this->privates['router.request_context'] = new \Symfony\Component\Routing\RequestContext('', 'GET', 'localhost', 'http', 80, 443);
var/cache/dev/ContainerFncx1D4/srcDevDebugProjectContainer.php:            'router.request_context.host' => 'localhost',

What I am expected, illustrated with doctrine for example is :

❯ grep -ri "DATABASE_URL" var       
var/cache/dev/srcDevDebugProjectContainer.xml:    <parameter key="env(DATABASE_URL)"></parameter>
var/cache/dev/srcDevDebugProjectContainer.xml:        <argument key="url">%env(resolve:DATABASE_URL)%</argument>
var/cache/dev/Container9xsATk3/getDoctrine_Dbal_DefaultConnectionService.php:return $this->services['doctrine.dbal.default_connection'] = (new \Doctrine\Bundle\DoctrineBundle\ConnectionFactory(array()))->createConnection(array('driver' => 'pdo_mysql', 'charset' => 'utf8mb4', 'url' => $this->getEnv('resolve:DATABASE_URL'), 'host' => 'localhost', 'port' => NULL, 'user' => 'root', 'password' => NULL, 'driverOptions' => array(), 'serverVersion' => '5.7', 'defaultTableOptions' => array('charset' => 'utf8mb4', 'collate' => 'utf8mb4_unicode_ci')), $c, $d, array());
var/cache/dev/Container9xsATk3/srcDevDebugProjectContainer.php:            'env(DATABASE_URL)' => '',
❯ grep -ri "db_name" var 
// nothing

[B-Galati]

After a downgrade to 2.0.6 I have the expected result :

❯ grep -ri "REDIS_URL" var            
var/cache/dev/srcDevDebugProjectContainer.xml:      <argument>%env(REDIS_URL)%</argument>
var/cache/dev/ContainerKMTwAcP/getSncRedis_DefaultService.php:return $this->services['snc_redis.default'] = new \Predis\Client(\Snc\RedisBundle\Factory\EnvParametersFactory::create(array('read_write_timeout' => NULL, 'iterable_multibulk' => false, 'profile' => 'default', 'prefix' => NULL, 'service' => NULL, 'async_connect' => false, 'timeout' => 5, 'persistent' => false, 'exceptions' => true, 'logging' => true, 'alias' => 'default'), 'Predis\\Connection\\Parameters', $this->getEnv('REDIS_URL')), new \Predis\Configuration\Options(array('read_write_timeout' => NULL, 'iterable_multibulk' => false, 'profile' => $a, 'prefix' => NULL, 'service' => NULL, 'async_connect' => false, 'timeout' => 5, 'persistent' => false, 'exceptions' => true, 'connections' => $b)));

[Majkl578]

https://github.com/snc/SncRedisBundle/pull/378/files#diff-4bb08082b89e25284bedb5c26e749d7aR108

using $container->resolveEnvPlaceholders($dsn, true) is definitely on blame here, but I don't know the background why that was done that way - cc @kozlice?

[B-Galati]

Yea in the PR it just says

Reverts pretty much everything from #353.

But no more information sadly :-(

[curry684]

Ok, I can also reproduce the issue from skeleton. Suppose I was looking for the wrong problem by trying it out in a dockerized environment, since that means I'm always implicitly dependent on a container compilation with the correct env variables present. In practice, the current implementation is only an issue on Heroku and similar environments.

I think we may just need to rethink the entire DSN parsing thing and ignore all of it until we're making a connection at runtime. That would be a big refactor though, so if @kozlice can chime in he may know how to quickfix it for now.

[ghost]

@B-Galati Hi, can you copy the exact error message you have (if any)? I was thinking to have the same issue as your, but after a long seek, I've found an other track, related to Symfony 4.1 (dev-master) and the Configuration.

[curry684]

@romain-pierre there is no error, the value is just replaced at compiletime instead of at runtime, as should be the case with env variables as it enables the Heroku usecase where the env variable may change between requests.

[B-Galati]

FYI, I am working on a fix ;-)

[kozlice]

@Majkl578 @curry684 I can't remember why exactly did I do it this way. But it's most likely I've used the same approach as Twig & Doctrine bundles (they had env vars working properly).

[curry684]

In that case you likely overlooked that unlike those 2 we 'compile' the DSNs during container compilation, removing the runtime reference.

[curry684]

Reopening as it's only fixed in master now and I'll have to see if I can merge it back in 2.1.

[B-Galati]

Let me know if you need anything

[curry684]

Merged & resolved conflicts @ https://github.com/snc/SncRedisBundle/commits/2.1