fix: Fix repeated 403 due to app namespace being undefined (argoproj#…

…20699) (argoproj#20819) Fixes argoproj#20699 Constructor may not get called every time the application changes, so previous this.appNamespace could be stale. But the update to use `this.props.match.params.appnamespace` could also fail if it's undefined. As a fix, create and use a helper function `getAppNamespace` which has a special case handling for undefined. Also, use a namespaced endpoint when namespace is not undefined. It needs to be cherry-picked to v2.11-2.13. Signed-off-by: Andrii Korotkov <andrii.korotkov@verkada.com>
crenshaw-dev · Nov 20, 2024 · 3da5a3d · 3da5a3d
1 parent 83953fe
commit 3da5a3d
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 19 deletions.
diff --git a/ui/src/app/applications/components/application-details/application-details-app-dropdown.tsx b/ui/src/app/applications/components/application-details/application-details-app-dropdown.tsx
@@ -3,6 +3,7 @@ import * as React from 'react';
 
 import {Context} from '../../../shared/context';
 import {services} from '../../../shared/services';
+import {getAppUrl} from '../utils';
 
 export const ApplicationsDetailsAppDropdown = (props: {appName: string}) => {
     const [opened, setOpened] = React.useState(false);
@@ -42,7 +43,7 @@ export const ApplicationsDetailsAppDropdown = (props: {appName: string}) => {
                                 })
                                 .slice(0, 100) // take top 100 results after filtering to avoid performance issues
                                 .map(app => (
-                                    <li key={app.metadata.name} onClick={() => ctx.navigation.goto(`/applications/${app.metadata.namespace}/${app.metadata.name}`)}>
+                                    <li key={app.metadata.name} onClick={() => ctx.navigation.goto(getAppUrl(app))}>
                                         {app.metadata.name} {app.metadata.name === props.appName && ' (current)'}
                                     </li>
                                 ))

diff --git a/ui/src/app/applications/components/application-details/application-details.tsx b/ui/src/app/applications/components/application-details/application-details.tsx
@@ -82,7 +82,6 @@ export class ApplicationDetails extends React.Component<RouteComponentProps<{app
     };
 
     private appChanged = new BehaviorSubject<appModels.Application>(null);
-    private appNamespace: string;
 
     constructor(props: RouteComponentProps<{appnamespace: string; name: string}>) {
         super(props);
@@ -95,11 +94,6 @@ export class ApplicationDetails extends React.Component<RouteComponentProps<{app
             collapsedNodes: [],
             ...this.getExtensionsState()
         };
-        if (typeof this.props.match.params.appnamespace === 'undefined') {
-            this.appNamespace = '';
-        } else {
-            this.appNamespace = this.props.match.params.appnamespace;
-        }
     }
 
     public componentDidMount() {
@@ -116,6 +110,13 @@ export class ApplicationDetails extends React.Component<RouteComponentProps<{app
         services.extensions.removeEventListener('topBar', this.onExtensionsUpdate);
     }
 
+    private getAppNamespace() {
+        if (typeof this.props.match.params.appnamespace === 'undefined') {
+            return '';
+        }
+        return this.props.match.params.appnamespace;
+    }
+
     private onExtensionsUpdate = () => {
         this.setState({...this.state, ...this.getExtensionsState()});
     };
@@ -426,7 +427,7 @@ export class ApplicationDetails extends React.Component<RouteComponentProps<{app
                         loadingRenderer={() => <Page title='Application Details'>Loading...</Page>}
                         input={this.props.match.params.name}
                         load={name =>
-                            combineLatest([this.loadAppInfo(name, this.props.match.params.appnamespace), services.viewPreferences.getPreferences(), q]).pipe(
+                            combineLatest([this.loadAppInfo(name, this.getAppNamespace()), services.viewPreferences.getPreferences(), q]).pipe(
                                 map(items => {
                                     const application = items[0].application;
                                     const pref = items[1].appDetails;
@@ -1188,8 +1189,8 @@ Are you sure you want to disable auto-sync and rollback application '${this.prop
                     update.spec.syncPolicy = {automated: null};
                     await services.applications.update(update);
                 }
-                await services.applications.rollback(this.props.match.params.name, this.appNamespace, revisionHistory.id);
-                this.appChanged.next(await services.applications.get(this.props.match.params.name, this.appNamespace));
+                await services.applications.rollback(this.props.match.params.name, this.getAppNamespace(), revisionHistory.id);
+                this.appChanged.next(await services.applications.get(this.props.match.params.name, this.getAppNamespace()));
                 this.setRollbackPanelVisible(-1);
             }
         } catch (e) {
@@ -1205,7 +1206,7 @@ Are you sure you want to disable auto-sync and rollback application '${this.prop
     }
 
     private async deleteApplication() {
-        await AppUtils.deleteApplication(this.props.match.params.name, this.appNamespace, this.appContext.apis);
+        await AppUtils.deleteApplication(this.props.match.params.name, this.getAppNamespace(), this.appContext.apis);
     }
 
     private async confirmDeletion(app: appModels.Application, title: string, message: string) {

diff --git a/ui/src/app/applications/components/applications-list/applications-table.tsx b/ui/src/app/applications/components/applications-list/applications-table.tsx
@@ -37,7 +37,7 @@ export const ApplicationsTable = (props: {
         keys: Key.ENTER,
         action: () => {
             if (selectedApp > -1) {
-                ctxh.navigation.goto(`/applications/${props.applications[selectedApp].metadata.name}`);
+                ctxh.navigation.goto(AppUtils.getAppUrl(props.applications[selectedApp]));
                 return true;
             }
             return false;
@@ -57,9 +57,7 @@ export const ApplicationsTable = (props: {
                                         key={AppUtils.appInstanceName(app)}
                                         className={`argo-table-list__row
                 applications-list__entry applications-list__entry--health-${app.status.health.status} ${selectedApp === i ? 'applications-tiles__selected' : ''}`}>
-                                        <div
-                                            className={`row applications-list__table-row`}
-                                            onClick={e => ctx.navigation.goto(`applications/${app.metadata.namespace}/${app.metadata.name}`, {}, {event: e})}>
+                                        <div className={`row applications-list__table-row`} onClick={e => ctx.navigation.goto(AppUtils.getAppUrl(app), {}, {event: e})}>
                                             <div className='columns small-4'>
                                                 <div className='row'>
                                                     <div className=' columns small-2'>

diff --git a/ui/src/app/applications/components/applications-list/applications-tiles.tsx b/ui/src/app/applications/components/applications-list/applications-tiles.tsx
@@ -66,7 +66,7 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
         keys: Key.ENTER,
         action: () => {
             if (selectedApp > -1) {
-                ctxh.navigation.goto(`/applications/${applications[selectedApp].metadata.name}`);
+                ctxh.navigation.goto(AppUtils.getAppUrl(applications[selectedApp]));
                 return true;
             }
             return false;
@@ -118,9 +118,7 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
                                             }`}>
                                             <div
                                                 className='row applications-tiles__wrapper'
-                                                onClick={e =>
-                                                    ctx.navigation.goto(`applications/${app.metadata.namespace}/${app.metadata.name}`, {view: pref.appDetails.view}, {event: e})
-                                                }>
+                                                onClick={e => ctx.navigation.goto(AppUtils.getAppUrl(app), {view: pref.appDetails.view}, {event: e})}>
                                                 <div
                                                     className={`columns small-12 applications-list__info qe-applications-list-${AppUtils.appInstanceName(
                                                         app

diff --git a/ui/src/app/applications/components/utils.tsx b/ui/src/app/applications/components/utils.tsx
@@ -1471,3 +1471,10 @@ export const userMsgsList: {[key: string]: string} = {
     groupNodes: `Since the number of pods has surpassed the threshold pod count of 15, you will now be switched to the group node view.
                  If you prefer the tree view, you can simply click on the Group Nodes toolbar button to deselect the current view.`
 };
+
+export function getAppUrl(app: appModels.Application): string {
+    if (typeof app.metadata.namespace === 'undefined') {
+        return `/applications/${app.metadata.name}`;
+    }
+    return `/applications/${app.metadata.namespace}/${app.metadata.name}`;
+}