Skip to content

Commit ca140d4

Browse files
vinokurigvinokurig
committed
Throw an error if Devfile from unsupported ssh repository is not resolved (#763)
Throw an error if Devfile from unsupported ssh repository is not resolved instead of returning an empty factory dto without devfile. The ApiException will be handled by dashboard and default devfile will be used: https://github.com/eclipse-che/che-dashboard/blob/05bf4383a45cb367924c788763beb5c7689a7c79/packages/dashboard-frontend/src/components/WorkspaceProgress/CreatingSteps/Fetch/Devfile/index.tsx#L215
1 parent 922974f commit ca140d4

File tree

6 files changed

+196
-12
lines changed

6 files changed

+196
-12
lines changed

‎.ci/openshift-ci/test-gitea-no-pat-oauth-flow.sh

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ trap "catchFinish" EXIT SIGINT
3131
setupTestEnvironment ${OCP_NON_ADMIN_USER_NAME}
3232
setupSSHKeyPairs "${GITEA_PRIVATE_KEY}" "${GITEA_PUBLIC_KEY}"
3333

34-
testFactoryResolverResponse ${PUBLIC_REPO_SSH_URL} 200
35-
testFactoryResolverResponse ${PRIVATE_REPO_SSH_URL} 200
34+
testFactoryResolverResponse ${PUBLIC_REPO_SSH_URL} 500
35+
testFactoryResolverResponse ${PRIVATE_REPO_SSH_URL} 500
3636
testFactoryResolverResponse ${PUBLIC_REPO_RAW_PATH_URL} 200
3737

3838
testCloneGitRepoProjectShouldExists ${PUBLIC_REPO_WORKSPACE_NAME} ${PUBLIC_PROJECT_NAME} ${PUBLIC_REPO_SSH_URL} ${USER_CHE_NAMESPACE}

‎wsmaster/che-core-api-factory-git-ssh/pom.xml

+4
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@
2626
<findbugs.failonerror>true</findbugs.failonerror>
2727
</properties>
2828
<dependencies>
29+
<dependency>
30+
<groupId>com.google.guava</groupId>
31+
<artifactId>guava</artifactId>
32+
</dependency>
2933
<dependency>
3034
<groupId>jakarta.inject</groupId>
3135
<artifactId>jakarta.inject-api</artifactId>

‎wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java

+4-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012-2024 Red Hat, Inc.
2+
* Copyright (c) 2012-2025 Red Hat, Inc.
33
* This program and the accompanying materials are made
44
* available under the terms of the Eclipse Public License 2.0
55
* which is available at https://www.eclipse.org/legal/epl-2.0/
@@ -12,7 +12,6 @@
1212
package org.eclipse.che.api.factory.server.git.ssh;
1313

1414
import static org.eclipse.che.api.factory.server.FactoryResolverPriority.LOWEST;
15-
import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
1615
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
1716
import static org.eclipse.che.dto.server.DtoFactory.newDto;
1817

@@ -35,7 +34,7 @@
3534
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
3635

3736
/**
38-
* Provides Factory Parameters resolver for Git Ssh repositories.
37+
* Provides Factory Parameters resolver for SSH urls of unsupported Git providers.
3938
*
4039
* @author Anatolii Bazko
4140
*/
@@ -90,15 +89,11 @@ public FactoryMetaDto createFactory(@NotNull final Map<String, String> factoryPa
9089
gitSshUrl, urlFetcher, personalAccessTokenManager),
9190
extractOverrideParams(factoryParameters),
9291
true)
93-
.orElseGet(
94-
() -> newDto(FactoryDevfileV2Dto.class).withV(CURRENT_VERSION).withSource("repo"))
92+
.orElseThrow(() -> new ApiException("Failed to fetch devfile"))
9593
.acceptVisitor(new GitSshFactoryVisitor(gitSshUrl));
9694
}
9795

