[SSSD] [INI] Ding-lIbs - Merging Configuration Snippets Installed in Different Locations (Design page)

Dmitri Pal dpal at redhat.com
Mon Apr 7 17:55:23 UTC 2014


On 04/07/2014 08:37 AM, Simo Sorce wrote:
>> I disagree with several points here, sorry.
>> >
>> >1) I agree that it would have been better if sssd.conf had a different
>> >semantics about the domains. But the situation right now is that the
>> >domains are listed. I agree we can go to your model but in reality there
>> >then will be more code and more logic in SSSD to actually identify which
>> >sections are domains and which are not.
> This is very easy, it's a string comparison on the name all domains
> start with domain/, but we can also add a 'type' key if people feel it
> is really necessary (I don't).

This is SSSD an specific change. The INI should be generic. It does not 
know anything about domains.

>
>> >  I am proposing to make it a part
>> >of the library so that we implement it once and SSSD does not need to
>> >change and write more custom code. The amount of code and complexity
>> >will be same it is just where you put it.
>> >
>> >Another thing is that domains is not the only option the provider list
>> >is constructed the same way. There is now two things that you have to
>> >change in the sssd.conf. IMO it is too much.
> I think we'll have to disagree, it is not a big deal on SSSD's side to
> change that behavior, and I retrospect I believe we should have done it
> that way from the start, exactly to make it easier to include sections
> from different files. So I would like to take the chance to do it now.

We can force SSSD to fix that. But this would mean a syntax change in 
sssd.conf. This would require new version of the config to be added to 
SSSD code. While I agree that we should not have done this in SSSD IMO 
we should leave SSSD as is and focus on making INI provide functionality 
that will be consumed by SSSD without additional changes. This is in 
fact the design goal of this proposal.
If we take it as a design goal the change can be made in couple months 
in INI. If we require SSSD to change it would take much more time to 
implement.
IMO it is not worth it.

>
>> >2) Include files are bad because they require you to modify the main
>> >config file. The use case I want to be able to support is that you can
>> >add domains without touching the main configuration file. Say you have a
>> >base image that you deploy and then you have some snippets that you drop
>> >in when you deploy things. Whether the configuration snippets are
>> >dropped in by an application or they are dropped by config tool like
>> >Puppet or they are created as part of Kickstart/Cloud-init or some other
>> >scripts run by orchestration software does not matter. What matters is
>> >that they should extend the configuration without touching the main
>> >file. That eliminates the include file approach altogether.
> I was not proposing to use include files, although I think they are a
> powerful tool that should not be dismissed.

include derectives do not fit into the concepts of a ini file which is 
KVP broken into sections.
It adds special syntax, special parsing etc.
It also implies that someone have to add the include directive into the 
main config file. And it is hard to do it automatically. Another problem 
is that it would be position dependent as the file with includes will be 
treated as a single stream of configuration (you just dive into the 
included files recursively) with this you loose the control that you can 
get with the explicit merge.

Keep in mind that merge is already possible in INI. The whole proposal 
is about starting to use thins functionality not about actually building it.

>
>> >3) The question about merges comes again and again and IMO it is the
>> >time to get serious about it.
>> >I explicitly said that there are several rules that would prevent issues
>> >that you are concerned about.
>> >a) There is not overwrite between snippets so any collision would cause
>> >an error. It will be recorded and snippet will be ignored.
> Nod
>
>> >b) We can force the snippets to have sections though strictly it is not
>> >required but we can do it if people think that it is 100% necessary.
> What would do you do if there is no section in sssd's case ?
> Where would directives end up ?

If there is no section defined and there is not .merge file that 
corresponds to this snippet with section key defined then ther snippet 
will be ignored and an error will be logged.
...

section = <name>

