Skip to content

Commit

Permalink
fix: Fix repeated 403 due to app namespace being undefined (#20699) (#…
Browse files Browse the repository at this point in the history
…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 <andrii.korotkov@verkada.com>
Co-authored-by: Andrii Korotkov <137232734+andrii-korotkov-verkada@users.noreply.github.com>
gcp-cherry-pick-bot[bot] and andrii-korotkov-verkada authored Nov 20, 2024

Verified

This commit was signed with the committer’s verified signature.
jbedard Jason Bedard
1 parent 6a8cb6e commit 68606c6
Showing 5 changed files with 24 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -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>
))
Original file line number Diff line number Diff line change
@@ -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;
@@ -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);
}
}

Original file line number Diff line number Diff line change
@@ -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'>
Original file line number Diff line number Diff line change
@@ -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
7 changes: 7 additions & 0 deletions ui/src/app/applications/components/utils.tsx
Original file line number Diff line number Diff line change
@@ -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}`;
}

0 comments on commit 68606c6

Please sign in to comment.