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

Improve/fix parsing of plugins/editors attribute #13385

Closed
amisevsk opened this issue May 21, 2019 · 1 comment
Closed

Improve/fix parsing of plugins/editors attribute #13385

amisevsk opened this issue May 21, 2019 · 1 comment
Labels
kind/question Questions that haven't been identified as being feature requests or bugs. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@amisevsk
Copy link
Contributor

Description

There are a few issues with how the plugins/editors attribute in workspaces is parsed and handled:

  1. Plugins with custom registry can cause weird parsing results. E.g. plugin http://registry.com/myPublisher/myPluginName (no version) parses to

    {
      "registry":  null,
      "id":        "com/myPublisher/myPluginName",
      "publisher": "com",
      "name":      "myPublisher",
      "version":   "myPluginName"
    }

    This should instead be treated as an invalid reference.

  2. The imposed structure requires even externally hosted meta.yamls to be placed according to the publisher/name/version structure, without much meaning (as long as there are three / in the URL it would work). We don't use the plugin string for anything meaningful when a non-default 'registry' is used, AFAIK.

  3. Allowing registry to be optionally specified complicates certain other features being implemented, such as defaulting to latest when version is not specified (If the plugin version is not specified - fall back on latest  redhat-developer/rh-che#1396)

With this in mind, we should consider the following paths:

  1. Rework how plugins are defined for workspaces. In general, parsing important information out of a (mostly) arbitrary string is error prone and difficult to understand. Instead, plugins could be specified in json that is friendlier to parsing, e.g.

    { 
      [...]
      "attributes": {
        "plugins": [
          {
            "registry": "https://myregistry.com", # optional; as current
            "id": "publisher/name/version",       # as current
            "reference": "https://myregistry.com/path/to/my/meta.yaml" # optional
          }, 
          [...]
        ]
      }
    }
    

    where reference is an overriding URL to the plugin's meta.yaml. While this is more complicated than previous, ideally end-users should not be editing the raw configs themselves, and the above format may be easier to comprehend.

    This change may also be helpful for the eventual future when the registry gets a REST API.

  2. Remove support for registry in plugin references. Instead of expecting plugins to specify an non-default registry in the plugin reference string ([registry/]publisher/name/version), we restrict plugin references to two separate options:

    • If a plugin reference matches the publisher/name/version spec, we treat it as such and assume it is in the default configured registry. Publisher and name must be alphanumeric plus '-', and version can also contain '.'.
    • If a plugin reference does not match above, we treat it as an arbitrary reference; instead of parsing out publisher, name, and version we treat it as a raw URL pointing at a meta.yaml.

    This approach is maybe simpler to implement currently, but more fragile overall.

cc: @l0rd @ibuziuk WDYT?

@amisevsk amisevsk added the kind/question Questions that haven't been identified as being feature requests or bugs. label May 21, 2019
amisevsk added a commit to amisevsk/che that referenced this issue May 24, 2019
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>
@l0rd l0rd mentioned this issue May 28, 2019
@che-bot
Copy link
Contributor

che-bot commented Nov 20, 2019

Issues go stale after 180 days of inactivity. lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.

Mark the issue as fresh with /remove-lifecycle stale in a new comment.

If this issue is safe to close now please do so.

Moderators: Add lifecycle/frozen label to avoid stale mode.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 20, 2019
@che-bot che-bot closed this as completed Nov 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/question Questions that haven't been identified as being feature requests or bugs. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

2 participants