fix: prevent deletion of active connection secrets when updating note…

…book environment variables (#3445)
opendatahub-io · Nov 7, 2024 · 01d0519 · 01d0519
1 parent 91584ea
commit 01d0519
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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