RHOAIENG-15335: feat(odh-nbc/webhook): only update oauth-proxy image in CREATE events

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)

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)
+			}
 		}
 	}