From 8d53557ba7167613df855635dd821c38ab4bf67e Mon Sep 17 00:00:00 2001 From: Craig Squire Date: Wed, 12 Dec 2018 12:25:32 -0600 Subject: [PATCH] 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. --- .../apache/cloudstack/acl/APIAclChecker.java | 23 +++++++++++++++++++ ...core-lifecycle-api-context-inheritable.xml | 5 ++++ .../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(-) create mode 100644 api/src/org/apache/cloudstack/acl/APIAclChecker.java 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 00000000000..3a00f26486d --- /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 7ab5523732d..655b7fe6572 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 @@ + + + + + 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 1f70e526147..2569d8b6487 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"> + + + + 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 d10c191151f..d8612a692f8 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 4a33a0876c3..6b40ab4ddff 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 e0171626d30..481db246bb2 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 @@ - +