Merge 8464364 into 5db0478

CHANGELOG.rst

@@ -38,6 +38,8 @@ Fixed
 * Replace ``sseclient`` library on which st2client and CLI depends on with ``sseclient-py``.
   ``sseclient`` has various issue which cause client to sometimes hang and keep the connection open
   which also causes ``st2 execution tail`` command to hang for a long time. (improvement) #4686
+* Fix orquesta st2kv to return empty string and null values. (bug fix) #4678
+* Allow the orquesta st2kv function to return default for nonexistent key. (improvement) #4678
 
 3.0.0 - April 18, 2019
 ----------------------

contrib/examples/actions/orquesta-st2kv-default.yaml

@@ -0,0 +1,17 @@
+---
+name: orquesta-st2kv-default
+description: A sample workflow that demonstrates st2kv usage
+runner_type: orquesta
+entry_point: workflows/orquesta-st2kv-default.yaml
+enabled: true
+parameters:
+  key_name:
+    required: true
+    type: string
+  decrypt:
+    required: false
+    type: boolean
+    default: false
+  default:
+    required: False
+    type: string

contrib/examples/actions/workflows/orquesta-st2kv-default.yaml

@@ -0,0 +1,16 @@
+version: 1.0
+
+description: A sample workflow that demonstrates st2kv usage.
+
+input:
+  - key_name
+  - decrypt
+  - default: null
+
+output:
+  - value_from_yaql: <% st2kv(ctx().key_name, decrypt => ctx().decrypt, default => ctx().default) %>
+  - value_from_jinja: "{{ st2kv(ctx().key_name, decrypt=ctx().decrypt, default=ctx().default) }}"
+
+tasks:
+  task1:
+    action: core.noop

contrib/examples/actions/workflows/orquesta-st2kv.yaml

@@ -7,14 +7,10 @@ input:
   - decrypt
 
 output:
-  - value: <% ctx().value %>
+  - value: <% task(task1).result.stdout %>
 
 tasks:
-  create_vm:
+  task1:
     action: core.local
     input:
       cmd: "echo <% st2kv(ctx().key_name, decrypt => ctx().decrypt) %>"
-    next:
-      - when: <% succeeded() %>
-        publish:
-          - value: <% result().stdout %>

contrib/runners/orquesta_runner/orquesta_functions/st2kv.py

@@ -18,17 +18,20 @@
 
 from orquesta import exceptions as exc
 
+from st2common.exceptions import db as db_exc
 from st2common.persistence import auth as auth_db_access
 from st2common.util import keyvalue as kvp_util
 
 
 LOG = logging.getLogger(__name__)
 
 
-def st2kv_(context, key, decrypt=False):
+def st2kv_(context, key, **kwargs):
     if not isinstance(key, six.string_types):
         raise TypeError('Given key is not typeof string.')
 
+    decrypt = kwargs.get('decrypt', False)
+
     if not isinstance(decrypt, bool):
         raise TypeError('Decrypt parameter is not typeof bool.')
 
@@ -43,11 +46,15 @@ def st2kv_(context, key, decrypt=False):
         raise Exception('Failed to retrieve User object for user "%s" % (username)' %
                         (six.text_type(e)))
 
-    kvp = kvp_util.get_key(key=key, user_db=user_db, decrypt=decrypt)
-
-    if not kvp:
-        raise exc.ExpressionEvaluationException(
-            'Key %s does not exist in StackStorm datastore.' % key
-        )
+    has_default = 'default' in kwargs
+    default_value = kwargs.get('default')
 
-    return kvp
+    try:
+        return kvp_util.get_key(key=key, user_db=user_db, decrypt=decrypt)
+    except db_exc.StackStormDBObjectNotFoundError as e:
+        if not has_default:
+            raise exc.ExpressionEvaluationException(str(e))
+        else:
+            return default_value
+    except Exception as e:
+        raise exc.ExpressionEvaluationException(str(e))

contrib/runners/orquesta_runner/tests/unit/test_functions_st2kv.py

@@ -14,6 +14,8 @@
 
 from __future__ import absolute_import
 