98-
/**
99-
* Visitor that puts the default devfile or updates devfile projects into the Git Ssh Factory, if
100-
* needed.
101-
*/
96+
/** Visitor that updates factory dto with git ssh information. */
10297
private class GitSshFactoryVisitor implements FactoryVisitor {
10398

10499
private final GitSshUrl gitSshUrl;

‎wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrl.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012-2023 Red Hat, Inc.
2+
* Copyright (c) 2012-2025 Red Hat, Inc.
33
* This program and the accompanying materials are made
44
* available under the terms of the Eclipse Public License 2.0
55
* which is available at https://www.eclipse.org/legal/epl-2.0/
@@ -73,6 +73,9 @@ public Optional<String> filename() {
7373

7474
@Override
7575
public String location() {
76+
// Since we do not know the location from an SSH URL, we return the filename instead. The
77+
// devfile content fetcher will always fail to fetch the devfile in this case.
78+
// TODO: throw an error in order to avoid http request to fetch the devfile content.
7679
return devfileFilename;
7780
}
7881
};

‎wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolverTest.java

+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright (c) 2012-2025 Red Hat, Inc.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Red Hat, Inc. - initial API and implementation
11+
*/
12+
package org.eclipse.che.api.factory.server.git.ssh;
13+
14+
import static java.util.Collections.singletonMap;
15+
import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION;
16+
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;
17+
import static org.eclipse.che.dto.server.DtoFactory.newDto;
18+
import static org.mockito.ArgumentMatchers.any;
19+
import static org.mockito.ArgumentMatchers.eq;
20+
import static org.mockito.Mockito.when;
21+
import static org.testng.Assert.assertEquals;
22+
import static org.testng.Assert.assertFalse;
23+
import static org.testng.Assert.assertTrue;
24+
25+
import com.google.common.collect.ImmutableMap;
26+
import java.util.Collections;
27+
import java.util.Map;
28+
import java.util.Optional;
29+
import org.eclipse.che.api.core.ApiException;
30+
import org.eclipse.che.api.factory.server.scm.AuthorisationRequestManager;
31+
import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager;
32+
import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider;
33+
import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder;
34+
import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto;
35+
import org.eclipse.che.api.factory.shared.dto.ScmInfoDto;
36+
import org.eclipse.che.api.workspace.server.devfile.FileContentProvider;
37+
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
38+
import org.mockito.Mock;
39+
import org.mockito.testng.MockitoTestNGListener;
40+
import org.testng.annotations.BeforeMethod;
41+
import org.testng.annotations.Listeners;
42+
import org.testng.annotations.Test;
43+
44+
@Listeners(MockitoTestNGListener.class)
45+
public class GitSshFactoryParametersResolverTest {
46+
47+
@Mock private DevfileFilenamesProvider devfileFilenamesProvider;
48+
@Mock private URLFetcher urlFetcher;
49+
@Mock private URLFactoryBuilder urlFactoryBuilder;
50+
@Mock private PersonalAccessTokenManager personalAccessTokenManager;
51+
@Mock private AuthorisationRequestManager authorisationRequestManager;
52+
@Mock private GitSshURLParser gitSshURLParser;
53+
@Mock private GitSshUrl gitSshUrl;
54+
private GitSshFactoryParametersResolver gitSshFactoryParametersResolver;
55+
56+
@BeforeMethod
57+
protected void init() {
58+
gitSshFactoryParametersResolver =
59+
new GitSshFactoryParametersResolver(
60+
gitSshURLParser,
61+
urlFetcher,
62+
urlFactoryBuilder,
63+
personalAccessTokenManager,
64+
authorisationRequestManager);
65+
}
66+
67+
@Test
68+
public void ShouldNotAcceptMissingParameter() {
69+
// given
70+
Map<String, String> parameters = singletonMap("foo", "this is a foo bar");
71+
// when
72+
boolean accept = gitSshFactoryParametersResolver.accept(parameters);
73+
// then
74+
assertFalse(accept);
75+
}
76+
77+
@Test
78+
public void ShouldNotAcceptInvalidUrl() {
79+
// given
80+
String url = "https://provider.com/user/repo.git";
81+
when(gitSshURLParser.isValid(eq(url))).thenReturn(false);
82+
Map<String, String> parameters = singletonMap(URL_PARAMETER_NAME, url);
83+
// when
84+
boolean accept = gitSshFactoryParametersResolver.accept(parameters);
85+
// then
86+
assertFalse(accept);
87+
}
88+
89+
@Test
90+
public void shouldAcceptValidUrl() {
91+
// given
92+
String url = "git@provider.com:user/repo.git";
93+
when(gitSshURLParser.isValid(eq(url))).thenReturn(true);
94+
Map<String, String> parameters = singletonMap(URL_PARAMETER_NAME, url);
95+
// when
96+
boolean accept = gitSshFactoryParametersResolver.accept(parameters);
97+
// then
98+
assertTrue(accept);
99+
}
100+
101+
@Test
102+
public void shouldCreateFactoryWithDevfile() throws Exception {
103+
// given
104+
String url = "git@provider.com:user/repo.git";
105+
when(gitSshUrl.getProviderName()).thenReturn("git-ssh");
106+
when(gitSshUrl.getRepositoryLocation()).thenReturn("repository-location");
107+
ImmutableMap<String, String> params = ImmutableMap.of(URL_PARAMETER_NAME, url);
108+
when(gitSshURLParser.parse(eq(url))).thenReturn(gitSshUrl);
109+
when(urlFactoryBuilder.createFactoryFromDevfile(
110+
eq(gitSshUrl), any(FileContentProvider.class), eq(Collections.emptyMap()), eq(true)))
111+
.thenReturn(Optional.of(generateDevfileV2Factory()));
112+
// when
113+
FactoryDevfileV2Dto factory =
114+
(FactoryDevfileV2Dto) gitSshFactoryParametersResolver.createFactory(params);
115+
// then
116+
ScmInfoDto scmInfo = factory.getScmInfo();
117+
assertEquals(scmInfo.getScmProviderName(), "git-ssh");
118+
assertEquals(scmInfo.getRepositoryUrl(), "repository-location");
119+
}
120+
121+
@Test(
122+
expectedExceptions = ApiException.class,
123+
expectedExceptionsMessageRegExp = "Failed to fetch devfile")
124+
public void shouldThrowException() throws Exception {
125+
// given
126+
String url = "git@provider.com:user/repo.git";
127+
ImmutableMap<String, String> params = ImmutableMap.of(URL_PARAMETER_NAME, url);
128+
when(gitSshURLParser.parse(eq(url))).thenReturn(gitSshUrl);
129+
when(urlFactoryBuilder.createFactoryFromDevfile(
130+
eq(gitSshUrl), any(FileContentProvider.class), eq(Collections.emptyMap()), eq(true)))
131+
.thenReturn(Optional.empty());
132+
// when
133+
FactoryDevfileV2Dto factory =
134+
(FactoryDevfileV2Dto) gitSshFactoryParametersResolver.createFactory(params);
135+
}
136+
137+
private FactoryDevfileV2Dto generateDevfileV2Factory() {
138+
return newDto(FactoryDevfileV2Dto.class)
139+
.withV(CURRENT_VERSION)
140+
.withSource("repo")
141+
.withDevfile(Map.of("schemaVersion", "2.0.0"));
142+
}
143+
}

‎wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrlTest.java

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (c) 2012-2025 Red Hat, Inc.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Red Hat, Inc. - initial API and implementation
11+
*/
12+
package org.eclipse.che.api.factory.server.git.ssh;
13+
14+
import static org.testng.Assert.assertEquals;
15+
16+
import java.util.Arrays;
17+
import java.util.List;
18+
import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl.DevfileLocation;
19+
import org.mockito.testng.MockitoTestNGListener;
20+
import org.testng.annotations.Listeners;
21+
import org.testng.annotations.Test;
22+
23+
@Listeners(MockitoTestNGListener.class)
24+
public class GitSshUrlTest {
25+
26+
@Test
27+
public void shouldReturnDevfileLocations() throws Exception {
28+
String[] devfileNames = {"devfile.yaml", ".devfile.yaml"};
29+
GitSshUrl sshUrl =
30+
new GitSshUrl()
31+
.withRepository("repository")
32+
.withHostName("hostname")
33+
.withDevfileFilenames(Arrays.asList(devfileNames));
34+
List<DevfileLocation> devfileLocations = sshUrl.devfileFileLocations();
35+
assertEquals(devfileLocations.size(), 2);
36+
assertEquals(devfileLocations.get(0).location(), "devfile.yaml");
37+
assertEquals(devfileLocations.get(1).location(), ".devfile.yaml");
38+
}
39+
}

0 commit comments

Comments
 (0)