feat: support @JsonClassDescription for top-level CRD description (#7375)#7390
feat: support @JsonClassDescription for top-level CRD description (#7375)#7390taole33 wants to merge 3 commits intofabric8io:mainfrom
Conversation
shawkins
left a comment
There was a problem hiding this comment.
Thank you for the PR, it looks very good.
Just wanted to add that it's not required to make changes to the older implementation. You can just add a v2 generator specific test.
|
|
||
| private static final Map<TypeRef, String> COMMON_MAPPINGS = new HashMap<>(); | ||
| public static final String ANNOTATION_JSON_FORMAT = "com.fasterxml.jackson.annotation.JsonFormat"; | ||
| public static final String ANNOTATION_JSON_CLASS_DESCRIPTION = "com.fasterxml.jackson.annotation.JsonClassDescription"; |
There was a problem hiding this comment.
This doesn't appear to be used.
|
@shawkins |
|
@shawkins |
c62c2d0 to
254ec3b
Compare
|
@shawkins
All CI checks should be green now. Thanks for waiting. |
shawkins
left a comment
There was a problem hiding this comment.
This looks great. The only nit would be that the description annotation is used at any level, not just the top-level object.
Could the changelog and a doc entry in https://github.com/fabric8io/kubernetes-client/blob/main/doc/CRD-generator.md#features-cheatsheet be updated to reflect this?
Sure, I’d be happy to update the changelog and documentation. |
…bric8io#7375) Signed-off-by: taole33 <taole33@hotmail.com>
…v2 generator Signed-off-by: taole33 <taole33@hotmail.com>
Signed-off-by: taole33 <taole33@hotmail.com>
254ec3b to
f905bac
Compare
|
@shawkins Thank you for the feedback. I have updated the documentation and the features cheatsheet in CRD-generator.md. While updating the docs, I discovered and fixed a logic issue in AbstractJsonSchema where a class-level description was being overwritten by a null property description when @JsonPropertyDescription was absent. This fix ensures that @JsonClassDescription is correctly preserved for all object types, including nested ones. I have also synchronized this branch with the latest upstream changes. Please let me know if you have any further comments. |
shawkins
left a comment
There was a problem hiding this comment.
Thank you for the latest changes.
While updating the docs, I discovered and fixed a logic issue in AbstractJsonSchema where a class-level description was being overwritten by a null property description when @JsonPropertyDescription was absent.
The logic hadn't needed to consider this case before. The update you made is appropriate and effectively makes the class description the default for the field description. I think this makes sense, but if any of the other reviewers feel otherwise we'll revisit it.
|
Thanks for the approval, @shawkins! Regarding the CI failure in the 'Build' job, it seems unrelated to these changes (appears to be a flaky test). I have analyzed both the Java 11 and Java 21 CI logs. The failure in ConfigRefreshTest occurs within the kubernetes-client core module, which is upstream of the crd-generator module I modified. Since the build failed before even reaching my changes, it appears to be a pre-existing flaky issue in the CI environment. Could someone from the team please re-run the failed jobs? Thank you! |
2162e2f to
d1c1045
Compare
Description
This PR introduces support for the Jackson
@JsonClassDescriptionannotation to provide a top-level description for generated Custom Resource Definitions (CRDs), addressing the request in #7375.Previously, only property-level descriptions via
@JsonPropertyDescriptionwere supported. With this change, bothv1andv2CRD generators can now extract and include the description at the root of the OpenAPI v3 schema when a Custom Resource class is annotated with@JsonClassDescription.Changes
AbstractJsonSchemaincrd-generator-api(v1) andcrd-generator-api-v2to process class-level annotations.crd-generator/testusing a sampleDescribedresource to verify the output YAML across all supported CRD versions.Type of change
Checklist
Fixes #7375