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

webhook: support validation rules #2885

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AlexanderYastrebov
Copy link
Member

Add new webhook admitter that evaluates Ingresses and RouteGroups against a set of rules.

Each rule defines properties of matching resource and rejection message.

The implementation uses Common Expression Language to match properties which is also used in Kubernetes.

@AlexanderYastrebov AlexanderYastrebov added the wip work in progress label Jan 24, 2024
Comment on lines 2 to 8
- reject: Missing application label, see https://example.test/reference/labels-selectors/#application
when: |
request.kind.kind in ["Ingress", "RouteGroup"] && (
!has(request.object.metadata.labels) ||
!has(request.object.metadata.labels.application) ||
request.object.metadata.labels.application == ''
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may also implement

rules:
  - warning: Feature X will be deprecated
    when:  request.kind.kind == "RouteGroup" && ...

that would allow request with warnings, see 9281783

@szuecs
Copy link
Member

szuecs commented Jan 25, 2024

I would assume that you would share pros and cons and why it's better than what we have today.
Not being able to parse eskip sounds like a step backwards.

@AlexanderYastrebov
Copy link
Member Author

AlexanderYastrebov commented Jan 29, 2024

I would assume that you would share pros and cons

This approach allows skipper operator to define custom rules that may be specific to a given setup or contain sensitive bits.
E.g. a custom rule may reject certain filter or predicate configurations or deny backend addresses.
The downside is usage of another language (although having very simple syntax) and a need to have end-to-end test suite to test custom rules.

Alternative is to deploy closed-source webhook written in go.

Not being able to parse eskip sounds like a step backwards.

Its possible, see TODO comment #2885 (comment), I decided not to invest into it until we decide to move forward with this (or abandon the idea).

@AlexanderYastrebov AlexanderYastrebov force-pushed the webhook/validation-rules branch 2 times, most recently from 90dbd9d to 99e05b3 Compare March 1, 2024 12:58
@AlexanderYastrebov AlexanderYastrebov added the major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs label Mar 1, 2024
@AlexanderYastrebov AlexanderYastrebov force-pushed the webhook/validation-rules branch 5 times, most recently from 9c94c55 to b2a5214 Compare March 1, 2024 17:30
Add new webhook admitter that evaluates Ingresses and RouteGroups
against a set of rules.

Each rule defines properties of matching resource and rejection message.

The implementation uses [Common Expression Language](https://github.com/google/cel-spec)
to match properties which is also used in [Kubernetes](https://kubernetes.io/docs/reference/using-api/cel/).

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major moderate risk, for example new API, small filter changes that have no risk like refactoring or logs wip work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants