Merge pull request #5677 from orchestral-st2/issue/get-key-value-by-name

Fixed a bug where key value pair was not returning data correctly when get_by_name function is called.

Author: m4dcoder

CHANGELOG.rst

@@ -19,6 +19,10 @@ Fixed
 
   Contributed by @S-T-A-R-L-O-R-D
 
+* Fixed a bug where calling 'get_by_name' on client for getting key details was not returning any results despite key being stored. #5677
+
+  Contributed by @bharath-orchestral
+
 
 * Fixed ``st2client/st2client/base.py`` file to use ``https_proxy``(not ``http_proxy``) to check HTTPS_PROXY environment variables.
 

st2api/st2api/controllers/v1/keyvalue.py

@@ -212,8 +212,14 @@ def get_all(
         # Set user scope prefix for the provided user (or current user if user not provided)
         # NOTE: It's very important raw_filters['prefix'] is set when requesting user scoped
         # items to avoid information leakage (aka user1 retrieves items for user2)
+        name_for_keyref = ""
+        if "name" in raw_filters and raw_filters["name"]:
+            name_for_keyref = raw_filters["name"]
+        else:
+            name_for_keyref = prefix or ""
+
         user_scope_prefix = get_key_reference(
-            name=prefix or "", scope=FULL_USER_SCOPE, user=user
+            name=name_for_keyref, scope=FULL_USER_SCOPE, user=user
         )
 
         # Special cases for ALL_SCOPE
@@ -277,7 +283,10 @@ def get_all(
         if scope in [ALL_SCOPE, USER_SCOPE, FULL_USER_SCOPE]:
             # Retrieves all the user scoped items that the current user owns.
             raw_filters["scope"] = FULL_USER_SCOPE
-            raw_filters["prefix"] = user_scope_prefix
+            if "name" in raw_filters and raw_filters["name"]:
+                raw_filters["name"] = user_scope_prefix
+            else:
+                raw_filters["prefix"] = user_scope_prefix
 
             items = self._get_all(
                 from_model_kwargs=from_model_kwargs,

st2client/st2client/client.py

@@ -38,6 +38,7 @@
 from st2client.models.core import ServiceRegistryGroupsManager
 from st2client.models.core import ServiceRegistryMembersManager
 from st2client.models.core import add_auth_token_to_kwargs_from_env
+from st2client.models.core import KeyValuePairResourceManager
 
 
 LOG = logging.getLogger(__name__)
@@ -273,7 +274,7 @@ def __init__(
             debug=self.debug,
             basic_auth=self.basic_auth,
         )
-        self.managers["KeyValuePair"] = ResourceManager(
+        self.managers["KeyValuePair"] = KeyValuePairResourceManager(
             models.KeyValuePair,
             self.endpoints["api"],
             cacert=self.cacert,

st2client/st2client/models/core.py

@@ -879,3 +879,38 @@ def list(self, group_id, **kwargs):
             result.append(item)
 
         return result
+
+
+class KeyValuePairResourceManager(ResourceManager):
+    @add_auth_token_to_kwargs_from_env
+    def get_by_name(self, name, **kwargs):
+
+        token = kwargs.get("token", None)
+        api_key = kwargs.get("api_key", None)
+        params = kwargs.get("params", {})
+
+        for k, v in six.iteritems(kwargs):
+            # Note: That's a special case to support api_key and token kwargs
+            if k not in ["token", "api_key", "params"]:
+                params[k] = v
+
+        url = "/%s/%s/?%s" % (
+            self.resource.get_url_path_name(),
+            name,
+            urllib.parse.urlencode(params),
+        )
+
+        if token:
+            response = self.client.get(url, token=token)
+        elif api_key:
+            response = self.client.get(url, api_key=api_key)
+        else:
+            response = self.client.get(url)
+
+        if response.status_code == http_client.NOT_FOUND:
+            # for query and query_with_count
+            return []
+        if response.status_code != http_client.OK:
+            self.handle_error(response)
+
+        return self.resource.deserialize(parse_api_response(response))

st2client/tests/unit/test_models.py

@@ -471,3 +471,20 @@ def test_resource_clone_failed(self):
         mgr = models.ResourceManager(base.FakeResource, base.FAKE_ENDPOINT)
         source_ref = "spack.saction"
         self.assertRaises(Exception, mgr.clone, source_ref, "dpack", "daction")
+
+
+class TestKeyValuePairResourceManager(unittest2.TestCase):
+    @mock.patch.object(
+        httpclient.HTTPClient,
+        "get",
+        mock.MagicMock(
+            return_value=base.FakeResponse(json.dumps(base.RESOURCES[0]), 200, "OK")
+        ),
+    )
+    def test_resource_get_by_name(self):
+        mgr = models.KeyValuePairResourceManager(base.FakeResource, base.FAKE_ENDPOINT)
+        # No X-Total-Count
+        resource = mgr.get_by_name("abc")
+        actual = resource.serialize()
+        expected = json.loads(json.dumps(base.RESOURCES[0]))
+        self.assertEqual(actual, expected)