Thanks for the detailed write-up On 1/14/13 3:47 PM, "Rohit Yadav" <rohit.ya...@citrix.com> wrote:
>Let me start by giving some background on the existing design and >implementation and later argue how we can make admin's life easier. Sorry >for the long email, I don't want people to miss anything I want to >propose :) > >Background >========== >At present, all the plugins which provide any sort of api has their own >commands.properties file that is basically a mapping of apiname (which >they provide but can be used irresponsibly) and role mask. > >To check API access, the code that the same was moved out as plugin >called StaticRoleBasedAPIAccessChecker in which checking is done based on >role. This plugin assumes that it will somehow have a mapping of apiname >and role mask. > >To make this process generic we have a APIChecker interface; > boolean checkAccess(User user, String apiCommandName) throws >PermissionDeniedException; > >Design >====== >There may be multiple plugins which are called by for loop and they >return: > - true: if user is allowed > - false: plugin is unable to handle that > - exception: if operation is not allowed > >Problem >====== >Each plugin has a commands.properties file which has no use for the >plugin itself, it's actually useful for the >StaticRoleBasedAPIAccessChecker to get the apiname, role mask mapping on >which basis it can do the checking. I can see moving out existing >features as plugins and new features like API throttling, API discovery >being implemented as plugins. It would be difficult and confusing for an >admin to keep track to so many configuration files and to edit them. >Further it's not dynamic enough, every time you want to change things in >properties files admin would need to restart the management server. > >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? > >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. > >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? > >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
