On Thu, Jan 17, 2013 at 7:09 PM, Min Chen <min.c...@citrix.com> wrote: > Hey all, > > I would like to merge my feature branch api_limit with master. This branch > deals with JIRA tickets CLOUDSTACK-618. Basically it implemented a plugin to > provide basic support for api rate limiting to avoid malicious attack on > CloudStack server. Implementation details can be found in this FS document > https://cwiki.apache.org/confluence/display/CLOUDSTACK/API+Request+Throttling. > For this release, we have chosen to implement Ehcache based rate limit > store. With clearly defined limit store interface, we can easily extend this > to provide other limit store implementations based on Memcached or Redis > which rely on setting up a dedicated proxy server. > > Testing done > > I have done the following two kinds of testing during development cycle: > > * Unit test to verify ApiRateLimitService pluggable service interface and > Limit Store interface methods. These unit testcases are located in > plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java. > * Integration test to verify rate limit feature and new APIs through a > running MS. These integration testcases are located in > plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/integration/RateLimitIntegrationTest.java. > These test cases are assuming that we have a "demo" user account created on > your locally running MS. > > Risk > > This has minimal risk due to its plugin implementation. We can easily disable > this feature by removing this plugin from components.xml.in. A potential > impact may be that if this is enabled, UI needs to handle this situation (api > failed due to over limit) gracefully. > > Database changes > none > > Documentation tracked in a separate ticket: CLOUDSTACK-866 > > Thanks > -min > >
No objection here. I especially appreciate calling out the risks, and making sure this had testing in place up front. --David
