9:20 a.m.
On 02/23/2016 09:17 PM, Jakub Hrozek wrote:
On Tue, Feb 23, 2016 at 05:06:57PM +0100, Michal Židek wrote:
> On 02/22/2016 09:21 AM, Jakub Hrozek wrote:
>> On Fri, Feb 19, 2016 at 04:35:38PM +0100, Michal Židek wrote:
>>> Thank you for comments! See my response.
>>>
>>> On 02/19/2016 12:18 PM, Jakub Hrozek wrote:
>>>> On Thu, Feb 18, 2016 at 07:00:53PM +0100, Michal Židek wrote:
>>>>> Hi!
>>>>>
>>>>> This is the WIP design stub for config file checks:
>>>>>
https://fedorahosted.org/sssd/wiki/DesignDocs/libini-config-file-checks
>>>>>
>>>>> For the first version we just want to
>>>>> have the typo detection mechanism. To be more
>>>>> precise, we want to detect:
>>>>> 1. options with wrong names
>>>>> 2. options in wrong sections
>>>>>
>>>>> For the more complicated checks in later
>>>>> version I would like to add possibility
>>>>> to define constraints for the config file.
>>>>>
>>>>> The idea is also described in the design stub.
>>>>>
>>>>> For the definitions of constraints, we can use INI format
>>>>> as well and not the custom format that I use now.
>>>>> However the custom format is quite simple and
>>>>> looks better IMO and allows to parse different
>>>>> attributes in different way which results in less
>>>>> verbose and better formatted file. However as I said it
>>>>> is not required to get the functionality.
>>>>
>>>> Well, using the INI format would have the advantage of having the parser
>>>> ready already. It feels a bit backwards to me to configure the INI
>>>> parser with another file format.
>>>
>>> Ok. I had the same feeling. In the earlier version I had
>>> it was not possible to use INI format for the rules, but I
>>> simplified it to the current form were it is. I will
>>> continue with the INI format.
>>>
>>>>
>>>> What about having the allowed options per section, like this?
>>>
>>> Having per section allowed options would save some
>>> typing for options that can be in the same sections.
>>> In the custom format I do this using inheritance from
>>> previous line, but this is not possible with the INI
>>> format.
>>>
>>> Also I wanted to have the 'section' value a regular expression
>>> so that it can be written for subsections like this
>>>
>>> domain/.*
>>>
>>> Because actually we use subsections for domain definitions and
>>> we can not predict how the domain will be called. Regexes seem
>>> to be flexible and easy to use in this situation.
>>
>> Right, but should we care in the parser? I man, the domain name is just
>> a label (with the exception of local, but we can remove that..)
>>
>
> My point was different. The name of the section (the regular
> expression, that will match sections in configuration file)
> cannot be specified in the file with constraints as a section
> name. So we still need to have the
>
> [alowed_options/domains]
> section = domain/.* // this is still needed
> option = value
> option = value
>
> or this
>
> [allowed_options/domains]
> option = section; value
> option = section; value
> ^^^^^^^ or this is needed
I don't mind this format too much, are you concerned it's too much
typing?
Yes. Because the section value will be the same for all options in the
same section and technically it is a regular expression. We may have
a section like this:
[allowed_options/service_common]
section = regex_that_describes_all_services
option = value
option = value
option = value
or
[allowed_options/service_common]
option = regex_that_describes_all_services, value
option = regex_that_describes_all_services, value
option = regex_that_describes_all_services, value
Now if you want to add a new service, you need to modify the
regex. It is better to have it on one place. Also
with one such regex per section, it is IMO visually
more readable.
>
> I forgot to mention the main disadvantage of the first format. The 'section'
> would be a reserved option. For SSSD it is OK, but in
> general applications can have option called 'section'.
>
> But we can solve this by having a special section (let's say [config])
> with option called section_specifier that could override this name and
> allow 'section' as name for application's own option:
>
> [config]
> section_specifier = in_section
This is OK to me, too, but feels a bit...magic :-)
Ok, I will go with it for now.
>
> [allowed_options/domains]
> in_section = domain/.*
> option = value
> option = value
>
> 'section' would be the default value for section_specifier.
>
>
>> I'm wondering if we can use the domain name for provider type?
>>
>> [allowed_options/domain/id/ad]
>> ...
>>
>> [allowed_options/domain/ipa/access]
>> ...
>>
>> IIRC we already use something like this in the Python config API.
>
> As I said already, this should be IMO done using the rules and not
> allowed_options. The allowed_options should be just very stupid
> list that is easy to extend without much thinking.
>
>>
>> (Also, do we need the "allowed_" prefix? Will we ever list other
options
>> than allowed?)
>
> Yes, we probably do not need that "allowed_" prefix.
>
>>
>>>
>>> To have per section allowed_options we could use
>>> subsections so that there are no conflicts, for example
>>> [allowed_options/pam]
>>>
>>> But it is not possible to use regular expression in the
>>> subsection name for example this is not possible
>>> [allowed_options/domain/.*]
>>>
>>> However we could use something like this:
>>>
>>> [allowed_options/domains]
>>> section = domain/.*
>>> option = value
>>> option = value
>>
>> The option=value format looks fine to me
>>
>>> ...
>>>
>>> instead of:
>>>
>>> [allowed_options]
>>> option = section; value
>>> option = section; value
>>> ...
>>>
>>> Would that be OK?
>>>
>>>>
>>>> [allowed_options]
>>>> section = domain
>>>> apply_if_exists = id_provider; ad
>>>> apply_if_exists = ldap_id_mapping; false
>>>> apply_contraint = all
>>>> option = value
>>>> option = value
>>>> option = value
>>>
>>> Also I would like to not mix allowed_options with the rules. That
>>> would make allowed_options very difficult IMO. allowed_options
>>> should only list all options that are allowed together with
>>> section (no sections means any section) and value (no value
>>> means any value). To get the typo detection and detection
>>> of misplaced options.
>>>
>>> If some options are allowed only with ad provider and ldap_id_mapping
>>> false, that should be written using a rule like this:
>>>
>>> [rules/only_ad_provider_id_mapping_false]
>>> scope = domain/.*
>>> apply_if_exists_any = here will be list of option-section-value triples that
>>> are allowed only with id_provider ad and id_mapping false
>>> fail_if_miss_any = id_provider, ,ad; ldap_id_mapping, ,false
>>> ^^^ ^^^
>>> The section here can be left blank because the scope was set. Any
>>> section specified here would actually be subsection of the section
>>> in scope.
>>
>> OK, at this point we 'just' need to write the design in an extensible
>> way so that we don't shoot ourselves in the foot later.
>>
>> btw I was also thinking about merging this with the configAPI. It
>> doesn't have to be for the next upstream release, but I already dislike
>> that we have maybe 4 places you need to change when adding a new option,
>> it would be great to eventually converge on one..and since the configAPI
>> essentially serves the same purpose as this INI schema you're adding it
>> looks like a nice first place to integrate..
>
> Yes, we will probably be able to stop using configAPI when rules
> are implemented.
We can't stop using configAPI, but we can use the INI schema as its
input :-)
Yes, that is what I meant :)
Michal