diff --git a/cmd/webhook/admission/admission_test.go b/cmd/webhook/admission/admission_test.go index 729e77cba0..e00dde1cf1 100644 --- a/cmd/webhook/admission/admission_test.go +++ b/cmd/webhook/admission/admission_test.go @@ -96,7 +96,7 @@ func TestRouteGroupAdmitter(t *testing.T) { { name: "not allowed", inputFile: "invalid-rg.json", - message: "Could not validate RouteGroup: error in route group n1/rg1: route group without spec", + message: "error in route group n1/rg1: route group without spec", }, { name: "valid eskip filters", @@ -105,7 +105,7 @@ func TestRouteGroupAdmitter(t *testing.T) { { name: "invalid eskip filters", inputFile: "rg-with-invalid-eskip-filters.json", - message: "Could not validate RouteGroup: parse failed after token status, last route id: , position 11: syntax error", + message: "parse failed after token status, last route id: , position 11: syntax error", }, { name: "valid eskip predicates", @@ -114,12 +114,12 @@ func TestRouteGroupAdmitter(t *testing.T) { { name: "invalid eskip predicates", inputFile: "rg-with-invalid-eskip-predicates.json", - message: "Could not validate RouteGroup: parse failed after token Method, last route id: Method, position 6: syntax error", + message: "parse failed after token Method, last route id: Method, position 6: syntax error", }, { name: "invalid eskip filters and predicates", inputFile: "rg-with-invalid-eskip-filters-and-predicates.json", - message: "Could not validate RouteGroup: parse failed after token status, last route id: , position 11: syntax error\\nparse failed after token Method, last route id: Method, position 6: syntax error", + message: "parse failed after token status, last route id: , position 11: syntax error\\nparse failed after token Method, last route id: Method, position 6: syntax error", }, } { t.Run(tc.name, func(t *testing.T) { @@ -135,7 +135,7 @@ func TestRouteGroupAdmitter(t *testing.T) { req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() - rgAdm := RouteGroupAdmitter{} + rgAdm := &RouteGroupAdmitter{} h := Handler(rgAdm) h(w, req) @@ -168,22 +168,22 @@ func TestIngressAdmitter(t *testing.T) { { name: "invalid eskip filters", inputFile: "invalid-filters.json", - message: `Ingress validation failed: parsing \"zalando.org/skipper-filter\" annotation failed: parse failed after token this, last route id: , position 9: syntax error`, + message: `invalid \"zalando.org/skipper-filter\" annotation: parse failed after token this, last route id: , position 9: syntax error`, }, { name: "invalid eskip predicates", inputFile: "invalid-predicates.json", - message: `Ingress validation failed: parsing \"zalando.org/skipper-predicate\" annotation failed: parse failed after token ), last route id: , position 15: syntax error`, + message: `invalid \"zalando.org/skipper-predicate\" annotation: parse failed after token ), last route id: , position 15: syntax error`, }, { name: "invalid eskip routes", inputFile: "invalid-routes.json", - message: `Ingress validation failed: parsing \"zalando.org/skipper-routes\" annotation failed: invalid predicate count arg`, + message: `invalid \"zalando.org/skipper-routes\" annotation: invalid predicate count arg`, }, { name: "invalid eskip filters and predicates", inputFile: "invalid-filters-and-predicates.json", - message: `Ingress validation failed: parsing \"zalando.org/skipper-filter\" annotation failed: parse failed after token this, last route id: , position 9: syntax error\nparsing \"zalando.org/skipper-predicate\" annotation failed: parse failed after token ), last route id: , position 15: syntax error`, + message: `invalid \"zalando.org/skipper-filter\" annotation: parse failed after token this, last route id: , position 9: syntax error\ninvalid \"zalando.org/skipper-predicate\" annotation: parse failed after token ), last route id: , position 15: syntax error`, }, } { t.Run(tc.name, func(t *testing.T) { @@ -199,7 +199,7 @@ func TestIngressAdmitter(t *testing.T) { req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() - ingressAdm := IngressAdmitter{} + ingressAdm := &IngressAdmitter{} h := Handler(ingressAdm) h(w, req) diff --git a/cmd/webhook/admission/ingressadmission.go b/cmd/webhook/admission/ingress.go similarity index 51% rename from cmd/webhook/admission/ingressadmission.go rename to cmd/webhook/admission/ingress.go index a67145a9f9..a7b5bf9b23 100644 --- a/cmd/webhook/admission/ingressadmission.go +++ b/cmd/webhook/admission/ingress.go @@ -2,45 +2,46 @@ package admission import ( "encoding/json" - "fmt" - log "github.com/sirupsen/logrus" "github.com/zalando/skipper/dataclients/kubernetes/definitions" ) -type IngressAdmitter struct{} +type IngressAdmitter struct { + IngressValidator *definitions.IngressV1Validator +} -func (IngressAdmitter) name() string { +func (iga *IngressAdmitter) name() string { return "ingress" } -func (IngressAdmitter) admit(req *admissionRequest) (*admissionResponse, error) { +func (iga *IngressAdmitter) admit(req *admissionRequest) (*admissionResponse, error) { + + // Serve as default validator if not set + if iga.IngressValidator == nil { + iga.IngressValidator = &definitions.IngressV1Validator{} + } + ingressItem := definitions.IngressV1Item{} err := json.Unmarshal(req.Object, &ingressItem) if err != nil { - emsg := fmt.Sprintf("Could not parse Ingress: %v", err) - log.Error(emsg) return &admissionResponse{ UID: req.UID, Allowed: false, Result: &status{ - Message: emsg, + Message: err.Error(), }, - }, nil + }, err } - ingressValidatore := definitions.IngressV1Validator{} - err = ingressValidatore.Validate(&ingressItem) + err = iga.IngressValidator.Validate(&ingressItem) if err != nil { - emsg := fmt.Sprintf("Ingress validation failed: %v", err) - log.Error(emsg) return &admissionResponse{ UID: req.UID, Allowed: false, Result: &status{ - Message: emsg, + Message: err.Error(), }, - }, nil + }, err } return &admissionResponse{ diff --git a/cmd/webhook/admission/rgadmission.go b/cmd/webhook/admission/routegroup.go similarity index 55% rename from cmd/webhook/admission/rgadmission.go rename to cmd/webhook/admission/routegroup.go index 803ab75398..58c5bb4af0 100644 --- a/cmd/webhook/admission/rgadmission.go +++ b/cmd/webhook/admission/routegroup.go @@ -2,45 +2,46 @@ package admission import ( "encoding/json" - "fmt" - log "github.com/sirupsen/logrus" "github.com/zalando/skipper/dataclients/kubernetes/definitions" ) type RouteGroupAdmitter struct { + RouteGroupValidator *definitions.RouteGroupValidator } -func (RouteGroupAdmitter) name() string { +func (rga *RouteGroupAdmitter) name() string { return "routegroup" } -func (RouteGroupAdmitter) admit(req *admissionRequest) (*admissionResponse, error) { +func (rga *RouteGroupAdmitter) admit(req *admissionRequest) (*admissionResponse, error) { + + // Serve as default validator if not set + if rga.RouteGroupValidator == nil { + rga.RouteGroupValidator = &definitions.RouteGroupValidator{} + } + rgItem := definitions.RouteGroupItem{} err := json.Unmarshal(req.Object, &rgItem) if err != nil { - emsg := fmt.Sprintf("Could not parse RouteGroup: %v", err) - log.Error(emsg) return &admissionResponse{ UID: req.UID, Allowed: false, Result: &status{ - Message: emsg, + Message: err.Error(), }, - }, nil + }, err } - err = definitions.ValidateRouteGroup(&rgItem) + err = rga.RouteGroupValidator.Validate(&rgItem) if err != nil { - emsg := fmt.Sprintf("Could not validate RouteGroup: %v", err) - log.Error(emsg) return &admissionResponse{ UID: req.UID, Allowed: false, Result: &status{ - Message: emsg, + Message: err.Error(), }, - }, nil + }, err } return &admissionResponse{ diff --git a/cmd/webhook/admission/testdata/ingress/valid-ingress-with-annotations.json b/cmd/webhook/admission/testdata/ingress/valid-ingress-with-annotations.json index 04182386bb..7fe8aa0e56 100644 --- a/cmd/webhook/admission/testdata/ingress/valid-ingress-with-annotations.json +++ b/cmd/webhook/admission/testdata/ingress/valid-ingress-with-annotations.json @@ -8,9 +8,9 @@ "name": "ing1", "namespace": "ing1", "annotations": { - "zalando.org/skipper-filter": "status(10) -> inlineContent(\"This should work\")", + "zalando.org/skipper-filter": "status(200) -> inlineContent(\"This should work\")", "zalando.org/skipper-predicate": "Header(\"test\", \"test\") && Path(\"/login\")", - "zalando.org/skipper-routes": "r1: Header(\"test2\", \"test2\") && Path(\"/login\") -> status(200) -> \"shunt\"" + "zalando.org/skipper-routes": "r1: Header(\"test2\", \"test2\") && Path(\"/login\") -> status(200) -> \"http://foo.test\"" } }, "spec": { diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go index 11a8bf84aa..82a67b303f 100644 --- a/cmd/webhook/main.go +++ b/cmd/webhook/main.go @@ -12,6 +12,7 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" log "github.com/sirupsen/logrus" "github.com/zalando/skipper/cmd/webhook/admission" + "github.com/zalando/skipper/dataclients/kubernetes/definitions" ) const ( @@ -52,8 +53,8 @@ func main() { var cfg = &config{} cfg.parse() - rgAdmitter := admission.RouteGroupAdmitter{} - ingressAdmitter := admission.IngressAdmitter{} + rgAdmitter := &admission.RouteGroupAdmitter{RouteGroupValidator: &definitions.RouteGroupValidator{}} + ingressAdmitter := &admission.IngressAdmitter{IngressValidator: &definitions.IngressV1Validator{}} handler := http.NewServeMux() handler.Handle("/routegroups", admission.Handler(rgAdmitter)) handler.Handle("/ingresses", admission.Handler(ingressAdmitter)) diff --git a/dataclients/kubernetes/clusterclient.go b/dataclients/kubernetes/clusterclient.go index 1b16fa4a3b..f2dde0510d 100644 --- a/dataclients/kubernetes/clusterclient.go +++ b/dataclients/kubernetes/clusterclient.go @@ -358,10 +358,11 @@ func (c *clusterClient) LoadRouteGroups() ([]*definitions.RouteGroupItem, error) return nil, err } + routeGroupValidator := &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 := routeGroupValidator.Validate(i); err != nil { log.Errorf("[routegroup] %v", err) continue } diff --git a/dataclients/kubernetes/definitions/ingressv1.go b/dataclients/kubernetes/definitions/ingressv1.go index 64eda3c6cd..0bcc7b86ce 100644 --- a/dataclients/kubernetes/definitions/ingressv1.go +++ b/dataclients/kubernetes/definitions/ingressv1.go @@ -6,6 +6,12 @@ import ( "strconv" ) +const ( + SkipperfilterAnnotationKey = "zalando.org/skipper-filter" + SkipperpredicateAnnotationKey = "zalando.org/skipper-predicate" + SkipperRoutesAnnotationKey = "zalando.org/skipper-routes" +) + var errInvalidPortType = errors.New("invalid port type") type IngressV1List struct { diff --git a/dataclients/kubernetes/definitions/ingressvalidator.go b/dataclients/kubernetes/definitions/ingressvalidator.go index 8c74bf7f8f..c769bcc9f6 100644 --- a/dataclients/kubernetes/definitions/ingressvalidator.go +++ b/dataclients/kubernetes/definitions/ingressvalidator.go @@ -6,12 +6,6 @@ import ( "github.com/zalando/skipper/eskip" ) -const ( - skipperfilterAnnotationKey = "zalando.org/skipper-filter" - skipperpredicateAnnotationKey = "zalando.org/skipper-predicate" - skipperRoutesAnnotationKey = "zalando.org/skipper-routes" -) - type IngressV1Validator struct{} func (igv *IngressV1Validator) Validate(item *IngressV1Item) error { @@ -25,10 +19,10 @@ func (igv *IngressV1Validator) Validate(item *IngressV1Item) error { } func (igv *IngressV1Validator) validateFilterAnnotation(annotations map[string]string) error { - if filters, ok := annotations[skipperfilterAnnotationKey]; ok { + if filters, ok := annotations[SkipperfilterAnnotationKey]; ok { _, err := eskip.ParseFilters(filters) if err != nil { - err = fmt.Errorf("parsing \"%s\" annotation failed: %w", skipperfilterAnnotationKey, err) + err = fmt.Errorf("invalid \"%s\" annotation: %w", SkipperfilterAnnotationKey, err) } return err } @@ -36,10 +30,10 @@ func (igv *IngressV1Validator) validateFilterAnnotation(annotations map[string]s } func (igv *IngressV1Validator) validatePredicateAnnotation(annotations map[string]string) error { - if predicates, ok := annotations[skipperpredicateAnnotationKey]; ok { + if predicates, ok := annotations[SkipperpredicateAnnotationKey]; ok { _, err := eskip.ParsePredicates(predicates) if err != nil { - err = fmt.Errorf("parsing \"%s\" annotation failed: %w", skipperpredicateAnnotationKey, err) + err = fmt.Errorf("invalid \"%s\" annotation: %w", SkipperpredicateAnnotationKey, err) } return err } @@ -47,10 +41,10 @@ func (igv *IngressV1Validator) validatePredicateAnnotation(annotations map[strin } func (igv *IngressV1Validator) validateRoutesAnnotation(annotations map[string]string) error { - if routes, ok := annotations[skipperRoutesAnnotationKey]; ok { + if routes, ok := annotations[SkipperRoutesAnnotationKey]; ok { _, err := eskip.Parse(routes) if err != nil { - err = fmt.Errorf("parsing \"%s\" annotation failed: %w", skipperRoutesAnnotationKey, err) + err = fmt.Errorf("invalid \"%s\" annotation: %w", SkipperRoutesAnnotationKey, err) } return err } diff --git a/dataclients/kubernetes/ingress.go b/dataclients/kubernetes/ingress.go index 0a385de782..db081297a4 100644 --- a/dataclients/kubernetes/ingress.go +++ b/dataclients/kubernetes/ingress.go @@ -20,9 +20,6 @@ const ( ingressRouteIDPrefix = "kube" backendWeightsAnnotationKey = "zalando.org/backend-weights" ratelimitAnnotationKey = "zalando.org/ratelimit" - skipperfilterAnnotationKey = "zalando.org/skipper-filter" - skipperpredicateAnnotationKey = "zalando.org/skipper-predicate" - skipperRoutesAnnotationKey = "zalando.org/skipper-routes" skipperLoadBalancerAnnotationKey = "zalando.org/skipper-loadbalancer" skipperBackendProtocolAnnotationKey = "zalando.org/skipper-backend-protocol" pathModeAnnotationKey = "zalando.org/skipper-ingress-path-mode" @@ -229,7 +226,7 @@ func annotationFilter(m *definitions.Metadata, logger *logger) []*eskip.Filter { if ratelimitAnnotationValue, ok := m.Annotations[ratelimitAnnotationKey]; ok { annotationFilter = ratelimitAnnotationValue } - if val, ok := m.Annotations[skipperfilterAnnotationKey]; ok { + if val, ok := m.Annotations[definitions.SkipperfilterAnnotationKey]; ok { if annotationFilter != "" { annotationFilter += " -> " } @@ -249,7 +246,7 @@ func annotationFilter(m *definitions.Metadata, logger *logger) []*eskip.Filter { // parse predicate annotation func annotationPredicate(m *definitions.Metadata) string { var annotationPredicate string - if val, ok := m.Annotations[skipperpredicateAnnotationKey]; ok { + if val, ok := m.Annotations[definitions.SkipperpredicateAnnotationKey]; ok { annotationPredicate = val } return annotationPredicate @@ -258,12 +255,12 @@ func annotationPredicate(m *definitions.Metadata) string { // parse routes annotation func extraRoutes(m *definitions.Metadata, logger *logger) []*eskip.Route { var extraRoutes []*eskip.Route - annotationRoutes := m.Annotations[skipperRoutesAnnotationKey] + annotationRoutes := m.Annotations[definitions.SkipperRoutesAnnotationKey] if annotationRoutes != "" { var err error extraRoutes, err = eskip.Parse(annotationRoutes) if err != nil { - logger.Errorf("Failed to parse routes from %s, skipping: %v", skipperRoutesAnnotationKey, err) + logger.Errorf("Failed to parse routes from %s, skipping: %v", definitions.SkipperRoutesAnnotationKey, err) } } return extraRoutes diff --git a/dataclients/kubernetes/kube_test.go b/dataclients/kubernetes/kube_test.go index 28bcd23349..6d69b60935 100644 --- a/dataclients/kubernetes/kube_test.go +++ b/dataclients/kubernetes/kube_test.go @@ -201,13 +201,13 @@ func testIngress(ns, name, defaultService, ratelimitCfg, filterString, predicate setAnnotation(i, ratelimitAnnotationKey, ratelimitCfg) } if filterString != "" { - setAnnotation(i, skipperfilterAnnotationKey, filterString) + setAnnotation(i, definitions.SkipperfilterAnnotationKey, filterString) } if predicateString != "" { - setAnnotation(i, skipperpredicateAnnotationKey, predicateString) + setAnnotation(i, definitions.SkipperpredicateAnnotationKey, predicateString) } if routesString != "" { - setAnnotation(i, skipperRoutesAnnotationKey, routesString) + setAnnotation(i, definitions.SkipperRoutesAnnotationKey, routesString) } if pathModeString != "" { setAnnotation(i, pathModeAnnotationKey, pathModeString) diff --git a/dataclients/kubernetes/path_test.go b/dataclients/kubernetes/path_test.go index 85287d7264..e636257146 100644 --- a/dataclients/kubernetes/path_test.go +++ b/dataclients/kubernetes/path_test.go @@ -86,7 +86,7 @@ func TestPathMatchingModes(t *testing.T) { annotation := strings.Join(annotations, " && ") if len(annotations) > 0 { - i.Metadata.Annotations[skipperpredicateAnnotationKey] = annotation + i.Metadata.Annotations[definitions.SkipperpredicateAnnotationKey] = annotation } api.ingresses.Items = []*definitions.IngressV1Item{i} diff --git a/dataclients/kubernetes/redirect.go b/dataclients/kubernetes/redirect.go index d574477e20..94f21c2b46 100644 --- a/dataclients/kubernetes/redirect.go +++ b/dataclients/kubernetes/redirect.go @@ -37,7 +37,7 @@ func createRedirectInfo(defaultEnabled bool, defaultCode int) *redirectInfo { func (r *redirectInfo) initCurrent(m *definitions.Metadata) { r.enable = m.Annotations[redirectAnnotationKey] == "true" r.disable = m.Annotations[redirectAnnotationKey] == "false" - r.ignore = strings.Contains(m.Annotations[skipperpredicateAnnotationKey], `Header("X-Forwarded-Proto"`) || strings.Contains(m.Annotations[skipperRoutesAnnotationKey], `Header("X-Forwarded-Proto"`) + r.ignore = strings.Contains(m.Annotations[definitions.SkipperpredicateAnnotationKey], `Header("X-Forwarded-Proto"`) || strings.Contains(m.Annotations[definitions.SkipperRoutesAnnotationKey], `Header("X-Forwarded-Proto"`) r.code = r.defaultCode if annotationCode, ok := m.Annotations[redirectCodeAnnotationKey]; ok {