*/section/*- optional, default name of the section
  to use if the file does not have a section defined and just has KVPs as
  in the example above.

The rule here is that the .merge file is mandatory so the snippet itself 
might be just KVPs.
A bit less of work for any script becuase you do not need to generate 
the section line, just put KVPs both in snippet and .merge file.




>
>> >c) There was a use case when person actually wanted to replace a
>> >section. In case of SSSD we probably do not want that but library should
>> >allow it. Merge logic already supports this so it is a question of a
>> >flag passed to an existing call - no extra code or complexity.
>> >In case of SSSD the rule would be (at least by default for now) that
>> >sections can only be unique and can can be added with no overwrite. Any
>> >colliding snippet that collides with a section in the main file will be
>> >recorded and ignored because SSSD should be able to start and work
>> >without the augmentations.
> Nod
>
>> >d) I actually think that merging values, i.e. adding a value to the list
>> >like in the list of the domains or providers is not a bad thing. It will
>> >allow us to avoid interpreting what each section is about by
>> >introspecting it. It will also avoid having stale sections that actually
>> >do not have any meaning or value. Moving forward when we have syntax
>> >validation we will be able to define and validate that the desired key
>> >and section are actually a list that is extensible. I had this in mind
>> >but this is outside of scope of the current proposal.
> I do not understand how it helps, for the specific sssd case I think it
> only has downsides.
>
> a) You have to define an additional [global] section in order to define
> the domain list with the snippet you are deploying, which means you have
> to special case the global section and allow section merging, but
> disallow it for everything else.
No I think merge logic can check that

mergesec = <section>
mergekey = <key>

are not present in the merge set, if any of snippets tries to overwrite 
this section then it will be an error. It has nothing to do with special 
casing, it can be a generic rule enforced by the library.
It actually makes perfect sense to check and enforce.
I would need to add this to the design.
Very good point, thanks for bringing it up!


> b) An additional global section can end up defining other values we'd
> rather not merge from a snippet, but you wouldn't know how to
> "blacklist" values unless we deploy yet another meta-configuration
> file(I am strongly opposed to meta-configuration files).

No black listing. This is in fact will lead to insanity.
The rule is simple if a set of snippets contains a Key that needs 
merging of values into this single list then the same set of snippets 
can't contain a snippet that updates that section. I think it should be 
treated as a global misconfiguration, reported and no snippets should be 
applied in this case.

> c) it will be harder for an admin to visualize how to enable/disable a
> domain.
>
> A key called enabled = yes|no*in*  the section, would avoid all 3 above,
> no need to merge global sections, no additional values, and easier to
> understand if a domain is enabled or not.
>
> And the same is for the providers.

Admin can just drop and snippet and follow the rules defined above.
I think it would be easy to visualize.
I think enable/disable is a good idea but it is for SSSD to implement. 
INI does not have a say here.


>
>> >Bottom line:
>> >We have use case that require merges, we have SSSD with its semantics.
>> >The proposal covers both cases. If you think that sub dirs is a bad is a
>> >bad idea I will budge but I think it is a mistake.
> I think a conf.d subdir is a great idea.
You mean subdirectories under conf.d dir so that snippets from one 
source do not have access to snippets from another source and can be 
sort of jailed in that subdirectory? Is that what you mean?

> But I think merging sections is a bad idea, I would prefer to have only
> whole sections defined in conf.d and conflicts always be ignored w/o any
> exception for [global].

I agree. Absolutely and I think the design does exactly that. Each 
snippet is treated as section, though its name can be recorded in the 
corresponding .merge file.
And as I described above there is not need to black list anything.

>
> I think merging values is generally a pretty bad idea, because you have
> no place to see what the final resulting value is except by cranking up
> sssd's log level and making it spit out the resulting config value.

I agree that merging values is generally bad. This is why we will check 
sections for collisions.
But I think that adding a value to the list is not that bad.

> I strongly prefer adding an enabled = yes|no parameter (it would be
> optional for backwards compatibility for the main config file, and
> required for the snippets).

This is up to sssd.

However you forget something. The change you propose would require 
significant semantic changes to the config file.
It would require a different traversal algorythim related to the 
configuration in SSSD. Right now one has to just get the list of domains 
or providers from a specific section and a specific key (pseudocode):

Value = getKeyForSection(Section,Key)
List = convertToList(Value)

for each Item in List
    Domain = getSection(Item)
    processDomain(Domain)

In the case of the approach you suggest

Value = getKeyForSection(Section,Key)
List = convertToList(Value)

Sections = getListOfSections(Config)

for each Sec in Section
     If Sec is Domain
         LIst+= Sec

for each Item in List
    Domain = getSection(Item)
    processDomain(Domain)


So you would have to parse the config and examine each an every section 
to build the list anyways. But INI will do it for you. The benefits are 
obvious:
a) You do not need to change SSSD and its logic of processing data
b) Same approach can be used in other applications and it is OK to 
follow SSSD example :-) . INI will do the change once and it can be reused.

And keep in mind that the change to merge values will be to the in 
memory object not to the text of the INI file itself, that would be 
insane for sure :-)


>
> Simo.


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager IdM portfolio
Red Hat, Inc.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20140407/366b2605/attachment.html>


More information about the sssd-devel mailing list