KAL does not detect zero value invalid via string validation markers on underlying struct

[sbueringer]

Wasn't sure how to phrase the issue title, but it's easier to understand with an example:

	// token is used for establishing bidirectional trust between nodes and control-planes.
	// Used for joining nodes in the cluster.
	// +required
	Token BootstrapTokenString `json:"token,omitempty,omitzero"`

...

// +kubebuilder:validation:Type=string
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=23
type BootstrapTokenString struct {
	ID     string `json:"-"`
	Secret string `json:"-"`
}

https://github.com/kubernetes-sigs/cluster-api/blob/release-1.11/api/bootstrap/kubeadm/v1beta2/kubeadm_types.go#L951-L962

Schema:

                        token:
                          maxLength: 23
                          minLength: 1
                          type: string

Finding:

api/bootstrap/kubeadm/v1beta2/kubeadm_types.go:427:2: requiredfields: field Token has a valid zero value ({"": "", "": ""}) and should be a pointer. (kubeapilinter)
        Token BootstrapTokenString `json:"token,omitempty,omitzero"`
        ^

[JoelSpeed]

Yeah so this is a feature gap/bug depending on how you look at it.

We currently don't have any special handling for types that have custom marshalling (this has it's own UnmarshalJSON() right?

I think we will need to work on the serialization checker to be able to understand when a struct is unmarshalled differently

Do you happen to have the generated schema for this?

Oh and I've noticed that the code for determining the string version of the zero value will also need to be updated, it shouldn't be counting fields with - in their json tag 🤔

[sbueringer]

We currently don't have any special handling for types that have custom marshalling (this has it's own UnmarshalJSON() right?

Yes. Edited the issue description to add a link to the struct

Do you happen to have the generated schema for this?

Yes. Added to the issue description

I think we will need to work on the serialization checker to be able to understand when a struct is unmarshalled differently

I think maybe not to handle cases like this specifically. We have a marker on the struct that tells controller-gen this is a string. I think this could be enough to just handle this struct field like a string field?

(Probably not as easy as that, but looking at it naively: string, MaxLength, MinLength should be enough to tell that the zero value is not valid here)

[JoelSpeed]

I think maybe not to handle cases like this specifically. We have a marker on the struct that tells controller-gen this is a string. I think this could be enough to just handle this struct field like a string field?

This is kind of what I was thinking. We need to look for format markers on types and handle them with the correct bounding checks

[saschagrunert]

Trying to approach a fix in #184

[saschagrunert]

/assign