Start with eskip.ParseFilters and fix failing tests logs

Signed-off-by: Mustafa Abdelrahman <[email protected]>

dataclients/kubernetes/definitions/routegroups.go

@@ -1,11 +1,13 @@
 package definitions
 
 import (
+	"encoding/json"
 	"fmt"
 
 	"github.com/pkg/errors"
 	"github.com/zalando/skipper/eskip"
 	"github.com/zalando/skipper/loadbalancer"
+	"gopkg.in/yaml.v2"
 )
 
 // adding Kubernetes specific backend types here. To be discussed.
@@ -200,3 +202,105 @@ func missingEndpoints(backendName string) error {
 func routeGroupError(m *Metadata, err error) error {
 	return fmt.Errorf("error in route group %s/%s: %w", namespaceString(m.Namespace), m.Name, err)
 }
+
+// UnmarshalJSON creates a new skipperBackend, safe to be called on nil pointer
+func (sb *SkipperBackend) UnmarshalJSON(value []byte) error {
+	if sb == nil {
+		return nil
+	}
+
+	var p skipperBackendParser
+	if err := json.Unmarshal(value, &p); err != nil {
+		return err
+	}
+
+	var perr error
+	bt, err := backendTypeFromString(p.Type)
+	if err != nil {
+		// we cannot return an error here, because then the parsing of
+		// all route groups would fail. We'll report the error in the
+		// validation phase, only for the containing route group
+		perr = err
+	}
+
+	a, err := loadbalancer.AlgorithmFromString(p.Algorithm)
+	if err != nil {
+		// we cannot return an error here, because then the parsing of
+		// all route groups would fail. We'll report the error in the
+		// validation phase, only for the containing route group
+		perr = err
+	}
+
+	if a == loadbalancer.None {
+		a = loadbalancer.RoundRobin
+	}
+
+	var b SkipperBackend
+	b.Name = p.Name
+	b.Type = bt
+	b.Address = p.Address
+	b.ServiceName = p.ServiceName
+	b.ServicePort = p.ServicePort
+	b.Algorithm = a
+	b.Endpoints = p.Endpoints
+	b.parseError = perr
+
+	*sb = b
+	return nil
+}
+
+func (rg *RouteGroupSpec) UniqueHosts() []string {
+	return uniqueStrings(rg.Hosts)
+}
+
+func (r *RouteSpec) UniqueMethods() []string {
+	return uniqueStrings(r.Methods)
+}
+
+// ParseRouteGroupsJSON parses a json list of RouteGroups into RouteGroupList
+func ParseRouteGroupsJSON(d []byte) (RouteGroupList, error) {
+	var rl RouteGroupList
+	err := json.Unmarshal(d, &rl)
+	return rl, err
+}
+
+// ParseRouteGroupsYAML parses a YAML list of RouteGroups into RouteGroupList
+func ParseRouteGroupsYAML(d []byte) (RouteGroupList, error) {
+	var rl RouteGroupList
+	err := yaml.Unmarshal(d, &rl)
+	return rl, err
+}
+
+func uniqueStrings(s []string) []string {
+	u := make([]string, 0, len(s))
+	m := make(map[string]bool)
+	for _, si := range s {
+		if m[si] {
+			continue
+		}
+
+		m[si] = true
+		u = append(u, si)
+	}
+
+	return u
+}
+
+func backendTypeFromString(s string) (eskip.BackendType, error) {
+	switch s {
+	case "", "service":
+		return ServiceBackend, nil
+	default:
+		return eskip.BackendTypeFromString(s)
+	}
+}
+
+func hasEmpty(s []string) bool {
+	for _, si := range s {
+		if si == "" {
+			return true
+		}
+	}
+
+	return false
+}

dataclients/kubernetes/definitions/routegroupvalidator.go

@@ -1,155 +1,52 @@
 package definitions
 
 import (
-	"encoding/json"
-	"fmt"
-
 	"errors"
 
 	"github.com/zalando/skipper/eskip"
 	"github.com/zalando/skipper/filters"
-	"github.com/zalando/skipper/loadbalancer"
-	"gopkg.in/yaml.v2"
 )
 
 type RouteGroupValidator struct {
 	FilterRegistry filters.Registry
 }
 
-var rgv = &RouteGroupValidator{}
-
-func (rgv *RouteGroupValidator) Validate(item *RouteGroupItem) error {
-	err := rgv.BasicValidation(item)
-	if rgv.FilterRegistry != nil {
-		err = errors.Join(err, rgv.FiltersValidation(item))
-	}
-	return err
-}
+var defaultRouteGroupValidator = &RouteGroupValidator{}
 
-func (rgv *RouteGroupValidator) BasicValidation(item *RouteGroupItem) error {
-	return item.validate()
-}
-
-func (rgv *RouteGroupValidator) FiltersValidation(item *RouteGroupItem) error {
-	return validateRouteGroupFilters(item, rgv.FilterRegistry)
-}
-
-func (rgv *RouteGroupValidator) ValidateList(rl *RouteGroupList) error {
-	err := rgv.BasicValidationList(rl)
-
-	if rgv.FilterRegistry != nil {
-		err = errors.Join(err, rgv.FiltersValidationList(rl))
-	}
-	return err
+// validateRouteGroup validates a RouteGroupItem
+func ValidateRouteGroup(rg *RouteGroupItem) error {
+	return defaultRouteGroupValidator.Validate(rg)
 }
 
-func (rgv *RouteGroupValidator) BasicValidationList(rl *RouteGroupList) error {
+func ValidateRouteGroupList(rgl RouteGroupList) error {
 	var err error
-	// avoid the user having to repeatedly validate to discover all errors
-	for _, i := range rl.Items {
-		err = errors.Join(err, rgv.BasicValidation(i))
+	for _, rg := range rgl.Items {
+		err = errors.Join(err, defaultRouteGroupValidator.Validate(rg))
 	}
 	return err
 }
 
-func (rgv *RouteGroupValidator) FiltersValidationList(rl *RouteGroupList) error {
-	var err error
-	// avoid the user having to repeatedly validate to discover all errors
-	for _, i := range rl.Items {
-		err = errors.Join(err, rgv.FiltersValidation(i))
-	}
+func (rgv *RouteGroupValidator) Validate(item *RouteGroupItem) error {
+	err := rgv.basicValidation(item)
+	err = errors.Join(err, rgv.filtersValidation(item))
 	return err
 }
 
-// ParseRouteGroupsJSON parses a json list of RouteGroups into RouteGroupList
-func ParseRouteGroupsJSON(d []byte) (RouteGroupList, error) {
-	var rl RouteGroupList
-	err := json.Unmarshal(d, &rl)
-	return rl, err
-}
-
-// ParseRouteGroupsYAML parses a YAML list of RouteGroups into RouteGroupList
-func ParseRouteGroupsYAML(d []byte) (RouteGroupList, error) {
-	var rl RouteGroupList
-	err := yaml.Unmarshal(d, &rl)
-	return rl, err
-}
-
-// UnmarshalJSON creates a new skipperBackend, safe to be called on nil pointer
-func (sb *SkipperBackend) UnmarshalJSON(value []byte) error {
-	if sb == nil {
-		return nil
-	}
-
-	var p skipperBackendParser
-	if err := json.Unmarshal(value, &p); err != nil {
-		return err
-	}
-
-	var perr error
-	bt, err := backendTypeFromString(p.Type)
-	if err != nil {
-		// we cannot return an error here, because then the parsing of
-		// all route groups would fail. We'll report the error in the
-		// validation phase, only for the containing route group
-		perr = err
-	}
-
-	a, err := loadbalancer.AlgorithmFromString(p.Algorithm)
-	if err != nil {
-		// we cannot return an error here, because then the parsing of
-		// all route groups would fail. We'll report the error in the
-		// validation phase, only for the containing route group
-		perr = err
-	}
-
-	if a == loadbalancer.None {
-		a = loadbalancer.RoundRobin
-	}
-
-	var b SkipperBackend
-	b.Name = p.Name
-	b.Type = bt
-	b.Address = p.Address
-	b.ServiceName = p.ServiceName
-	b.ServicePort = p.ServicePort
-	b.Algorithm = a
-	b.Endpoints = p.Endpoints
-	b.parseError = perr
-
-	*sb = b
-	return nil
-}
-
-func (rg *RouteGroupSpec) UniqueHosts() []string {
-	return uniqueStrings(rg.Hosts)
-}
-
-func (r *RouteSpec) UniqueMethods() []string {
-	return uniqueStrings(r.Methods)
-}
-
-// validateRouteGroup validates a RouteGroupItem
-func ValidateRouteGroup(rg *RouteGroupItem) error {
-	return rgv.Validate(rg)
+func (rgv *RouteGroupValidator) basicValidation(item *RouteGroupItem) error {
+	return item.validate()
 }
 
-// ValidateRouteGroups validates a RouteGroupList
-func ValidateRouteGroups(rl *RouteGroupList) error {
-	return rgv.ValidateList(rl)
+func (rgv *RouteGroupValidator) filtersValidation(item *RouteGroupItem) error {
+	return validateRouteGroupFilters(item)
 }
 
-func validateRouteGroupFilters(rg *RouteGroupItem, fr filters.Registry) error {
-	// basic for now
+func validateRouteGroupFilters(rg *RouteGroupItem) error {
 	for _, r := range rg.Spec.Routes {
 		for _, f := range r.Filters {
-			parsedFilter, err := eskip.ParseFilters(f)
+			_, err := eskip.ParseFilters(f)
 			if err != nil {
 				return err
 			}
-			if _, ok := fr[parsedFilter[0].Name]; !ok {
-				return fmt.Errorf("filter %q not found", parsedFilter[0].Name)
-			}
 		}
 	}
 
@@ -303,37 +200,3 @@ func (sb *SkipperBackend) validate() error {
 
 	return nil
 }
-
-func uniqueStrings(s []string) []string {
-	u := make([]string, 0, len(s))
-	m := make(map[string]bool)
-	for _, si := range s {
-		if m[si] {
-			continue
-		}
-
-		m[si] = true
-		u = append(u, si)
-	}
-
-	return u
-}
-
-func backendTypeFromString(s string) (eskip.BackendType, error) {
-	switch s {
-	case "", "service":
-		return ServiceBackend, nil
-	default:
-		return eskip.BackendTypeFromString(s)
-	}
-}
-
-func hasEmpty(s []string) bool {
-	for _, si := range s {
-		if si == "" {
-			return true
-		}
-	}
-
-	return false
-}

dataclients/kubernetes/kubernetestest/fixtures.go

@@ -321,6 +321,7 @@ func testFixture(t *testing.T, f fixtureSet) {
 			}
 
 			t.Errorf("Failed to match log: %v.", err)
+			t.Logf("Got: %s", logBuf.String())
 			t.Logf("Expected: %s", string(b))
 		}
 	}

dataclients/kubernetes/logger_test.go

@@ -16,7 +16,7 @@ import (
 )
 
 func TestLogger(t *testing.T) {
-	manifest, err := os.Open("testdata/routegroups/convert/failing-filter.yaml")
+	manifest, err := os.Open("testdata/routegroups/convert/failing-predicate.yaml")
 	require.NoError(t, err)
 	defer manifest.Close()
 

dataclients/kubernetes/testdata/routegroups/convert/failing-filter.log

@@ -1,3 +1 @@
-\[eskip\] filter
-parse failed
-kind=RouteGroup name=myapp ns=foo
+\[routegroup\] parse failed after token foo, last route id: , position 8: syntax error

dataclients/kubernetes/testdata/routegroups/convert/no-catchall-for-failed-route-group.log

@@ -1,2 +1 @@
-kind=RouteGroup name=myapp
-syntax error
+\[routegroup\] parse failed after token foo, last route id: , position 8: syntax error