feat(crd-generator): introduce @JSONSchema annotation

[MikeEdgar]

Description

Closes #6999

This is a POC to demonstrate the idea behind #6999 (it may also close several other issues related to the CRD generator's capabilities). The new @JSONSchema annotation may be used on a class or property to short-circuit other schema scanning for the target. The exception is when the annotation's implementation is used. In that case, the implementation class will be scanned as normal, then the @JSONSchema annotation attributes will be used to override those from the scan.

The annotation also includes a structural boolean attribute that enables certain handling for structural schemas. This includes copying properties from allOf, anyOf, oneOf, and not to the schema with the annotation, and also removing the restricted attributes without those sub-schemas.

Several of the attributes of the annotation may not actually be valid for a Kube CRD schema. However, the intent here is simply to expose the CRD schema model entirely to give that control to the user.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• Feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change
• Chore (non-breaking change which doesn't affect codebase;
  test, version modification, documentation, etc.)

Checklist

• Code contributed by me aligns with current project license: Apache 2.0
• I Added CHANGELOG entry regarding this change
• I have implemented unit tests to cover my changes
• I have added/updated the javadocs and other documentation accordingly
• No new bugs, code smells, etc. in SonarCloud report
• I tested my code in Kubernetes
• I tested my code in OpenShift

[MikeEdgar]

Still a work-in-progress, needs lots of testing and feedback/comments on whether this seems generally viable :)

[shawkins]

Thanks @MikeEdgar this looks promising.

It does seem highly related to SchemaSwap - at least for the usage of implemenation or type.

SchemaSwap could be altered to work at a field level, but one of the usage models it supports is for Keycloak, and maybe other projects, where the classes are defined in some other module and don't need or want CRD related annotations - so you are forced to define the swaps at a higher level than directly on the intended fields.

cc @andreaTP I know you have mentioned in the past wanted to see a general schema option - did you think of it working vis SchemaSwap or a separate annotation?

[MikeEdgar]

It does seem highly related to SchemaSwap - at least for the usage of implemenation or type.

Yes, I think the implementation attribute would be somewhat similar to SchemaSwap, but as you say the location is potentially different. The swaps are still limited to whatever schema can be obtained from the referenced class(es) using the existing scanning however, correct?

[shawkins]

The swaps are still limited to whatever schema can be obtained from the referenced class(es) using the existing scanning however, correct?

That is correct, you can't specify an arbitrary type in string form. I also forgot to mention SchemaFrom (the localized limited form of SchemaSwap). So instead of:

  @JSONSchema(type = "string")
  private Object customizedType;

You could do either:

  @SchemaFrom(type = String.class)
  private Object customizedType;

or

  @SchemaSwap(originalType = JsonSchemaAnnoSpec.class, targetField = "customizedType", targetType = String.class)
  ...

With a similar comparison for implementation.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
81.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[andreaTP]

Hi @MikeEdgar ! Sorry I completely missed the notification on this one 😓

This proposal is highly related to SchemaSwap and SchemaFrom, have you attempted to implement the required functionality using one of the above?

I know you have mentioned in the past wanted to see a general schema option - did you think of it working vis SchemaSwap or a separate annotation?

I'd be interested in a "general schema option" where we can swap in plain json or yaml without limiting the user to a fully typed interface to be as forward compatible as possible.
I have no strong opinion on SchemaSwap or another annotation, but, this is a big hammer, I'll leave the power to the user and let them inject whatever instead of trying to provide a safe typed access that will require maintenance and updates over time.

[MikeEdgar]

This proposal is highly related to SchemaSwap and SchemaFrom, have you attempted to implement the required functionality using one of the above?

I have not. Ultimately the problem with those annotations is that they redirect to another class/type to perform the same scanning logic, so there is no way to specify the schema attributes not already supported.

I'd be interested in a "general schema option" where we can swap in plain json or yaml without limiting the user to a fully typed interface to be as forward compatible as possible.

I think allowing a JSON or YAML string in the annotation would be a nice feature, but it still needs to be parsed into io.fabric8.kubernetes.api.model.apiextensions.v1.JSONSchemaProps, so the type enforcement would just happen at runtime anyway. The new annotation in this PR could support it with a value property that is mutually exclusive with all other properties on the annotation.

I have no strong opinion on SchemaSwap or another annotation, but, this is a big hammer, I'll leave the power to the user and let them inject whatever instead of trying to provide a safe typed access that will require maintenance and updates over time.

Exactly, the intent with the annotation is to hand the control (big hammer) entirely to the user, with the "exit" of incorporating the default scanning using the implementation attribute. As mentioned earlier, type safety can't really be avoided in the model, so why not expose it in the annotation as well?

[andreaTP]

The new annotation in this PR could support it with a value property that is mutually exclusive with all other properties on the annotation.

Probably better to have a new annotation if a field is going to change the semantics.

so the type enforcement would just happen at runtime anyway

Thanks for explaining! Makes sense!

why not expose it in the annotation as well?

io.fabric8.kubernetes.api.model.apiextensions.v1.JSONSchemaProps is generated by ModelGenerator, in this proposal there is a 1:1 mapping manually defined, if it was my choice(and is not) I'd avoid it as it's a source of additional maintenance.

That said, the proposal and overall design makes sense, maybe let's try to fish for feedback on @manusa

[shawkins]

Ultimately the problem with those annotations is that they redirect to another class/type to perform the same scanning logic, so there is no way to specify the schema attributes not already supported.

Correct me if I'm wrong, but is being proposed (and what is already supported) emcompasses:

• swap to
  • a class / implementation
  • to plain text
• overlay from the annotation (although this is possibly exclusive to the class / implementation mode)
• define at
  • the class level
    • for the whole class
    • transitively applies to a particular class / field, for the case when you cannot modify the object model
  • at the field level

If the overlay behavior is distinct to swapping to a class / implementation, then it would make the most sense to me to put the overlay behavior in the existing SchemaSwap annotation, and add a new annotation(s) only for the swapping to text. I would be good with allowing SchemaSwap to be applied at the field level as well so that the new functionality does not have to also be added to SchemaFrom.