+import mock
+import six
 import unittest2
 
 import st2tests
@@ -31,16 +33,17 @@
 from st2common.models.db import keyvalue as kvp_db
 from st2common.persistence import keyvalue as kvp_db_access
 from st2common.util import crypto
+from st2common.util import keyvalue as kvp_util
 
 
-MOCK_ORCHESTRA_CTX = {'__vars': {'st2': {'user': 'stanley'}}}
-MOCK_ORCHESTRA_CTX_NO_USER = {'__vars': {'st2': {}}}
+MOCK_CTX = {'__vars': {'st2': {'user': 'stanley'}}}
+MOCK_CTX_NO_USER = {'__vars': {'st2': {}}}
 
 
 class DatastoreFunctionTest(unittest2.TestCase):
 
     def test_missing_user_context(self):
-        self.assertRaises(KeyError, st2kv.st2kv_, MOCK_ORCHESTRA_CTX_NO_USER, 'foo')
+        self.assertRaises(KeyError, st2kv.st2kv_, MOCK_CTX_NO_USER, 'foo')
 
     def test_invalid_input(self):
         self.assertRaises(TypeError, st2kv.st2kv_, None, 123)
@@ -57,45 +60,76 @@ def setUpClass(cls):
         super(UserScopeDatastoreFunctionTest, cls).setUpClass()
         user = auth_db.UserDB(name='stanley')
         user.save()
-
-    def setUp(self):
-        super(UserScopeDatastoreFunctionTest, self).setUp()
         scope = kvp_const.FULL_USER_SCOPE
+        cls.kvps = {}
 
-        # Plain key
-        key_id = 'stanley:foo'
-        instance = kvp_db.KeyValuePairDB(name=key_id, value='bar', scope=scope)
-        self.kvp = kvp_db_access.KeyValuePair.add_or_update(instance)
+        # Plain keys
+        keys = {
+            'stanley:foo': 'bar',
+            'stanley:foo_empty': '',
+            'stanley:foo_null': None
+        }
+
+        for k, v in six.iteritems(keys):
+            instance = kvp_db.KeyValuePairDB(name=k, value=v, scope=scope)
+            cls.kvps[k] = kvp_db_access.KeyValuePair.add_or_update(instance)
 
         # Secret key
-        key_id = 'stanley:fu'
-        value = crypto.symmetric_encrypt(kvp_api.KeyValuePairAPI.crypto_key, 'bar')
-        instance = kvp_db.KeyValuePairDB(name=key_id, value=value, scope=scope, secret=True)
-        self.secret_kvp = kvp_db_access.KeyValuePair.add_or_update(instance)
+        keys = {
+            'stanley:fu': 'bar',
+            'stanley:fu_empty': ''
+        }
 
-    def tearDown(self):
-        if hasattr(self, 'kvp') and self.kvp:
-            self.kvp.delete()
+        for k, v in six.iteritems(keys):
+            value = crypto.symmetric_encrypt(kvp_api.KeyValuePairAPI.crypto_key, v)
+            instance = kvp_db.KeyValuePairDB(name=k, value=value, scope=scope, secret=True)
+            cls.kvps[k] = kvp_db_access.KeyValuePair.add_or_update(instance)
 
-        if hasattr(self, 'secret_kvp') and self.secret_kvp:
-            self.secret_kvp.delete()
+    @classmethod
+    def tearDownClass(cls):
+        for k, v in six.iteritems(cls.kvps):
+            v.delete()
 
-        super(UserScopeDatastoreFunctionTest, self).tearDown()
+        super(UserScopeDatastoreFunctionTest, cls).tearDownClass()
 
     def test_key_exists(self):
-        self.assertEqual(st2kv.st2kv_(MOCK_ORCHESTRA_CTX, 'foo'), 'bar')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'foo'), 'bar')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'foo_empty'), '')
+        self.assertIsNone(st2kv.st2kv_(MOCK_CTX, 'foo_null'))
 
     def test_key_does_not_exist(self):
