Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support raw devfile urls without yaml extension #683

Merged
merged 5 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2012-2023 Red Hat, Inc.
* Copyright (c) 2012-2024 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/
Expand All @@ -17,6 +17,7 @@
import static org.eclipse.che.api.factory.shared.Constants.URL_PARAMETER_NAME;

import jakarta.validation.constraints.NotNull;
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
Expand All @@ -30,8 +31,10 @@
import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl;
import org.eclipse.che.api.factory.server.urlfactory.URLFactoryBuilder;
import org.eclipse.che.api.factory.shared.dto.FactoryMetaDto;
import org.eclipse.che.api.workspace.server.devfile.DevfileParser;
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
import org.eclipse.che.api.workspace.server.devfile.URLFileContentProvider;
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException;

/**
* {@link FactoryParametersResolver} implementation to resolve factory based on url parameter as a
Expand All @@ -45,13 +48,15 @@ public class RawDevfileUrlFactoryParameterResolver extends BaseFactoryParameterR

protected final URLFactoryBuilder urlFactoryBuilder;
protected final URLFetcher urlFetcher;
private final DevfileParser devfileParser;

@Inject
public RawDevfileUrlFactoryParameterResolver(
URLFactoryBuilder urlFactoryBuilder, URLFetcher urlFetcher) {
URLFactoryBuilder urlFactoryBuilder, URLFetcher urlFetcher, DevfileParser devfileParser) {
super(null, urlFactoryBuilder, PROVIDER_NAME);
this.urlFactoryBuilder = urlFactoryBuilder;
this.urlFetcher = urlFetcher;
this.devfileParser = devfileParser;
}

/**
Expand All @@ -64,7 +69,17 @@ public RawDevfileUrlFactoryParameterResolver(
@Override
public boolean accept(Map<String, String> factoryParameters) {
String url = factoryParameters.get(URL_PARAMETER_NAME);
return !isNullOrEmpty(url) && PATTERN.matcher(url).matches();
return !isNullOrEmpty(url) && PATTERN.matcher(url).matches() || containsDevfile(url);
vinokurig marked this conversation as resolved.
Show resolved Hide resolved
}

private boolean containsDevfile(String requestURL) {
try {
String fetch = urlFetcher.fetch(requestURL);
devfileParser.parseYaml(fetch);
return true;
} catch (IOException | DevfileFormatException e) {
return !e.getMessage().startsWith("Cannot construct instance of");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds unclear

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reworked to parsing raw yaml

}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.testng.Assert.assertFalse;
import static org.testng.Assert.fail;
import static org.testng.AssertJUnit.assertEquals;
Expand All @@ -38,6 +39,7 @@
import org.eclipse.che.api.workspace.server.devfile.DevfileVersionDetector;
import org.eclipse.che.api.workspace.server.devfile.URLFetcher;
import org.eclipse.che.api.workspace.server.devfile.URLFileContentProvider;
import org.eclipse.che.api.workspace.server.devfile.exception.DevfileFormatException;
import org.eclipse.che.api.workspace.server.devfile.validator.ComponentIntegrityValidator;
import org.eclipse.che.api.workspace.server.devfile.validator.ComponentIntegrityValidator.NoopComponentIntegrityValidator;
import org.eclipse.che.api.workspace.server.devfile.validator.DevfileIntegrityValidator;
Expand All @@ -63,6 +65,7 @@ public class RawDevfileUrlFactoryParameterResolverTest {
+ " reference: ../localfile\n";

@Mock private URLFetcher urlFetcher;
@Mock private DevfileParser devfileParser;

@InjectMocks private RawDevfileUrlFactoryParameterResolver rawDevfileUrlFactoryParameterResolver;

Expand All @@ -84,7 +87,7 @@ public void shouldResolveRelativeFiles() throws Exception {
"editor", "plugin", false, devfileParser, new DevfileVersionDetector());

RawDevfileUrlFactoryParameterResolver res =
new RawDevfileUrlFactoryParameterResolver(factoryBuilder, urlFetcher);
new RawDevfileUrlFactoryParameterResolver(factoryBuilder, urlFetcher, devfileParser);

// set up our factory with the location of our devfile that is referencing our localfile
Map<String, String> factoryParameters = new HashMap<>();
Expand All @@ -106,7 +109,7 @@ public void shouldFilterAndProvideOverrideParameters() throws Exception {
URLFetcher urlFetcher = mock(URLFetcher.class);

RawDevfileUrlFactoryParameterResolver res =
new RawDevfileUrlFactoryParameterResolver(urlFactoryBuilder, urlFetcher);
new RawDevfileUrlFactoryParameterResolver(urlFactoryBuilder, urlFetcher, devfileParser);

Map<String, String> factoryParameters = new HashMap<>();
factoryParameters.put(URL_PARAMETER_NAME, "http://myloc/devfile");
Expand Down Expand Up @@ -137,7 +140,7 @@ public void shouldThrowExceptionOnInvalidURL(String url, String message) throws
URLFetcher urlFetcher = mock(URLFetcher.class);

RawDevfileUrlFactoryParameterResolver res =
new RawDevfileUrlFactoryParameterResolver(urlFactoryBuilder, urlFetcher);
new RawDevfileUrlFactoryParameterResolver(urlFactoryBuilder, urlFetcher, devfileParser);

Map<String, String> factoryParameters = new HashMap<>();
factoryParameters.put(URL_PARAMETER_NAME, url);
Expand Down Expand Up @@ -166,11 +169,33 @@ public void shouldAcceptRawDevfileUrl(String url) {
}

@Test
public void shouldNotAcceptRawDevfileUrl() {
public void shouldAcceptRawDevfileUrlWithUnrecognizedDevfile() throws Exception {
// given
String url = "https://host/path/devfile";
when(urlFetcher.fetch(eq(url))).thenReturn(DEVFILE);
when(devfileParser.parseYaml(eq(DEVFILE)))
.thenThrow(new DevfileFormatException("Unrecognized field \"schemaVersion\""));

// when
boolean result =
rawDevfileUrlFactoryParameterResolver.accept(singletonMap(URL_PARAMETER_NAME, url));

// then
assertTrue(result);
}

@Test
public void shouldNotAcceptGitRepositoryUrl() throws Exception {
// given
String gitRepositoryUrl = "https://host/user/repo.git";
when(urlFetcher.fetch(eq(gitRepositoryUrl))).thenReturn("unsupported content");
when(devfileParser.parseYaml(eq("unsupported content")))
.thenThrow(new DevfileFormatException("Cannot construct instance of ..."));

// when
boolean result =
rawDevfileUrlFactoryParameterResolver.accept(
singletonMap(URL_PARAMETER_NAME, "https://host/user/repo.git"));
singletonMap(URL_PARAMETER_NAME, gitRepositoryUrl));

// then
assertFalse(result);
Expand All @@ -195,10 +220,12 @@ private Object[] devfileUrls() {
"https://host/path/.devfile.yaml",
"https://host/path/any-name.yaml",
"https://host/path/any-name.yml",
"https://host/path/any-name",
"https://host/path/devfile.yaml?token=TOKEN123",
"https://host/path/.devfile.yaml?token=TOKEN123",
"https://host/path/any-name.yaml?token=TOKEN123",
"https://host/path/any-name.yml?token=TOKEN123"
"https://host/path/any-name.yml?token=TOKEN123",
"https://host/path/any-name?token=TOKEN123"
};
}
}
Loading