| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-01 | |||
| 18:41:20 | mnaser | ugh | |
| 18:41:42 | mnaser | What does devstack use | |
| 18:42:04 | mnaser | I wonder how it hasn’t been affected | |
| 18:42:15 | melwitt | must not be using lite? | |
| 18:43:07 | melwitt | bc this was removed from the vnc_lite.html file, unless I'm missing something | |
| 18:43:45 | mnaser | Of course | |
| 18:43:58 | mnaser | NOVNC_FROM_PACKAGE defined in jobs | |
| 18:44:43 | melwitt | http://logs.openstack.org/65/627765/12/check/tempest-full-py3/b00b2ab/controller/logs/devstacklog.txt.gz#_2019-04-01_08_01_25_051 | |
| 18:45:17 | mnaser | Yikes well i guess we can ask novnc to revert | |
| 18:49:53 | melwitt | mnaser: I don't understand, how do you use lite vs not? | |
| 18:50:23 | mnaser | melwitt: vnc_lite seems to be what vnc_auto was a while ago | |
| 18:50:36 | mnaser | Searching the code doesn’t give any results for token | |
| 18:50:40 | melwitt | mnaser: and which one you use depends on the url you use? | |
| 18:51:05 | melwitt | yeah, ok. looks like we have to ask for a revert of at least that part | |
| 18:51:22 | mnaser | melwitt: I assume we’re using the lite which handled the token stuff and did the cookie handoff | |
| 18:51:45 | melwitt | mnaser: yeah, I'm just saying, how do you choose whether to use lite or not | |
| 18:51:56 | mnaser | dunno :P | |
| 18:52:01 | melwitt | haha ok | |
| 18:52:13 | mnaser | It seems like there’s something called token plugin | |
| 18:52:24 | melwitt | at first I was thinking, can we use non-lite but then you pointed out only lite was handling token | |
| 18:52:30 | melwitt | so my thought was moo | |
| 18:52:49 | mnaser | Don’t quote me on this stuff I’m going over it on my phone heh | |
| 18:53:30 | melwitt | :) | |
| 18:54:43 | melwitt | I do wonder why we have to rely on the cookie forwarding. I wonder why it doesn't just show up as a GET request param by the time we get it | |
| 18:54:59 | melwitt | from the way our code is written, it looks like the expectation was that the cookie was a fall back | |
| 18:55:14 | melwitt | but in practice, we were always falling back | |
| 18:56:41 | melwitt | going to see if I can find out | |
| 19:15:57 | openstackgerrit | Eric Fried proposed openstack/nova master: Correct lower-constraints.txt and the related tox job https://review.openstack.org/622972 | |
| 19:16:34 | openstackgerrit | Eric Fried proposed openstack/nova master: Correct lower-constraints.txt and the related tox job https://review.openstack.org/622972 | |
| 19:17:38 | openstackgerrit | Eric Fried proposed openstack/nova master: Mock time.sleep() in unit tests https://review.openstack.org/648762 | |
| 19:25:36 | mnaser | melwitt: so I think the problem here is the /websockify HTTP request comes afterwards | |
| 19:26:01 | mnaser | so its GET /vnc_lite.html?token=foobar .. and then CONNECT /websockify | |
| 19:26:12 | mnaser | so you can't really reliably tie that GET request and the CONNECT request | |
| 19:26:28 | mnaser | UNLESS we sent a Set-Cookie: token when you visit vnc_lite.html | |
| 19:26:36 | mnaser | that would workaround it | |
| 19:27:18 | melwitt | mnaser: I see. well, from some googling it looks like something like this might work? GET /vnc_lite.html?path=/websockify?token=foobar | |
| 19:28:19 | openstackgerrit | Eric Fried proposed openstack/nova master: Add placement as required project to functional py36 and 37 https://review.openstack.org/649068 | |
| 19:28:29 | melwitt | second ? should probably be a & | |
| 19:28:45 | mnaser | I think it would be cleaner to set-cookie | |
| 19:28:59 | melwitt | or maybe not | |
| 19:30:02 | melwitt | mnaser: ok. I can see if I can figure out how to do that, unless you want to | |
| 19:32:32 | mnaser | melwitt: im looking around right now, just need to figure out where the HTTP requests are handled in nova-novncconsole | |
| 19:32:35 | mnaser | ill welcome all help :P | |
| 19:32:44 | mnaser | *novncproxy | |
| 19:34:15 | mnaser | think it relies on upstream this https://github.com/novnc/websockify/blob/master/websockify/websocketproxy.py#L267 | |
| 19:34:26 | mnaser | self.token_plugin = kwargs.pop('token_plugin', None) | |
| 19:34:28 | mnaser | oh I wonder what that is | |
| 19:35:39 | melwitt | mnaser: I looked at that and it doesn't seem like something that would help us. but I could be wrong https://github.com/novnc/websockify/wiki/Token-based-target-selection | |
| 19:37:42 | mnaser | maybe we somehow override the html portion | |
| 19:37:42 | mnaser | https://github.com/novnc/websockify/blob/master/websockify/websocketserver.py#L35-L52 | |
| 19:37:48 | melwitt | mnaser: I think handling of HTTP (GET) requests might be this https://github.com/novnc/websockify/blob/6e09ec25486f431e2acc463f994c4eb4bebc6ddd/websockify/websockifyserver.py#L268 | |
| 19:37:57 | mnaser | oh that's even better | |
| 19:38:26 | melwitt | one of those, I don't really understand it yet | |
| 19:39:08 | efried | This one came across my desk again just now, can I please get a couple of pairs of core eyeballs on it? https://review.openstack.org/#/c/635533/ | |
| 19:41:10 | mnaser | I'm thinking https://github.com/novnc/websockify/wiki/Token-based-target-selection#custom-plugins | |
| 19:43:08 | melwitt | mnaser: hm. so instead of validating the token in new_websocket_client we'd do it in a new lookup method? | |
| 19:43:24 | melwitt | 'lookup' | |
| 19:43:29 | mnaser | melwitt: yeah, if we write a custom plugin, we can validate the token in it and then give it back to novnc | |
| 19:43:33 | efried | mriedem: This looks like a deadlock starting n-api? http://logs.openstack.org/36/649036/1/check/tempest-full-py3/a8c7223/controller/logs/screen-n-api.txt.gz | |
| 19:43:53 | melwitt | mnaser: it says lookup has to return a host:port tuple? | |
| 19:44:31 | mnaser | or none, none (if we don't validate I guess) | |
| 19:44:43 | efried | (maybe not, I guess the locks are being released quickly) | |
| 19:44:55 | mnaser | and the host:port tuple business can be grabbed from the db | |
| 19:46:27 | mnaser | the example module there seems to be reasonable | |
| 19:47:20 | mnaser | ugh but even then | |
| 19:47:26 | mnaser | I don't think anything will pass ?token=foo | |
| 19:48:08 | melwitt | mnaser: ok, I was trying to say when you said "give the token back to novnc" I don't think you can do that in lookup() | |
| 19:48:51 | mnaser | oooo | |
| 19:48:58 | melwitt | mnaser: earlier I was saying maybe we could do the entire validation in lookup() and then no longer need the token after that? | |
| 19:49:02 | mnaser | melwitt: https://github.com/novnc/websockify/blob/6e09ec25486f431e2acc463f994c4eb4bebc6ddd/websockify/websocketproxy.py#L164-L175 | |
| 19:49:16 | melwitt | yeah | |
| 19:49:47 | mnaser | so we get lookup called with the token, grab the stuff from the db, and return it back, and we can probably drop all that extra code | |
| 19:49:59 | melwitt | what do you mean by return it back? | |
| 19:50:23 | mnaser | so in the token plugin, we check the db, see if the var is still valid, and return (host, port) | |
| 19:50:36 | mnaser | and technically... I think none will just do the rest of the issues | |
| 19:50:43 | melwitt | ok. I thought you meant return the token | |
| 19:50:45 | melwitt | ok. that's what I was trying to say earlier | |
| 19:50:53 | mnaser | ah no I meant return the host/port, sorry distracted | |
| 19:51:00 | mnaser | people keep walking into my office >:( | |
| 19:51:09 | melwitt | how dare they! | |
| 19:52:39 | mnaser | let me try to hack at this | |
| 19:54:12 | melwitt | sounds good. I will review it when it's ready :) | |
| 19:54:27 | mnaser | honestly I think this could completely eliminate the websocketproxy.py | |
| 19:54:53 | mnaser | a lot of that stuff exists in webproxify but I dunno how much of this can be back portable at this point.. | |
| 19:55:28 | melwitt | maybe. there is some extra stuff in there though | |
| 20:19:18 | mnaser | melwitt: well, I got a nice refactor out of it, but still I think it expects I to be there | |
| 20:21:16 | melwitt | mnaser: yeah, you'd have to change https://github.com/openstack/nova/blob/master/nova/cmd/baseproxy.py#L68 too | |
| 20:23:32 | mnaser | ok well to get it just to work I found a gross fix.. I think | |
| 20:23:38 | mnaser | let me test it against the original code | |
| 20:23:49 | mnaser | vnc_lite.html?path=?token=<token> seems to be calling my token plugin | |
| 20:25:14 | mnaser | But I think then the original code works | |
| 20:25:22 | mnaser | We would just need to tweak the default value :/ | |
| 20:25:49 | melwitt | yeah, shouldn't need path= right | |
| 20:25:58 | melwitt | default value of what? | |
| 20:27:45 | mnaser | The nova configuration value | |
| 20:27:51 | mnaser | The one pointing at vnc_lite or whatever | |
| 20:28:19 | mnaser | To not longer suggest ?token=foo is the way but ?path=?token=foo | |
| 20:28:39 | melwitt | I thought earlier you told me including path= wasn't as clean :P | |
| 20:28:51 | mnaser | Yeah it’s just BLAH | |
| 20:28:57 | mnaser | why people gotta rip code out | |