From d8f106b836506a813f712c622aa3f6b2c09e1a07 Mon Sep 17 00:00:00 2001 From: Oleksii Orel Date: Wed, 5 Oct 2022 16:52:42 +0300 Subject: [PATCH] feat: handle gracefully when an invalid devfile is found in a git repository Signed-off-by: Oleksii Orel --- .../Factory/Steps/Apply/Devfile/index.tsx | 64 +++++++++++++++---- .../Steps/CheckExistingWorkspaces/index.tsx | 17 +++-- .../Factory/Steps/Fetch/Devfile/index.tsx | 62 ++++++++++++++++-- .../GetStartedTab/SamplesListGallery.tsx | 6 +- .../src/services/bootstrap/index.ts | 12 +++- .../src/store/DevfileRegistries/selectors.ts | 29 +++++++++ .../src/store/FactoryResolver/index.ts | 4 +- 7 files changed, 165 insertions(+), 29 deletions(-) diff --git a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx index 8c77eeff57..f05c1790b8 100644 --- a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx +++ b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx @@ -36,6 +36,9 @@ import { FactoryParams } from '../../../types'; import buildFactoryParams from '../../../buildFactoryParams'; import { AbstractLoaderStep, LoaderStepProps, LoaderStepState } from '../../../../AbstractStep'; import { AlertItem } from '../../../../../../services/helpers/types'; +import { selectDefaultDevfile } from '../../../../../../store/DevfileRegistries/selectors'; +import { generateWorkspaceName } from '../../../../../../services/helpers/generateName'; +import { getProjectName } from '../../../../../../services/helpers/getProjectName'; export type Props = MappedProps & LoaderStepProps & { @@ -48,6 +51,13 @@ export type State = LoaderStepState & { shouldCreate: boolean; // should the loader create a workspace }; +export class ApplyingDevfileError extends Error { + constructor(message: string) { + super(message); + this.name = 'ApplyingDevfileError'; + } +} + class StepApplyDevfile extends AbstractLoaderStep { protected readonly toDispose = new DisposableCollection(); @@ -126,12 +136,26 @@ class StepApplyDevfile extends AbstractLoaderStep { this.props.onRestart(); } + private updateCurrentDevfile(devfile): void { + const { factoryParams } = this.state; + const { factoryId, policiesCreate, storageType } = factoryParams; + // test the devfile name to decide if we need to append a suffix to is + const nameConflict = this.props.allWorkspaces.some(w => devfile.metadata.name === w.name); + const appendSuffix = policiesCreate === 'perclick' || nameConflict; + + const updatedDevfile = prepareDevfile(devfile, factoryId, storageType, appendSuffix); + + this.setState({ + devfile: updatedDevfile, + newWorkspaceName: updatedDevfile.metadata.name, + }); + } + protected async runStep(): Promise { await delay(MIN_STEP_DURATION_MS); - const { factoryResolverConverted } = this.props; + const { factoryResolverConverted, factoryResolver, defaultDevfile } = this.props; const { shouldCreate, factoryParams, devfile } = this.state; - const { factoryId, policiesCreate, storageType } = factoryParams; const workspace = this.findTargetWorkspace(this.props, this.state); if (workspace !== undefined) { @@ -150,21 +174,34 @@ class StepApplyDevfile extends AbstractLoaderStep { } if (devfile === undefined) { + if (factoryResolver === undefined) { + const _devfile = defaultDevfile; + if (_devfile === undefined) { + throw new Error('Failed to resolve the default devfile.'); + } + if (_devfile.projects === undefined) { + _devfile.projects = []; + } + if (_devfile.projects.length === 0) { + // adds a default project from the source URL + const sourceUrl = new URL(factoryParams.sourceUrl); + const name = getProjectName(factoryParams.sourceUrl); + const origin = sourceUrl.pathname.endsWith('.git') + ? `${sourceUrl.origin}${sourceUrl.pathname}` + : `${sourceUrl.origin}${sourceUrl.pathname}.git`; + _devfile.projects[0] = { git: { remotes: { origin } }, name }; + // change default name + _devfile.metadata.name = name; + _devfile.metadata.generateName = name; + } + this.updateCurrentDevfile(_devfile); + return false; + } const _devfile = factoryResolverConverted?.devfileV2; if (_devfile === undefined) { throw new Error('Failed to resolve the devfile.'); } - - // test the devfile name to decide if we need to append a suffix to is - const nameConflict = this.props.allWorkspaces.some(w => _devfile.metadata.name === w.name); - const appendSuffix = policiesCreate === 'perclick' || nameConflict; - - const updatedDevfile = prepareDevfile(_devfile, factoryId, storageType, appendSuffix); - - this.setState({ - devfile: updatedDevfile, - newWorkspaceName: updatedDevfile.metadata.name, - }); + this.updateCurrentDevfile(_devfile); return false; } @@ -249,6 +286,7 @@ const mapStateToProps = (state: AppState) => ({ defaultNamespace: selectDefaultNamespace(state), factoryResolver: selectFactoryResolver(state), factoryResolverConverted: selectFactoryResolverConverted(state), + defaultDevfile: selectDefaultDevfile(state), }); const connector = connect( diff --git a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx index 8ba137ae9c..7ab23385df 100644 --- a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx +++ b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx @@ -20,7 +20,10 @@ import { DisposableCollection } from '../../../../../services/helpers/disposable import { selectAllWorkspaces } from '../../../../../store/Workspaces/selectors'; import { delay } from '../../../../../services/helpers/delay'; import { FactoryLoaderPage } from '../../../../../pages/Loader/Factory'; -import { selectDevWorkspaceResources } from '../../../../../store/DevfileRegistries/selectors'; +import { + selectDevWorkspaceResources, + selectEmptyWorkspaceUrl, +} from '../../../../../store/DevfileRegistries/selectors'; import { buildIdeLoaderLocation } from '../../../../../services/helpers/location'; import { Workspace } from '../../../../../services/workspace-adapter'; import { FactoryParams } from '../../types'; @@ -147,13 +150,17 @@ class StepCheckExistingWorkspaces extends AbstractLoaderStep { let newWorkspaceName: string; if (prevLoaderStep.id === LoadingStep.CREATE_WORKSPACE__FETCH_DEVFILE) { if ( - factoryResolver?.location !== factoryParams.sourceUrl || - factoryResolverConverted?.devfileV2 === undefined + factoryResolver?.location === factoryParams.sourceUrl && + factoryResolverConverted?.devfileV2 !== undefined ) { + const devfile = factoryResolverConverted.devfileV2; + newWorkspaceName = devfile.metadata.name; + } else { + if (factoryResolver === undefined) { + return true; + } throw new Error('Failed to resolve the devfile.'); } - const devfile = factoryResolverConverted.devfileV2; - newWorkspaceName = devfile.metadata.name; } else { const resources = devWorkspaceResources[factoryParams.sourceUrl]?.resources; if (resources === undefined) { diff --git a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx index f014ad48ff..b31d59d1fb 100644 --- a/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx +++ b/packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx @@ -13,7 +13,7 @@ import React from 'react'; import { connect, ConnectedProps } from 'react-redux'; import { isEqual } from 'lodash'; -import { helpers } from '@eclipse-che/common'; +import common, { helpers } from '@eclipse-che/common'; import { AlertVariant } from '@patternfly/react-core'; import { AppState } from '../../../../../../store'; import * as FactoryResolverStore from '../../../../../../store/FactoryResolver'; @@ -37,6 +37,13 @@ import buildFactoryParams from '../../../buildFactoryParams'; import { AbstractLoaderStep, LoaderStepProps, LoaderStepState } from '../../../../AbstractStep'; import { AlertItem } from '../../../../../../services/helpers/types'; +export class ApplyingDevfileError extends Error { + constructor(message) { + super(message); + this.name = 'ApplyingDevfileError'; + } +} + const RELOADS_LIMIT = 2; type ReloadsInfo = { [url: string]: number; @@ -49,6 +56,7 @@ export type Props = MappedProps & export type State = LoaderStepState & { factoryParams: FactoryParams; shouldResolve: boolean; + useDefaultDevfile: boolean; }; class StepFetchDevfile extends AbstractLoaderStep { @@ -60,6 +68,7 @@ class StepFetchDevfile extends AbstractLoaderStep { this.state = { factoryParams: buildFactoryParams(props.searchParams), shouldResolve: true, + useDefaultDevfile: false, }; } @@ -106,9 +115,9 @@ class StepFetchDevfile extends AbstractLoaderStep { private init() { const { factoryResolver } = this.props; - const { factoryParams } = this.state; + const { factoryParams, useDefaultDevfile } = this.state; const { sourceUrl } = factoryParams; - if (sourceUrl && sourceUrl === factoryResolver?.location) { + if (sourceUrl && (useDefaultDevfile || sourceUrl === factoryResolver?.location)) { // prevent a resource being fetched one more time this.setState({ shouldResolve: false, @@ -129,7 +138,7 @@ class StepFetchDevfile extends AbstractLoaderStep { protected async runStep(): Promise { await delay(MIN_STEP_DURATION_MS); - const { factoryParams, shouldResolve } = this.state; + const { factoryParams, shouldResolve, useDefaultDevfile } = this.state; const { currentStepIndex, factoryResolver, factoryResolverConverted, loaderSteps } = this.props; const { sourceUrl } = factoryParams; @@ -151,14 +160,30 @@ class StepFetchDevfile extends AbstractLoaderStep { } if (shouldResolve === false) { + if (useDefaultDevfile) { + // go to the next step + return true; + } + if (this.state.lastError instanceof Error) { throw this.state.lastError; } throw new Error('Failed to resolve the devfile.'); } - // start resolving the devfile - const resolveDone = await this.resolveDevfile(sourceUrl); + let resolveDone = false; + try { + // start resolving the devfile + resolveDone = await this.resolveDevfile(sourceUrl); + } catch (e) { + const errorMessage = common.helpers.errors.getMessage(e); + if (errorMessage.startsWith('Error')) { + const message = `The Devfile in the git repository is invalid. If you continue it will be ignored and a regular workspace will be created. + You will have a chance to fix the Devfile from the remote IDE once the dev environment is started.`; + throw new ApplyingDevfileError(message); + } + throw e; + } if (resolveDone === false) { return false; } @@ -176,6 +201,13 @@ class StepFetchDevfile extends AbstractLoaderStep { } } + private async handleDevfileError(): Promise { + this.setState({ + useDefaultDevfile: true, + }); + this.clearStepError(); + } + /** * Resolves promise with `true` if devfile resolved successfully. Resolves promise with `false` if the devfile needs to be resolved one more time after authentication. */ @@ -272,6 +304,24 @@ class StepFetchDevfile extends AbstractLoaderStep { } private getAlertItem(error: unknown): AlertItem | undefined { + if (error instanceof ApplyingDevfileError) { + return { + key: 'factory-loader-devfile-error', + title: 'Warning', + variant: AlertVariant.warning, + children: helpers.errors.getMessage(error), + actionCallbacks: [ + { + title: 'Continue with the default devfile', + callback: () => this.handleDevfileError(), + }, + { + title: 'Reload', + callback: () => this.clearStepError(), + }, + ], + }; + } if (!error) { return; } diff --git a/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx b/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx index c1035b3ba9..398c77a50a 100644 --- a/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx +++ b/packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx @@ -31,7 +31,10 @@ import { AppState } from '../../../store'; import * as DevfileRegistriesStore from '../../../store/DevfileRegistries'; import { SampleCard } from './SampleCard'; import { AlertItem } from '../../../services/helpers/types'; -import { selectMetadataFiltered } from '../../../store/DevfileRegistries/selectors'; +import { + EMPTY_WORKSPACE_TAG, + selectMetadataFiltered, +} from '../../../store/DevfileRegistries/selectors'; import { selectWorkspacesSettings } from '../../../store/Workspaces/Settings/selectors'; import * as FactoryResolverStore from '../../../store/FactoryResolver'; import { isDevworkspacesEnabled } from '../../../services/helpers/devworkspace'; @@ -60,7 +63,6 @@ type State = { }; export const VISIBLE_TAGS = ['Community', 'Tech-Preview']; -export const EMPTY_WORKSPACE_TAG = 'Empty'; const EXCLUDED_TARGET_EDITOR_NAMES = ['dirigible', 'jupyter', 'eclipseide', 'code-server']; diff --git a/packages/dashboard-frontend/src/services/bootstrap/index.ts b/packages/dashboard-frontend/src/services/bootstrap/index.ts index b9c82e1b74..df3249b377 100644 --- a/packages/dashboard-frontend/src/services/bootstrap/index.ts +++ b/packages/dashboard-frontend/src/services/bootstrap/index.ts @@ -42,6 +42,7 @@ import { buildDetailsLocation, buildIdeLoaderLocation } from '../helpers/locatio import { Workspace } from '../workspace-adapter'; import { WorkspaceRunningError, WorkspaceStoppedDetector } from './workspaceStoppedDetector'; import { selectOpenVSXUrl } from '../../store/ServerConfig/selectors'; +import { selectEmptyWorkspaceUrl } from '../../store/DevfileRegistries/selectors'; /** * This class executes a few initial instructions @@ -88,7 +89,7 @@ export default class Bootstrap { this.fetchPlugins().then(() => this.fetchDevfileSchema()), this.fetchDwPlugins(), this.fetchDefaultDwPlugins(), - this.fetchRegistriesMetadata(), + this.fetchRegistriesMetadata().then(() => this.fetchEmptyWorkspace()), this.watchNamespaces(), this.updateDevWorkspaceTemplates(), this.fetchWorkspaces().then(() => this.checkWorkspaceStopped()), @@ -287,6 +288,15 @@ export default class Bootstrap { ); } + private async fetchEmptyWorkspace(): Promise { + const { requestDevfile } = DevfileRegistriesStore.actionCreators; + const state = this.store.getState(); + const emptyWorkspaceUrl = selectEmptyWorkspaceUrl(state); + if (emptyWorkspaceUrl) { + await requestDevfile(emptyWorkspaceUrl)(this.store.dispatch, this.store.getState, undefined); + } + } + private async fetchDevfileSchema(): Promise { const { requestJsonSchema } = DevfileRegistriesStore.actionCreators; return requestJsonSchema()(this.store.dispatch, this.store.getState, undefined); diff --git a/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts b/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts index 58c157ae81..1f24f2cfb6 100644 --- a/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts +++ b/packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts @@ -15,6 +15,10 @@ import { AppState } from '..'; import match from '../../services/helpers/filter'; import { selectWorkspacesSettingsState } from '../Workspaces/Settings/selectors'; import { isDevworkspacesEnabled } from '../../services/helpers/devworkspace'; +import { load } from 'js-yaml'; +import devfileApi from '../../services/devfileApi'; + +export const EMPTY_WORKSPACE_TAG = 'Empty'; const selectState = (state: AppState) => state.devfileRegistries; @@ -63,6 +67,31 @@ export const selectMetadataFiltered = createSelector( }, ); +export const selectEmptyWorkspaceUrl = createSelector( + selectState, + selectRegistriesMetadata, + (state, metadata) => { + const v2Metadata = filterDevfileV2Metadata(metadata); + const emptyWorkspaceMetadata = v2Metadata.find(meta => meta.tags.includes(EMPTY_WORKSPACE_TAG)); + return emptyWorkspaceMetadata?.links?.v2; + }, +); + +export const selectDefaultDevfile = createSelector( + selectState, + selectEmptyWorkspaceUrl, + (state, devfileLocation) => { + if (!devfileLocation) { + return undefined; + } + const devfileContent = state.devfiles[devfileLocation]?.content; + if (devfileContent) { + return load(devfileContent) as devfileApi.Devfile; + } + return undefined; + }, +); + function matches(meta: che.DevfileMetaData, filterValue: string): boolean { return match(meta.displayName, filterValue) || match(meta.description || '', filterValue); } diff --git a/packages/dashboard-frontend/src/store/FactoryResolver/index.ts b/packages/dashboard-frontend/src/store/FactoryResolver/index.ts index 8889857779..63589c4a6e 100644 --- a/packages/dashboard-frontend/src/store/FactoryResolver/index.ts +++ b/packages/dashboard-frontend/src/store/FactoryResolver/index.ts @@ -37,7 +37,7 @@ export type OAuthResponse = { oauth_authentication_url: string; }; errorCode: number; - message: string; + message: string | undefined; }; export function isOAuthResponse(responseData: any): responseData is OAuthResponse { @@ -80,7 +80,7 @@ interface ReceiveFactoryResolverAction { interface ReceiveFactoryResolverErrorAction { type: 'RECEIVE_FACTORY_RESOLVER_ERROR'; - error: string; + error: string | undefined; } export type KnownAction =