Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create validator object to give more validation flexibility #2484

Merged
merged 12 commits into from
Aug 2, 2023
Merged
Next Next commit
Create validator object to give more validation flexiability
Signed-off-by: Mustafa Abdelrahman <mustafa.abdelrahman@zalando.de>
  • Loading branch information
MustafaSaber committed Aug 2, 2023
commit ad8aab5d30b3a81cc8b22e7e8dd83db43883a902
4 changes: 2 additions & 2 deletions cmd/webhook/admission/admission.go
Original file line number Diff line number Diff line change
@@ -83,8 +83,8 @@ func (RouteGroupAdmitter) admit(req *admissionRequest) (*admissionResponse, erro
},
}, nil
}

err = definitions.ValidateRouteGroup(&rgItem)
rgValidator := definitions.RouteGroupValidator{}
err = rgValidator.Validate(&rgItem)
MustafaSaber marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
emsg := fmt.Sprintf("could not validate RouteGroup, %v", err)
log.Error(emsg)
3 changes: 2 additions & 1 deletion dataclients/kubernetes/clusterclient.go
Original file line number Diff line number Diff line change
@@ -358,10 +358,11 @@ 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 := definitions.ValidateRouteGroup(i); err != nil {
if err := rgValidator.Validate(i); err != nil {
log.Errorf("[routegroup] %v", err)
continue
}
259 changes: 0 additions & 259 deletions dataclients/kubernetes/definitions/routegroups.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package definitions

import (
"encoding/json"
"fmt"

"errors"
@@ -202,261 +201,3 @@ 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)
}

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 (sb *SkipperBackend) validate() error {
if sb.parseError != nil {
return sb.parseError
}

if sb == nil || sb.Name == "" {
return errUnnamedBackend
}

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)
}

return nil
}

// 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 hasEmpty(s []string) bool {
for _, si := range s {
if si == "" {
return true
}
}

return false
}

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{}{}

if err := br.validate(backends); err != nil {
return err
}
}
return nil
}

func (br *BackendReference) validate(backends map[string]bool) error {
if br == nil || br.BackendName == "" {
return errUnnamedBackendReference
}

if !backends[br.BackendName] {
return invalidBackendReference(br.BackendName)
}

if br.Weight < 0 {
return invalidBackendWeight(br.BackendName, br.Weight)
}

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:
if len(rg.Backends) == 0 {
return errRouteGroupWithoutBackend
}

// backends valid and have unique names:
backends := make(map[string]bool)
for _, b := range rg.Backends {
if backends[b.Name] {
return backendsWithDuplicateName(b.Name)
}

backends[b.Name] = true
if err := b.validate(); err != nil {
return invalidBackend(b.Name, err)
}
}

hasDefault := len(rg.DefaultBackends) > 0
if err := rg.DefaultBackends.validate(backends); err != nil {
return err
}

if !hasDefault && len(rg.Routes) == 0 {
return errMissingBackendReference
}

for i, r := range rg.Routes {
if err := r.validate(hasDefault, backends); err != nil {
return invalidRoute(i, err)
}
}

return nil
}

// TODO: we need to pass namespace/name in all errors
func (r *RouteSpec) validate(hasDefault bool, backends map[string]bool) error {
if r == nil {
return errInvalidRouteSpec
}

if !hasDefault && len(r.Backends) == 0 {
return errMissingBackendReference
}

if err := r.Backends.validate(backends); err != nil {
return err
}

if r.Path != "" && r.PathSubtree != "" {
return errBothPathAndPathSubtree
}

if hasEmpty(r.Predicates) {
return errInvalidPredicate
}

if hasEmpty(r.Filters) {
return errInvalidFilter
}

if hasEmpty(r.Methods) {
return errInvalidMethod
}

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
}

// has spec:
if r.Spec == nil {
return routeGroupError(r.Metadata, errRouteGroupWithoutSpec)
}

if err := r.Spec.validate(); err != nil {
return routeGroupError(r.Metadata, 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
}

// ValidateRouteGroup validates a RouteGroupItem
func ValidateRouteGroup(rg *RouteGroupItem) error {
return rg.validate()
}

// ValidateRouteGroups validates a RouteGroupList
func ValidateRouteGroups(rl *RouteGroupList) error {
var errs []error
// avoid the user having to repeatedly validate to discover all errors
for _, i := range rl.Items {
errs = append(errs, ValidateRouteGroup(i))
}
return errorsJoin(errs...)
}
Loading