-        self.assertRaises(
+        self.assertRaisesRegexp(
             exc.ExpressionEvaluationException,
+            'The key ".*" does not exist in the StackStorm datastore.',
             st2kv.st2kv_,
-            MOCK_ORCHESTRA_CTX,
+            MOCK_CTX,
             'foobar'
         )
 
-    def test_key_decrypt(self):
-        self.assertEqual(st2kv.st2kv_(MOCK_ORCHESTRA_CTX, 'fu', decrypt=True), 'bar')
+    def test_key_does_not_exist_but_return_default(self):
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'foobar', default='foosball'), 'foosball')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'foobar', default=''), '')
+        self.assertIsNone(st2kv.st2kv_(MOCK_CTX, 'foobar', default=None))
 
+    def test_key_decrypt(self):
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'fu'), 'bar')
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'fu', decrypt=False), 'bar')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'fu', decrypt=True), 'bar')
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'fu_empty'), '')
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'fu_empty', decrypt=False), '')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'fu_empty', decrypt=True), '')
+
+    @mock.patch.object(
+        kvp_util, 'get_key',
+        mock.MagicMock(side_effect=Exception('Mock failure.')))
+    def test_get_key_exception(self):
+        self.assertRaisesRegexp(
+            exc.ExpressionEvaluationException,
+            'Mock failure.',
+            st2kv.st2kv_,
+            MOCK_CTX,
+            'foo'
+        )
 
 class SystemScopeDatastoreFunctionTest(st2tests.ExecutionDbTestCase):
 
@@ -104,41 +138,73 @@ def setUpClass(cls):
         super(SystemScopeDatastoreFunctionTest, cls).setUpClass()
         user = auth_db.UserDB(name='stanley')
         user.save()
-
-    def setUp(self):
-        super(SystemScopeDatastoreFunctionTest, self).setUp()
         scope = kvp_const.FULL_SYSTEM_SCOPE
+        cls.kvps = {}
 
         # Plain key
-        key_id = 'foo'
-        instance = kvp_db.KeyValuePairDB(name=key_id, value='bar', scope=scope)
-        self.kvp = kvp_db_access.KeyValuePair.add_or_update(instance)
+        keys = {
+            'foo': 'bar',
+            'foo_empty': '',
+            'foo_null': None
+        }
+
+        for k, v in six.iteritems(keys):
+            instance = kvp_db.KeyValuePairDB(name=k, value=v, scope=scope)
+            cls.kvps[k] = kvp_db_access.KeyValuePair.add_or_update(instance)
 
         # Secret key
-        key_id = 'fu'
-        value = crypto.symmetric_encrypt(kvp_api.KeyValuePairAPI.crypto_key, 'bar')
-        instance = kvp_db.KeyValuePairDB(name=key_id, value=value, scope=scope, secret=True)
-        self.secret_kvp = kvp_db_access.KeyValuePair.add_or_update(instance)
+        keys = {
+            'fu': 'bar',
+            'fu_empty': ''
+        }
 
-    def tearDown(self):
-        if hasattr(self, 'kvp') and self.kvp:
-            self.kvp.delete()
+        for k, v in six.iteritems(keys):
+            value = crypto.symmetric_encrypt(kvp_api.KeyValuePairAPI.crypto_key, v)
+            instance = kvp_db.KeyValuePairDB(name=k, value=value, scope=scope, secret=True)
+            cls.kvps[k] = kvp_db_access.KeyValuePair.add_or_update(instance)
 
-        if hasattr(self, 'secret_kvp') and self.secret_kvp:
-            self.secret_kvp.delete()
+    @classmethod
+    def tearDownClass(cls):
+        for k, v in six.iteritems(cls.kvps):
+            v.delete()
 
-        super(SystemScopeDatastoreFunctionTest, self).tearDown()
+        super(SystemScopeDatastoreFunctionTest, cls).tearDownClass()
 
     def test_key_exists(self):
-        self.assertEqual(st2kv.st2kv_(MOCK_ORCHESTRA_CTX, 'system.foo'), 'bar')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'system.foo'), 'bar')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'system.foo_empty'), '')
+        self.assertIsNone(st2kv.st2kv_(MOCK_CTX, 'system.foo_null'))
 
     def test_key_does_not_exist(self):
