webhook: validate skipper annotations in ingress

cmd/webhook/admission/admission.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/prometheus/client_golang/prometheus"
 	log "github.com/sirupsen/logrus"
-	"github.com/zalando/skipper/dataclients/kubernetes/definitions"
 )
 
 const (
@@ -58,51 +57,10 @@ type admitter interface {
 	admit(req *admissionRequest) (*admissionResponse, error)
 }
 
-type RouteGroupAdmitter struct {
-}
-
 func init() {
 	prometheus.MustRegister(totalRequests, invalidRequests, rejectedRequests, approvedRequests, admissionDuration)
 }
 
-func (RouteGroupAdmitter) name() string {
-	return "routegroup"
-}
-
-func (RouteGroupAdmitter) admit(req *admissionRequest) (*admissionResponse, error) {
-	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,
-			},
-		}, nil
-	}
-
-	err = definitions.ValidateRouteGroup(&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,
-			},
-		}, nil
-	}
-
-	return &admissionResponse{
-		UID:     req.UID,
-		Allowed: true,
-	}, nil
-}
-
 func Handler(admitter admitter) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		admitterName := admitter.name()

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) {
@@ -128,14 +128,14 @@ func TestRouteGroupAdmitter(t *testing.T) {
 				expectedResponse = fmt.Sprintf(responseNotAllowedFmt, tc.message)
 			}
 
-			input, err := os.ReadFile("testdata/" + tc.inputFile)
+			input, err := os.ReadFile("testdata/rg/" + tc.inputFile)
 			require.NoError(t, err)
 
 			req := httptest.NewRequest("POST", "http://example.com/foo", bytes.NewBuffer(input))
 			req.Header.Set("Content-Type", "application/json")
 
 			w := httptest.NewRecorder()
-			rgAdm := RouteGroupAdmitter{}
+			rgAdm := &RouteGroupAdmitter{}
 
 			h := Handler(rgAdm)
 			h(w, req)
@@ -150,3 +150,67 @@ func TestRouteGroupAdmitter(t *testing.T) {
 		})
 	}
 }
+
+func TestIngressAdmitter(t *testing.T) {
+	for _, tc := range []struct {
+		name      string
+		inputFile string
+		message   string
+	}{
+		{
+			name:      "allowed without annotations",
+			inputFile: "valid-ingress-without-annotations.json",
+		},
+		{
+			name:      "allowed with valid annotations",
+			inputFile: "valid-ingress-with-annotations.json",
+		},
+		{
+			name:      "invalid eskip filters",
+			inputFile: "invalid-filters.json",
+			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:   `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:   `invalid \"zalando.org/skipper-routes\" annotation: invalid predicate count arg`,
+		},
+		{
+			name:      "invalid eskip filters and predicates",
+			inputFile: "invalid-filters-and-predicates.json",
+			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) {
+			expectedResponse := responseAllowedFmt
+			if len(tc.message) > 0 {
+				expectedResponse = fmt.Sprintf(responseNotAllowedFmt, tc.message)
+			}
+
+			input, err := os.ReadFile("testdata/ingress/" + tc.inputFile)
+			require.NoError(t, err)
+
+			req := httptest.NewRequest("POST", "http://example.com/foo", bytes.NewBuffer(input))
+			req.Header.Set("Content-Type", "application/json")
+
+			w := httptest.NewRecorder()
+			ingressAdm := &IngressAdmitter{}
+
+			h := Handler(ingressAdm)
+			h(w, req)
+			resp := w.Result()
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+
+			rb, err := io.ReadAll(resp.Body)
+			require.NoError(t, err)
+			resp.Body.Close()
+
+			assert.JSONEq(t, expectedResponse, string(rb))
+		})
+	}
+}

cmd/webhook/admission/ingress.go

@@ -0,0 +1,34 @@
+package admission
+
+import (
+	"encoding/json"
+
+	"github.com/zalando/skipper/dataclients/kubernetes/definitions"
+)
+
+type IngressAdmitter struct {
+	IngressValidator *definitions.IngressV1Validator
+}
+
+func (iga *IngressAdmitter) name() string {
+	return "ingress"
+}
+
+func (iga *IngressAdmitter) admit(req *admissionRequest) (*admissionResponse, error) {
+
+	ingressItem := definitions.IngressV1Item{}
+	err := json.Unmarshal(req.Object, &ingressItem)
+	if err != nil {
+		return nil, err
+	}
+
+	err = iga.IngressValidator.Validate(&ingressItem)
+	if err != nil {
+		return nil, err
+	}
+
+	return &admissionResponse{
+		UID:     req.UID,
+		Allowed: true,
+	}, nil
+}

