On first glance it looks great! I’ll look at it first thing in the morning since it is getting late here in EST.
Thanks! Chris -- Chris Suich chris.su...@netapp.com NetApp Software Engineer Data Center Platforms – Cloud Solutions Citrix, Cisco & Red Hat On Oct 24, 2013, at 6:08 PM, Darren Shepherd <darren.s.sheph...@gmail.com> wrote: > Chris, > > This change was simple enough I just put up a review for it > https://reviews.apache.org/r/14914/ You want to download the patch > and see if that works for you? > > Darren > > On Thu, Oct 24, 2013 at 2:28 PM, Darren Shepherd > <darren.s.sheph...@gmail.com> wrote: >> Currently any new API extension to CloudStack must edit >> commands.properties to add the appropriate ACLs. This generally works >> fine for ACS as we control the contents of that file and distribute >> all the code ourself. The hang up comes when somebody develops code >> outside of ACS and want to add their code to an existing ACS >> installation. The Spring work that has been done has made this much >> easier, but you are still required to manually edit >> commands.properties. I propose that we add the ACL metadata as an >> optional field in @APICommand. The logic will be as follows. >> >> First check commands.properties for ACL info. If ACL info exists, use >> that to authorize the command. If no ACL information exists (ie >> null), then look at the @APICommand annotation. The defaults of >> @APICommand will provide no ACL info. If the @APICommand annotation >> provides no ACL info, use that. >> >> Effectively what this means is that for all existing "ACS distributed" >> code @APICommand will provide no ACL info (as the default is none) so >> commands.properties will be used. If somebody extends ACS, they can >> set the ACL info in @APICommand. >> >> The scope of proposal is focused on "non-ACS" distributed code. In >> order for ACS to move to the annotations fully and make >> commands.properties optional more things need to change. Specifically >> the documentation and some python utilities look at >> command.properties. It would be a nice future enhancement to make >> commands.properties fully optional, but for the time being this >> proposal will address the core issue. >> >> Darren
