KAL incorrectly recommends making a *[]corev1.Taint field with MinItems=0 a non-pointer

[sbueringer]

We found this while using KAL in Cluster API, xref: kubernetes-sigs/cluster-api#12299 (comment)

The field looks like this

	// +optional
	// +kubebuilder:validation:MinItems=0
	// +kubebuilder:validation:MaxItems=100
	Taints *[]corev1.Taint `json:"taints,omitempty"`

And we get this finding

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go:304:2: optionalfields: field Taints is optional but the underlying type does not need to be a pointer. The pointer should be removed. (kubeapilinter)
        Taints *[]corev1.Taint `json:"taints,omitempty"`
        ^

[JoelSpeed]

A zero length list is not compatible with core API types (those built with API machinery/API servers, think K/K or aggregated API servers) since the protobuf serialization that is used cannot distinguish between an unset list and an empty list in the same way Go can.

Officially, the API conventions recommend (note this is being discussed presently) not to ever make a distinction between empty and unset lists.

However, because validation in CRDs is implemented in openapi, and the storage is unstructured json, this can work for CRDs.

In the future, I think we will have a need for a minlength implementation that requires all strings, lists and maps to have the correct markers for min length/items/properties respectively. I think in the configuration for this, we should default to not allowing zero length as a minimum for each of these types, but allow that to be configured to be ignored, which would suit the custom resource use case.

We will also need to update the serialization checker implementation (see #131) to allow for zero length lists/maps, as it doesn't presently

[sbueringer]

The only case for 0-length arrays I'm aware of is taints in kubeadm:

• unset means set default taints (i.e. control plane taint on control plane Nodes, no taints for workers)
• [] means set no taints

To generalize. Basically if you have an array field where the behavior of unset (~ default) is not identical to [].

I think if we would say that this is not allowed or recommended for CRDs it would be good to have an alternative way to model this that we would recommend instead.

Just because the same doesn't work with builtin types does not seem reason enough to discourage it for CRDs. But I assume there are other reasons (I'll wait for the conclusion of that discussion)