Take plugin params from ENV rather than default

The way the argparse options were being structured, if there was a
default value set on the option it would use this value as the default
and not check the environment variables.

This is wrong, we expect the environment variables to be used and the
default value to be the final fallback.

Change-Id: Ifbd68c9de329c2e0c70824ba873caa579e8e86d0
Closes-Bug: #1388076

keystoneclient/auth/base.py

@@ -155,14 +155,12 @@ class BaseAuthPlugin(object):
                 args.append('--os-%s' % o.name)
                 envs.append('OS_%s' % o.name.replace('-', '_').upper())
 
-            default = opt.default
-            if default is None:
-                # select the first ENV that is not false-y or return None
-                env_vars = (os.environ.get(e) for e in envs)
-                default = six.next(six.moves.filter(None, env_vars), None)
+            # select the first ENV that is not false-y or return None
+            env_vars = (os.environ.get(e) for e in envs)
+            default = six.next(six.moves.filter(None, env_vars), None)
 
             parser.add_argument(*args,
-                                default=default,
+                                default=default or opt.default,
                                 metavar=opt.metavar,
                                 help=opt.help,
                                 dest='os_%s' % opt.dest)

keystoneclient/tests/auth/test_cli.py

@@ -13,6 +13,7 @@
 import argparse
 import uuid
 
+import fixtures
 import mock
 from oslo.config import cfg
 
@@ -43,6 +44,13 @@ class CliTests(utils.TestCase):
         super(CliTests, self).setUp()
         self.p = argparse.ArgumentParser()
 
+    def env(self, name, value=None):
+        if value is not None:
+            # environment variables are always strings
+            value = str(value)
+
+        return self.useFixture(fixtures.EnvironmentVariable(name, value))
+
     def test_creating_with_no_args(self):
         ret = cli.register_argparse_arguments(self.p, [])
         self.assertIsNone(ret)
@@ -140,6 +148,18 @@ class CliTests(utils.TestCase):
         self.assertIs(utils.MockPlugin, klass)
         m.assert_called_once_with(name)
 
+    @utils.mock_plugin
+    def test_env_overrides_default_opt(self, m):
+        name = uuid.uuid4().hex
+        val = uuid.uuid4().hex
+        self.env('OS_A_STR', val)
+
+        klass = cli.register_argparse_arguments(self.p, [], default=name)
+        opts = self.p.parse_args([])
+        a = klass.load_from_argparse_arguments(opts)
+
+        self.assertEqual(val, a['a_str'])
+
     def test_deprecated_cli_options(self):
         TesterPlugin.register_argparse_arguments(self.p)
         val = uuid.uuid4().hex

keystoneclient/tests/auth/utils.py

@@ -30,6 +30,8 @@ class MockPlugin(base.BaseAuthPlugin):
     INT_DESC = 'test int'
     FLOAT_DESC = 'test float'
     BOOL_DESC = 'test bool'
+    STR_DESC = 'test str'
+    STR_DEFAULT = uuid.uuid4().hex
 
     def __init__(self, **kwargs):
         self._data = kwargs
@@ -49,6 +51,7 @@ class MockPlugin(base.BaseAuthPlugin):
             cfg.IntOpt('a-int', default='3', help=cls.INT_DESC),
             cfg.BoolOpt('a-bool', help=cls.BOOL_DESC),
             cfg.FloatOpt('a-float', help=cls.FLOAT_DESC),
+            cfg.StrOpt('a-str', help=cls.STR_DESC, default=cls.STR_DEFAULT),
         ]