Skip to content

Commit

Permalink
fix(ui): source can in fact be undefined (#20381) (#20420)
Browse files Browse the repository at this point in the history 
* fix(ui): source can in fact be `undefined`

The assumption that a source is always there is not always true. To
repro, create an app-of-apps containing a single app without any `source`
present. In the UI this will crash, horribly. This PR fixes that so
that instead of crashing the user will get useful info indicating what
is wrong with the app.



* chore(ui): some cr tweaks



* chore(ui): some cr tweaks



---------

Signed-off-by: Blake Pettersson <[email protected]>
  • Loading branch information
@blakepettersson
blakepettersson authored Oct 17, 2024
1 parent 5f5fb0d commit 6e6857e
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 32 deletions.
2 changes: 1 addition & 1 deletion ui/src/app/applications/components/application-deployment-history/revision-metadata-rows.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {ApplicationSource, RevisionMetadata, ChartDetails} from '../../../shared
import {services} from '../../../shared/services';

export const RevisionMetadataRows = (props: {applicationName: string; applicationNamespace: string; source: ApplicationSource; index: number; versionId: number}) => {
if (props.source.chart) {
if (props?.source?.chart) {
return (
<DataLoader
input={props}
Expand Down
8 changes: 5 additions & 3 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 @@ -890,18 +890,20 @@ export class ApplicationDetails extends React.Component<RouteComponentProps<{app
{
iconClassName: 'fa fa-info-circle',
title: <ActionMenuItem actionLabel='Details' />,
action: () => this.selectNode(fullName)
action: () => this.selectNode(fullName),
disabled: !app.spec.source
},
{
iconClassName: 'fa fa-file-medical',
title: <ActionMenuItem actionLabel='Diff' />,
action: () => this.selectNode(fullName, 0, 'diff'),
disabled: app.status.sync.status === appModels.SyncStatuses.Synced
disabled: app.status.sync.status === appModels.SyncStatuses.Synced || !app.spec.source
},
{
iconClassName: 'fa fa-sync',
title: <ActionMenuItem actionLabel='Sync' />,
action: () => AppUtils.showDeploy('all', null, this.appContext.apis)
action: () => AppUtils.showDeploy('all', null, this.appContext.apis),
disabled: !app.spec.source
},
{
iconClassName: 'fa fa-info-circle',
Expand Down
11 changes: 6 additions & 5 deletions ui/src/app/applications/components/application-parameters/application-parameters.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -556,23 +556,24 @@ function gatherCoreSourceDetails(i: number, attributes: EditablePanelItem[], sou
)
});
} else {
const targetRevision = source ? source.targetRevision || 'HEAD' : 'Unknown';
attributes.push({
title: 'TARGET REVISION',
view: <Revision repoUrl={source.repoURL} revision={source.targetRevision || 'HEAD'} />,
edit: (formApi: FormApi) => <RevisionFormField helpIconTop={'0'} hideLabel={true} formApi={formApi} repoURL={source.repoURL} fieldValue={revisionField} />
view: <Revision repoUrl={source?.repoURL} revision={targetRevision} />,
edit: (formApi: FormApi) => <RevisionFormField helpIconTop={'0'} hideLabel={true} formApi={formApi} repoURL={source?.repoURL} fieldValue={revisionField} />
});
attributes.push({
title: 'PATH',
view: (
<Revision repoUrl={source.repoURL} revision={source.targetRevision || 'HEAD'} path={source.path} isForPath={true}>
{processPath(source.path)}
<Revision repoUrl={source?.repoURL} revision={targetRevision} path={source?.path} isForPath={true}>
{processPath(source?.path)}
</Revision>
),
edit: (formApi: FormApi) => <FormField formApi={formApi} field={sourcesPathField} component={Text} />
});
attributes.push({
title: 'REF',
view: <span>{source.ref}</span>,
view: <span>{source?.ref}</span>,
edit: (formApi: FormApi) => <FormField formApi={formApi} field={refField} component={Text} />
});
}
Expand Down
4 changes: 2 additions & 2 deletions ui/src/app/applications/components/application-status-panel/application-status-panel.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export const ApplicationStatusPanel = ({application, showDiff, showOperation, sh
application.status.sync &&
(hasMultipleSources
? application.status.sync.revisions && application.status.sync.revisions[0] && application.spec.sources && !application.spec.sources[0].chart
: application.status.sync.revision && !application.spec.source.chart) && (
: application.status.sync.revision && !application.spec?.source?.chart) && (
<div className='application-status-panel__item-name'>
<RevisionMetadataPanel
appName={application.metadata.name}
Expand Down Expand Up @@ -160,7 +160,7 @@ export const ApplicationStatusPanel = ({application, showDiff, showOperation, sh
<RevisionMetadataPanel
appName={application.metadata.name}
appNamespace={application.metadata.namespace}
type={source.chart && 'helm'}
type={source?.chart && 'helm'}
revision={operationStateRevision}
versionId={utils.getAppCurrentVersion(application)}
/>
Expand Down
8 changes: 2 additions & 6 deletions ui/src/app/applications/components/application-summary/application-summary.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,19 +172,15 @@ export const ApplicationSummary = (props: ApplicationSummaryProps) => {
},
!hasMultipleSources && {
title: 'REPO URL',
view: <Repo url={source.repoURL} />,
view: <Repo url={source?.repoURL} />,
edit: (formApi: FormApi) => <FormField formApi={formApi} field='spec.source.repoURL' component={Text} />
},
...(!hasMultipleSources
? isHelm
? [
{
title: 'CHART',
view: (
<span>
{source.chart}:{source.targetRevision}
</span>
),
view: <span>{source && `${source.chart}:${source.targetRevision}`}</span>,
edit: (formApi: FormApi) =>
hasMultipleSources ? (
helpTip('CHART is not editable for applications with multiple sources. You can edit them in the "Manifest" tab.')
Expand Down
2 changes: 1 addition & 1 deletion ui/src/app/applications/components/applications-list/applications-source.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {ApplicationSource as ApplicationSourceType} from '../../../shared/models
import './applications-source.scss';

export const ApplicationsSource = ({source}: {source: ApplicationSourceType}) => {
const sourceString = `${source.repoURL}/${source.path || source.chart}`;
const sourceString = source ? `${source.repoURL}/${source.path || source.chart}` : '';
return (
<Tooltip content={sourceString}>
<div className='application-source'>{sourceString}</div>
Expand Down
17 changes: 9 additions & 8 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 @@ -108,6 +108,7 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
<div className='applications-tiles argo-table-list argo-table-list--clickable' ref={appContainerRef}>
{applications.map((app, i) => {
const source = getAppDefaultSource(app);
const targetRevision = source ? source.targetRevision || 'HEAD' : 'Unknown';
return (
<div
key={AppUtils.appInstanceName(app)}
Expand All @@ -126,7 +127,7 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
)} applications-tiles__item`}>
<div className='row '>
<div className={app.status.summary.externalURLs?.length > 0 ? 'columns small-10' : 'columns small-11'}>
<i className={'icon argo-icon-' + (source.chart != null ? 'helm' : 'git')} />
<i className={'icon argo-icon-' + (source?.chart != null ? 'helm' : 'git')} />
<Tooltip content={AppUtils.appInstanceName(app)}>
<span className='applications-list__title'>
{AppUtils.appQualifiedName(app, useAuthSettingsCtx?.appsInAnyNamespaceEnabled)}
Expand Down Expand Up @@ -208,31 +209,31 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
Repository:
</div>
<div className='columns small-9'>
<Tooltip content={source.repoURL} zIndex={4}>
<span>{source.repoURL}</span>
<Tooltip content={source?.repoURL} zIndex={4}>
<span>{source?.repoURL}</span>
</Tooltip>
</div>
</div>
<div className='row'>
<div className='columns small-3' title='Target Revision:'>
Target Revision:
</div>
<div className='columns small-9'>{source.targetRevision || 'HEAD'}</div>
<div className='columns small-9'>{targetRevision}</div>
</div>
{source.path && (
{source?.path && (
<div className='row'>
<div className='columns small-3' title='Path:'>
Path:
</div>
<div className='columns small-9'>{source.path}</div>
<div className='columns small-9'>{source?.path}</div>
</div>
)}
{source.chart && (
{source?.chart && (
<div className='row'>
<div className='columns small-3' title='Chart:'>
Chart:
</div>
<div className='columns small-9'>{source.chart}</div>
<div className='columns small-9'>{source?.chart}</div>
</div>
)}
<div className='row'>
Expand Down
12 changes: 6 additions & 6 deletions ui/src/app/applications/components/utils.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -706,10 +706,10 @@ export function renderResourceButtons(
export function syncStatusMessage(app: appModels.Application) {
const source = getAppDefaultSource(app);
const revision = getAppDefaultSyncRevision(app);
const rev = app.status.sync.revision || source.targetRevision || 'HEAD';
let message = source.targetRevision || 'HEAD';
const rev = app.status.sync.revision || (source ? source.targetRevision || 'HEAD' : 'Unknown');
let message = source ? source?.targetRevision || 'HEAD' : 'Unknown';

if (revision) {
if (revision && source) {
if (source.chart) {
message += ' (' + revision + ')';
} else if (revision.length >= 7 && !revision.startsWith(source.targetRevision)) {
Expand Down Expand Up @@ -1080,7 +1080,7 @@ export const getPodReadinessGatesState = (pod: appModels.State): {nonExistingCon
for (const condition of podStatusConditions) {
existingConditions.set(condition.type, true);
// priority order of conditions
// eg. if there are multiple conditions set with same name then the one which comes first is evaluated
// e.g. if there are multiple conditions set with same name then the one which comes first is evaluated
if (podConditions.has(condition.type)) {
continue;
}
Expand Down Expand Up @@ -1127,10 +1127,10 @@ export function isAppNode(node: appModels.ResourceNode) {

export function getAppOverridesCount(app: appModels.Application) {
const source = getAppDefaultSource(app);
if (source.kustomize && source.kustomize.images) {
if (source?.kustomize?.images) {
return source.kustomize.images.length;
}
if (source.helm && source.helm.parameters) {
if (source?.helm?.parameters) {
return source.helm.parameters.length;
}
return 0;
Expand Down

0 comments on commit 6e6857e

Please sign in to comment.