Check plugins' extensions for conflicts during metadata broker #93

amisevsk · 2020-02-11T02:05:06Z

What does this PR do?

During metadata brokering process, check plugins for conflicting extensions (i.e. two plugins use the same extension), and warn user in case of conflict.

Sample output:

List of plugins and editors to install
- redhat/java11/latest - Java Linting, Intellisense, formatting, refactoring, Maven/Gradle support and more...
- ms-kubernetes-tools/vscode-kubernetes-tools/latest - Develop, deploy and debug Kubernetes applications
- redhat/vscode-openshift-connector/latest - Interacting with Red Hat OpenShift clusters and providing a streamlined developer experience using Eclipse Che
- eclipse/che-machine-exec-plugin/7.8.0 - Che Plug-in with che-machine-exec service to provide creation terminal or tasks for Eclipse CHE workspace containers.
- eclipse/che-theia/7.8.0 - Eclipse Theia
Warning: some plugins in this workspace have conflicting dependencies:
    Plugins
        {ms-kubernetes-tools/vscode-kubernetes-tools/latest, redhat/vscode-openshift-connector/latest}
    conflict on extension
        https://download.jboss.org/jbosstools/vscode/3rdparty/vscode-kubernetes-tools/vscode-kubernetes-tools-1.0.9.vsix
    Plugins
        {ms-kubernetes-tools/vscode-kubernetes-tools/latest, redhat/vscode-openshift-connector/latest}
    conflict on extension
        https://github.com/redhat-developer/vscode-yaml/releases/download/0.4.0/redhat.vscode-yaml-0.4.0.vsix
If you encounter issues with plugins, please disable conflicting plugins above.
All plugin metadata has been successfully processed

The current approach is limited to matching URLs; as a result it can miss some conflicts (e.g. different URLs for the same extension or different versions of the same extension).

Additionally, due to eclipse-che/che#15758, sometimes the order of messages can sometimes be confused slightly.

What issues does this PR fix or reference?

Component of eclipse-che/che#15966

codecov · 2020-02-11T02:05:57Z

Codecov Report

Merging #93 into master will increase coverage by 0.31%.
The diff coverage is 81.48%.

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   73.81%   74.13%   +0.31%     
==========================================
  Files          13       14       +1     
  Lines         634      661      +27     
==========================================
+ Hits          468      490      +22     
- Misses        157      161       +4     
- Partials        9       10       +1

