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

Use validations as specified by devfile library & api #4361

Closed
maysunfaisal opened this issue Jan 14, 2021 · 6 comments · Fixed by #4512
Closed

Use validations as specified by devfile library & api #4361

maysunfaisal opened this issue Jan 14, 2021 · 6 comments · Fixed by #4512
Assignees
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it.

Comments

@maysunfaisal
Copy link
Contributor

Currently the devfile generic validations are in pkg https://github.com/openshift/odo/tree/master/pkg/devfile/validate/generic. Once the issue devfile/api#151 to consolidate validations in devfile api is done, odo should switch to that version instead of maintaining the generic pkg copy.

@maysunfaisal maysunfaisal added the area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. label Jan 14, 2021
@dharmit
Copy link
Member

dharmit commented Jan 19, 2021

@kadel I see you added this for consideration in Sprint 196, but I don't think we can start working on this before the issue devfile/api#151 mentioned in the description is addressed. Also, as mentioned in devfile/api#151 (comment), wouldn't we require a list (doc?) of validations that need to be moved out of odo?

@maysunfaisal
Copy link
Contributor Author

maysunfaisal commented Jan 19, 2021

@dharmit the validation has been refactored in https://github.com/openshift/odo/tree/master/pkg/devfile/validate/generic which contains generic validation not specific to odo. These have been moved out to the doc https://github.com/devfile/api/blob/master/pkg/validation/validation-rule.md (covers all validations on top of those moved out from the generic pkg in odo) and implemented in the PR devfile/api#301 which is up for review.

The events issue will however need to be addressed - #4187. It can be in the same PR or handled with a different PR.

@yangcao77
Copy link
Contributor

After this PR (devfile/library#61) has been merged, Odo can pull in latest devfile/library and devfile/api, start consuming validator from devfile/library.
the validation has been called in ParseAndValidate already, so Odo should just remove validate.ValidateDevfileData after every call to ParseAndValidate
For example:
https://github.com/openshift/odo/blob/adaa1b44e8de5ab0aa0d357294174f8efb8bdd24/pkg/odo/cli/component/test.go#L64-L71

Other tips on adopting the latest devfile/api and devfile/library pkg:

  1. Since library wants all functions to be more generic, the return type of GetCommands has been changed to []v1.Command. Odo is currently expecting a map [command.Id]v1.Command.

  2. Library added filter to GET functions. Therefore, functions like GetCommands, GetComponents, GetProjects/GetStarterProject etc., are expecting a param with typecommon.DevfileOptions. Odo does not have specific filter applied, so should just pass in an empty filter common.DevfileOptions{}. In addition, those functions return an extra err, which needs to be caught and returned.

  3. The name and id pattern has been added into schema, Odo test code need to make change accordingly Odo test samples and resgirties need to match id/name pattern defined in devfile 2.0 schema #4393

@mik-dass
Copy link
Contributor

mik-dass commented Mar 3, 2021

We need to replace the use of the functions in https://github.com/openshift/odo/tree/master/pkg/devfile/validate/generic to the functions in the devfile api's https://github.com/devfile/api/tree/master/pkg/validation

@maysunfaisal
Copy link
Contributor Author

@mik-dass You dont have to call the individual funcs in devfile/api. If you're using the latest devfile/library this is already covered by the func here https://github.com/devfile/library/blob/master/pkg/devfile/parse.go#L52-L67

@mik-dass
Copy link
Contributor

mik-dass commented Mar 4, 2021

@mik-dass You dont have to call the individual funcs in devfile/api. If you're using the latest devfile/library this is already covered by the func here https://github.com/devfile/library/blob/master/pkg/devfile/parse.go#L52-L67

Thanks @maysunfaisal 👍

We will need replace the generic library call here https://github.com/openshift/odo/blob/2f7f4a090674e404467f0295bd5ceb8acdd528c6/pkg/devfile/validate/validate.go#L21 to the devfile api one.

We will also need to find ways to handle/ignore errors like https://github.com/openshift/odo/blob/2f7f4a090674e404467f0295bd5ceb8acdd528c6/pkg/devfile/adapters/common/command.go#L76-L78
or remove them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants