On Sun, 2014-04-06 at 01:00 -0400, Dmitri Pal wrote:
On 04/06/2014 12:11 AM, Simo Sorce wrote:
> On Fri, 2014-04-04 at 18:25 -0400, Dmitri Pal wrote:
>> Ah you mean the two conditions:
>>
>> - all files must start with a valid section header, i.e. each file has
>> to be valid ini file
>> - duplicated section names are not allowed, i.e. no merging of any
>> kind
>> will be done
>>
>> OK so what you are suggesting is to try to avoid .merge files.
>> We can check that snippets start with a valid section.
>> We can check that the section name is not a dup but without .merge
>> file
>> we can't specify what list this snippet should be merged into.
>> I mean if the snippet creates a new provider or domain then it will
>> be
>> in config but the key that lists the domains or provider would not be
>> update and this this section would not be used by the SSSD logic.
>> There are sections that can just be dropped in this is why */mergesec
>> and /**/mergekey /*are optional but they are not the main use case.
> I have caught up with this proposal only now, so I am sorry for jumping
> in late, but I see no use for merging configurations in the sense of
> combining values of a sections together from 2 files without explicit
> include files.
>
> In general it is, in general, a bad idea to try to merge arbitrary
> configurations from 2 files because it is hard to mentally visualize
> what will be the result and errors where sections have the same name by
> mistake will cause very hard to diagnose issues.
>
> I think in abstract it is an interesting idea, but in actual usage it
> may be more pain than gain.
>
> Include files are a different proposition altogether they are not full
> sections, and can be ordered with regard to existing data from the
> inclusion file so to control what gets included before/after and have an
> idea of what will take precedence.
> They are also generally manually controlled by admins, although we do
> abuse include files in krb5.conf with sssd...
>
>> For SSSD they actually be sort of required.
> The only reason you'd need to do this for sssd is to set the domain list
> in the [global] section where not only you need to merge directives, you
> actually have to "merge" the values of a particular key.
> This is honestly ... madness :-)
>
> I think the solution here is to change the way sssd behaves by
> augmenting the domain list by adding a key in the domain sections to
> mark the enabled/disabled. The order of the domains not explicitly added
> to the global list that are explicitly enabled would depend on the order
> they are declared. Problem solved, no more merging needed.
>
>> What I referred to being insecure is the fact that if you have two
>> applications X and Y being able to add augmentation to the same App
>> does not mean that you trust them the same way. If you have one top
>> level directory and the drop snippets in the common .d directory they
>> can also mess with each other files. If they are always forced to
>> their own sub-dir we can use SELinux for making sure one application
>> would not mess with the config file of another application. IMO
>> without this we are susceptible to virus kind of attacks. This is why
>> I see it as insecure.
> I think this goes too far, normally generic applications cannot drop
> configurations files 'live', usually the stuff that drop configuration
> snippets is either the package manager (implicitly trusted, as it can
> change *anything*) or it is a highly privileged management tool that
> is also trusted by the admins.
> In general I cannot think of a case where you would ever allow a "less"
> trusted application to change main configuration data of a higher
> privileged app. In those cases you would probably always manage
> configurations separately with explicit invocations of the configuration
> library on the specific files, and careful filtering of the inputs under
> much stricter rules.
>
> In short I think you can keep things a lot simpler and achiever useful
> results faster by imposing better restrictions on the architecture that
> will actually make the library better for admins, even thought it would
> be theoretically less flexible.
>
> Simo.
>
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).
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.
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.
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 ?
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.
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).
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.
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.
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 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 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).
Simo.
--
Simo Sorce * Red Hat, Inc * New York