[Freeipa-devel] CLI parameter: TextFile, BinaryFile and mutually exclusive group

2015-07-30 Thread Christian Heimes
Hello,

While I was working on the ticket
https://fedorahosted.org/freeipa/ticket/5155, I noticed a couple of
additional places that may raise an IOError. Instead of a File()
paramaeter, the vault plugin uses Str() paramater in combination with
open() to read files.

For passwords I can mostly replace the Str() parameter with File().
There is only one minor issue. The File() class has no encoding flag.
ipalib.cli.cli.load_files() uses the encoding of sys.stdin to
determinate the encoding. In some cases the encoding of sys.stdin can be
ASCII. For that reason I like to add an encoding parameter to File().

For public and private key file I can't use File(). File() is a subclass
of Str(), which requires unicode text. The vault code treats public and
private key data as bytes. I assume it wants to support DER encoded key
data, too. I like to introduce a new BinaryFile() parameter, which
subclasses Bytes(). It might make sense to alias File as TextFile and
deprecate the File name.

Finally the vault plugin has several mutually exclusive paramater, e.g.
passsword and password-file. The plugin has seven distinct checks for
mutual exclusion. IMHO this should be better handled by the parameter
parsing code. Python's argparse module has a similar feature:
https://docs.python.org/2/library/argparse.html#mutual-exclusion

I like to handle the case with a mutually_exclusive flag such as:

Str(
'password?',
cli_name='password',
doc=_('Vault password'),
mutually_exclusive='password',
),
File(
'password_file?',
cli_name='password_file',
doc=_('File containing the vault password'),
mutually_exclusive='password',
),

If more than one parameter with the same mutually_exclusive group name
is given, then a MutuallyExclusiveError is raised.

Christian



signature.asc
Description: OpenPGP digital signature
-- 
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

Re: [Freeipa-devel] CLI parameter: TextFile, BinaryFile and mutually exclusive group

2015-07-30 Thread Christian Heimes
On 2015-07-30 14:37, Jan Cholasta wrote:
 Hi,
 
 Dne 30.7.2015 v 14:07 Christian Heimes napsal(a):
 Hello,

 While I was working on the ticket
 https://fedorahosted.org/freeipa/ticket/5155, I noticed a couple of
 additional places that may raise an IOError. Instead of a File()
 paramaeter, the vault plugin uses Str() paramater in combination with
 open() to read files.

 For passwords I can mostly replace the Str() parameter with File().
 There is only one minor issue. The File() class has no encoding flag.
 ipalib.cli.cli.load_files() uses the encoding of sys.stdin to
 determinate the encoding. In some cases the encoding of sys.stdin can be
 ASCII. For that reason I like to add an encoding parameter to File().

 For public and private key file I can't use File(). File() is a subclass
 of Str(), which requires unicode text. The vault code treats public and
 private key data as bytes. I assume it wants to support DER encoded key
 data, too. I like to introduce a new BinaryFile() parameter, which
 subclasses Bytes(). It might make sense to alias File as TextFile and
 deprecate the File name.

 Finally the vault plugin has several mutually exclusive paramater, e.g.
 passsword and password-file. The plugin has seven distinct checks for
 mutual exclusion. IMHO this should be better handled by the parameter
 parsing code. Python's argparse module has a similar feature:
 https://docs.python.org/2/library/argparse.html#mutual-exclusion

 I like to handle the case with a mutually_exclusive flag such as:

  Str(
  'password?',
  cli_name='password',
  doc=_('Vault password'),
 mutually_exclusive='password',
  ),
  File(
  'password_file?',
  cli_name='password_file',
  doc=_('File containing the vault password'),
 mutually_exclusive='password',
  ),

 If more than one parameter with the same mutually_exclusive group name
 is given, then a MutuallyExclusiveError is raised.
 
 NACK, instead of having duplicate definitions for a single logical
 parameter and dealing with their inherent mutual exclusiveness on the
 framework level, this should be handled exclusively by the CLI by
 generating multiple command line options for different dispositions of
 the logical parameter. If anything, File should be completely removed,
 not further extended, as it is inherently broken and never worked properly.
 
 I have an almost working patch which implements this, but I don't think
 it's 4.2.1 material, so I would suggest doing a simple fix for #5155 for
 now.

I wasn't aware that you have a mostly working patch. In that case I'll
come up with a simple fix. I can take care of a redesign when your patch
has landed in the future.

Thanks for the feedback!
Christian




signature.asc
Description: OpenPGP digital signature
-- 
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