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.
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
Added mask of rule action secret parameters #4788
Changes from all commits
cfe0aa7
0f1bae2
c604c15
4f7f7a1
36980e5
6b9eb4e
42eb393
76cd651
5defcda
421321d
0cc4cb5
c07522a
8b01949
9f3feaf
7ff661e
4182f4b
46dd3fa
7ad78a9
4c0fd03
3271499
4ffaa6f
4d7e552
ee220c1
b73e1c3
a202c4b
1ca2854
a644014
8faa9ab
a1b97c6
07d4eb7
b3a7531
8277c78
61b8b58
f1c4de4
8b85474
cb3b35c
4614d44
8e80197
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,11 +13,16 @@ | |
# limitations under the License. | ||
|
||
from __future__ import absolute_import | ||
import copy | ||
import mongoengine as me | ||
|
||
from mongoengine.queryset import Q | ||
from st2common.persistence.action import Action | ||
from st2common.models.db import MongoDBAccess | ||
from st2common.models.db import stormbase | ||
from st2common.constants.types import ResourceType | ||
from st2common.util.secrets import get_secret_parameters | ||
from st2common.util.secrets import mask_secret_parameters | ||
|
||
|
||
class RuleTypeDB(stormbase.StormBaseDB): | ||
|
@@ -101,6 +106,55 @@ class RuleDB(stormbase.StormFoundationDB, stormbase.TagsMixin, | |
stormbase.UIDFieldMixin.get_indexes()) | ||
} | ||
|
||
def mask_secrets(self, value): | ||
""" | ||
Process the model dictionary and mask secret values. | ||
|
||
:type value: ``dict`` | ||
:param value: Document dictionary. | ||
|
||
:rtype: ``dict`` | ||
""" | ||
result = copy.deepcopy(value) | ||
if('action' not in result): | ||
return result | ||
action_db = self._get_referenced_models(rule=result) | ||
|
||
secret_parameters = get_secret_parameters(parameters=action_db['parameters']) | ||
result['action']['parameters'] = mask_secret_parameters( | ||
parameters=result['action']['parameters'], | ||
secret_parameters=secret_parameters) | ||
|
||
return result | ||
|
||
def _get_referenced_models(self, rule): | ||
""" | ||
Return the action model referenced from rule. | ||
|
||
:type rule: ``dict`` | ||
:param rule: rule | ||
|
||
:rtype: ``ActionDB`` | ||
""" | ||
action_ref = rule['action']['ref'] | ||
|
||
def ref_query_args(ref): | ||
return {'ref': ref} | ||
|
||
action_db = self._get_entity(model_persistence=Action, | ||
ref=action_ref, | ||
query_args=ref_query_args) | ||
|
||
return action_db | ||
|
||
def _get_entity(self, model_persistence, ref, query_args): | ||
q = Q(**query_args(ref)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we really only need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On a related note - is there a specific reason why you used To not break the abstraction, we should probably utilize There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a heads up. I will look into making the changes mentioned above^. This way we can hopefully get this across the finish line and merge it soon. |
||
|
||
if q: | ||
return model_persistence._get_impl().model.objects(q).first() | ||
|
||
return None | ||
|
||
def __init__(self, *args, **values): | ||
super(RuleDB, self).__init__(*args, **values) | ||
self.ref = self.get_reference().ref | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this change looks good to me, my only concern is performance impact of this change.
This change means we now need to perform 1 additional query for each Rule DB object.
This is not an issue for "get one" operation (only one additional query), but when retrieving multiple rules aka "get all" API operations, this would result in
N
additional queries whereN
is number of rules on the page.