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

factory resolver misses volume component from devfile 2.0 #19563

Closed
23 tasks
sleshchenko opened this issue Apr 12, 2021 · 4 comments
Closed
23 tasks

factory resolver misses volume component from devfile 2.0 #19563

sleshchenko opened this issue Apr 12, 2021 · 4 comments
Labels
area/che-server engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.

Comments

@sleshchenko
Copy link
Member

sleshchenko commented Apr 12, 2021

Describe the bug

Factory resolver misses volume component from devfile 2.0 with leads invalid devfile:

"message": "DevWorkspace.workspace.devfile.io "spring-petclinic" is invalid: [\u003cnil\u003e: Invalid value: "": "spec.template.components" must validate one and only one schema (oneOf). Found none valid, spec.template.components.container: Required value]",
Resolved devfile: mavenrepo component has only name.

{
  "v": "4.0",
  "devfile": {
    "schemaVersion": "2.0.0",
    "metadata": {
      "name": "spring-petclinic"
    },
    "components": [
      {
        "name": "maven",
        "container": {
          "image": "quay.io/eclipse/che-java8-maven:nightly",
          "volumeMounts": [
            {
              "name": "mavenrepo",
              "path": "/root/.m2"
            }
          ],
          "env": [
            {
              "name": "ENV_VAR",
              "value": "value"
            }
          ],
          "memoryLimit": "1536M"
        }
      },
      {
        "name": "mavenrepo"
      }
    ]
  }
}

Che version

  • latest
  • nightly
  • other: please specify

Steps to reproduce

  1. Deploy Che with devworkspace support enabled.
  2. Accept factory from devfile with volume component, like $CHE_HOST#https://gist.githubusercontent.com/sleshchenko/e6382a45094b520118db2e5a318cc050/raw/b7d445c943c512092fea28206108d234aa761478/devfile2.yaml

Expected behavior

Che Server returns a valid devfile, which is a valid, so it's possible to create devworkspace from it.

Runtime

  • kubernetes (include output of kubectl version)
  • Openshift (include output of oc version)
  • minikube (include output of minikube version and kubectl version)
  • minishift (include output of minishift version and oc version)
  • docker-desktop + K8S (include output of docker version and kubectl version)
  • other: (please specify)

Screenshots

Installation method

  • chectl
    • provide a full command that was used to deploy Eclipse Che (including the output)
    • provide an output of chectl version command
  • OperatorHub
  • I don't know

Environment

  • my computer
    • Windows
    • Linux
    • macOS
  • Cloud
    • Amazon
    • Azure
    • GCE
    • other (please specify)
  • Dev Sandbox (workspaces.openshift.com)
  • other: please specify

Eclipse Che Logs

Additional context

@sleshchenko sleshchenko added kind/bug Outline of a bug - must adhere to the bug report template. engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. area/che-server labels Apr 12, 2021
@che-bot che-bot added the status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. label Apr 12, 2021
@Katka92 Katka92 added severity/P1 Has a major impact to usage or development of the system. and removed status/need-triage An issue that needs to be prioritized by the curator responsible for the triage. See https://github. labels Apr 12, 2021
@mshaposhnik
Copy link
Contributor

So since we didn't have model for Devfile v2, we serialize it as just a Map<String, Object> where inner objects are also Maps etc.
And due to this code, since legacy mode seems to be turned off,
the NullOrEmptyCollectionAdapter removes empty fields. cc @skabashnyuk @sparkoo any ideas how to fix ?

@sparkoo
Copy link
Member

sparkoo commented Apr 23, 2021

So when is this information lost? When parsing the devfile yaml into Map? Or when translating the Map into yaml content when creating the response for api call?

Is NullOrEmptyCollectionAdapter still relevant? Do we really need it?

What about parsing the devfile into JsonObject (or whatever is the class) instead of Map? Would it help? What about keep it as raw string and parse to Map only for validation?

@mshaposhnik
Copy link
Contributor

mshaposhnik commented Apr 23, 2021

when converting FactoryV2Dto into json type response.

@skabashnyuk
Copy link
Contributor

At this point, we can't fix this issue for factory.resolve method without massive compatibility registration. Reason for that #13515 when we do not serialize maps for all DTO. alternative fo that 1. Do not add a component with empty content or sooner we are going to implement an alternative method to get content of the file Allow resolver API to resolve other files than [.]devfile.yaml #19540

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/che-server engine/devworkspace Issues related to Che configured to use the devworkspace controller as workspace engine. kind/bug Outline of a bug - must adhere to the bug report template. severity/P1 Has a major impact to usage or development of the system.
Projects
None yet
Development

No branches or pull requests

6 participants