From ff7d8a06bc34a53623e1c0127ddc4d6f96c692a5 Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Fri, 31 May 2024 12:50:18 +0200 Subject: [PATCH] config: support multiple default filters flag values (#3084) This is useful for templated configurations that conditionally add various default filters. Signed-off-by: Alexander Yastrebov --- config/defaultfilterflags.go | 27 +++-- config/defaultfilterflags_test.go | 165 +++++++++++++++++++++--------- 2 files changed, 138 insertions(+), 54 deletions(-) diff --git a/config/defaultfilterflags.go b/config/defaultfilterflags.go index 6be5ea6d28..162c92f6f7 100644 --- a/config/defaultfilterflags.go +++ b/config/defaultfilterflags.go @@ -2,16 +2,18 @@ package config import ( "fmt" + "strings" "github.com/zalando/skipper/eskip" ) type defaultFiltersFlags struct { + values []string filters []*eskip.Filter } func (dpf defaultFiltersFlags) String() string { - return "" + return strings.Join(dpf.values, " -> ") } func (dpf *defaultFiltersFlags) Set(value string) error { @@ -20,14 +22,27 @@ func (dpf *defaultFiltersFlags) Set(value string) error { return fmt.Errorf("failed to parse default filters: %w", err) } - dpf.filters = fs + dpf.values = append(dpf.values, value) + dpf.filters = append(dpf.filters, fs...) + return nil } func (dpf *defaultFiltersFlags) UnmarshalYAML(unmarshal func(interface{}) error) error { - var value string - if err := unmarshal(&value); err != nil { - return err + values := make([]string, 1) + if err := unmarshal(&values); err != nil { + // Try to unmarshal as string for backwards compatibility. + // UnmarshalYAML allows calling unmarshal more than once. + if err := unmarshal(&values[0]); err != nil { + return err + } + } + dpf.values = nil + dpf.filters = nil + for _, v := range values { + if err := dpf.Set(v); err != nil { + return err + } } - return dpf.Set(value) + return nil } diff --git a/config/defaultfilterflags_test.go b/config/defaultfilterflags_test.go index 716bdbf8b1..f1bc55004a 100644 --- a/config/defaultfilterflags_test.go +++ b/config/defaultfilterflags_test.go @@ -5,85 +5,154 @@ import ( "gopkg.in/yaml.v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/zalando/skipper/eskip" ) -func Test_defaultFiltersFlags_String(t *testing.T) { +func TestDefaultFiltersFlagsSet(t *testing.T) { + oneFilter := eskip.MustParseFilters(`tee("https://www.zalando.de/")`) + manyFilters := eskip.MustParseFilters(`ratelimit(5, "10s") -> tee("https://www.zalando.de/")`) + manyMoreFilters := eskip.MustParseFilters(`ratelimit(5, "10s") -> tee("https://www.zalando.de/") -> inlineContent("OK\n")`) + tests := []struct { - name string - filters []*eskip.Filter - want string + name string + args []string + yaml string + want []*eskip.Filter + wantString string }{ { - name: "test string", - filters: []*eskip.Filter{}, - want: "", + name: "test no filter", + args: nil, + want: nil, + wantString: "", + }, + { + name: "test empty filter", + args: []string{""}, + yaml: "", + want: nil, + wantString: "", + }, + { + name: "test one filter", + args: []string{`tee("https://www.zalando.de/")`}, + yaml: `field: tee("https://www.zalando.de/")`, + want: oneFilter, + wantString: `tee("https://www.zalando.de/")`, + }, + { + name: "test many filters in one value", + args: []string{`ratelimit(5, "10s") -> tee("https://www.zalando.de/")`}, + yaml: `field: ratelimit(5, "10s") -> tee("https://www.zalando.de/")`, + want: manyFilters, + wantString: `ratelimit(5, "10s") -> tee("https://www.zalando.de/")`, + }, + { + name: "test multiple values", + args: []string{`ratelimit(5, "10s") -> tee("https://www.zalando.de/")`, `inlineContent("OK\n")`}, + yaml: ` +field: + - ratelimit(5, "10s") -> tee("https://www.zalando.de/") + - inlineContent("OK\n") +`, + want: manyMoreFilters, + wantString: `ratelimit(5, "10s") -> tee("https://www.zalando.de/") -> inlineContent("OK\n")`, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - dpf := &defaultFiltersFlags{ - filters: tt.filters, + cfg := struct { + Field *defaultFiltersFlags + }{ + Field: &defaultFiltersFlags{}, } - if got := dpf.String(); got != tt.want { - t.Errorf("defaultFiltersFlags.String() = %v, want %v", got, tt.want) + + var err error + for _, arg := range tt.args { + err = cfg.Field.Set(arg) + if err != nil { + break + } + } + require.NoError(t, err) + + assert.Equal(t, tt.wantString, cfg.Field.String()) + assert.Equal(t, tt.want, cfg.Field.filters) + + if tt.yaml != "" { + err := yaml.Unmarshal([]byte(tt.yaml), &cfg) + require.NoError(t, err) + + assert.Equal(t, tt.want, cfg.Field.filters) } }) } } -func Test_defaultFiltersFlags_Set(t *testing.T) { - oneFilter, _ := eskip.ParseFilters(`tee("https://www.zalando.de/")`) - manyFilters, _ := eskip.ParseFilters(`ratelimit(5, "10s") -> tee("https://www.zalando.de/")`) +func TestDefaultFiltersFlagsInvalidFlag(t *testing.T) { tests := []struct { - name string - args string - want []*eskip.Filter - wantErr bool + name string + args []string }{ { - name: "test no filter", - args: "", - want: nil, - wantErr: false, + name: "test single filter", + args: []string{"invalid-filter"}, }, { - name: "test error filter", - args: "invalid-filter", - wantErr: true, + name: "test multiple filters", + args: []string{`status(204)`, "invalid-filter"}, }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := struct { + Field *defaultFiltersFlags + }{ + Field: &defaultFiltersFlags{}, + } + + var err error + for _, arg := range tt.args { + err = cfg.Field.Set(arg) + if err != nil { + break + } + } + assert.Error(t, err) + }) + } +} + +func TestDefaultFiltersFlagsInvalidYaml(t *testing.T) { + tests := []struct { + name string + yaml string + }{ { - name: "test one filter", - args: `tee("https://www.zalando.de/")`, - want: oneFilter, - wantErr: false, + name: "test one value", + yaml: `field: invalid-filter`, }, { - name: "test many filters", - args: `ratelimit(5, "10s") -> tee("https://www.zalando.de/")`, - want: manyFilters, - wantErr: false, + name: "test multiple values", + yaml: ` +field: + - ratelimit(5, "10s") -> tee("https://www.zalando.de/") + - invalid-filter + `, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - dpf := &defaultFiltersFlags{} - if err := dpf.Set(tt.args); (err != nil) != tt.wantErr { - t.Errorf("defaultFiltersFlags.Set() error = %v, wantErr %v", err, tt.wantErr) + cfg := struct { + Field *defaultFiltersFlags + }{ + Field: &defaultFiltersFlags{}, } - if !tt.wantErr { - if len(tt.want) != len(dpf.filters) { - t.Errorf("defaultFiltersFlags size mismatch got %d want %d", len(dpf.filters), len(tt.want)) - } - if err := yaml.Unmarshal([]byte(tt.args), dpf); err != nil { - t.Errorf("defaultFiltersFlags.UnmarshalYAML() error = %v, wantErr %v", err, tt.wantErr) - } - - if len(tt.want) != len(dpf.filters) { - t.Errorf("defaultFiltersFlags from yaml size mismatch got %d want %d", len(dpf.filters), len(tt.want)) - } - } + err := yaml.Unmarshal([]byte(tt.yaml), &cfg) + require.Error(t, err) }) } }