[Freeipa-devel] [freeipa PR#573][opened] Provide centralized management of user short name resolution

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Author: martbab
Title: #573: Provide centralized management of user short name resolution
Action: opened

PR body:
"""
This PR implement an initial version of AD user short name resolution
infrastructure consumable by SSSD.[1]

Most of the stuff described in the design page[2] is in-place except of hooks
that would refresh the domain resolution orders after trust domain removal or
disablement. I would like to do them in a separate PR.

Also some edge cases like specifying only separator (':') or an empty domain
('dom1::dom2') have no special treatment, the current code will just complain
about empty DNS labels. Should I improve this behavior?

[1] https://pagure.io/freeipa/issue/6372
[2] https://www.freeipa.org/page/V4/AD_User_Short_Names
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/573/head:pr573
git checkout pr573

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

MartinBasti commented:
"""
ACIs? AFAIK SSSD should be able to read this
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286094363

[HonzaCholasta]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

HonzaCholasta commented:
"""
I would rather avoid the refactoring in 4.5 - this is fragile code you are touching and I'm afraid it might break in some cases (think different client / server version combinations, thin client vs fat client, etc.).

As for the edge case values, IMO we should allow `:` without complaining as a special case to support "no domains in the list" configuration, and otherwise require known domain names (like in `certmaprule-add`).
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286095738

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Author: martbab
Title: #573: Provide centralized management of user short name resolution
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/573/head:pr573
git checkout pr573

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

martbab commented:
"""
Updated PR, added ACIs and fixed Py2/Py3 compatibility of doctests.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286096789

[abbra]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

abbra commented:
"""
I don't see ACI.txt regenerated.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286097962

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Author: martbab
Title: #573: Provide centralized management of user short name resolution
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/573/head:pr573
git checkout pr573

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

martbab commented:
"""
@HonzaCholasta I agree, I have removed the commit which introduces special param handling and resorted to simple splitting in validator. I have also regenerated ACIs in the respective commits.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286348952

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

martbab commented:
"""
Server upgrade consists only from adding the objectclass to ipaConfig which is taken care of in the update file. The idview object schema is modified on-demand when the attribute is set. Is there something else I need to take care of?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286385187

[HonzaCholasta]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

HonzaCholasta commented:
"""
IMO you should add the object class to all existing idviews on upgrade rather than add it on-demand.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286390880

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

MartinBasti commented:
"""
@HonzaCholasta it will break in case when idview entry is created on older replica, so it is more safe to appending the objectclass dynamically
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286394244

[HonzaCholasta]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

HonzaCholasta commented:
"""
Ah, right.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286395012

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Author: martbab
Title: #573: Provide centralized management of user short name resolution
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/573/head:pr573
git checkout pr573

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Author: martbab
Title: #573: Provide centralized management of user short name resolution
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/573/head:pr573
git checkout pr573

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

martbab commented:
"""
PR rebased, I have fixed bugs in ID view objectclass handling and re-used the trusted domain retrieval code in certmap plugin. This is a separate commit so it can be removed if necessary.

I have noticed that with current PR we can not add the domain resolution order to Default Trust View, as it is protected from both modification and removal. @abbra is this expected also in this case?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286454496

[abbra]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

abbra commented:
"""
Yes, it is expected too. Remember that 'Default Trust View' is a view that applies globally. You have already global setting to apply.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286456329

[martbab]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

martbab commented:
"""
Ok thanks for explanation.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286464608

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

Label: +ack

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

Label: +pushed

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/573
Title: #573: Provide centralized management of user short name resolution

MartinBasti commented:
"""
master:

* 594c87daf873ceec0c0cf3464bcb1aadb9f2b92a Short name resolution: introduce the required schema
* 1b5f56d15455b6019dd532cb9635fa2c44cb0022 ipaconfig: add the ability to manipulate domain resolution order
* 544d66b7109300e570fb6849f0f9bab8020f3b66 idview: add domain_resolution_order attribute
* 4e5e3eebb223b7f2760e21f22e42775982104b0d Re-use trust domain retrieval code in certmap validators
"""

See the full comment at https://github.com/freeipa/freeipa/pull/573#issuecomment-286500632

[MartinBasti]

URL: https://github.com/freeipa/freeipa/pull/573
Author: martbab
Title: #573: Provide centralized management of user short name resolution
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/573/head:pr573
git checkout pr573