Skip to content

Commit

Permalink
feat: implement optional validation webhook. Fixes #817. (#832)
Browse files Browse the repository at this point in the history
Signed-off-by: Dillen Padhiar <[email protected]>
  • Loading branch information
dpadhiar committed Jul 13, 2023
1 parent c9f7e1e commit 609d8b3
Show file tree
Hide file tree
Showing 24 changed files with 1,334 additions and 2 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ manifests: crds
kubectl kustomize config/advanced-install/namespaced-numaflow-server > config/advanced-install/namespaced-numaflow-server.yaml
kubectl kustomize config/advanced-install/numaflow-server > config/advanced-install/numaflow-server.yaml
kubectl kustomize config/advanced-install/minimal-crds > config/advanced-install/minimal-crds.yaml
kubectl kustomize config/extensions/webhook > config/validating-webhook-install.yaml

$(GOPATH)/bin/golangci-lint:
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b `go env GOPATH`/bin v1.49.0
Expand Down
1 change: 1 addition & 0 deletions cmd/commands/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,5 @@ func init() {
rootCmd.AddCommand(NewDaemonServerCommand())
rootCmd.AddCommand(NewServerCommand())
rootCmd.AddCommand(NewServerInitCommand())
rootCmd.AddCommand(NewWebhookCommand())
}
34 changes: 34 additions & 0 deletions cmd/commands/webhook.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
Copyright 2022 The Numaproj Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package commands

import (
"github.com/spf13/cobra"

webhookcmd "github.com/numaproj/numaflow/webhook/cmd"
)

func NewWebhookCommand() *cobra.Command {
command := &cobra.Command{
Use: "webhook-service",
Short: "Start validating Numaflow webhook server",
Run: func(cmd *cobra.Command, args []string) {
webhookcmd.Start()
},
}
return command
}
14 changes: 14 additions & 0 deletions config/extensions/webhook/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- numaflow-webhook-sa.yaml
- rbac
- numaflow-webhook-deployment.yaml
- numaflow-webhook-service.yaml

namespace: numaflow-system

images:
- name: quay.io/numaproj/numaflow
newTag: latest
36 changes: 36 additions & 0 deletions config/extensions/webhook/numaflow-webhook-deployment.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
apiVersion: apps/v1
kind: Deployment
metadata:
name: numaflow-webhook
spec:
replicas: 1
selector:
matchLabels:
app.kubernetes.io/part-of: numaflow
app.kubernetes.io/component: numaflow-webhook
template:
metadata:
labels:
app.kubernetes.io/part-of: numaflow
app.kubernetes.io/component: numaflow-webhook
spec:
containers:
- name: webhook
image: quay.io/numaproj/numaflow:latest
imagePullPolicy: Always
args:
- webhook-service
env:
- name: NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: PORT
value: "443"
- name: DEPLOYMENT_NAME
value: numaflow-webhook
- name: SERVICE_NAME
value: numaflow-webhook
- name: CLUSTER_ROLE_NAME
value: numaflow-webhook
serviceAccountName: numaflow-webhook-sa
4 changes: 4 additions & 0 deletions config/extensions/webhook/numaflow-webhook-sa.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
apiVersion: v1
kind: ServiceAccount
metadata:
name: numaflow-webhook-sa
11 changes: 11 additions & 0 deletions config/extensions/webhook/numaflow-webhook-service.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
apiVersion: v1
kind: Service
metadata:
name: numaflow-webhook
spec:
ports:
- port: 443
targetPort: 443
selector:
app.kubernetes.io/part-of: numaflow
app.kubernetes.io/component: numaflow-webhook
6 changes: 6 additions & 0 deletions config/extensions/webhook/rbac/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

resources:
- numaflow-webhook-cluster-role.yaml
- numaflow-webhook-cluster-role-binding.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: numaflow-webhook-binding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: numaflow-webhook
subjects:
- kind: ServiceAccount
name: numaflow-webhook-sa
namespace: numaflow-system
60 changes: 60 additions & 0 deletions config/extensions/webhook/rbac/numaflow-webhook-cluster-role.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: numaflow-webhook
rules:
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- create
- update
- delete
- patch
- watch
- apiGroups:
- ""
resources:
- configmaps
verbs:
- get
- list
- watch
- apiGroups:
- apps
resources:
- deployments
verbs:
- get
- list
- apiGroups:
- admissionregistration.k8s.io
resources:
- validatingwebhookconfigurations
verbs:
- get
- list
- create
- update
- delete
- patch
- watch
- apiGroups:
- numaproj.io
verbs:
- get
- list
- watch
resources:
- interstepbufferservices
- pipelines
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterroles
verbs:
- get
- list
130 changes: 130 additions & 0 deletions config/validating-webhook-install.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
apiVersion: v1
kind: ServiceAccount
metadata:
name: numaflow-webhook-sa
namespace: numaflow-system
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
name: numaflow-webhook
rules:
- apiGroups:
- ""
resources:
- secrets
verbs:
- get
- list
- create
- update
- delete
- patch
- watch
- apiGroups:
- ""
resources:
- configmaps
verbs:
- get
- list
- watch
- apiGroups:
- apps
resources:
- deployments
verbs:
- get
- list
- apiGroups:
- admissionregistration.k8s.io
resources:
- validatingwebhookconfigurations
verbs:
- get
- list
- create
- update
- delete
- patch
- watch
- apiGroups:
- numaproj.io
resources:
- interstepbufferservices
- pipelines
verbs:
- get
- list
- watch
- apiGroups:
- rbac.authorization.k8s.io
resources:
- clusterroles
verbs:
- get
- list
---
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
name: numaflow-webhook-binding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
name: numaflow-webhook
subjects:
- kind: ServiceAccount
name: numaflow-webhook-sa
namespace: numaflow-system
---
apiVersion: v1
kind: Service
metadata:
name: numaflow-webhook
namespace: numaflow-system
spec:
ports:
- port: 443
targetPort: 443
selector:
app.kubernetes.io/component: numaflow-webhook
app.kubernetes.io/part-of: numaflow
---
apiVersion: apps/v1
kind: Deployment
metadata:
name: numaflow-webhook
namespace: numaflow-system
spec:
replicas: 1
selector:
matchLabels:
app.kubernetes.io/component: numaflow-webhook
app.kubernetes.io/part-of: numaflow
template:
metadata:
labels:
app.kubernetes.io/component: numaflow-webhook
app.kubernetes.io/part-of: numaflow
spec:
containers:
- args:
- webhook-service
env:
- name: NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: PORT
value: "443"
- name: DEPLOYMENT_NAME
value: numaflow-webhook
- name: SERVICE_NAME
value: numaflow-webhook
- name: CLUSTER_ROLE_NAME
value: numaflow-webhook
image: quay.io/numaproj/numaflow:latest
imagePullPolicy: Always
name: webhook
serviceAccountName: numaflow-webhook-sa
48 changes: 48 additions & 0 deletions docs/operations/validating-webhook.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Validating Admission Webhook

