To elaborate further, I would like to add that even if this is adding some if-else code around checking the enum value when generating the response, it is replacing several other if-else's that were present in the code earlier that used to check if the user is an Admin/Domain Admin/regular user against the Db.
With IAM, we cannot have such if-else conditions around hardcoded roles. The design should work with custom roles and custom 'response views' allocated to the user. This change is a first step in achieving this. Prachi -----Original Message----- From: Min Chen [mailto:min.c...@citrix.com] Sent: Tuesday, June 24, 2014 10:04 AM To: dev@cloudstack.apache.org Cc: int-toolkit Subject: Re: Why to have API Commands for Admin actions? Hi Wilder, This is a recent change introduced by IAM feature, see FS here https://cwiki.apache.org/confluence/display/CLOUDSTACK/CloudStack+Identity+ and+Access+Management+%28IAM%29+Plugin, particularly see details in Response View section. The intention of this is to eventually provide custom response view for different custom IAM roles. As the first step, we only provided two static response view: Full and Restricted, and just map Full view to current admin commands. In the future, we should allow admin to define custom response view through column filter, and associate each custom response view with IAM policy. Hope that this can give you some context on this part of code. Thanks. -min On 6/24/14 4:55 AM, "Wilder Rodrigues" <wrodrig...@schubergphilis.com> wrote: >Hi all, > >We are currently working on the redundant VPC implementation. In order >to take the right steps from the beginning, we started analyzing the >existing code base, from the API commands into the >VPCVirtualNetworkAppliancaManagerImpl. > >Although it's not related to the feature itself, we found out that the >current way of using the APICommand annotation and the CreateVPCCmd >class (and its derived) is not really clear. For example, there are 2 >command classes to create a VPC. The difference between them is: one >has ResponseView.Full parameter in the @APICommand (ie. >CreateVPCCmdByAdmin); and the other has ResponseView.Rstricted >parameter in the @APICommand (i.e. CreateVPCCmd). Moreover, the call to >responseGenerator.createVpcresponde() method uses a ResponseView enum >according to what has been specified in the annotation parameter. > >We understand that having a different enum in the >responseGenerator.createVpcresponde() method will affect many things, >because it goes deep into the code until reaches the APIDBUtils and the >database. It is also checked in the ApiServer class, when the command >is evaluated based on a string passed to the getCmdClass() method. > >Since we can identify the user in the account manager, also checking >the kind of access the user has, what is the point in having the >Annotation + the Enum in the code? Keep in mind that the latter is >passed several times as parameters to other methods, which adds many >"ifs" and unnecessary complexity. > >We could also make possible to use the Annotation in the method itself, >which could avoid having to pass the Enum to the method. That means >that the Enum would be removed from the annotation and we would use the >annotation only to identify the API name, response object and entity >type. In order to check the user's credentials, we would use the >checkAccess() from the account manager, as it is already being used in >the ApiDispatcher class. > >I know those are huge changes, if we actually agree in going for >anything like this in the future. But the Admin commands are not doing >much except for change the enum which is passed to the create response >method. Most of the content of the execute() method is a copy/paste >from the extended Command class. > >Just trying to start some chat towards the subject. > >Thanks for your time. > >Cheers, >Wilder
