Skip to content

Commit

Permalink
valid circuit-breaker filter spec by correlation (#551)
Browse files Browse the repository at this point in the history
* valid circuit-breaker filter spec by correlation

* fix  circuit-breaker filter failure legality validate

* fix error message
  • Loading branch information
sodaRyCN authored and localvar committed Mar 29, 2022
1 parent 919e41b commit ee00bd6
Show file tree
Hide file tree
Showing 2 changed files with 107 additions and 4 deletions.
30 changes: 26 additions & 4 deletions pkg/filters/circuitbreaker/circuitbreaker.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,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"`
Expand Down Expand Up @@ -98,8 +98,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
Expand All @@ -119,6 +119,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 set 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,
Expand Down
81 changes: 81 additions & 0 deletions pkg/filters/circuitbreaker/circuitbreaker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,3 +158,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")
}
})
}

0 comments on commit ee00bd6

Please sign in to comment.