Re: [Kimchi-devel] [PATCH] [WoK] Asynchronous UI notification implementation

On 02/28/2017 10:31 AM, Aline Manera wrote: > > > On 02/28/2017 10:24 AM, Daniel Henrique Barboza wrote: >> >> >> On 02/28/2017 10:02 AM, Lucio Correia wrote: >>> On 27/02/2017 18:30, Daniel Henrique Barboza wrote: >>>> >>>> >>>> On 02/27/2017 06:18 PM, Lucio Correia wrote: >>>>> On 27/02/2017 18:12, Daniel Henrique Barboza wrote: >>>>>>>>> +BASE_DIRECTORY = '/run' >>>>>>>> Suggestion to use: >>>>>>>> os.path.join('/run/user', str(os.getuid())) >>>>>>>> in order tests may be run with root. >>>>>> >>>>>> I would prefer to choose a path that can be written by anyone >>>>>> else to >>>>>> allow the push_server >>>>>> to be started without root. >>>>> >>>>> Sorry, I meant *without* root. >>>>> /run/user/<UID> allows for that, it's writable by the user that >>>>> started wokd, be it root or not. >>>>> >>>> This change would require further UI changes to allow the /config API >>>> (or other) to inform >>>> the UI of the current websocket URL. This will not solve the problems >>>> I've seen with the unit tests >>>> though - multiple instances of the push_server will not be >>>> possible and >>>> the unit tests will >>>> break. >>> >>> I'm not seeing the relation between that BASE_DIR and the URL used >>> by UI. Anyway, I believe we both agree that just one server and one >>> URL is fine. >> The unix socket, now /run/woknotifications, is used by the push >> server to send >> notifications. The websocket creates a proxy between this unix >> socket and a >> common TCP socket that the UI can read/write to. >> >> This connection is made by the following URL: >> >> <host:port>/websockify?token=<token> >> >> The UI knows what token to use to connect to the unix socket because >> the >> token is based on the file path of the unix socket. This is the >> relevant UI >> code: >> >> var token = wok.urlSafeB64Encode('woknotifications').replace(/=*$/g, >> ""); >> >> >> This means that if I change the file path to /run/user_id, the token >> changes too. If the UI can't >> predict what unix socket is being used in WoK then we need to supply >> this info to the UI >> in another way. > > Daniel, for the novnc/spice/serial console, the token is the virtual > machine name, ie, it is not related to the real path in the system. > The websocket knows on which directory to look for the token and it > is all. > > Why is it different for the notifications? Shouldn't the directory be > known by the server and the UI only request a token? The same as > novnc/spice/serial does. In these cases the UI doesn't request a token, the UI already knows it. It is embedded in the URL. kimchi.api.js: serialToVM : function(vm) { (...) url = 'https://' + location.hostname + ':' + proxy_port; url += server_root; url += "/plugins/kimchi/serial/html/serial.html"; url += "?port=" + proxy_port; url += "&path=" + server_root + "/websockify"; url += "?token=" + wok.urlSafeB64Encode(vm+'-console').replace(/=*$/g, ""); url += '&encrypt=1'; (same thing with novnc and spice, check vncToVM and spiceToVM in the same file) In serial.html the token is retrieved as follows: var url = 'wss://' + window.location.hostname + ':' + params.get('port'); url += '/' + params.get('path'); url += '?token=' + params.get('token'); var socket = new WebSocket(url, ['base64']); Note that for serial,novnc and spice the token is already knew by the UI due to the URL. In the situation we're describing with the push server this will not be case - we can't predict the current user information and embed it in the URL (not in the frontend level, at least).

This is why I am saying that we need to deliver this information in another fashion to the UI, perhaps using the /config API.

> >> >>> >>> My only concern is the directory to save that woknotifications file >>> and the permissions required for it. >>> >>> >>>> >>>> We need to discuss an alternative where: >>>> >>>> - any user can start the push_server, as you require >>> >>> I'm not requiring it. Just saying that if it will be started by >>> tests, it needs to run without sudo (i.e. use paths in filesystems >>> that allow for that). >>> >>> As I mentioned earlier, testing for 'test' option is not enough to >>> avoid PushServer to be started during tests, since as of now 'test' >>> option only governs which model will be used. >> >> Strange, it worked for me. I think you're referring to WoK changes >> that were made >> right after/before I sent this patch set. I'll re-test it and see >> how I can prevent it >> from running on test mode. >> >>> >>> >>>> >>>> - if we're to allow the push_server to be run in test_mode*, we >>>> need to >>>> think in a way of >>>> generating random paths that can be written by any user as well. >>>> You are >>>> working closely >>>> in Debian changes. What system dir can be used that can be written by >>>> any user and, >>>> preferably, exists in RPM distros too? >>> >>> On Fedora 25, /run/user/<UID>/libvirt is the default dir used to >>> store runtime data by libvirt when it is started with >>> "qemu:///session" url (regular user session). >>> >>> In fact it seems to be default place for that kind of usage in both >>> distros: >>> >>> Ubuntu 16.04: >>> $ ls /run/user/1000/ >>> dbus-session gnome-shell gvfs-burn pulse systemd >>> upstart-dbus-bridge.4451.pid upstart-udev-bridge.2946.pid >>> dconf gvfs keyring speech-dispatcher upstart >>> upstart-file-bridge.4451.pid upstart-udev-bridge.4451.pid >>> >>> Fedora 25: >>> $ ls /run/user/1000/ >>> bus libvirt systemd >> >> I see. Perhaps changing the dir to /run/user/<UID> is the right >> thing to do in >> this case. This means that we can't avoid "breaking" the current >> design - we'll >> need to use an additional API to inform the UI of the current unix >> socket (token) >> being used. >> >> >>> >>> >> >> _______________________________________________ >> Kimchi-devel mailing list >> Kimchi-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >> >