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

Workflow Open API validation #859

Closed
vicaire opened this issue May 19, 2018 · 7 comments · Fixed by #3075
Closed

Workflow Open API validation #859

vicaire opened this issue May 19, 2018 · 7 comments · Fixed by #3075
Labels
type/feature Feature request

Comments

@vicaire
Copy link

vicaire commented May 19, 2018

Is this a BUG REPORT or FEATURE REQUEST?: FEATURE REQUEST

Do you have plans to support validation of the Argo workflow spec?

It should be added here:

https://github.com/argoproj/argo/blob/06c0d324bf93a037010186fe54e40590ea39d92c/install/manifests/01_workflow-crd.yaml

Here is the documentation:

https://kubernetes.io/docs/tasks/access-kubernetes-api/extend-api-custom-resource-definitions/

Here is a sample:

apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
name: crontabs.stable.example.com
spec:
group: stable.example.com
version: v1
scope: Namespaced
names:
plural: crontabs
singular: crontab
kind: CronTab
shortNames:
- ct
validation:

openAPIV3Schema is the schema for validating custom objects.

openAPIV3Schema:
  properties:
    spec:
      properties:
        cronSpec:
          type: string
          pattern: '^(\d+|\*)(/\d+)?(\s+(\d+|\*)(/\d+)?){4}$'
        replicas:
          type: integer
          minimum: 1
          maximum: 10
@jessesuen
Copy link
Member

Yes, I agree with this. I even went so far as to generate an open-api schema for exactly this purpose:
https://github.com/argoproj/argo/blob/master/api/openapi-spec/swagger.json

The main motivation for having validating schema is to avoid problems like this:
#632

That said, adding the OpenAPI validation schema to the CRD needs to be optional because once you define the schema, you may have trouble running multiple controllers at different argo versions -- which will cause problems when bundling the argo workflow controller as part of a higher level application.

Also, the internal struct representation of Item object, will be changing in the near future to support protobuf encoding. So we will want to wait for this to come in:
#853

@vicaire
Copy link
Author

vicaire commented May 24, 2018

Sounds good. It makes sense to have this optional.

@jessesuen jessesuen changed the title Argo workflow validation Workflow Open API validation Apr 19, 2019
@jessesuen jessesuen added the type/feature Feature request label Apr 19, 2019
@stormmore
Copy link

Do we have any ETA for this enhancement? I filed argoproj/argo-events#281 for the same reasons and I suspect that it impacts all the CRDs used by Argo - Worflows, Events, CD.

I did see that Rollouts has the validation in there

@danxmoran
Copy link
Contributor

danxmoran commented Jan 14, 2020

Re: Making it optional, it looks like that won't be possible if/when the CRDs update from apiextensions.k8s.io/v1beta1 to apiextensions.k8s.io/v1. I'm not sure if it's feasible to keep using the beta API version forever (some version of k8s will drop support for it eventually? not sure)

Edit: forgot to link: https://kubernetes.io/docs/tasks/access-kubernetes-api/custom-resources/custom-resource-definitions/#specifying-a-structural-schema

@logicfox
Copy link
Contributor

logicfox commented Mar 5, 2020

@jessesuen Now that Argo 2.6 has an OpenAPI spec (https://github.com/argoproj/argo/blob/v2.6.1/api/openapi-spec/swagger.json#L2526-L2680), can we not use that to solve this issue and migrate to apiextensions.k8s.io/v1?

@crenshaw-dev
Copy link
Contributor

My use case is just Intellij linting. Validation wouldn't even have to be built into the CRD that gets installed. It would just have to exist in another file that can be imported to Intellij's Kubernetes plugin.

@alexec
Copy link
Contributor

alexec commented May 4, 2020

@jessesuen @mukulikak I'm gathering some notes. The lack of YAML validation is one of the key reason that people want to use a Python DSL. I think we should revisit this popular issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature Feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants