From 1eb40a60d6a6450135a02f4b1de3fa29f5eff700 Mon Sep 17 00:00:00 2001 From: Yauheni Kaliuta Date: Thu, 9 Nov 2023 15:30:50 +0200 Subject: [PATCH] DSC, DSCI: webhook skeleton Originally it was generated with ```operator-sdk create webhook --group datasciencecluster --version v1 --kind DataScienceCluster --programmatic-validation``` but webhook.Validator interface (like described in the kubebuilder book[1]) does not work well for the purpose of the webhook due to needs to access openshift cluster (client.Client) to check existing instances of DSC. So, direct implementation of Handler was done inspired by [2] and odh-notebooks implementation [3]. Move it from api package closer to controllers as in [3] as well since it's not DataScienceCluster or DSCInitialization extention anymore. certmanager files removed too due to usage of OpenShift service serving certificates[4] (see also service.beta.openshift.io/inject-cabundle annotation in config/webhook/kustomization.yaml). Add webhook generation to `make manifests` target so webhook/manifests.yaml is generated with it. [1] https://book.kubebuilder.io/cronjob-tutorial/webhook-implementation [2] https://book-v1.book.kubebuilder.io/beyond_basics/sample_webhook.html [3] https://github.com/opendatahub-io/kubeflow/blob/v1.7-branch/components/odh-notebook-controller/controllers/notebook_webhook.go [4] https://docs.openshift.com/container-platform/4.9/security/certificates/service-serving-certificate.html Signed-off-by: Yauheni Kaliuta --- Makefile | 4 +- PROJECT | 6 + ...b-operator-webhook-service_v1_service.yaml | 23 +++ ...atahub-operator.clusterserviceversion.yaml | 40 ++++++ config/default/kustomization.yaml | 5 +- config/default/manager_webhook_patch.yaml | 23 +++ config/webhook/kustomization.yaml | 9 ++ config/webhook/kustomizeconfig.yaml | 25 ++++ config/webhook/manifests.yaml | 28 ++++ config/webhook/service.yaml | 22 +++ controllers/webhook/webhook.go | 64 +++++++++ controllers/webhook/webhook_suite_test.go | 133 ++++++++++++++++++ main.go | 3 + 13 files changed, 381 insertions(+), 4 deletions(-) create mode 100644 bundle/manifests/opendatahub-operator-webhook-service_v1_service.yaml create mode 100644 config/default/manager_webhook_patch.yaml create mode 100644 config/webhook/kustomization.yaml create mode 100644 config/webhook/kustomizeconfig.yaml create mode 100644 config/webhook/manifests.yaml create mode 100644 config/webhook/service.yaml create mode 100644 controllers/webhook/webhook.go create mode 100644 controllers/webhook/webhook_suite_test.go diff --git a/Makefile b/Makefile index 34f914745ee..16b9cfa5050 100644 --- a/Makefile +++ b/Makefile @@ -124,9 +124,7 @@ endef .PHONY: manifests manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. -# TODO: enable below when we do webhook -# $(CONTROLLER_GEN) rbac:roleName=controller-manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases - $(CONTROLLER_GEN) rbac:roleName=controller-manager-role crd:ignoreUnexportedFields=true paths="./..." output:crd:artifacts:config=config/crd/bases + $(CONTROLLER_GEN) rbac:roleName=controller-manager-role crd:ignoreUnexportedFields=true webhook paths="./..." output:crd:artifacts:config=config/crd/bases $(call fetch-external-crds,github.com/openshift/api,route/v1) $(call fetch-external-crds,github.com/openshift/api,user/v1) diff --git a/PROJECT b/PROJECT index 40d0da83180..8619f2be939 100644 --- a/PROJECT +++ b/PROJECT @@ -21,6 +21,9 @@ resources: kind: DSCInitialization path: github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1 version: v1 + webhooks: + validation: true + webhookVersion: v1 - api: crdVersion: v1 namespaced: false @@ -30,4 +33,7 @@ resources: kind: DataScienceCluster path: github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1 version: v1 + webhooks: + validation: true + webhookVersion: v1 version: "3" diff --git a/bundle/manifests/opendatahub-operator-webhook-service_v1_service.yaml b/bundle/manifests/opendatahub-operator-webhook-service_v1_service.yaml new file mode 100644 index 00000000000..916c369e093 --- /dev/null +++ b/bundle/manifests/opendatahub-operator-webhook-service_v1_service.yaml @@ -0,0 +1,23 @@ +apiVersion: v1 +kind: Service +metadata: + annotations: + service.beta.openshift.io/inject-cabundle: "true" + service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-controller-webhook-cert + labels: + app.kubernetes.io/component: webhook + app.kubernetes.io/created-by: opendatahub-operator + app.kubernetes.io/instance: webhook-service + app.kubernetes.io/managed-by: kustomize + app.kubernetes.io/name: service + app.kubernetes.io/part-of: opendatahub-operator + name: opendatahub-operator-webhook-service +spec: + ports: + - port: 443 + protocol: TCP + targetPort: 9443 + selector: + control-plane: controller-manager +status: + loadBalancer: {} diff --git a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml index ee263c57229..5ee0f2b36b7 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -1759,6 +1759,9 @@ spec: periodSeconds: 20 name: manager ports: + - containerPort: 9443 + name: webhook-server + protocol: TCP - containerPort: 8080 name: http protocol: TCP @@ -1780,10 +1783,19 @@ spec: capabilities: drop: - ALL + volumeMounts: + - mountPath: /tmp/k8s-webhook-server/serving-certs + name: cert + readOnly: true securityContext: runAsNonRoot: true serviceAccountName: opendatahub-operator-controller-manager terminationGracePeriodSeconds: 10 + volumes: + - name: cert + secret: + defaultMode: 420 + secretName: opendatahub-operator-controller-webhook-cert permissions: - rules: - apiGroups: @@ -1855,3 +1867,31 @@ spec: matchLabels: component: opendatahub-operator version: 2.4.0 + webhookdefinitions: + - admissionReviewVersions: + - v1 + containerPort: 443 + deploymentName: opendatahub-operator-controller-manager + failurePolicy: Ignore + generateName: operator.opendatahub.io + rules: + - apiGroups: + - datasciencecluster.opendatahub.io + apiVersions: + - v1 + operations: + - CREATE + resources: + - datascienceclusters + - apiGroups: + - dscinitialization.opendatahub.io + apiVersions: + - v1 + operations: + - CREATE + resources: + - dscinitializations + sideEffects: None + targetPort: 9443 + type: ValidatingAdmissionWebhook + webhookPath: /validate-opendatahub-io-v1 diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index c11aec33da6..7c2b111419f 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -21,7 +21,7 @@ resources: - ../manager # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml -#- ../webhook +- ../webhook # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. 'WEBHOOK' components are required. #- ../certmanager # [PROMETHEUS] To enable prometheus monitor, uncomment all sections with 'PROMETHEUS'. @@ -37,6 +37,7 @@ resources: # [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in # crd/kustomization.yaml +# Moved below to patches #- manager_webhook_patch.yaml # [CERTMANAGER] To enable cert-manager, uncomment all sections with 'CERTMANAGER'. @@ -76,3 +77,5 @@ resources: patches: - path: manager_auth_proxy_patch.yaml +# [WEBHOOK] +- path: manager_webhook_patch.yaml diff --git a/config/default/manager_webhook_patch.yaml b/config/default/manager_webhook_patch.yaml new file mode 100644 index 00000000000..05bb4280ecf --- /dev/null +++ b/config/default/manager_webhook_patch.yaml @@ -0,0 +1,23 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + namespace: system +spec: + template: + spec: + containers: + - name: manager + ports: + - containerPort: 9443 + name: webhook-server + protocol: TCP + volumeMounts: + - mountPath: /tmp/k8s-webhook-server/serving-certs + name: cert + readOnly: true + volumes: + - name: cert + secret: + defaultMode: 420 + secretName: opendatahub-operator-controller-webhook-cert diff --git a/config/webhook/kustomization.yaml b/config/webhook/kustomization.yaml new file mode 100644 index 00000000000..8428859f524 --- /dev/null +++ b/config/webhook/kustomization.yaml @@ -0,0 +1,9 @@ +resources: +- manifests.yaml +- service.yaml + +commonAnnotations: + service.beta.openshift.io/inject-cabundle: "true" + +configurations: +- kustomizeconfig.yaml diff --git a/config/webhook/kustomizeconfig.yaml b/config/webhook/kustomizeconfig.yaml new file mode 100644 index 00000000000..25e21e3c963 --- /dev/null +++ b/config/webhook/kustomizeconfig.yaml @@ -0,0 +1,25 @@ +# the following config is for teaching kustomize where to look at when substituting vars. +# It requires kustomize v2.1.0 or newer to work properly. +nameReference: +- kind: Service + version: v1 + fieldSpecs: + - kind: MutatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/name + - kind: ValidatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/name + +namespace: +- kind: MutatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/namespace + create: true +- kind: ValidatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/namespace + create: true + +varReference: +- path: metadata/annotations diff --git a/config/webhook/manifests.yaml b/config/webhook/manifests.yaml new file mode 100644 index 00000000000..43aeb2049ae --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,28 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-opendatahub-io-v1 + failurePolicy: Fail + name: operator.opendatahub.io + rules: + - apiGroups: + - datasciencecluster.opendatahub.io + - dscinitialization.opendatahub.io + apiVersions: + - v1 + operations: + - CREATE + - UPDATE + resources: + - datascienceclusters + - dscinitializations + sideEffects: None diff --git a/config/webhook/service.yaml b/config/webhook/service.yaml new file mode 100644 index 00000000000..4f62377f651 --- /dev/null +++ b/config/webhook/service.yaml @@ -0,0 +1,22 @@ + +apiVersion: v1 +kind: Service +metadata: + labels: + app.kubernetes.io/name: service + app.kubernetes.io/instance: webhook-service + app.kubernetes.io/component: webhook + app.kubernetes.io/created-by: opendatahub-operator + app.kubernetes.io/part-of: opendatahub-operator + app.kubernetes.io/managed-by: kustomize + name: webhook-service + namespace: system + annotations: + service.beta.openshift.io/serving-cert-secret-name: opendatahub-operator-controller-webhook-cert +spec: + ports: + - port: 443 + protocol: TCP + targetPort: 9443 + selector: + control-plane: controller-manager diff --git a/controllers/webhook/webhook.go b/controllers/webhook/webhook.go new file mode 100644 index 00000000000..07b6d63c2ad --- /dev/null +++ b/controllers/webhook/webhook.go @@ -0,0 +1,64 @@ +/* +Copyright 2023. + +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 webhook + +import ( + "context" + "fmt" + "net/http" + + admissionv1 "k8s.io/api/admission/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + dsc "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + dsci "github.com/opendatahub-io/opendatahub-operator/v2/apis/dscinitialization/v1" +) + +var log = ctrl.Log.WithName("odh-controller-webhook") + +//+kubebuilder:webhook:path=/validate-opendatahub-io-v1,mutating=false,failurePolicy=fail,sideEffects=None,groups=datasciencecluster.opendatahub.io;dscinitialization.opendatahub.io,resources=datascienceclusters;dscinitializations,verbs=create;update,versions=v1,name=operator.opendatahub.io,admissionReviewVersions=v1 +//nolint:lll + +type OpenDataHubWebhook struct { + client client.Client + decoder *admission.Decoder +} + +func (w *OpenDataHubWebhook) SetupWithManager(mgr ctrl.Manager) { + hookServer := mgr.GetWebhookServer() + odhWebhook := &webhook.Admission{ + Handler: w, + } + hookServer.Register("/validate-opendatahub-io-v1", odhWebhook) +} + +func (w *OpenDataHubWebhook) InjectDecoder(d *admission.Decoder) error { + w.decoder = d + return nil +} + +func (w *OpenDataHubWebhook) InjectClient(c client.Client) error { + w.client = c + return nil +} + +func (w *OpenDataHubWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { + return admission.ValidationResponse(true, "") +} diff --git a/controllers/webhook/webhook_suite_test.go b/controllers/webhook/webhook_suite_test.go new file mode 100644 index 00000000000..0a0a3af730c --- /dev/null +++ b/controllers/webhook/webhook_suite_test.go @@ -0,0 +1,133 @@ +/* +Copyright 2023. + +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 webhook + +import ( + "context" + "crypto/tls" + "fmt" + "net" + "path/filepath" + "testing" + "time" + + admissionv1beta1 "k8s.io/api/admission/v1beta1" + //+kubebuilder:scaffold:imports + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + + dsc "github.com/opendatahub-io/opendatahub-operator/v2/apis/datasciencecluster/v1" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// These tests use Ginkgo (BDD-style Go testing framework). Refer to +// http://onsi.github.io/ginkgo/ to learn more about Ginkgo. + +var cfg *rest.Config +var k8sClient client.Client +var testEnv *envtest.Environment +var ctx context.Context +var cancel context.CancelFunc + +func TestAPIs(t *testing.T) { + RegisterFailHandler(Fail) + + RunSpecs(t, "Webhook Suite") +} + +var _ = BeforeSuite(func() { + logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) + + ctx, cancel = context.WithCancel(context.TODO()) + + By("bootstrapping test environment") + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("..", "..", "..", "config", "crd", "bases")}, + ErrorIfCRDPathMissing: false, + WebhookInstallOptions: envtest.WebhookInstallOptions{ + Paths: []string{filepath.Join("..", "..", "..", "config", "webhook")}, + }, + } + + var err error + // cfg is defined in this file globally. + cfg, err = testEnv.Start() + Expect(err).NotTo(HaveOccurred()) + Expect(cfg).NotTo(BeNil()) + + scheme := runtime.NewScheme() + err = dsc.AddToScheme(scheme) + Expect(err).NotTo(HaveOccurred()) + + err = admissionv1beta1.AddToScheme(scheme) + Expect(err).NotTo(HaveOccurred()) + + //+kubebuilder:scaffold:scheme + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme}) + Expect(err).NotTo(HaveOccurred()) + Expect(k8sClient).NotTo(BeNil()) + + // start webhook server using Manager + webhookInstallOptions := &testEnv.WebhookInstallOptions + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme, + Host: webhookInstallOptions.LocalServingHost, + Port: webhookInstallOptions.LocalServingPort, + CertDir: webhookInstallOptions.LocalServingCertDir, + LeaderElection: false, + MetricsBindAddress: "0", + }) + Expect(err).NotTo(HaveOccurred()) + + (&OpenDataHubWebhook{}).SetupWithManager(mgr) + + //+kubebuilder:scaffold:webhook + + go func() { + defer GinkgoRecover() + err = mgr.Start(ctx) + Expect(err).NotTo(HaveOccurred()) + }() + + // wait for the webhook server to get ready + dialer := &net.Dialer{Timeout: time.Second} + addrPort := fmt.Sprintf("%s:%d", webhookInstallOptions.LocalServingHost, webhookInstallOptions.LocalServingPort) + Eventually(func() error { + conn, err := tls.DialWithDialer(dialer, "tcp", addrPort, &tls.Config{InsecureSkipVerify: true}) + if err != nil { + return err + } + conn.Close() + return nil + }).Should(Succeed()) + +}) + +var _ = AfterSuite(func() { + cancel() + By("tearing down the test environment") + err := testEnv.Stop() + Expect(err).NotTo(HaveOccurred()) +}) diff --git a/main.go b/main.go index 030786c9a7a..8491ce5e2d6 100644 --- a/main.go +++ b/main.go @@ -51,6 +51,7 @@ import ( dscontr "github.com/opendatahub-io/opendatahub-operator/v2/controllers/datasciencecluster" dscicontr "github.com/opendatahub-io/opendatahub-operator/v2/controllers/dscinitialization" "github.com/opendatahub-io/opendatahub-operator/v2/controllers/secretgenerator" + "github.com/opendatahub-io/opendatahub-operator/v2/controllers/webhook" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy" "github.com/opendatahub-io/opendatahub-operator/v2/pkg/upgrade" ) @@ -132,6 +133,8 @@ func main() { //nolint:funlen os.Exit(1) } + (&webhook.OpenDataHubWebhook{}).SetupWithManager(mgr) + if err = (&dscicontr.DSCInitializationReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(),