Skip to content

Adding get_by_ref for ParametersViewController #5509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arm4b merged 1 commit into from
Jan 18, 2022
Merged

Adding get_by_ref for ParametersViewController #5509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ Fixed
Added
~~~~~

* Added st2 API get action parameters by ref. #5509

API endpoint ``/api/v1/actions/views/parameters/{action_id}`` accepts ``ref_or_id``.

Contributed by @DavidMeu

* Enable setting ttl for MockDatastoreService. #5468

Contributed by @ytjohn
Expand Down Expand Up @@ -4230,4 +4236,4 @@ v0.5.1 - November 3rd, 2014
Added
~~~~~

* Initial public release
* Initial public release
28 changes: 22 additions & 6 deletions st2api/st2api/controllers/v1/action_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
from st2common.content import utils
from st2common.models.api.action import ActionAPI
from st2common.models.utils import action_param_utils
from st2common.models.system.common import ResourceReference
from st2common.persistence.action import Action
from st2common.persistence.runner import RunnerType
from st2common.rbac.types import PermissionType
Expand All @@ -50,6 +51,18 @@ def _get_action_by_id(id):
LOG.exception(msg)
abort(http_client.NOT_FOUND, msg)

@staticmethod
def _get_action_by_ref(ref):
try:
action_db = Action.get_by_ref(ref)
if not action_db:
Copy link
Contributor

@amanda11 amanda11 Dec 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the other methods, then we don't usually check the value of the return value from get_by_id etc, just return it. So for consistency I would do the same and return None rather than throw an exception to be consistent with others.

Copy link
Author

@DavidMeu DavidMeu Dec 23, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thing is I wanted to throw a meaningful exception in case it was not found in DB same there is for get_by_id.
When I perform a lookup for non existing id I get:

{
    "faultstring": "Database lookup for id=\"61b9fa409a66b66d529d8345\" resulted in exception. Unable to find the ActionDB instance. {'id': '61b9fa409a66b66d529d8345'}"
}

@amanda11 Seems you are right, I assume it should behave the same for Action.get_by_ref:

st2/st2common/st2common/models/db/__init__.py

Line 521 in 76feb92

msg = "Unable to find the %s instance. %s" % (self.model.__name__, kwargs)

Copy link
Author

@DavidMeu DavidMeu Dec 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@amanda11 did a re-check and seems I need to handle that exception as when I perform a lookup for a non exesting action ref I get:

{
    "faultstring": "Internal Server Error"
}

Full trace from st2api:

2021-12-28 09:36:27,898 ERROR [-] Failed to call controller function "get_one" for operation "st2api.controllers.v1.action_view
s:parameters_view_controller.get_one": 'NoneType' object has no attribute 'get_uid'
Traceback (most recent call last):
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2common/router.py", line 516, in __call__
    resp = func(**kw)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2api/controllers/v1/action_views.py", line 84, in get_one
    return self._get_one(action_id, requester_user=requester_user)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2api/controllers/v1/action_views.py", line 104, in _get_one
    permission_type=permission_type,
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2rbac_backend/utils.py", line 123, in assert_user_has_resource_db_per
mission
    permission_type=permission_type)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2rbac_backend/utils.py", line 282, in user_has_resource_db_permission

    permission_type=permission_type)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2rbac_backend/resolvers.py", line 415, in user_has_resource_db_permis
sion
    action_uid = resource_db.get_uid()
AttributeError: 'NoneType' object has no attribute 'get_uid'
2021-12-28 09:36:27,898 ERROR [-] API call failed: 'NoneType' object has no attribute 'get_uid'
Traceback (most recent call last):
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2common/middleware/error_handling.py", line 49, in __call__
    return self.app(environ, start_response)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2common/middleware/streaming.py", line 48, in __call__
    return self.app(environ, start_response)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2common/router.py", line 599, in as_wsgi
    resp = self(req)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2common/router.py", line 524, in __call__
    raise e
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2common/router.py", line 516, in __call__
    resp = func(**kw)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2api/controllers/v1/action_views.py", line 84, in get_one
    return self._get_one(action_id, requester_user=requester_user)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2api/controllers/v1/action_views.py", line 104, in _get_one
    permission_type=permission_type,
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2rbac_backend/utils.py", line 123, in assert_user_has_resource_db_per
mission
    permission_type=permission_type)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2rbac_backend/utils.py", line 282, in user_has_resource_db_permission

    permission_type=permission_type)
  File "/opt/stackstorm/st2/lib/python3.6/site-packages/st2rbac_backend/resolvers.py", line 415, in user_has_resource_db_permis
