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

Publish OpenAPI schemas separately for CRD validation schemas #4477

Closed
brabster opened this issue Nov 6, 2020 · 20 comments
Closed

Publish OpenAPI schemas separately for CRD validation schemas #4477

brabster opened this issue Nov 6, 2020 · 20 comments
Labels
type/feature Feature request

Comments

@brabster
Copy link
Contributor

brabster commented Nov 6, 2020

Summary

Publish the OpenAPI schemas that are used for CRD validation as standalone schemas for use in applications that are OpenAPI-aware, but are not K8s CRD validation-aware.

Use Cases

CRDs are published and can be used with Kubernetes-aware IDE capabilities to provide schema validation.

VSCode has no extensions with the appropriate level of Kubernetes awareness to use them directly, but it does have an extension supporting YAML validation that does work with the OpenAPI schemas if they are standalone rather than embedded. See https://argoproj.slack.com/archives/C8J6SGN12/p1604444305320200 for details of VSCode configuration.

Propose to publish the embedded OpenAPI schemas alongside the CRDs that embed them for use in OpenAPI-aware but not K8s CRD-validation aware applications.

Example

Example Schema

https://gist.github.com/brabster/5a64fd7f10ef824c06a09f1d9cbdba5d

This is the content of the openAPIV3Schema tag in https://raw.githubusercontent.com/argoproj/argo/master/manifests/base/crds/full/argoproj.io_workflows.yaml. The only added fields is "title" which is rendered in VSCode to indicate what is providing the validation error. With that in mind, a title including the version details might be helpful.

Publishing the schemas in this form certainly works with the VSCode plugin.

Example VSCode settings.json snippet

"yaml.schemas": {
        "https://gist.githubusercontent.com/brabster/ffbd735d5c819dec926ee727350402fa/raw/256d6f75363293a3806eb386c6f65984ecc61673/argo-workflowtemplate-schema-2.11.7.yaml": "*.wftemplate.yaml",
        "https://gist.githubusercontent.com/brabster/9d849d0db50ca01cb3b8054ffa1b78a5/raw/83b7c47784a373002274a17b7405f29f9b45819c/argo-cronworkflow-schema-2.11.7.yaml": "*.cronwf.yaml",
        "https://gist.githubusercontent.com/brabster/5a64fd7f10ef824c06a09f1d9cbdba5d/raw/75eae9fac885cbbfa722f38b079384efba10d2b9/argo-workflow-schema-2.11.7.yaml": "*.wf.yaml"
    }

VSCode UX

image


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

@brabster brabster added the type/feature Feature request label Nov 6, 2020
@alexec
Copy link
Contributor

alexec commented Nov 6, 2020

We publish the OpenAPI definitions:

https://raw.githubusercontent.com/argoproj/argo/master/api/openapi-spec/swagger.json

We use them in the UI for code-completion.

Could you use that?

@brabster
Copy link
Contributor Author

brabster commented Nov 7, 2020

I did try that at first but I had no luck - doesn't work with the YAML extension and there's no other extension I could find. Same problem as the schemas embedded in the CRDs, I think. That definition describes a while API - the schema I need to validate a workflow is embedded in there but there's no simple way to pull it out. Thus my thought about generating/publishing the data schemas separately

I've been trying to work out how the CRDs are generated in the repo - expected it to be part of the CI pipeline but looks like they're checked in. Is there any guidance on how that works?

@alexec
Copy link
Contributor

alexec commented Nov 7, 2020

To generate crds run make codegen.
Kubernetes OpenAPI spec is verbose and kind-of non-standard. E.g. it inlines types and has some special types. You're better on basing your schema on hack/swagger/secondaryswaggergen.go:

func secondarySwaggerGen() {
	definitions := make(map[string]interface{})
	for n, d := range wfv1.GetOpenAPIDefinitions(func(path string) spec.Ref {
		return spec.Ref{
			Ref: jsonreference.MustCreateRef("#/definitions/" + strings.ReplaceAll(path, "/", ".")),
		}
	}) {
		n = strings.ReplaceAll(n, "/", ".")
		println(n)
		definitions[n] = d.Schema
	}
	swagger := map[string]interface{}{
		"definitions": definitions,
	}
	data, err := json.MarshalIndent(swagger, "", "  ")
	if err != nil {
		panic(err)
	}
	err = ioutil.WriteFile("pkg/apiclient/_.secondary.swagger.json", data, 0644)
	if err != nil {
		panic(err)
	}
}

