Dne 20.4.2015 v 10:06 Martin Babinsky napsal(a):
On 04/20/2015 09:48 AM, Jan Cholasta wrote:
Dne 15.4.2015 v 15:17 Martin Babinsky napsal(a):
On 04/13/2015 02:16 PM, Martin Babinsky wrote:
On 04/09/2015 03:38 PM, Jan Cholasta wrote:

Some comments:

Patch 15:

1) The functions should be as similar as possible:

     a) kinit_password() should have a 'ccache_path' argument
instead of
passing the path in KRB5CCNAME in the 'env' argument.

     b) I don't think kinit_password() should have the 'env'
argument at
all. You can always call kinit with LC_ALL=C and set other
variables in
os.environ if you want.

     c) The arguments should have the same ordering.

     d) Either set KRB5CCNAME in both kinit_keytab() and
kinit_password() or in none of them.

     e) Either rename armor_ccache to armor_ccache_path or ccache_path
to ccache.

I have done some reordering of parameters in both functions so they are
very similar now and the parameter ordering should make more sense (at
least to me).

Neither of them sets KRB5CCNAME env. variable since I think that it is
not a very good practice and the developer should be responsible for
pointing to correct CCache path. Jan agrees with this and the other
patches are updated accordingly.

2) Space before comma in docstring:

+    Given a ccache_path , keytab file and a principal kinit as that

3) I would prefer if the default value of 'armor_ccache' in
kinit_password() was None.


Patch 16:

1) The callback should not be named 'validate_kinit_attempts_option',
but rather 'kinit_attempts_callback', as it doesn't just validate the


2) Why is there the sys.maxint upper bound on --kinit-attempts
again? A
comment with explanation would be nice.

It actually doesn't make much sense to have such upper bound, so I have
removed it from the check and updated the error message accordingly.

Patch 17:

1) Is there a reason for the ccache filename changes in DNSSEC code?

That was Petr Spacek's request since a sane naming of persistent
makes debugging of Kerberos-related errors a bit easier for him.

Attaching updated patches.

Jan had some further suggestions so I am attaching updated patches which
should reflect them.

He also recommended to split the naming changes of DNSSEC daemon
credential caches to a separate patch, so I will submit them later when
this patchset is pushed.

ACK. The patches need to be rebased on top of ipa-4-1 though.

Right, attaching rebased patches.


Pushed to:
master: 3d2feac0e416c66ba37eee53ef5b3833c2c3e414
ipa-4-1: 0ca8254959f3566c322eb7b20c6d6522814d78d1

Jan Cholasta

Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to