diff --git a/cmd/webhook/admission/admission_test.go b/cmd/webhook/admission/admission_test.go index a2ad4b26da..9563c2a7d3 100644 --- a/cmd/webhook/admission/admission_test.go +++ b/cmd/webhook/admission/admission_test.go @@ -133,6 +133,11 @@ func TestRouteGroupAdmitter(t *testing.T) { inputFile: "rg-with-multiple-predicates.json", message: `single predicate expected at \"Method(\\\"GET\\\") && Path(\\\"/\\\")\"\nsingle predicate expected at \" \"`, }, + { + name: "routegroup with invalid backends", + inputFile: "rg-with-invalid-backend-path.json", + message: `backend address \"http://example.com/foo\" contains path, query or missing scheme\nbackend address \"http://example.com/foo/bar\" contains path, query or missing scheme\nbackend address \"http://example.com/foo/\" contains path, query or missing scheme\nbackend address \"/foo\" contains path, query or missing scheme\nbackend address \"http://example.com/\" contains path, query or missing scheme\nbackend address \"example.com/\" contains path, query or missing scheme\nbackend address \"example.com/foo\" contains path, query or missing scheme\nbackend address \"http://example.com?foo=bar\" contains path, query or missing scheme\nbackend address \"example.com\" contains path, query or missing scheme`, + }, } { t.Run(tc.name, func(t *testing.T) { expectedResponse := responseAllowedFmt diff --git a/cmd/webhook/admission/testdata/rg/rg-with-invalid-backend-path.json b/cmd/webhook/admission/testdata/rg/rg-with-invalid-backend-path.json new file mode 100644 index 0000000000..d604b97244 --- /dev/null +++ b/cmd/webhook/admission/testdata/rg/rg-with-invalid-backend-path.json @@ -0,0 +1,103 @@ +{ + "request": { + "uid": "req-uid", + "name": "req1", + "operation": "create", + "kind": { + "group": "zalando", + "version": "v1", + "kind": "RouteGroup" + }, + "namespace": "n1", + "object": { + "metadata": { + "name": "rg1", + "namespace": "n1" + }, + "spec": { + "backends": [ + { + "name": "valid-backend1", + "type": "shunt" + }, + { + "name": "valid-backend3", + "type": "network", + "address": "http://example.com" + }, + { + "name": "invalid-backend2", + "type": "network", + "address": "http://example.com/foo" + }, + { + "name": "invalid-backend3", + "type": "network", + "address": "http://example.com/foo/bar" + }, + { + "name": "invalid-backend4", + "type": "network", + "address": "http://example.com/foo/" + }, + { + "name": "invalid-backend5", + "type": "network", + "address": "/foo" + }, + { + "name": "valid-backend2", + "type": "network", + "address": "http://user:pass@example.com" + }, + { + "name": "invalid-backend6", + "type": "network", + "address": "http://example.com/" + }, + { + "name": "invalid-backend7", + "type": "network", + "address": "example.com/" + }, + { + "name" : "invalid-backend8", + "type" : "network", + "address" : "example.com/foo" + }, + { + "name" : "invalid-backend9", + "type" : "network", + "address" : "http://example.com?foo=bar" + }, + { + "name": "invalid-backend10", + "type": "network", + "address": "example.com" + } + ], + "defaultBackends": [ + { + "backendName": "valid-backend1" + } + ], + "routes": [ + { + "backends": [ + { + "backendName": "valid-backend1" + } + ], + "filters": [ + "status(201)" + ], + "path": "/", + "predicates": [ + "Method(\"GET\")" + ] + } + ] + } + } + } +} diff --git a/dataclients/kubernetes/definitions/routegroupvalidator.go b/dataclients/kubernetes/definitions/routegroupvalidator.go index 4911f7910c..21fa31427b 100644 --- a/dataclients/kubernetes/definitions/routegroupvalidator.go +++ b/dataclients/kubernetes/definitions/routegroupvalidator.go @@ -3,6 +3,7 @@ package definitions import ( "errors" "fmt" + "net/url" "github.com/zalando/skipper/eskip" ) @@ -35,8 +36,9 @@ func (rgv *RouteGroupValidator) Validate(item *RouteGroupItem) error { return err } var errs []error - errs = append(errs, rgv.filtersValidation(item)) - errs = append(errs, rgv.predicatesValidation(item)) + errs = append(errs, rgv.validateFilters(item)) + errs = append(errs, rgv.validatePredicates(item)) + errs = append(errs, rgv.validateBackends(item)) return errorsJoin(errs...) } @@ -60,7 +62,7 @@ func (rgv *RouteGroupValidator) basicValidation(r *RouteGroupItem) error { return nil } -func (rgv *RouteGroupValidator) filtersValidation(item *RouteGroupItem) error { +func (rgv *RouteGroupValidator) validateFilters(item *RouteGroupItem) error { var errs []error for _, r := range item.Spec.Routes { for _, f := range r.Filters { @@ -76,7 +78,7 @@ func (rgv *RouteGroupValidator) filtersValidation(item *RouteGroupItem) error { return errorsJoin(errs...) } -func (rgv *RouteGroupValidator) predicatesValidation(item *RouteGroupItem) error { +func (rgv *RouteGroupValidator) validatePredicates(item *RouteGroupItem) error { var errs []error for _, r := range item.Spec.Routes { for _, p := range r.Predicates { @@ -91,6 +93,21 @@ func (rgv *RouteGroupValidator) predicatesValidation(item *RouteGroupItem) error return errorsJoin(errs...) } +func (rgv *RouteGroupValidator) validateBackends(item *RouteGroupItem) error { + var errs []error + for _, backend := range item.Spec.Backends { + if backend.Type == eskip.NetworkBackend { + address, err := url.Parse(backend.Address) + if err != nil { + errs = append(errs, fmt.Errorf("failed to parse backend address %q: %w", backend.Address, err)) + } else if address.Path != "" || address.RawQuery != "" || address.Scheme == "" { + errs = append(errs, fmt.Errorf("backend address %q contains path, query or missing scheme", backend.Address)) + } + } + } + return errorsJoin(errs...) +} + // TODO: we need to pass namespace/name in all errors func (rg *RouteGroupSpec) validate() error { // has at least one backend: diff --git a/dataclients/kubernetes/definitions/testdata/errorwrapdata/errors.log b/dataclients/kubernetes/definitions/testdata/errorwrapdata/errors.log index 836138e99f..b5c5856864 100644 --- a/dataclients/kubernetes/definitions/testdata/errorwrapdata/errors.log +++ b/dataclients/kubernetes/definitions/testdata/errorwrapdata/errors.log @@ -4,3 +4,10 @@ single predicate expected at "Path(\"/foo\") && Method(\"GET\")" single predicate expected at "" single filter expected at "inlineContent(\"/foo\") -> status(200)" single filter expected at " " +backend address "http://example.com/foo" contains path, query or missing scheme +backend address "http://example.com/foo/bar" contains path, query or missing scheme +backend address "http://example.com/foo/" contains path, query or missing scheme +backend address "/foo" contains path, query or missing scheme +backend address "example.com/" contains path, query or missing scheme +backend address "http://example.com?foo=bar" contains path, query or missing scheme +backend address "example.com" contains path, query or missing scheme diff --git a/dataclients/kubernetes/definitions/testdata/errorwrapdata/routegroups.json b/dataclients/kubernetes/definitions/testdata/errorwrapdata/routegroups.json index 270d491ec9..c512a644bc 100644 --- a/dataclients/kubernetes/definitions/testdata/errorwrapdata/routegroups.json +++ b/dataclients/kubernetes/definitions/testdata/errorwrapdata/routegroups.json @@ -134,6 +134,82 @@ } ] } + }, + { + "apiVersion": "zalando.org/v1", + "metadata": { + "name": "rg1" + }, + "kind": "RouteGroup", + "spec": { + "backends": [ + { + "name": "backend1", + "type": "network", + "address": "http://example.com" + }, + { + "name": "backend2", + "type": "network", + "address": "http://example.com/foo" + }, + { + "name": "backend3", + "type": "network", + "address": "http://example.com/foo/bar" + }, + { + "name": "backend4", + "type": "network", + "address": "http://example.com/foo/" + }, + { + "name": "backend5", + "type": "network", + "address": "/foo" + }, + { + "name": "backend6", + "type": "network", + "address": "example.com/" + }, + { + "name": "backend7", + "type": "network", + "address": "http://user:pass@example.com" + }, + { + "name": "backend8", + "type": "network", + "address": "http://example.com?foo=bar" + }, + { + "name": "backend9", + "type": "network", + "address": "example.com" + }, + { + "name": "shunt", + "type": "shunt" + } + ], + "hosts": [ + "test.example.com" + ], + "routes": [ + { + "backends": [ + { + "backendName": "shunt" + } + ], + "filters": [ + "inlineContent(\"/foo\")" + ], + "path": "/foo" + } + ] + } } ] } diff --git a/dataclients/kubernetes/definitions/testdata/validation/route-with-invalid-backend-with-path.log b/dataclients/kubernetes/definitions/testdata/validation/route-with-invalid-backend-with-path.log new file mode 100644 index 0000000000..c6d33f774f --- /dev/null +++ b/dataclients/kubernetes/definitions/testdata/validation/route-with-invalid-backend-with-path.log @@ -0,0 +1,8 @@ +backend address \\\"http://example\.com/foo\\\" contains path, query or missing scheme +backend address \\\"http://example\.com/foo/bar\\\" contains path, query or missing scheme +backend address \\\"/foo\\\" contains path, query or missing scheme +backend address \\\"/foo/bar\\\" contains path, query or missing scheme +backend address \\\"example\.com/foo\\\" contains path, query or missing scheme +backend address \\\"http://example\.com/\\\" contains path, query or missing scheme +backend address \\\"http://example\.com\?foo=bar\\\" contains path, query or missing scheme +backend address \\\"example\.com\\\" contains path, query or missing scheme diff --git a/dataclients/kubernetes/definitions/testdata/validation/route-with-invalid-backend-with-path.yaml b/dataclients/kubernetes/definitions/testdata/validation/route-with-invalid-backend-with-path.yaml new file mode 100644 index 0000000000..dc7b64a935 --- /dev/null +++ b/dataclients/kubernetes/definitions/testdata/validation/route-with-invalid-backend-with-path.yaml @@ -0,0 +1,56 @@ +apiVersion: zalando.org/v1 +kind: RouteGroup +metadata: + name: test-route-group +spec: + hosts: + - example.org + backends: + - name: app + type: service + serviceName: app-svc + servicePort: 80 + - name: backend1 + type: network + address: http://example.com + - name: backend2 + type: network + address: http://example.com/foo + - name: backend3 + type: network + address: http://example.com/foo/bar + - name: backend4 + type: network + address: /foo + - name: backend5 + type: network + address: /foo/bar + - name: backend6 + type: network + address: example.com/foo + - name: backend7 + type: network + address: http://example.com/ + - name: backend8 + type: network + address: http://user:pass@example.com + - name: backend9 + type: network + address: http://example.com?foo=bar + - name: backend10 + type: network + address: example.com + defaultBackends: + - backendName: app + routes: + - path: / + methods: + - GET + - HEAD + predicates: + - Foo("X-Bar", "42") + filters: + - foo(42) + - bar(24) + backends: + - backendName: app diff --git a/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-filters-one-item.log b/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-filters-one-item.log index 6300163089..1b78680a21 100644 --- a/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-filters-one-item.log +++ b/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-filters-one-item.log @@ -1 +1 @@ -single filter expected +single filter expected at \\\"foo\(42\) -> bar\(24\)\\\" diff --git a/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-predicates-one-item.log b/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-predicates-one-item.log index 6ec8ec4cbd..ee42e11293 100644 --- a/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-predicates-one-item.log +++ b/dataclients/kubernetes/definitions/testdata/validation/route-with-multiple-predicates-one-item.log @@ -1 +1 @@ -single predicate expected +single predicate expected at \\\"Foo\(\\\\\\"X-Bar\\\\\\", \\\\\\"42\\\\\\"\) && Bar\(\\\\\\"X-Foo\\\\\\", \\\\\\"24\\\\\\"\)\\\"