CRDGenerator: Add support for size constraints

[baloo42]

Is your enhancement related to a problem? Please describe

At the moment the CRDGenerator does not support size constraints:

• minItems
• minProperties
• minLength
• maxItems
• maxProperties
• maxLength

Describe the solution you'd like

A new annotation @Size allows to define min and/or max constraints on a field or its accessors:

@Target({ ElementType.ANNOTATION_TYPE, ElementType.FIELD, ElementType.METHOD })
@Retention(RetentionPolicy.RUNTIME)
public @interface Size {
  long min() default 0;
  long max() default Long.MAX_VALUE;
}

The CRDGenerator detects this annotation and adds JSONSchema constraints dependending on the java type:

Java Type	Constraints
String	minLength maxLength
Array/List	minItems maxItems
Map	minProperties maxProperties

If the annotation is used on an unsupported type, a warning message is logged.

Describe alternatives you've considered

No response

Additional context

Size constraints become more important if someone wants to use CEL Validation rules on fields of a CRD. Because of the cost calculations of a CEL rule, constraints on strings/lists/maps are often only allowed if limits are defined.

[rohanKanojia]

@andreaTP : Could you please share your thoughts on this one? Do you think this could be a good addition to CRD Generator?

[andreaTP]

Hi @baloo42 and thanks a lot for this proposal.

This intersects with the discussion around CEL and validation, I would prefer to have a(shared) understanding of the final target from this effort before commenting on (perceived?)"sub-tasks" like this one.

Can you please come up with a design proposal along with a mocked/dummy dev-UX sample?

[baloo42]

This intersects with the discussion around CEL and validation, I would prefer to have a(shared) understanding of the final target from this effort before commenting on (perceived?)"sub-tasks" like this one.

In my opinion it is independent of CEL/Kubernetes Validation Rules: minLength/maxLength, minItems/maxItems, minProperties/maxProperties are JSON Schema / OpenAPI validation constraints and can be useful even without the combination with Kubernetes Validation Rules. If we look at other similar projects like Spring-Doc / swagger-core, they implemented such constraints by detecting the JSR-303 annotation @Size and adding the appropriate constraints. We should do the same but as previously decided with own annotations:

If a field or one of its accessors is annotated with io.fabric8.generator.annotation.Size

public class ExampleSpec {
  @Size(min = 1, max = 3)
  String stringWithLowerAndUpperLimits;
  @Size(min = 1, max = 3)
  List<String> listWithLowerAndUpperLimits;
  @Size(min = 1, max = 3)
  String[] arrayWithLowerAndUpperLimits;
  @Size(min = 1, max = 3)
  Map<String, String> mapWithLowerAndUpperLimits;
}

The field will have the minLength/maxLength or minItems/maxItems or minProperties/maxProperties properties in the generated CRD, such as:

          spec:
            properties:
              stringWithLowerAndUpperLimits:
                maxLength: 3
                minLength: 1
                type: string
              listWithLowerAndUpperLimits:
                items:
                  type: string
                maxItems: 3
                minItems: 1
                type: array
              arrayWithLowerAndUpperLimits:
                items:
                  type: string
                maxItems: 3
                minItems: 1
                type: array
              mapWithLowerAndUpperLimits:
                additionalProperties:
                  type: string
                maxProperties: 3
                minProperties: 1
                type: object
            type: object

see PoC: https://github.com/baloo42/kubernetes-client/pull/4

And what about the relation to Kubernetes Validation rules?

Size constraints become more important if someone wants to use CEL Validation rules on fields of a CRD. Because of the cost calculations of a CEL rule, constraints on strings/lists/maps are often only allowed if limits are defined.

--> see https://kubernetes.io/blog/2022/09/23/crd-validation-rules-beta/#runtime-cost-limits-for-crd-validation-rules

In addition to the estimated cost limit, CEL keeps track of actual cost while evaluating a CEL expression and will halt execution of the expression if a limit is exceeded.

With the estimated cost limit already in place, the runtime cost limit is rarely encountered. But it is possible. For example, it might be encountered for a large resource composed entirely of a single large list and a validation rule that is either evaluated on each element in the list, or traverses the entire list.

CRD authors can ensure the runtime cost limit will not be exceeded in much the same way the estimated cost limit is avoided: by setting maxItems, maxProperties and maxLength on array, map and string types.

[andreaTP]

OpenAPI validation constraints

right, thanks for pointing this out, in this case we would need to support more constructs, e.g.: multipleOf, uniqueItems, exclusiveMaximum etc. do you mind crafting a comprehensive list?

Thanks a lot for the POC and the rest, highly useful!

On top of my mind:

• probably we should consider a better naming other than Size or figure out a little hierarchy of validators
• those things are useful even outside the generator, options are:
  • a separate(optional?) validation module
  • baked into the core models

cc. @shawkins and @manusa

[baloo42]

right, thanks for pointing this out, in this case we would need to support more constructs, e.g.: multipleOf, uniqueItems, exclusiveMaximum etc. do you mind crafting a comprehensive list?

No problem, I will create a new issue with an overview. There is already an outdated issue #3768. I think this can be closed afterwards.

Notes:

• uniqueItems is not supported by Kubernetes.
• multipleOf: https://swagger.io/docs/specification/data-models/data-types/#multipleOf
• exclusiveMinimum / exclusiveMaximum could be implemented by extending the existing @Max and @Min annotations. See https://swagger.io/docs/specification/data-models/data-types/#range

probably we should consider a better naming other than Size

Why is the name Size bad? (I like it, because it results in the same behaviour as developers are used to with JSR-303). We could also use something like @Schema and @ArraySchema as it is used in swagger-core. This could be also useful for additional features like x-kubernetes-list-type but requires more annotations and maybe a hierarchy as you said.

or figure out a little hierarchy of validators

In my opinion, we don't have to implement a hierarchy (yet). @Min and @Max define a range where an integer or float is valid. The value to define the limit is of the type float and can be also negative. In contrast, the values ​​for the limits of a @Size constraint can only be positive and an integer.

those things are useful even outside the generator, options are: a separate(optional?) validation module baked into the core models

Can you explain this more in detail?

[andreaTP]

No problem, I will create a new issue with an overview.

Appreciated, thanks 🙏

uniqueItems is not supported by Kubernetes.

mentioning because its extremely interesting the nuance here ( ref ) :

• uniqueItems is supported
• uniqueItems cannot be set to true

🤦‍♂️

Can you explain this more in detail?

Yes, everything we are talking about in this thread is not related to the CRDGenerator but to CRDs in general. We should evaluate placing those new annotations in io.fabric8.kubernetes.model.annotation so that they can be used (to be defined "how") by:

• crd-generator
• java-generator
• plain Java definition of CRDs

Does it make more sense now?

cc. @metacosm as he demonstrated interest in chipping in this discussion.

[baloo42]

mentioning because its extremely interesting the nuance here ( ref ) :

uniqueItems is supported uniqueItems cannot be set to true

Yeah, it's very misleading.

Yes, everything we are talking about in this thread is not related to the CRDGenerator but to CRDs in general. We should evaluate placing those new annotations in io.fabric8.kubernetes.model.annotation so that they can be used (to be defined "how") by:

• crd-generator
• java-generator
• plain Java definition of CRDs Does it make more sense now?

Yes, now it makes sense. I agree with you that the annotations should be stored centrally so that they can be reused. And if I read your comment in https://github.com/fabric8io/kubernetes-client/discussions/5851#discussioncomment-8981167 again, I agree with you too, that this also applies for a "validation module" to implement something like "offline" validation / "client-side" validations.

[stale[bot]]

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!

[baloo42]

Can someone reopen this issue? If we want to go further with seperate annotations, this is the way to go.