sion
    action_uid = resource_db.get_uid()
AttributeError: 'NoneType' object has no attribute 'get_uid' (_exception_class='AttributeError',_exception_message="'NoneType'
object has no attribute 'get_uid'",_exception_data={})

Seems it is not raising the exception when the action reference is not found

raise ValueError('Referenced action "%s" doesnt exist' % (ref))
return action_db
except Exception as e:
msg = 'Database lookup for ref="%s" resulted in exception. %s' % (ref, e)
Copy link
Contributor

@amanda11 amanda11 Jan 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a little odd to raise an exception in this method only to re-catch it 2 lines down. But equally you need to handle generic and the None being returned, so not sure I could think of a cleaner way.

LOG.exception(msg)
abort(http_client.NOT_FOUND, msg)

@staticmethod
def _get_runner_by_id(id):
try:
Expand All @@ -70,18 +83,21 @@ def _get_runner_by_name(name):


class ParametersViewController(object):
def get_one(self, action_id, requester_user):
return self._get_one(action_id, requester_user=requester_user)
def get_one(self, ref_or_id, requester_user):
return self._get_one(ref_or_id, requester_user=requester_user)

@staticmethod
def _get_one(action_id, requester_user):
def _get_one(ref_or_id, requester_user):
"""
List merged action & runner parameters by action id.
List merged action & runner parameters by action id or ref.

Handle:
GET /actions/views/parameters/1
"""
action_db = LookupUtils._get_action_by_id(action_id)
if ResourceReference.is_resource_reference(ref_or_id):
action_db = LookupUtils._get_action_by_ref(ref_or_id)
else:
action_db = LookupUtils._get_action_by_id(ref_or_id)

permission_type = PermissionType.ACTION_VIEW
rbac_utils = get_rbac_backend().get_utils_class()
Expand Down Expand Up @@ -193,7 +209,7 @@ def get_all(
def _transform_action_api(action_api, requester_user):
action_id = action_api.id
result = ParametersViewController._get_one(
action_id=action_id, requester_user=requester_user
ref_or_id=action_id, requester_user=requester_user
)
action_api.parameters = result.get("parameters", {})
return action_api
Expand Down
14 changes: 13 additions & 1 deletion st2api/tests/unit/controllers/v1/test_action_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class ActionViewsParametersControllerTestCase(FunctionalTest):
@mock.patch.object(
action_validator, "validate_action", mock.MagicMock(return_value=True)
)
def test_get_one(self):
def test_get_one_by_id(self):
post_resp = self.app.post_json("/v1/actions", ACTION_1)
action_id = post_resp.json["id"]
try:
Expand All @@ -208,6 +208,18 @@ def test_get_one(self):
finally:
self.app.delete("/v1/actions/%s" % action_id)

@mock.patch.object(
action_validator, "validate_action", mock.MagicMock(return_value=True)
)
def test_get_one_by_ref(self):
post_resp = self.app.post_json("/v1/actions", ACTION_1)
action_ref = post_resp.json["ref"]
try:
get_resp = self.app.get("/v1/actions/views/parameters/%s" % action_ref)
self.assertEqual(get_resp.status_int, 200)
finally:
self.app.delete("/v1/actions/%s" % action_ref)


class ActionEntryPointViewControllerTestCase(FunctionalTest):
@mock.patch.object(
Expand Down
6 changes: 3 additions & 3 deletions st2common/st2common/openapi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,15 +465,15 @@ paths:
description: Unexpected error
schema:
$ref: '#/definitions/Error'
/api/v1/actions/views/parameters/{action_id}:
/api/v1/actions/views/parameters/{ref_or_id}:
get:
operationId: st2api.controllers.v1.action_views:parameters_view_controller.get_one
description: |
Get parameters for an action.
parameters:
- name: action_id
- name: ref_or_id
in: path
description: Entity id
description: Entity reference or id
type: string
required: true
x-parameters:
Expand Down
6 changes: 3 additions & 3 deletions st2common/st2common/openapi.yaml.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,15 +461,15 @@ paths:
description: Unexpected error
schema:
$ref: '#/definitions/Error'
/api/v1/actions/views/parameters/{action_id}:
/api/v1/actions/views/parameters/{ref_or_id}:
get:
operationId: st2api.controllers.v1.action_views:parameters_view_controller.get_one
description: |
Get parameters for an action.
parameters:
- name: action_id
- name: ref_or_id
in: path
description: Entity id
description: Entity reference or id
type: string
required: true
x-parameters:
Expand Down