[Freeipa-devel] [freeipa PR#667][opened] idrange-mod: properly handle empty --dom-name option

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/667
Author: flo-renaud
Title: #667: idrange-mod: properly handle empty --dom-name option
Action: opened

PR body:
"""
When idrange-mod is called with --dom-name=, the CLI exits with
ipa: ERROR: an internal error has occurred
This happens because the code checks if the option is provided but does not
check if the value is None.

We need to handle empty dom-name as if the option was not specified.

https://pagure.io/freeipa/issue/6404
"""

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

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-mod: properly handle empty --dom-name option

stlaz commented:
"""
LGTM, except you're talking about `idrange-mod` in the commit message but are fixing `idrange-add` (`idrange-mod` does not have the option at all).
"""

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

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/667
Author: flo-renaud
Title: #667: idrange-mod: properly handle empty --dom-name option
Action: synchronized

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

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/667
Author: flo-renaud
Title: #667: idrange-mod: properly handle empty --dom-name option
Action: edited

Changed field: body
Original value:
"""
When idrange-mod is called with --dom-name=, the CLI exits with
ipa: ERROR: an internal error has occurred
This happens because the code checks if the option is provided but does not
check if the value is None.

We need to handle empty dom-name as if the option was not specified.

https://pagure.io/freeipa/issue/6404
"""

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/667
Author: flo-renaud
Title: #667: idrange-add: properly handle empty --dom-name option
Action: edited

Changed field: title
Original value:
"""
idrange-mod: properly handle empty --dom-name option
"""

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-add: properly handle empty --dom-name option

flo-renaud commented:
"""
Hi @stlaz
I fixed the commit message.

In contrary to what I told you offline, you need to configure an AD trust with ipa-adtrust-install and ipa trust-add ... in order to reproduce the original issue. My bad...
"""

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

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-add: properly handle empty --dom-name option

stlaz commented:
"""
@flo-renaud That's completely OK :)
I thought we could probably add an assert to `CIDict.__contains__()` method since I realize the issue was somewhere else than this fixed check, but the current situation fails verbosely enough so that's probably fine.
ACK.
"""

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

[stlaz]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-add: properly handle empty --dom-name option

Label: +ack

[martbab]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-add: properly handle empty --dom-name option

martbab commented:
"""
@flo-renaud can you please add a test case for this to `ipatests/test_xmlrpc/test_range_plugin.py` so that we do not regress in the future?
"""

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

[tomaskrizek]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-add: properly handle empty --dom-name option

Label: +pushed

[tomaskrizek]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-add: properly handle empty --dom-name option

tomaskrizek commented:
"""
master:

* 70743c8c48db54309a09d510b3a5d8ae86c29e58 idrange-add: properly handle empty --dom-name option


ipa-4-5:

* 077a61524d79ac5ab6f0eb46450c82ad5594bd2b idrange-add: properly handle empty --dom-name option


"""

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

[tomaskrizek]

URL: https://github.com/freeipa/freeipa/pull/667
Author: flo-renaud
Title: #667: idrange-add: properly handle empty --dom-name option
Action: closed

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

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/667
Title: #667: idrange-add: properly handle empty --dom-name option

flo-renaud commented:
"""
@martbab
thank you for the suggestion. The new test is available in PR #692
"""

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