Closed baloo42 closed 3 months ago
@baloo42 thanks as always for those issues, keep them coming!
I like the proposal overall! A few comments:
example
, externalDocs
etc.) I think is a no-brainer to go ahead with this or a similar proposalmultipleOf
) I would like to understand how this will play with #5859 and how we can integrate in the java-generator
tootype
s (e.g. x-kubernetes-list-type
) fields, I think that I'm missing the full picture and I'll need to research a little more, I don't know if we have to interpret and validate/do-something with the values or if it's transparent behaviorembeddedResource
I'm not sure how this plays with the rest, pretty convinced that it has implicationsIn general: The idea comes from the official Java swagger-annotations:
It contains any possible option a Schema-Object allows. This approach follows the same principle but allows only those options which are allowed in Kubernetes.
multipleOf
This is an option to define that a given integer / floating-point number must be the multiple of the another number.
For example:
properties:
example:
type: integer
multipleOf: 2
min: 2
Kubernetes would validate this to allow only the following values for the example
property: 2
, 4
, 6
, 8
...
The CRDGenerator would just use it to generate the Schema like we do with others e.g. pattern
, format
...
how we can integrate in the java-generator too
I think that's something which the java-generator should support only optionally.
Jackson, for example, doesn't need to know multipleOf
to deserialize data. (The value is not changed, the resulting datatype is also the same).
But a possible feature could be to let the user somehow configure additional annotions for such cases, so that custom annotations are generated into the Java code if a field is annoted with such a property:
For example, if the Java-Generator gets as input the following snippet:
properties:
example:
type: integer
multipleOf: 2
And is configured as follows:
annotationMappings:
multipleOf: "com.example.ValidMultipleOf(value={{value}})"
The Java-Generator generates:
class Test {
com.example.ValidMultipleOf(2)
private Integer example;
}
This could be really useful, because many tools would get support out of the box and the decision of choosing the tool is left out to the user. (And doing the actual validation is left to the tool). I think for example at JSR-380, JSR-349, JSR-303 or a cel-java lib.
x-kubernetes-list-type
This property can be used additionally and doesn't change the type or the rest of the schema definiton. It's only an indicator how tools should behave on merging two objects. As such it's not important to implement it specifically in the Java-Generator. But it would also fit into the idea from above.
embeddedResource
This property indicates that an object is an embedded resource. An embedded resource is a value that has apiVersion, kind and metadata fields. They are validated implicitly according to the semantics of the currently running apiserver. It is not necessary to add any additional schema for these field, yet it is possible. This can be combined with PreserveUnknownFields.
See also: https://book.kubebuilder.io/reference/markers/crd-validation
The CRDGenerator could detect it and could validate the given structural schema. But I think that's not important for the first implementation.
@baloo42 I thought a little about this proposal.
I think that the fastest way would be to start merging PRs with separate annotations for each field (i.e. name
, title
, example
), this is how we have done so far, and following the path of least resistance would be the most straightforward way of having progress over this issue.
In this way, we can have scoped discussions when time comes to more "semantically meaningful" field integration. For full disclosure, we are about to start developing on a 7.X branch and we can re-evaluate later on what to do if we feel that a breaking change would improve the user experience.
wdyt?
I don't think it would be the fastest way. The problem is that we can't work on it in parallel, because each PR requires changes at the same places which would result in quite a few merge conflicts. Especially if we reorganize the tests or the CRDGenerator itself every time a little bit. (That's why I created #5896 first, to have some easy to use structural tests which can be extended in parallel)
At the same time, most of the additions are no-brainers and easy to implement. In my opinion it's more a question on how we design it aka which strategy we want to follow.
Can we discuss the overall strategy in #5859? I don't think we should talk in every PR about the same topics. I would rather create some more issues and describe the overall solution more in detail before we start implementing it.
Because then, it might be possible to implement a whole feature set at once...
Can we discuss the overall strategy in https://github.com/fabric8io/kubernetes-client/issues/5859?
Agreed, let's move this discussion there.
This issue has been automatically marked as stale because it has not had any activity since 90 days. It will be closed if no further activity occurs within 7 days. Thank you for your contributions!
Is your enhancement related to a problem? Please describe
At the moment the CRDGenerator does not support some schema properties like
format
,example
,externalDocs
(see #5859).Describe the solution you'd like
Introduce
@Schema
annotation which allows to define such leftovers:The
@Schema
annotation always wins if there are other annotations defined, which would result in the same property.For example:
Describe alternatives you've considered
This approach avoids the explosion of annotations in the first place. But additional annotations can be easily added in the future as it would not be a breaking change.
For example, the
@ListType
could be added later:Additional context