search

fabric8io / kubernetes-client

Java client for Kubernetes & OpenShift
http://fabric8.io
Apache License 2.0
3.41k stars 1.46k forks source link

CRD-Generator: Overview about validation constraints / Kubernetes OpenAPI schema properties #5859

Open baloo42 opened 7 months ago

baloo42 commented 7 months ago

Is your enhancement related to a problem? Please describe

At the moment the CRD-Generator does not allow to set all possible details to the resulting CRDs.

Describe the solution you'd like

The CRD-Generator supports all Kubernetes features to describe a CRD.

Additional context

This issue aims to provide an overview of the current state and what we plan to implement.

[!IMPORTANT] This overview is now targeting api-v2!

Current state

Annotation Description
com.fasterxml.jackson.annotation.JsonProperty The field is named after the provided value instead of looking up the java field name
com.fasterxml.jackson.annotation.JsonPropertyDescription The provided text is be embedded in the description of the field
com.fasterxml.jackson.annotation.JsonIgnore The field is ignored
io.fabric8.crd.generator.annotation.PreserveUnknownFields The field have x-kubernetes-preserve-unknown-fields: true defined
com.fasterxml.jackson.annotation.JsonAnyGetter The corresponding object have x-kubernetes-preserve-unknown-fields: true defined
com.fasterxml.jackson.annotation.JsonAnySetter The corresponding object have x-kubernetes-preserve-unknown-fields: true defined
io.fabric8.generator.annotation.Min The field defines a validation min
io.fabric8.generator.annotation.Max The field defines a validation max
io.fabric8.generator.annotation.Pattern The field defines a validation pattern
io.fabric8.generator.annotation.Nullable The field is marked as nullable
io.fabric8.generator.annotation.Required The field is marked as required
io.fabric8.generator.annotation.ValidationRule The field or object is validated by a CEL rule (x-kubernetes-validation-rules)
io.fabric8.crd.generator.annotation.SchemaFrom The field type for the generation is the one coming from the annotation
io.fabric8.crd.generator.annotation.SchemaSwap Similar to SchemaFrom, but can be applied at any point in the class hierarchy
io.fabric8.crd.generator.annotation.Annotations Additional annotations in metadata
io.fabric8.crd.generator.annotation.Labels Additional labels in metadata
io.fabric8.crd.generator.annotation.PrinterColumn Customize columns shown by the kubectl get command

Missing schema properties

Property Description
minLength/maxLength see #5836
minItems/maxItems see #5836
minProperties/maxProperties see #5836
exclusiveMinimum / exclusiveMaximum see #5868
multipleOf see #5881
x-kubernetes-map-type see #5881
x-kubernetes-list-type see #5881
x-kubernetes-list-map-keys see #5881
x-kubernetes-embedded-resource see #5881
format see #5881, #5867 (PR: #5885)
externalDocs see #5881
example see #5881

Other missing properties

Property Description
conversion see #5794 (includes NoneConversion and WebhookConversion), PR: #5875
categories see #5795

Not Supported / Useless

andreaTP commented 6 months ago

Quick note: this is a pretty large change and we can prepare for it, but I would like to roll it out when we start working on version 7.X of this repo.

baloo42 commented 6 months ago

When do you plan to release v7 ?

Can we chat somewhere on this topic?

baloo42 commented 6 months ago

My general suggestion on this topic is to create issues for each property first. Those issues should describe the solution (intended annotation(s), addditional structural validations) to get a clear picture of what the overall solution could look like.

If we have an agreement on the overall solution, we can prioritize and maybe also group it a little bit.

Please keep also in mind, that all missing properties from above could be implemented in a non breaking way.

But if you think this would be too dangerous, we can also talk about a new side-project, where we can try out things faster. Something like "Next-CRD-Generator".

andreaTP commented 6 months ago

Can we chat somewhere on this topic?

Yes, we are meeting every Wednesday (the next one is tomorrow), my private email is public on my GH profile, please reach me and I'll forward an invite, @matteriben feel free to do the same as you are working on overlapping concerns.

adriansuarez commented 3 months ago

Not sure if my comment is relevant, but I'll go ahead and make it here before creating a new issue.

What I would like to see besides more comprehensive support for validation constraints, which is what this issue seems to be about, is interoperability with other annotations that do similar things with other libraries. For example, I would like to be able to use the corresponding Jakarta Bean Validation annotations to attach min, max, pattern properties of the generated jsonschema.

My request comes from the fact that I'm already using Jakarta Bean Validation for certain objects that I want to include inside of a CRD. The same applies also to the @Schema annotation from Swagger, which is used to attach arbitrary jsonschema properties. I do not want to include both annotations, which specify the exact same thing.

baloo42 commented 3 months ago

@adriansuarez

The problem with using Jakarta Bean Validation Annotations is, that they have slightly different semantics than constraints in Kubernetes. This was one of the reasons to introduce own annotations. Read more here: https://github.com/fabric8io/kubernetes-client/pull/4298

As far as I know, support for those annotations is not planned at the moment. But this might be reconsidered if the rewritten CRD-Generator v2 is fully in place (release v7).

adriansuarez commented 2 months ago

The problem with using Jakarta Bean Validation Annotations is, that they have slightly different semantics than constraints in Kubernetes. This was one of the reasons to introduce own annotations. Read more here: #4298

As far as I know, support for those annotations is not planned at the moment. But this might be reconsidered if the rewritten CRD-Generator v2 is fully in place (release v7).

@baloo42 I'm not sure what the plan is for v2, but I noticed that the CRD generator currently uses the Jackson JSONSchema module, which its own maintainers do not recommend to use.

I ended up implementing a custom CRD generator that uses victools/jsonschema-generator, and that project has a plugin framework with various modules to enable support for third-party annotations (Jakarta, Swagger, etc).

Maybe a similar approach can be adopted for v2.

shawkins commented 2 months ago

but I noticed that the CRD generator currently uses the Jackson JSONSchema module

The v2 one does also https://github.com/fabric8io/kubernetes-client/tree/main/crd-generator/api-v2/src/main/java/io/fabric8/crdv2/generator

which its own maintainers do not recommend to use

It's been touched on in a couple of other discussions why it was the simplest choice to stick with jackson jsonSchema. If / when we have to move to Jackson v3 support (it's been in the works for 7 years) we can revisit this.

Have a look at the new implementation - it is more straight-forward than when we were consuming the sundrio model and is likely more adaptable to your needs.

Maybe a similar approach can be adopted for v2.

It is certainly possible to make the annotation support extensible - there is some support for that in the JSONSchema module as well. It wouldn't be much work to expose that via the CRD generator. Or if you want to contribute an alternative implementation based upon victools that could be looked at as well.