Skip to content

Commit 8d53557

Browse files
csquireyadvr
authored andcommitted
api: don't throttle api discovery for listApis command (#2894)
Users reported that they weren't getting all apis listed in cloudmonkey when running a sync. After some debugging, I found that the problem is that the ApiDiscoveryService is calling ApiRateLimitServiceImpl.checkAccess(), so the results of the listApis command are being truncated because Cloudstack believes the user has exceeded their API throttling rate. I enabled throttling with a 25 request per second limit. I then created a test role with only list* permissions and assigned it to a test user. When this user calls listApis, they will typically receive anywhere from 15-18 results. Checking the logs, you see The given user has reached his/her account api limit, please retry after 218 ms.. I raised the limit to 200 requests per second, restarted the management server and tried again. This time I got 143 results and no log messages about the user being throttled.
1 parent 408cce4 commit 8d53557

File tree

6 files changed

+36
-3
lines changed

6 files changed

+36
-3
lines changed
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.acl;
18+
19+
/**
20+
* Marker interface to differentiate ACL APICheckers from others (for example, a rate limit checker)
21+
*/
22+
public interface APIAclChecker extends APIChecker {
23+
}

core/resources/META-INF/cloudstack/api/spring-core-lifecycle-api-context-inheritable.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,11 @@
5151
<property name="registry" ref="apiCheckersRegistry" />
5252
<property name="typeClass" value="org.apache.cloudstack.acl.APIChecker" />
5353
</bean>
54+
55+
<bean class="org.apache.cloudstack.spring.lifecycle.registry.RegistryLifecycle">
56+
<property name="registry" ref="apiAclCheckersRegistry" />
57+
<property name="typeClass" value="org.apache.cloudstack.acl.APIAclChecker" />
58+
</bean>
5459

5560
<bean class="org.apache.cloudstack.spring.lifecycle.registry.RegistryLifecycle">
5661
<property name="registry" ref="querySelectorsRegistry" />

core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,11 @@
264264
class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">
265265
<property name="excludeKey" value="api.checkers.exclude" />
266266
</bean>
267+
268+
<bean id="apiAclCheckersRegistry"
269+
class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">
270+
<property name="excludeKey" value="api.checkers.acl.exclude" />
271+
</bean>
267272

268273
<bean id="querySelectorsRegistry"
269274
class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">

plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
import com.cloud.utils.component.PluggableService;
3737
import com.google.common.base.Strings;
3838

39-
public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements APIChecker {
39+
public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements APIAclChecker {
4040

4141
@Inject
4242
private AccountService accountService;

plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
// This is the default API access checker that grab's the user's account
4242
// based on the account type, access is granted
4343
@Deprecated
44-
public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIChecker {
44+
public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIAclChecker {
4545

4646
protected static final Logger LOGGER = Logger.getLogger(StaticRoleBasedAPIAccessChecker.class);
4747

server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444

4545
<bean id="apiDiscoveryServiceImpl"
4646
class="org.apache.cloudstack.discovery.ApiDiscoveryServiceImpl">
47-
<property name="apiAccessCheckers" value="#{apiCheckersRegistry.registered}" />
47+
<property name="apiAccessCheckers" value="#{apiAclCheckersRegistry.registered}" />
4848
<property name="services" value="#{apiCommandsRegistry.registered}" />
4949
</bean>
5050

0 commit comments

Comments
 (0)