Deleting workbench resource makes workbench editable

opendatahub-io · Oct 24, 2024 · a1234bb · a1234bb
1 parent 3f5fe8d
commit a1234bb
Show file tree

Hide file tree

Showing 10 changed files with 414 additions and 30 deletions.
diff --git a/frontend/src/__mocks__/mockNotebookK8sResource.ts b/frontend/src/__mocks__/mockNotebookK8sResource.ts
@@ -4,14 +4,15 @@ import { DEFAULT_NOTEBOOK_SIZES } from '~/pages/projects/screens/spawner/const';
 import { ContainerResources, TolerationEffect, TolerationOperator, VolumeMount } from '~/types';
 import { genUID } from '~/__mocks__/mockUtils';
 import { RecursivePartial } from '~/typeHelpers';
+import { EnvironmentFromVariable } from '~/pages/projects/types';
 
 type MockResourceConfigType = {
   name?: string;
   displayName?: string;
   namespace?: string;
   user?: string;
   description?: string;
-  envFromName?: string;
+  envFrom?: EnvironmentFromVariable[];
   resources?: ContainerResources;
   image?: string;
   lastImageSelection?: string;
@@ -23,7 +24,14 @@ type MockResourceConfigType = {
 export const mockNotebookK8sResource = ({
   name = 'test-notebook',
   displayName = 'Test Notebook',
-  envFromName = 'aws-connection-db-1',
+  envFrom = [
+    {
+      secretRef: {
+        name: 'secret',
+      },
+    },
+  ],
+
   namespace = 'test-project',
   user = 'test-user',
   description = '',
@@ -99,13 +107,7 @@ export const mockNotebookK8sResource = ({
                       'image-registry.openshift-image-registry.svc:5000/opendatahub/code-server-notebook:2023.2',
                   },
                 ],
-                envFrom: [
-                  {
-                    secretRef: {
-                      name: envFromName,
-                    },
-                  },
-                ],
+                envFrom,
                 image,
                 imagePullPolicy: 'Always',
                 livenessProbe: {

diff --git a/frontend/src/__tests__/cypress/cypress/pages/workbench.ts b/frontend/src/__tests__/cypress/cypress/pages/workbench.ts
@@ -378,6 +378,10 @@ class EditSpawnerPage extends CreateSpawnerPage {
     return this;
   }
 
+  findAlertMessage() {
+    return cy.findByTestId('env-variable-alert-message');
+  }
+
   findCancelButton() {
     return cy.findByTestId('workbench-cancel-button');
   }

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/dataConnection.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/dataConnection.cy.ts
@@ -54,7 +54,7 @@ const initIntercepts = ({ isEmpty = false }: HandlersProps) => {
   cy.interceptK8sList(PodModel, mockK8sResourceList([mockPodK8sResource({})]));
   cy.interceptK8sList(
     NotebookModel,
-    mockK8sResourceList([mockNotebookK8sResource({ envFromName: '' })]),
+    mockK8sResourceList([mockNotebookK8sResource({ envFrom: [] })]),
   );
   cy.interceptK8sList(
     ProjectModel,
@@ -173,7 +173,9 @@ describe('Data connections', () => {
     editDataConnectionModal.findNotebookRestartAlert().should('exist');
     cy.interceptK8sList(
       NotebookModel,
-      mockK8sResourceList([mockNotebookK8sResource({ envFromName: 'test-secret' })]),
+      mockK8sResourceList([
+        mockNotebookK8sResource({ envFrom: [{ secretRef: { name: 'test-secret' } }] }),
+      ]),
     ).as('addConnectedWorkbench');
     cy.interceptK8s('PUT', SecretModel, mockSecretK8sResource({})).as('editDataConnection');
 

diff --git a/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts b/frontend/src/__tests__/cypress/cypress/tests/mocked/projects/workbench.cy.ts
@@ -1,4 +1,5 @@
 import {
+  mockCustomSecretK8sResource,
   mockDashboardConfig,
   mockDscStatus,
   mockK8sResourceList,
@@ -35,22 +36,25 @@ import {
   StorageClassModel,
   AcceleratorProfileModel,
 } from '~/__tests__/cypress/cypress/utils/models';
-import { mock200Status } from '~/__mocks__/mockK8sStatus';
+import { mock200Status, mock404Error } from '~/__mocks__/mockK8sStatus';
 import type { NotebookSize } from '~/types';
 import { mockAcceleratorProfile } from '~/__mocks__/mockAcceleratorProfile';
 import { mockConnectionTypeConfigMap } from '~/__mocks__/mockConnectionType';
 import type { PodKind } from '~/k8sTypes';
+import type { EnvironmentFromVariable } from '~/pages/projects/types';
 
 const configYamlPath = '../../__mocks__/mock-upload-configmap.yaml';
 
 type HandlersProps = {
   isEmpty?: boolean;
   notebookSizes?: NotebookSize[];
   mockPodList?: PodKind[];
+  envFrom?: EnvironmentFromVariable[];
 };
 
 const initIntercepts = ({
   isEmpty = false,
+  envFrom,
   notebookSizes = [
     {
       name: 'Medium',
@@ -139,6 +143,23 @@ const initIntercepts = ({
       }),
     ]),
   );
+  cy.interceptK8s(
+    { model: SecretModel, ns: 'test-project', name: 'secret' },
+    mockCustomSecretK8sResource({
+      name: 'secret',
+      namespace: 'test-project',
+      data: { test: 'c2RzZA==' },
+    }),
+  );
+  cy.interceptK8s(
+    'PUT',
+    { model: SecretModel, ns: 'test-project', name: 'secret' },
+    mockCustomSecretK8sResource({
+      name: 'secret',
+      namespace: 'test-project',
+      data: { test: 'c2RzZA==' },
+    }),
+  );
   cy.interceptK8s(SecretModel, mockSecretK8sResource({ name: 'aws-connection-db-1' }));
   cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('stopWorkbench');
   cy.interceptK8s(RouteModel, mockRouteK8sResource({ notebookName: 'test-notebook' }));
@@ -152,6 +173,7 @@ const initIntercepts = ({
         ? []
         : [
             mockNotebookK8sResource({
+              envFrom,
               opts: {
                 metadata: {
                   name: 'test-notebook',
@@ -667,6 +689,7 @@ describe('Workbench page', () => {
       mockK8sResourceList([mockPVCK8sResource({ name: 'test-notebook' })]),
     );
     editSpawnerPage.visit('test-notebook');
+    editSpawnerPage.findAlertMessage().should('not.exist');
     editSpawnerPage.k8sNameDescription.findDisplayNameInput().should('have.value', 'Test Notebook');
     editSpawnerPage.shouldHaveNotebookImageSelectInput('Test Image');
     editSpawnerPage.shouldHaveContainerSizeInput('Small');
@@ -696,6 +719,19 @@ describe('Workbench page', () => {
         spec: {
           template: {
             spec: {
+              containers: [
+                {
+                  envFrom: [
+                    {
+                      secretRef: {
+                        name: 'secret',
+                      },
+                    },
+                  ],
+
+                  name: 'test-notebook',
+                },
+              ],
               volumes: [
                 { name: 'test-notebook', persistentVolumeClaim: { claimName: 'test-notebook' } },
               ],
@@ -710,6 +746,157 @@ describe('Workbench page', () => {
     });
   });
 
+  it('Edit workbench when either configMap or secret variables not present', () => {
+    initIntercepts({
+      envFrom: [
+        {
+          secretRef: {
+            name: 'secret-1',
+          },
+        },
+        {
+          secretRef: {
+            name: 'secret-2',
+          },
+        },
+      ],
+    });
+    cy.interceptK8s(
+      {
+        model: SecretModel,
+        ns: 'test-project',
+        name: 'secret-1',
+      },
+      {
+        statusCode: 404,
+        body: mock404Error({}),
+      },
+    );
+    cy.interceptK8s(
+      {
+        model: SecretModel,
+        ns: 'test-project',
+        name: 'secret-2',
+      },
+      {
+        statusCode: 404,
+        body: mock404Error({}),
+      },
+    );
+    editSpawnerPage.visit('test-notebook');
+    editSpawnerPage.findAlertMessage().should('exist');
+    editSpawnerPage.findAlertMessage().contains('secret-1 and secret-2');
+    cy.interceptK8s('PUT', NotebookModel, mockNotebookK8sResource({})).as('editWorkbenchDryRun');
+    cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('editWorkbench');
+    editSpawnerPage.findSubmitButton().click();
+    cy.wait('@editWorkbenchDryRun').then((interception) => {
+      expect(interception.request.url).to.include('?dryRun=All');
+      expect(interception.request.body).to.containSubset({
+        metadata: {
+          annotations: {
+            'openshift.io/description': '',
+            'openshift.io/display-name': 'Test Notebook',
+            'opendatahub.io/image-display-name': 'Test Image',
+            'opendatahub.io/accelerator-name': '',
+          },
+        },
+        spec: {
+          template: {
+            spec: {
+              containers: [
+                {
+                  envFrom: [],
+
+                  name: 'test-notebook',
+                },
+              ],
+            },
+          },
+        },
+      });
+    });
+    // Actual request
+    cy.wait('@editWorkbench').then((interception) => {
+      expect(interception.request.url).not.to.include('?dryRun=All');
+    });
+  });
+
+  it('Edit workbench when both configMap and secret are deleted', () => {
+    initIntercepts({
+      envFrom: [
+        {
+          secretRef: {
+            name: 'secret-1',
+          },
+        },
+        {
+          configMapRef: {
+            name: 'secret-2',
+          },
+        },
+      ],
+    });
+    cy.interceptK8s(
+      {
+        model: SecretModel,
+        ns: 'test-project',
+        name: 'secret-1',
+      },
+      {
+        statusCode: 404,
+        body: mock404Error({}),
+      },
+    );
+    cy.interceptK8s(
+      {
+        model: ConfigMapModel,
+        ns: 'test-project',
+        name: 'secret-2',
+      },
+      {
+        statusCode: 404,
+        body: mock404Error({}),
+      },
+    );
+    editSpawnerPage.visit('test-notebook');
+    editSpawnerPage.findAlertMessage().should('exist');
+    editSpawnerPage.findAlertMessage().contains('secret-1 secret');
+    editSpawnerPage.findAlertMessage().contains('secret-2 config map');
+    cy.interceptK8s('PUT', NotebookModel, mockNotebookK8sResource({})).as('editWorkbenchDryRun');
+    cy.interceptK8s('PATCH', NotebookModel, mockNotebookK8sResource({})).as('editWorkbench');
+    editSpawnerPage.findSubmitButton().click();
+    cy.wait('@editWorkbenchDryRun').then((interception) => {
+      expect(interception.request.url).to.include('?dryRun=All');
+      expect(interception.request.body).to.containSubset({
+        metadata: {
+          annotations: {
+            'openshift.io/description': '',
+            'openshift.io/display-name': 'Test Notebook',
+            'opendatahub.io/image-display-name': 'Test Image',
+            'opendatahub.io/accelerator-name': '',
+          },
+        },
+        spec: {
+          template: {
+            spec: {
+              containers: [
+                {
+                  envFrom: [],
+
+                  name: 'test-notebook',
+                },
+              ],
+            },
+          },
+        },
+      });
+    });
+    // Actual request
+    cy.wait('@editWorkbench').then((interception) => {
+      expect(interception.request.url).not.to.include('?dryRun=All');
+    });
+  });
+
   it('Handle deleted notebook sizes in workbenches table', () => {
     initIntercepts({
       notebookSizes: [
@@ -761,7 +948,42 @@ describe('Workbench page', () => {
   });
 
   it('Delete Workbench', () => {
-    initIntercepts({});
+    initIntercepts({
+      envFrom: [
+        {
+          secretRef: {
+            name: 'secret-1',
+          },
+        },
+        {
+          configMapRef: {
+            name: 'secret-2',
+          },
+        },
+      ],
+    });
+    cy.interceptK8s(
+      {
+        model: SecretModel,
+        ns: 'test-project',
+        name: 'secret-1',
+      },
+      {
+        statusCode: 404,
+        body: mock404Error({}),
+      },
+    );
+    cy.interceptK8s(
+      {
+        model: ConfigMapModel,
+        ns: 'test-project',
+        name: 'secret-2',
+      },
+      {
+        statusCode: 404,
+        body: mock404Error({}),
+      },
+    );
     workbenchPage.visit('test-project');
     const notebookRow = workbenchPage.getNotebookRow('Test Notebook');
     notebookRow.findKebabAction('Delete workbench').click();

diff --git a/frontend/src/api/k8s/__tests__/notebooks.spec.ts b/frontend/src/api/k8s/__tests__/notebooks.spec.ts
@@ -908,7 +908,7 @@ describe('removeNotebookSecret', () => {
         {
           op: 'replace',
           path: '/spec/template/spec/containers/0/envFrom',
-          value: [{ secretRef: { name: 'aws-connection-db-1' } }],
+          value: [{ secretRef: { name: 'secret' } }],
         },
       ],
       queryOptions: { name, ns: namespace },
@@ -982,7 +982,7 @@ describe('removeNotebookSecret', () => {
         {
           op: 'replace',
           path: '/spec/template/spec/containers/0/envFrom',
-          value: [{ secretRef: { name: 'aws-connection-db-1' } }],
+          value: [{ secretRef: { name: 'secret' } }],
         },
       ],
       queryOptions: { name, ns: namespace },