frontend: Prevent success pop-up on invalid operations

Signed-off-by: Evangelos Skopelitis <eskopelitis@microsoft.com>
headlamp-k8s · Jul 15, 2024 · d66061d · d66061d
1 parent 6d5b487
commit d66061d
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 31 deletions.
diff --git a/frontend/src/components/common/Resource/CreateButton.tsx b/frontend/src/components/common/Resource/CreateButton.tsx
@@ -30,10 +30,12 @@ export default function CreateButton(props: CreateButtonProps) {
   const dispatchCreateEvent = useEventCallback(HeadlampEventType.CREATE_RESOURCE);
 
   const applyFunc = async (newItems: KubeObjectInterface[], clusterName: string) => {
+    let success = true;
     await Promise.allSettled(newItems.map(newItem => apply(newItem, clusterName))).then(
       (values: any) => {
         values.forEach((value: any, index: number) => {
           if (value.status === 'rejected') {
+            success = false;
             let msg;
             const kind = newItems[index].kind;
             const name = newItems[index].metadata.name;
@@ -54,6 +56,7 @@ export default function CreateButton(props: CreateButtonProps) {
         });
       }
     );
+    return success;
   };
 
   function handleSave(newItemDefs: KubeObjectInterface[]) {
@@ -84,26 +87,29 @@ export default function CreateButton(props: CreateButtonProps) {
 
     const clusterName = getCluster() || '';
 
-    dispatch(
-      clusterAction(() => applyFunc(massagedNewItemDefs, clusterName), {
-        startMessage: t('translation|Applying {{ newItemName }}…', {
-          newItemName: resourceNames.join(','),
-        }),
-        cancelledMessage: t('translation|Cancelled applying {{ newItemName }}.', {
-          newItemName: resourceNames.join(','),
-        }),
-        successMessage: t('translation|Applied {{ newItemName }}.', {
-          newItemName: resourceNames.join(','),
-        }),
-        errorMessage: t('translation|Failed to apply {{ newItemName }}.', {
-          newItemName: resourceNames.join(','),
-        }),
-        cancelUrl,
-      })
-    );
-
-    dispatchCreateEvent({
-      status: EventStatus.CONFIRMED,
+    applyFunc(massagedNewItemDefs, clusterName).then(success => {
+      if (success) {
+        dispatch(
+          clusterAction(() => Promise.resolve(), {
+            startMessage: t('translation|Applying {{ newItemName }}…', {
+              newItemName: resourceNames.join(','),
+            }),
+            cancelledMessage: t('translation|Cancelled applying {{ newItemName }}.', {
+              newItemName: resourceNames.join(','),
+            }),
+            successMessage: t('translation|Applied {{ newItemName }}.', {
+              newItemName: resourceNames.join(','),
+            }),
+            errorMessage: t('translation|Failed to apply {{ newItemName }}.', {
+              newItemName: resourceNames.join(','),
+            }),
+            cancelUrl,
+          })
+        );
+        dispatchCreateEvent({
+          status: EventStatus.CONFIRMED,
+        });
+      }
     });
   }
 

diff --git a/frontend/src/lib/k8s/apiProxy.test.ts b/frontend/src/lib/k8s/apiProxy.test.ts
@@ -675,11 +675,15 @@ describe('apiProxy', () => {
     });
 
     it('Successfully creates a new resource with POST', async () => {
+      const mockConfigMapWithoutResourceVersion = {
+        ...mockConfigMap,
+        metadata: { ...mockConfigMap.metadata, resourceVersion: undefined },
+      };
       nock(baseApiUrl)
         .post(`/clusters/${clusterName}/api/v1/namespaces/${namespace}/configmaps`)
         .reply(201, mockConfigMap);
 
-      const response = await apiProxy.apply(mockConfigMap);
+      const response = await apiProxy.apply(mockConfigMapWithoutResourceVersion);
       expect(response).toEqual(mockConfigMap);
     });
 
@@ -715,7 +719,7 @@ describe('apiProxy', () => {
     it('Successfully assigns default namespace if not given', async () => {
       const configMapWithoutNamespace = {
         ...mockConfigMap,
-        metadata: { ...mockConfigMap.metadata, namespace: undefined },
+        metadata: { ...mockConfigMap.metadata, namespace: undefined, resourceVersion: undefined },
       };
 
       nock(baseApiUrl)

diff --git a/frontend/src/lib/k8s/apiProxy.ts b/frontend/src/lib/k8s/apiProxy.ts
@@ -1549,16 +1549,18 @@ export async function apply(body: KubeObjectInterface, clusterName?: string): Pr
 
   try {
     delete bodyToApply.metadata.resourceVersion;
-    return await apiEndpoint.post(bodyToApply, {}, cluster);
+
+    if (!resourceVersion) {
+      // Try to create the resource
+      return await apiEndpoint.post(bodyToApply, {}, cluster);
+    } else {
+      // Try to update the resource if resourceVersion is provided
+      bodyToApply.metadata.resourceVersion = resourceVersion;
+      return await apiEndpoint.put(bodyToApply, {}, cluster);
+    }
   } catch (err) {
-    // Check to see if failed because the record already exists.
-    // If the failure isn't a 409 (i.e. Confilct), just rethrow.
-    if ((err as ApiError).status !== 409) throw err;
-
-    // Preserve the resourceVersion if its an update request
-    bodyToApply.metadata.resourceVersion = resourceVersion;
-    // We had a conflict. Try a PUT
-    return apiEndpoint.put(bodyToApply, {}, cluster);
+    console.error(`Error applying the resource ${bodyToApply}: ${err}`);
+    throw err;
   }
 }