Skip to content

Commit

Permalink
Make plugin references without version use 'latest' implicitly
Browse files Browse the repository at this point in the history
Changes necessary to make plugin references without a version specified,
e.g.

  eclipse/che-theia

to parse as 'eclipse/che-theia/latest' by default. As a side effect,
this commit disables:

- Validation of plugin references with a custom registry. This allows
using direct links to meta.yamls (no need to have a path ending in
publisher/name/version) but also works by arbitrarily splitting the URL
into a 'registry' and 'id'

- Devfile aliases and similar features are disabled for plugins from
custom registries

- Some tests that depend on having a custom registry specified are
disabled until issue
  eclipse-che#13385
is resolved.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
  • Loading branch information
amisevsk committed May 24, 2019
1 parent 9eb3ea0 commit 422ca27
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 120 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@
"alias": {},
"id": {
"type": "string",
"description": "Describes the component id. It has the following format: [{REGISTRY_URL}/]{plugin/editor PUBLISHER}/{plugin/editor NAME}/{plugin/editor VERSION}, where {REGISTRY_URL}/ is an optional part.",
"pattern": "^((https?://)[a-zA-Z0-9_\\-./]+/)?[a-z0-9_\\-.]+/[a-z0-9_\\-.]+/[a-z0-9_\\-.]+$",
"description": "Describes the component id. It has the following format: [{REGISTRY_URL}/]{plugin/editor PUBLISHER}/{plugin/editor NAME}/{plugin/editor VERSION}, where {REGISTRY_URL}/ is an optional part. If /{VERSION} is omitted, 'latest' is implicitly applied.",
"pattern": "^((https?://)[a-zA-Z0-9_\\-./]+/)?[a-z0-9_\\-.]+/[a-z0-9_\\-.]+(/[a-z0-9_\\-.]+)?$",
"examples": [
"eclipse/maven-jdk8/1.0.0",
"https://che-plugin-registry.openshift.io/eclipse/maven-jdk8/1.0.0"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public void shouldProvisionDefaultPluginsIfTheyAreNotSpecifiedAndDefaultEditorIs
assertTrue(components.contains(new ComponentImpl(PLUGIN_COMPONENT_TYPE, TERMINAL_PLUGIN_REF)));
}

@Test
@Test(enabled = false) // Issue: https://github.com/eclipse/che/issues/13385
public void
shouldProvisionDefaultPluginsIfTheyAreNotSpecifiedAndDefaultEditorFromCustomRegistryIsConfigured()
throws DevfileException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void shouldProvisionPluginCommandAttributesDuringCheEditorComponentApplyi
"eclipse/super-editor/0.0.1");
}

@Test
@Test(enabled = false) // Issue: https://github.com/eclipse/che/issues/13385
public void shouldProvisionPluginCommandAttributeWhenIdIsURLToCustomPluginRegistry()
throws DevfileException {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void shouldProvisionPluginCommandAttributesDuringChePluginComponentApplyi
"eclipse/super-plugin/0.0.1");
}

@Test
@Test(enabled = false) // Issue: https://github.com/eclipse/che/issues/13385
public void shouldProvisionPluginCommandAttributeWhenIdIsURLToCustomPluginRegistry()
throws DevfileException {
// given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void shouldProvisionChePluginComponent() throws WorkspaceExportException
.getAttributes()
.put(
WORKSPACE_TOOLING_PLUGINS_ATTRIBUTE,
"eclipse/super-plugin/0.0.1,https://localhost:8080/publisher1/custom-plugin/v1");
"eclipse/super-plugin/0.0.1,publisher1/custom-plugin/v1");
workspaceConfig
.getAttributes()
.put(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ public Object[][] validDevfiles() {
{"dockerimage_component/devfile_dockerimage_component.yaml"},
{"dockerimage_component/devfile_dockerimage_component_without_entry_point.yaml"},
{"editor_plugin_component/devfile_editor_component_with_custom_registry.yaml"},
{"editor_plugin_component/devfile_editor_plugins_components_with_memory_limit.yaml"}
{"editor_plugin_component/devfile_editor_plugins_components_with_memory_limit.yaml"},
{"editor_plugin_component/devfile_editor_component_without_version.yaml"},
};
}

