Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Marker docs about required fields are not fully accurate #3675

Closed
ahmetb opened this issue Nov 2, 2023 · 1 comment
Closed

docs: Marker docs about required fields are not fully accurate #3675

ahmetb opened this issue Nov 2, 2023 · 1 comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation.

Comments

@ahmetb
Copy link
Member

ahmetb commented Nov 2, 2023

What broke? What's expected?

In CRD validation documentation (https://book.kubebuilder.io/reference/markers/crd-validation) //+kubebuilder:validation:Required marker is advertised as "specifies that this field is required".

However, it seems for a struct field, empty struct in a YAML manifest (i.e. {}) or for a string field empty string "" bypasses this check.

I think it's somewhat misleading that this validation allows zero values (which the omitempty would've omitted) through just fine.

Reproducing this issue

  1. Use package level //+kubebuilder:validation:Required
  2. Do not specify ",omitempty" on a string field.
  3. kubectl apply --validate=false your custom resource without specifying the field → observe the validation error
  4. specify "" as the value for the string field and re-apply → no error.

KubeBuilder (CLI) Version

3.10.0

PROJECT version

3

Plugin versions

go.kubebuilder.io/v4

Other versions

sigs.k8s.io/controller-runtime v0.16.2
kube-apiserver 1.21.14
kubectl 1.21.14

Extra Labels

/kind documentation

@ahmetb ahmetb added the kind/bug Categorizes issue or PR as related to a bug. label Nov 2, 2023
@k8s-ci-robot k8s-ci-robot added the kind/documentation Categorizes issue or PR as related to documentation. label Nov 2, 2023
@camilamacedo86
Copy link
Member

Hi @ahmetb,

By defining a String spec with "" you are defining a string with an empty value.
So, it does not seem to be an error but the expected behaviour.

In your case scenario, seems that you want ensure that:
a) The String spec exist ( +kubebuilder:validation:Required )
b) If its value is bigger than X size ( +kubebuilder:validation:MinLength )

So you would need to use both.

However, if you think that the description or behaviour should be changed then can you please raise an issue against: https://github.com/kubernetes-sigs/controller-tools.

See that the markers are defined in this project. Kubebuilder uses controller-gen (controller-tools) to perform these validations. Indeed the doc is generated automatically from the descriptions added in controller-tools, see: https://github.com/kubernetes-sigs/controller-tools/blob/f185f4d8e8f1c0a006fee16fbb80c832594a1fcf/pkg/crd/markers/validation.go#L77

Therefore, I am closing this one.
But if you need, please feel free to reopen this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/documentation Categorizes issue or PR as related to documentation.
Projects
None yet
Development

No branches or pull requests

3 participants