search

eclipse-che / che

Kubernetes based Cloud Development Environments for Enterprise Teams
http://eclipse.org/che
Eclipse Public License 2.0
6.96k stars 1.19k forks source link

Dashboard IDE selection and devworkspace creation should not use the same displayName variable #21704

Closed nickboldt closed 1 year ago

nickboldt commented 2 years ago

Is your enhancement related to a problem? Please describe

As discovered in CRW-3098 and CRW-3327, the Dashboard (as of 7.52.x = 3.2) presents a list of availale IDEs for a given sample project using the displayName fields in the che-editors.yaml file, then converts the found string to Title Case.

Here's how the IDE selection looks upstream in Che:

image

Note that the names shown are inconsistent (IDEA has a full name, where the other two do not), and "che-code" is not an official trademark-compliant name. Also "Theia-Ide" is less descriptive than "Eclipse Theia" and weirdly cased as "Ide" does not look like an acronym -- it should be "IDE".

I attempted to clean up the displayed names to comply with trademark/legal requirements:

image

But this broke the Dashboard and Factory's ability to launch workspaces w/ theia and vscode as the same displayName is used to create the devworkspace*.yaml files in the offline devfile registry.

So, this is problematic for a number of reasons:

image

Describe the solution you'd like

  1. We could add a new field to the che-editors.yaml that can be used to generate the workspace subdomain prefix, eg., workspacePrefix = {idea, theia, vscode}
  2. We could add a new field for the Dashboard to use when displaying the IDE list, eg., selectionName = {"IntelliJ IDEA Community", "Eclipse Theia", "VS Code - Open Source"}

Describe alternatives you've considered

No response

Additional context

No response

svor commented 2 years ago

Probably as an another solution we can improve libraries that generate DevWorkspace templates to check that DevWorkspaceTemplate metadata.name contains only lower case alphanumeric characters, '-' or '.' and I think another check should be done on dashboard side.

In that case we won't need to add new fields

nickboldt commented 1 year ago

Adding a compliance check would be cool, but I still won't be able to pass pretty strings to the Dashboard UI.

And even if you clean up the passed in string by removing spaces and lowercasing things ... if the string I pass in is "VS Code - Open Source" and you turn that into "vscode-opensource" we might still end up with errors if the workspace subdomain ends up too long. "IntelliJ IDEA Community" ==> "intellijideacommunity" which is also a long string for a prefix.

Could we improve the library so that we:

That would give us 4 or 5 chars as the workspace name prefix:

Upstream we have right now:

image

So we could use the same strings/logic above + add pycharm:

ibuziuk commented 1 year ago

The dashboard displays the values coming from https://github.com/eclipse-che/che-plugin-registry/blob/main/che-editors.yaml as-is. If anything need to be changed there please provide a fix and let us know. On the dashboard end we are not going to add any extra logic related to editor names - all the metadata should come from the registry

nickboldt commented 1 year ago

First two PRs:

Once merged, will need a PR for Dashboard too, as we'll hit this error: 

displayName must conform to lowercase RFC 1123 subdomain rules

-- https://github.com/redhat-developer/devspaces/commit/0c64a21414555c35ef93391a093ac70f13b190f8

I'm not sure what needs to change but I'm guessing it's something to do with editorId or how we load targetEditors from the plugin registry...

nickboldt commented 1 year ago

I think we can resolve this as we have updated the display names in the dashboard and can still open workspaces. See https://issues.redhat.com/browse/CRW-3098