Remove plugins from the Devfile schema

[davidfestal]

What does this PR do?

This PR proposes the very light changes required to remove the plugin components from the user-facing Devfile Json schema, while keeping them in GO sources and Kubernetes CRDs, mainly at the SPI level, as a way for tooling implementations to complete a devfile with additional content according to their own logic.

This PR is created as a Draft, since I didn't change the samples, as well as other possibly required changes in tests, etc ...
Its purpose was mainly to contribute the required change in the generator source code.

So this PR should be completed with according changes in samples, tests, etc... before being mergeable.

What issues does this PR fix or reference?

Not sure an issue has been created already in this GH repo. But this removal of plugins from devfile schema has been discussed in the Devfile Cabal and forseen as a change that should be done asap, as soon as a complete agreement with all stakeholders has been reached.

--
EDIT by Maysun:
Fixes #407

Is your PR tested? Consider putting some instruction how to test your changes

Yes: Just open the existing devfile samples (in VSCode typically) and see how each plugin component is flagged with the following json schema validation error: Property plugin is not allowed

Docs PR

[amisevsk]

Not a comment on the code directly, but I have some concerns about removing plugins from the devfile spec:

• It's unclear how to specify basic required plugins -- e.g. how do I say in my devfile that I want to use a specific editor? Would this be done via an annotation? Maybe I have a devfile that's intended for use with editor X but whatever is processing my devfile uses editor Y as a default
• It leaves a large section of the codebase in this repo as internal/not public. This is hard to explain in general.
• Having a different spec between devfile and devworkspace leaves a gap in terms of definition between a devfile and the related DevWorkspace and changes a few semantics in an unclear way. Every time you process a devfile you could potentially get a different outcome.
• Removing plugins from the devfile spec effectively makes plugins Theia-specific functionality; from the DevWorkspace Operator/Che perspective, removing plugins from devfile forces all editors to implement a system similar to the current planned Theia plugin functionality if they want sidecars

In my opinion, these downsides outweigh the benefit of making the devfile spec simpler. This change would force the full definition of a DevWorkspace to depend on a) the devfile and b) whatever program converts the devfile into a DevWorkspace, which makes this less clear in some cases.

[elsony]

@maysunfaisal @yangcao77 Are there any changes needed in the library to respond to this change?
@mmulholla Are there any changes needed in the tests to respond to this change?

[yangcao77]

Yes. Library now parses plugin if uri is provided. After this PR landed, library needs to remove plugin support.

[mmulholla]

None of the tests currently merged use plugins

[l0rd]

@amisevsk some comments to your concerns:

It's unclear how to specify basic required plugins -- e.g. how do I say in my devfile that I want to use a specific editor? Would this be done via an annotation? Maybe I have a devfile that's intended for use with editor X but whatever is processing my devfile uses editor Y as a default

We have some specific issues in eclipse/che to discuss that. I have added a commented here to clarify this particular point.

It leaves a large section of the codebase in this repo as internal/not public. This is hard to explain in general.

