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
user.
3) I would prefer if the default value of 'armor_ccache' in
kinit_password() was None.
Fixed.
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
value.
Fixed.
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 Ccaches
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.
--
Jan Cholasta
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
