Skip to content

Commit

Permalink
fix: Fix repeated 403 due to app namespace being undefined (argoproj#…
Browse files Browse the repository at this point in the history 
…20699)

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>
  • Loading branch information
@andrii-korotkov-verkada
andrii-korotkov-verkada committed Nov 18, 2024
1 parent 6d8d32f commit d9d7110
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 15 deletions.
9 changes: 8 additions & 1 deletion ui/src/app/applications/components/application-details/application-details-app-dropdown.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,14 @@ 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}`)}>
// Use not namespaced endpoint if namespace is undefined
<li key={app.metadata.name} onClick={() =>
ctx.navigation.goto(
typeof app.metadata.namespace !== 'undefined'
? `/applications/${app.metadata.namespace}/${app.metadata.name}`
: `/applications/${app.metadata.name}`
)
}>
{app.metadata.name} {app.metadata.name === props.appName && ' (current)'}
</li>
))
Expand Down
22 changes: 12 additions & 10 deletions ui/src/app/applications/components/application-details/application-details.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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() {
Expand All @@ -116,6 +110,14 @@ 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 '';
} else {
return this.props.match.params.appnamespace;
}
}

private onExtensionsUpdate = () => {
this.setState({...this.state, ...this.getExtensionsState()});
};
Expand Down Expand Up @@ -426,7 +428,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;
Expand Down Expand Up @@ -1188,8 +1190,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) {
Expand All @@ -1205,7 +1207,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) {
Expand Down
21 changes: 19 additions & 2 deletions ui/src/app/applications/components/applications-list/applications-table.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,14 @@ export const ApplicationsTable = (props: {
keys: Key.ENTER,
action: () => {
if (selectedApp > -1) {
ctxh.navigation.goto(`/applications/${props.applications[selectedApp].metadata.name}`);
let targetUrl;
if (typeof props.applications[selectedApp].metadata.namespace !== 'undefined') {
targetUrl = `/applications/${props.applications[selectedApp].metadata.namespace}/${props.applications[selectedApp].metadata.name}`;
} else {
targetUrl = `/applications/${props.applications[selectedApp].metadata.name}`;
}
// Use not namespaced endpoint if namespace is undefined
ctxh.navigation.goto(targetUrl);
return true;
}
return false;
Expand All @@ -59,7 +66,17 @@ export const ApplicationsTable = (props: {
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})}>
// Use not namespaced endpoint if namespace is undefined
onClick={e =>
ctx.navigation.goto(
typeof app.metadata.namespace !== 'undefined'
? `/applications/${app.metadata.namespace}/${app.metadata.name}`
: `/applications/${app.metadata.name}`,
{},
{event: e}
)
}
>
<div className='columns small-4'>
<div className='row'>
<div className=' columns small-2'>
Expand Down
18 changes: 16 additions & 2 deletions ui/src/app/applications/components/applications-list/applications-tiles.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
keys: Key.ENTER,
action: () => {
if (selectedApp > -1) {
ctxh.navigation.goto(`/applications/${applications[selectedApp].metadata.name}`);
let targetUrl;
// Use not namespaced endpoint if namespace is undefined
if (typeof applications[selectedApp].metadata.namespace !== 'undefined') {
targetUrl = `/applications/${applications[selectedApp].metadata.namespace}/${applications[selectedApp].metadata.name}`;
} else {
targetUrl = `/applications/${applications[selectedApp].metadata.name}`;
}
ctxh.navigation.goto(targetUrl);
return true;
}
return false;
Expand Down Expand Up @@ -118,8 +125,15 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
}`}>
<div
className='row applications-tiles__wrapper'
// Use not namespaced endpoint if namespace is undefined
onClick={e =>
ctx.navigation.goto(`applications/${app.metadata.namespace}/${app.metadata.name}`, {view: pref.appDetails.view}, {event: e})
ctx.navigation.goto(
typeof app.metadata.namespace !== 'undefined'
? `/applications/${app.metadata.namespace}/${app.metadata.name}`
: `/applications/${app.metadata.name}`,
{view: pref.appDetails.view},
{event: e}
)
}>
<div
className={`columns small-12 applications-list__info qe-applications-list-${AppUtils.appInstanceName(
Expand Down

0 comments on commit d9d7110

Please sign in to comment.