From 01d0519ccd69d0cbd411096000a6ad23a46ecab2 Mon Sep 17 00:00:00 2001
From: Gage Krumbach <gkrumbach@gmail.com>
Date: Thu, 7 Nov 2024 15:13:52 -0600
Subject: [PATCH] fix: prevent deletion of active connection secrets when
 updating notebook environment variables (#3445)

---
 .../projects/screens/spawner/SpawnerFooter.tsx      |  1 +
 .../pages/projects/screens/spawner/SpawnerPage.tsx  |  9 ++++++---
 .../useNotebookEnvVariables.tsx                     |  2 ++
 .../screens/spawner/environmentVariables/utils.ts   | 13 +++++++++++--
 .../src/pages/projects/screens/spawner/service.ts   |  6 ++++++
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx
index 54ed3f874f..2c26fe6175 100644
--- a/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx
+++ b/frontend/src/pages/projects/screens/spawner/SpawnerFooter.tsx
@@ -154,6 +154,7 @@ const SpawnerFooter: React.FC<SpawnerFooterProps> = ({
       envVariables,
       dataConnection,
       existingNotebookDataConnection,
+      connections,
       dryRun,
     );
 
diff --git a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx
index 76cdc2b597..a4b847d9fb 100644
--- a/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx
+++ b/frontend/src/pages/projects/screens/spawner/SpawnerPage.tsx
@@ -88,9 +88,6 @@ const SpawnerPage: React.FC<SpawnerPageProps> = ({ existingNotebook }) => {
     defaultStorageClassName,
   );
 
-  const [envVariables, setEnvVariables, envVariablesLoaded, deletedConfigMaps, deletedSecrets] =
-    useNotebookEnvVariables(existingNotebook);
-
   const [dataConnectionData, setDataConnectionData] = useNotebookDataConnection(
     dataConnections.data,
     existingNotebook,
@@ -103,6 +100,12 @@ const SpawnerPage: React.FC<SpawnerPageProps> = ({ existingNotebook }) => {
       : [],
   );
 
+  const [envVariables, setEnvVariables, envVariablesLoaded, deletedConfigMaps, deletedSecrets] =
+    useNotebookEnvVariables(existingNotebook, [
+      ...notebookConnections.map((connection) => connection.metadata.name),
+      dataConnectionData.existing?.secretRef.name || '',
+    ]);
+
   const restartNotebooks = useWillNotebooksRestart([existingNotebook?.metadata.name || '']);
 
   const [data, loaded, loadError] = useNotebookImageData(existingNotebook);
diff --git a/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx b/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx
index 700dd5af17..124bf77205 100644
--- a/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx
+++ b/frontend/src/pages/projects/screens/spawner/environmentVariables/useNotebookEnvVariables.tsx
@@ -75,6 +75,7 @@ export const fetchNotebookEnvVariables = (notebook: NotebookKind): Promise<EnvVa
 
 export const useNotebookEnvVariables = (
   notebook?: NotebookKind,
+  excludedResources: string[] = [],
 ): [
   envVariables: EnvVariable[],
   setEnvVariables: (envVars: EnvVariable[]) => void,
@@ -96,6 +97,7 @@ export const useNotebookEnvVariables = (
   const { deletedConfigMaps, deletedSecrets } = getDeletedConfigMapOrSecretVariables(
     notebook,
     existingEnvVariables,
+    excludedResources,
   );
   React.useEffect(() => {
     setEnvVariables(existingEnvVariables);
diff --git a/frontend/src/pages/projects/screens/spawner/environmentVariables/utils.ts b/frontend/src/pages/projects/screens/spawner/environmentVariables/utils.ts
index 6f404ce4f9..3de7163107 100644
--- a/frontend/src/pages/projects/screens/spawner/environmentVariables/utils.ts
+++ b/frontend/src/pages/projects/screens/spawner/environmentVariables/utils.ts
@@ -23,6 +23,7 @@ export const isStringKeyValuePairObject = (object: unknown): object is Record<st
 export const getDeletedConfigMapOrSecretVariables = (
   notebook: NotebookKind | undefined,
   envVariables: EnvVariable[],
+  excludedResources: string[] = [],
 ): {
   deletedSecrets: string[];
   deletedConfigMaps: string[];
@@ -32,10 +33,18 @@ export const getDeletedConfigMapOrSecretVariables = (
   const deletedSecrets: string[] = [];
 
   (notebook?.spec.template.spec.containers[0].envFrom || []).forEach((env) => {
-    if (env.configMapRef && !existingEnvVariables.has(env.configMapRef.name)) {
+    if (
+      env.configMapRef &&
+      !existingEnvVariables.has(env.configMapRef.name) &&
+      !excludedResources.includes(env.configMapRef.name)
+    ) {
       deletedConfigMaps.push(env.configMapRef.name);
     }
-    if (env.secretRef && !existingEnvVariables.has(env.secretRef.name)) {
+    if (
+      env.secretRef &&
+      !existingEnvVariables.has(env.secretRef.name) &&
+      !excludedResources.includes(env.secretRef.name)
+    ) {
       deletedSecrets.push(env.secretRef.name);
     }
   });
diff --git a/frontend/src/pages/projects/screens/spawner/service.ts b/frontend/src/pages/projects/screens/spawner/service.ts
index 8bcf97fa86..f92dbe3f7c 100644
--- a/frontend/src/pages/projects/screens/spawner/service.ts
+++ b/frontend/src/pages/projects/screens/spawner/service.ts
@@ -24,6 +24,7 @@ import {
 } from '~/pages/projects/types';
 import { ROOT_MOUNT_PATH } from '~/pages/projects/pvc/const';
 import { ConfigMapKind, NotebookKind, SecretKind } from '~/k8sTypes';
+import { Connection } from '~/concepts/connectionTypes/types';
 import { getVolumesByStorageData } from './spawnerUtils';
 import { fetchNotebookEnvVariables } from './environmentVariables/useNotebookEnvVariables';
 import { getDeletedConfigMapOrSecretVariables } from './environmentVariables/utils';
@@ -194,12 +195,17 @@ export const updateConfigMapsAndSecretsForNotebook = async (
   envVariables: EnvVariable[],
   dataConnection?: DataConnectionData,
   existingDataConnection?: DataConnection,
+  connections?: Connection[],
   dryRun = false,
 ): Promise<EnvironmentFromVariable[]> => {
   const existingEnvVars = await fetchNotebookEnvVariables(notebook);
   const { deletedConfigMaps, deletedSecrets } = getDeletedConfigMapOrSecretVariables(
     notebook,
     existingEnvVars,
+    [
+      existingDataConnection?.data.metadata.name || '',
+      ...(connections || []).map((connection) => connection.metadata.name),
+    ],
   );
   const newDataConnection =
     dataConnection?.enabled && dataConnection.type === 'creating' && dataConnection.creating