Impacted Files	Coverage Δ
brokers/metadata/broker.go	`69.86% <0%> (-5.14%)`	⬇️
utils/dependencies.go	`100% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac7f5c3...e93e689. Read the comment docs.

nickboldt · 2020-02-11T02:24:39Z

Good idea, error message needs work.

Warning: some plugins in this workspace have conflicting dependencies:
    Plugins
        {ms-kubernetes-tools/vscode-kubernetes-tools/latest, redhat/vscode-openshift-connector/latest}
    conflict on extension
        https://download.jboss.org/jbosstools/vscode/3rdparty/vscode-kubernetes-tools/vscode-kubernetes-tools-1.0.9.vsix
    Plugins
        {ms-kubernetes-tools/vscode-kubernetes-tools/latest, redhat/vscode-openshift-connector/latest}
    conflict on extension
        https://github.com/redhat-developer/vscode-yaml/releases/download/0.4.0/redhat.vscode-yaml-0.4.0.vsix

You should identify WHY it's a conflict... if both k8s and os plugins use the same 1.0.9 version of k8s-tools, then why is it a conflict? I'd suggest something more like:

The following plugins require more than one version of the same extension:
* ms-kubernetes-tools/vscode-kubernetes-tools/latest -> vscode-kubernetes-tools-1.0.9.vsix
* redhat/vscode-openshift-connector/latest -> vscode-kubernetes-tools-1.50.59.vsix

* ms-kubernetes-tools/vscode-kubernetes-tools/latest -> redhat.vscode-yaml-0.4.0.vsix
* redhat/vscode-openshift-connector/latest -> redhat.vscode-yaml-0.54.50.vsix

Also is your call to action even doable?

If you encounter issues with plugins, please disable conflicting plugins above.
All plugin metadata has been successfully processed

eg., if you have a devfile which loads a couple plugins (like k8s and yml), how can you reorganize the devfile to load only ONE version of an extension? Do you need to rebuild the plugin registry to not list more than one extension?

What if we remodel the plugin registry so that it no longer has this 1:1:N mapping of 1 plugin definition to 1 sidecar to N vsix files? If things we 1:1:1 then a user would just enable or disable plugins without having to know about extensions. Or even better... could we add vsix files directly as plugins without the need to wrap them in extra metadata and a sidecar? (I suspect no, but thought it might be worth considering for SOME simple no-runtime-needed extensions like the yaml one.)

sleshchenko

LGTM

utils/dependencies.go

amisevsk · 2020-02-12T03:37:59Z

Updated the log message to be a little cleaner:

Warning: some plugins in this workspace have conflicting dependencies:
  Plugins
    - ms-kubernetes-tools/vscode-kubernetes-tools/0.1.17
    - redhat/vscode-openshift-connector/0.1.2
  both depend on extension
    https://github.com/redhat-developer/vscode-yaml/releases/download/0.4.0/redhat.vscode-yaml-0.4.0.vsix
If you encounter issues with plugins, please disable conflicting plugins above.

amisevsk · 2020-02-12T03:47:40Z

@nickboldt I think you're imagining my fixup is better than it actually is :)

This PR just checks if two plugins use the exact same dependency (including source URL). As for suggesting a better remediation than "turn one of these off", there's not much we can do at the moment. At least the warning will give us something to work off of in bug reports. Your example

The following plugins require more than one version of the same extension:
* ms-kubernetes-tools/vscode-kubernetes-tools/latest -> vscode-kubernetes-tools-1.0.9.vsix
* redhat/vscode-openshift-connector/latest -> vscode-kubernetes-tools-1.50.59.vsix

* ms-kubernetes-tools/vscode-kubernetes-tools/latest -> redhat.vscode-yaml-0.4.0.vsix
* redhat/vscode-openshift-connector/latest -> redhat.vscode-yaml-0.54.50.vsix

will not even be noticed by this PR.

Further improvements depend on eclipse-che/che#15376 and is a part of eclipse-che/che#15966.

Note also that for most of our plugins, the relationship would be strict subsets (e.g. the extensions in vscode-yaml are a subset of the extensions in kuberentes-tooling are a subset of the extensions in openshift-connector). In that case, enabling openshift-connector gives you yaml and kubernetes support. The only exception to this is the camelk extension, which depends on kubernetes-tooling, making it incompatible with openshift-connector. See also the description of PR eclipse-che/che-plugin-registry#355

amisevsk · 2020-02-12T19:39:54Z

@l0rd @benoitf Before I merge and include this in 7.9.0, I'd like to get your signoff on this approach. Is this sufficient for now, given the state of plugin deps? Should we be doing more to guide the user?

benoitf

I think it's a good step so I would merge it ASAP

nickboldt · 2020-02-12T20:20:39Z

My only remaining confusion w/ the updated warning message is that it talks about "conflicting dependencies" but the dependencies are to the SAME plugin... so why is that conflicting?

If the issue is that the user has multiple references to latest which could move unexpectedly, sure. But why is 2 refs to the same plugin at the same URL a problem?

benoitf · 2020-02-12T20:24:52Z

It's because you can only deploy one instance of plugin.

Deploying pluginA on sidecar1 and pluginA on sidecar2 is an error ( pluginA being a dependency on some other plugins for example)

nickboldt · 2020-02-12T20:27:53Z

So which sidecar gets the plugin? First come first plugged? Alphabetical? Random hash?

Explanation makes sense but isn't clear from the statement of "conflicting dependencies" and a newb user might ask the same question I am.

Not blocking push, just wondering if we want to better explain the reason for the conflict, eg.,

Warning: some plugins in this workspace have conflicting dependencies:
  Plugins
    - ms-kubernetes-tools/vscode-kubernetes-tools/0.1.17
    - redhat/vscode-openshift-connector/0.1.2
  both depend on extension
    https://github.com/redhat-developer/vscode-yaml/releases/download/0.4.0/redhat.vscode-yaml-0.4.0.vsix
  but only one can be activated and used at a time. 
If you encounter issues with these plugins, please 
disable all but one of the conflicting plugins above.

benoitf · 2020-02-12T20:33:55Z

It's random

l0rd · 2020-02-13T04:57:58Z

A few change proposals to the warning text (otherwise LGTM):

WARNING instead of Warning
multiple instances of the same extension will be included in this workspace instead of some plugins in this workspace have conflicting dependencies
both depend and embed instead of both depend
As a consequence those plugins may not work as expected. If that's the case try disabling all but one of the conflicting plugins instead of If you encounter issues with plugins, please disable conflicting plugins above.

cc @beaumorley for the message wording

nickboldt

+1 after new message changes.

beaumorley · 2020-02-14T14:40:39Z

Is it possible to paste the final message here?

amisevsk · 2020-02-14T15:50:37Z

@beaumorley With Mario's suggested changes, an example output for the broker, shown during workspace startup, would be

List of plugins and editors to install
- ms-kubernetes-tools/vscode-kubernetes-tools/0.1.17 - Develop, deploy and debug Kubernetes applications
- redhat/vscode-openshift-connector/0.1.2 - Interacting with Red Hat OpenShift clusters and providing a streamlined developer experience using Eclipse Che
- eclipse/che-machine-exec-plugin/0.0.1 - Che Plug-in with che-machine-exec service to provide creation terminal or tasks for Eclipse CHE workspace machines.
- redhat/java/0.43.0 - Java Linting, debugging, Intellisense, formatting, refactoring, Maven/Gradle support and more...
- eclipse/che-theia/next - Eclipse Theia, get the latest release each day.
WARNING: multiple instances of the same extension will be included in this workspace:
  Plugins
    - ms-kubernetes-tools/vscode-kubernetes-tools/0.1.17
    - redhat/vscode-openshift-connector/0.1.2
  both depend on and embed extension
    https://github.com/redhat-developer/vscode-yaml/releases/download/0.4.0/redhat.vscode-yaml-0.4.0.vsix
These plugins may not work as expected. If errors occur please try disabling all but one of the conflicting plugins.
All plugin metadata has been successfully processed

beaumorley · 2020-02-14T17:05:48Z

thanks @amisevsk lgtm!

To warn the user of issues with multiple plugins depending on the same extension, add a step to metadata brokering to detect extensions used by mutliple plugins and print a warning. Current approach is very limited, as it depends on extension URLs matching, so some cases are misse Signed-off-by: Angel Misevski <amisevsk@redhat.com>

amisevsk requested review from l0rd and sleshchenko February 11, 2020 02:05

amisevsk requested review from nickboldt and rhopp as code owners February 11, 2020 02:05

sleshchenko approved these changes Feb 11, 2020

View reviewed changes

utils/dependencies.go Outdated Show resolved Hide resolved

utils/dependencies.go Outdated Show resolved Hide resolved

amisevsk force-pushed the plugin-deps branch from 512f5e4 to 391364a Compare February 12, 2020 03:35

amisevsk requested a review from benoitf February 12, 2020 19:38

benoitf approved these changes Feb 12, 2020

View reviewed changes

nickboldt approved these changes Feb 14, 2020

View reviewed changes

amisevsk force-pushed the plugin-deps branch from 28726d7 to e93e689 Compare February 14, 2020 18:57

amisevsk merged commit ee45bb5 into eclipse-che:master Feb 14, 2020

amisevsk mentioned this pull request Feb 14, 2020

Update plugin brokers to v3.1.1 eclipse-che/che#16046

Merged

l0rd mentioned this pull request Feb 17, 2020

Che Plugins Dependencies Resolution eclipse-che/che#15966

Open

6 tasks

amisevsk mentioned this pull request Feb 18, 2020

Plugin dependencies resolution causes unreported incompatibilites between plugins eclipse-che/che#15794

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check plugins' extensions for conflicts during metadata broker #93

Check plugins' extensions for conflicts during metadata broker #93

amisevsk commented Feb 11, 2020

codecov bot commented Feb 11, 2020 •

edited

Loading

nickboldt commented Feb 11, 2020

sleshchenko left a comment

amisevsk commented Feb 12, 2020

amisevsk commented Feb 12, 2020

amisevsk commented Feb 12, 2020

benoitf left a comment

nickboldt commented Feb 12, 2020

benoitf commented Feb 12, 2020

nickboldt commented Feb 12, 2020 •

edited

Loading

benoitf commented Feb 12, 2020

l0rd commented Feb 13, 2020 •

edited

Loading

nickboldt left a comment

beaumorley commented Feb 14, 2020

amisevsk commented Feb 14, 2020

beaumorley commented Feb 14, 2020

Check plugins' extensions for conflicts during metadata broker #93

Check plugins' extensions for conflicts during metadata broker #93

Conversation

amisevsk commented Feb 11, 2020

What does this PR do?

What issues does this PR fix or reference?

codecov bot commented Feb 11, 2020 • edited Loading

Codecov Report

nickboldt commented Feb 11, 2020

sleshchenko left a comment

Choose a reason for hiding this comment

amisevsk commented Feb 12, 2020

amisevsk commented Feb 12, 2020

amisevsk commented Feb 12, 2020

benoitf left a comment

Choose a reason for hiding this comment

nickboldt commented Feb 12, 2020

benoitf commented Feb 12, 2020

nickboldt commented Feb 12, 2020 • edited Loading

benoitf commented Feb 12, 2020

l0rd commented Feb 13, 2020 • edited Loading

nickboldt left a comment

Choose a reason for hiding this comment

beaumorley commented Feb 14, 2020

amisevsk commented Feb 14, 2020

beaumorley commented Feb 14, 2020

codecov bot commented Feb 11, 2020 •

edited

Loading

nickboldt commented Feb 12, 2020 •

edited

Loading

l0rd commented Feb 13, 2020 •

edited

Loading