You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2018/12/12 18:25:41 UTC
[cloudstack] branch 4.11 updated: api: don't throttle api discovery
for listApis command (#2894)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch 4.11
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.11 by this push:
new 8d53557 api: don't throttle api discovery for listApis command (#2894)
8d53557 is described below
commit 8d53557ba7167613df855635dd821c38ab4bf67e
Author: Craig Squire <cr...@gmail.com>
AuthorDate: Wed Dec 12 12:25:32 2018 -0600
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.
---
.../org/apache/cloudstack/acl/APIAclChecker.java | 23 ++++++++++++++++++++++
...ring-core-lifecycle-api-context-inheritable.xml | 5 +++++
.../core/spring-core-registry-core-context.xml | 5 +++++
.../acl/DynamicRoleBasedAPIAccessChecker.java | 2 +-
.../acl/StaticRoleBasedAPIAccessChecker.java | 2 +-
.../core/spring-server-core-misc-context.xml | 2 +-
6 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/api/src/org/apache/cloudstack/acl/APIAclChecker.java b/api/src/org/apache/cloudstack/acl/APIAclChecker.java
new file mode 100644
index 0000000..3a00f26
--- /dev/null
+++ b/api/src/org/apache/cloudstack/acl/APIAclChecker.java
@@ -0,0 +1,23 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.acl;
+
+/**
+ * Marker interface to differentiate ACL APICheckers from others (for example, a rate limit checker)
+ */
+public interface APIAclChecker extends APIChecker {
+}
diff --git a/core/resources/META-INF/cloudstack/api/spring-core-lifecycle-api-context-inheritable.xml b/core/resources/META-INF/cloudstack/api/spring-core-lifecycle-api-context-inheritable.xml
index 7ab5523..655b7fe 100644
--- a/core/resources/META-INF/cloudstack/api/spring-core-lifecycle-api-context-inheritable.xml
+++ b/core/resources/META-INF/cloudstack/api/spring-core-lifecycle-api-context-inheritable.xml
@@ -51,6 +51,11 @@
<property name="registry" ref="apiCheckersRegistry" />
<property name="typeClass" value="org.apache.cloudstack.acl.APIChecker" />
</bean>
+
+ <bean class="org.apache.cloudstack.spring.lifecycle.registry.RegistryLifecycle">
+ <property name="registry" ref="apiAclCheckersRegistry" />
+ <property name="typeClass" value="org.apache.cloudstack.acl.APIAclChecker" />
+ </bean>
<bean class="org.apache.cloudstack.spring.lifecycle.registry.RegistryLifecycle">
<property name="registry" ref="querySelectorsRegistry" />
diff --git a/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml b/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml
index 1f70e52..2569d8b 100644
--- a/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml
+++ b/core/resources/META-INF/cloudstack/core/spring-core-registry-core-context.xml
@@ -264,6 +264,11 @@
class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">
<property name="excludeKey" value="api.checkers.exclude" />
</bean>
+
+ <bean id="apiAclCheckersRegistry"
+ class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">
+ <property name="excludeKey" value="api.checkers.acl.exclude" />
+ </bean>
<bean id="querySelectorsRegistry"
class="org.apache.cloudstack.spring.lifecycle.registry.ExtensionRegistry">
diff --git a/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java b/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
index d10c191..d8612a6 100644
--- a/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
+++ b/plugins/acl/dynamic-role-based/src/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java
@@ -36,7 +36,7 @@ import com.cloud.utils.component.AdapterBase;
import com.cloud.utils.component.PluggableService;
import com.google.common.base.Strings;
-public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements APIChecker {
+public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements APIAclChecker {
@Inject
private AccountService accountService;
diff --git a/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java b/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
index 4a33a08..6b40ab4 100644
--- a/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
+++ b/plugins/acl/static-role-based/src/org/apache/cloudstack/acl/StaticRoleBasedAPIAccessChecker.java
@@ -41,7 +41,7 @@ import com.cloud.utils.component.PluggableService;
// This is the default API access checker that grab's the user's account
// based on the account type, access is granted
@Deprecated
-public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIChecker {
+public class StaticRoleBasedAPIAccessChecker extends AdapterBase implements APIAclChecker {
protected static final Logger LOGGER = Logger.getLogger(StaticRoleBasedAPIAccessChecker.class);
diff --git a/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml b/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml
index e017162..481db24 100644
--- a/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml
+++ b/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml
@@ -44,7 +44,7 @@
<bean id="apiDiscoveryServiceImpl"
class="org.apache.cloudstack.discovery.ApiDiscoveryServiceImpl">
- <property name="apiAccessCheckers" value="#{apiCheckersRegistry.registered}" />
+ <property name="apiAccessCheckers" value="#{apiAclCheckersRegistry.registered}" />
<property name="services" value="#{apiCommandsRegistry.registered}" />
</bean>