diff --git a/CHANGELOG.md b/CHANGELOG.md index a021859..1d5bb5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Latest * [#150](https://github.com/cleverage/ui-process-bundle/issues/150) Require `symfony/monolog-bundle` and `monolog/monolog`, used directly by the bundle (log handlers, `monolog` configuration prepended, `monolog.logger` tags) but only installed through `cleverage/process-bundle`, which may stop requiring MonologBundle (cleverage/process-bundle#269). ## Fixes +* [#171](https://github.com/cleverage/ui-process-bundle/issues/171) Configuration: `config:dump-reference clever_age_ui_process` and `debug:config clever_age_ui_process` no longer fail (`CleverAgeUiProcessExtension::getConfiguration()` gives the kernel environment to `Configuration`, so the PHP config builder is generated too); the options are documented with `info()`. The `logs.database_level`, `logs.file_level` and `logs.report_increment_level` options are validated when the container is compiled, an invalid level no longer makes every process execution fail at runtime (environment variables are still allowed, checked at runtime). Update documentation, add tests. * [#169](https://github.com/cleverage/ui-process-bundle/issues/169) Security: the configuration added by the bundle to the `security` extension (`process_user_provider` user provider, provider, login form, logout and API token authenticator of the `main` firewall) overrides the application one, it can now be disabled with the new `security.configure_firewall` option (default `true`, same behaviour as before; `false`: no `security` configuration is added, the application configures the provider and the firewall, e.g. a firewall dedicated to the UI). The `monolog`, `doctrine_migrations` and `security` configurations are skipped when the bundle is not enabled, instead of a `LogicException`. The documentation no longer says that these configurations are prepended (only the Messenger one is). Update documentation, add tests. v3.2 diff --git a/docs/reference/01-bundle_configuration.md b/docs/reference/01-bundle_configuration.md index da0b2d2..674a8c9 100644 --- a/docs/reference/01-bundle_configuration.md +++ b/docs/reference/01-bundle_configuration.md @@ -4,7 +4,9 @@ Bundle configuration YAML Configuration ------------------ -Every key is optional. Default values are shown below. +Every key is optional. Default values are shown below (also displayed, with the description of each option, by +`bin/console config:dump-reference clever_age_ui_process`; `bin/console debug:config clever_age_ui_process` shows the +current configuration). ```yaml # config/packages/clever_age_ui_process.yaml @@ -57,7 +59,9 @@ See [process executions & logs](04-process_executions_and_logs.md) for the whole See [log retention](04-process_executions_and_logs.md#log-retention). Levels are Monolog level names (case-insensitive): `Debug`, `Info`, `Notice`, `Warning`, `Error`, `Critical`, -`Alert`, `Emergency`. +`Alert`, `Emergency`; `database_level` and `file_level` also accept the level values (`100` to `600`). An invalid level +fails when the container is compiled (`Invalid configuration for path "clever_age_ui_process.logs."`). A level +given by an environment variable is only checked at runtime, by Monolog. ### design diff --git a/src/DependencyInjection/CleverAgeUiProcessExtension.php b/src/DependencyInjection/CleverAgeUiProcessExtension.php index 402e2fc..5d232da 100644 --- a/src/DependencyInjection/CleverAgeUiProcessExtension.php +++ b/src/DependencyInjection/CleverAgeUiProcessExtension.php @@ -18,6 +18,7 @@ use CleverAge\UiProcessBundle\Controller\Admin\UserCrudController; use CleverAge\UiProcessBundle\Entity\User; use CleverAge\UiProcessBundle\Message\ProcessExecuteMessage; +use Monolog\Level; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -34,9 +35,7 @@ public function load(array $configs, ContainerBuilder $container): void { $this->findServices($container, __DIR__.'/../../config/services'); - /** @var string $env */ - $env = $container->getParameter('kernel.environment'); - $configuration = new Configuration($env); + $configuration = $this->getConfiguration($configs, $container); /** @var array{ * security: array{roles: list, configure_firewall: bool}, * logs: array{store_in_database: bool, database_level: string, file_level: string, report_increment_level: string, database_max_days: ?int, file_max_days: ?int}, @@ -46,6 +45,7 @@ public function load(array $configs, ContainerBuilder $container): void * } $config */ $config = $this->processConfiguration($configuration, $configs); + $this->validateLogLevels($config['logs'], $container); $container->getDefinition(UserCrudController::class) ->setArgument('$roles', array_combine($config['security']['roles'], $config['security']['roles'])); @@ -83,6 +83,54 @@ public function load(array $configs, ContainerBuilder $container): void } } + /** + * The log levels are validated here instead of in the configuration tree, so that environment variables are + * still allowed: an environment variable is only resolved at runtime (its value is then validated by Monolog). + * + * @param array{database_level: mixed, file_level: mixed, report_increment_level: mixed, ...} $logs + */ + private function validateLogLevels(array $logs, ContainerBuilder $container): void + { + $levels = [ + // Logger::toMonologLevel() of the handlers: a level name (case-insensitive) or value + 'database_level' => true, + 'file_level' => true, + // Level::fromName() of ProcessHandler::setReportIncrementLevel(): a level name (case-insensitive) + 'report_increment_level' => false, + ]; + foreach ($levels as $option => $valueAllowed) { + $level = $logs[$option]; + $usedEnvs = []; + $container->resolveEnvPlaceholders($level, null, $usedEnvs); + if ([] !== $usedEnvs) { + continue; + } + if ($valueAllowed && (\is_int($level) || (\is_string($level) && is_numeric($level)))) { + $valid = Level::tryFrom((int) $level) instanceof Level; + } else { + $valid = \is_string($level) && \in_array(strtoupper($level), Level::NAMES, true); + } + if (!$valid) { + throw new InvalidConfigurationException(\sprintf('Invalid configuration for path "%s.logs.%s": invalid Monolog level %s, use one of: %s.', $this->getAlias(), $option, json_encode($level), implode(', ', Level::NAMES))); + } + } + } + + /** + * The default log levels depend on the kernel environment, so the configuration cannot be instantiated without it + * by the parent method (used by config:dump-reference, debug:config and the PHP config builders). + * + * @param array $config + */ + #[\Override] + public function getConfiguration(array $config, ContainerBuilder $container): Configuration + { + /** @var string $env */ + $env = $container->getParameter('kernel.environment'); + + return new Configuration($env); + } + /** * Configure the other bundles: Monolog handlers, migrations path, Messenger transport and security. * diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 0df0ebe..cdc97eb 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -46,8 +46,14 @@ protected function addSecuritySection(ArrayNodeDefinition $node): void ->children() ->arrayNode('security')->addDefaultsIfNotSet() ->children() - ->arrayNode('roles')->defaultValue(['ROLE_ADMIN'])->scalarPrototype()->end()->end() // Roles displayed inside user edit form - ->booleanNode('configure_firewall')->defaultTrue()->end() // add the process_user_provider user provider and configure the main firewall (see CleverAgeUiProcessExtension::prepend()), false to configure the security of the UI in the application + ->arrayNode('roles') + ->info('Roles that can be assigned to a user in the user edit form (ROLE_USER is always granted).') + ->defaultValue(['ROLE_ADMIN'])->scalarPrototype()->end() + ->end() + ->booleanNode('configure_firewall') + ->info('Add the "process_user_provider" user provider and configure the "main" firewall of the application (overriding its provider, login form and logout); false: configure the security of the UI in the application. Must be a literal boolean.') + ->defaultTrue() + ->end() ->end() ->end() ->end() @@ -61,12 +67,30 @@ protected function addLogSection(ArrayNodeDefinition $node): void ->children() ->arrayNode('logs')->addDefaultsIfNotSet() ->children() - ->booleanNode('store_in_database')->defaultValue(true)->end() // enable/disable store log in database (log_record table) - ->scalarNode('database_level')->defaultValue($defaultLevel)->end() // min log level to store log record in database - ->scalarNode('file_level')->defaultValue($defaultLevel)->end() // min log level to store log record in file - ->scalarNode('report_increment_level')->defaultValue(Level::Warning->name)->end() // min log level to increment process execution report - ->integerNode('database_max_days')->defaultNull()->min(1)->end() // days after which the log records are deleted from database by the purge command, null to keep them, can be overridden by process - ->integerNode('file_max_days')->defaultNull()->min(1)->end() // days after which the log files are deleted by the purge command, null to keep them, can be overridden by process + ->booleanNode('store_in_database') + ->info('Store the process logs in database (log_record table). The log file is always written.') + ->defaultValue(true) + ->end() + ->scalarNode('database_level') + ->info('Minimum Monolog level (name or value) of the log records stored in database (Debug in "dev" environment, else Info).') + ->defaultValue($defaultLevel) + ->end() + ->scalarNode('file_level') + ->info('Minimum Monolog level (name or value) of the log records written in the execution log file (Debug in "dev" environment, else Info).') + ->defaultValue($defaultLevel) + ->end() + ->scalarNode('report_increment_level') + ->info('Minimum Monolog level (name) from which a log record increments the execution report.') + ->defaultValue(Level::Warning->name) + ->end() + ->integerNode('database_max_days') + ->info('Days after which the log records are deleted from database by the cleverage:ui-process:purge-logs command, null to keep them. Can be overridden by process.') + ->defaultNull()->min(1) + ->end() + ->integerNode('file_max_days') + ->info('Days after which the log files are deleted by the cleverage:ui-process:purge-logs command, null to keep them. Can be overridden by process.') + ->defaultNull()->min(1) + ->end() ->end() ->end() ->end() @@ -79,7 +103,10 @@ protected function addDesignSection(ArrayNodeDefinition $node): void ->children() ->arrayNode('design')->addDefaultsIfNotSet() ->children() - ->scalarNode('logo_path')->defaultValue('bundles/cleverageuiprocess/logo.jpg')->end() + ->scalarNode('logo_path') + ->info('Path of the logo displayed in the dashboard, relative to the public directory.') + ->defaultValue('bundles/cleverageuiprocess/logo.jpg') + ->end() ->end() ->end() ->end() @@ -92,13 +119,21 @@ protected function addNotificationSection(ArrayNodeDefinition $node): void ->children() ->arrayNode('notification')->addDefaultsIfNotSet() ->children() - ->booleanNode('enabled')->defaultFalse()->end() // notify the end of the process executions (requires symfony/notifier), can be overridden by process - ->arrayNode('statuses') // ends of process executions to notify + ->booleanNode('enabled') + ->info('Notify the end of the process executions (requires symfony/notifier). Can be overridden by process.') + ->defaultFalse() + ->end() + ->arrayNode('statuses') + ->info('Ends of process executions to notify.') ->defaultValue([NotificationTrigger::Failed->value, NotificationTrigger::FinishWithReport->value]) ->enumPrototype()->values(NotificationTrigger::values())->end() ->end() - ->arrayNode('channels')->scalarPrototype()->end()->end() // notifier channels (e.g. "chat/slack", "email"), the channel policy of the notifier if empty - ->arrayNode('recipients') // the admin recipients of the notifier if empty + ->arrayNode('channels') + ->info('Notifier channels (e.g. "chat/slack", "email"); the channel policy of the notifier if empty.') + ->scalarPrototype()->end() + ->end() + ->arrayNode('recipients') + ->info('Recipients of the notifications; the admin recipients of the notifier if empty.') ->arrayPrototype() ->children() ->scalarNode('email')->defaultNull()->end() @@ -122,8 +157,14 @@ protected function addProcessExecutionSection(ArrayNodeDefinition $node): void ->children() ->arrayNode('process_execution')->addDefaultsIfNotSet() ->children() - ->integerNode('heartbeat_interval')->defaultValue(5)->min(1)->end() // seconds between two updates of the heartbeat of a running process execution (and checks of its stop request) - ->integerNode('stale_after')->defaultValue(900)->min(1)->end() // seconds without heartbeat after which a started process execution is considered as not responding + ->integerNode('heartbeat_interval') + ->info('Seconds between two updates of the heartbeat of a running process execution (and checks of its stop request).') + ->defaultValue(5)->min(1) + ->end() + ->integerNode('stale_after') + ->info('Seconds without heartbeat after which a started process execution is considered as not responding.') + ->defaultValue(900)->min(1) + ->end() ->end() ->end() ->end() diff --git a/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php b/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php index bab0091..528a3d5 100644 --- a/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php +++ b/tests/DependencyInjection/CleverAgeUiProcessExtensionTest.php @@ -22,11 +22,14 @@ use CleverAge\UiProcessBundle\Message\ProcessExecuteMessage; use CleverAge\UiProcessBundle\Notifier\NotificationTrigger; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException; +use Symfony\Component\Config\Definition\Processor; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Extension\Extension; +use Symfony\Component\DependencyInjection\ParameterBag\EnvPlaceholderParameterBag; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; #[CoversClass(CleverAgeUiProcessExtension::class)] @@ -77,6 +80,79 @@ public function testLoadWithDefaultConfiguration(): void ); } + /** + * @return iterable + */ + public static function provideInvalidLogLevels(): iterable + { + yield 'unknown name' => ['database_level', 'Verbose']; + yield 'unknown value' => ['file_level', 150]; + yield 'not a level' => ['file_level', true]; + yield 'empty' => ['file_level', '']; + yield 'report increment level value' => ['report_increment_level', 300]; + yield 'unknown report increment level' => ['report_increment_level', 'Loud']; + } + + #[DataProvider('provideInvalidLogLevels')] + public function testInvalidLogLevel(string $option, mixed $level): void + { + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage(\sprintf('Invalid configuration for path "clever_age_ui_process.logs.%s": invalid Monolog level %s, use one of: DEBUG, INFO, NOTICE, WARNING, ERROR, CRITICAL, ALERT, EMERGENCY.', $option, json_encode($level))); + + (new CleverAgeUiProcessExtension())->load([['logs' => [$option => $level]]], $this->createContainer('prod')); + } + + /** + * @return iterable + */ + public static function provideValidLogLevels(): iterable + { + yield 'name' => ['database_level', 'Error']; + yield 'lower case name' => ['file_level', 'error']; + yield 'value' => ['file_level', 400]; + yield 'numeric string' => ['database_level', '400']; + yield 'report increment level, upper case name' => ['report_increment_level', 'ERROR']; + } + + #[DataProvider('provideValidLogLevels')] + public function testValidLogLevel(string $option, int|string $level): void + { + $container = $this->createContainer('prod'); + (new CleverAgeUiProcessExtension())->load([['logs' => [$option => $level]]], $container); + + // Passed as is to the handlers + $fileHandler = $container->getDefinition('cleverage_ui_process.monolog_handler.process'); + $configured = match ($option) { + 'database_level' => $container->getDefinition('cleverage_ui_process.monolog_handler.doctrine_process')->getArgument('$level'), + 'file_level' => $fileHandler->getArgument('$level'), + default => $fileHandler->getMethodCalls(), + }; + self::assertSame('report_increment_level' === $option ? [['setReportIncrementLevel', [$level]]] : $level, $configured); + } + + /** + * Environment variables are resolved at runtime: not validated when the container is compiled. + */ + public function testLogLevelFromAnEnvironmentVariable(): void + { + $container = $this->createContainer('prod', new EnvPlaceholderParameterBag()); + $placeholder = $container->getParameterBag()->get('env(UI_LOG_LEVEL)'); + self::assertIsString($placeholder); + + (new CleverAgeUiProcessExtension())->load([['logs' => ['file_level' => $placeholder, 'report_increment_level' => $placeholder]]], $container); + + self::assertSame($placeholder, $container->getDefinition('cleverage_ui_process.monolog_handler.process')->getArgument('$level')); + } + + public function testGetConfigurationUsesTheKernelEnvironment(): void + { + $configuration = (new CleverAgeUiProcessExtension())->getConfiguration([], $this->createContainer('dev')); + + /** @var array{logs: array{file_level: string}} $config */ + $config = (new Processor())->processConfiguration($configuration, []); + self::assertSame('Debug', $config['logs']['file_level']); + } + public function testLoadWithNotificationConfiguration(): void { $container = $this->createContainer('prod'); @@ -293,15 +369,17 @@ public function testPrependedFrameworkConfigurationComesFirst(): void self::assertSame(['secret' => 'app'], $framework[1]); } - private function createContainer(string $env): ContainerBuilder + private function createContainer(string $env, ParameterBag $parameterBag = new ParameterBag()): ContainerBuilder { - return new ContainerBuilder(new ParameterBag([ + $parameterBag->add([ 'kernel.environment' => $env, 'kernel.debug' => false, 'kernel.project_dir' => sys_get_temp_dir(), 'kernel.logs_dir' => sys_get_temp_dir().'/logs', 'kernel.cache_dir' => sys_get_temp_dir().'/cache', - ])); + ]); + + return new ContainerBuilder($parameterBag); } private function createExtension(string $alias): Extension diff --git a/tests/DependencyInjection/ConfigurationTest.php b/tests/DependencyInjection/ConfigurationTest.php index b04ddc2..1038d06 100644 --- a/tests/DependencyInjection/ConfigurationTest.php +++ b/tests/DependencyInjection/ConfigurationTest.php @@ -206,6 +206,31 @@ public static function provideInvalidConfigurations(): iterable ]; } + /** + * @return iterable + */ + public static function provideValidLevels(): iterable + { + yield 'name' => ['database_level', 'Error']; + yield 'lower case name' => ['file_level', 'error']; + yield 'upper case name' => ['database_level', 'ERROR']; + yield 'value' => ['file_level', 400]; + yield 'numeric string' => ['database_level', '400']; + yield 'report increment level, lower case name' => ['report_increment_level', 'error']; + } + + /** + * Levels kept as is (validated by the extension, see CleverAgeUiProcessExtensionTest). + */ + #[DataProvider('provideValidLevels')] + public function testValidLevel(string $option, int|string $level): void + { + $config = $this->process('prod', ['logs' => [$option => $level]]); + + self::assertIsArray($config['logs']); + self::assertSame($level, $config['logs'][$option]); + } + /** * @param array $config */