valid circuit-breaker filter spec by correlation

pkg/filter/circuitbreaker/circuitbreaker.go

@@ -49,8 +49,8 @@ type (
 	Policy struct {
 		Name                             string `yaml:"name" jsonschema:"required"`
 		SlidingWindowType                string `yaml:"slidingWindowType"  jsonschema:"omitempty,enum=COUNT_BASED,enum=TIME_BASED"`
-		FailureRateThreshold             uint8  `yaml:"failureRateThreshold" jsonschema:"omitempty,minimum=1,maximum=100"`
-		SlowCallRateThreshold            uint8  `yaml:"slowCallRateThreshold" jsonschema:"omitempty,minimum=1,maximum=100"`
+		FailureRateThreshold             uint8  `yaml:"failureRateThreshold" jsonschema:"omitempty,maximum=100"`
+		SlowCallRateThreshold            uint8  `yaml:"slowCallRateThreshold" jsonschema:"omitempty,maximum=100"`
 		CountingNetworkError             bool   `yaml:"countingNetworkError" jsonschema:"omitempty"`
 		SlidingWindowSize                uint32 `yaml:"slidingWindowSize" jsonschema:"omitempty,minimum=1"`
 		PermittedNumberOfCallsInHalfOpen uint32 `yaml:"permittedNumberOfCallsInHalfOpenState" jsonschema:"omitempty"`
@@ -87,8 +87,8 @@ type (
 	}
 )
 
-// Validate implements custom validation for Spec
-func (spec Spec) Validate() error {
+// check policy of url usage whether defined
+func (spec Spec) validateURLPoliciesUsage() error {
 URLLoop:
 	for _, u := range spec.URLs {
 		name := u.PolicyRef
@@ -108,6 +108,28 @@ URLLoop:
 	return nil
 }
 
+func (spec Spec) validatePoliciesSpec() error {
+	for _, p := range spec.Policies {
+		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
+}
+
+// Validate implements custom validation for Spec
+func (spec Spec) Validate() error {
+	err := spec.validateURLPoliciesUsage()
+	if err != nil {
+		return err
+	}
+	err = spec.validatePoliciesSpec()
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 func (url *URLRule) buildPolicy() *libcb.Policy {
 	policy := libcb.Policy{
 		FailureRateThreshold:             url.policy.FailureRateThreshold,

pkg/filter/circuitbreaker/circuitbreaker_test.go

@@ -159,3 +159,84 @@ func TestBuildPolicy(t *testing.T) {
 		t.Error("wait duration in open is not 1m")
 	}
 }
+
+func TestValidate(t *testing.T) {
+	t.Run("invalidFailureCode", func(t *testing.T) {
+		const yamlSpec = `
+kind: CircuitBreaker
+name: circuitbreaker
+policies:
+- name: default
+  failureRateThreshold: 50
+  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 threshold and not set failure code, that did not fail")
+		}
+	})
+	t.Run("validFailureLegalityByCountingNetworkError", func(t *testing.T) {
+		const yamlSpec = `
+kind: CircuitBreaker
+name: circuitbreaker
+policies:
+- name: default
+  failureRateThreshold: 50
+  slidingWindowType: COUNT_BASED
+  slidingWindowSize: 10
+  countingNetworkError: true
+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 threshold and countingNetworkError is true, not set failure code, that is fail")
+		}
+	})
+
+	t.Run("validFailureLegalityByFailureStatusCodes", func(t *testing.T) {
+		const yamlSpec = `
+kind: CircuitBreaker
+name: circuitbreaker
+policies:
+- name: default
+  failureRateThreshold: 50
+  slidingWindowType: COUNT_BASED
+  slidingWindowSize: 10
+  failureStatusCodes: [500]
+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 threshold and countingNetworkError is true, not set failure code, that is fail")
+		}
+	})
+}