From d3d8c8ed06d7da9e75517051446e5f534a3c292e Mon Sep 17 00:00:00 2001
From: sodaRyCN <757083350@qq.com>
Date: Tue, 29 Mar 2022 12:51:54 +0800
Subject: [PATCH] fix  circuit-breaker filter failure legality validate

---
 pkg/filter/circuitbreaker/circuitbreaker.go   | 16 +------
 .../circuitbreaker/circuitbreaker_test.go     | 46 ++++---------------
 2 files changed, 12 insertions(+), 50 deletions(-)

diff --git a/pkg/filter/circuitbreaker/circuitbreaker.go b/pkg/filter/circuitbreaker/circuitbreaker.go
index d41d897f90..482b549701 100644
--- a/pkg/filter/circuitbreaker/circuitbreaker.go
+++ b/pkg/filter/circuitbreaker/circuitbreaker.go
@@ -110,20 +110,8 @@ URLLoop:
 
 func (spec Spec) validatePoliciesSpec() error {
 	for _, p := range spec.Policies {
-		//group of failure
-		if p.FailureRateThreshold == 0 && len(p.FailureStatusCodes) != 0 {
-			return fmt.Errorf("policy '%s' has setted failure status code, but not set failure threshold ", p.Name)
-		}
-		if p.FailureRateThreshold != 0 && len(p.FailureStatusCodes) == 0 {
-			return fmt.Errorf("policy '%s' has setted failure threshold, but not set failure status code ", p.Name)
-		}
-
-		//group of slow
-		if p.SlowCallRateThreshold != 0 && (p.SlowCallDurationThreshold == "" || strings.TrimSpace(p.SlowCallDurationThreshold) == "") {
-			return fmt.Errorf("policy '%s' has setted slow call threshold, but not set slow call duration ", p.Name)
-		}
-		if p.SlowCallRateThreshold == 0 && (p.SlowCallDurationThreshold != "" || strings.TrimSpace(p.SlowCallDurationThreshold) != "") {
-			return fmt.Errorf("policy '%s' has setted slow call duration, but not set slow call threshold ", p.Name)
+		if p.FailureRateThreshold != 0 && len(p.FailureStatusCodes) == 0 && !p.CountingNetworkError {
+			return fmt.Errorf("policy '%s' has setted failure threshold and countingNetworkError is false, but not set failure status code", p.Name)
 		}
 	}
 	return nil
diff --git a/pkg/filter/circuitbreaker/circuitbreaker_test.go b/pkg/filter/circuitbreaker/circuitbreaker_test.go
index ee50d74b1d..2bdb58e34e 100644
--- a/pkg/filter/circuitbreaker/circuitbreaker_test.go
+++ b/pkg/filter/circuitbreaker/circuitbreaker_test.go
@@ -48,7 +48,6 @@ policies:
   slidingWindowType: COUNT_BASED
   slidingWindowSize: 10
   minimumNumberOfCalls: 5
-  slowCallDurationThreshold: 5s
   failureStatusCodes: [500, 503]
 defaultPolicyRef: default
 urls:
@@ -187,42 +186,16 @@ urls:
 			t.Errorf("set failure threshold and not set failure code, that did not fail")
 		}
 	})
-
-	t.Run("invalidFailureThreshold", func(t *testing.T) {
-		const yamlSpec = `
-kind: CircuitBreaker
-name: circuitbreaker
-policies:
-- name: default
-  failureStatusCodes: [500]
-  slidingWindowType: COUNT_BASED
-  slidingWindowSize: 10
-defaultPolicyRef: default
-urls:
-- methods: []
-  url:
-    exact: /circuitbreak
-    prefix:
-    regex:
-`
-		rawSpec := make(map[string]interface{})
-		yamltool.Unmarshal([]byte(yamlSpec), &rawSpec)
-
-		_, err := httppipeline.NewFilterSpec(rawSpec, nil)
-		if err == nil {
-			t.Errorf("set failure code and not set failure rate, that did not fail")
-		}
-	})
-
-	t.Run("invalidSlowDuration", func(t *testing.T) {
+	t.Run("validFailureLegalityByCountingNetworkError", func(t *testing.T) {
 		const yamlSpec = `
 kind: CircuitBreaker
 name: circuitbreaker
 policies:
 - name: default
+  failureRateThreshold: 50
   slidingWindowType: COUNT_BASED
   slidingWindowSize: 10
-  slowCallRateThreshold: 1
+  countingNetworkError: true
 defaultPolicyRef: default
 urls:
 - methods: []
@@ -235,20 +208,21 @@ urls:
 		yamltool.Unmarshal([]byte(yamlSpec), &rawSpec)
 
 		_, err := httppipeline.NewFilterSpec(rawSpec, nil)
-		if err == nil {
-			t.Errorf("set slow call threshold and not set slow call duration, that did not fail")
+		if err != nil {
+			t.Errorf("set failure threshold and countingNetworkError is true, not set failure code, that is fail")
 		}
 	})
 
-	t.Run("invalidSlowCallThreshold", func(t *testing.T) {
+	t.Run("validFailureLegalityByFailureStatusCodes", func(t *testing.T) {
 		const yamlSpec = `
 kind: CircuitBreaker
 name: circuitbreaker
 policies:
 - name: default
+  failureRateThreshold: 50
   slidingWindowType: COUNT_BASED
   slidingWindowSize: 10
-  slowCallDurationThreshold: 1m
+  failureStatusCodes: [500]
 defaultPolicyRef: default
 urls:
 - methods: []
@@ -261,8 +235,8 @@ urls:
 		yamltool.Unmarshal([]byte(yamlSpec), &rawSpec)
 
 		_, err := httppipeline.NewFilterSpec(rawSpec, nil)
-		if err == nil {
-			t.Errorf("set slow call duration and not set slow call threshold, that did not fail")
+		if err != nil {
+			t.Errorf("set failure threshold and countingNetworkError is true, not set failure code, that is fail")
 		}
 	})
 }