From ed4c0ee7138da579a4392824b0632a7056a1d106 Mon Sep 17 00:00:00 2001 From: Blake Pettersson Date: Wed, 16 Oct 2024 15:55:17 -1000 Subject: [PATCH] fix(ui): source can in fact be `undefined` (#20381) * 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. Signed-off-by: Blake Pettersson * chore(ui): some cr tweaks Signed-off-by: Blake Pettersson * chore(ui): some cr tweaks Signed-off-by: Blake Pettersson --------- Signed-off-by: Blake Pettersson --- .../revision-metadata-rows.tsx | 2 +- .../application-details.tsx | 8 ++-- .../application-parameters.tsx | 11 +++-- .../application-parameters/source-panel.tsx | 48 +++++++++---------- .../application-status-panel.tsx | 4 +- .../application-summary.tsx | 8 +--- .../applications-list/applications-source.tsx | 2 +- .../applications-list/applications-tiles.tsx | 17 +++---- ui/src/app/applications/components/utils.tsx | 12 ++--- 9 files changed, 56 insertions(+), 56 deletions(-) diff --git a/ui/src/app/applications/components/application-deployment-history/revision-metadata-rows.tsx b/ui/src/app/applications/components/application-deployment-history/revision-metadata-rows.tsx index 1043d0bfa5659..be141a3b879c4 100644 --- a/ui/src/app/applications/components/application-deployment-history/revision-metadata-rows.tsx +++ b/ui/src/app/applications/components/application-deployment-history/revision-metadata-rows.tsx @@ -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 ( , - action: () => this.selectNode(fullName) + action: () => this.selectNode(fullName), + disabled: !app.spec.source }, { iconClassName: 'fa fa-file-medical', title: , 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: , - 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', diff --git a/ui/src/app/applications/components/application-parameters/application-parameters.tsx b/ui/src/app/applications/components/application-parameters/application-parameters.tsx index ec6d67dae46a1..041880f007928 100644 --- a/ui/src/app/applications/components/application-parameters/application-parameters.tsx +++ b/ui/src/app/applications/components/application-parameters/application-parameters.tsx @@ -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: , - edit: (formApi: FormApi) => + view: , + edit: (formApi: FormApi) => }); attributes.push({ title: 'PATH', view: ( - - {processPath(source.path)} + + {processPath(source?.path)} ), edit: (formApi: FormApi) => }); attributes.push({ title: 'REF', - view: {source.ref}, + view: {source?.ref}, edit: (formApi: FormApi) => }); } diff --git a/ui/src/app/applications/components/application-parameters/source-panel.tsx b/ui/src/app/applications/components/application-parameters/source-panel.tsx index 8e750b6e4a9b9..c5d6ca4050291 100644 --- a/ui/src/app/applications/components/application-parameters/source-panel.tsx +++ b/ui/src/app/applications/components/application-parameters/source-panel.tsx @@ -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 }; @@ -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'); } @@ -206,12 +206,12 @@ export const SourcePanel = (props: { {(repoType === 'git' && ( - +
(src.repoURL && @@ -247,7 +247,7 @@ export const SourcePanel = (props: { new Array() }> {(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 (
@@ -284,15 +284,15 @@ export const SourcePanel = (props: { const typePanel = () => ( { - 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: {} })); @@ -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': @@ -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); } }))} diff --git a/ui/src/app/applications/components/application-status-panel/application-status-panel.tsx b/ui/src/app/applications/components/application-status-panel/application-status-panel.tsx index 643e24034d54a..ee76418546a4e 100644 --- a/ui/src/app/applications/components/application-status-panel/application-status-panel.tsx +++ b/ui/src/app/applications/components/application-status-panel/application-status-panel.tsx @@ -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) && (
diff --git a/ui/src/app/applications/components/application-summary/application-summary.tsx b/ui/src/app/applications/components/application-summary/application-summary.tsx index 174c66c5dc196..6dbd5081806ae 100644 --- a/ui/src/app/applications/components/application-summary/application-summary.tsx +++ b/ui/src/app/applications/components/application-summary/application-summary.tsx @@ -172,7 +172,7 @@ export const ApplicationSummary = (props: ApplicationSummaryProps) => { }, !hasMultipleSources && { title: 'REPO URL', - view: , + view: , edit: (formApi: FormApi) => }, ...(!hasMultipleSources @@ -180,11 +180,7 @@ export const ApplicationSummary = (props: ApplicationSummaryProps) => { ? [ { title: 'CHART', - view: ( - - {source.chart}:{source.targetRevision} - - ), + view: {source && `${source.chart}:${source.targetRevision}`}, edit: (formApi: FormApi) => hasMultipleSources ? ( helpTip('CHART is not editable for applications with multiple sources. You can edit them in the "Manifest" tab.') diff --git a/ui/src/app/applications/components/applications-list/applications-source.tsx b/ui/src/app/applications/components/applications-list/applications-source.tsx index 0a5fbe51f37c0..d0fe9d096444a 100644 --- a/ui/src/app/applications/components/applications-list/applications-source.tsx +++ b/ui/src/app/applications/components/applications-list/applications-source.tsx @@ -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 (
{sourceString}
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 ce27238b87d60..fb6b1f158bd4a 100644 --- a/ui/src/app/applications/components/applications-list/applications-tiles.tsx +++ b/ui/src/app/applications/components/applications-list/applications-tiles.tsx @@ -108,6 +108,7 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
{applications.map((app, i) => { const source = getAppDefaultSource(app); + const targetRevision = source ? source.targetRevision || 'HEAD' : 'Unknown'; return (
0 ? 'columns small-10' : 'columns small-11'}> - + {AppUtils.appQualifiedName(app, useAuthSettingsCtx?.appsInAnyNamespaceEnabled)} @@ -208,8 +209,8 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat Repository:
- - {source.repoURL} + + {source?.repoURL}
@@ -217,22 +218,22 @@ export const ApplicationTiles = ({applications, syncApplication, refreshApplicat
Target Revision:
-
{source.targetRevision || 'HEAD'}
+
{targetRevision}
- {source.path && ( + {source?.path && (
Path:
-
{source.path}
+
{source?.path}
)} - {source.chart && ( + {source?.chart && (
Chart:
-
{source.chart}
+
{source?.chart}
)}
diff --git a/ui/src/app/applications/components/utils.tsx b/ui/src/app/applications/components/utils.tsx index 14b31b5edeafd..e92939112ee1b 100644 --- a/ui/src/app/applications/components/utils.tsx +++ b/ui/src/app/applications/components/utils.tsx @@ -740,10 +740,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)) { @@ -1114,7 +1114,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; } @@ -1161,10 +1161,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;