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 14, 2022
1 parent 003c5ea commit d46d1bd
Show file tree
Hide file tree
Showing 18 changed files with 959 additions and 40 deletions.
10 changes: 5 additions & 5 deletions .deps/prod.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@
| `@eclipse-che/[email protected]` | EPL-2.0 | ecd.che |
| [`@eclipse-che/[email protected]`](git+https://github.com/che-incubator/che-code.git) | EPL-2.0 | ecd.che |
| [`@eclipse-che/[email protected]`](git+https://github.com/eclipse-che/che-theia.git) | EPL-2.0 | ecd.che |
| [`@eclipse-che/common@7.54.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che |
| [`@eclipse-che/dashboard-backend@7.54.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che |
| [`@eclipse-che/dashboard-frontend@7.54.0-next`](git://github.com/eclipse/che-dashboard.git) | EPL-2.0 | ecd.che |
| [`@eclipse-che/common@7.56.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che |
| [`@eclipse-che/dashboard-backend@7.56.0-next`](https://github.com/eclipse-che/che-dashboard) | EPL-2.0 | ecd.che |
| [`@eclipse-che/dashboard-frontend@7.56.0-next`](git://github.com/eclipse/che-dashboard.git) | EPL-2.0 | ecd.che |
| [`@eclipse-che/[email protected]`](git+https://github.com/che-incubator/devfile-converter.git) | EPL-2.0 | ecd.che |
| [`@eclipse-che/[email protected]1632305737`](https://github.com/eclipse/che-workspace-client) | EPL-2.0 | ecd.che |
| [`@eclipse-che/[email protected]1663851810`](https://github.com/eclipse/che-workspace-client) | EPL-2.0 | ecd.che |
| [`@fastify/[email protected]`](git+https://github.com/fastify/ajv-compiler.git) | MIT | clearlydefined |
| [`@fastify/[email protected]`](git+https://github.com/fastify/fastify-cors.git) | MIT | clearlydefined |
| [`@fastify/[email protected]`](git+https://github.com/fastify/fastify-error.git) | MIT | clearlydefined |
Expand Down Expand Up @@ -324,7 +324,7 @@
| [`[email protected]`](git+ssh://[email protected]/pinojs/pino-std-serializers.git) | MIT | clearlydefined |
| [`[email protected]`](git+https://github.com/pinojs/pino.git) | MIT | clearlydefined |
| [`[email protected]`](git+https://github.com/FezVrasta/popper.js.git) | MIT | [CQ22353](https://dev.eclipse.org/ipzilla/show_bug.cgi?id=22353) |
| [`[email protected]`](https://github.com/postcss/postcss.git) | MIT | clearlydefined |
| [`[email protected]`](https://github.com/postcss/postcss.git) | MIT | #3545 |
| [`[email protected]`](https://github.com/prettier/prettier.git) | MIT | #1523 |
| [`[email protected]`](git://github.com/benjamn/private.git) | MIT | clearlydefined |
| [`[email protected]`](git+https://github.com/fastify/processs-warning.git) | MIT | clearlydefined |
Expand Down
2 changes: 1 addition & 1 deletion packages/dashboard-frontend/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"process": "^0.11.10",
"qs": "^6.9.4",
"react": "^16.14.0",
"react-copy-to-clipboard": "^5.0.2",
"react-copy-to-clipboard": "^5.1.0",
"react-dom": "^16.12.0",
"react-helmet": "^6.1.0",
"react-pluralize": "^1.6.3",
Expand Down
111 changes: 95 additions & 16 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 @@ -14,7 +14,7 @@ import React from 'react';
import { connect, ConnectedProps } from 'react-redux';
import { isEqual } from 'lodash';
import { AlertVariant } from '@patternfly/react-core';
import { helpers } from '@eclipse-che/common';
import common, { helpers } from '@eclipse-che/common';
import { AppState } from '../../../../../../store';
import * as WorkspacesStore from '../../../../../../store/Workspaces';
import { DisposableCollection } from '../../../../../../services/helpers/disposable';
Expand All @@ -36,6 +36,10 @@ 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 { getProjectName } from '../../../../../../services/helpers/getProjectName';
import CreateWorkspaceErrorItems from '../../../../../../pages/Loader/Factory/CreateWorkspaceErrorItems';
import { isEqual } from 'lodash';

export type Props = MappedProps &
LoaderStepProps & {
Expand All @@ -48,6 +52,13 @@ export type State = LoaderStepState & {
shouldCreate: boolean; // should the loader create a workspace
};

export class CreateWorkspaceError extends Error {
constructor(message: string) {
super(message);
this.name = 'CreateWorkspaceError';
}
}

class StepApplyDevfile extends AbstractLoaderStep<Props, State> {
protected readonly toDispose = new DisposableCollection();

Expand Down Expand Up @@ -126,12 +137,45 @@ class StepApplyDevfile extends AbstractLoaderStep<Props, State> {
this.props.onRestart();
}

private updateCurrentDevfile(devfile: devfileApi.Devfile): void {
const { factoryResolver, allWorkspaces, defaultDevfile } = this.props;
const { factoryParams } = this.state;
const { factoryId, policiesCreate, storageType } = factoryParams;
// check if it is a using default devfile flow
if (factoryResolver === undefined && isEqual(devfile, defaultDevfile)) {
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;
}
}
// test the devfile name to decide if we need to append a suffix to is
const nameConflict = 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 { shouldCreate, factoryParams, devfile } = this.state;
const { factoryId, policiesCreate, storageType } = factoryParams;
const { factoryResolverConverted, factoryResolver, defaultDevfile } = this.props;
const { shouldCreate, devfile } = this.state;

const workspace = this.findTargetWorkspace(this.props, this.state);
if (workspace !== undefined) {
Expand All @@ -150,25 +194,28 @@ 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.');
}
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;
}

await this.createWorkspaceFromDevfile(devfile);
try {
await this.createWorkspaceFromDevfile(devfile);
} catch (e) {
const errorMessage = common.helpers.errors.getMessage(e);
throw new CreateWorkspaceError(errorMessage);
}

// wait for the workspace creation to complete
try {
Expand Down Expand Up @@ -206,7 +253,38 @@ class StepApplyDevfile extends AbstractLoaderStep<Props, State> {
);
}

private handleCreateWorkspaceError(): void {
const { defaultDevfile } = this.props;
const { devfile } = this.state;
const _devfile = defaultDevfile;
if (_devfile && devfile) {
_devfile.projects = devfile.projects;
_devfile.metadata.name = devfile.metadata.name;
_devfile.metadata.generateName = devfile.metadata.generateName;
this.updateCurrentDevfile(_devfile);
}
this.clearStepError();
}

private getAlertItem(error: unknown): AlertItem | undefined {
if (error instanceof CreateWorkspaceError) {
return {
key: 'factory-loader-create-workspace-error',
title: 'Warning',
variant: AlertVariant.warning,
children: <CreateWorkspaceErrorItems error={error} />,
actionCallbacks: [
{
title: 'Continue with the default devfile',
callback: () => this.handleCreateWorkspaceError(),
},
{
title: 'Reload',
callback: () => this.clearStepError(),
},
],
};
}
if (!error) {
return;
}
Expand Down Expand Up @@ -249,6 +327,7 @@ const mapStateToProps = (state: AppState) => ({
defaultNamespace: selectDefaultNamespace(state),
factoryResolver: selectFactoryResolver(state),
factoryResolverConverted: selectFactoryResolverConverted(state),
defaultDevfile: selectDefaultDevfile(state),
});

const connector = connect(
Expand Down
12 changes: 8 additions & 4 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 @@ -147,13 +147,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
64 changes: 57 additions & 7 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 @@ -36,6 +36,14 @@ import buildFactoryParams from '../../../buildFactoryParams';
import { AbstractLoaderStep, LoaderStepProps, LoaderStepState } from '../../../../AbstractStep';
import { AlertItem } from '../../../../../../services/helpers/types';
import OAuthService, { isOAuthResponse } from '../../../../../../services/oauth';
import ApplyingDevfileErrorItems from '../../../../../../pages/Loader/Factory/ApplyingDevfileErrorItems';

export class ApplyingDevfileError extends Error {
constructor(message) {
super(message);
this.name = 'ApplyingDevfileError';
}
}

const RELOADS_LIMIT = 2;
type ReloadsInfo = {
Expand All @@ -49,6 +57,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 +69,7 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
this.state = {
factoryParams: buildFactoryParams(props.searchParams),
shouldResolve: true,
useDefaultDevfile: false,
};
}

Expand Down Expand Up @@ -106,9 +116,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 +139,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,15 +161,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);
if (resolveDone === false) {
let resolveDone = false;
try {
// start resolving the devfile
resolveDone = await this.resolveDevfile(sourceUrl);
} catch (e) {
const errorMessage = common.helpers.errors.getMessage(e);
// check if it is a scheme validation error
if (errorMessage.includes('schema validation failed')) {
throw new ApplyingDevfileError(errorMessage);
}
throw e;
}
if (!resolveDone) {
return false;
}

Expand All @@ -176,6 +201,13 @@ class StepFetchDevfile extends AbstractLoaderStep<Props, State> {
}
}

private handleDevfileError(): 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 @@ -261,6 +293,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: <ApplyingDevfileErrorItems error={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
Loading

0 comments on commit d46d1bd

Please sign in to comment.