[sssd PR#226][opened] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: opened

PR body:
"""
Some updates for src/config/cfg_rules.ini

The most controversial is the third patch. It removes the special rule for application domains and only uses the rule for normal domains in both application and normal domains. The reason is that the validator ini_allowed_options checks all sections that match the regex in section_re and allows only listed options. This is done for all rules that use that validator *separately and there is not 'include' directive or anything like that. So we can either duplicate all the options from domain section or allow the one mistake where the inherit_from used in normal domain section will be undetected. I am more in favor of the second option, because adding inherit_from by mistake is unlikely and the rules look better this way.

However it would be good to enhance the libini to add solution for this by introducing the ability to somehow merger the rules.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][synchronized] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][synchronized] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
It would be good to write an test. (probably integration).
But we would need to skip test on platforms with old libini_config

Following should work
```python
import platform
(name, version, _) =  platform.dist()
if ((name in ('redhat', 'centos') and version[0] == "6")
    or (name == 'debian' and version[0] == "8")):
    pytest.skip("sssctl config-check is not supported due to old libini_config")
```

"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-293260151

[sssd PR#226][+Changes requested] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

Label: +Changes requested

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
Actually, it would not be complicated to write unit test either.
```C
struct sss_ini_initdata {
    char **error_list;
    struct ref_array *ra_success_list;
    struct ref_array *ra_error_list;
    struct ini_cfgobj *sssd_config;
    struct value_obj *obj;
    const struct stat *cstat;
    struct ini_cfgfile *file;
};

#include "util/sss_ini.h"
#include <ini_configobj.h>

static void test_config_validation(void **state)
{
    int ret;
    size_t num_errors;
    char **strs = NULL;

struct sss_ini_initdata *ini_data = sss_ini_initdata_init(NULL);
    assert_non_null(ini_data);

char config[] = "[domain/tesst_~^domain]\n";
    ret = ini_config_file_from_mem(config,
                                   sizeof(config),
                                   &ini_data->file);

assert_int_equal(ret, EOK);

ret = ini_config_create(&(ini_data->sssd_config));
    assert_int_equal(ret, EOK);

ret = ini_config_parse(ini_data->file,
                           INI_STOP_ON_ANY,
                           INI_MV1S_OVERWRITE,
                           INI_PARSE_NOWRAP,
                           ini_data->sssd_config);
    assert_int_equal(ret, EOK);

char *rules_path = talloc_asprintf(NULL, "%s/src/config/cfg_rules.ini",
                                       getenv("ABS_TOP_SRCDIR"));
    assert_non_null(rules_path);

ret = sss_ini_call_validators_strs(NULL, ini_data, rules_path,
                                       &strs, &num_errors);
    assert_int_equal(ret, EOK);
    assert_int_equal(num_errors, 0);
}
```

Just use `#ifdef HAVE_LIBINI_CONFIG_V1_3`. Unit tests are usually more faster :-)
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-293557171

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
bump
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-296182101

[sssd PR#226][synchronized] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][comment] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

mzidek-rh commented:
"""
Pushed the unit test. They should be easy to extend.
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304354777

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
FAIL: config_check_tests (exit: 6) on el6
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304634347

[sssd PR#226][synchronized] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][comment] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

mzidek-rh commented:
"""
Pushed the new version, the tests are now not built for rhel6 because they need new libini.
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304685048

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
I had few inline comments. It would be good to also update sssd.conf
e.g. fix typo in trusted domain
```
TRUSTED DOMAIN SECTION
       Some options used in the domain section can also be used in the trusted domain section, that is,
       in a section called “[domain/DOMAIN_NAME]/TRUSTED_DOMAIN_NAME]”. Currently supported options in
       the trusted domain section are:
```

I have also question about the option `inherit_from`. It is allowed in any domain section. What would happen if I use this option between two LDAP domain (not application domains). Is it expected?
Do we want to support it? If yes then we need to update man page (together with change in schema file.
And some questions might be answered by @jhrozek

"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304805424

[sssd PR#226][comment] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

mzidek-rh commented:
"""
About the inherit_from ... please read the first comment in this PR.
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304824983

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
> About the inherit_from ... please read the first comment in this PR.

A) 1st commit "VALIDATORS: Add subdomain section" doesn't say anything about `inherit_from`
B) it is not important what is written in the commit message " VALIDATORS: Remove application section domain". The most important is how does it behave. And that was my question. BTW this commit is just temporary workaround. Because if `inherit_from` is not allowed in standard domain then `config-check` should report it. How it will be done is an implementation detail :-)
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304833227

[sssd PR#226][synchronized] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][comment] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

mzidek-rh commented:
"""
Pushed new version. Added a patch to fix the man page typo in trusted domain section.
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304834340

[sssd PR#226][synchronized] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][comment] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

mzidek-rh commented:
"""
I added one small patch to apply the same restrictions we have on domain section to the application section.
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304850704

[sssd PR#226][-Changes requested] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

Label: -Changes requested

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
`config_check` still allows `inherit_from` in domain section with the latest patchset.
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-304988604

[sssd PR#226][+Changes requested] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

Label: +Changes requested

[sssd PR#226][synchronized] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][comment] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

mzidek-rh commented:
"""
Added new patch with sssd specific validator + unit test for it.
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-305174730

[sssd PR#226][-Changes requested] Config check regex

[mzidek-rh]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

Label: -Changes requested

[sssd PR#226][+Accepted] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

Label: +Accepted

[sssd PR#226][comment] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

lslebodn commented:
"""
master:
* 39c5aee02fa36ba1cb78fe7669f272fec5277018
* 0bfc49133ab1a5f912b8cedef3f690318df7c50f
* 88d723396c674ed5e5d03e859e9f7ce1779a927b
* 537103f296b7112d9fd505af941a6d83d7902eb1
* a6a750d981a7a9088daa823ed3894b94e156a763
* f433e4dc4d80bef398ed8d9aac123963015bd2f0
* 0ab730850171c125cd4ce578c123656dd88eb484
"""

See the full comment at https://github.com/SSSD/sssd/pull/226#issuecomment-305208793

[sssd PR#226][closed] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Author: mzidek-rh
 Title: #226: Config check regex
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/226/head:pr226
git checkout pr226

[sssd PR#226][+Pushed] Config check regex

[lslebodn]

URL: https://github.com/SSSD/sssd/pull/226
Title: #226: Config check regex

Label: +Pushed