Skip to content

Commit

Permalink
fix(ui): source can in fact be undefined (#20381) (#20418)
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 <blake.pettersson@gmail.com>
Co-authored-by: Blake Pettersson <blake.pettersson@gmail.com>
  • Loading branch information
@gcp-cherry-pick-bot @blakepettersson
gcp-cherry-pick-bot[bot] and blakepettersson authored Oct 17, 2024
1 parent 97a49a2 commit 262c8fa
Show file tree
Hide file tree
Showing 9 changed files with 56 additions and 56 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 @@ -919,18 +919,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 @@ -644,23 +644,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
48 changes: 24 additions & 24 deletions ui/src/app/applications/components/application-parameters/source-panel.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,37 +104,37 @@ export const SourcePanel = (props: {
}
});
}
if (a.spec.source.repoURL && a.spec.source.chart) {
if (a.spec?.source?.repoURL && a.spec?.source?.chart) {
props.appCurrent.spec.sources.forEach(source => {
if (
source.repoURL === a.spec.source.repoURL &&
source.chart === a.spec.source.chart &&
source.targetRevision === a.spec.source.targetRevision
source?.repoURL === a.spec?.source?.repoURL &&
source?.chart === a.spec?.source?.chart &&
source?.targetRevision === a.spec?.source?.targetRevision
) {
sameChartVersion = true;
chartError =
'Version ' +
source.targetRevision +
source?.targetRevision +
' of chart ' +
source.chart +
source?.chart +
' from the selected repository was already added to this multi-source application';
}
});
}
if (!samePath) {
if (!a.spec.source.path && !a.spec.source.chart && !a.spec.source.ref) {
if (!a.spec?.source?.path && !a.spec?.source?.chart && !a.spec?.source?.ref) {
pathError = 'Path or Ref is required';
}
}
if (!sameChartVersion) {
if (!a.spec.source.chart && !a.spec.source.path && !a.spec.source.ref) {
if (!a.spec?.source?.chart && !a.spec?.source?.path && !a.spec?.source?.ref) {
chartError = 'Chart is required';
}
}
return {
'spec.source.repoURL': !a.spec.source.repoURL && 'Repository URL is required',
'spec.source.repoURL': !a.spec?.source?.repoURL && 'Repository URL is required',
// eslint-disable-next-line no-prototype-builtins
'spec.source.targetRevision': !a.spec.source.targetRevision && a.spec.source.hasOwnProperty('chart') && 'Version is required',
'spec.source.targetRevision': !a.spec?.source?.targetRevision && a.spec?.source?.hasOwnProperty('chart') && 'Version is required',
'spec.source.path': pathError,
'spec.source.chart': chartError
};
Expand All @@ -157,8 +157,8 @@ export const SourcePanel = (props: {
getApi={props.getFormApi}>
{api => {
// eslint-disable-next-line no-prototype-builtins
const repoType = (api.getFormState().values.spec.source.hasOwnProperty('chart') && 'helm') || 'git';
const repoInfo = reposInfo.find(info => info.repo === api.getFormState().values.spec.source.repoURL);
const repoType = (api.getFormState().values.spec?.source?.hasOwnProperty('chart') && 'helm') || 'git';
const repoInfo = reposInfo.find(info => info.repo === api.getFormState().values.spec?.source?.repoURL);
if (repoInfo) {
normalizeAppSource(appInEdit, repoInfo.type || 'git');
}
Expand Down Expand Up @@ -206,12 +206,12 @@ export const SourcePanel = (props: {
</div>
{(repoType === 'git' && (
<React.Fragment>
<RevisionFormField formApi={api} helpIconTop={'2.5em'} repoURL={api.getFormState().values.spec.source.repoURL} />
<RevisionFormField formApi={api} helpIconTop={'2.5em'} repoURL={api.getFormState().values.spec?.source?.repoURL} />
<div className='argo-form-row'>
<DataLoader
input={{
repoURL: api.getFormState().values.spec.source.repoURL,
revision: api.getFormState().values.spec.source.targetRevision
repoURL: api.getFormState().values.spec?.source?.repoURL,
revision: api.getFormState().values.spec?.source?.targetRevision
}}
load={async src =>
(src.repoURL &&
Expand Down Expand Up @@ -247,7 +247,7 @@ export const SourcePanel = (props: {
new Array<models.HelmChart>()
}>
{(charts: models.HelmChart[]) => {
const selectedChart = charts.find(chart => chart.name === api.getFormState().values.spec.source.chart);
const selectedChart = charts.find(chart => chart.name === api.getFormState().values.spec?.source?.chart);
return (
<div className='row argo-form-row'>
<div className='columns small-10'>
Expand Down Expand Up @@ -284,15 +284,15 @@ export const SourcePanel = (props: {
const typePanel = () => (
<DataLoader
input={{
repoURL: appInEdit.spec.source.repoURL,
path: appInEdit.spec.source.path,
chart: appInEdit.spec.source.chart,
targetRevision: appInEdit.spec.source.targetRevision,
repoURL: appInEdit.spec?.source?.repoURL,
path: appInEdit.spec?.source?.path,
chart: appInEdit.spec?.source?.chart,
targetRevision: appInEdit.spec?.source?.targetRevision,
appName: appInEdit.metadata.name
}}
load={async src => {
if (src.repoURL && src.targetRevision && (src.path || src.chart)) {
return services.repos.appDetails(src, src.appName, props.appCurrent.spec.project, 0, 0).catch(() => ({
if (src?.repoURL && src?.targetRevision && (src?.path || src?.chart)) {
return services.repos.appDetails(src, src?.appName, props.appCurrent.spec?.project, 0, 0).catch(() => ({
type: 'Directory',
details: {}
}));
Expand All @@ -304,7 +304,7 @@ export const SourcePanel = (props: {
}
}}>
{(details: models.RepoAppDetails) => {
const type = (explicitPathType && explicitPathType.path === appInEdit.spec.source.path && explicitPathType.type) || details.type;
const type = (explicitPathType && explicitPathType.path === appInEdit.spec?.source?.path && explicitPathType.type) || details.type;
if (details.type !== type) {
switch (type) {
case 'Helm':
Expand Down Expand Up @@ -337,7 +337,7 @@ export const SourcePanel = (props: {
items={appTypes.map(item => ({
title: item.type,
action: () => {
setExplicitPathType({type: item.type, path: appInEdit.spec.source.path});
setExplicitPathType({type: item.type, path: appInEdit.spec?.source?.path});
normalizeTypeFields(api, item.type);
}
}))}
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
Loading

0 comments on commit 262c8fa

Please sign in to comment.