Skip to content

Commit

Permalink
webhook: validate skipper annotations in ingress (#2493)
Browse files Browse the repository at this point in the history 
Introduce skipper ingress annotations eskip validation to `AdmissionValidationWebhook` and our webhook binary

Signed-off-by: Mustafa Abdelrahman <[email protected]>
  • Loading branch information
@MustafaSaber
MustafaSaber authored Aug 10, 2023
1 parent 2e836aa commit 679e064
Show file tree
Hide file tree
Showing 25 changed files with 459 additions and 98 deletions.
42 changes: 0 additions & 42 deletions cmd/webhook/admission/admission.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/prometheus/client_golang/prometheus"
log "github.com/sirupsen/logrus"
"github.com/zalando/skipper/dataclients/kubernetes/definitions"
)

const (
Expand Down Expand Up @@ -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()
Expand Down
76 changes: 70 additions & 6 deletions cmd/webhook/admission/admission_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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) {
Expand All @@ -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)
Expand All @@ -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))
})
}
}
34 changes: 34 additions & 0 deletions cmd/webhook/admission/ingress.go
View file
Original file line number Diff line number Diff line change
@@ -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
}
34 changes: 34 additions & 0 deletions cmd/webhook/admission/routegroup.go
View file
Original file line number Diff line number Diff line change
@@ -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
}
40 changes: 40 additions & 0 deletions cmd/webhook/admission/testdata/ingress/invalid-filters-and-predicates.json
View file
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
}
]
}
}
}
}
39 changes: 39 additions & 0 deletions cmd/webhook/admission/testdata/ingress/invalid-filters.json
View file
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
}
]
}
}
}
}
39 changes: 39 additions & 0 deletions cmd/webhook/admission/testdata/ingress/invalid-predicates.json
View file
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
}
]
}
}
}
}
Loading

0 comments on commit 679e064

Please sign in to comment.