Using YAML configuration file to pass args for skipper binary (#1205)

* parse flags into struct that can also be unmarshaled from yaml Signed-off-by: Guilherme Oliveira <[email protected]> * adjusting and extending tests for yaml configuration unmarshal Signed-off-by: Guilherme Oliveira <[email protected]> * adding a section of documentation to Basics and an yaml alternative at Plugins Signed-off-by: Guilherme Oliveira <[email protected]> * give preference to flags instead of yaml file Signed-off-by: Guilherme Oliveira <[email protected]> * add deprecation warning log when using local rate limit Signed-off-by: Guilherme Oliveira <[email protected]> * renaming method to validate since it doesnt return boolean and small adjustment for test output format Signed-off-by: Guilherme Oliveira <[email protected]>
zalando · Oct 14, 2019 · 7cd0f61 · 7cd0f61
1 parent 68865a5
commit 7cd0f61
Show file tree

Hide file tree

Showing 23 changed files with 1,565 additions and 902 deletions.
diff --git a/circuit/breaker.go b/circuit/breaker.go
@@ -1,6 +1,7 @@
 package circuit
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
 	"time"
@@ -9,6 +10,26 @@ import (
 // BreakerType defines the type of the used breaker: consecutive, rate or disabled.
 type BreakerType int
 
+func (b *BreakerType) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var value string
+	if err := unmarshal(&value); err != nil {
+		return err
+	}
+
+	switch value {
+	case "consecutive":
+		*b = ConsecutiveFailures
+	case "rate":
+		*b = FailureRate
+	case "disabled":
+		*b = BreakerDisabled
+	default:
+		return fmt.Errorf("invalid breaker type %v (allowed values are: consecutive, rate or disabled)", value)
+	}
+
+	return nil
+}
+
 const (
 	BreakerNone BreakerType = iota
 	ConsecutiveFailures
@@ -21,12 +42,13 @@ const (
 // See the package overview for the detailed merging/overriding rules of the settings and for the meaning of the
 // individual fields.
 type BreakerSettings struct {
-	Type             BreakerType
-	Host             string
-	Window, Failures int
-	Timeout          time.Duration
-	HalfOpenRequests int
-	IdleTTL          time.Duration
+	Type             BreakerType   `yaml:"type"`
+	Host             string        `yaml:"host"`
+	Window           int           `yaml:"window"`
+	Failures         int           `yaml:"failures"`
+	Timeout          time.Duration `yaml:"timeout"`
+	HalfOpenRequests int           `yaml:"half-open-requests"`
+	IdleTTL          time.Duration `yaml:"idle-ttl"`
 }
 
 type breakerImplementation interface {

diff --git a/cmd/skipper/breakerflags.go b/cmd/skipper/breakerflags.go
@@ -24,15 +24,11 @@ const enableBreakersUsage = `enable breakers to be set from filters without prov
 
 type breakerFlags []circuit.BreakerSettings
 
-var errInvalidBreakerConfig = errors.New("invalid breaker config")
+var errInvalidBreakerConfig = errors.New("invalid breaker config (allowed values are: consecutive, rate or disabled)")
 
-func (b *breakerFlags) String() string {
-	if b == nil {
-		return ""
-	}
-
-	s := make([]string, len(*b))
-	for i, bi := range *b {
+func (b breakerFlags) String() string {
+	s := make([]string, len(b))
+	for i, bi := range b {
 		s[i] = bi.String()
 	}
 
@@ -110,3 +106,13 @@ func (b *breakerFlags) Set(value string) error {
 	*b = append(*b, s)
 	return nil
 }
+
+func (b *breakerFlags) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	var breakerSettings circuit.BreakerSettings
+	if err := unmarshal(&breakerSettings); err != nil {
+		return err
+	}
+
+	*b = append(*b, breakerSettings)
+	return nil
+}
diff --git a/cmd/skipper/breakerflags_test.go b/cmd/skipper/breakerflags_test.go
@@ -4,6 +4,8 @@ import (
 	"testing"
 	"time"
 
+	"gopkg.in/yaml.v2"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/zalando/skipper/circuit"
 )
@@ -99,6 +101,11 @@ func Test_breakerFlags_Set(t *testing.T) {
 				IdleTTL:          5 * time.Second,
 			},
 		},
+		{
+			name:    "test breaker settings invalid type",
+			args:    "type=invalid,host=example.com,timeout=3s,half-open-requests=3,idle-ttl=5s",
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -114,9 +121,92 @@ func Test_breakerFlags_Set(t *testing.T) {
 					t.Errorf("Failed to have breaker created: %d != 1", len(b))
 				}
 
-				bs := b[0]
-				if bs.Type != tt.want.Type || bs.Host != tt.want.Host || bs.Window != tt.want.Window || bs.Failures != tt.want.Failures || bs.Timeout != tt.want.Timeout || bs.HalfOpenRequests != tt.want.HalfOpenRequests || bs.IdleTTL != tt.want.IdleTTL {
-					t.Errorf("breakerFlags.Set() got v, want v, %v", cmp.Diff(bs, tt.want))
+				if cmp.Equal(b[0], tt.want) == false {
+					t.Errorf("breakerFlags.Set() got v, want v, %v", cmp.Diff(b[0], tt.want))
+				}
+			}
+
+		})
+	}
+}
+
+func Test_breakerFlags_UnmarshalYAML(t *testing.T) {
+	tests := []struct {
+		name    string
+		yml     string
+		wantErr bool
+		want    circuit.BreakerSettings
+	}{
+		{
+			name: "test breaker settings",
+			yml: `type: consecutive
+host: example.com
+timeout: 3s
+half-open-requests: 3
+idle-ttl: 5s`,
+			wantErr: false,
+			want: circuit.BreakerSettings{
+				Type:             circuit.ConsecutiveFailures,
+				Host:             "example.com",
+				Timeout:          3 * time.Second,
+				HalfOpenRequests: 3,
+				IdleTTL:          5 * time.Second,
+			},
+		},
+		{
+			name: "test breaker settings with window",
+			yml: `type: consecutive
+window: 4
+host: example.com
+timeout: 3s
+half-open-requests: 3
+idle-ttl: 5s`,
+			wantErr: false,
+			want: circuit.BreakerSettings{
+				Type:             circuit.ConsecutiveFailures,
+				Host:             "example.com",
+				Timeout:          3 * time.Second,
+				HalfOpenRequests: 3,
+				IdleTTL:          5 * time.Second,
+				Window:           4,
+			},
+		},
+		{
+			name: "test breaker settings with wrong window",
+			yml: `type: disabled
+window: 4s
+host: example.com
+timeout: 3s
+half-open-requests: 3
+idle-ttl: 5s`,
+			wantErr: true,
+		},
+		{
+			name: "test breaker settings invalid type",
+			yml: `type: invalid
+host: example.com
+timeout: 3s
+half-open-requests: 3
+idle-ttl: 5s`,
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			bp := &breakerFlags{}
+
+			if err := yaml.Unmarshal([]byte(tt.yml), bp); (err != nil) != tt.wantErr {
+				t.Errorf("breakerFlags.UnmarshalYAML() error = %v, wantErr %v", err, tt.wantErr)
+			}
+
+			if !tt.wantErr {
+				b := *bp
+				if len(b) != 1 {
+					t.Errorf("Failed to have breaker created: %d != 1", len(b))
+				}
+
+				if cmp.Equal(b[0], tt.want) == false {
+					t.Errorf("breakerFlags.UnmarshalYAML() got v, want v, %v", cmp.Diff(b[0], tt.want))
 				}
 			}