diff --git a/cmd/webhook/admission/admission.go b/cmd/webhook/admission/admission.go index 0c9ad54ff5..a13dfe3437 100644 --- a/cmd/webhook/admission/admission.go +++ b/cmd/webhook/admission/admission.go @@ -83,8 +83,8 @@ func (RouteGroupAdmitter) admit(req *admissionRequest) (*admissionResponse, erro }, }, nil } - rgValidator := definitions.RouteGroupValidator{} - err = rgValidator.Validate(&rgItem) + + err = definitions.ValidateRouteGroup(&rgItem) if err != nil { emsg := fmt.Sprintf("could not validate RouteGroup, %v", err) log.Error(emsg) diff --git a/dataclients/kubernetes/clusterclient.go b/dataclients/kubernetes/clusterclient.go index 4084a5872f..1b16fa4a3b 100644 --- a/dataclients/kubernetes/clusterclient.go +++ b/dataclients/kubernetes/clusterclient.go @@ -358,11 +358,10 @@ func (c *clusterClient) LoadRouteGroups() ([]*definitions.RouteGroupItem, error) return nil, err } - rgValidator := definitions.RouteGroupValidator{} rgs := make([]*definitions.RouteGroupItem, 0, len(rgl.Items)) for _, i := range rgl.Items { // Validate RouteGroup item. - if err := rgValidator.Validate(i); err != nil { + if err := definitions.ValidateRouteGroup(i); err != nil { log.Errorf("[routegroup] %v", err) continue } diff --git a/dataclients/kubernetes/definitions/routegroupvalidator.go b/dataclients/kubernetes/definitions/routegroupvalidator.go index ab0b99fcce..a55e30be5f 100644 --- a/dataclients/kubernetes/definitions/routegroupvalidator.go +++ b/dataclients/kubernetes/definitions/routegroupvalidator.go @@ -2,60 +2,77 @@ package definitions import ( "encoding/json" + "fmt" + + "errors" - "github.com/pkg/errors" "github.com/zalando/skipper/eskip" "github.com/zalando/skipper/filters" "github.com/zalando/skipper/loadbalancer" "gopkg.in/yaml.v2" ) -func uniqueStrings(s []string) []string { - u := make([]string, 0, len(s)) - m := make(map[string]bool) - for _, si := range s { - if m[si] { - continue - } +type RouteGroupValidator struct { + FilterRegistry filters.Registry +} - m[si] = true - u = append(u, si) +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 +} - return u +func (rgv *RouteGroupValidator) BasicValidation(item *RouteGroupItem) error { + return item.validate() } -func backendTypeFromString(s string) (eskip.BackendType, error) { - switch s { - case "", "service": - return ServiceBackend, nil - default: - return eskip.BackendTypeFromString(s) - } +func (rgv *RouteGroupValidator) FiltersValidation(item *RouteGroupItem) error { + return validateRouteGroupFilters(item, rgv.FilterRegistry) } -func (sb *SkipperBackend) validate() error { - if sb.parseError != nil { - return sb.parseError +func (rgv *RouteGroupValidator) ValidateList(rl *RouteGroupList) error { + err := rgv.BasicValidationList(rl) + + if rgv.FilterRegistry != nil { + err = errors.Join(err, rgv.FiltersValidationList(rl)) } + return err +} - if sb == nil || sb.Name == "" { - return errUnnamedBackend +func (rgv *RouteGroupValidator) BasicValidationList(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.BasicValidation(i)) } + return err +} - switch { - case sb.Type == eskip.NetworkBackend && sb.Address == "": - return missingAddress(sb.Name) - case sb.Type == ServiceBackend && sb.ServiceName == "": - return missingServiceName(sb.Name) - case sb.Type == ServiceBackend && - (sb.ServicePort == 0 || sb.ServicePort != int(uint16(sb.ServicePort))): - return invalidServicePort(sb.Name, sb.ServicePort) - case sb.Type == eskip.LBBackend && len(sb.Endpoints) == 0: - return missingEndpoints(sb.Name) +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)) } + return err +} - return nil +// 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 @@ -108,54 +125,56 @@ func (rg *RouteGroupSpec) UniqueHosts() []string { return uniqueStrings(rg.Hosts) } -func hasEmpty(s []string) bool { - for _, si := range s { - if si == "" { - return true - } - } +func (r *RouteSpec) UniqueMethods() []string { + return uniqueStrings(r.Methods) +} - return false +// validateRouteGroup validates a RouteGroupItem +func ValidateRouteGroup(rg *RouteGroupItem) error { + return rgv.Validate(rg) } -func (brs BackendReferences) validate(backends map[string]bool) error { - if brs == nil { - return nil - } - names := make(map[string]struct{}, len(brs)) - for _, br := range brs { - if _, ok := names[br.BackendName]; ok { - return duplicateBackendReference(br.BackendName) - } - names[br.BackendName] = struct{}{} +// ValidateRouteGroups validates a RouteGroupList +func ValidateRouteGroups(rl *RouteGroupList) error { + return rgv.ValidateList(rl) +} - if err := br.validate(backends); err != nil { - return err +func validateRouteGroupFilters(rg *RouteGroupItem, fr filters.Registry) error { + // basic for now + for _, r := range rg.Spec.Routes { + for _, f := range r.Filters { + parsedFilter, 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) + } } } + return nil } -func (br *BackendReference) validate(backends map[string]bool) error { - if br == nil || br.BackendName == "" { - return errUnnamedBackendReference +// TODO: we need to pass namespace/name in all errors +func (r *RouteGroupItem) validate() error { + // has metadata and name: + if r == nil || validate(r.Metadata) != nil { + return errRouteGroupWithoutName } - if !backends[br.BackendName] { - return invalidBackendReference(br.BackendName) + // has spec: + if r.Spec == nil { + return routeGroupError(r.Metadata, errRouteGroupWithoutSpec) } - if br.Weight < 0 { - return invalidBackendWeight(br.BackendName, br.Weight) + if err := r.Spec.validate(); err != nil { + return routeGroupError(r.Metadata, err) } return nil } -func (r *RouteSpec) UniqueMethods() []string { - return uniqueStrings(r.Methods) -} - // TODO: we need to pass namespace/name in all errors func (rg *RouteGroupSpec) validate() error { // has at least one backend: @@ -227,104 +246,94 @@ func (r *RouteSpec) validate(hasDefault bool, backends map[string]bool) error { return nil } -// TODO: we need to pass namespace/name in all errors -func (r *RouteGroupItem) validate() error { - // has metadata and name: - if r == nil || validate(r.Metadata) != nil { - return errRouteGroupWithoutName +func (br *BackendReference) validate(backends map[string]bool) error { + if br == nil || br.BackendName == "" { + return errUnnamedBackendReference } - // has spec: - if r.Spec == nil { - return routeGroupError(r.Metadata, errRouteGroupWithoutSpec) + if !backends[br.BackendName] { + return invalidBackendReference(br.BackendName) } - if err := r.Spec.validate(); err != nil { - return routeGroupError(r.Metadata, err) + if br.Weight < 0 { + return invalidBackendWeight(br.BackendName, br.Weight) } return nil } -// 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 (brs BackendReferences) validate(backends map[string]bool) error { + if brs == nil { + return nil + } + names := make(map[string]struct{}, len(brs)) + for _, br := range brs { + if _, ok := names[br.BackendName]; ok { + return duplicateBackendReference(br.BackendName) + } + names[br.BackendName] = struct{}{} -type RouteGroupValidator struct { - FilterRegistry filters.Registry + if err := br.validate(backends); err != nil { + return err + } + } + return nil } -func (rgv *RouteGroupValidator) Validate(item *RouteGroupItem) error { - err := validateRouteGroup(item) +func (sb *SkipperBackend) validate() error { + if sb.parseError != nil { + return sb.parseError + } - if rgv.FilterRegistry != nil { - nerr := validateRouteGroupFilters(item, rgv.FilterRegistry) - if nerr != nil { - err = errors.Wrap(err, nerr.Error()) - } + if sb == nil || sb.Name == "" { + return errUnnamedBackend } - return err -} + switch { + case sb.Type == eskip.NetworkBackend && sb.Address == "": + return missingAddress(sb.Name) + case sb.Type == ServiceBackend && sb.ServiceName == "": + return missingServiceName(sb.Name) + case sb.Type == ServiceBackend && + (sb.ServicePort == 0 || sb.ServicePort != int(uint16(sb.ServicePort))): + return invalidServicePort(sb.Name, sb.ServicePort) + case sb.Type == eskip.LBBackend && len(sb.Endpoints) == 0: + return missingEndpoints(sb.Name) + } -// validateRouteGroup validates a RouteGroupItem -func validateRouteGroup(rg *RouteGroupItem) error { - return rg.validate() + return nil } -func validateRouteGroupFilters(rg *RouteGroupItem, fr filters.Registry) error { - // basic for now - for _, r := range rg.Spec.Routes { - for _, f := range r.Filters { - parsedFilter, err := eskip.ParseFilters(f) - if err != nil { - return err - } - if _, ok := fr[parsedFilter[0].Name]; !ok { - return errors.Errorf("filter %s not found", parsedFilter[0].Name) - } +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 nil + return u } -func (rgv *RouteGroupValidator) ValidateList(rl *RouteGroupList) error { - err := validateRouteGroups(rl) - if err != nil { - return err - } - if rgv.FilterRegistry != nil { - for _, rg := range rl.Items { - nerr := validateRouteGroupFilters(rg, rgv.FilterRegistry) - if nerr != nil { - err = errors.Wrap(err, nerr.Error()) - } - } +func backendTypeFromString(s string) (eskip.BackendType, error) { + switch s { + case "", "service": + return ServiceBackend, nil + default: + return eskip.BackendTypeFromString(s) } - - return nil } -// validateRouteGroups validates a RouteGroupList -func validateRouteGroups(rl *RouteGroupList) error { - var err error - // avoid the user having to repeatedly validate to discover all errors - for _, i := range rl.Items { - nerr := validateRouteGroup(i) - if nerr != nil { - err = errors.Wrap(err, nerr.Error()) +func hasEmpty(s []string) bool { + for _, si := range s { + if si == "" { + return true } } - return err + + return false }