Vojtech,
Your assumption about the structure of the pluginDefinitions object is correct.
It’s no longer a String->String mapping , but a String to Object mapping.
I liked the original IIFE approach, except that it seemed that having
additional static resources (jquery, images, html templates, etc) was going to
be more cumbersome. I don’t think having the plugin author write a basic
start.html is that big of a burden :).
I agree that the plugin configuration was always going to be a resource
(probably a local file) that the end user could customize. I’m not sure it I
really needs to be separate from the plugin definition file
(/usr/share/ovirt-engine/ui-plugins/test.json). I suppose it depends on how
complex the configuration is going to be and on some of the implementation
details surrounding the plugin definition file.
In my patch, I simply used Jackson to parse the file into a tree of JsonNodes.
Should the plugin definition be a java object of some sort? (please please
please don’t make me learn about java beans…). I stuck with the JsonNodes
because Jackson makes them easy to work with and they’re really easy to
re-serialize back to json to give to the webadmin.
We should probably turn on JsonParser.Feature.ALLOW_COMMENTS. The definition
and config files will difficult for end-users (or even developers) to
understand without comments.
We need to formalize the structure of the plugin definition and decide which
fields are mandatory and which are optional:
{
# Mandatory fields: name, enabled, version, url, apiversion, author, license
# Name of the plugin
"name": "test",
# Whether or not plugin is enabed
"enabled": true,
# version of the plugin
"version": "1.0",
# How to load the plugin
"url": "/webadmin/webadmin/plugin/test/start.html",
# Which version of engine plugin is meant to work with
"apiversion": "3.1.0",
# Who wrote the plugin and how is it licensed?
"author": "SuperBig Corporation",
"license": "Proprietary",
# Optional fields path, config
# Where to locate plugin (if loaded by webadmin/plugin)
"path": "/tmp",
# Plugin configuration information (if any)
"config": "test-config.json",
}
I can work on the plugin Definition loader some more and make it enforce
mandatory/optional fields. I’ll also investigate the directory climbing issue
I mentioned in my previous mail.
Also, I’m curious how things are going to work when the “url” points to a
foreign resource as the plugin start page. I don’t think the plugin’s iframe
is going to be able to access parent.pluginApi. Perhaps there is some aspect
of CORS that I don’t understand?
Thanks,
--Chris
From: Vojtech Szocs [mailto:[email protected]]
Sent: Thursday, August 23, 2012 7:14 AM
To: Frantz, Chris
Cc: engine-devel
Subject: Re: UI Plugins configuration
Hi Chris,
thanks for taking the time to make this patch, these are some excellent ideas!
(CC'ing engine-devel so that we can discuss this with other guys as well)
First of all, I really like the way you designed plugin source page URLs (going
through PluginSourcePageServlet), e.g.
"/webadmin/webadmin/plugin/<pluginName>/<pluginSourcePage>.html", plus the
concept of "path" JSON attribute.
WebadminDynamicHostingServlet loads and caches all plugin definitions (*.json
files), and directly embeds them into WebAdmin host page as pluginDefinitions
JavaScript object. I'm assuming that pluginDefinitions object will now look
like this:
var pluginDefinitions = {
"test": {
"name": "test",
"version": "1.0",
"url": "/webadmin/webadmin/plugin/test/foo.html",
"path": "/tmp",
"config": {"a":1, "b":2, "c":3}
}
}
Originally, the pluginDefinitions object looked like this:
var pluginDefinitions = {
"test": "/webadmin/webadmin/plugin/test/foo.html" // Simple pluginName ->
pluginSourcePageUrl mappings
}
This is because PluginManager (WebAdmin) only needs pluginName ("name") and
pluginSourcePageUrl ("url") during startup, when creating plugin iframe. But
this can be changed :)
Plugin "version" makes sense, plus the plugin configuration object ("config")
can be useful directly on the client. Let me explain:
Originally, plugin configuration was supposed to be passed to actual plugin
code (through immediately-invoked-function-expression, or IIFE), just like this:
(function (pluginApi, pluginConfig) { // JavaScript IIFE
// ... actual plugin code ...
})(
parent.pluginApi, /* reference to global pluginApi object */
{"a":1, "b":2, "c":3} /* embedded plugin configuration as JavaScript object */
);
The whole purpose of PluginSourcePageServlet was to "wrap" actual plugin code
into HTML, so that users don't need to write HTML pages for their plugins
manually. PluginSourcePageServlet would handle any plugin dependencies (placed
into HTML head), with actual plugin code being wrapped into IIFE, as shown
above. Plugin configuration was meant to be stored in a separate file, e.g.
<pluginName>-config.json, so that users could change the default plugin
configuration to suit their needs.
Inspired by your patch, rather than reading/embedding plugin configuration when
serving plugin HTML page (PluginSourcePageServlet), it's even better to have
the plugin configuration embedded directly into WebAdmin host page, along with
introducing new pluginApi function to retrieve the plugin configuration object.
Based on this, I suggest following modifications to the original concept:
- modify original pluginDefinitions structure, from pluginName ->
pluginSourcePageUrl, to pluginName -> pluginDefObject
- pluginDefObject is basically a subset of physical plugin definition
(test.json, see below), suitable for use on the client
- add following attributes to pluginDefObject: version, url, config
* note #1: name is not needed, since it's already the key of pluginName ->
pluginDefObject mapping
* note #2: path is not needed on the client (more on this below)
- introduce pluginApi.config(pluginName) function for plugins to retrieve their
configuration object, and remove pluginConfig parameter from main IIFE (as
shown above)
[a] Physical plugin definition file (JSON) might be located at oVirt "DataDir",
e.g. /usr/share/ovirt-engine/ui-plugins/test.json, for example:
{
"name": "test",
"version": "1.0",
"url": "/webadmin/webadmin/plugin/test/start.html",
"path": "/tmp",
"config": "test-config.json"
}
[b] Plugin configuration file (JSON) might be located at oVirt "ConfigDir",
e.g. /etc/ovirt-engine/ui-plugins/test-config.json, for example:
{
"a":1, "b":2, "c":3
}
[c] Finally, plugin static resources (plugin source page, actual plugin code,
plugin dependencies, CSS/images, etc.) would be located at /tmp (as shown in
[a]), for example:
/tmp/start.html -> plugin source page, used to load actual plugin code
/tmp/test.js -> actual plugin code
/tmp/deps/jquery-min.js -> simulate 3rd party plugin dependency
For example:
"/webadmin/webadmin/plugin/test/start.html" will be mapped to /tmp/start.html
"/webadmin/webadmin/plugin/test/deps/jquery-min.js" will be mapped to
/tmp/deps/jquery-min.js
This approach has some pros and cons:
(+) plugin static resources can be served through PluginSourcePageServlet
(pretty much like oVirt documentation resources, served through oVirt Engine
root war's FileServlet)
(+) plugin author has complete control over plugin source page
(-) plugin author actually needs to write plugin source page
Overall, I think this approach is better than the previous one (where
PluginSourcePageServlet took care of rendering plugin source page, but
sacrificed some flexibility).
By the way, here's what would happen behind the scenes:
1. user requests WebAdmin host page, WebadminDynamicHostingServlet loads and
caches all plugin definitions [a] + plugin configurations [b] and
constructs/embeds appropriate pluginDefinitions JavaScript object
2. during WebAdmin startup, PluginManager registers the plugin
(name/version/url/config), and creates/attaches the iframe to fetch plugin
source page ansynchronously
3. PluginSourcePageServlet handles plugin source page request, resolves the
correct path [c] and just streams the file content back to client
> 1. The plugin configuration files should probably have an "enabled" field
> and an "apiVersion" field that should be examined to determine whether or not
> to use the plugin.
Sounds good, we can implement these later on :)
> 2. I suspect the way I've modified PluginSourcePage makes it vulnerable to
> directory climbing attacks.
Yes, but we can defend against these, restricting access only to plugin's
"path" and its sub-directories.
> 3. Is /usr/share/ovirt-engine the right place for the plugin config files?
I suppose you mean plugin definition files [a], cannot tell for sure, but we
can change this anytime :)
Chris, please let me know what you think, and again - many thanks for sending
the patch!
Regards,
Vojtech
________________________________
From: "Chris Frantz" <[email protected]<mailto:[email protected]>>
To: [email protected]<mailto:[email protected]>
Sent: Wednesday, August 22, 2012 7:56:45 PM
Subject: UI Plugins configuration
Vojtech,
I decided to work on making the plugin patch a bit more configurable, following
some of the ideas expressed by Itamar and others in the meeting yesterday. The
attached patch is a simple first-attempt.
Plugin configurations are stored in /usr/share/ovirt-engine/ui-plugins/*.json.
Example:
{
"name": "test",
"version": "1.0",
"url": "/webadmin/webadmin/plugin/test/foo.html",
"path": "/tmp",
"config": {"a":1, "b":2, "c": 3}
}
The engine reads all of the *.json files in that directory to build the list of
known plugins and gives that list to the webadmin.
When webadmin loads a plugin, it requests the URL given in the plugin config
file. The "plugin" URL is mapped to PluginSourcePage, which will translate the
first part of the path ("test") into whatever path is stored in pluginConfig
("/tmp") in this case, and then serve the static file (e.g. "/tmp/foo.html").
I didn't use the renderPluginSourcePage() method in favor of just serving a
static file, but I have no strong opinion on the matter. However, a plugin may
want to store static resources at "path" and have the engine serve those
resources. By just serving files through PluginSourcePage, we don't need any
other servlets to provide those resources.
There is still a bit of work to do:
1. The plugin configuration files should probably have an "enabled" field and
an "apiVersion" field that should be examined to determine whether or not to
use the plugin.
2. I suspect the way I've modified PluginSourcePage makes it vulnerable to
directory climbing attacks.
3. Is /usr/share/ovirt-engine the right place for the plugin config files?
Let me know what you think,
--Chris
_______________________________________________
Engine-devel mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-devel