This validating webhook will prevent disallowed spec changes to immutable fields of Numaflow CRDs including Pipelines and InterStepBufferServices.
It also prevents creating a CRD with a faulty spec.
The user sees an error immediately returned by the server explaining why the request was denied.

## Installation

To install the validating webhook, run the following command line:

```shell
kubectl apply -n numaflow-system -f https://raw.githubusercontent.com/numaproj/numaflow/stable/config/validating-webhook-install.yaml
```

## Examples

Currently, the validating webhook prevents updating the type of an InterStepBufferService from JetStream to Redis for example.

Example spec:
```yaml
apiVersion: numaflow.numaproj.io/v1alpha1
kind: InterStepBufferService
metadata:
name: default
spec:
jetstream: // change to redis and reapply will cause below error
version: latest
```
```shell
Error from server (BadRequest): error when applying patch:
{"metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"numaflow.numaproj.io/v1alpha1\",\"kind\":\"InterStepBufferService\",\"metadata\":{\"annotations\":{},\"name\":\"default\",\"namespace\":\"numaflow-system\"},\"spec\":{\"redis\":{\"native\":{\"version\":\"7.0.11\"}}}}\n"}},"spec":{"jetstream":null,"redis":{"native":{"version":"7.0.11"}}}}
to:
Resource: "numaflow.numaproj.io/v1alpha1, Resource=interstepbufferservices", GroupVersionKind: "numaflow.numaproj.io/v1alpha1, Kind=InterStepBufferService"
Name: "default", Namespace: "numaflow-system"
for: "redis.yaml": error when patching "redis.yaml": admission webhook "webhook.numaflow.numaproj.io" denied the request: Can not change ISB Service type from Jetstream to Redis
```
There is also validation that prevents the `interStepBufferServiceName` of a Pipeline from being updated.

```shell
Error from server (BadRequest): error when applying patch:
{"metadata":{"annotations":{"kubectl.kubernetes.io/last-applied-configuration":"{\"apiVersion\":\"numaflow.numaproj.io/v1alpha1\",\"kind\":\"Pipeline\",\"metadata\":{\"annotations\":{},\"name\":\"simple-pipeline\",\"namespace\":\"numaflow-system\"},\"spec\":{\"edges\":[{\"from\":\"in\",\"to\":\"cat\"},{\"from\":\"cat\",\"to\":\"out\"}],\"interStepBufferServiceName\":\"change\",\"vertices\":[{\"name\":\"in\",\"source\":{\"generator\":{\"duration\":\"1s\",\"rpu\":5}}},{\"name\":\"cat\",\"udf\":{\"builtin\":{\"name\":\"cat\"}}},{\"name\":\"out\",\"sink\":{\"log\":{}}}]}}\n"}},"spec":{"interStepBufferServiceName":"change","vertices":[{"name":"in","source":{"generator":{"duration":"1s","rpu":5}}},{"name":"cat","udf":{"builtin":{"name":"cat"}}},{"name":"out","sink":{"log":{}}}]}}
to:
Resource: "numaflow.numaproj.io/v1alpha1, Resource=pipelines", GroupVersionKind: "numaflow.numaproj.io/v1alpha1, Kind=Pipeline"
Name: "simple-pipeline", Namespace: "numaflow-system"
for: "examples/1-simple-pipeline.yaml": error when patching "examples/1-simple-pipeline.yaml": admission webhook "webhook.numaflow.numaproj.io" denied the request: Cannot update pipeline with different interStepBufferServiceName
```
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ require (
github.com/gavv/httpexpect/v2 v2.3.1
github.com/gin-contrib/static v0.0.2-0.20220606235426-ae09b2ea7e39
github.com/gin-gonic/gin v1.9.1
github.com/go-openapi/inflect v0.19.0
github.com/go-swagger/go-swagger v0.28.0
github.com/goccy/go-json v0.10.2
github.com/gogo/protobuf v1.3.2
Expand Down Expand Up @@ -84,7 +85,6 @@ require (
github.com/go-logr/logr v1.2.3 // indirect
github.com/go-openapi/analysis v0.20.1 // indirect
github.com/go-openapi/errors v0.20.1 // indirect
github.com/go-openapi/inflect v0.19.0 // indirect
github.com/go-openapi/jsonpointer v0.19.5 // indirect
github.com/go-openapi/jsonreference v0.19.6 // indirect
github.com/go-openapi/loads v0.20.3 // indirect
Expand Down
Loading

0 comments on commit 609d8b3

Please sign in to comment.