From 78d2210252c8fd1d70dd85519b75321d228b9529 Mon Sep 17 00:00:00 2001 From: Sven Thoms <21118431+shalberd@users.noreply.github.com> Date: Tue, 23 Apr 2024 10:23:25 +0200 Subject: [PATCH] add back notebook container env var from central proxy config logic so notebook containers have HTTP_PROXY, HTTPS_PROXY, NO_PROXY env vars injected if central cluster proxy config exists Signed-off-by: Sven Thoms <21118431+shalberd@users.noreply.github.com> --- .../controllers/notebook_webhook.go | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index 5b0db331f81..f8570b5311d 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -19,6 +19,7 @@ import ( "context" "encoding/json" "fmt" + configv1 "github.com/openshift/api/config/v1" "net/http" "sort" "strings" @@ -53,6 +54,8 @@ type NotebookWebhook struct { Namespace string } +var proxyEnvVars = make(map[string]string, 3) + // 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 @@ -230,6 +233,29 @@ func InjectOAuthProxy(notebook *nbv1.Notebook, oauth OAuthConfig) error { return nil } +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 + return true + } + } + } + return false + +} + // Handle transforms the Notebook objects. func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) admission.Response { @@ -279,6 +305,14 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm } } + // If cluster-wide-proxy is enabled add environment variables + if w.ClusterWideProxyIsEnabled() { + err = InjectProxyConfigEnvVars(notebook) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + } + // RHOAIENG-14552: Running notebook cannot be updated carelessly, or we may end up restarting the pod when // the webhook runs after e.g. the oauth-proxy image has been updated mutatedNotebook, needsRestart, err := w.maybeRestartRunningNotebook(ctx, req, notebook) @@ -369,6 +403,57 @@ func (w *NotebookWebhook) maybeRestartRunningNotebook(ctx context.Context, req a return mutatedNotebook, &UpdatesPending{Reason: diff}, nil } +func InjectProxyConfigEnvVars(notebook *nbv1.Notebook) error { + notebookContainers := ¬ebook.Spec.Template.Spec.Containers + var imgContainer corev1.Container + + // Update Notebook Image container with env variables from central cluster proxy config + 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...) + } + + // Update container with Proxy Env 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 +} + // 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 {