pkg/apiclient/_.secondary.swagger.json is in .gitignore, but I wonder if it could work for you?

@brabster
Copy link
Contributor Author

Generating that secondary.swagger.json and wrapping it with:

{
    "$schema": "http://json-schema.org/schema#",
    "$id": "http://foo.com/bar/bazschema.json",
    "type": "object",
    "oneOf": [
        {
            "$ref": "#/definitions/github.com.argoproj.argo.pkg.apis.workflow.v1alpha1.CronWorkflow"
        },
        {
            "$ref": "#/definitions/github.com.argoproj.argo.pkg.apis.workflow.v1alpha1.Workflow"
        }
    ],

gets me reasonably far - I get the appropriate validation errors and autocomplete working. I removed anything referencing the k8s API as I can't resolve any of that directly - but doing so does remove a lot of the value, e.g. the container key is defined in the k8s spec. Also not sure the schema is actually valid as a specification for the YAML files we create as opposed to the API, e.g. github.com.argoproj.argo.pkg.apis.workflow.v1alpha1.CronWorkflowStatus is in there.

Not sure where to go next.

@alexec
Copy link
Contributor

alexec commented Nov 16, 2020

@brabster can you still demo on Wednesday? thank you.,

alexec added a commit to alexec/argo-workflows that referenced this issue Nov 16, 2020
Signed-off-by: Alex Collins <alex_collins@intuit.com>
@alexec
Copy link
Contributor

alexec commented Nov 16, 2020

@brabster
Copy link
Contributor Author

brabster commented Nov 16, 2020

Almost... doesn't validate because all the oneOf schemas match my CronWorkflow and that's invalid. I fixed that (and forgot to say so) by setting the kind to the correct const value, allowing differentiation... eg

"kind": {
          "description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.io.k8s.community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
          "const": "Workflow"
        },

When I add the const Kinds for the 5 types in the oneOf, I detect two problems in my CronWorkflow (correctly, I think):

image

@brabster
Copy link
Contributor Author

@brabster can you still demo on Wednesday? thank you.,

If we can get the schema published (which is looking hopeful!) then sure! I wouldn't think it'd be helpful to demo something before folks can really use it.

@alexec
Copy link
Contributor

alexec commented Nov 16, 2020

can you show how it should change please?

@brabster
Copy link
Contributor Author

https://gist.github.com/brabster/04cfc4410bb133b9341fa9793044c9d6#file-argo-schema-with-discriminator-values-json-L428

Line 428, switched "type":"string" to "const":"CronWorkflow" - same for the other 4 types of YAML

@alexec
Copy link
Contributor

alexec commented Nov 16, 2020

so we use kind as a type discriminator - I get it now

@brabster
Copy link
Contributor Author

Also happy to do a docs update with instructions for VSCode when the schema is available at a stable location 👍

@alexec
Copy link
Contributor

alexec commented Nov 16, 2020

@brabster
Copy link
Contributor Author

brabster commented Nov 17, 2020

OK that's working pretty well now. Trying it on more complex files I've found a problem with the schema, io.argoproj.workflow.v1alpha1.DAGTask says that template is required, but it's not if you use a templateRef - not sure how that's happened...?

Maybe that's a different issue. Certainly that schema seems to work correctly with the VSCode YAML plugin 🥇

@alexec
Copy link
Contributor

alexec commented Nov 17, 2020

Cool. Will you be able to demo tomorrow at the community meeting still please?

@brabster
Copy link
Contributor Author

Yep. Will ping on Slack.

@brabster
Copy link
Contributor Author

Thought occurred to me - IDEA also supports YAML validation via JSON Schema natively (the current instructions require the Kubernetes plugin - only works on the for-$$$ Ultimate Edition which is why I couldn't try it)

image

Maybe switching over would have the added benefit of only one schema to support for IDEs?

@alexec
Copy link
Contributor

alexec commented Nov 18, 2020

Maybe switching over would have the added benefit of only one schema to support for IDEs?

Good idea!

brabster pushed a commit to brabster/argo that referenced this issue Nov 24, 2020
Signed-off-by: Alex Collins <alex_collins@intuit.com>
Signed-off-by: Paul Brabban <paul.brabban@gmail.com>
@alexec
Copy link
Contributor

alexec commented Nov 24, 2020

Good work!

@alexec alexec closed this as completed Nov 24, 2020
@alexec
Copy link
Contributor

alexec commented Nov 24, 2020

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.

2 participants