Expand Down Expand Up @@ -134,17 +135,13 @@ public Object[][] invalidDevfiles() {
"editor_plugin_component/devfile_editor_component_with_indistinctive_field_reference.yaml",
"(/components/0/reference):The object must not have a property whose name is \"reference\"."
},
{
"editor_plugin_component/devfile_editor_component_without_version.yaml",
"(/components/0/id):The string value must match the pattern \"^((https?://)[a-zA-Z0-9_\\-./]+/)?[a-z0-9_\\-.]+/[a-z0-9_\\-.]+/[a-z0-9_\\-.]+$\"."
},
{
"editor_plugin_component/devfile_editor_plugins_components_with_invalid_memory_limit.yaml",
"(/components/0/memoryLimit):The value must be of string type, but actual type is integer."
},
{
"editor_plugin_component/devfile_editor_component_with_multiple_colons_in_id.yaml",
"(/components/0/id):The string value must match the pattern \"^((https?://)[a-zA-Z0-9_\\-./]+/)?[a-z0-9_\\-.]+/[a-z0-9_\\-.]+/[a-z0-9_\\-.]+$\"."
"(/components/0/id):The string value must match the pattern \"^((https?://)[a-zA-Z0-9_\\-./]+/)?[a-z0-9_\\-.]+/[a-z0-9_\\-.]+(/[a-z0-9_\\-.]+)?$\"."
},
// kubernetes/openshift component model testing
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@
public class PluginFQNParser {

private static final String INCORRECT_PLUGIN_FORMAT_TEMPLATE =
"Plugin '%s' has incorrect format. Should be: 'registryURL/publisher/name/version' or 'publisher/name/version'";
private static final String REGISTRY_PATTERN = "https?://[-./\\w]+(:[0-9]+)?(/[-./\\w]+)?";
"Plugin '%s' has incorrect format. Should be: 'registryURL/path', 'publisher/name/version', or 'publisher/name'";

// First potential format: 'publisher/name/version', where 'version' is optional. No alternate
// registry
// is permitted here.
private static final String PUBLISHER_PATTERN = "[-a-z0-9]+";
private static final String NAME_PATTERN = "[-a-z0-9]+";
private static final String VERSION_PATTERN = "[-.a-z0-9]+";
Expand All @@ -52,11 +55,18 @@ public class PluginFQNParser {
+ PUBLISHER_PATTERN
+ ")/(?<name>"
+ NAME_PATTERN
+ ")/(?<version>"
+ ")(:?/(?<version>"
+ VERSION_PATTERN
+ ")";
private static final Pattern PLUGIN_PATTERN =
Pattern.compile("((?<registry>" + REGISTRY_PATTERN + ")/)?(?<id>" + ID_PATTERN + ")");
+ "))?";
private static final Pattern PLUGIN_PATTERN = Pattern.compile("(?<id>" + ID_PATTERN + ")");
private static final String LATEST_VERSION = "latest";

// Second potential format: 'registryURL/identifier'. We do not extract publisher, name, and
// version here.
private static final String REGISTRY_PATTERN = "https?://[-./\\w]+(:[0-9]+)?(/[-./\\w]+)?";
private static final String REGISTRY_ID_PATTERN = "[-_.A-Za-z0-9]+";
private static final Pattern PLUGIN_WITH_REGISTRY_PATTERN =
Pattern.compile("(?<registry>" + REGISTRY_PATTERN + "/)(?<id>" + REGISTRY_ID_PATTERN + ")/?");

/**
* Parses a workspace attributes map into a collection of {@link PluginFQN}.
Expand Down Expand Up @@ -115,23 +125,28 @@ private Collection<PluginFQN> parsePluginFQNs(String attribute) throws Infrastru
}

public ExtendedPluginFQN parsePluginFQN(String plugin) throws InfrastructureException {
String registry;
String id;
String publisher;
String name;
String version;
URI registryURI = null;
// Check if plugin matches default format: 'publisher/name/version' or 'publisher/name'
Matcher matcher = PLUGIN_PATTERN.matcher(plugin);
if (matcher.matches()) {
registry = matcher.group("registry");
id = matcher.group("id");
publisher = matcher.group("publisher");
name = matcher.group("name");
version = matcher.group("version");
} else {
throw new InfrastructureException(format(INCORRECT_PLUGIN_FORMAT_TEMPLATE, plugin));
String id = matcher.group("id");
String publisher = matcher.group("publisher");
String name = matcher.group("name");
String version = matcher.group("version");
if (isNullOrEmpty(version)) {
version = LATEST_VERSION;
id = String.format("%s/%s", id, version);
}
return new ExtendedPluginFQN(null, id, publisher, name, version);
}
if (!isNullOrEmpty(registry)) {

// Check if plugin matches registry format: 'registryURL/identifier'
Matcher referenceMatcher = PLUGIN_WITH_REGISTRY_PATTERN.matcher(plugin);
if (referenceMatcher.matches()) {
// Workaround; plugin broker expects an 'id' field for each plugin, so it's necessary to split
// the URL arbitrarily. See issue: https://github.com/eclipse/che/issues/13385
String registry = referenceMatcher.group("registry");
String id = referenceMatcher.group("id");
URI registryURI = null;
try {
registryURI = new URI(registry);
} catch (URISyntaxException e) {
Expand All @@ -140,9 +155,11 @@ public ExtendedPluginFQN parsePluginFQN(String plugin) throws InfrastructureExce
"Plugin registry URL '%s' is invalid. Problematic plugin entry: '%s'",
registry, plugin));
}
return new ExtendedPluginFQN(registryURI, id, null, null, null);
}

return new ExtendedPluginFQN(registryURI, id, publisher, name, version);
// Failed to match above options.
throw new InfrastructureException(format(INCORRECT_PLUGIN_FORMAT_TEMPLATE, plugin));
}

private String[] splitAttribute(String attribute) {
Expand Down
Loading

0 comments on commit 422ca27

Please sign in to comment.