From 27505d65b5d03d019e5cd3b0fc6ceed2e70e3893 Mon Sep 17 00:00:00 2001 From: Igor Vinokur Date: Wed, 26 Feb 2025 11:19:03 +0200 Subject: [PATCH] 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 --- .../test-gitea-no-pat-oauth-flow.sh | 4 +- wsmaster/che-core-api-factory-git-ssh/pom.xml | 4 + .../ssh/GitSshFactoryParametersResolver.java | 13 +- .../api/factory/server/git/ssh/GitSshUrl.java | 5 +- .../GitSshFactoryParametersResolverTest.java | 143 ++++++++++++++++++ .../factory/server/git/ssh/GitSshUrlTest.java | 39 +++++ 6 files changed, 196 insertions(+), 12 deletions(-) create mode 100644 wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolverTest.java create mode 100644 wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrlTest.java diff --git a/.ci/openshift-ci/test-gitea-no-pat-oauth-flow.sh b/.ci/openshift-ci/test-gitea-no-pat-oauth-flow.sh index af58e0be7a7..27e93606d48 100644 --- a/.ci/openshift-ci/test-gitea-no-pat-oauth-flow.sh +++ b/.ci/openshift-ci/test-gitea-no-pat-oauth-flow.sh @@ -31,8 +31,8 @@ trap "catchFinish" EXIT SIGINT setupTestEnvironment ${OCP_NON_ADMIN_USER_NAME} setupSSHKeyPairs "${GITEA_PRIVATE_KEY}" "${GITEA_PUBLIC_KEY}" -testFactoryResolverResponse ${PUBLIC_REPO_SSH_URL} 200 -testFactoryResolverResponse ${PRIVATE_REPO_SSH_URL} 200 +testFactoryResolverResponse ${PUBLIC_REPO_SSH_URL} 500 +testFactoryResolverResponse ${PRIVATE_REPO_SSH_URL} 500 testFactoryResolverResponse ${PUBLIC_REPO_RAW_PATH_URL} 200 testCloneGitRepoProjectShouldExists ${PUBLIC_REPO_WORKSPACE_NAME} ${PUBLIC_PROJECT_NAME} ${PUBLIC_REPO_SSH_URL} ${USER_CHE_NAMESPACE} diff --git a/wsmaster/che-core-api-factory-git-ssh/pom.xml b/wsmaster/che-core-api-factory-git-ssh/pom.xml index 349d1eff6f0..12613a20c8b 100644 --- a/wsmaster/che-core-api-factory-git-ssh/pom.xml +++ b/wsmaster/che-core-api-factory-git-ssh/pom.xml @@ -26,6 +26,10 @@ true + + com.google.guava + guava + jakarta.inject jakarta.inject-api diff --git a/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java b/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java index 890e3a8b255..0ce68f53fe5 100644 --- a/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java +++ b/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolver.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2024 Red Hat, Inc. + * Copyright (c) 2012-2025 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -12,7 +12,6 @@ package org.eclipse.che.api.factory.server.git.ssh; import static org.eclipse.che.api.factory.server.FactoryResolverPriority.LOWEST; -import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION; import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; import static org.eclipse.che.dto.server.DtoFactory.newDto; @@ -35,7 +34,7 @@ import org.eclipse.che.api.workspace.server.devfile.URLFetcher; /** - * Provides Factory Parameters resolver for Git Ssh repositories. + * Provides Factory Parameters resolver for SSH urls of unsupported Git providers. * * @author Anatolii Bazko */ @@ -90,15 +89,11 @@ public FactoryMetaDto createFactory(@NotNull final Map factoryPa gitSshUrl, urlFetcher, personalAccessTokenManager), extractOverrideParams(factoryParameters), true) - .orElseGet( - () -> newDto(FactoryDevfileV2Dto.class).withV(CURRENT_VERSION).withSource("repo")) + .orElseThrow(() -> new ApiException("Failed to fetch devfile")) .acceptVisitor(new GitSshFactoryVisitor(gitSshUrl)); } - /** - * Visitor that puts the default devfile or updates devfile projects into the Git Ssh Factory, if - * needed. - */ + /** Visitor that updates factory dto with git ssh information. */ private class GitSshFactoryVisitor implements FactoryVisitor { private final GitSshUrl gitSshUrl; diff --git a/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrl.java b/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrl.java index ae180c9e07b..9e3ac61d8de 100644 --- a/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrl.java +++ b/wsmaster/che-core-api-factory-git-ssh/src/main/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrl.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2023 Red Hat, Inc. + * Copyright (c) 2012-2025 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -73,6 +73,9 @@ public Optional filename() { @Override public String location() { + // Since we do not know the location from an SSH URL, we return the filename instead. The + // devfile content fetcher will always fail to fetch the devfile in this case. + // TODO: throw an error in order to avoid http request to fetch the devfile content. return devfileFilename; } }; diff --git a/wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolverTest.java b/wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolverTest.java new file mode 100644 index 00000000000..befedeb5b77 --- /dev/null +++ b/wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshFactoryParametersResolverTest.java @@ -0,0 +1,143 @@ +/* + * Copyright (c) 2012-2025 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.api.factory.server.git.ssh; + +import static java.util.Collections.singletonMap; +import static org.eclipse.che.api.factory.shared.Constants.CURRENT_VERSION; +import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME; +import static org.eclipse.che.dto.server.DtoFactory.newDto; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import com.google.common.collect.ImmutableMap; +import java.util.Collections; +import java.util.Map; +import java.util.Optional; +import org.eclipse.che.api.core.ApiException; +import org.eclipse.che.api.factory.server.scm.AuthorisationRequestManager; +import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; +import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider; +import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder; +import org.eclipse.che.api.factory.shared.dto.FactoryDevfileV2Dto; +import org.eclipse.che.api.factory.shared.dto.ScmInfoDto; +import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; +import org.eclipse.che.api.workspace.server.devfile.URLFetcher; +import org.mockito.Mock; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +@Listeners(MockitoTestNGListener.class) +public class GitSshFactoryParametersResolverTest { + + @Mock private DevfileFilenamesProvider devfileFilenamesProvider; + @Mock private URLFetcher urlFetcher; + @Mock private URLFactoryBuilder urlFactoryBuilder; + @Mock private PersonalAccessTokenManager personalAccessTokenManager; + @Mock private AuthorisationRequestManager authorisationRequestManager; + @Mock private GitSshURLParser gitSshURLParser; + @Mock private GitSshUrl gitSshUrl; + private GitSshFactoryParametersResolver gitSshFactoryParametersResolver; + + @BeforeMethod + protected void init() { + gitSshFactoryParametersResolver = + new GitSshFactoryParametersResolver( + gitSshURLParser, + urlFetcher, + urlFactoryBuilder, + personalAccessTokenManager, + authorisationRequestManager); + } + + @Test + public void ShouldNotAcceptMissingParameter() { + // given + Map parameters = singletonMap("foo", "this is a foo bar"); + // when + boolean accept = gitSshFactoryParametersResolver.accept(parameters); + // then + assertFalse(accept); + } + + @Test + public void ShouldNotAcceptInvalidUrl() { + // given + String url = "https://provider.com/user/repo.git"; + when(gitSshURLParser.isValid(eq(url))).thenReturn(false); + Map parameters = singletonMap(URL_PARAMETER_NAME, url); + // when + boolean accept = gitSshFactoryParametersResolver.accept(parameters); + // then + assertFalse(accept); + } + + @Test + public void shouldAcceptValidUrl() { + // given + String url = "git@provider.com:user/repo.git"; + when(gitSshURLParser.isValid(eq(url))).thenReturn(true); + Map parameters = singletonMap(URL_PARAMETER_NAME, url); + // when + boolean accept = gitSshFactoryParametersResolver.accept(parameters); + // then + assertTrue(accept); + } + + @Test + public void shouldCreateFactoryWithDevfile() throws Exception { + // given + String url = "git@provider.com:user/repo.git"; + when(gitSshUrl.getProviderName()).thenReturn("git-ssh"); + when(gitSshUrl.getRepositoryLocation()).thenReturn("repository-location"); + ImmutableMap params = ImmutableMap.of(URL_PARAMETER_NAME, url); + when(gitSshURLParser.parse(eq(url))).thenReturn(gitSshUrl); + when(urlFactoryBuilder.createFactoryFromDevfile( + eq(gitSshUrl), any(FileContentProvider.class), eq(Collections.emptyMap()), eq(true))) + .thenReturn(Optional.of(generateDevfileV2Factory())); + // when + FactoryDevfileV2Dto factory = + (FactoryDevfileV2Dto) gitSshFactoryParametersResolver.createFactory(params); + // then + ScmInfoDto scmInfo = factory.getScmInfo(); + assertEquals(scmInfo.getScmProviderName(), "git-ssh"); + assertEquals(scmInfo.getRepositoryUrl(), "repository-location"); + } + + @Test( + expectedExceptions = ApiException.class, + expectedExceptionsMessageRegExp = "Failed to fetch devfile") + public void shouldThrowException() throws Exception { + // given + String url = "git@provider.com:user/repo.git"; + ImmutableMap params = ImmutableMap.of(URL_PARAMETER_NAME, url); + when(gitSshURLParser.parse(eq(url))).thenReturn(gitSshUrl); + when(urlFactoryBuilder.createFactoryFromDevfile( + eq(gitSshUrl), any(FileContentProvider.class), eq(Collections.emptyMap()), eq(true))) + .thenReturn(Optional.empty()); + // when + FactoryDevfileV2Dto factory = + (FactoryDevfileV2Dto) gitSshFactoryParametersResolver.createFactory(params); + } + + private FactoryDevfileV2Dto generateDevfileV2Factory() { + return newDto(FactoryDevfileV2Dto.class) + .withV(CURRENT_VERSION) + .withSource("repo") + .withDevfile(Map.of("schemaVersion", "2.0.0")); + } +} diff --git a/wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrlTest.java b/wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrlTest.java new file mode 100644 index 00000000000..f997b7702dc --- /dev/null +++ b/wsmaster/che-core-api-factory-git-ssh/src/test/java/org/eclipse/che/api/factory/server/git/ssh/GitSshUrlTest.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2012-2025 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.api.factory.server.git.ssh; + +import static org.testng.Assert.assertEquals; + +import java.util.Arrays; +import java.util.List; +import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl.DevfileLocation; +import org.mockito.testng.MockitoTestNGListener; +import org.testng.annotations.Listeners; +import org.testng.annotations.Test; + +@Listeners(MockitoTestNGListener.class) +public class GitSshUrlTest { + + @Test + public void shouldReturnDevfileLocations() throws Exception { + String[] devfileNames = {"devfile.yaml", ".devfile.yaml"}; + GitSshUrl sshUrl = + new GitSshUrl() + .withRepository("repository") + .withHostName("hostname") + .withDevfileFilenames(Arrays.asList(devfileNames)); + List devfileLocations = sshUrl.devfileFileLocations(); + assertEquals(devfileLocations.size(), 2); + assertEquals(devfileLocations.get(0).location(), "devfile.yaml"); + assertEquals(devfileLocations.get(1).location(), ".devfile.yaml"); + } +}