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

feat(ws): add validation webhooks for Workspace and WorkspaceKind #34

Merged
merged 8 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion workspaces/controller/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ endef

define prompt_for_e2e_test_execution
if [ "$$(echo "$(KUBEFLOW_TEST_PROMPT)" | tr '[:upper:]' '[:lower:]')" = "false" ]; then \
echo "Skipping E2E test confirmation prompt (KUBEFLOW_TEST_PROMPT is set to true)"; \
echo "Skipping E2E test confirmation prompt (KUBEFLOW_TEST_PROMPT is set to false)"; \
else \
current_k8s_context=$$(kubectl config current-context); \
echo "================================ WARNING ================================"; \
Expand Down
6 changes: 6 additions & 0 deletions workspaces/controller/PROJECT
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@ resources:
kind: Workspace
path: github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1
version: v1beta1
webhooks:
validation: true
webhookVersion: v1
- api:
crdVersion: v1
controller: true
domain: kubeflow.org
kind: WorkspaceKind
path: github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1
version: v1beta1
webhooks:
validation: true
webhookVersion: v1
version: "3"
38 changes: 34 additions & 4 deletions workspaces/controller/cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook"

kubefloworgv1beta1 "github.com/kubeflow/notebooks/workspaces/controller/api/v1beta1"
"github.com/kubeflow/notebooks/workspaces/controller/internal/controller"
controllerInternal "github.com/kubeflow/notebooks/workspaces/controller/internal/controller"
"github.com/kubeflow/notebooks/workspaces/controller/internal/helper"
webhookInternal "github.com/kubeflow/notebooks/workspaces/controller/internal/webhook"
//+kubebuilder:scaffold:imports
)

Expand Down Expand Up @@ -115,21 +117,30 @@ func main() {
// the manager stops, so would be fine to enable this option. However,
// if you are doing or is intended to do any operation such as perform cleanups
// after the manager stops then its usage might be unsafe.
// LeaderElectionReleaseOnCancel: true,
//
// TODO: check if we are doing anything which would prevent us from using this option.
//LeaderElectionReleaseOnCancel: true,
})
if err != nil {
setupLog.Error(err, "unable to start manager")
os.Exit(1)
}

if err = (&controller.WorkspaceReconciler{
// setup field indexers on the manager cache. we use these indexes to efficiently
// query the cache for things like which Workspaces are using a particular WorkspaceKind
if err := helper.SetupManagerFieldIndexers(mgr); err != nil {
setupLog.Error(err, "unable to setup field indexers")
os.Exit(1)
}

if err = (&controllerInternal.WorkspaceReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
}).SetupWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create controller", "controller", "Workspace")
os.Exit(1)
}
if err = (&controller.WorkspaceKindReconciler{
if err = (&controllerInternal.WorkspaceKindReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
}).SetupWithManager(mgr); err != nil {
Expand All @@ -138,6 +149,25 @@ func main() {
}
//+kubebuilder:scaffold:builder

if os.Getenv("ENABLE_WEBHOOKS") != "false" {
if err = (&webhookInternal.WorkspaceValidator{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
}).SetupWebhookWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create webhook", "webhook", "Workspace")
os.Exit(1)
}
}
if os.Getenv("ENABLE_WEBHOOKS") != "false" {
if err = (&webhookInternal.WorkspaceKindValidator{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
}).SetupWebhookWithManager(mgr); err != nil {
setupLog.Error(err, "unable to create webhook", "webhook", "WorkspaceKind")
os.Exit(1)
}
}

if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {
setupLog.Error(err, "unable to set up health check")
os.Exit(1)
Expand Down
39 changes: 39 additions & 0 deletions workspaces/controller/config/certmanager/certificate.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# The following manifests contain a self-signed issuer CR and a certificate CR.
# More document can be found at https://docs.cert-manager.io
# WARNING: Targets CertManager v1.0. Check https://cert-manager.io/docs/installation/upgrading/ for breaking changes.
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
labels:
app.kubernetes.io/name: certificate
app.kubernetes.io/instance: serving-cert
app.kubernetes.io/component: certificate
app.kubernetes.io/created-by: workspace-controller
app.kubernetes.io/part-of: workspace-controller
app.kubernetes.io/managed-by: kustomize
name: selfsigned-issuer
namespace: system
spec:
selfSigned: {}
---
apiVersion: cert-manager.io/v1
kind: Certificate
metadata:
labels:
app.kubernetes.io/name: certificate
app.kubernetes.io/instance: serving-cert
app.kubernetes.io/component: certificate
app.kubernetes.io/created-by: workspace-controller
app.kubernetes.io/part-of: workspace-controller
app.kubernetes.io/managed-by: kustomize
name: serving-cert # this name should match the one appeared in kustomizeconfig.yaml
namespace: system
spec:
# SERVICE_NAME and SERVICE_NAMESPACE will be substituted by kustomize
dnsNames:
- SERVICE_NAME.SERVICE_NAMESPACE.svc
- SERVICE_NAME.SERVICE_NAMESPACE.svc.cluster.local
issuerRef:
kind: Issuer
name: selfsigned-issuer
secretName: webhook-server-cert # this secret will not be prefixed, since it's not managed by kustomize
5 changes: 5 additions & 0 deletions workspaces/controller/config/certmanager/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
resources:
- certificate.yaml

configurations:
- kustomizeconfig.yaml
8 changes: 8 additions & 0 deletions workspaces/controller/config/certmanager/kustomizeconfig.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# This configuration is for teaching kustomize how to update name ref substitution
nameReference:
- kind: Issuer
group: cert-manager.io
fieldSpecs:
- kind: Certificate
group: cert-manager.io
path: spec/issuerRef/name
8 changes: 4 additions & 4 deletions workspaces/controller/config/crd/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ patches:

# [CERTMANAGER] To enable cert-manager, uncomment all the sections with [CERTMANAGER] prefix.
# patches here are for enabling the CA injection for each CRD
#- path: patches/cainjection_in_workspaces.yaml
#- path: patches/cainjection_in_workspacekinds.yaml
- path: patches/cainjection_in_workspaces.yaml
- path: patches/cainjection_in_workspacekinds.yaml
#+kubebuilder:scaffold:crdkustomizecainjectionpatch

# [WEBHOOK] To enable webhook, uncomment the following section
# the following config is for teaching kustomize how to do kustomization for CRDs.

#configurations:
#- kustomizeconfig.yaml
configurations:
- kustomizeconfig.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# The following patch adds a directive for certmanager to inject CA into the CRD
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
cert-manager.io/inject-ca-from: CERTIFICATE_NAMESPACE/CERTIFICATE_NAME
name: workspacekinds.kubeflow.org
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# The following patch adds a directive for certmanager to inject CA into the CRD
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
cert-manager.io/inject-ca-from: CERTIFICATE_NAMESPACE/CERTIFICATE_NAME
name: workspaces.kubeflow.org
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# The following patch enables a conversion webhook for the CRD
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: workspacekinds.kubeflow.org
spec:
conversion:
strategy: Webhook
webhook:
clientConfig:
service:
namespace: system
name: webhook-service
path: /convert
conversionReviewVersions:
- v1
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# The following patch enables a conversion webhook for the CRD
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: workspaces.kubeflow.org
spec:
conversion:
strategy: Webhook
webhook:
clientConfig:
service:
namespace: system
name: webhook-service
path: /convert
conversionReviewVersions:
- v1
Loading