st2 fails to store a Key-Value in the datastore if there is a "/" in the key name.

[RaviTezu]

SUMMARY

st2 fails to store a Key-Value in the datastore if there is a "/" in the key name. I am sorry if I missed anything in the docs, which says that you cannot store a key with "/" in it and please feel free to close this issue if that is the case. Thank you.

STACKSTORM VERSION

st2 3.1.0, on Python 2.7.5

OS, environment, install method:

CentOS/Docker, custom install.

Steps to reproduce the problem

bash-4.2$ st2 key set "foo / bar" "foo_bar"
ERROR: 404 Client Error: Not Found
MESSAGE: The resource could not be found. for url: http://127.0.0.1:9101/v1/keys/foo%20/%20bar

Expected Results

st2 should have accepted the key-value pair and stored it.

Actual Results

st2 has failed to store the key-value pair.

[armab]

If we can't save key with / in k/v storage due to API endpoints, - we should at least output validation error.

[RaviTezu]

Hi @armab I would like to take this up and send a fix.

[Kami]

I believe this issue probably lies in st2client / CLI and not the API itself (likely / character is not correctly URL encoded).

I believe we even have some st2api API level tests for keys with / in the name (and if we don't, we should add some).

[RaviTezu]

Hi @Kami
I tried via the st2 command line utility and also from the actions I have implemented, both self.action_service.set_value and self.action_service.get_value are failing with the same error. So I assume, it should be an API level issue.

[Kami]

It could be, although action service also utilizes st2client code which talks to the API :)

If there is not an existing test already, I would start with an API level test for that functionality.

[RaviTezu]

@Kami @armab I see no tests which stores a key-value pair with a "/" in the key name.

Also, I am not able to run the tests in test_kvps.py file using the following command from the root directory as mentioned in this doc on a Ubuntu machine:
nosetests --nocapture st2api/tests/unit/controllers/v1/test_kvps.py
It says ERROR: Failure: ImportError (No module named st2tests.api)

I tried exporting/setting PYTHONPATH to st2 directory but no luck. Could you please help me to get the tests running ? I tried with both Python2 and Python3.

Thanks.

[Kami]

For developing StackStorm platform itself, I would recommend you to use this Vagrant image - https://github.com/StackStorm/st2vagrantdev

In short, you need to run make requirements which will create virtualenv, install all the dependencies and set PYTHONPATH correctly.

[RaviTezu]

@Kami Thanks. This is Great.
I am able to setup the vagrant environment and I can run the tests now. Also, I have added a test which stores a KV and there is a "/" in the key name and it fails. I will dig through the code and see, if we can encode "/" someway.
@armab

[RaviTezu]

@Kami @armab
I modified the serialize method here to quote (from six.moves.urllib_parse import quote) so that the key for example a/b will be converted into a%2Fb. However, the st2 key set "a/b" "some value" is failing with resource could not be found error.

ERROR: 404 Client Error: Not Found
MESSAGE: The resource could not be found. for url: http://127.0.0.1:9101/v1/keys/a%2Fb

I think, I will have to make some modifications on the API as well to accept this kind of keys?
Thank you.

[RaviTezu]

Hi @Kami @armab Do you think, we should be able to encode and store keys with /? or I will try to implement something which will throw an error if the key has / in it. Thank you.

[armab]

Yes, I think it's absolutely reasonable to be able to use the / in Key name.
Quick example: https://www.consul.io/docs/commands/kv/put.html#examples

If you could make it work and support your enhancement with tests as well, - that would be a great addition :+1:

[RaviTezu]

Thanks for the reply @armab
As I mentioned in my previous comment, I can encode the key on the st2client side and I can see the PUT call is also made on the encoded key.
See below example:

(virtualenv) vagrant@ubuntu-xenial:~/local/st2$ st2 key set foo/bar foo
# -------- begin 140367719226128 request ----------
curl -X PUT -H  'Connection: keep-alive' -H  'Accept-Encoding: gzip, deflate' -H  'Accept: */*' -H  'User-Agent: python-requests/2.23.0' -H  'content-type: application/json' -H  'Content-Length: 70' --data-binary '{"scope": "system", "name": "foo%2Fbar", "value": "foo", "user": null}' http://127.0.0.1:9101/v1/keys/foo%2Fbar
# -------- begin 140367719226128 response ----------
{
    "faultstring": "The resource could not be found."
}
# -------- end 140367719226128 response ------------

ERROR: 404 Client Error: Not Found
MESSAGE: The resource could not be found. for url: http://127.0.0.1:9101/v1/keys/foo%2Fbar