I agree. But explaining to a devfile author what’s a plugin, and why it’s useful would have been harder. I think removing plugins from plugins only is fine as a temporary solution. But in the future we should decide if we want to introduce a better abstraction in the devfile spec (editors and editors' plugins) or we want to definitely remove plugins from the DevWorkspace CRD too.

Having a different spec between devfile and devworkspace leaves a gap in terms of definition between a devfile and the related DevWorkspace and changes a few semantics in an unclear way. Every time you process a devfile you could potentially get a different outcome.

True: Che adds implicit components that are not included in the original devfile. We have been doing that for v1 devfiles and nobody complained. On the other hand we don’t want the DevWorkspace controller to add implicit components. DevWorkspace should not have any notion of Editor or Editor plugin. Che adds those implicit components based on it's internal logic.

Removing plugins from the devfile spec effectively makes plugins Theia-specific functionality; from the DevWorkspace Operator/Che perspective, removing plugins from devfile forces all editors to implement a system similar to the current planned Theia plugin functionality if they want sidecars

But that was the issue: users were confused because they expected plugins to be universal, cross editor although they were Che Theia specific and we had no plan to support other editors.

[amisevsk]

But in the future we should decide if we want to introduce a better abstraction in the devfile spec (editors and editors' plugins) or we want to definitely remove plugins from the DevWorkspace CRD too.

This sounds like it's drifting further toward "more che-specific", which was the main reason for wanting to remove plugins in the first place. I'd suggest we rename plugins to "templates" or something similar, to better represent what they do. Removing plugins from the DevWorkspace CRD has very significant design implications across the DevWorkspace and Web Terminal operators, so if this is on the table we need to discuss plans and timeline.

Che adds implicit components that are not included in the original devfile. We have been doing that for v1 devfiles and nobody complained.

We do this for non-user-facing components; this is for the main entrypoint into a workspace. I think these cases are different. Before we go ahead with this I'd like at least a clear answer on how I select my editor for a given devfile.

But that was the issue: users were confused because they expected plugins to be universal, cross editor although they were Che Theia specific and we had no plan to support other editors.

There are much easier ways to solve this problem than by changing the spec, IMO. Users were confused because they could press a toggle to add the java LSP to IntelliJ; we should remove that toggle. The whole UI around this was created to suggest that plugins were universal.

[l0rd]

This sounds like it's drifting further toward "more che-specific", which was the main reason for wanting to remove plugins in the first place. I'd suggest we rename plugins to "templates" or something similar, to better represent what they do. Removing plugins from the DevWorkspace CRD has very significant design implications across the DevWorkspace and Web Terminal operators, so if this is on the table we need to discuss plans and timeline.

Because the abstraction was wrong plugins have been misused in Che and not used at all by other tools. To that we should add that in v2 we have introduced parents. And a generic plugins/template abstraction overlaps with parents a lot. I am worried that this is only going to confuse users and adopters. That's why we are removing plugins.

On the other hand adding an "Editor" recommendation in the definition of a development workspace makes things clear.

We do this for non-user-facing components; this is for the main entrypoint into a workspace. I think these cases are different. Before we go ahead with this I'd like at least a clear answer on how I select my editor for a given devfile.

Editors are non-user-facing? I have updated the issue with the details about the plugins and editors support in devfile v2 based Che workspaes. Feedback would be appreciated.

There are much easier ways to solve this problem than by changing the spec, IMO. Users were confused because they could press a toggle to add the java LSP to IntelliJ; we should remove that toggle. The whole UI around this was created to suggest that plugins were universal.

The devfile UI has influenced Che User Dashboard UI. The dashboard has been one of the first victims of that false promise of the Devfile v1 plugins 😄

[amisevsk]

And a generic plugins/template abstraction overlaps with parents a lot

I would say it doesn't, there are probably fewer use cases for parents than there are for plugins, since you can only have one parent. With the plugins mechanism, I could write a "postgres" or "mongoDB" plugin that can be reused in any devfile since it would just convert to containers, volumes, and endpoints.

Considering how we don't have many tools using the spec fully, it's hard to say "nobody is using <feature>" -- I use plugins every day, they're a very useful abstraction. If removing plugins from the DevWorkspace itself is on the table I potentially have to throw out months of work.

[amisevsk]

Speaking from a more DevWorkspace Operator-specific perspective -- suppose I wanted to e.g. work on kubernetes/kubernetes using a devfile I write. How do I define a devfile that starts with Theia as my editor and the Go plugin installed?

[l0rd]

I would say it doesn't, there are probably fewer use cases for parents than there are for plugins, since you can only have one parent. With the plugins mechanism, I could write a "postgres" or "mongoDB" plugin that can be reused in any devfile since it would just convert to containers, volumes, and endpoints.

You are right. Parents are more restrictive than plugins. In both cases you are referencing a Devfile from another Devfile but you can compose plugins. As a user I would probably want to use plugins rather than parents. I would be confused about the use case for parents. I would not know if a maven container should be a parent or a plugin for instance.

Considering how we don't have many tools using the spec fully, it's hard to say "nobody is using " -- I use plugins every day, they're a very useful abstraction. If removing plugins from the DevWorkspace itself is on the table I potentially have to throw out months of work.

I only would like to put that on hold. If we remove plugins now, we will still be able to add back a new (better) component type in next releases. If we keep them it will hard to improve the spec once devfile v2 get adopted.

[amisevsk]

I only would like to put that on hold. If we remove plugins now, we will still be able to add back a new (better) component type in next releases. If we keep them it will hard to improve the spec once devfile v2 get adopted.

We still need to think about compatibility, in terms of the DevWorkspace resource. Despite my reservations, I'm fine removing plugins from devfile spec (from the DevWorkspace Operator perspective), but a breaking change in what plugins are on the DevWorkspace side will be very difficult to support already. If the goal is to have a different user-facing description of the concepts behind plugins, which resolves to something compatible with the current implementation of plugins on the DevWorkspace side, then that makes perfect sense to me. If adding a new, better component means a breaking change in Plugin components, then that's very difficult. From a DevWorkspace Operator perspective, I'm happy as long as I can write some Go code that takes a devfile written today (with plugins) and converts it to whatever new format we choose.

[l0rd]

If the goal is to have a different user-facing description of the concepts behind plugins, which resolves to something compatible with the current implementation of plugins on the DevWorkspace side, then that makes perfect sense to me.

@amisevsk I have created #364. Are you ok if we move forward with this PR and continue the discussion there?

[maysunfaisal]

@amisevsk @davidfestal can you pls have a look? I've rebased and updated the tests for this PR

[elsony]

Removing the milestone. Removal of the plugin from the schema is now tracked by issue #407 on the milestone plan.

[amisevsk]

I can't see any problems with this approach, except that dealing with devfiles-as-a-struct might be confusing (there are fields that aren't in the schema). I'm fine with this since the alternative is using different structs for the DevWorkspace/Template spec and devfile contents, which is harder to manage.

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: amisevsk, davidfestal

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment