RHOAIENG-25707: test(odh-nbc): additional testcases as per review feedback (#643)

* RHOAIENG-13916: refactor(tests): enhance Notebook webhook test cases to utilize helper functions for resource creation

* RHOAIENG-23765: enhance(tests): expand test coverage for Notebook webhook image resolution logic, including namespace-specific and multi-ImageStream scenarios

* NO-JIRA: chore(tests): fix import formatting in `notebook_webhook_test.go`

Author: jiridanek

components/odh-notebook-controller/controllers/notebook_webhook_test.go

@@ -25,6 +25,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	. "github.com/onsi/ginkgo"
@@ -48,20 +49,70 @@ var _ = Describe("The Openshift Notebook webhook", func() {
 			}
 		})
 
+		var newImageStream = func(name, namespace, tag, dockerImageReference string) *imagev1.ImageStream {
+			return &imagev1.ImageStream{
+				TypeMeta: metav1.TypeMeta{
+					Kind:       "ImageStream",
+					APIVersion: "image.openshift.io/v1",
+				},
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      name,
+					Namespace: namespace,
+				},
+				Spec: imagev1.ImageStreamSpec{
+					LookupPolicy: imagev1.ImageLookupPolicy{
+						Local: true,
+					},
+				},
+				Status: imagev1.ImageStreamStatus{
+					Tags: []imagev1.NamedTagEventList{{
+						Tag: tag,
+						Items: []imagev1.TagEvent{{
+							DockerImageReference: dockerImageReference,
+						}},
+					}},
+				},
+			}
+		}
+
+		var newNotebook = func(annotations map[string]string, initialImage string) *nbv1.Notebook {
+			return &nbv1.Notebook{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        Name,
+					Namespace:   Namespace,
+					Annotations: annotations,
+				},
+				Spec: nbv1.NotebookSpec{
+					Template: nbv1.NotebookTemplateSpec{
+						Spec: corev1.PodSpec{Containers: []corev1.Container{{
+							Name:  Name,
+							Image: initialImage,
+						}}},
+					},
+				},
+			}
+		}
+
+		// https://go.dev/wiki/TableDrivenTests
 		testCases := []struct {
-			name        string
-			imageStream *imagev1.ImageStream
-			notebook    *nbv1.Notebook
+			name string
+
+			imageStreams []*imagev1.ImageStream
+			notebook     *nbv1.Notebook
+
 			// currently we expect that Notebook CR is always created,
-			// and when unable to resolve imagestream, image: is left alone
+			// and when unable to resolve ImageStream, image: is left alone
 			expectedImage string
 			// see https://www.youtube.com/watch?v=prLRI3VEVq4 for Observability Driven Development intro
 			expectedEvents   []string
 			unexpectedEvents []string
 		}{
 			{
-				name: "ImageStream with all that is needful",
-				imageStream: &imagev1.ImageStream{
+				name: "ImageStream with all that is needful is correctly resolved",
+
+				// The first test case does not use the CR creation helpers
+				// so that everything going in is fully visible
+				imageStreams: []*imagev1.ImageStream{{
 					TypeMeta: metav1.TypeMeta{
 						Kind:       "ImageStream",
 						APIVersion: "image.openshift.io/v1",
@@ -92,7 +143,7 @@ var _ = Describe("The Openshift Notebook webhook", func() {
 							}},
 						}},
 					},
-				},
+				}},
 				notebook: &nbv1.Notebook{
 					ObjectMeta: metav1.ObjectMeta{
 						Name:      Name,
@@ -111,14 +162,16 @@ var _ = Describe("The Openshift Notebook webhook", func() {
 							}}}},
 					},
 				},
+
 				expectedImage: "quay.io/modh/odh-generic-data-science-notebook@sha256:76e6af79c601a323f75a58e7005de0beac66b8cccc3d2b67efb6d11d85f0cfa1",
 				unexpectedEvents: []string{
 					IMAGE_STREAM_NOT_FOUND_EVENT,
 				},
 			},
 			{
 				name: "ImageStream with a tag without items (RHOAIENG-13916)",
-				imageStream: &imagev1.ImageStream{
+
+				imageStreams: []*imagev1.ImageStream{{
 					TypeMeta: metav1.TypeMeta{
 						Kind:       "ImageStream",
 						APIVersion: "image.openshift.io/v1",
@@ -144,86 +197,96 @@ var _ = Describe("The Openshift Notebook webhook", func() {
 							}},
 						}},
 					},
-				},
-				notebook: &nbv1.Notebook{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      Name,
-						Namespace: Namespace,
-						Annotations: map[string]string{
-							"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
-						},
-					},
-					Spec: nbv1.NotebookSpec{
-						Template: nbv1.NotebookTemplateSpec{
-							Spec: corev1.PodSpec{Containers: []corev1.Container{{
-								Name:  Name,
-								Image: ":some-tag",
-							}}}},
-					},
-				},
+				}},
+				notebook: newNotebook(map[string]string{
+					"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
+				}, ":some-tag"),
+
 				// there is no update to the Notebook
 				expectedImage: ":some-tag",
 				expectedEvents: []string{
 					IMAGE_STREAM_TAG_NOT_FOUND_EVENT,
 				},
 			},
+			//
+			// Tests for RHOAIENG-23609 project-scoped workbench imagestreams feature,
+			// tasks RHOAIENG-23765, and RHOAIENG-25707
+			//
 			{
 				name: "ImageStream in the same namespace as the Notebook",
-				imageStream: &imagev1.ImageStream{
-					TypeMeta: metav1.TypeMeta{
-						Kind:       "ImageStream",
-						APIVersion: "image.openshift.io/v1",
-					},
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      "some-image",
-						Namespace: Namespace,
-					},
-					Spec: imagev1.ImageStreamSpec{
-						LookupPolicy: imagev1.ImageLookupPolicy{
-							Local: true,
-						},
-					},
-					Status: imagev1.ImageStreamStatus{
-						Tags: []imagev1.NamedTagEventList{{
-							Tag: "some-tag",
-							Items: []imagev1.TagEvent{{
-								Created:              toMetav1Time("2025-05-14T02:36:21Z"),
-								DockerImageReference: "quay.io/modh/odh-generic-data-science-notebook@sha256:5999547f847ca841fe067ff84e2972d2cbae598066c2418e236448e115c1728e",
-								Image:                "sha256:5999547f847ca841fe067ff84e2972d2cbae598066c2418e236448e115c1728e",
-								Generation:           2,
-							}},
-							Conditions: []imagev1.TagEventCondition{{
-								Type:               "ImportSuccess",
-								Status:             "False",
-								LastTransitionTime: toMetav1Time("2025-05-14T02:36:21Z"),
-								Reason:             "NotFound",
-							}},
-						}},
-					},
-				},
-				notebook: &nbv1.Notebook{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:      Name,
-						Namespace: Namespace,
-						Annotations: map[string]string{
-							"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
-							"opendatahub.io/workbench-image-namespace":      Namespace,
-						},
-					},
-					Spec: nbv1.NotebookSpec{
-						Template: nbv1.NotebookTemplateSpec{
-							Spec: corev1.PodSpec{Containers: []corev1.Container{{
-								Name:  Name,
-								Image: ":some-tag",
-							}}}},
-					},
+
+				imageStreams: []*imagev1.ImageStream{
+					newImageStream("some-image", Namespace, "some-tag", "quay.io/modh/odh-generic-data-science-notebook@sha256:5999547f847ca841fe067ff84e2972d2cbae598066c2418e236448e115c1728e"),
 				},
-				// valid new image is picked from user namespace.
+				notebook: newNotebook(map[string]string{
+					"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
+					"opendatahub.io/workbench-image-namespace":      Namespace,
+				}, ":some-tag"),
+
 				expectedImage: "quay.io/modh/odh-generic-data-science-notebook@sha256:5999547f847ca841fe067ff84e2972d2cbae598066c2418e236448e115c1728e",
 				unexpectedEvents: []string{
 					IMAGE_STREAM_NOT_FOUND_EVENT,
 				},
 			},
