From 5cad282b80f0babafb7b6bf995b5bc2732bc655c Mon Sep 17 00:00:00 2001
From: Jiri Podivin <jpodivin@redhat.com>
Date: Fri, 12 Nov 2021 10:01:47 +0100
Subject: [PATCH] Expanding parser actions to allow for multiple key-value
 pairs

KeyValueAction previously allowed only one key=>value pair to be
supplied in the CLI per argument.
While the operator could supply multiple key=>value pairs by repeatedly
using the same argument it was unnecessarily verbose and tedious approach.

Similar behavior is implemented in the tripleoclient validator CLI
by the resulution to the rhbz#1959492 [0].

Tests were adjusted.

[0]https://bugzilla.redhat.com/show_bug.cgi?id=1959492:

Signed-off-by: Jiri Podivin <jpodivin@redhat.com>
Change-Id: I28c40bb8156d73dd95af9641acaab3cce721be2d
---
 validations_libs/cli/parseractions.py         | 33 ++++++++++++++-----
 validations_libs/tests/cli/fakes.py           |  2 +-
 .../tests/cli/test_parseractions.py           | 14 --------
 validations_libs/tests/cli/test_run.py        |  9 -----
 4 files changed, 25 insertions(+), 33 deletions(-)

diff --git a/validations_libs/cli/parseractions.py b/validations_libs/cli/parseractions.py
index 6fec6cb6..2a7a93bc 100644
--- a/validations_libs/cli/parseractions.py
+++ b/validations_libs/cli/parseractions.py
@@ -16,6 +16,8 @@
 
 import argparse
 
+from validations_libs.utils import LOG
+
 
 class CommaListAction(argparse.Action):
     def __call__(self, parser, namespace, values, option_string=None):
@@ -33,16 +35,29 @@ class KeyValueAction(argparse.Action):
             setattr(namespace, self.dest, {})
 
         # Add value if an assignment else remove it
-        if values.count('=') == 1:
-            values_list = values.split('=', 1)
-            if '' == values_list[0]:
-                msg = (
-                    "Property key must be specified: {}"
-                ).format(str(values))
+        if values.count('=') >= 1:
+            for key_value in values.split(','):
+                key, value = key_value.split('=', 1)
+                if '' == key:
+                    msg = (
+                        "Property key must be specified: {}"
+                    ).format(str(values))
 
-                raise argparse.ArgumentTypeError(msg)
-            else:
-                getattr(namespace, self.dest, {}).update([values_list])
+                    raise argparse.ArgumentTypeError(msg)
+                elif value.count('=') > 0:
+                    msg = (
+                        "Only a single '=' sign is allowed: {}"
+                    ).format(str(values))
+
+                    raise argparse.ArgumentTypeError(msg)
+                else:
+                    if key in getattr(namespace, self.dest, {}):
+                        LOG.warning((
+                                "Duplicate key '%s' provided."
+                                "Value '%s' Overriding previous value. '%s'"
+                            ) % (
+                            key, getattr(namespace, self.dest)[key], value))
+                    getattr(namespace, self.dest, {}).update({key: value})
         else:
             msg = (
                 "Expected 'key=value' type, but got: {}"
diff --git a/validations_libs/tests/cli/fakes.py b/validations_libs/tests/cli/fakes.py
index 5895d45a..8de80455 100644
--- a/validations_libs/tests/cli/fakes.py
+++ b/validations_libs/tests/cli/fakes.py
@@ -58,5 +58,5 @@ KEYVALUEACTION_VALUES = {
     'invalid_noeq': 'foo>bar',
     'invalid_multieq': 'foo===bar',
     'invalid_nokey': '=bar',
-    'invalid_multikey': 'foo=bar,fizz=buzz'
+    'invalid_multikey': 'foo=bar,baz=,fizz=buzz,baz'
 }
diff --git a/validations_libs/tests/cli/test_parseractions.py b/validations_libs/tests/cli/test_parseractions.py
index 2b3b7b94..5755d996 100644
--- a/validations_libs/tests/cli/test_parseractions.py
+++ b/validations_libs/tests/cli/test_parseractions.py
@@ -88,17 +88,3 @@ class TestParserActions(TestCase):
         self.assertIn('fizz', dir(self.mock_namespace))
         self.assertDictEqual({}, self.mock_namespace.fizz)
         self.tearDown()
-
-    def test_keyvalueaction_invalid_invalid_multikey(self):
-
-        self.assertRaises(
-            argparse.ArgumentTypeError,
-            self.action,
-            self.mock_parser,
-            self.mock_namespace,
-            self.test_values['invalid_multikey']
-        )
-
-        self.assertIn('fizz', dir(self.mock_namespace))
-        self.assertDictEqual({}, self.mock_namespace.fizz)
-        self.tearDown()
diff --git a/validations_libs/tests/cli/test_run.py b/validations_libs/tests/cli/test_run.py
index 7ba0d107..05bf1868 100644
--- a/validations_libs/tests/cli/test_run.py
+++ b/validations_libs/tests/cli/test_run.py
@@ -365,15 +365,6 @@ class TestRun(BaseCommand):
 
         self.assertDictEqual(call_args, run_called_args)
 
-    def test_run_command_exclusive_wrong_extra_vars(self):
-        arglist = ['--validation', 'foo',
-                   '--extra-vars', 'key=value1,key=value2']
-        verifylist = [('validation_name', ['foo']),
-                      ('extra_vars', {'key': 'value2'})]
-
-        self.assertRaises(Exception, self.check_parser, self.cmd,
-                          arglist, verifylist)
-
     @mock.patch('validations_libs.utils.find_config_file',
                 return_value="/etc/validations_foo.cfg")
     @mock.patch('validations_libs.constants.VALIDATIONS_LOG_BASEDIR')