cmd/webhook/admission/routegroup.go

@@ -0,0 +1,34 @@
+package admission
+
+import (
+	"encoding/json"
+
+	"github.com/zalando/skipper/dataclients/kubernetes/definitions"
+)
+
+type RouteGroupAdmitter struct {
+	RouteGroupValidator *definitions.RouteGroupValidator
+}
+
+func (rga *RouteGroupAdmitter) name() string {
+	return "routegroup"
+}
+
+func (rga *RouteGroupAdmitter) admit(req *admissionRequest) (*admissionResponse, error) {
+
+	rgItem := definitions.RouteGroupItem{}
+	err := json.Unmarshal(req.Object, &rgItem)
+	if err != nil {
+		return nil, err
+	}
+
+	err = rga.RouteGroupValidator.Validate(&rgItem)
+	if err != nil {
+		return nil, err
+	}
+
+	return &admissionResponse{
+		UID:     req.UID,
+		Allowed: true,
+	}, nil
+}

cmd/webhook/admission/testdata/ingress/invalid-filters-and-predicates.json

@@ -0,0 +1,40 @@
+{
+    "request": {
+        "uid": "req-uid",
+        "name": "req1",
+        "namespace": "n1",
+        "object": {
+            "metadata": {
+                "name": "ing1",
+                "namespace": "ing1",
+                "annotations": {
+                    "zalando.org/skipper-filter": "this-is-invalid(10) -> inlineContent(\"This should work\")",
+                    "zalando.org/skipper-predicate": "Header(\"test\") & Path(\"/login\")"
+                }
+            },
+            "spec": {
+                "rules": [
+                    {
+                        "host": "example.com",
+                        "http": {
+                            "paths": [
+                                {
+                                    "backend": {
+                                        "service": {
+                                            "name": "example-service",
+                                            "port": {
+                                                "number": 80
+                                            }
+                                        }
+                                    },
+                                    "path": "/",
+                                    "pathType": "Prefix"
+                                }
+                            ]
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}

cmd/webhook/admission/testdata/ingress/invalid-filters.json

@@ -0,0 +1,39 @@
+{
+    "request": {
+        "uid": "req-uid",
+        "name": "req1",
+        "namespace": "n1",
+        "object": {
+            "metadata": {
+                "name": "ing1",
+                "namespace": "ing1",
+                "annotations": {
+                    "zalando.org/skipper-filter": "this-is-invalid(10) -> inlineContent(\"This should't work\")"
+                }
+            },
+            "spec": {
+                "rules": [
+                    {
+                        "host": "example.com",
+                        "http": {
+                            "paths": [
+                                {
+                                    "backend": {
+                                        "service": {
+                                            "name": "example-service",
+                                            "port": {
+                                                "number": 80
+                                            }
+                                        }
+                                    },
+                                    "path": "/",
+                                    "pathType": "Prefix"
+                                }
+                            ]
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}

cmd/webhook/admission/testdata/ingress/invalid-predicates.json

@@ -0,0 +1,39 @@
+{
+    "request": {
+        "uid": "req-uid",
+        "name": "req1",
+        "namespace": "n1",
+        "object": {
+            "metadata": {
+                "name": "ing1",
+                "namespace": "ing1",
+                "annotations": {
+                    "zalando.org/skipper-predicate": "Header(\"test\") & Path(\"/login\")"
+                }
+            },
+            "spec": {
+                "rules": [
+                    {
+                        "host": "example.com",
+                        "http": {
+                            "paths": [
+                                {
+                                    "backend": {
+                                        "service": {
+                                            "name": "example-service",
+                                            "port": {
+                                                "number": 80
+                                            }
+                                        }
+                                    },
+                                    "path": "/",
+                                    "pathType": "Prefix"
+                                }
+                            ]
+                        }
+                    }
+                ]
+            }
+        }
+    }
+}