-        self.assertRaises(
+        self.assertRaisesRegexp(
             exc.ExpressionEvaluationException,
+            'The key ".*" does not exist in the StackStorm datastore.',
             st2kv.st2kv_,
-            MOCK_ORCHESTRA_CTX,
+            MOCK_CTX,
             'foo'
         )
 
+    def test_key_does_not_exist_but_return_default(self):
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'system.foobar', default='foosball'), 'foosball')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'system.foobar', default=''), '')
+        self.assertIsNone(st2kv.st2kv_(MOCK_CTX, 'system.foobar', default=None))
+
     def test_key_decrypt(self):
-        self.assertEqual(st2kv.st2kv_(MOCK_ORCHESTRA_CTX, 'system.fu', decrypt=True), 'bar')
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'system.fu'), 'bar')
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'system.fu', decrypt=False), 'bar')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'system.fu', decrypt=True), 'bar')
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'system.fu_empty'), '')
+        self.assertNotEqual(st2kv.st2kv_(MOCK_CTX, 'system.fu_empty', decrypt=False), '')
+        self.assertEqual(st2kv.st2kv_(MOCK_CTX, 'system.fu_empty', decrypt=True), '')
+
+    @mock.patch.object(
+        kvp_util, 'get_key',
+        mock.MagicMock(side_effect=Exception('Mock failure.')))
+    def test_get_key_exception(self):
+        self.assertRaisesRegexp(
+            exc.ExpressionEvaluationException,
+            'Mock failure.',
+            st2kv.st2kv_,
+            MOCK_CTX,
+            'system.foo'
+        )

st2common/st2common/persistence/keyvalue.py

@@ -13,11 +13,13 @@
 # limitations under the License.
 
 from __future__ import absolute_import
+
 from st2common import log as logging
 from st2common.constants.triggers import KEY_VALUE_PAIR_CREATE_TRIGGER
 from st2common.constants.triggers import KEY_VALUE_PAIR_UPDATE_TRIGGER
 from st2common.constants.triggers import KEY_VALUE_PAIR_VALUE_CHANGE_TRIGGER
 from st2common.constants.triggers import KEY_VALUE_PAIR_DELETE_TRIGGER
+from st2common.exceptions.db import StackStormDBObjectNotFoundError
 from st2common.models.api.keyvalue import KeyValuePairAPI
 from st2common.models.db.keyvalue import keyvaluepair_access
 from st2common.models.system.common import ResourceReference
@@ -107,6 +109,11 @@ def get_by_scope_and_name(cls, scope, name):
         :rtype: :class:`KeyValuePairDB` or ``None``
         """
         query_result = cls.impl.query(scope=scope, name=name)
+
+        if not query_result:
+            msg = 'The key "%s" does not exist in the StackStorm datastore.'
+            raise StackStormDBObjectNotFoundError(msg % name)
+
         return query_result.first() if query_result else None
 
     @classmethod

st2common/st2common/services/config.py

@@ -24,6 +24,7 @@
 from st2common.util.crypto import symmetric_decrypt
 from st2common.models.api.keyvalue import KeyValuePairAPI
 from st2common.persistence.keyvalue import KeyValuePair
+from st2common.exceptions.db import StackStormDBObjectNotFoundError
 
 __all__ = [
     'set_datastore_value_for_config_key',
@@ -65,7 +66,11 @@ def set_datastore_value_for_config_key(pack_name, key_name, value, secret=False,
     kvp_db = KeyValuePairAPI.to_model(kvp_api)
 
     # TODO: Obtain a lock
-    existing_kvp_db = KeyValuePair.get_by_scope_and_name(scope=scope, name=name)
+    try:
+        existing_kvp_db = KeyValuePair.get_by_scope_and_name(scope=scope, name=name)
+    except StackStormDBObjectNotFoundError:
+        existing_kvp_db = None
+
     if existing_kvp_db:
         kvp_db.id = existing_kvp_db.id