feat: support env var in varnish config for servers key (#564)

* feat: new config options servers_from_jsonenv to support a variable amount of proxy servers defined via an env var Co-authored-by: Tim Kask <[email protected]> Co-authored-by: David Buchmann <[email protected]>
FriendsOfSymfony · Sep 15, 2021 · 466dd92 · 466dd92
1 parent e5e1e42
commit 466dd92
Show file tree

Hide file tree

Showing 9 changed files with 214 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,13 @@
 Changelog
 =========
 
+2.11.0
+------
+
+### Added
+
+* New configuration option `servers_from_jsonenv` to support a variable amount of proxy servers defined via an environment variable.
+
 2.10.3
 ------
 

diff --git a/Resources/doc/reference/configuration/proxy-client.rst b/Resources/doc/reference/configuration/proxy-client.rst
@@ -37,6 +37,8 @@ varnish
                     servers:
                         - 123.123.123.1:6060
                         - 123.123.123.2
+                    # alternatively, if you configure the varnish servers in an environment variable:
+                    # servers_from_jsonenv: '%env(json:VARNISH_SERVERS)%'
                     base_url: yourwebsite.com
 
 ``header_length``
@@ -69,6 +71,26 @@ When using a multi-server setup, make sure to include **all** proxy servers in
 this list. Invalidation must happen on all systems or you will end up with
 inconsistent caches.
 
+.. note::
+
+    When using a variable amount of proxy servers that are defined via environment
+    variable, use the ``http.servers_from_jsonenv`` option below.
+
+``http.servers_from_jsonenv``
+"""""""""""""""""""""""""""""
+
+**type**: ``string``
+
+Json encoded servers array as string. The servers array has the same specs as ``http.servers``.
+
+Use this option only when using a variable amount of proxy servers that shall be defined via
+environment variable. Otherwise use the regular ``http.servers`` option.
+
+Usage:
+* fos_http_cache.yaml: ``servers_from_jsonenv: '%env(json:VARNISH_SERVERS)%'``
+* environment definition: ``VARNISH_SERVERS='["123.123.123.1:6060","123.123.123.2"]'``
+
+
 ``http.base_url``
 """""""""""""""""
 

diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php
@@ -488,19 +488,29 @@ private function addProxyClientSection(ArrayNodeDefinition $rootNode)
                         ->always()
                         ->then(function ($config) {
                             foreach ($config as $proxyName => $proxyConfig) {
-                                $serversConfigured = isset($proxyConfig['http']) && isset($proxyConfig['http']['servers']) && \is_array($proxyConfig['http']['servers']);
+                                // we only want either the servers config or the servers_from_jsonenv config
+                                if (isset($proxyConfig['http']['servers']) && !count($proxyConfig['http']['servers'])) {
+                                    unset($proxyConfig['http']['servers'], $config[$proxyName]['http']['servers']);
+                                }
+
+                                $arrayServersConfigured = isset($proxyConfig['http']['servers']) && \is_array($proxyConfig['http']['servers']);
+                                $jsonServersConfigured = isset($proxyConfig['http']['servers_from_jsonenv']) && \is_string($proxyConfig['http']['servers_from_jsonenv']);
+
+                                if ($arrayServersConfigured && $jsonServersConfigured) {
+                                    throw new InvalidConfigurationException(sprintf('You can only set one of "http.servers" or "http.servers_from_jsonenv" but not both to avoid ambiguity for the proxy "%s"', $proxyName));
+                                }
 
                                 if (!\in_array($proxyName, ['noop', 'default', 'symfony'])) {
-                                    if (!$serversConfigured) {
-                                        throw new \InvalidArgumentException(sprintf('The "http.servers" section must be defined for the proxy "%s"', $proxyName));
+                                    if (!$arrayServersConfigured && !$jsonServersConfigured) {
+                                        throw new InvalidConfigurationException(sprintf('The "http.servers" or "http.servers_from_jsonenv" section must be defined for the proxy "%s"', $proxyName));
                                     }
 
                                     return $config;
                                 }
 
                                 if ('symfony' === $proxyName) {
-                                    if (!$serversConfigured && false === $proxyConfig['use_kernel_dispatcher']) {
-                                        throw new \InvalidArgumentException('Either configure the "http.servers" section or enable "proxy_client.symfony.use_kernel_dispatcher"');
+                                    if (!$arrayServersConfigured && !$jsonServersConfigured && false === $proxyConfig['use_kernel_dispatcher']) {
+                                        throw new InvalidConfigurationException('Either configure the "http.servers" or "http.servers_from_jsonenv" section or enable "proxy_client.symfony.use_kernel_dispatcher"');
                                     }
                                 }
                             }
@@ -532,12 +542,14 @@ private function getHttpDispatcherNode()
             ->fixXmlConfig('server')
             ->children()
                 ->arrayNode('servers')
-                    ->info('Addresses of the hosts the caching proxy is running on. May be hostname or ip, and with :port if not the default port 80.')
+                    ->info('Addresses of the hosts the caching proxy is running on. The values may be hostnames or ips, and with :port if not the default port 80.')
                     ->useAttributeAsKey('name')
-                    ->isRequired()
                     ->requiresAtLeastOneElement()
                     ->prototype('scalar')->end()
                 ->end()
+                ->scalarNode('servers_from_jsonenv')
+                    ->info('Addresses of the hosts the caching proxy is running on (env var that contains a json array as a string). The values may be hostnames or ips, and with :port if not the default port 80.')
+                ->end()
                 ->scalarNode('base_url')
                     ->defaultNull()
                     ->info('Default host name and optional path for path based invalidation.')

diff --git a/src/DependencyInjection/FOSHttpCacheExtension.php b/src/DependencyInjection/FOSHttpCacheExtension.php
@@ -365,12 +365,23 @@ private function loadProxyClient(ContainerBuilder $container, XmlFileLoader $loa
      */
     private function createHttpDispatcherDefinition(ContainerBuilder $container, array $config, $serviceName)
     {
-        foreach ($config['servers'] as $url) {
+        if (array_key_exists('servers', $config)) {
+            foreach ($config['servers'] as $url) {
+                $usedEnvs = [];
+                $container->resolveEnvPlaceholders($url, null, $usedEnvs);
+                if (0 === \count($usedEnvs)) {
+                    $this->validateUrl($url, 'Not a valid Varnish server address: "%s"');
+                }
+            }
+        }
+        if (array_key_exists('servers_from_jsonenv', $config) && is_string($config['servers_from_jsonenv'])) {
+            // check that the config contains an env var
             $usedEnvs = [];
-            $container->resolveEnvPlaceholders($url, null, $usedEnvs);
+            $container->resolveEnvPlaceholders($config['servers_from_jsonenv'], null, $usedEnvs);
             if (0 === \count($usedEnvs)) {
-                $this->validateUrl($url, 'Not a valid Varnish server address: "%s"');
+                throw new InvalidConfigurationException('Not a valid Varnish servers_from_jsonenv configuration: '.$config['servers_from_jsonenv']);
             }
+            $config['servers'] = $config['servers_from_jsonenv'];
         }
         if (!empty($config['base_url'])) {
             $baseUrl = $config['base_url'];

diff --git a/tests/Resources/Fixtures/config/servers_from_jsonenv.php b/tests/Resources/Fixtures/config/servers_from_jsonenv.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the FOSHttpCacheBundle package.
+ *
+ * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+$container->loadFromExtension('fos_http_cache', [
+    'proxy_client' => [
+        'varnish' => [
+            'http' => [
+                'servers_from_jsonenv' => '%env(json:VARNISH_SERVERS)%',
+                'base_url' => '/test',
+                'http_client' => 'acme.guzzle.nginx',
+            ],
+        ],
+    ],
+]);
diff --git a/tests/Resources/Fixtures/config/servers_from_jsonenv.xml b/tests/Resources/Fixtures/config/servers_from_jsonenv.xml
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<container xmlns="http://symfony.com/schema/dic/services">
+
+    <config xmlns="http://example.org/schema/dic/fos_http_cache">
+        <proxy-client>
+            <varnish>
+                <http base-url="/test" http-client="acme.guzzle.nginx" servers-from-jsonenv="%env(json:VARNISH_SERVERS)%" />
+            </varnish>
+        </proxy-client>
+
+    </config>
+</container>
diff --git a/tests/Resources/Fixtures/config/servers_from_jsonenv.yml b/tests/Resources/Fixtures/config/servers_from_jsonenv.yml
@@ -0,0 +1,8 @@
+fos_http_cache:
+
+  proxy_client:
+    varnish:
+      http:
+        servers_from_jsonenv: '%env(json:VARNISH_SERVERS)%'
+        base_url: /test
+        http_client: acme.guzzle.nginx
diff --git a/tests/Unit/DependencyInjection/ConfigurationTest.php b/tests/Unit/DependencyInjection/ConfigurationTest.php
@@ -301,7 +301,7 @@ public function testSupportsSymfony()
     public function testEmptyServerConfigurationIsNotAllowed()
     {
         $this->expectException(InvalidConfigurationException::class);
-        $this->expectExceptionMessage('Either configure the "http.servers" section or enable "proxy_client.symfony.use_kernel_dispatcher');
+        $this->expectExceptionMessage('Either configure the "http.servers" or "http.servers_from_jsonenv" section or enable "proxy_client.symfony.use_kernel_dispatcher');
 
         $params = $this->getEmptyConfig();
         $params['proxy_client'] = [
@@ -734,4 +734,37 @@ private function getEmptyConfig()
             ],
         ];
     }
+
+    public function testSupportsServersFromJsonEnv(): void
+    {
+        $expectedConfiguration = $this->getEmptyConfig();
+        $expectedConfiguration['proxy_client'] = [
+            'varnish' => [
+                'http' => [
+                    'servers_from_jsonenv' => '%env(json:VARNISH_SERVERS)%',
+                    'base_url' => '/test',
+                    'http_client' => 'acme.guzzle.nginx',
+                ],
+                'tag_mode' => 'ban',
+                'tags_header' => 'X-Cache-Tags',
+            ],
+        ];
+        $expectedConfiguration['cache_manager']['enabled'] = 'auto';
+        $expectedConfiguration['cache_manager']['generate_url_type'] = 'auto';
+        $expectedConfiguration['tags']['enabled'] = 'auto';
+        $expectedConfiguration['invalidation']['enabled'] = 'auto';
+        $expectedConfiguration['user_context']['logout_handler']['enabled'] = true;
+
+        $formats = array_map(function ($path) {
+            return __DIR__.'/../../Resources/Fixtures/'.$path;
+        }, [
+            'config/servers_from_jsonenv.yml',
+            'config/servers_from_jsonenv.xml',
+            'config/servers_from_jsonenv.php',
+        ]);
+
+        foreach ($formats as $format) {
+            $this->assertProcessedConfigurationEquals($expectedConfiguration, [$format]);
+        }
+    }
 }
diff --git a/tests/Unit/DependencyInjection/FOSHttpCacheExtensionTest.php b/tests/Unit/DependencyInjection/FOSHttpCacheExtensionTest.php
@@ -17,10 +17,13 @@
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
 use Symfony\Component\DependencyInjection\ChildDefinition;
+use Symfony\Component\DependencyInjection\Compiler\ResolveEnvPlaceholdersPass;
+use Symfony\Component\DependencyInjection\Compiler\ResolveParameterPlaceHoldersPass;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\DefinitionDecorator;
-use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
+use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpKernel\Controller\ControllerResolverInterface;
 use Symfony\Component\HttpKernel\Kernel;
@@ -662,10 +665,81 @@ public function testVarnishCustomTagsHeader()
         $this->assertEquals(['tag_mode' => 'ban', 'tags_header' => 'myheader'], $container->getParameter('fos_http_cache.proxy_client.varnish.options'));
     }
 
+    /**
+     * @param array|null        $serversValue            array that contains servers, `null` if not set
+     * @param string|null       $serversFromJsonEnvValue string that should contain an env var (use `VARNISH_SERVERS` for this test), `null` if not set
+     * @param string|mixed|null $envValue                _ENV['VARNISH_SERVERS'] will be set to this value; only used if `$serversFromJsonEnvValue` is used; should be a string, otherwise an error will show up
+     * @param array|null        $expectedServersValue    expected servers value the http dispatcher receives
+     * @param string|null       $expectExceptionClass    the exception class the configuration might throw, `null` if no exception is thrown
+     * @param string|null       $expectExceptionMessage  the message the exception throws, anything if no exception is thrown
+     *
+     * @dataProvider dataVarnishServersConfig
+     */
+    public function testVarnishServersConfig($serversValue, $serversFromJsonEnvValue, $envValue, $expectedServersValue, $expectExceptionClass, $expectExceptionMessage): void
+    {
+        $_ENV['VARNISH_SERVERS'] = $envValue;
+        $container = $this->createContainer();
+
+        // workaround to get the possible env string into the EnvPlaceholderParameterBag
+        $container->setParameter('triggerServersValue', $serversValue);
+        $container->setParameter('triggerServersFromJsonEnvValue', $serversFromJsonEnvValue);
+        (new ResolveParameterPlaceHoldersPass())->process($container);
+
+        $config = $this->getBaseConfig();
+
+        if (null === $serversValue) {
+            unset($config['proxy_client']['varnish']['http']['servers']);
+        } else {
+            $config['proxy_client']['varnish']['http']['servers'] = $container->getParameter('triggerServersValue');
+        }
+        if (null !== $serversFromJsonEnvValue) {
+            $config['proxy_client']['varnish']['http']['servers_from_jsonenv'] = $container->getParameter('triggerServersFromJsonEnvValue');
+        }
+
+        if ($expectExceptionClass) {
+            $this->expectException($expectExceptionClass);
+            $this->expectExceptionMessage($expectExceptionMessage);
+        }
+
+        $this->extension->load([$config], $container);
+
+        // Note: until here InvalidConfigurationException should be thrown
+        if (InvalidConfigurationException::class === $expectExceptionClass) {
+            return;
+        }
+
+        (new ResolveEnvPlaceholdersPass())->process($container);
+
+        // Note: now all expected exceptions should be thrown
+        if ($expectExceptionClass) {
+            return;
+        }
+
+        $definition = $container->getDefinition('fos_http_cache.proxy_client.varnish.http_dispatcher');
+        static::assertEquals($expectedServersValue, $definition->getArgument(0));
+    }
+
+    public function dataVarnishServersConfig()
+    {
+        return [
+            // working case before implementing the feature 'env vars in servers key'
+            'regular array as servers value allowed' => [['my-server-1', 'my-server-2'], null, null, ['my-server-1', 'my-server-2'], null, null],
+            // testing the feature 'env vars in servers_from_jsonenv key'
+            'env var with json array as servers value allowed' => [null, '%env(json:VARNISH_SERVERS)%', '["my-server-1","my-server-2"]', ['my-server-1', 'my-server-2'], null, null],
+            // not allowed cases (servers_from_jsonenv)
+            'plain string as servers value is forbidden' => [null, 'plain_string_not_allowed_as_servers_from_jsonenv_value', null, null, InvalidConfigurationException::class, 'Not a valid Varnish servers_from_jsonenv configuration: plain_string_not_allowed_as_servers_from_jsonenv_value'],
+            'an int as servers value is forbidden' => [null, 1, 'env_value_not_used', null, InvalidConfigurationException::class, 'The "http.servers" or "http.servers_from_jsonenv" section must be defined for the proxy "varnish"'],
+            'env var with string as servers value is forbidden (at runtime)' => [null, '%env(json:VARNISH_SERVERS)%', 'wrong_usage_of_env_value', 'no_servers_value', RuntimeException::class, 'Invalid JSON in env var "VARNISH_SERVERS": Syntax error'],
+            // more cases
+            'no definition leads to error' => [null, null, 'not_used', 'not_used', InvalidConfigurationException::class, 'The "http.servers" or "http.servers_from_jsonenv" section must be defined for the proxy "varnish"'],
+            'both servers and servers_from_jsonenv defined leads to error' => [['my-server-1', 'my-server-2'], '%env(json:VARNISH_SERVERS)%', 'not_used', 'not_used', InvalidConfigurationException::class, 'You can only set one of "http.servers" or "http.servers_from_jsonenv" but not both to avoid ambiguity for the proxy "varnish"'],
+        ];
+    }
+
     private function createContainer()
     {
         $container = new ContainerBuilder(
-            new ParameterBag(['kernel.debug' => false])
+            new EnvPlaceholderParameterBag(['kernel.debug' => false])
         );
 
         // The cache_manager service depends on the router service