Skip to content

Commit

Permalink
Start workspace from default devfile on private repository SSH url (#…
Browse files Browse the repository at this point in the history 
…1286)

Remove the Devfile resolve from a privatre repositry via an SSH url is not supported warning and start workspace from the default devfile in this case.
  • Loading branch information
@vinokurig
vinokurig authored Jan 7, 2025
1 parent e3d2d8d commit c8ce7d9
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 223 deletions.
1 change: 1 addition & 0 deletions ...end/src/components/WorkspaceProgress/CreatingSteps/Apply/Devfile/__tests__/index.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ describe('Creating steps, applying a devfile', () => {
expect(prepareDevfile).toHaveBeenCalledWith(
expect.objectContaining({
attributes: {
'controller.devfile.io/bootstrap-devworkspace': true,
defaultDevfile: true,
},
}),
Expand Down
21 changes: 8 additions & 13 deletions ...dashboard-frontend/src/components/WorkspaceProgress/CreatingSteps/Apply/Devfile/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { ProgressStepTitle } from '@/components/WorkspaceProgress/StepTitle';
import { TimeLimit } from '@/components/WorkspaceProgress/TimeLimit';
import { lazyInject } from '@/inversify.config';
import devfileApi from '@/services/devfileApi';
import { FactoryLocationAdapter } from '@/services/factory-location-adapter';
import {
buildFactoryParams,
FactoryParams,
Expand Down Expand Up @@ -181,14 +180,12 @@ class CreatingStepApplyDevfile extends ProgressStep<Props, State> {

// when using the default devfile instead of a user devfile
if (factoryResolver === undefined && isEqual(devfile, defaultDevfile)) {
if (FactoryLocationAdapter.isSshLocation(factoryParams.sourceUrl)) {
if (!devfile.attributes) {
devfile.attributes = {};
}

devfile.attributes['controller.devfile.io/bootstrap-devworkspace'] = true;
if (!devfile.attributes) {
devfile.attributes = {};
}

devfile.attributes['controller.devfile.io/bootstrap-devworkspace'] = true;

if (devfile.projects === undefined) {
devfile.projects = [];
}
Expand All @@ -203,13 +200,11 @@ class CreatingStepApplyDevfile extends ProgressStep<Props, State> {
}
}
} else if (factoryResolver?.source === 'repo') {
if (FactoryLocationAdapter.isSshLocation(factoryParams.sourceUrl)) {
if (!devfile.attributes) {
devfile.attributes = {};
}

devfile.attributes['controller.devfile.io/bootstrap-devworkspace'] = true;
if (!devfile.attributes) {
devfile.attributes = {};
}

devfile.attributes['controller.devfile.io/bootstrap-devworkspace'] = true;
}

if (remotes) {
Expand Down
143 changes: 10 additions & 133 deletions ...end/src/components/WorkspaceProgress/CreatingSteps/Fetch/Devfile/__tests__/index.spec.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */

import { FACTORY_LINK_ATTR } from '@eclipse-che/common';
import { AlertVariant } from '@patternfly/react-core';
import { cleanup, screen, waitFor } from '@testing-library/react';
import userEvent, { UserEvent } from '@testing-library/user-event';
import React from 'react';
Expand Down Expand Up @@ -346,7 +345,7 @@ describe('Creating steps, fetching a devfile', () => {
});
});

describe('unsupported git provider error', () => {
describe('unsupported git provider', () => {
let emptyStore: Store;
const rejectReason = 'Failed to fetch devfile';

Expand All @@ -355,110 +354,13 @@ describe('Creating steps, fetching a devfile', () => {
mockRequestFactoryResolver.mockRejectedValueOnce(rejectReason);
});

test('alert title', async () => {
test('should continue with the default devfile', async () => {
renderComponent(emptyStore, searchParams);

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

const expectAlertItem = expect.objectContaining({
title: 'Warning',
actionCallbacks: [
expect.objectContaining({
title: 'Continue with default devfile',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Reload',
callback: expect.any(Function),
}),
],
});
await waitFor(() => expect(mockOnError).toHaveBeenCalledWith(expectAlertItem));

expect(mockOnNextStep).not.toHaveBeenCalled();
});

test('action "Continue with default devfile"', async () => {
// this deferred object will help run the callback at the right time
const deferred = getDefer();

const actionTitle = 'Continue with default devfile';
mockOnError.mockImplementationOnce((alertItem: AlertItem) => {
const action = alertItem.actionCallbacks?.find(_action =>
_action.title.startsWith(actionTitle),
);
expect(action).toBeDefined();

if (action) {
deferred.promise.then(action.callback);
} else {
throw new Error('Action not found');
}
});

renderComponent(emptyStore, searchParams);
await jest.runAllTimersAsync();

await waitFor(() => expect(mockOnError).toHaveBeenCalled());
expect(mockOnRestart).not.toHaveBeenCalled();
expect(mockOnNextStep).not.toHaveBeenCalled();

mockOnError.mockClear();

/* test the action */
await jest.runOnlyPendingTimersAsync();

// resolve deferred to trigger the callback
deferred.resolve();

await waitFor(() => expect(mockOnNextStep).toHaveBeenCalled());
expect(mockOnRestart).not.toHaveBeenCalled();
expect(mockOnError).not.toHaveBeenCalled();
});

test('action "Reload"', async () => {
// this deferred object will help run the callback at the right time
const deferred = getDefer();

const actionTitle = 'Reload';
mockOnError.mockImplementationOnce(async (alertItem: AlertItem) => {
const action = alertItem.actionCallbacks?.find(_action =>
_action.title.startsWith(actionTitle),
);
expect(action).toBeDefined();

if (action) {
deferred.promise.then(action.callback);
} else {
throw new Error('Action not found');
}
});

renderComponent(emptyStore, searchParams);
await jest.runAllTimersAsync();

await waitFor(() => expect(mockOnError).toHaveBeenCalled());
expect(mockOnRestart).not.toHaveBeenCalled();
expect(mockOnNextStep).not.toHaveBeenCalled();

// first call resolves with error
expect(mockRequestFactoryResolver).toHaveBeenCalledTimes(1);

mockOnError.mockClear();

/* test the action */

await jest.runAllTimersAsync();

// resolve deferred to trigger the callback
deferred.resolve();

await waitFor(() => expect(mockOnRestart).toHaveBeenCalled());
expect(mockOnNextStep).not.toHaveBeenCalled();
expect(mockOnError).not.toHaveBeenCalled();

// should request the factory resolver for the second time
await waitFor(() => expect(mockRequestFactoryResolver).toHaveBeenCalledTimes(2));
await waitFor(() => expect(mockOnError).not.toHaveBeenCalled());
expect(mockOnNextStep).toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -695,31 +597,6 @@ describe('Creating steps, fetching a devfile', () => {
const protocol = 'http://';
const factoryUrl = 'git@github.com:user/repository-name.git';
const emptyStore = new MockStoreBuilder().build();
const sshPrivateRepoAllertItem = expect.objectContaining({
title: 'Warning',
variant: AlertVariant.warning,
children: (
<ExpandableWarning
textBefore="Devfile resolve from a privatre repositry via an SSH url is not supported."
errorMessage="Could not reach devfile"
textAfter="Apply a Personal Access Token to fetch the devfile.yaml content."
/>
),
actionCallbacks: [
expect.objectContaining({
title: 'Continue with default devfile',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Reload',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Open Documentation page',
callback: expect.any(Function),
}),
],
});

let spyWindowLocation: jest.SpyInstance;
let location: Location;
Expand Down Expand Up @@ -771,32 +648,32 @@ describe('Creating steps, fetching a devfile', () => {
expect(mockOnError).not.toHaveBeenCalled();
});

it('should show warning on SSH url', async () => {
it('should use default devfile on private SSH url', async () => {
searchParams = new URLSearchParams({
[FACTORY_URL_ATTR]: 'git@github.com:user/repository.git',
});

renderComponent(emptyStore, searchParams, location);

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);
await waitFor(() => expect(mockOnNextStep).not.toHaveBeenCalled);
await waitFor(() => expect(mockOnNextStep).toHaveBeenCalled());

expect(mockOpenOAuthPage).not.toHaveBeenCalled();
expect(mockOnError).toHaveBeenCalledWith(sshPrivateRepoAllertItem);
expect(mockOnError).not.toHaveBeenCalled();
});

it('should show warning on bitbucket-server SSH url', async () => {
it('should use default devfile on bitbucket-server SSH url', async () => {
searchParams = new URLSearchParams({
[FACTORY_URL_ATTR]: 'ssh://git@bitbucket-server.com/~user/repository.git',
});

renderComponent(emptyStore, searchParams, location);

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);
await waitFor(() => expect(mockOnNextStep).not.toHaveBeenCalled);
await waitFor(() => expect(mockOnNextStep).toHaveBeenCalled);

expect(mockOpenOAuthPage).not.toHaveBeenCalled();
expect(mockOnError).toHaveBeenCalledWith(sshPrivateRepoAllertItem);
expect(mockOnError).not.toHaveBeenCalled();
});
});
});
Expand Down
81 changes: 4 additions & 77 deletions ...dashboard-frontend/src/components/WorkspaceProgress/CreatingSteps/Fetch/Devfile/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,6 @@ export class ApplyingDevfileError extends Error {
}
}

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

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

const RELOADS_LIMIT = 2;
type ReloadsInfo = {
[url: string]: number;
Expand All @@ -77,7 +63,6 @@ export type State = ProgressStepState & {

class CreatingStepFetchDevfile extends ProgressStep<Props, State> {
protected readonly name = 'Inspecting repo';
private readonly sshPattern = new RegExp('(git@|(ssh|git)://).*');

constructor(props: Props) {
super(props);
Expand Down Expand Up @@ -188,10 +173,6 @@ class CreatingStepFetchDevfile extends ProgressStep<Props, State> {
this.clearStepError();
}

protected handleOpenDocumentationPage(): void {
window.open(this.props.branding.docs.startWorkspaceFromGit, '_blank');
}

protected handleTimeout(): void {
const timeoutError = new Error(
`Devfile hasn't been resolved in the last ${TIMEOUT_TO_RESOLVE_SEC} seconds.`,
Expand Down Expand Up @@ -231,14 +212,12 @@ class CreatingStepFetchDevfile extends ProgressStep<Props, State> {
}
if (
errorMessage === 'Failed to fetch devfile' ||
errorMessage ===
'Cannot build factory with any of the provided parameters. Please check parameters correctness, and resend query.' ||
errorMessage.startsWith('Could not reach devfile')
) {
// check if the source url is an SSH url
if (this.sshPattern.test(sourceUrl)) {
throw new SSHPrivateRepositoryUrlError(errorMessage);
} else {
throw new UnsupportedGitProviderError(errorMessage);
}
this.setState({ useDefaultDevfile: true });
return true;
}
throw e;
}
Expand Down Expand Up @@ -359,58 +338,6 @@ class CreatingStepFetchDevfile extends ProgressStep<Props, State> {
],
};
}
if (error instanceof UnsupportedGitProviderError) {
return {
key,
title: 'Warning',
variant: AlertVariant.warning,
children: (
<ExpandableWarning
textBefore="Could not find any devfile in the Git repository"
errorMessage={helpers.errors.getMessage(error)}
textAfter="The Git provider is not supported."
/>
),
actionCallbacks: [
{
title: 'Continue with default devfile',
callback: () => this.handleDefaultDevfile(key),
},
{
title: 'Reload',
callback: () => this.handleRestart(key),
},
],
};
}
if (error instanceof SSHPrivateRepositoryUrlError) {
return {
key,
title: 'Warning',
variant: AlertVariant.warning,
children: (
<ExpandableWarning
textBefore="Devfile resolve from a privatre repositry via an SSH url is not supported."
errorMessage={helpers.errors.getMessage(error)}
textAfter="Apply a Personal Access Token to fetch the devfile.yaml content."
/>
),
actionCallbacks: [
{
title: 'Continue with default devfile',
callback: () => this.handleDefaultDevfile(key),
},
{
title: 'Reload',
callback: () => this.handleRestart(key),
},
{
title: 'Open Documentation page',
callback: () => this.handleOpenDocumentationPage(),
},
],
};
}
return {
key,
title: 'Failed to create the workspace',
Expand Down

0 comments on commit c8ce7d9

Please sign in to comment.