+			//
+			// The following test cases require
+			// multiple ImageStreams to be created.
+			//
+			{
+				name: "last-image-selection is specified, workbench-image-namespace is also specified",
+
+				imageStreams: []*imagev1.ImageStream{
+					newImageStream("some-image", Namespace, "some-tag", "quay.io/workbench-namespace-imagestream:latest"),
+					newImageStream("some-image", odhNotebookControllerTestNamespace, "some-tag", "quay.io/operator-namespace-imagestream:latest"),
+				},
+				notebook: newNotebook(map[string]string{
+					"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
+					"opendatahub.io/workbench-image-namespace":      Namespace,
+				}, ":some-tag"),
+
+				// image is picked from notebook namespace
+				expectedImage: "quay.io/workbench-namespace-imagestream:latest",
+				unexpectedEvents: []string{
+					IMAGE_STREAM_NOT_FOUND_EVENT,
+				},
+			},
+			{
+				name: "last-image-selection is specified, workbench-image-namespace is not specified",
+
+				imageStreams: []*imagev1.ImageStream{
+					newImageStream("some-image", Namespace, "some-tag", "quay.io/workbench-namespace-imagestream:latest"),
+					newImageStream("some-image", odhNotebookControllerTestNamespace, "some-tag", "quay.io/operator-namespace-imagestream:latest"),
+				},
+				notebook: newNotebook(map[string]string{
+					"notebooks.opendatahub.io/last-image-selection": "some-image:some-tag",
+					"opendatahub.io/workbench-image-namespace":      "",
+				}, ":some-tag"),
+
+				// image is picked from operator namespace
+				expectedImage: "quay.io/operator-namespace-imagestream:latest",
+				unexpectedEvents: []string{
+					IMAGE_STREAM_NOT_FOUND_EVENT,
+				},
+			},
+			//
+			// Something that Dashboard should never do, but we want to check it nevertheless
+			//
+			{
+				name: "last-image-selection is not specified",
+
+				imageStreams: []*imagev1.ImageStream{
+					newImageStream("some-image", Namespace, "some-tag", "quay.io/modh/odh-generic-data-science-notebook@sha256:5999547f847ca841fe067ff84e2972d2cbae598066c2418e236448e115c1728e"),
+				},
+				notebook: newNotebook(map[string]string{
+					"opendatahub.io/workbench-image-namespace": Namespace,
+				}, ":some-tag"),
+
+				// there is no update to the Notebook
+				expectedImage: ":some-tag",
+				unexpectedEvents: []string{
+					IMAGE_STREAM_NOT_FOUND_EVENT,
+				},
+			},
 		}
 
 		BeforeEach(func() {
@@ -233,10 +296,15 @@ var _ = Describe("The Openshift Notebook webhook", func() {
 
 		for _, testCase := range testCases {
 			Context(fmt.Sprintf("The Notebook webhook test case: %s", testCase.name), func() {
+				BeforeEach(func() {
+					By("Creating the requisite ImageStream resources successfully")
+					for _, imageStream := range testCase.imageStreams {
+						Expect(cli.Create(ctx, imageStream, &client.CreateOptions{})).To(Succeed())
+					}
+				})
 				It("Should create a Notebook resource successfully", func() {
-					By("Creating a Notebook resource successfully")
-					Expect(cli.Create(ctx, testCase.imageStream, &client.CreateOptions{})).To(Succeed())
 					// if our webhook panics, then cli.Create will err
+					By("Creating a Notebook resource successfully")
 					Expect(cli.Create(ctx, testCase.notebook, &client.CreateOptions{})).To(Succeed())
 
 					By("Checking that the webhook modified the notebook CR with the expected image")
@@ -259,7 +327,9 @@ var _ = Describe("The Openshift Notebook webhook", func() {
 				AfterEach(func() {
 					By("Deleting the created resources")
 					Expect(cli.Delete(ctx, testCase.notebook, &client.DeleteOptions{})).To(Succeed())
-					Expect(cli.Delete(ctx, testCase.imageStream, &client.DeleteOptions{})).To(Succeed())
+					for _, imageStream := range testCase.imageStreams {
+						Expect(cli.Delete(ctx, imageStream, &client.DeleteOptions{})).To(Succeed())
+					}
 				})
 			})
 		}
@@ -268,8 +338,6 @@ var _ = Describe("The Openshift Notebook webhook", func() {
 
 func toMetav1Time(timeString string) metav1.Time {
 	parsedTime, err := time.Parse(time.RFC3339, timeString)
-	if err != nil {
-		return metav1.Time{}
-	}
+	Expect(err).ToNot(HaveOccurred())
 	return metav1.NewTime(parsedTime)
 }