Skip to content

Commit

Permalink
fix(apiusagemonitoring): create noop filter in case of invalid config (…
Browse files Browse the repository at this point in the history 
…zalando#949)
  • Loading branch information
@maxim-tschumak
maxim-tschumak committed Feb 25, 2019
1 parent 7a2b444 commit cbde191
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 55 deletions.
4 changes: 2 additions & 2 deletions filters/apiusagemonitoring/noop.go
View file
Expand Up @@ -16,5 +16,5 @@ func (s *noopSpec) CreateFilter(config []interface{}) (filters.Filter, error) {

type noopFilter struct{}

func (*noopFilter) Request(filters.FilterContext) {}
func (*noopFilter) Response(filters.FilterContext) {}
func (noopFilter) Request(filters.FilterContext) {}
func (noopFilter) Response(filters.FilterContext) {}
4 changes: 2 additions & 2 deletions filters/apiusagemonitoring/spec.go
View file
Expand Up @@ -2,7 +2,6 @@ package apiusagemonitoring

import (
"encoding/json"
"fmt"
"github.com/sirupsen/logrus"
"github.com/zalando/skipper/filters"
"regexp"
Expand Down Expand Up @@ -115,7 +114,8 @@ func (s *apiUsageMonitoringSpec) CreateFilter(args []interface{}) (filter filter
paths := s.buildPathInfoListFromConfiguration(apis)

if len(paths) == 0 {
return nil, fmt.Errorf("no valid configurations")
log.Error("no valid configurations, creating noop api usage monitoring filter")
return noopFilter{}, nil
}

filter = &apiUsageMonitoringFilter{
Expand Down
91 changes: 40 additions & 51 deletions filters/apiusagemonitoring/spec_test.go
View file
Expand Up @@ -88,101 +88,93 @@ func Test_FeatureNotEnabled_TypeNameAndCreatedFilterAreRight(t *testing.T) {
assert.Equal(t, filter, &noopFilter{})
}

func Test_CreateFilter_NoParam_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_NoParam_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{})
filter, err := spec.CreateFilter([]interface{}{})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_EmptyString_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_EmptyString_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{""})
filter, err := spec.CreateFilter([]interface{}{""})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_NotAString_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_NotAString_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{1234})
filter, err := spec.CreateFilter([]interface{}{1234})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_NotJson_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_NotJson_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{"I am not JSON"})
filter, err := spec.CreateFilter([]interface{}{"I am not JSON"})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_EmptyJson_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_EmptyJson_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{"{}"})
filter, err := spec.CreateFilter([]interface{}{"{}"})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_NoPathTemplate_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_NoPathTemplate_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{`{
filter, err := spec.CreateFilter([]interface{}{`{
"application_id": "app",
"api_id": "api",
"path_templates": []
}`})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_EmptyPathTemplate_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_EmptyPathTemplate_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{`{
filter, err := spec.CreateFilter([]interface{}{`{
"application_id": "my_app",
"api_id": "my_api",
"path_templates": [
""
]
}`})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_TypoInPropertyNames_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_TypoInPropertyNames_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

// path_template has no `s` and should cause a JSON decoding error.
_, err := spec.CreateFilter([]interface{}{`{
filter, err := spec.CreateFilter([]interface{}{`{
"application_id": "my_app",
"api_id": "my_api",
"path_template": [
""
]
}`})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_NonParsableParametersShouldBeLoggedAndIgnored(t *testing.T) {
Expand Down Expand Up @@ -265,36 +257,33 @@ func Test_CreateFilter_FullConfigSingleApi(t *testing.T) {
})
}

func Test_CreateFilter_NoApplicationId_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_NoApplicationId_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{`{
filter, err := spec.CreateFilter([]interface{}{`{
"api_id": "api",
"path_templates": [
"foo/orders"
]
}`})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())
assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)

}

func Test_CreateFilter_NoApiId_ShouldReturnError(t *testing.T) {
func Test_CreateFilter_NoApiId_ShouldReturnNoopFilter(t *testing.T) {
spec := NewApiUsageMonitoring(true, "", "", "")

_, err := spec.CreateFilter([]interface{}{`{
filter, err := spec.CreateFilter([]interface{}{`{
"application_id": "api",
"path_templates": [
"foo/orders"
]
}`})

assert.NotNil(t, err)
assert.Error(t, err)
assert.Regexp(t, `.*no valid configurations.*`, err.Error())

assert.Nil(t, err)
assert.Equal(t, noopFilter{}, filter)
}

func Test_CreateFilter_FullConfigMultipleApis(t *testing.T) {
Expand Down

0 comments on commit cbde191

Please sign in to comment.