From a0168ec5f08dc5f81f9b9de899856d011895d726 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) [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 --- PROJECT | 3 + ...b-operator-webhook-service_v1_service.yaml | 24 ++++ ...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 | 34 +++++ config/webhook/service.yaml | 22 +++ controllers/webhook/webhook.go | 61 ++++++++ controllers/webhook/webhook_suite_test.go | 133 ++++++++++++++++++ main.go | 3 + 12 files changed, 381 insertions(+), 1 deletion(-) 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/PROJECT b/PROJECT index 40d0da83180..756c620adf7 100644 --- a/PROJECT +++ b/PROJECT @@ -30,4 +30,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..71d4a49d17d --- /dev/null +++ b/bundle/manifests/opendatahub-operator-webhook-service_v1_service.yaml @@ -0,0 +1,24 @@ +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 + creationTimestamp: null + 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 7f08cff8ee3..9258cbab433 100644 --- a/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml +++ b/bundle/manifests/opendatahub-operator.clusterserviceversion.yaml @@ -1685,6 +1685,9 @@ spec: periodSeconds: 20 name: manager ports: + - containerPort: 9443 + name: webhook-server + protocol: TCP - containerPort: 8080 name: http protocol: TCP @@ -1706,10 +1709,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: @@ -1781,3 +1793,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..5fd6a5dfac6 --- /dev/null +++ b/config/webhook/manifests.yaml @@ -0,0 +1,34 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + creationTimestamp: null + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-opendatahub-io-v1 + failurePolicy: Ignore + name: 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 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..33d4c1bafe9 --- /dev/null +++ b/controllers/webhook/webhook.go @@ -0,0 +1,61 @@ +/* +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") + +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 70cf4679f2b..22464a3b676 100644 --- a/main.go +++ b/main.go @@ -46,6 +46,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" ) @@ -156,6 +157,8 @@ func main() { //nolint:funlen os.Exit(1) } + (&webhook.OpenDataHubWebhook{}).SetupWithManager(mgr) + // Create new uncached client to run initial setup setupCfg, err := config.GetConfig() if err != nil {