Hi Chiradeep,

Thanks for the reply.

On 15-Jan-2013, at 11:26 AM, Chiradeep Vittal <chiradeep.vit...@citrix.com> 
wrote:

> Thanks for the detailed write-up
> 
>> Proposed Fix (aka make admin's life easier)
>> ===========
>> 
>> 1. Append this mapping in one single commands.properties file get rid of
>> all the other commands.properties files, because the apiname:mask mapping
>> is not for the plugin or mgmt server, it's just for the
>> StaticRoleBasedAPIAccessChecker. This way an APIChecker plugin won't have
>> to depend on other plugins and vice-versa. It would be a clear separation
>> of policy local to a plugin. In future one can be free to implement
>> another APIChecker which may be dynamic and instead of reading from a
>> properties file, queries an API service. Plugin authors pl. comment?
> 
> Couldn't we use something like log4j's configureAndWatch() [1]. That way
> whenever the file changes it gets reloaded?

First of all, this plugin and all other plugins should never have access to 
internal cfg, layers etc. Secondly I though about it but the problem is log4j 
configureAndWatch is a thread that polls for file changes, in a case where 
admin is still working on the file and saves it in between of changes, it will 
mess up.

So, since the plugin itself is an independent entity, it should be responsible 
for providing mechanism to the admin and my proposal was to make it explicit so 
when admin is done working on the commands.properties file they can either 
restart the mgmt server or run an api provided by this plugin to reload the acl 
mapping.

> 
>> 
>> 2. Make online reloading easier. Implement an api (updateAccessChecker or
>> something) with StaticRoleBasedAPIAccessChecker itself which an admin can
>> call to reload the commands.properties mapping in case the admin changes
>> it and wants the new policy to be reflected without having to restart
>> mgmt server.
>> 
>> 3. Each plugin (if they implement a service exposed via an API) would
>> return a list of API cmd classes. Right now the code uses java
>> reflections to grab the classes with @APICommand annotation and generate
>> the mappings. This can be incorrect in case there is some component which
>> was not loaded but exists in class path. Such plugins implement
>> PluggableService which is a contract that implementing entity will have a
>> way to return list of cmd classes.
>> 
>> Convention: We've to have a standard convention for writing plugins. So,
>> plugin writers should use the @APICommand and other annotations for
>> writing API cmd and response classes. The only artifacts from cloudstack
>> they should depend on should be cloud-api and/or cloud-util. The plugins
>> are not required to be under org.apache.cloudstack or com.cloud package
>> names.
> 
> Is this part of the discussion, or a separate topic? I don't mind this
> convention.

This was done part of api_refactoring to make the behaviour and annotation 
uniform. If a new plugin follows this, they can escape uuid conversion, acl 
validations etc. and make it easier to find new apis and generate documentation 
for them.

>> 
>> Future
>> =====
>> 
>> Online pluggable service: (Hugo, Alex and I talked about this during
>> ccc12, we can do it in future maybe, but at least we should start
>> packaging them separately)
>> 
>> As part of packaging, all plugins be bundled as separate debs/rpms, the
>> core system (cloudstack-server) can depend on few ought-to have plugins.
>> The plugins get installed in a specific directory, say
>> /usr/share/java/cloudstack/plugins. This directory is within class path
>> of the server process.
> 
> There is a separate discussion on this. [2]
> Perhaps we can move the topic of plugin packaging to that discussion?

Okay, let's do it, will post this on the mentioned thread.

Regards.

> 
>> 
>> If an admin trusts a plugin, he installs/place the jar in this directory,
>> changes ACL either in /etc/cloud/commands.properties or depending upon
>> the choice of APIChecker plugin and calls the an API to tell mgmt server
>> to load the plugin. It would be tricky and some plugins may require
>> restart.
> 
>> 
>> Regards.
> 
> [1] http://s.apache.org/s5
> [2] http://markmail.org/thread/4nd3ymy3zzrlpmpe
> 

Reply via email to