Skip to content

Commit

Permalink
feat: handle gracefully when an invalid devfile is found in a git rep…
Browse files Browse the repository at this point in the history 
…ository

Signed-off-by: Oleksii Orel <[email protected]>
  • Loading branch information
@olexii4
olexii4 committed Oct 5, 2022
1 parent dcee2f7 commit d8f106b
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 29 deletions.
64 changes: 51 additions & 13 deletions packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Apply/Devfile/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 & {
Expand All @@ -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<Props, State> {
protected readonly toDispose = new DisposableCollection();

Expand Down Expand Up @@ -126,12 +136,26 @@ class StepApplyDevfile extends AbstractLoaderStep<Props, State> {
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<boolean> {
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) {
Expand All @@ -150,21 +174,34 @@ class StepApplyDevfile extends AbstractLoaderStep<Props, State> {
}

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;
}

Expand Down Expand Up @@ -249,6 +286,7 @@ const mapStateToProps = (state: AppState) => ({
defaultNamespace: selectDefaultNamespace(state),
factoryResolver: selectFactoryResolver(state),
factoryResolverConverted: selectFactoryResolverConverted(state),
defaultDevfile: selectDefaultDevfile(state),
});

const connector = connect(
Expand Down
17 changes: 12 additions & 5 deletions .../dashboard-frontend/src/containers/Loader/Factory/Steps/CheckExistingWorkspaces/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -147,13 +150,17 @@ class StepCheckExistingWorkspaces extends AbstractLoaderStep<Props, State> {
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) {
Expand Down
62 changes: 56 additions & 6 deletions packages/dashboard-frontend/src/containers/Loader/Factory/Steps/Fetch/Devfile/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand All @@ -49,6 +56,7 @@ export type Props = MappedProps &
export type State = LoaderStepState & {
factoryParams: FactoryParams;
shouldResolve: boolean;
useDefaultDevfile: boolean;
};

class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
Expand All @@ -60,6 +68,7 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
this.state = {
factoryParams: buildFactoryParams(props.searchParams),
shouldResolve: true,
useDefaultDevfile: false,
};
}

Expand Down Expand Up @@ -106,9 +115,9 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {

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,
Expand All @@ -129,7 +138,7 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
protected async runStep(): Promise<boolean> {
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;

Expand All @@ -151,14 +160,30 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
}

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;
}
Expand All @@ -176,6 +201,13 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
}
}

private async handleDevfileError(): Promise<void> {
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.
*/
Expand Down Expand Up @@ -272,6 +304,24 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
}

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;
}
Expand Down
6 changes: 4 additions & 2 deletions packages/dashboard-frontend/src/pages/GetStarted/GetStartedTab/SamplesListGallery.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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'];

Expand Down
12 changes: 11 additions & 1 deletion packages/dashboard-frontend/src/services/bootstrap/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()),
Expand Down Expand Up @@ -287,6 +288,15 @@ export default class Bootstrap {
);
}

private async fetchEmptyWorkspace(): Promise<void> {
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<void> {
const { requestJsonSchema } = DevfileRegistriesStore.actionCreators;
return requestJsonSchema()(this.store.dispatch, this.store.getState, undefined);
Expand Down
29 changes: 29 additions & 0 deletions packages/dashboard-frontend/src/store/DevfileRegistries/selectors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/dashboard-frontend/src/store/FactoryResolver/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -80,7 +80,7 @@ interface ReceiveFactoryResolverAction {

interface ReceiveFactoryResolverErrorAction {
type: 'RECEIVE_FACTORY_RESOLVER_ERROR';
error: string;
error: string | undefined;
}

export type KnownAction =
Expand Down

0 comments on commit d8f106b

Please sign in to comment.