I am trying to find the code which is accepting this request and make it work for encoded / or do you think I am missing something? Any help in making me understand the api side of things would be appreciated. Any docs will also help. Thank you.
@Kami

[RaviTezu]

@Kami @armab Where can I find the API side related code? I tried searching in st2api but couldn't quite find it. Do I have to check any gunicorn or nginx config somewhere?

As you can see in my previous comment, API isn't considering a key with encoded / as a new key in the PUT call.
Could you please direct me to some documentation or any other reference/example? Thank you.

[armab]

Start with the st2 api KeyValue controller in: https://github.com/StackStorm/st2/blob/master/st2api/st2api/controllers/v1/keyvalue.py
For development environment, standards and expectations, check the https://docs.stackstorm.com/development/index.html

[RaviTezu]

@armab @Kami It looks like if I encode a key foo/bar to foo%2Fbar on the client side, the call which will be made will look like this:

curl -X PUT -H  'Connection: keep-alive' -H  'Accept-Encoding: gzip, deflate' -H  'Accept: */*' -H  'User-Agent: python-requests/2.23.0' -H  'content-type: application/json' -H  'Content-Length: 69' --data-binary '{"scope": "system", "name": "foo%2Fbar", "value": "f1", "user": null}' http://127.0.0.1:9101/v1/keys/foo%2Fbar

But on the api side, it is failing while trying to match the requests path to a controller here.
For some reason, the req.path is going back to foo/bar instead of being the encodedfoo%2Fbar. As you can see, this happening before entering the code in https://github.com/StackStorm/st2/blob/master/st2api/st2api/controllers/v1/keyvalue.py

So I am guessing somewhere else, we need to make change to persist the encoded key.

[amanda11]

I did this little test based on the code in router.py:

import webob.compat
path="http://127.0.0.1:9101/v1/keys/foo%2Fbar"
webob.compat.url_unquote(path)
'http://127.0.0.1:9101/v1/keys/foo/bar'

So it looks like it gets altered to foo/bar by the url_unquote call.

I believe the webob.compat inturn calls the urllib method, which is defined to be:
urllib.parse.unquote(string, encoding='utf-8', errors='replace')¶
Replace %xx escapes by their single-character equivalent. The optional encoding and errors parameters specify how to decode percent-encoded sequences into Unicode characters, as accepted by the bytes.decode() method.

So that seems to be the behaviour of the unquote.

[RaviTezu]

Thank you for the input @amanda11.
I just tried with double encoding/quoting the key name if it contains a / while I set or get the KV pair and it is working as expected. If this is OK, I will go ahead and submit a pull request.
@Kami @armab

git diff:

diff --git a/st2client/st2client/commands/keyvalue.py b/st2client/st2client/commands/keyvalue.py
index 8eed47364..d1a2305eb 100644
--- a/st2client/st2client/commands/keyvalue.py
+++ b/st2client/st2client/commands/keyvalue.py
@@ -21,6 +21,7 @@ import logging
 from os.path import join as pjoin

 import six
+from six.moves.urllib_parse import quote

 from st2client.commands import resource
 from st2client.commands.noop import NoopCommand
@@ -141,6 +142,8 @@ class KeyValuePairGetCommand(resource.ResourceGetCommand):
     @resource.add_auth_token_to_kwargs_from_cli
     def run(self, args, **kwargs):
         resource_name = getattr(args, self.pk_argument_name, None)
+        if '/' in resource_name:
+            resource_name = quote(quote(resource_name, safe=''))
         decrypt = getattr(args, 'decrypt', False)
         scope = getattr(args, 'scope', DEFAULT_GET_SCOPE)
         kwargs['params'] = {'decrypt': str(decrypt).lower()}
@@ -185,8 +188,14 @@ class KeyValuePairSetCommand(resource.ResourceCommand):
     @resource.add_auth_token_to_kwargs_from_cli
     def run(self, args, **kwargs):
         instance = KeyValuePair()
-        instance.id = args.name  # TODO: refactor and get rid of id
-        instance.name = args.name
+        key_name = args.name
+        # urllib_parse.quote the key name to support keys with '/' in them.
+        # We double quote it here, as it will unquoted once on the API side.
+        if '/' in key_name:
+            key_name = quote(quote(args.name, safe=''))
+
+        instance.id = key_name  # TODO: refactor and get rid of id
+        instance.name = key_name
         instance.value = args.value
         instance.scope = args.scope
         instance.user = args.user