diff --git a/components/odh-notebook-controller/controllers/notebook_controller_test.go b/components/odh-notebook-controller/controllers/notebook_controller_test.go index 4d251af574d..5411440e3f8 100644 --- a/components/odh-notebook-controller/controllers/notebook_controller_test.go +++ b/components/odh-notebook-controller/controllers/notebook_controller_test.go @@ -608,21 +608,23 @@ var _ = Describe("The Openshift Notebook controller", func() { }, duration, interval).Should(BeTrue()) }) - It("Should reconcile the Notebook when modified", func() { - By("By simulating a manual Notebook modification") - notebook.Spec.Template.Spec.ServiceAccountName = "foo" - notebook.Spec.Template.Spec.Containers[1].Image = "bar" - notebook.Spec.Template.Spec.Volumes[1].VolumeSource = corev1.VolumeSource{} - Expect(cli.Update(ctx, notebook)).Should(Succeed()) - time.Sleep(interval) - - By("By checking that the webhook has restored the Notebook spec") - Eventually(func() error { - key := types.NamespacedName{Name: Name, Namespace: Namespace} - return cli.Get(ctx, key, notebook) - }, duration, interval).Should(Succeed()) - Expect(CompareNotebooks(*notebook, expectedNotebook)).Should(BeTrue()) - }) + // RHOAIENG-14552: We *do not* reconcile OAuth in the notebook when it's modified + + //It("Should reconcile the Notebook when modified", func() { + // By("By simulating a manual Notebook modification") + // notebook.Spec.Template.Spec.ServiceAccountName = "foo" + // notebook.Spec.Template.Spec.Containers[1].Image = "bar" + // notebook.Spec.Template.Spec.Volumes[1].VolumeSource = corev1.VolumeSource{} + // Expect(cli.Update(ctx, notebook)).Should(Succeed()) + // time.Sleep(interval) + // + // By("By checking that the webhook has restored the Notebook spec") + // Eventually(func() error { + // key := types.NamespacedName{Name: Name, Namespace: Namespace} + // return cli.Get(ctx, key, notebook) + // }, duration, interval).Should(Succeed()) + // Expect(CompareNotebooks(*notebook, expectedNotebook)).Should(BeTrue()) + //}) serviceAccount := &corev1.ServiceAccount{} expectedServiceAccount := createOAuthServiceAccount(Name, Namespace) diff --git a/components/odh-notebook-controller/controllers/notebook_webhook.go b/components/odh-notebook-controller/controllers/notebook_webhook.go index bd91ae05470..49e2a009e48 100644 --- a/components/odh-notebook-controller/controllers/notebook_webhook.go +++ b/components/odh-notebook-controller/controllers/notebook_webhook.go @@ -265,13 +265,17 @@ func (w *NotebookWebhook) Handle(ctx context.Context, req admission.Request) adm } // Inject the OAuth proxy if the annotation is present but only if Service Mesh is disabled - if OAuthInjectionIsEnabled(notebook.ObjectMeta) { - if ServiceMeshIsEnabled(notebook.ObjectMeta) { - return admission.Denied(fmt.Sprintf("Cannot have both %s and %s set to true. Pick one.", AnnotationServiceMesh, AnnotationInjectOAuth)) - } - err = InjectOAuthProxy(notebook, w.OAuthConfig) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) + if OAuthInjectionIsEnabled(notebook.ObjectMeta) && ServiceMeshIsEnabled(notebook.ObjectMeta) { + return admission.Denied(fmt.Sprintf("Cannot have both %s and %s set to true. Pick one.", AnnotationServiceMesh, AnnotationInjectOAuth)) + } + // RHOAIENG-14552: Updating oauth-proxy in a running notebook may lead to notebook restart. Updating only + // on Create is safe as it cannot cause a restart in already running notebook when oauth-proxy image changes. + if req.Operation == admissionv1.Create { + if OAuthInjectionIsEnabled(notebook.ObjectMeta) { + err = InjectOAuthProxy(notebook, w.OAuthConfig) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } } }