From 030c7154884e210032532954b628331f9a1be5a0 Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Tue, 19 Jul 2022 08:33:26 +0530 Subject: [PATCH 1/9] Fixed the bug when client tries to get a stored key by name --- st2api/st2api/controllers/v1/keyvalue.py | 13 +++++++-- st2client/st2client/client.py | 3 ++- st2client/st2client/models/core.py | 34 ++++++++++++++++++++++++ st2client/tests/unit/test_models.py | 16 +++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/st2api/st2api/controllers/v1/keyvalue.py b/st2api/st2api/controllers/v1/keyvalue.py index c554d97c2c..83545af8b6 100644 --- a/st2api/st2api/controllers/v1/keyvalue.py +++ b/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.get("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, diff --git a/st2client/st2client/client.py b/st2client/st2client/client.py index ec2878758d..4b3e74e849 100644 --- a/st2client/st2client/client.py +++ b/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, diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 412abd99be..7bea19687a 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -879,3 +879,37 @@ 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)) \ No newline at end of file diff --git a/st2client/tests/unit/test_models.py b/st2client/tests/unit/test_models.py index 7f6f7f9ffa..5861ab3520 100644 --- a/st2client/tests/unit/test_models.py +++ b/st2client/tests/unit/test_models.py @@ -471,3 +471,19 @@ 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) \ No newline at end of file From 31df643a02217bacfe0d3d355b4fe04d2e5a589a Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Tue, 19 Jul 2022 08:38:58 +0530 Subject: [PATCH 2/9] Small change as requested by github approver --- st2api/st2api/controllers/v1/keyvalue.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2api/st2api/controllers/v1/keyvalue.py b/st2api/st2api/controllers/v1/keyvalue.py index 83545af8b6..3e6163e78f 100644 --- a/st2api/st2api/controllers/v1/keyvalue.py +++ b/st2api/st2api/controllers/v1/keyvalue.py @@ -214,7 +214,7 @@ def get_all( # 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.get("name") + name_for_keyref = raw_filters["name"] else: name_for_keyref = prefix or "" From 2487e829722c03eaed2648c179d80d01a63495dd Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Wed, 20 Jul 2022 07:56:01 +0530 Subject: [PATCH 3/9] Added changes in CHANGELOG.rst --- CHANGELOG.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 440c65b4a9..e4d999db06 100644 --- a/CHANGELOG.rst +++ b/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 + + Contributed by @bharath-orchestral + Added ~~~~~ From 8a99a8bec28a7f6e2c2f25febbae6ef9ae92ca0e Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Wed, 20 Jul 2022 08:17:57 +0530 Subject: [PATCH 4/9] Resolved conflict, added changes in CHANGELOG.rst --- CHANGELOG.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e4d999db06..4e1a4bedf5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -23,6 +23,12 @@ Fixed Contributed by @bharath-orchestral + +* Fixed ``st2client/st2client/base.py`` file to use ``https_proxy``(not ``http_proxy``) to check HTTPS_PROXY environment variables. + + Contributed by @wfgydbu + + Added ~~~~~ From ed51319791860ef41c2280d7fa6b631a671b48d2 Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Sat, 23 Jul 2022 02:03:49 +0000 Subject: [PATCH 5/9] Fixed Lint, Black Checks --- CHANGELOG.rst | 2 +- st2client/st2client/models/core.py | 7 ++++--- st2client/tests/unit/test_models.py | 3 ++- .../packs/dummy_pack_23/actions/workflows/__init__.py | 0 4 files changed, 7 insertions(+), 5 deletions(-) delete mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2a9e8a2540..85dd574e78 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -22,7 +22,7 @@ Fixed * Fixed a bug where calling 'get_by_name' on client for getting key details was not returning any results despite key being stored Contributed by @bharath-orchestral - + * Fixed ``st2client/st2client/base.py`` file to use ``https_proxy``(not ``http_proxy``) to check HTTPS_PROXY environment variables. diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 7bea19687a..e66e0e7800 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -880,6 +880,7 @@ def list(self, group_id, **kwargs): return result + class KeyValuePairResourceManager(ResourceManager): @add_auth_token_to_kwargs_from_env def get_by_name(self, name, **kwargs): @@ -898,18 +899,18 @@ def get_by_name(self, name, **kwargs): 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)) \ No newline at end of file + return self.resource.deserialize(parse_api_response(response)) diff --git a/st2client/tests/unit/test_models.py b/st2client/tests/unit/test_models.py index 5861ab3520..8f5d6bd6d7 100644 --- a/st2client/tests/unit/test_models.py +++ b/st2client/tests/unit/test_models.py @@ -472,6 +472,7 @@ def test_resource_clone_failed(self): source_ref = "spack.saction" self.assertRaises(Exception, mgr.clone, source_ref, "dpack", "daction") + class TestKeyValuePairResourceManager(unittest2.TestCase): @mock.patch.object( httpclient.HTTPClient, @@ -486,4 +487,4 @@ def test_resource_get_by_name(self): resource = mgr.get_by_name("abc") actual = resource.serialize() expected = json.loads(json.dumps(base.RESOURCES[0])) - self.assertEqual(actual, expected) \ No newline at end of file + self.assertEqual(actual, expected) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 From 50ff372cfa937eb12226387605dff3884def0fec Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Mon, 25 Jul 2022 15:00:57 +0000 Subject: [PATCH 6/9] added __init_.py back in st2tests/..../dummy_pack_23/actions/workflows/ --- .../dummy_pack_23/actions/workflows/__init__.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py new file mode 100644 index 0000000000..30c83ff44f --- /dev/null +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py @@ -0,0 +1,14 @@ +# Copyright 2020 The StackStorm Authors. +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. From 8cca750e4b20283ba06db2c27519d59b2638fb46 Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Tue, 26 Jul 2022 01:06:54 +0000 Subject: [PATCH 7/9] Removed headers in __init__.py --- .../dummy_pack_23/actions/workflows/__init__.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py index 30c83ff44f..e69de29bb2 100644 --- a/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py +++ b/st2tests/st2tests/fixtures/packs/dummy_pack_23/actions/workflows/__init__.py @@ -1,14 +0,0 @@ -# Copyright 2020 The StackStorm Authors. -# Copyright 2019 Extreme Networks, Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. From b020e1c320c73674979dd2f600be18993a645c31 Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Mon, 8 Aug 2022 05:32:14 +0000 Subject: [PATCH 8/9] added pull request number to change log --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 85dd574e78..b19726339c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,7 +19,7 @@ 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 +* 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 From ea4f99cb085523b1554f422a15c6a99cab5eab04 Mon Sep 17 00:00:00 2001 From: Bharath Reddy Date: Mon, 8 Aug 2022 05:54:40 +0000 Subject: [PATCH 9/9] small change to change log --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b19726339c..cc55c21ccf 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -19,7 +19,7 @@ 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 +* 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