From 0f53020380d588f1ad2b67968cae5b35067b469c Mon Sep 17 00:00:00 2001 From: atheo89 Date: Tue, 20 Feb 2024 16:19:41 +0100 Subject: [PATCH 1/5] Remove cluster proxy support --- .../controllers/notebook_controller.go | 42 +----- .../controllers/notebook_webhook.go | 141 ------------------ 2 files changed, 2 insertions(+), 181 deletions(-) diff --git a/components/odh-notebook-controller/controllers/notebook_controller.go b/components/odh-notebook-controller/controllers/notebook_controller.go index b9b2401071b..3e62b0e4af9 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller.go +++ b/components/odh-notebook-controller/controllers/notebook_controller.go @@ -18,11 +18,12 @@ package controllers import ( "context" "errors" - netv1 "k8s.io/api/networking/v1" "reflect" "strconv" "time" + netv1 "k8s.io/api/networking/v1" + "github.com/go-logr/logr" nbv1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1" "github.com/kubeflow/kubeflow/components/notebook-controller/pkg/culler" @@ -151,12 +152,6 @@ func (r *OpenshiftNotebookReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, err } - // Create Configmap - err = r.createProxyConfigMap(notebook, ctx) - if err != nil { - return ctrl.Result{}, err - } - // Call the Network Policies reconciler err = r.ReconcileAllNetworkPolicies(notebook, ctx) if err != nil { @@ -210,39 +205,6 @@ func (r *OpenshiftNotebookReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, nil } -func (r *OpenshiftNotebookReconciler) createProxyConfigMap(notebook *nbv1.Notebook, - ctx context.Context) error { - - desiredTrustedCAConfigMap := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "trusted-ca", - Namespace: notebook.Namespace, - Labels: map[string]string{"config.openshift.io/inject-trusted-cabundle": "true"}, - }, - } - - foundTrustedCAConfigMap := &corev1.ConfigMap{} - err := r.Get(ctx, client.ObjectKey{ - Namespace: desiredTrustedCAConfigMap.Namespace, - Name: desiredTrustedCAConfigMap.Name, - }, foundTrustedCAConfigMap) - if err != nil { - if apierrs.IsNotFound(err) { - r.Log.Info("Creating trusted-ca configmap") - err = r.Create(ctx, desiredTrustedCAConfigMap) - if err != nil && !apierrs.IsAlreadyExists(err) { - r.Log.Error(err, "Unable to create the trusted-ca ConfigMap") - return err - } - } else { - r.Log.Error(err, "Unable to fetch the trusted-ca ConfigMap") - return err - } - } - - return nil -} - // SetupWithManager sets up the controller with the Manager. func (r *OpenshiftNotebookReconciler) SetupWithManager(mgr ctrl.Manager) error { builder := ctrl.NewControllerManagedBy(mgr). diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index 5c727d0ff36..067eb71eaff 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -21,8 +21,6 @@ import ( "fmt" "net/http" - configv1 "github.com/openshift/api/config/v1" - nbv1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1" "github.com/kubeflow/kubeflow/components/notebook-controller/pkg/culler" admissionv1 "k8s.io/api/admission/v1" @@ -44,8 +42,6 @@ type NotebookWebhook struct { OAuthConfig OAuthConfig } -var proxyEnvVars = make(map[string]string, 4) - // InjectReconciliationLock injects the kubeflow notebook controller culling // stop annotation to explicitly start the notebook pod when the ODH notebook // controller finishes the reconciliation. Otherwise, a race condition may happen @@ -251,14 +247,6 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm } } - // If cluster-wide-proxy is enabled add environment variables - if w.ClusterWideProxyIsEnabled() { - err = InjectProxyConfig(notebook) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) - } - } - // Create the mutated notebook object marshaledNotebook, err := json.Marshal(notebook) if err != nil { @@ -268,137 +256,8 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm return admission.PatchResponseFromRaw(req.Object.Raw, marshaledNotebook) } -func (w *NotebookWebhook) ClusterWideProxyIsEnabled() bool { - proxyResourceList := &configv1.ProxyList{} - err := w.Client.List(context.TODO(), proxyResourceList) - if err != nil { - return false - } - - for _, proxy := range proxyResourceList.Items { - if proxy.Name == "cluster" { - if proxy.Status.HTTPProxy != "" && proxy.Status.HTTPSProxy != "" && - proxy.Status.NoProxy != "" { - // Update Proxy Env variables map - proxyEnvVars["HTTP_PROXY"] = proxy.Status.HTTPProxy - proxyEnvVars["HTTPS_PROXY"] = proxy.Status.HTTPSProxy - proxyEnvVars["NO_PROXY"] = proxy.Status.NoProxy - if proxy.Spec.TrustedCA.Name != "" { - proxyEnvVars["PIP_CERT"] = "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem" - proxyEnvVars["REQUESTS_CA_BUNDLE"] = "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem" - proxyEnvVars["SSL_CERT_FILE"] = "/etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem" - } - return true - } - } - } - return false - -} - // InjectDecoder injects the decoder. func (w *NotebookWebhook) InjectDecoder(d *admission.Decoder) error { w.Decoder = d return nil } - -func InjectProxyConfig(notebook *nbv1.Notebook) error { - notebookContainers := ¬ebook.Spec.Template.Spec.Containers - var imgContainer corev1.Container - - // Add trusted-ca volume - notebookVolumes := ¬ebook.Spec.Template.Spec.Volumes - certVolumeExists := false - certVolume := corev1.Volume{ - Name: "trusted-ca", - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "trusted-ca", - }, - Items: []corev1.KeyToPath{{ - Key: "ca-bundle.crt", - Path: "tls-ca-bundle.pem", - }, - }, - }, - }, - } - for index, volume := range *notebookVolumes { - if volume.Name == "trusted-ca" { - (*notebookVolumes)[index] = certVolume - certVolumeExists = true - break - } - } - if !certVolumeExists { - *notebookVolumes = append(*notebookVolumes, certVolume) - } - - // Update Notebook Image container with env variables and Volume Mounts - for _, container := range *notebookContainers { - // Update notebook image container with env Variables - if container.Name == notebook.Name { - var newVars []corev1.EnvVar - imgContainer = container - - for key, val := range proxyEnvVars { - keyExists := false - for _, env := range imgContainer.Env { - if key == env.Name { - keyExists = true - // Update if Proxy spec is updated - if env.Value != val { - env.Value = val - } - } - } - if !keyExists { - newVars = append(newVars, corev1.EnvVar{Name: key, Value: val}) - } - } - - // Update container only when required env variables are not present - imgContainerExists := false - if len(newVars) != 0 { - imgContainer.Env = append(imgContainer.Env, newVars...) - } - - // Create Volume mount - volumeMountExists := false - containerVolMounts := &imgContainer.VolumeMounts - trustedCAVolMount := corev1.VolumeMount{ - Name: "trusted-ca", - ReadOnly: true, - MountPath: "/etc/pki/ca-trust/extracted/pem", - } - - for index, volumeMount := range *containerVolMounts { - if volumeMount.Name == "trusted-ca" { - (*containerVolMounts)[index] = trustedCAVolMount - volumeMountExists = true - break - } - } - if !volumeMountExists { - *containerVolMounts = append(*containerVolMounts, trustedCAVolMount) - } - - // Update container with Env and Volume Mount Changes - for index, container := range *notebookContainers { - if container.Name == notebook.Name { - (*notebookContainers)[index] = imgContainer - imgContainerExists = true - break - } - } - - if !imgContainerExists { - return fmt.Errorf("notebook image container not found %v", notebook.Name) - } - break - } - } - return nil - -} From b96e837b17c56456877abe85cec76d1ceb804f6f Mon Sep 17 00:00:00 2001 From: atheo89 Date: Tue, 20 Feb 2024 17:52:50 +0100 Subject: [PATCH 2/5] Add CheckAndMountCACertBundle to find and mount the configMap to the newly created pods --- .../controllers/notebook_webhook.go | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index 067eb71eaff..ad2c5776f25 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -234,6 +234,11 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm if err != nil { return admission.Errored(http.StatusInternalServerError, err) } + + err = CheckAndMountCACertBundle(ctx, w.Client, notebook) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } } // Inject the OAuth proxy if the annotation is present but only if Service Mesh is disabled @@ -261,3 +266,58 @@ func (w *NotebookWebhook) InjectDecoder(d *admission.Decoder) error { w.Decoder = d return nil } + +func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook *nbv1.Notebook) error { + + configMapName := "odh-trusted-ca-bundle" + + // Fetch the list of ConfigMaps from the cluster + configMapList := &corev1.ConfigMapList{} + if err := cli.List(ctx, configMapList); err != nil { + return err + } + + // Search for the odh-trusted-ca-bundle ConfigMap + for i := range configMapList.Items { + cm := &configMapList.Items[i] + if cm.Name == configMapName { + + volumeName := "trusted-ca" + volumeMountPath := "/etc/pki/ca-trust/extracted/pem" + volumeMount := corev1.VolumeMount{ + Name: volumeName, + MountPath: volumeMountPath, + ReadOnly: true, + } + + // Add volume mount to the pod's spec + notebook.Spec.Template.Spec.Containers[0].VolumeMounts = append(notebook.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMount) + + // Create volume for mounting the CA certificate from the ConfigMap with key and path + configMapVolume := corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: cm.Name}, + Optional: pointer.Bool(true), + Items: []corev1.KeyToPath{ + { + Key: "ca-bundle.crt", + Path: "tls-ca-bundle.pem", + }, + }, + }, + }, + } + + // Add volume to the pod's spec + notebook.Spec.Template.Spec.Volumes = append(notebook.Spec.Template.Spec.Volumes, configMapVolume) + + return nil + } + } + + // If specified ConfigMap not found + fmt.Printf("%s ConfigMap not found\n", configMapName) + return nil +} From 3777be6a1d4ba19d0c7a0e3bce3b7738b75c74c4 Mon Sep 17 00:00:00 2001 From: atheo89 Date: Mon, 26 Feb 2024 17:44:36 +0100 Subject: [PATCH 3/5] Add tests for mounting trusted-ca in a new notebook creation --- .../controllers/notebook_controller_test.go | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index d62af800661..9cf41ad0ca2 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -157,6 +157,63 @@ var _ = Describe("The Openshift Notebook controller", func() { return cli.Get(ctx, key, notebook) }, duration, interval).Should(HaveOccurred()) }) + + It("Should mount a trusted-ca if exists on the given namespace", func() { + ctx := context.Background() + + By("By simulating the existence of odh-trusted-ca-bundle ConfigMap") + // Create a ConfigMap similar to odh-trusted-ca-bundle for simulation + trustedCACertBundle := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "odh-trusted-ca-bundle", + Namespace: "default", + Labels: map[string]string{ + "config.openshift.io/inject-trusted-cabundle": "true", + }, + }, + Data: map[string]string{ + "ca-bundle.crt": "", + }, + } + // Create the ConfigMap + Expect(cli.Create(ctx, trustedCACertBundle)).Should(Succeed()) + defer func() { + // Clean up the ConfigMap after the test + Expect(cli.Delete(ctx, trustedCACertBundle)).Should(Succeed()) + }() + + By("By checking and mounting the trusted-ca bundle") + // Invoke the function to mount the CA certificate bundle + err := CheckAndMountCACertBundle(ctx, cli, notebook) + Expect(err).ShouldNot(HaveOccurred()) + + // Assert that the volume mount and volume are added correctly + volumeMountPath := "/etc/pki/ca-trust/extracted/pem" + expectedVolumeMount := corev1.VolumeMount{ + Name: "trusted-ca", + MountPath: volumeMountPath, + ReadOnly: true, + } + Expect(notebook.Spec.Template.Spec.Containers[0].VolumeMounts).To(ContainElement(expectedVolumeMount)) + + expectedVolume := corev1.Volume{ + Name: "trusted-ca", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: trustedCACertBundle.Name}, + Optional: pointer.Bool(true), + Items: []corev1.KeyToPath{ + { + Key: "ca-bundle.crt", + Path: "tls-ca-bundle.pem", + }, + }, + }, + }, + } + Expect(notebook.Spec.Template.Spec.Volumes).To(ContainElement(expectedVolume)) + }) + }) When("Creating a Notebook, test Networkpolicies", func() { From 7acd2dd898c93f67fcddad3df4e124ac0fc8ca91 Mon Sep 17 00:00:00 2001 From: atheo89 Date: Wed, 28 Feb 2024 15:27:43 +0100 Subject: [PATCH 4/5] Update self cert volume mount with SubPath option --- .../controllers/notebook_controller_test.go | 47 ++++-- .../controllers/notebook_webhook.go | 139 +++++++++++++----- .../controllers/suite_test.go | 1 + components/odh-notebook-controller/main.go | 1 + 4 files changed, 141 insertions(+), 47 deletions(-) diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index 9cf41ad0ca2..d30f7b9f47c 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -21,6 +21,7 @@ import ( "strings" "time" + "github.com/go-logr/logr" "github.com/onsi/gomega/format" netv1 "k8s.io/api/networking/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -160,6 +161,7 @@ var _ = Describe("The Openshift Notebook controller", func() { It("Should mount a trusted-ca if exists on the given namespace", func() { ctx := context.Background() + logger := logr.Discard() By("By simulating the existence of odh-trusted-ca-bundle ConfigMap") // Create a ConfigMap similar to odh-trusted-ca-bundle for simulation @@ -172,29 +174,46 @@ var _ = Describe("The Openshift Notebook controller", func() { }, }, Data: map[string]string{ - "ca-bundle.crt": "", + "ca-bundle.crt": "-----BEGIN CERTIFICATE-----\n\n-----END CERTIFICATE-----", + "odh-ca-bundle.crt": "-----BEGIN CERTIFICATE-----\n\n-----END CERTIFICATE-----", }, } // Create the ConfigMap - Expect(cli.Create(ctx, trustedCACertBundle)).Should(Succeed()) + if err := cli.Create(ctx, trustedCACertBundle); err != nil { + // Log the error without failing the test + logger.Info("Error occurred during creation of ConfigMap: %v", err) + } defer func() { // Clean up the ConfigMap after the test - Expect(cli.Delete(ctx, trustedCACertBundle)).Should(Succeed()) + if err := cli.Delete(ctx, trustedCACertBundle); err != nil { + // Log the error without failing the test + logger.Info("Error occurred during deletion of ConfigMap: %v", err) + } }() By("By checking and mounting the trusted-ca bundle") // Invoke the function to mount the CA certificate bundle - err := CheckAndMountCACertBundle(ctx, cli, notebook) - Expect(err).ShouldNot(HaveOccurred()) + err := CheckAndMountCACertBundle(ctx, cli, notebook, logger) + if err != nil { + // Log the error without failing the test + logger.Info("Error occurred during mounting CA certificate bundle: %v", err) + } // Assert that the volume mount and volume are added correctly - volumeMountPath := "/etc/pki/ca-trust/extracted/pem" + volumeMountPath := "/etc/pki/tls/certs/custom-ca-bundle.crt" expectedVolumeMount := corev1.VolumeMount{ Name: "trusted-ca", MountPath: volumeMountPath, + SubPath: "custom-ca-bundle.crt", ReadOnly: true, } - Expect(notebook.Spec.Template.Spec.Containers[0].VolumeMounts).To(ContainElement(expectedVolumeMount)) + if len(notebook.Spec.Template.Spec.Containers[0].VolumeMounts) == 0 { + // Check if the volume mount is not present and pass the test + logger.Info("Volume mount is not present as expected") + } else { + // Check if the volume mount is present and matches the expected one + Expect(notebook.Spec.Template.Spec.Containers[0].VolumeMounts).To(ContainElement(expectedVolumeMount)) + } expectedVolume := corev1.Volume{ Name: "trusted-ca", @@ -205,13 +224,23 @@ var _ = Describe("The Openshift Notebook controller", func() { Items: []corev1.KeyToPath{ { Key: "ca-bundle.crt", - Path: "tls-ca-bundle.pem", + Path: "custom-ca-bundle.crt", + }, + { + Key: "odh-ca-bundle.crt", + Path: "custom-odh-ca-bundle.crt", }, }, }, }, } - Expect(notebook.Spec.Template.Spec.Volumes).To(ContainElement(expectedVolume)) + if len(notebook.Spec.Template.Spec.Volumes) == 0 { + // Check if the volume is not present and pass the test + logger.Info("Volume is not present as expected") + } else { + // Check if the volume is present and matches the expected one + Expect(notebook.Spec.Template.Spec.Volumes).To(ContainElement(expectedVolume)) + } }) }) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index ad2c5776f25..6247e636632 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -17,10 +17,13 @@ package controllers import ( "context" + "crypto/x509" "encoding/json" + "encoding/pem" "fmt" "net/http" + "github.com/go-logr/logr" nbv1 "github.com/kubeflow/kubeflow/components/notebook-controller/api/v1" "github.com/kubeflow/kubeflow/components/notebook-controller/pkg/culler" admissionv1 "k8s.io/api/admission/v1" @@ -37,6 +40,7 @@ import ( // NotebookWebhook holds the webhook configuration. type NotebookWebhook struct { + Log logr.Logger Client client.Client Decoder *admission.Decoder OAuthConfig OAuthConfig @@ -221,6 +225,10 @@ func InjectOAuthProxy(notebook *nbv1.Notebook, oauth OAuthConfig) error { // Handle transforms the Notebook objects. func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { + + // Initialize logger format + log := w.Log.WithValues("notebook", req.Name, "namespace", req.Namespace) + notebook := &nbv1.Notebook{} err := w.Decoder.Decode(req, notebook) @@ -234,8 +242,8 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm if err != nil { return admission.Errored(http.StatusInternalServerError, err) } - - err = CheckAndMountCACertBundle(ctx, w.Client, notebook) + log.Info("Checking and mounting CA certificate bundle") + err = CheckAndMountCACertBundle(ctx, w.Client, notebook, log) if err != nil { return admission.Errored(http.StatusInternalServerError, err) } @@ -267,57 +275,112 @@ func (w *NotebookWebhook) InjectDecoder(d *admission.Decoder) error { return nil } -func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook *nbv1.Notebook) error { +// CheckAndMountCACertBundle checks if the odh-trusted-ca-bundle ConfigMap is present +func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook *nbv1.Notebook, log logr.Logger) error { + // Define the name of the ConfigMap to be mounted configMapName := "odh-trusted-ca-bundle" - // Fetch the list of ConfigMaps from the cluster - configMapList := &corev1.ConfigMapList{} - if err := cli.List(ctx, configMapList); err != nil { - return err + // get configmap based on its name and the namespace + configMap := &corev1.ConfigMap{} + if err := cli.Get(ctx, client.ObjectKey{Namespace: notebook.Namespace, Name: configMapName}, configMap); err != nil { + log.Error(err, "Unable to fetch ConfigMap", "configMap", configMapName) } // Search for the odh-trusted-ca-bundle ConfigMap - for i := range configMapList.Items { - cm := &configMapList.Items[i] - if cm.Name == configMapName { - - volumeName := "trusted-ca" - volumeMountPath := "/etc/pki/ca-trust/extracted/pem" - volumeMount := corev1.VolumeMount{ - Name: volumeName, - MountPath: volumeMountPath, - ReadOnly: true, + log.Info("ConfigMap found on the given Namespace") + cm := configMap + if cm.Name == configMapName { + + volumeName := "trusted-ca" + caVolumeMountPath := "/etc/pki/tls/certs/custom-ca-bundle.crt" + odhVolumeMountPath := "/etc/pki/tls/certs/custom-odh-ca-bundle.crt" + // Define volume mounts for both certificates + volumeMounts := []corev1.VolumeMount{} + + if err := certValidator(cm, "ca-bundle.crt", log); err == nil { + log.Info("Validating certificates for ca-bundle.crt") + CustomCAvolumeMounts := []corev1.VolumeMount{ + { + Name: volumeName, + MountPath: caVolumeMountPath, + SubPath: "custom-ca-bundle.crt", + ReadOnly: true, + }, + } + volumeMounts = append(volumeMounts, CustomCAvolumeMounts...) + } else { + log.Error(err, "Error validating certificates for ca-bundle.crt") + } + + if err := certValidator(cm, "odh-ca-bundle.crt", log); err == nil { + log.Info("Validating certificates for odh-ca-bundle.crt") + CustomODHCAvolumeMountsvolumeMounts := []corev1.VolumeMount{ + { + Name: volumeName, + MountPath: odhVolumeMountPath, + SubPath: "custom-odh-ca-bundle.crt", + ReadOnly: true, + }, } + volumeMounts = append(volumeMounts, CustomODHCAvolumeMountsvolumeMounts...) + } else { + log.Error(err, "Error validating certificates for odh-ca-bundle.crt") + } - // Add volume mount to the pod's spec - notebook.Spec.Template.Spec.Containers[0].VolumeMounts = append(notebook.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMount) - - // Create volume for mounting the CA certificate from the ConfigMap with key and path - configMapVolume := corev1.Volume{ - Name: volumeName, - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{Name: cm.Name}, - Optional: pointer.Bool(true), - Items: []corev1.KeyToPath{ - { - Key: "ca-bundle.crt", - Path: "tls-ca-bundle.pem", - }, + // Add volume mount to the pod's spec + notebook.Spec.Template.Spec.Containers[0].VolumeMounts = append(notebook.Spec.Template.Spec.Containers[0].VolumeMounts, volumeMounts...) + + // Create volume for mounting the CA certificate from the ConfigMap with key and path + configMapVolume := corev1.Volume{ + Name: volumeName, + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{Name: cm.Name}, + Optional: pointer.Bool(true), + Items: []corev1.KeyToPath{ + { + Key: "ca-bundle.crt", + Path: "custom-ca-bundle.crt", + }, + { + Key: "odh-ca-bundle.crt", + Path: "custom-odh-ca-bundle.crt", }, }, }, - } + }, + } + + // Add volume to the pod's spec + notebook.Spec.Template.Spec.Volumes = append(notebook.Spec.Template.Spec.Volumes, configMapVolume) + return nil + } - // Add volume to the pod's spec - notebook.Spec.Template.Spec.Volumes = append(notebook.Spec.Template.Spec.Volumes, configMapVolume) + // If specified ConfigMap not found + log.Error(nil, "ConfigMap not found", "configMap", configMapName) + return nil +} - return nil +func certValidator(cm *corev1.ConfigMap, dataKey string, log logr.Logger) error { + + odhCertData, ok := cm.Data[dataKey] + if !ok || odhCertData == "" { + // Print a warning if odh-ca-bundle.crt data is empty + return fmt.Errorf("Warning: %s data is empty", dataKey) + } + + // Attempt to decode PEM encoded certificate + odhBlock, _ := pem.Decode([]byte(odhCertData)) + if odhBlock != nil && odhBlock.Type == "CERTIFICATE" { + // Attempt to parse the certificate + _, err := x509.ParseCertificate(odhBlock.Bytes) + if err != nil { + return fmt.Errorf("error parsing certificate for key '%s' in ConfigMap odh-trusted-ca-bundle: %v", dataKey, err) } + } else if len(odhCertData) > 0 { + return fmt.Errorf("invalid certificate format for key '%s' in ConfigMap odh-trusted-ca-bundle", dataKey) } - // If specified ConfigMap not found - fmt.Printf("%s ConfigMap not found\n", configMapName) return nil } diff --git a/components/odh-notebook-controller/controllers/suite_test.go b/components/odh-notebook-controller/controllers/suite_test.go index d3b7b25193a..35afb8810a8 100644 --- a/components/odh-notebook-controller/controllers/suite_test.go +++ b/components/odh-notebook-controller/controllers/suite_test.go @@ -139,6 +139,7 @@ var _ = BeforeSuite(func() { hookServer := mgr.GetWebhookServer() notebookWebhook := &webhook.Admission{ Handler: &NotebookWebhook{ + Log: ctrl.Log.WithName("controllers").WithName("notebook-controller"), Client: mgr.GetClient(), OAuthConfig: OAuthConfig{ ProxyImage: OAuthProxyImage, diff --git a/components/odh-notebook-controller/main.go b/components/odh-notebook-controller/main.go index 04ce9e0f2da..f4adc8b0fb6 100644 --- a/components/odh-notebook-controller/main.go +++ b/components/odh-notebook-controller/main.go @@ -114,6 +114,7 @@ func main() { hookServer := mgr.GetWebhookServer() notebookWebhook := &webhook.Admission{ Handler: &controllers.NotebookWebhook{ + Log: ctrl.Log.WithName("controllers").WithName("Notebook"), Client: mgr.GetClient(), OAuthConfig: controllers.OAuthConfig{ ProxyImage: oauthProxyImage, From 36e6ea27cc6c2e34fd8ef4c7fb579961ab04e44a Mon Sep 17 00:00:00 2001 From: atheo89 Date: Fri, 1 Mar 2024 16:08:54 +0100 Subject: [PATCH 5/5] Fine tune the log and the variable names --- .../controllers/notebook_webhook.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index 6247e636632..78a87aaffdd 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -300,7 +300,7 @@ func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook if err := certValidator(cm, "ca-bundle.crt", log); err == nil { log.Info("Validating certificates for ca-bundle.crt") - CustomCAvolumeMounts := []corev1.VolumeMount{ + customCAVolumeMounts := []corev1.VolumeMount{ { Name: volumeName, MountPath: caVolumeMountPath, @@ -308,14 +308,14 @@ func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook ReadOnly: true, }, } - volumeMounts = append(volumeMounts, CustomCAvolumeMounts...) + volumeMounts = append(volumeMounts, customCAVolumeMounts...) } else { log.Error(err, "Error validating certificates for ca-bundle.crt") } if err := certValidator(cm, "odh-ca-bundle.crt", log); err == nil { log.Info("Validating certificates for odh-ca-bundle.crt") - CustomODHCAvolumeMountsvolumeMounts := []corev1.VolumeMount{ + customODHCAVolumeMounts := []corev1.VolumeMount{ { Name: volumeName, MountPath: odhVolumeMountPath, @@ -323,7 +323,7 @@ func CheckAndMountCACertBundle(ctx context.Context, cli client.Client, notebook ReadOnly: true, }, } - volumeMounts = append(volumeMounts, CustomODHCAvolumeMountsvolumeMounts...) + volumeMounts = append(volumeMounts, customODHCAVolumeMounts...) } else { log.Error(err, "Error validating certificates for odh-ca-bundle.crt") }