From 68606c6caf058f7c4f2ecdbb4381d2cba40aa249 Mon Sep 17 00:00:00 2001 From: "gcp-cherry-pick-bot[bot]" <98988430+gcp-cherry-pick-bot[bot]@users.noreply.github.com> Date: Wed, 20 Nov 2024 13:13:11 +0200 Subject: [PATCH] fix: Fix repeated 403 due to app namespace being undefined (#20699) (#20819) (#20860) Fixes #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 Co-authored-by: Andrii Korotkov <137232734+andrii-korotkov-verkada@users.noreply.github.com> --- .../application-details-app-dropdown.tsx | 3 ++- .../application-details.tsx | 21 ++++++++++--------- .../applications-list/applications-table.tsx | 6 ++---- .../applications-list/applications-tiles.tsx | 6 ++---- ui/src/app/applications/components/utils.tsx | 7 +++++++ 5 files changed, 24 insertions(+), 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 index 905785765540c..615ddf1a47511 100644 --- 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 => ( -
  • ctx.navigation.goto(`/applications/${app.metadata.namespace}/${app.metadata.name}`)}> +
  • ctx.navigation.goto(getAppUrl(app))}> {app.metadata.name} {app.metadata.name === props.appName && ' (current)'}
  • )) diff --git a/ui/src/app/applications/components/application-details/application-details.tsx b/ui/src/app/applications/components/application-details/application-details.tsx index 51c9d30c03106..28db4ca5aa1f6 100644 --- 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(null); - private appNamespace: string; constructor(props: RouteComponentProps<{appnamespace: string; name: string}>) { super(props); @@ -95,11 +94,6 @@ export class ApplicationDetails extends React.Component { this.setState({...this.state, ...this.getExtensionsState()}); }; @@ -426,7 +427,7 @@ export class ApplicationDetails extends React.Component Loading...} 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; @@ -1170,8 +1171,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) { @@ -1187,7 +1188,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); } } diff --git a/ui/src/app/applications/components/applications-list/applications-table.tsx b/ui/src/app/applications/components/applications-list/applications-table.tsx index 4952a0f08fb9a..b90f940a79480 100644 --- 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' : ''}`}> -
    ctx.navigation.goto(`applications/${app.metadata.namespace}/${app.metadata.name}`, {}, {event: e})}> +
    ctx.navigation.goto(AppUtils.getAppUrl(app), {}, {event: e})}>
    diff --git a/ui/src/app/applications/components/applications-list/applications-tiles.tsx b/ui/src/app/applications/components/applications-list/applications-tiles.tsx index fb6b1f158bd4a..ccbbccf75ae10 100644 --- 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 }`}>
    - 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})}>