Enhanced config test semantics
- Dominant language
- Java
- Stars
- 14.9k
- Forks
- 3.5k
- Avg merge
- 19h 14m
- Merged PRs (30d)
- 63
Description
In the current logstash codebase we use a [config mixin](https://github.com/elastic/logstash/blob/master/logstash-core/lib/logstash/config/mixin.rb) as the way to handle plugin configuration, this mixin is the responsible of (_short version_):
- Parameter validation, making sure that for example required parameters are there.
- Value validation where mostly checks are performed to be sure that the right data type is used, some might include a degree of semantics verification.
- Value coercion, this phase converts the initial value to the right expected one, for example when we deal with passwords.
This class has lived a long life since the beginning of logstash, well @jordansissel might provide here his historician background for sure, but certainly it has it's difficulties for:
- Maintenance, adding new features requires to understand a complex structure that has intrinsic relations between the different methods and responsibilities.
- Testing, because of the way code is structured makes tests require an initial setup making them hard to compose.
In this issue I propose a strategy to enhance this module that will provide:
- A cleaner interface and separation of concerns into different objects responsible of parameters and validation, coercion, etc.
- An easier interface for testing by decoupling the mixin from the actual code responsible of the different tasks.
- An small cleanup of old features like, `Regexp` , `milestones`, etc.
- An extension to the current validators:
- to let users have custom classes and Procs/Blocks, basically enabling a handy extension point for more semantic validation.
- Perform cross validations, currently being done in the [register method](https://github.com/logstash-plugins/logstash-input-jdbc/blob/master/lib/logstash/inputs/jdbc.rb#L208) when necessary.
### Validators and Coercers
In the new validation mechanism I propose to break out the config initialization into:
- Validators, simple set of classes responsible of making sure that a required parameter exist, a value has the right type, etc.
- Coercers, another simple set of classes basically responsible of converting config values to the right type.
A validator might be as simple as:
``` ruby
module LogStash::Config
module TypeValidators
class Ipaddr < Abstract
def valid?(value)
octets = value.split(".")
if octets.length != 4
add_errors "Expected IPaddr, got #{value.inspect}"
return false
end
octets.each do |o|
if (o.to_i < 0 || o.to_i > 255)
add_errors "Expected IPaddr, got #{value.inspect}"
return false
end
end
end
end
end
end
```
Validators could be:
- A Symbol, the former method that will basically pull a logstash-core validator for it.
- A Proc, or anything that respond to call like lambda.
- A custom class provided by the plugin.
Coercer are like:
``` ruby
module LogStash::Config
module TypeCoercers
module Boolean
def self.coerce(value)
return value if [TrueClass, FalseClass].include?(value.class)
return true if value =~ (/^(true|t|yes|y|1)$/i)
return false if value.empty? || value =~ (/^(false|f|no|n|0)$/i)
return value
end
end
end
end
```
This two concepts will enable the logstash-core and plugin ecosystem with an easy to extend mechanism to handle the plugin config.
With this, and a few more concepts, the initial config check up end up being simple as:
``` ruby
@registry = LogStash::Config::InternalRegistry.new(self, params, @logger)
@registry.setup do |validator, _params|
valid, _ = validator.valid_params?(_params)
if !valid
raise LogStash::ConfigurationError, I18n.t("logstash.runner.configuration.invalid_plugin_settings")
end
valid, errors = validator.valid_values?(_params)
if !valid
raise LogStash::ConfigurationError, errors.join('\n')
end
validator.coerce_values!(_params)
end
# now that we know the parameters are valid, we can obfuscate the original copy
# of the parameters before storing them as an instance variable
@registry.secure_params!(original_params)
```
where the `validator` object will be the responsible of performing:
- the parameter validation, to be sure all required params are there.
- the value verification, to be sure all values are ok.
- and last the value coercion, so all data types are in order.
### Benefits
With an strategy like this we will enable:
- An easy time doing maintenance and testing the configuration verification.
- An extension point for plugins to add semantic verification, for example checking if a date pattern for joda in the logstash-filter-date is correct before starting the plugin.
The latest is very interesting and is thanks to the fact that now we might have other type of validators, not just the predefined ones.
### Follow-up
The changes proposed in this issue are big, involving changing important logstash-core parts, so as agreed with @jsvd and @jordansissel, to facilitate a code based discussion I will open a PR where we can have a proper discussion [PR link].
[This](https://github.com/purbon/logstash-filter-date/tree/feature/extended_validation) is an example of how the logstash-filter-date will look like using the new config mixin, the important change is the introduction of the new MatchValidator where the config verification will make sure that the match field:
- Have the proper format, is required to be an array and be of at least two elements.
- The joda pattern used is actually a valid one.
This two verifications are done now as part of the register method, so not run when a user does a config verification. With the changes proposed in this issue, plugin authors will have the option to add more semantic power to the config validation, so users will have a powerful verification mechanism for configs.
Related with #3297 #3199 #2901 #2437 #2325 #2074 #1609 #3301
Contributor guide
Assessment
This issue has not been assessed yet.