search

kubernetes / kube-openapi

Kubernetes OpenAPI spec generation & serving
Apache License 2.0
319 stars 207 forks source link

Generated enum fails validation when used for CRD creation #395

Open jingyuanliang opened 1 year ago

jingyuanliang commented 1 year ago

https://github.com/kubernetes/kube-openapi/blob/78281498afbbfb6fdabe43b8b6a7898f75892c44/pkg/generators/openapi_test.go#L1618

This shows an example of generated code:

https://github.com/kubernetes/kube-openapi/blob/78281498afbbfb6fdabe43b8b6a7898f75892c44/pkg/generators/openapi_test.go#L1655-L1662

However if we use this code to generate CRD it fails validation. The error message is like:

CustomResourceDefinition.apiextensions.k8s.io "..." is invalid: spec.validation.openAPIV3Schema...default: Unsupported value: "": supported values: "a", "b"
Jefftree commented 1 year ago

/cc @apelisse @jiahuif @alexzielenski

apelisse commented 1 year ago

So you're using the test code to make a CRD and the CRD is invalid? From a very cursory look, that seems normal and expected, but the examples included in the tests should probably be valid. Alternative, we could also detect if the default is not part of the enum at generation time, and fail at that time, that'd probably be better.

jingyuanliang commented 1 year ago

I'm using code generated by kube-openapi with an enum to make a CRD and the CRD can't be made for having an invalid default value, because the default zero value is not one of the enum values. I didn't provide the default value and the generator chose a zero value based on datatype. The example included in tests is not valid for the same reason, and that's why I showcased it using the example in tests.

apelisse commented 1 year ago

Looked at it again. It looks like the value is REQUIRED anyway, the default is kind of forced because the field is not a pointer, so I would say this works as expected. There's not really any other great way to do that.

jingyuanliang commented 1 year ago

Uhm I worked around it by removing the Default: "", line and it worked. Is this "REQUIRED" not enforced?

apelisse commented 1 year ago

Yeah, that's a little weird I agree. Also not entirely sure depending on your exact situation. (though required should ALWAYS be respected tbh).

jingyuanliang commented 1 year ago

Can we have the default value use the first enum value then?

alexzielenski commented 1 year ago

Looked at it again. It looks like the value is REQUIRED anyway, the default is kind of forced because the field is not a pointer, so I would say this works as expected. There's not really any other great way to do that.

IMO the schema is invalid if the default value wouldn’t pass validation itself. The schema generator should throw an error for that

apelisse commented 1 year ago

yeah, I don't think that's wrong. I'm not exactly sure why we have a default here, I need to double check.

k8s-triage-robot commented 9 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

jingyuanliang commented 9 months ago

/remove-lifecycle stale

k8s-triage-robot commented 6 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

jingyuanliang commented 6 months ago

/remove-lifecycle stale

k8s-triage-robot commented 3 months ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

jingyuanliang commented 3 months ago

/remove-lifecycle stale

k8s-triage-robot commented 2 weeks ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

jingyuanliang commented 2 weeks ago

/remove-lifecycle stale