Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Configuration Validation #329

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[WIP] Configuration Validation #329

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0826814
[WIP]
DonCallisto Oct 20, 2019
88a5a3b
[WIP] Changes after review
DonCallisto May 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
224 changes: 219 additions & 5 deletions DependencyInjection/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@
*
* - fingers_crossed:
* - handler: the wrapped handler's name
* - [action_level|activation_strategy]: minimum level or service id to activate the handler, defaults to WARNING
* - [action_level|activation_strategy]: minimum level or service id to activate the handler, defaults to null
* - [excluded_404s]: if set, the strategy will be changed to one that excludes 404s coming from URLs matching any of those patterns
* - [excluded_http_codes]: if set, the strategy will be changed to one that excludes specific HTTP codes (requires Symfony Monolog bridge 4.1+)
* - [buffer_size]: defaults to 0 (unlimited)
Expand Down Expand Up @@ -328,6 +328,105 @@
*/
class Configuration implements ConfigurationInterface
{
private $acceptedParamsByHandlerType = [
'stream' => ['path', 'level', 'bubble', 'file_permission', 'use_locking'],
'console' => ['verbosity_levels', 'level', 'bubble', 'console_formater_options'],
'firephp' => ['bubble', 'level'],
'browser_console' => ['bubble', 'level'],
'gelf' => ['publisher', 'bubble', 'level'],
'chromephp' => ['bubble', 'level'],
'rotating_file' => ['path', 'max_files', 'level', 'bubble', 'file_permission', 'filename_format', 'date_format'],
'mongo' => ['mongo', 'level', 'bubble'],
'elasticsearch' => ['elasticsearch', 'index', 'document_type', 'level', 'bubble'],
'redis' => ['redis'],
'predis' => ['redis'],
'fingers_crossed' => [
'handler',
'action_level',
'activation_strategy',
'excluded_404s',
'excluded_http_codes',
'buffer_size',
'stop_buffering',
'passthru_level',
'bubble'
],
'filter' => ['handler', 'accepted_levels', 'min_level', 'max_level', 'bubble'],
'buffer' => ['handler', 'buffer_size', 'level', 'bubble', 'flush_on_overflow'],
'deduplication' => ['handler', 'store', 'deduplication_level', 'time', 'bubble'],
'group' => ['members', 'bubble'],
'whatfailuregroup' => ['members', 'bubble'],
'syslog' => ['ident', 'facility', 'logopts', 'level', 'bubble'],
'syslogudp' => ['host', 'port', 'facility', 'logopts', 'level', 'bubble', 'ident'],
'swift_mailer' => [
'from_email',
'to_email',
'subject',
'email_prototype',
'content_type',
'mailer',
'level',
'bubble',
'lazy'
],
'native_mailer' => ['from_email', 'to_email', 'subject', 'level', 'bubble', 'headers'],
'socket' => ['connection_string', 'timeout', 'connection_timeout', 'persistent', 'level', 'bubble'],
'pushover' => ['token', 'user', 'title', 'level', 'bubble', 'timeout', 'connection_timeout'],
'raven' => ['dsn', 'client_id', 'release', 'level', 'bubble', 'auto_log_stacks', 'environment'],
'sentry' => ['dsn', 'client_id', 'release', 'level', 'bubble', 'auto_log_stacks', 'environment'],
'newrelic' => ['level', 'bubble', 'app_name'],
'hipchat' => [
'token',
'room',
'notify',
'nickname',
'level',
'bubble',
'use_ssl',
'message_format',
'host',
'api_version',
'timeout',
'connection_timeout'
],
'slack' => [
'token',
'channel',
'bot_name',
'icon_emoji',
'use_attachment',
'use_short_attachment',
'include_extra',
'level',
'bubble',
'timeout',
'connection_timeout'
],
'slackwebhook' => [
'webhook_url',
'channel',
'bot_name',
'icon_emoji',
'use_attachment',
'use_short_attachment',
'include_extra',
'level',
'bubble'
],
'slackbot' => ['team', 'token', 'channel', 'level', 'bubble'],
'cube' => ['url', 'level', 'bubble'],
'amqp' => ['exchange', 'exchange_name', 'level', 'bubble'],
'error_log' => ['message_type', 'level', 'bubble'],
'null' => ['level', 'bubble'],
'test' => ['level', 'bubble'],
'debug' => ['level', 'bubble'],
'loggly' => ['token', 'level', 'bubble', 'tags'],
'logentries' => ['token', 'use_ssl', 'level', 'bubble', 'timeout', 'connection_timeout'],
'insightops' => ['token', 'region', 'use_ssl', 'level', 'bubble'],
'flowdock' => ['token', 'source', 'from_email', 'level', 'bubble'],
'server_log' => ['host', 'level', 'bubble'],
];

/**
* Generates the configuration tree builder.
*
Expand Down Expand Up @@ -774,6 +873,10 @@ public function getConfigTreeBuilder()
->scalarNode('formatter')->end()
->booleanNode('nested')->defaultFalse()->end()
->end()
->beforeNormalization()
->always()
->then(function ($v) { return $this->handlerTypeAcceptedOptionsValidation($v); })
->end()
->validate()
->ifTrue(function ($v) { return 'service' === $v['type'] && !empty($v['formatter']); })
->thenInvalid('Service handlers can not have a formatter configured in the bundle, you must reconfigure the service itself instead')
Expand All @@ -794,10 +897,6 @@ public function getConfigTreeBuilder()
->ifTrue(function ($v) { return 'fingers_crossed' === $v['type'] && !empty($v['excluded_http_codes']) && !empty($v['excluded_404s']); })
->thenInvalid('You can not use excluded_http_codes together with excluded_404s in a FingersCrossedHandler')
->end()
->validate()
->ifTrue(function ($v) { return 'fingers_crossed' !== $v['type'] && (!empty($v['excluded_http_codes']) || !empty($v['excluded_404s'])); })
->thenInvalid('You can only use excluded_http_codes/excluded_404s with a FingersCrossedHandler definition')
->end()
->validate()
->ifTrue(function ($v) { return 'filter' === $v['type'] && "DEBUG" !== $v['min_level'] && !empty($v['accepted_levels']); })
->thenInvalid('You can not use min_level together with accepted_levels in a FilterHandler')
Expand Down Expand Up @@ -963,4 +1062,119 @@ public function getConfigTreeBuilder()

return $treeBuilder;
}

private function handlerTypeAcceptedOptionsValidation(array $v)
{
if (!array_key_exists('type', $v)) {
return $v;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should rather throw if type is missing? AFAIK there is no valid handler config without a type. Same below if the type is unknown I'd say throw instead of returning a valid value

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By returning $v, I let other validations to take place. Here I'm validating only if handler type has correct options, so I think it pretty safe to return $v directly. WDYT?

}

if (!array_key_exists(strtolower($v['type']), $this->acceptedParamsByHandlerType)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly to the note below, strtolower here and at line 1080 probably should be removed.. types were not case insensitive until now?

return $v;
}

// @todo array_keys should be converted to lowercase?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so? I don't think there is an expectation that this stuff is case insensitive generally speaking, but I might be mistaken.

$acceptableParamsForHandlers = array_intersect($this->provideAllConfigurationParams(), array_keys($v));
$unacceptableParamForHandler = array_diff(
$acceptableParamsForHandlers,
$this->acceptedParamsByHandlerType[strtolower($v['type'])]
);
if (!empty($unacceptableParamForHandler)) {
throw new InvalidConfigurationException(sprintf(
'The handler type %s does not support %s %s',
strtolower($v['type']),
implode(', ', $unacceptableParamForHandler),
count($unacceptableParamForHandler) == 1 ? 'parameter' : 'parameters'
));
}

return $v;
}

private function provideAllConfigurationParams()
{
return [
'accepted_levels',
'action_level',
'activation_strategy',
'api_version',
'app_name',
'auto_log_stacks',
'bubble',
'bot_name',
'buffer_size',
'channel',
'client_id',
'config',
'connection_string',
'connection_timeout',
'console_formater_options',
'content_type',
'date_format',
'deduplication_level',
'document_type',
'dsn',
'elasticsearch',
'email_prototype',
'environment',
'exchange',
'exchange_name',
'excluded_404s',
'excluded_http_codes',
'facility',
'filename_format',
'file_permission',
'flush_on_overflow',
'from_email',
'handler',
'headers',
'host',
'icon_emoji',
'id',
'ident',
'include_extra',
'index',
'lazy',
'level',
'logopts',
'mailer',
'max_files',
'max_level',
'message_format',
'message_type',
'min_level',
'members',
'mongo',
'nickname',
'notify',
'path',
'passthru_level',
'persistent',
'port',
'publisher',
'redis',
'release',
'region',
'room',
'source',
'stop_buffering',
'store',
'subject',
'tags',
'team',
'time',
'timeout',
'title',
'token',
'to_email',
'url',
'user',
'use_attachment',
'use_locking',
'use_short_attachment',
'use_ssl',
'verbosity_levels',
'webhook_url',
];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be generated by recursively merging $this->acceptedParamsByHandlerType instead? So it doesn't need to be kept up to date, because I'm pretty sure it'll get outdated otherwise.

}
}
Loading