On 19.8.2016 13:11, Fraser Tweedale wrote:
Bump for review.
On Mon, Aug 15, 2016 at 05:15:16PM +1000, Fraser Tweedale wrote:
Thanks for reviews. Rebased and updated patches attached (and one
new patch). No substantive changes to 92..94. Patch order is:
92-2, 93-2, 94-2, 98, 90-3
Other comments inline.
On Fri, Aug 12, 2016 at 11:33:28AM +0200, Jan Cholasta wrote:
Patch 0092: ACK
Patch 0093: ACK
Patch 0094: ACK
Please fix this PEP8 issue before pushing:
./ipaserver/plugins/cert.py:597:17: W503 line break before binary operator
Patch 0098: ACK
1) Generic otherNames (san_other) do not work correctly. The OID is not
included in the value and names with complex type other than
KerberosPrincipal are not parsed correctly. The value should include the OID
and DER blob of the name.
Updated to use "OID:b64(DER)" as the attribute value.
2) With --all, san_other should be included in the result for all
otherNames, even the known ones, to provide (limited) forward compatibility.
Done; when --all given, known otherName kinds are included in
'san_other' attribute in addition to their own attribute.
3) Do we have to support *all* the name types? I mean we could, for the sake
of completeness, but it might be easier to just keep the few ones we
actually care about (email, DNS name, principal name, UPN and directory name
in your patch 0095).
Yeah, why not support them all? See also Petr's comments in other
branch of thread.
Works for me, but see Lukáš's reply, I think he has a point. Maybe we
can make a compromise and show only supported name types by default and
everything with --all?
+ obj.setdefault(attr_name, ).append(unicode(name))
The value should not (always) be unicode, but of the type declared by the
param (unicode or ipapython.kerberos.Principal or
I now pass the value to the constructor of whatever type the
attr_value = self.params[attr_name].type(name_formatted)
5) san_directoryname should be a DNParam rather than Str.
6) Could we use "Subject <name type>" instead of "Subject Alternative
Name (<name type>)" for labels? Or something else which is shorter and
has the name type more "visible" than the current form.
7) The patch needs a rebase.
Manage your subscription for the Freeipa-devel mailing list:
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code