You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2021/01/11 11:14:38 UTC
[servicecomb-java-chassis] 05/07: [SCB-2116]add configuration
entity validation
This is an automated email from the ASF dual-hosted git repository.
liubao pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
commit bf6e0a4d9e04a61e0d95cddc428b0dc45e26b9a3
Author: liubao <bi...@qq.com>
AuthorDate: Tue Jan 5 16:18:46 2021 +0800
[SCB-2116]add configuration entity validation
---
.../governance/entity/Configurable.java | 25 +++++++++++
.../servicecomb/governance/marker/Matcher.java | 8 ++++
.../governance/marker/TrafficMarker.java | 16 ++++++-
.../governance/policy/AbstractPolicy.java | 20 +++++++--
.../governance/policy/BulkheadPolicy.java | 11 +++++
.../governance/policy/CircuitBreakerPolicy.java | 24 ++++++++++
.../governance/policy/GovernanceRule.java | 7 +++
.../governance/policy/RateLimitingPolicy.java | 14 ++++++
.../servicecomb/governance/policy/RetryPolicy.java | 51 +++++-----------------
.../properties/GovernanceProperties.java | 13 ++++--
governance/src/test/resources/application.yaml | 17 +++++++-
11 files changed, 156 insertions(+), 50 deletions(-)
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/entity/Configurable.java b/governance/src/main/java/org/apache/servicecomb/governance/entity/Configurable.java
new file mode 100644
index 0000000..987ddf0
--- /dev/null
+++ b/governance/src/main/java/org/apache/servicecomb/governance/entity/Configurable.java
@@ -0,0 +1,25 @@
+/*
+ * 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.servicecomb.governance.entity;
+
+/**
+ * Indicates a object can be configure in configuration file or config center.
+ */
+public interface Configurable {
+ boolean isValid();
+}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java b/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java
index 371460c..6b1b37a 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/marker/Matcher.java
@@ -19,6 +19,7 @@ package org.apache.servicecomb.governance.marker;
import java.util.List;
import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
import org.apache.servicecomb.governance.marker.operator.RawOperator;
public class Matcher {
@@ -30,6 +31,13 @@ public class Matcher {
private String name;
+ public boolean isValid() {
+ if (StringUtils.isEmpty(name)) {
+ return false;
+ }
+ return true;
+ }
+
public Map<String, RawOperator> getHeaders() {
return headers;
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/marker/TrafficMarker.java b/governance/src/main/java/org/apache/servicecomb/governance/marker/TrafficMarker.java
index b64c168..90cb6e2 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/marker/TrafficMarker.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/marker/TrafficMarker.java
@@ -20,8 +20,9 @@ import java.util.Arrays;
import java.util.List;
import org.apache.commons.lang3.StringUtils;
+import org.apache.servicecomb.governance.entity.Configurable;
-public class TrafficMarker {
+public class TrafficMarker implements Configurable {
private String services;
@@ -31,6 +32,19 @@ public class TrafficMarker {
return services;
}
+ @Override
+ public boolean isValid() {
+ if (matches == null || matches.isEmpty()) {
+ return false;
+ }
+ for (Matcher matcher : matches) {
+ if (!matcher.isValid()) {
+ return false;
+ }
+ }
+ return true;
+ }
+
public void setServices(String services) {
this.services = services;
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java
index 3e496c4..2f2aad5 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/AbstractPolicy.java
@@ -18,11 +18,14 @@ package org.apache.servicecomb.governance.policy;
import java.util.List;
-public abstract class AbstractPolicy implements Policy {
+import org.apache.commons.lang3.StringUtils;
+import org.apache.servicecomb.governance.entity.Configurable;
- private String name;
+public abstract class AbstractPolicy implements Policy, Configurable {
- private GovernanceRule rules;
+ protected String name;
+
+ protected GovernanceRule rules;
public GovernanceRule getRules() {
return rules;
@@ -37,6 +40,17 @@ public abstract class AbstractPolicy implements Policy {
}
@Override
+ public boolean isValid() {
+ if (StringUtils.isEmpty(name)) {
+ return false;
+ }
+ if (rules == null) {
+ return false;
+ }
+ return rules.isValid();
+ }
+
+ @Override
public boolean match(List<String> items) {
if (rules == null) {
return false;
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/BulkheadPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/BulkheadPolicy.java
index e414e1d..57703b2 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/BulkheadPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/BulkheadPolicy.java
@@ -46,6 +46,17 @@ public class BulkheadPolicy extends AbstractPolicy {
}
@Override
+ public boolean isValid() {
+ if (maxConcurrentCalls <= 0) {
+ return false;
+ }
+ if (maxWaitDuration < 0) {
+ return false;
+ }
+ return super.isValid();
+ }
+
+ @Override
public String handler() {
return BulkheadHandler.class.getSimpleName();
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/CircuitBreakerPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/CircuitBreakerPolicy.java
index 4353a25..6470299 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/CircuitBreakerPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/CircuitBreakerPolicy.java
@@ -58,6 +58,30 @@ public class CircuitBreakerPolicy extends AbstractPolicy {
public CircuitBreakerPolicy() {
}
+ @Override
+ public boolean isValid() {
+ if (failureRateThreshold > 100 || failureRateThreshold <= 0) {
+ return false;
+ }
+ if (slowCallRateThreshold > 100 || slowCallRateThreshold <= 0) {
+ return false;
+ }
+ if (waitDurationInOpenState <= 0) {
+ return false;
+ }
+ if (slowCallDurationThreshold <= 0) {
+ return false;
+ }
+ if (permittedNumberOfCallsInHalfOpenState <= 0) {
+ return false;
+ }
+ if (minimumNumberOfCalls <= 0) {
+ return false;
+ }
+
+ return super.isValid();
+ }
+
public int getFailureRateThreshold() {
return failureRateThreshold;
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/GovernanceRule.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/GovernanceRule.java
index d75b250..480cf55 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/GovernanceRule.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/GovernanceRule.java
@@ -47,6 +47,13 @@ public class GovernanceRule {
this.precedence = precedence;
}
+ public boolean isValid() {
+ if (StringUtils.isEmpty(match)) {
+ return false;
+ }
+ return true;
+ }
+
public boolean match(String name) {
if (StringUtils.isEmpty(this.match)) {
return false;
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/RateLimitingPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/RateLimitingPolicy.java
index 2d82d76..8bb312a 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/RateLimitingPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/RateLimitingPolicy.java
@@ -70,6 +70,20 @@ public class RateLimitingPolicy extends AbstractPolicy {
}
@Override
+ public boolean isValid() {
+ if (timeoutDuration < 0) {
+ return false;
+ }
+ if (limitRefreshPeriod <= 0) {
+ return false;
+ }
+ if (rate <= 0) {
+ return false;
+ }
+ return super.isValid();
+ }
+
+ @Override
public String handler() {
return RateLimitingHandler.class.getSimpleName();
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java b/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
index b6ec4b9..944b349 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/policy/RetryPolicy.java
@@ -16,16 +16,9 @@
*/
package org.apache.servicecomb.governance.policy;
-import org.springframework.util.StringUtils;
-
import org.apache.servicecomb.governance.handler.RetryHandler;
+import org.springframework.util.StringUtils;
-/**
- * intervalFunction 失败时可以更改等待时间的函数
- * retryOnResultPredicate 根据返回结果决定是否进行重试
- * retryOnExceptionPredicate 根据失败异常决定是否进行重试
- *
- */
public class RetryPolicy extends AbstractPolicy {
public static final int DEFAULT_MAX_ATTEMPTS = 3;
@@ -43,14 +36,6 @@ public class RetryPolicy extends AbstractPolicy {
//需要重试的http status, 逗号分隔
private String retryOnResponseStatus;
- //TODO: 需要进行重试的异常列表，反射取异常
- private String retryExceptions;
-
- //TODO: 需要进行忽略的异常列表
- private String ignoreExceptions;
-
- private boolean onSame;
-
public String getRetryOnResponseStatus() {
if (StringUtils.isEmpty(retryOnResponseStatus)) {
retryOnResponseStatus = DEFAULT_RETRY_ON_RESPONSE_STATUS;
@@ -78,28 +63,15 @@ public class RetryPolicy extends AbstractPolicy {
this.waitDuration = waitDuration;
}
- public String getRetryExceptions() {
- return retryExceptions;
- }
-
- public void setRetryExceptions(String retryExceptions) {
- this.retryExceptions = retryExceptions;
- }
-
- public String getIgnoreExceptions() {
- return ignoreExceptions;
- }
-
- public void setIgnoreExceptions(String ignoreExceptions) {
- this.ignoreExceptions = ignoreExceptions;
- }
-
- public boolean isOnSame() {
- return onSame;
- }
-
- public void setOnSame(boolean onSame) {
- this.onSame = onSame;
+ @Override
+ public boolean isValid() {
+ if (maxAttempts < 1) {
+ return false;
+ }
+ if (waitDuration < 0) {
+ return false;
+ }
+ return super.isValid();
}
@Override
@@ -113,9 +85,6 @@ public class RetryPolicy extends AbstractPolicy {
"maxAttempts=" + maxAttempts +
", waitDuration=" + waitDuration +
", retryOnResponseStatus='" + retryOnResponseStatus + '\'' +
- ", retryExceptions='" + retryExceptions + '\'' +
- ", ignoreExceptions='" + ignoreExceptions + '\'' +
- ", onSame=" + onSame +
'}';
}
}
diff --git a/governance/src/main/java/org/apache/servicecomb/governance/properties/GovernanceProperties.java b/governance/src/main/java/org/apache/servicecomb/governance/properties/GovernanceProperties.java
index ab852eb..695adb6 100644
--- a/governance/src/main/java/org/apache/servicecomb/governance/properties/GovernanceProperties.java
+++ b/governance/src/main/java/org/apache/servicecomb/governance/properties/GovernanceProperties.java
@@ -23,6 +23,9 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import org.apache.servicecomb.governance.entity.Configurable;
+import org.apache.servicecomb.governance.event.ConfigurationChangedEvent;
+import org.apache.servicecomb.governance.event.EventManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.InitializingBean;
@@ -42,10 +45,7 @@ import org.yaml.snakeyaml.representer.Representer;
import com.google.common.eventbus.Subscribe;
-import org.apache.servicecomb.governance.event.ConfigurationChangedEvent;
-import org.apache.servicecomb.governance.event.EventManager;
-
-public abstract class GovernanceProperties<T> implements InitializingBean {
+public abstract class GovernanceProperties<T extends Configurable> implements InitializingBean {
private static final Logger LOGGER = LoggerFactory.getLogger(GovernanceProperties.class);
private final Representer representer = new Representer();
@@ -164,6 +164,11 @@ public abstract class GovernanceProperties<T> implements InitializingBean {
Yaml entityParser = new Yaml(new Constructor(new TypeDescription(entityClass, entityClass)), representer);
T result = entityParser.loadAs(value, entityClass);
setName(result, key);
+
+ if (!result.isValid()) {
+ LOGGER.warn("Entity configuration is not valid and ignored. Key [{}], value [{}]", key, value);
+ return null;
+ }
return result;
} catch (YAMLException e) {
LOGGER.error("governance config yaml is illegal : {}", e.getMessage());
diff --git a/governance/src/test/resources/application.yaml b/governance/src/test/resources/application.yaml
index 69925df..db4ff33 100644
--- a/governance/src/test/resources/application.yaml
+++ b/governance/src/test/resources/application.yaml
@@ -22,6 +22,8 @@ servicecomb:
- apiPath:
exact: "/hello"
name: match0
+ wrong-name-inogred: |
+ wrong: some
demo-retry: |
matches:
- apiPath:
@@ -42,21 +44,34 @@ servicecomb:
rules:
match: demo-rateLimiting.match0
rate: 1
+ wrongIngored: |
+ rate: 0
retry:
retry0: |
rules:
match: demo-retry.xx
maxAttempts: 3
+ wrongIngored: |
+ rules:
+ wrong: 0
circuitBreaker:
circuitBreaker0: |
rules:
match: demo-circuitBreaker.xx
minimumNumberOfCalls: 2
slidingWindowSize: 2
+ wrongIngored: |
+ rules:
+ match: demo-circuitBreaker.xx
+ minimumNumberOfCalls: -1
bulkhead:
bulkhead0: |
rules:
match: demo-bulkhead.xx
precedence: 100
maxConcurrentCalls: 1
- maxWaitDuration: 3000
\ No newline at end of file
+ maxWaitDuration: 3000
+ wrongIngored: |
+ rules:
+ match: demo-bulkhead.xx
+ maxWaitDuration: -1
\ No newline at end of file